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 <[email protected]>
---
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
