Re: [PATCH] xen/arm: ignore spurious interrupts from virtual timer

[Julien Grall <julien@xxxxxxx>]

Hi,

On 12/01/2026 12:50, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
>
> If a spurious virtual timer interrupt occurs (i.e. the interrupt fires
> but CNTV_CTL_EL0 does not report it as pending), Xen masks the virtual
> timer and injects the vtimer IRQ into the guest. For Linux guests, the
> timer interrupt is unmasked only after programming a new CVAL value from
> the timer interrupt handler. When the interrupt is not reported as
> pending, the handler can skip that programming step, leaving the timer
> masked and stalling the affected CPU.
I guess this is happening if Linux is trying to modify CVAL with the 
local interrupt masked?
> This patch mirrors the Linux arm generic timer handler: if the interrupt
> fires but the pending bit is not set, treat it as spurious and ignore it.
Have you considered fixing properly our virtual timer emulation? I know 
this requires more code, but at least we are not adding more 
non-compliant code which requires patching the Guest OS.
IIRC there was a series from Stewart to solve it and it was in pretty 
good shape at the time it was posted.
> This issue is reproducible under heavy load on the R-Car X5H board
> (Cortex-A720AE r0p0).
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> ---
>   xen/arch/arm/include/asm/perfc_defn.h |  7 ++++---
>   xen/arch/arm/time.c                   | 11 ++++++++++-
>   2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/perfc_defn.h 
> b/xen/arch/arm/include/asm/perfc_defn.h
> index effd25b69e..f83989d95a 100644
> --- a/xen/arch/arm/include/asm/perfc_defn.h
> +++ b/xen/arch/arm/include/asm/perfc_defn.h
> @@ -69,9 +69,10 @@ PERFCOUNTER(ppis,                 "#PPIs")
>   PERFCOUNTER(spis,                 "#SPIs")
>   PERFCOUNTER(guest_irqs,           "#GUEST-IRQS")
>   
> -PERFCOUNTER(hyp_timer_irqs,   "Hypervisor timer interrupts")
> -PERFCOUNTER(virt_timer_irqs,  "Virtual timer interrupts")
> -PERFCOUNTER(maintenance_irqs, "Maintenance interrupts")
> +PERFCOUNTER(hyp_timer_irqs,             "Hypervisor timer interrupts")
> +PERFCOUNTER(virt_timer_irqs,            "Virtual timer interrupts")
> +PERFCOUNTER(virt_timer_spurious_irqs,   "Virtual timer spurious interrupts")
> +PERFCOUNTER(maintenance_irqs,           "Maintenance interrupts")
>   
>   PERFCOUNTER(atomics_guest,    "atomics: guest access")
>   PERFCOUNTER(atomics_guest_paused,   "atomics: guest paused")
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index cc3fcf47b6..d18d6568bb 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -258,6 +258,8 @@ static void htimer_interrupt(int irq, void *dev_id)
>   
>   static void vtimer_interrupt(int irq, void *dev_id)
>   {
> +    register_t ctl;
> +
>       /*
>        * Edge-triggered interrupts can be used for the virtual timer. Even
>        * if the timer output signal is masked in the context switch, the
> @@ -271,9 +273,16 @@ static void vtimer_interrupt(int irq, void *dev_id)
>       if ( unlikely(is_idle_vcpu(current)) )
>           return;
>   
> +    ctl = READ_SYSREG(CNTV_CTL_EL0);
> +    if ( unlikely(!(ctl & CNTx_CTL_PENDING)) )
For the others, the Armv8 specification names this field ISTATUS. 
Regardless what I wrote above, the change look alright. Before I ack, 
can you confirm whether you checked other OSes (I am thinking at least 
Zephyr) will also ignore spurious interrupt?
Cheers,

--
Julien Grall