Introduce a virtual timer structure along with functions to initialize
and destroy the virtual timer.

Add a vtimer_expired() function and implement it as a stub, as the timer
and tasklet subsystems are not functional at this stage.

Call vcpu_vtimer_init() in arch_vcpu_create().

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/Makefile             |  1 +
 xen/arch/riscv/domain.c             |  8 ++++--
 xen/arch/riscv/include/asm/domain.h |  4 +++
 xen/arch/riscv/include/asm/vtimer.h | 25 ++++++++++++++++++
 xen/arch/riscv/vtimer.c             | 39 +++++++++++++++++++++++++++++
 5 files changed, 75 insertions(+), 2 deletions(-)
 create mode 100644 xen/arch/riscv/include/asm/vtimer.h
 create mode 100644 xen/arch/riscv/vtimer.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 8863d4b15605..5bd180130165 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -22,6 +22,7 @@ obj-y += traps.o
 obj-y += vmid.o
 obj-y += vm_event.o
 obj-y += vsbi/
+obj-y += vtimer.o
 
 $(TARGET): $(TARGET)-syms
 	$(OBJCOPY) -O binary -S $< $@
diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
index 44387d056546..dd3c237d163d 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -6,6 +6,7 @@
 #include <asm/cpufeature.h>
 #include <asm/csr.h>
 #include <asm/riscv_encoding.h>
+#include <asm/vtimer.h>
 
 static void vcpu_csr_init(struct vcpu *v)
 {
@@ -97,11 +98,14 @@ int arch_vcpu_create(struct vcpu *v)
     if ( is_idle_vcpu(v) )
         return rc;
 
+    if ( (rc = vcpu_vtimer_init(v)) )
+        goto fail;
+
     /*
-     * As the vtimer and interrupt controller (IC) are not yet implemented,
+     * As interrupt controller (IC) is not yet implemented,
      * return an error.
      *
-     * TODO: Drop this once the vtimer and IC are implemented.
+     * TODO: Drop this once IC is implemented.
      */
     rc = -EOPNOTSUPP;
     goto fail;
diff --git a/xen/arch/riscv/include/asm/domain.h b/xen/arch/riscv/include/asm/domain.h
index a0ffbbc09c6f..be7ddaff30e7 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -8,6 +8,7 @@
 #include <public/hvm/params.h>
 
 #include <asm/p2m.h>
+#include <asm/vtimer.h>
 
 struct vcpu_vmid {
     uint64_t generation;
@@ -52,6 +53,9 @@ struct arch_vcpu
     struct cpu_info *cpu_info;
     void *stack;
 
+    struct vtimer vtimer;
+    bool vtimer_initialized;
+
     /* CSRs */
     register_t hstatus;
     register_t hedeleg;
diff --git a/xen/arch/riscv/include/asm/vtimer.h b/xen/arch/riscv/include/asm/vtimer.h
new file mode 100644
index 000000000000..a2ca704cf0cc
--- /dev/null
+++ b/xen/arch/riscv/include/asm/vtimer.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * (c) 2023-2024 Vates
+ */
+
+#ifndef ASM__RISCV__VTIMER_H
+#define ASM__RISCV__VTIMER_H
+
+#include <xen/timer.h>
+
+struct domain;
+struct vcpu;
+struct xen_arch_domainconfig;
+
+struct vtimer {
+    struct vcpu *v;
+    struct timer timer;
+};
+
+int vcpu_vtimer_init(struct vcpu *v);
+void vcpu_timer_destroy(struct vcpu *v);
+
+int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config);
+
+#endif /* ASM__RISCV__VTIMER_H */
diff --git a/xen/arch/riscv/vtimer.c b/xen/arch/riscv/vtimer.c
new file mode 100644
index 000000000000..5ba533690bc2
--- /dev/null
+++ b/xen/arch/riscv/vtimer.c
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <xen/sched.h>
+
+#include <public/xen.h>
+
+#include <asm/vtimer.h>
+
+int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config)
+{
+    /* Nothing to do at the moment */
+
+    return 0;
+}
+
+static void vtimer_expired(void *data)
+{
+    panic("%s: TBD\n", __func__);
+}
+
+int vcpu_vtimer_init(struct vcpu *v)
+{
+    struct vtimer *t = &v->arch.vtimer;
+
+    t->v = v;
+    init_timer(&t->timer, vtimer_expired, t, v->processor);
+
+    v->arch.vtimer_initialized = true;
+
+    return 0;
+}
+
+void vcpu_timer_destroy(struct vcpu *v)
+{
+    if ( !v->arch.vtimer_initialized )
+        return;
+
+    kill_timer(&v->arch.vtimer.timer);
+}
-- 
2.52.0

On 24.12.2025 18:03, Oleksii Kurochko wrote:
> Introduce a virtual timer structure along with functions to initialize
> and destroy the virtual timer.
> 
> Add a vtimer_expired() function and implement it as a stub, as the timer
> and tasklet subsystems are not functional at this stage.

Shouldn't those pieces of infrastructure be made work then first? I also
don't quite understand why the subsystems not being functional prevents
the function to be implemented as far as possible. Most if not all
functions you need from both subsystems should be available, for living
in common code.

