On 1/8/26 11:28 AM, Jan Beulich wrote:
On 24.12.2025 18:03, Oleksii Kurochko wrote:
--- a/xen/arch/riscv/include/asm/vtimer.h
+++ b/xen/arch/riscv/include/asm/vtimer.h
@@ -22,4 +22,6 @@ void vcpu_timer_destroy(struct vcpu *v);
int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config);
+void vtimer_set_timer(struct vtimer *t, const uint64_t ticks);
+
#endif /* ASM__RISCV__VTIMER_H */
diff --git a/xen/arch/riscv/vtimer.c b/xen/arch/riscv/vtimer.c
index 5ba533690bc2..99a0c5986f1d 100644
--- a/xen/arch/riscv/vtimer.c
+++ b/xen/arch/riscv/vtimer.c
@@ -1,6 +1,8 @@
/* SPDX-License-Identifier: GPL-2.0-only */
+#include <xen/domain.h>
Is this really needed, when ...
#include <xen/sched.h>
... this is already there?
With the way how includes look in xen/sched.h - no.
+#include <xen/time.h>
Don't you mean xen/timer.h here?
You are right, it should be xen/timer.h as set_timer(), stop_timer() and
migrate_timer()
are from xen/timer.h.
@@ -15,7 +17,9 @@ int domain_vtimer_init(struct domain *d, struct
xen_arch_domainconfig *config)
static void vtimer_expired(void *data)
{
- panic("%s: TBD\n", __func__);
+ struct vtimer *t = data;
Pointer-to-const please.
@@ -37,3 +41,27 @@ void vcpu_timer_destroy(struct vcpu *v)
kill_timer(&v->arch.vtimer.timer);
}
+
+void vtimer_set_timer(struct vtimer *t, const uint64_t ticks)
+{
+ s_time_t expires = ticks_to_ns(ticks - boot_clock_cycles);
boot_clock_cycles is known to just Xen. If the guest provided input is an
absolute value, how would that work across migration? Doesn't there need
to be a guest-specific bias instead?
I think that I don't understand fully your questions, but it sounds like it is
a job
for htimedelta register.
+ vcpu_unset_interrupt(t->v, IRQ_VS_TIMER);
+
+ /*
+ * According to the RISC-V sbi 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.
+ */
And SBI is the only way to set the expiry value? No CSR access? (Question
also concerns the unconditional vcpu_unset_interrupt() above.)
If we don't have SSTC extension support then I suppose yes, as CSR_MI{E,P} could
be accessed only from M-mode:
(code from OpenSBI)
void sbi_timer_event_start(u64 next_event)
{
sbi_pmu_ctr_incr_fw(SBI_PMU_FW_SET_TIMER);
/**
* Update the stimecmp directly if available. This allows
* the older software to leverage sstc extension on newer hardware.
*/
if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
SBI_HART_EXT_SSTC)) {
#if __riscv_xlen == 32
csr_write(CSR_STIMECMP, next_event & 0xFFFFFFFF);
csr_write(CSR_STIMECMPH, next_event >> 32);
#else
csr_write(CSR_STIMECMP, next_event);
#endif
} 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);
}
+ if ( ticks == ((uint64_t)~0ULL) )
Nit: With the cast you won't need the ULL suffix.
+ {
+ stop_timer(&t->timer);
+
+ return;
+ }
+
+ set_timer(&t->timer, expires);
See the handling of VCPUOP_set_singleshot_timer for what you may want to
do if the expiry asked for is (perhaps just very slightly) into the past.
I got an idea why we want to check if "expires" already expired, but ...
There you'll also find a use of migrate_timer(), which you will want to
at least consider using here as well.
... I don't get why we want to migrate timer before set_timer() here.
Could you please explain that?
Thanks.
~ Oleksii
