Re: [PATCH v1 08/15] xen/riscv: introduce vtimer_set_timer() and vtimer_expired()

[Jan Beulich <jbeulich@xxxxxxxx>]

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?

> +#include <xen/time.h>

Don't you mean xen/timer.h here?

> @@ -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?

> +    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 ( 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.
There you'll also find a use of migrate_timer(), which you will want to
at least consider using here as well.

Jan