> --- a/xen/arch/riscv/include/asm/domain.h
> +++ b/xen/arch/riscv/include/asm/domain.h
> @@ -8,6 +8,7 @@
>  #include <public/hvm/params.h>
>  
>  #include <asm/p2m.h>
> +#include <asm/vtimer.h>
>  
>  struct vcpu_vmid {
>      uint64_t generation;
> @@ -52,6 +53,9 @@ struct arch_vcpu
>      struct cpu_info *cpu_info;
>      void *stack;
>  
> +    struct vtimer vtimer;
> +    bool vtimer_initialized;

Assuming the field is really needed (see remark further down), why is this
not part of the struct?

> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/vtimer.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * (c) 2023-2024 Vates
> + */
> +
> +#ifndef ASM__RISCV__VTIMER_H
> +#define ASM__RISCV__VTIMER_H
> +
> +#include <xen/timer.h>
> +
> +struct domain;
> +struct vcpu;

I don't think this one is needed, as long as you have ...

> +struct xen_arch_domainconfig;
> +
> +struct vtimer {
> +    struct vcpu *v;

... this. Question is why this is here: You should be able to get hold of the
struct vcpu containing a struct vtimer using container_of().

> --- /dev/null
> +++ b/xen/arch/riscv/vtimer.c
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include <xen/sched.h>
> +
> +#include <public/xen.h>
> +
> +#include <asm/vtimer.h>
> +
> +int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config)
> +{
> +    /* Nothing to do at the moment */
> +
> +    return 0;
> +}

The function has no caller and does nothing - why do we need it?

> +static void vtimer_expired(void *data)
> +{
> +    panic("%s: TBD\n", __func__);
> +}
> +
> +int vcpu_vtimer_init(struct vcpu *v)
> +{
> +    struct vtimer *t = &v->arch.vtimer;
> +
> +    t->v = v;
> +    init_timer(&t->timer, vtimer_expired, t, v->processor);
> +
> +    v->arch.vtimer_initialized = true;

init_timer() has specific effects (like setting t->function to non-NULL
and t->status to other than TIMER_STATUS_invalid). Can't you leverage
that instead of having a separate boolean? (Iirc we do so elsewhere.)

Jan

On 1/7/26 4:21 PM, Jan Beulich wrote:
> On 24.12.2025 18:03, Oleksii Kurochko wrote:
>> Introduce a virtual timer structure along with functions to initialize
>> and destroy the virtual timer.
>>
>> Add a vtimer_expired() function and implement it as a stub, as the timer
>> and tasklet subsystems are not functional at this stage.
> Shouldn't those pieces of infrastructure be made work then first?

It could be an option; it’s just not really critical until a guest is running.

I actually considered adding this in the current patch series, but decided to
introduce it later to avoid making the series too large. (On the other hand, it
would be only one additional patch, IIRC)

> I also
> don't quite understand why the subsystems not being functional prevents
> the function to be implemented as far as possible. Most if not all
> functions you need from both subsystems should be available, for living
> in common code.

I chose the wrong words here; this is not the main (that some subsystems isn't
fully functional) reason why I’m using a stub here instead of something functional.

Basically, implementing this requires vcpu_kick() and vcpu_set_interrupt(),
which are introduced later in this patch series.

As an alternative, I could drop vtimer_expired() and the related code from this
patch and reintroduce them after vcpu_kick() and vcpu_set_interrupt() are
available.

>
>> --- a/xen/arch/riscv/include/asm/domain.h
>> +++ b/xen/arch/riscv/include/asm/domain.h
>> @@ -8,6 +8,7 @@
>>   #include <public/hvm/params.h>
>>   
>>   #include <asm/p2m.h>
>> +#include <asm/vtimer.h>
>>   
>>   struct vcpu_vmid {
>>       uint64_t generation;
>> @@ -52,6 +53,9 @@ struct arch_vcpu
>>       struct cpu_info *cpu_info;
>>       void *stack;
>>   
>> +    struct vtimer vtimer;
>> +    bool vtimer_initialized;
> Assuming the field is really needed (see remark further down), why is this
> not part of the struct?

Agree, it would be better to have it as a part of struct vtimer if it will
be used in future.

>
>> --- /dev/null
>> +++ b/xen/arch/riscv/include/asm/vtimer.h
>> @@ -0,0 +1,25 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * (c) 2023-2024 Vates
>> + */
>> +
>> +#ifndef ASM__RISCV__VTIMER_H
>> +#define ASM__RISCV__VTIMER_H
>> +
>> +#include <xen/timer.h>
>> +
>> +struct domain;
>> +struct vcpu;
> I don't think this one is needed, as long as you have ...
>
>> +struct xen_arch_domainconfig;
>> +
>> +struct vtimer {
>> +    struct vcpu *v;
> ... this. Question is why this is here: You should be able to get hold of the
> struct vcpu containing a struct vtimer using container_of().

Good point, I haven't thought about that. It could really be done using container_of().


>
>> --- /dev/null
>> +++ b/xen/arch/riscv/vtimer.c
>> @@ -0,0 +1,39 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#include <xen/sched.h>
>> +
>> +#include <public/xen.h>
>> +
>> +#include <asm/vtimer.h>
>> +
>> +int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config)
>> +{
>> +    /* Nothing to do at the moment */
>> +
>> +    return 0;
>> +}
> The function has no caller and does nothing - why do we need it?

It will be called later in arch_domain_create().

It will be needed if SSTC extension will be supported but could be dropped now.

>
>> +static void vtimer_expired(void *data)
>> +{
>> +    panic("%s: TBD\n", __func__);
>> +}
>> +
>> +int vcpu_vtimer_init(struct vcpu *v)
>> +{
>> +    struct vtimer *t = &v->arch.vtimer;
>> +
>> +    t->v = v;
>> +    init_timer(&t->timer, vtimer_expired, t, v->processor);
>> +
>> +    v->arch.vtimer_initialized = true;
> init_timer() has specific effects (like setting t->function to non-NULL
> and t->status to other than TIMER_STATUS_invalid). Can't you leverage
> that instead of having a separate boolean? (Iirc we do so elsewhere.)

Nice, it could be used instead of having vtimer_initialized in struct vtimer.

Thanks.

~ Oleksii