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

[Jan Beulich <jbeulich@suse.com>]

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).

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?

> +    /*
> +     * 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?

Jan

> +     *       This would also reduce reliance on a specific SBI implementation.
> +     *       For example, it is not ideal to panic() if sbi_set_timer() returns
> +     *       a non-zero value. Currently it can return 0 or -ENOSUPP, and
> +     *       without SSTC we still need an implementation because only the
> +     *       M-mode timer is available, and it can only be programmed in
> +     *       M-mode.
> +     */
> +    if ( (rc = sbi_set_timer(deadline)) )
> +        panic("%s: timer wasn't set because: %d\n", __func__, rc);
> +
> +    return 1;
> +}
> +
>  void __init preinit_xen_time(void)
>  {
>      if ( acpi_disabled )