Re: [PATCH] KVM: x86: Ignore pending PV EOI if the vCPU has since disabled PV EOIs

[Sean Christopherson]

On Thu, Jun 25, 2026, Kai Huang wrote:
> On Wed, 2026-06-24 at 15:05 -0700, Sean Christopherson wrote:
> > Ignore KVM's internal "service pending PV EOI" request if the vCPU has
> > disabled PV EOIs since the request was made.  Asserting that PV EOIs are
> > enabled can fail if reading guest memory in pv_eoi_get_user() fails, i.e.
> > if pv_eoi_test_and_clr_pending() bails early, *and* the vCPU also disables
> > PV EOIs.
> > 
> >   kernel BUG at arch/x86/kvm/lapic.c:3338!
> >   Oops: invalid opcode: 0000 [#1] SMP
> >   CPU: 4 UID: 1000 PID: 890 Comm: pv_eoi_test Not tainted 7.0.0-d585aa5894d8-vm #337 PREEMPT
> >   Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
> >   RIP: 0010:kvm_lapic_sync_from_vapic+0x12b/0x140 [kvm]
> >   Call Trace:
> >    <TASK>
> >    kvm_arch_vcpu_ioctl_run+0x1075/0x1c30 [kvm]
> >    kvm_vcpu_ioctl+0x2d5/0x980 [kvm]
> >    __x64_sys_ioctl+0x8a/0xd0
> >    do_syscall_64+0xb5/0xb40
> >    entry_SYSCALL_64_after_hwframe+0x4b/0x53
> >    </TASK>
> >   Modules linked in: kvm_intel kvm irqbypass
> >   ---[ end trace 0000000000000000 ]---
> > 
> > Fixes: ae7a2a3fb6f8 ("KVM: host side for eoi optimization")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > ---
> >  arch/x86/kvm/lapic.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index 6f30bbdddb5a..38bba9a1114c 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -3371,6 +3371,12 @@ static void apic_sync_pv_eoi_from_guest(struct kvm_vcpu *vcpu,
> >  					struct kvm_lapic *apic)
> >  {
> >  	int vector;
> > +
> > +	if (unlikely(!pv_eoi_enabled(vcpu))) {
> > +		__clear_bit(KVM_APIC_PV_EOI_PENDING, &vcpu->arch.apic_attention);
> > +		return;
> > +	}
> > +
> 
> Do you think whether it's better to do this in kvm_lapic_set_pv_eoi(), i.e.,
> when guest disables PV_EOI for this vCPU?

Yes, if we were doing the initial implementation.  I opted not to go that route
because it would require writing to guest memory to put the shared flag back to
KVM_PV_EOI_DISABLED, at which point this becomes a guest-visible change.  It's
probably fine?  But very, very, theoretically, a guest could toggle the feature
on/off and expect memory to remain unchanged.

And I want to get rid of the BUG_ON() no matter what, so I opted to leave the
WRMSR path alone and go with the minimal (albeit ugly) fix.

> If we do so, it seems we can save the unnecessary pv_eoi_set_pending() (which
> writes guest memory) called from apic_sync_pv_eoi_to_guest(), or it's not
> possible to happen?

No, the whole point of apic_sync_pv_eoi_to_guest() is to do pv_eoi_set_pending().
The naming of the flags and the way the code is written obfuscates the logic to
some extent, but what it's doing in pseudocode is:

    IF vCPU has in-service IRQ and no pending IRQs; THEN
        vCPU.pv_eoi = ACTIVE
    FI

where vCPU.pv_eoi is the shared memory.  The guest side, in its APIC EOI path:

  static notrace __maybe_unused void kvm_guest_apic_eoi_write(void)
  {
	/**
	 * This relies on __test_and_clear_bit to modify the memory
	 * in a way that is atomic with respect to the local CPU.
	 * The hypervisor only accesses this memory from the local CPU so
	 * there's no need for lock or memory barriers.
	 * An optimization barrier is implied in apic write.
	 */
	if (__test_and_clear_bit(KVM_PV_EOI_BIT, this_cpu_ptr(&kvm_apic_eoi)))
		return;
	apic_native_eoi();
  }

then does:

    IF vCPU.pv_eoi != ACTIVE; THEN
        do APIC EOI
    FI