Re: [PATCH v2 13/16] xen/riscv: implement reprogram_timer() via SBI

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 2/4/26 11:39 AM, Jan Beulich wrote:
> On 22.01.2026 17:47, Oleksii Kurochko wrote:
> > @@ -40,6 +43,46 @@ static void __init preinit_dt_xen_time(void)
> >       cpu_khz = rate / 1000;
> >   }
> >   
> > +int reprogram_timer(s_time_t timeout)
> > +{
> > +    uint64_t deadline, now;
> > +    int rc;
> > +
> > +    if ( timeout == 0 )
> > +    {
> > +        /* Disable timers */
> > +        csr_clear(CSR_SIE, BIT(IRQ_S_TIMER, UL));
> For here and below: Is it guaranteed that the SIE bit is writable? The 
> privileged
> spec looks to have provisions for the case that it isn't (together with the
> corresponding SIP bit).
My understanding is that yes if S-mode is present.

> As to the comment - why plural here, when ...
>
> > +        return 1;
> > +    }
> > +
> > +    deadline = ns_to_ticks(timeout) + boot_clock_cycles;
> > +    now = get_cycles();
> > +    if ( deadline <= now )
> > +        return 0;
> > +
> > +    /* Enable timer */
> > +    csr_set(CSR_SIE, BIT(IRQ_S_TIMER, UL));
> ... it's singular here? Also in both cases, isn't it the timer interrupt you
> enable, not the timer itself?
It is timer interrupt. I will correct the comments.


> > +    /*
> > +     * TODO: When the SSTC extension is supported, it would be preferable to
> > +     *       use the supervisor timer registers directly here for better
> > +     *       performance, since an SBI call and context switch would no longer
> > +     *       be required.
> I think you mean a mode switch here, not a context one?
Right, mode switch is better.

~ Oleksii