Re: [PATCH v1 13/15] xen/riscv: implement reprogram_timer() using SBI

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 1/12/26 4:24 PM, Jan Beulich wrote:
> On 24.12.2025 18:03, Oleksii Kurochko wrote:
> > @@ -39,6 +43,33 @@ 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));
> > +
> > +        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));
> Still learning RISC-V, so question for my understanding: Even if the timeout
> is short enough to expire before the one SIE bit will be set, the interrupt
> will still occur (effectively immediately)? (Else the bit may need setting
> first.)
The interrupt will become pending first (when mtime >= mtimecmp or
mtime >= CSR_STIMECMP in case of SSTC) and then fire immediately once
|SIE.STIE |(and global|SIE|) are enabled.

> > +    if ( (rc = sbi_set_timer(deadline)) )
> > +        panic("%s: timer wasn't set because: %d\n", __func__, rc);
> Hmm, if this function ends up being used from any guest accessible path (e.g.
> a hypercall), such panic()-ing better shouldn't be there.
I don't have such use cases now and I don't expect that guest should use
this function.

> > +    return 1;
> > +}
> Finally, before we get yet another instance of this de-fact boolean function:
> Wouldn't we better globally switch it to be properly "bool" first?
We could do that, I will prepare a separate patch in the next version of
this patch series.

Thanks.

~ Oleksii