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.0On 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 )
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
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
On 1/25/26 12:13 AM, Teddy Astie wrote:
> 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 ?
According to OpenSBI spec:
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.
It seems like it never stops it even if to use OpenSBI call with -1 arguemnt.
>
> 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).
Considering how it is implemented:
void sbi_timer_event_start(u64 next_event)
{
...
} else if (timer_dev && timer_dev->timer_event_start) {
timer_dev->timer_event_start(next_event);
csr_clear(CSR_MIP, MIP_STIP);
}
csr_set(CSR_MIE, MIP_MTIP);
}
It will clear timer pending bit even it was set before. So move this:
+ /* Enable timer */
+ csr_set(CSR_SIE, BIT(IRQ_S_TIMER, UL));
after sbi_set_timer() should work.
As an option we could just use sbi_set_timer(UINT64_MAX) instead of
" csr_clear(CSR_SIE, BIT(IRQ_S_TIMER, UL));" to "stop" a timer.
But first one option looks better for me.
~ Oleksii
>
>> +
>> + 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
>
>
© 2016 - 2026 Red Hat, Inc.