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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 2/4/26 7:50 AM, Jan Beulich wrote:
> On 22.01.2026 17:47, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/include/asm/sbi.h
> > +++ b/xen/arch/riscv/include/asm/sbi.h
> > @@ -29,6 +29,10 @@
> >   
> >   #define SBI_EXT_BASE                    0x10
> >   #define SBI_EXT_RFENCE                  0x52464E43
> > +#define SBI_EXT_TIME                    0x54494D45
> > +
> > +/* SBI function IDs for TIME extension */
> > +#define SBI_EXT_TIME_SET_TIMER          0x0
> Nit: Do you really mean to have the time extension IDs above ...
>
> >   /* SBI function IDs for BASE extension */
> >   #define SBI_EXT_BASE_GET_SPEC_VERSION   0x0
> ... the base extension ones?
I will move it after SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID to be aligned with 
how
extensions are declared.

> > @@ -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.
> > + * If the supervisor wishes to clear the timer interrupt without scheduling the
> > + * next timer event, it can either request a timer interrupt infinitely far
> > + * into the future (i.e., (uint64_t)-1), or it can instead mask the timer
> > + * interrupt by clearing sie.STIE CSR bit.
> > + * The stime_value parameter represents absolute time measured in ticks.
> > + *
> > + * This SBI call returns 0 upon success or an implementation specific negative
> > + * error code.
> > + */
> > +extern int (*sbi_set_timer)(uint64_t stime_value);
> __read_mostly or even __ro_after_init?
I will add __ro_after_init to be in sync with sbi_rfence.

> > @@ -326,6 +358,14 @@ int __init sbi_init(void)
> >               sbi_rfence = sbi_rfence_v02;
> >               printk("SBI v0.2 RFENCE extension detected\n");
> >           }
> > +
> > +        if ( sbi_probe_extension(SBI_EXT_TIME) > 0 )
> > +        {
> > +            sbi_set_timer = sbi_set_timer_v02;
> > +            printk("SBI v0.2 TIME extension detected\n");
> Is this really relevant to log especially in release builds? IOW can this at
> least be downgraded to dprintk()?
Probably not, it could be useful for debugging to understand what kind and 
version
of extension is used. I am okay with using of dprintk().

Thanks.

~ Oleksii