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

Oleksii Kurochko posted 16 patches 3 days, 14 hours ago
[PATCH v2 13/16] xen/riscv: implement reprogram_timer() via SBI
Posted by Oleksii Kurochko 3 days, 14 hours ago
Implement reprogram_timer() on RISC-V using the standard SBI timer call.

The privileged architecture only defines machine-mode timer interrupts
(using mtime/mtimecmp). Therefore, timer services for S/HS/VS mode must
be provided by M-mode via SBI calls. SSTC (Supervisor-mode Timer Control)
is optional and is not supported on the boards available to me, so the
only viable approach today is to program the timer through SBI.

reprogram_timer() enables/disables the supervisor timer interrupt and
programs the next timer deadline using sbi_set_timer(). If the SBI call
fails, the code panics, because sbi_set_timer() is expected to return
either 0 or -ENOSUPP (this has been stable from early OpenSBI versions to
the latest ones). The SBI spec does not define a standard negative error
code for this call, and without SSTC there is no alternative method to
program the timer, so the SBI timer call must be available.

reprogram_timer() currently returns int for compatibility with the
existing prototype. While it might be cleaner to return bool, keeping the
existing signature avoids premature changes in case sbi_set_timer() ever
needs to return other values (based on which we could try to avoid
panic-ing) in the future.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v2:
 - Add TODO comment above sbi_set_timer() call.
 - Update the commit message.
---
 xen/arch/riscv/stubs.c |  5 -----
 xen/arch/riscv/time.c  | 43 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
index 1f0add97b361..cb7546558b8e 100644
--- a/xen/arch/riscv/stubs.c
+++ b/xen/arch/riscv/stubs.c
@@ -21,11 +21,6 @@ nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
 
 /* time.c */
 
-int reprogram_timer(s_time_t timeout)
-{
-    BUG_ON("unimplemented");
-}
-
 void send_timer_event(struct vcpu *v)
 {
     BUG_ON("unimplemented");
diff --git a/xen/arch/riscv/time.c b/xen/arch/riscv/time.c
index 2c7af0a5d63b..f021ceab8ec4 100644
--- a/xen/arch/riscv/time.c
+++ b/xen/arch/riscv/time.c
@@ -7,6 +7,9 @@
 #include <xen/time.h>
 #include <xen/types.h>
 
+#include <asm/csr.h>
+#include <asm/sbi.h>
+
 unsigned long __ro_after_init cpu_khz; /* CPU clock frequency in kHz. */
 uint64_t __ro_after_init boot_clock_cycles;
 
@@ -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));
+
+        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));
+
+    /*
+     * 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.
+     *
+     *       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 )
-- 
2.52.0
Re: [PATCH v2 13/16] xen/riscv: implement reprogram_timer() via SBI
Posted by Teddy Astie 1 day, 8 hours ago
Le 22/01/2026 à 17:49, Oleksii Kurochko a écrit :
> Implement reprogram_timer() on RISC-V using the standard SBI timer call.
> 
> The privileged architecture only defines machine-mode timer interrupts
> (using mtime/mtimecmp). Therefore, timer services for S/HS/VS mode must
> be provided by M-mode via SBI calls. SSTC (Supervisor-mode Timer Control)
> is optional and is not supported on the boards available to me, so the
> only viable approach today is to program the timer through SBI.
> 
> reprogram_timer() enables/disables the supervisor timer interrupt and
> programs the next timer deadline using sbi_set_timer(). If the SBI call
> fails, the code panics, because sbi_set_timer() is expected to return
> either 0 or -ENOSUPP (this has been stable from early OpenSBI versions to
> the latest ones). The SBI spec does not define a standard negative error
> code for this call, and without SSTC there is no alternative method to
> program the timer, so the SBI timer call must be available.
> 
> reprogram_timer() currently returns int for compatibility with the
> existing prototype. While it might be cleaner to return bool, keeping the
> existing signature avoids premature changes in case sbi_set_timer() ever
> needs to return other values (based on which we could try to avoid
> panic-ing) in the future.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in v2:
>   - Add TODO comment above sbi_set_timer() call.
>   - Update the commit message.
> ---
>   xen/arch/riscv/stubs.c |  5 -----
>   xen/arch/riscv/time.c  | 43 ++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 43 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
> index 1f0add97b361..cb7546558b8e 100644
> --- a/xen/arch/riscv/stubs.c
> +++ b/xen/arch/riscv/stubs.c
> @@ -21,11 +21,6 @@ nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
>   
>   /* time.c */
>   
> -int reprogram_timer(s_time_t timeout)
> -{
> -    BUG_ON("unimplemented");
> -}
> -
>   void send_timer_event(struct vcpu *v)
>   {
>       BUG_ON("unimplemented");
> diff --git a/xen/arch/riscv/time.c b/xen/arch/riscv/time.c
> index 2c7af0a5d63b..f021ceab8ec4 100644
> --- a/xen/arch/riscv/time.c
> +++ b/xen/arch/riscv/time.c
> @@ -7,6 +7,9 @@
>   #include <xen/time.h>
>   #include <xen/types.h>
>   
> +#include <asm/csr.h>
> +#include <asm/sbi.h>
> +
>   unsigned long __ro_after_init cpu_khz; /* CPU clock frequency in kHz. */
>   uint64_t __ro_after_init boot_clock_cycles;
>   
> @@ -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));
> +
> +        return 1;
> +    }

Do disabling the timers interrupt actually stops the timer or just 
prevents Xen from receiving the timer interrupt ?

If it doesn't "stop the timer", we probably would want to swap "enabling 
the timer interrupt" and "setting the timer through SBI" (to avoid 
potentially receiving a timer interrupt between these 2 operations).

Though, it's unclear in SBI specification if the sbi_set_timer touches 
the timer interrupt masking or not (at least it does if you set a timer 
too far in the future).

> +
> +    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));
> +
> +    /*
> +     * 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.
> +     *
> +     *       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 )



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech