Re: [PATCH v4 3/8] x86/HPET: move legacy tick IRQ count adjustment

["Roger Pau Monné" <roger.pau@citrix.com>]

On Mon, Nov 17, 2025 at 03:37:45PM +0100, Jan Beulich wrote:
> If already we play with the IRQ count, we should do so only if we actually
> "consume" the interrupt; normal timer IRQs should not have any adjustment
> done.
> 
> Fixes: 353533232730 ("cpuidle: fix the menu governor to enhance IO performance")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> _Why_ we do these adjustments (also elsewhere) I don't really know.

I think I have an idea of what's going on here.  This accounting is
used by the idle governor to decide when to go idle.  On Linux (where
the code is imported from) the governor took into account the inflight
IO request state.  However that's not available to Xen and instead
they decided to mimic the tracking of the IO activity by counting
interrupts.  I bet then realized the timer interrupt would "skew"
those results and make it look like there's IO activity when the
system is otherwise mostly idle.

> 
> --- a/xen/arch/x86/hpet.c
> +++ b/xen/arch/x86/hpet.c
> @@ -808,13 +808,13 @@ int hpet_broadcast_is_available(void)
>  
>  int hpet_legacy_irq_tick(void)
>  {
> -    this_cpu(irq_count)--;

I think you want to pull this decrease into timer_interrupt() itself,
so it does the decrease unconditionally of whether the interrupt is a
legacy HPET one or from the PIT?

By gating the decrease on the interrupt having been originated from
the HPET you completely avoid the decrease in the PIT case AFAICT.

> -
>      if ( !hpet_events ||
>           (hpet_events->flags & (HPET_EVT_DISABLE|HPET_EVT_LEGACY)) !=
>           HPET_EVT_LEGACY )
>          return 0;
>  
> +    this_cpu(irq_count)--;

Also in hpet_interrupt_handler() we might consider only doing the
decrease after we ensure it's not a spurious interrupt?  We don't seem
to decrease irq_count for spurious interrupts elsewhere.

Thanks, Roger.