[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <179aa29aa9d5baa698171b884bb6fbe66ad5c2b3.camel@infradead.org>
Date: Fri, 02 Jan 2026 16:41:28 +0000
From: David Woodhouse <dwmw2@...radead.org>
To: Khushit Shah <khushit.shah@...anix.com>, seanjc@...gle.com,
pbonzini@...hat.com, kai.huang@...el.com
Cc: mingo@...hat.com, x86@...nel.org, bp@...en8.de, hpa@...or.com,
linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
dave.hansen@...ux.intel.com, tglx@...utronix.de, jon@...anix.com,
shaju.abraham@...anix.com, stable@...r.kernel.org
Subject: Re: [PATCH v5 3/3] KVM: x86: Add x2APIC "features" to control EOI
broadcast suppression
On Mon, 2025-12-29 at 11:17 +0000, Khushit Shah wrote:
> Add two flags for KVM_CAP_X2APIC_API to allow userspace to control support
> for Suppress EOI Broadcasts, which KVM completely mishandles. When x2APIC
> support was first added, KVM incorrectly advertised and "enabled" Suppress
> EOI Broadcast, without fully supporting the I/O APIC side of the equation,
> i.e. without adding directed EOI to KVM's in-kernel I/O APIC.
>
> That flaw was carried over to split IRQCHIP support, i.e. KVM advertised
> support for Suppress EOI Broadcasts irrespective of whether or not the
> userspace I/O APIC implementation supported directed EOIs. Even worse,
> KVM didn't actually suppress EOI broadcasts, i.e. userspace VMMs without
> support for directed EOI came to rely on the "spurious" broadcasts.
>
> KVM "fixed" the in-kernel I/O APIC implementation by completely disabling
> support for Suppress EOI Broadcasts in commit 0bcc3fb95b97 ("KVM: lapic:
> stop advertising DIRECTED_EOI when in-kernel IOAPIC is in use"), but
> didn't do anything to remedy userspace I/O APIC implementations.
>
> KVM's bogus handling of Suppress EOI Broadcast is problematic when the
> guest relies on interrupts being masked in the I/O APIC until well after
> the initial local APIC EOI. E.g. Windows with Credential Guard enabled
> handles interrupts in the following order:
> 1. Interrupt for L2 arrives.
> 2. L1 APIC EOIs the interrupt.
> 3. L1 resumes L2 and injects the interrupt.
> 4. L2 EOIs after servicing.
> 5. L1 performs the I/O APIC EOI.
>
> Because KVM EOIs the I/O APIC at step #2, the guest can get an interrupt
> storm, e.g. if the IRQ line is still asserted and userspace reacts to the
> EOI by re-injecting the IRQ, because the guest doesn't de-assert the line
> until step #4, and doesn't expect the interrupt to be re-enabled until
> step #5.
>
> Unfortunately, simply "fixing" the bug isn't an option, as KVM has no way
> of knowing if the userspace I/O APIC supports directed EOIs, i.e.
> suppressing EOI broadcasts would result in interrupts being stuck masked
> in the userspace I/O APIC due to step #5 being ignored by userspace. And
> fully disabling support for Suppress EOI Broadcast is also undesirable, as
> picking up the fix would require a guest reboot, *and* more importantly
> would change the virtual CPU model exposed to the guest without any buy-in
> from userspace.
>
> Add KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST and
> KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST flags to allow userspace to
> explicitly enable or disable support for Suppress EOI Broadcasts. This
> gives userspace control over the virtual CPU model exposed to the guest,
> as KVM should never have enabled support for Suppress EOI Broadcast without
> userspace opt-in. Not setting either flag will result in legacy quirky
> behavior for backward compatibility.
>
> When KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST is set and using in-kernel
> IRQCHIP mode, KVM will use I/O APIC version 0x20, which includes support
> for the EOI Register.
>
> Note, Suppress EOI Broadcasts is defined only in Intel's SDM, not in AMD's
> APM. But the bit is writable on some AMD CPUs, e.g. Turin, and KVM's ABI
> is to support Directed EOI (KVM's name) irrespective of guest CPU vendor.
>
> Fixes: 7543a635aa09 ("KVM: x86: Add KVM exit for IOAPIC EOIs")
> Closes: https://lore.kernel.org/kvm/7D497EF1-607D-4D37-98E7-DAF95F099342@nutanix.com
> Cc: stable@...r.kernel.org
Do we want the Cc:stable? And if we do we'd want it on all three
patches, surely?
> Suggested-by: David Woodhouse <dwmw2@...radead.org>
> Co-developed-by: Sean Christopherson <seanjc@...gle.com>
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> Signed-off-by: Khushit Shah <khushit.shah@...anix.com>
Reviewed-by: David Woodhouse <dwmw@...zon.co.uk>
Although...
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1229,6 +1229,12 @@ enum kvm_irqchip_mode {
> KVM_IRQCHIP_SPLIT, /* created with KVM_CAP_SPLIT_IRQCHIP */
> };
>
> +enum kvm_suppress_eoi_broadcast_mode {
> + KVM_SUPPRESS_EOI_BROADCAST_QUIRKED, /* Legacy behavior */
I believe it's cosmetic but I think I'd be slightly happier with an
explicit '= 0' on that, as we rely on that field being initialised to
zero with the allocation of struct kvm, don't we?
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5069 bytes)
Powered by blists - more mailing lists