Re: [PATCH v5 11/21] KVM: selftests: Verify IRQs wake up halted vCPUs in IRQ test

[Sean Christopherson <seanjc@google.com>]

On Thu, Jun 04, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The guest code introduces a missed wakeup (TOCTOU) race condition between checking the `done` flag and executing the `hlt` instruction. This can cause the guest vCPU to sleep indefinitely, leading to a test hang.
> --
> 
> commit 9a28612ad82c9a519d18da6d85e17ef536f35e8d
> Author: David Matlack <dmatlack@google.com>
> 
> KVM: selftests: Verify IRQs wake up halted vCPUs in IRQ test
>     
> This commit adds a '-b' flag to the KVM IRQ selftest to verify that
> interrupts correctly wake up vCPUs from a halted state.

s/halted/blocking, to connect '-b' to "blocking".

> If the flag is provided, the guest uses the HLT instruction instead of
> spinning.

Describe the *change* as a command, don't describe the effect of the code.  E.g.

  If blocking mode is enabled, execute HLT in the guest (for x86) instead of
  simply doing cpu_relax(), blah blah blah.

> > diff --git a/tools/testing/selftests/kvm/irq_test.c b/tools/testing/selftests/kvm/irq_test.c
> > index 90cff165c3512..671338f0e6122 100644
> > --- a/tools/testing/selftests/kvm/irq_test.c
> > +++ b/tools/testing/selftests/kvm/irq_test.c
> 
> [ ... ]
> 
> > @@ -43,8 +44,12 @@ static void guest_code(void)
> >  
> >  	WRITE_ONCE(guest_ready_for_irqs[guest_get_vcpu_id()], true);
> >  
> > -	while (!READ_ONCE(done))
> > -		cpu_relax();
> > +	while (!READ_ONCE(done)) {
> > +		if (block_vcpus)
> > +			hlt();
> 
> [Severity: Medium]
> Is there a missed wakeup (TOCTOU) race condition between checking the done
> flag and executing the hlt instruction?
> 
> If the host sets done to true and triggers the final wake-up interrupt
> after the guest evaluates !READ_ONCE(done) but before it executes hlt(),
> the interrupt handler will run immediately. Upon returning, the guest will
> blindly execute hlt() and go to sleep. 
> 
> Since the host only sends one final interrupt per vCPU, does the guest
> need to disable interrupts (e.g., using cli()) before checking the condition
> and use safe_halt() to atomically re-enable interrupts and halt?

Yeah, this is all kinds of broken.  Even if the test passes (I haven't actually
tried this version), there's zero chance it's actually providing the coverage I
want it to provide.

In the interest of landing this test sooner than later, as I *really* want this
coverage, I'll skip this patch and punt on getting a functionally correct version
to the future.