Re: [PATCH v3 12/16] xen/riscv: introduce sbi_set_timer()

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 2/11/26 4:03 PM, Jan Beulich wrote:
> On 09.02.2026 17:52, Oleksii Kurochko wrote:
> > Introduce a function pointer for sbi_set_timer(), since different OpenSBI
> > versions may implement the TIME extension with different extension IDs
> > and/or function IDs.
> >
> > If the TIME extension is not available, fall back to the legacy timer
> > mechanism. This is useful when Xen runs as a guest under another Xen,
> > because the TIME extension is not currently virtualised and therefore
> > will not appear as available.
> > Despite of the fact that sbi_set_timer_v01 is introduced and used as
> > fall back, SBI v0.1 still isn't fully supported (with the current SBI
> > calls usage, sbi_rfence_v01 should be introduced too), so panic()
> > in sbi_init() isn't removed.
> >
> > The sbi_set_timer() pointer will be used by reprogram_timer() to program
> > Xen’s physical timer as without SSTC extension there is no any other
> > option except SBI call to do that as only M-timer is available for us.
> >
> > Use dprintk() for all the cases to print that a speicifc SBI extension
> > is available as it isn't really necessary in case of release builds.
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> with one further request:
>
> > @@ -134,6 +138,20 @@ int sbi_remote_hfence_gvma(const cpumask_t *cpu_mask, 
> > vaddr_t start,
> >   int sbi_remote_hfence_gvma_vmid(const cpumask_t *cpu_mask, vaddr_t start,
> >                                   size_t size, unsigned long vmid);
> >   
> > +/*
> > + * Programs the clock for next event after stime_value time. This function also
> > + * clears the pending timer interrupt bit.
> "after stime_value time" reads as if this was a relative input, stime_value 
> units
> of time need to pass until the event. Iirc values passed are absolute, though.
> Furthermore it was my understanding that the interrupt being raised is dependent
> upon clock >= value, not clock > value (where the latter is what "after" means
> when taken to apply to an absolute value).
Interesting that sbi_set_timer() has different description for OpenSBI v0.1 and 
v0.2.
What you see in the comment it is what v0.1 tells, but v0.2 tells that:
  
  "Programs the clock for next event after stime_value time.*stime_value is in absolute time.* This function must clear the pending timer interrupt bit as well."
"stime_value is in absolute time" has been added in v0.2. I'll update the 
comment for
a clarity.

Thanks.

~ Oleksii