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 while using split IRQCHIP mode. 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 a userspace opt-in. Not setting either flag will result in legacy quirky behavior for backward compatibility.
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@vger.kernel.org Suggested-by: David Woodhouse dwmw2@infradead.org Co-developed-by: Sean Christopherson seanjc@google.com Signed-off-by: Sean Christopherson seanjc@google.com Signed-off-by: Khushit Shah khushit.shah@nutanix.com --- v4: - 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 while using split IRQCHIP mode.
After the inclusion of David Woodhouse's patch to support IOAPIC version 0x20, we can tweak the uAPI to support kernel IRQCHIP mode as well.
Testing: - Setting both the flags fails with EINVAL. - Setting flags in kernel IRQCHIP mode fails with EINVAL. - Setting flags in split IRQCHIP mode succeeds and both the flags work as expected. --- Documentation/virt/kvm/api.rst | 27 ++++++++++++++++++++-- arch/x86/include/asm/kvm_host.h | 7 ++++++ arch/x86/include/uapi/asm/kvm.h | 6 +++-- arch/x86/kvm/lapic.c | 40 +++++++++++++++++++++++++++++++++ arch/x86/kvm/x86.c | 19 +++++++++++++--- 5 files changed, 92 insertions(+), 7 deletions(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 57061fa29e6a..b26528e0fec1 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -7800,8 +7800,10 @@ Will return -EBUSY if a VCPU has already been created.
Valid feature flags in args[0] are::
- #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0) - #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1) + #define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0) + #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1) + #define KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST (1ULL << 2) + #define KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST (1ULL << 3)
Enabling KVM_X2APIC_API_USE_32BIT_IDS changes the behavior of KVM_SET_GSI_ROUTING, KVM_SIGNAL_MSI, KVM_SET_LAPIC, and KVM_GET_LAPIC, @@ -7814,6 +7816,27 @@ as a broadcast even in x2APIC mode in order to support physical x2APIC without interrupt remapping. This is undesirable in logical mode, where 0xff represents CPUs 0-7 in cluster 0.
+Setting KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST instructs KVM to enable +Suppress EOI Broadcasts. KVM will advertise support for Suppress EOI Broadcast +to the guest and suppress LAPIC EOI broadcasts when the guest sets the +Suppress EOI Broadcast bit in the SPIV register. + +Setting KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST disables support for +Suppress EOI Broadcasts entirely, i.e. instructs KVM to NOT advertise support +to the guest. + +Modern VMMs should either enable KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST or +KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST. If not, legacy quirky behavior will +be used by KVM, which is to advertise support for Suppress EOI Broadcasts but +not actually suppressing EOI broadcasts. + +Currently, both KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST and +KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST must only be set when in split IRQCHIP +mode. Otherwise, the ioctl will fail with an EINVAL error. + +Setting both KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST and +KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST will fail with an EINVAL error. + 7.8 KVM_CAP_S390_USER_INSTR0 ----------------------------
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 48598d017d6f..4a6d94dc7a2a 100644 --- 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 */ + KVM_SUPPRESS_EOI_BROADCAST_ENABLED, /* Enable Suppress EOI broadcast */ + KVM_SUPPRESS_EOI_BROADCAST_DISABLED /* Disable Suppress EOI broadcast */ +}; + struct kvm_x86_msr_filter { u8 count; bool default_allow:1; @@ -1480,6 +1486,7 @@ struct kvm_arch {
bool x2apic_format; bool x2apic_broadcast_quirk_disabled; + enum kvm_suppress_eoi_broadcast_mode suppress_eoi_broadcast_mode;
bool has_mapped_host_mmio; bool guest_can_read_msr_platform_info; diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h index d420c9c066d4..d30241429fa8 100644 --- a/arch/x86/include/uapi/asm/kvm.h +++ b/arch/x86/include/uapi/asm/kvm.h @@ -913,8 +913,10 @@ struct kvm_sev_snp_launch_finish { __u64 pad1[4]; };
-#define KVM_X2APIC_API_USE_32BIT_IDS (1ULL << 0) -#define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (1ULL << 1) +#define KVM_X2APIC_API_USE_32BIT_IDS (_BITULL(0)) +#define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK (_BITULL(1)) +#define KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST (_BITULL(2)) +#define KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST (_BITULL(3))
struct kvm_hyperv_eventfd { __u32 conn_id; diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 0ae7f913d782..1ef0bd3eff1e 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -105,6 +105,34 @@ bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector) apic_test_vector(vector, apic->regs + APIC_IRR); }
+static inline bool kvm_lapic_advertise_suppress_eoi_broadcast(struct kvm *kvm) +{ + /* + * Advertise Suppress EOI broadcast support to the guest unless the VMM + * explicitly disabled it. + * + * Historically, KVM advertised this capability even though it did not + * actually suppress EOIs. + */ + return kvm->arch.suppress_eoi_broadcast_mode != + KVM_SUPPRESS_EOI_BROADCAST_DISABLED; +} + +static inline bool kvm_lapic_ignore_suppress_eoi_broadcast(struct kvm *kvm) +{ + /* + * Returns true if KVM should ignore the suppress EOI broadcast bit set by + * the guest and broadcast EOIs anyway. + * + * Only returns false when the VMM explicitly enabled Suppress EOI + * broadcast. If disabled by VMM, the bit should be ignored as it is not + * supported. Legacy behavior was to ignore the bit and broadcast EOIs + * anyway. + */ + return kvm->arch.suppress_eoi_broadcast_mode != + KVM_SUPPRESS_EOI_BROADCAST_ENABLED; +} + __read_mostly DEFINE_STATIC_KEY_FALSE(kvm_has_noapic_vcpu); EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_has_noapic_vcpu);
@@ -562,6 +590,7 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu) * IOAPIC. */ if (guest_cpu_cap_has(vcpu, X86_FEATURE_X2APIC) && + kvm_lapic_advertise_suppress_eoi_broadcast(vcpu->kvm) && !ioapic_in_kernel(vcpu->kvm)) v |= APIC_LVR_DIRECTED_EOI; kvm_lapic_set_reg(apic, APIC_LVR, v); @@ -1517,6 +1546,17 @@ static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector)
/* Request a KVM exit to inform the userspace IOAPIC. */ if (irqchip_split(apic->vcpu->kvm)) { + /* + * Don't exit to userspace if the guest has enabled Directed + * EOI, a.k.a. Suppress EOI Broadcasts, in which case the local + * APIC doesn't broadcast EOIs (the guest must EOI the target + * I/O APIC(s) directly). Ignore the suppression if userspace + * has NOT explicitly enabled Suppress EOI broadcast. + */ + if ((kvm_lapic_get_reg(apic, APIC_SPIV) & APIC_SPIV_DIRECTED_EOI) && + !kvm_lapic_ignore_suppress_eoi_broadcast(apic->vcpu->kvm)) + return; + apic->vcpu->arch.pending_ioapic_eoi = vector; kvm_make_request(KVM_REQ_IOAPIC_EOI_EXIT, apic->vcpu); return; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c9c2aa6f4705..81b40fdb5f5f 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -121,8 +121,11 @@ static u64 __read_mostly efer_reserved_bits = ~((u64)EFER_SCE);
#define KVM_CAP_PMU_VALID_MASK KVM_PMU_CAP_DISABLE
-#define KVM_X2APIC_API_VALID_FLAGS (KVM_X2APIC_API_USE_32BIT_IDS | \ - KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK) +#define KVM_X2APIC_API_VALID_FLAGS \ + (KVM_X2APIC_API_USE_32BIT_IDS | \ + KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK | \ + KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST | \ + KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST)
static void update_cr8_intercept(struct kvm_vcpu *vcpu); static void process_nmi(struct kvm_vcpu *vcpu); @@ -6777,12 +6780,22 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, r = -EINVAL; if (cap->args[0] & ~KVM_X2APIC_API_VALID_FLAGS) break; + if ((cap->args[0] & KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST) && + (cap->args[0] & KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST)) + break; + if (!irqchip_split(kvm) && + ((cap->args[0] & KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST) || + (cap->args[0] & KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST))) + break;
if (cap->args[0] & KVM_X2APIC_API_USE_32BIT_IDS) kvm->arch.x2apic_format = true; if (cap->args[0] & KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK) kvm->arch.x2apic_broadcast_quirk_disabled = true; - + if (cap->args[0] & KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST) + kvm->arch.suppress_eoi_broadcast_mode = KVM_SUPPRESS_EOI_BROADCAST_ENABLED; + if (cap->args[0] & KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST) + kvm->arch.suppress_eoi_broadcast_mode = KVM_SUPPRESS_EOI_BROADCAST_DISABLED; r = 0; break; case KVM_CAP_X86_DISABLE_EXITS:
A bunch of nits, but I'll fix them up when applying, assuming on one else has feedback.
On Thu, Dec 11, 2025, Khushit Shah wrote:
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 48598d017d6f..4a6d94dc7a2a 100644 --- 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 */
- KVM_SUPPRESS_EOI_BROADCAST_ENABLED, /* Enable Suppress EOI broadcast */
- KVM_SUPPRESS_EOI_BROADCAST_DISABLED /* Disable Suppress EOI broadcast */
+};
struct kvm_x86_msr_filter { u8 count; bool default_allow:1; @@ -1480,6 +1486,7 @@ struct kvm_arch { bool x2apic_format; bool x2apic_broadcast_quirk_disabled;
- enum kvm_suppress_eoi_broadcast_mode suppress_eoi_broadcast_mode;
For brevity, I vote for eoi_broadcast_mode here, i.e.:
enum kvm_suppress_eoi_broadcast_mode eoi_broadcast_mode;
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 0ae7f913d782..1ef0bd3eff1e 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -105,6 +105,34 @@ bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector) apic_test_vector(vector, apic->regs + APIC_IRR); } +static inline bool kvm_lapic_advertise_suppress_eoi_broadcast(struct kvm *kvm)
Formletter...
Do not use "inline" for functions that are visible only to the local compilation unit. "inline" is just a hint, and modern compilers are smart enough to inline functions when appropriate without a hint.
A longer explanation/rant here: https://lore.kernel.org/all/ZAdfX+S323JVWNZC@google.com
+{
- /*
* Advertise Suppress EOI broadcast support to the guest unless the VMM* explicitly disabled it.** Historically, KVM advertised this capability even though it did not* actually suppress EOIs.*/- return kvm->arch.suppress_eoi_broadcast_mode !=
KVM_SUPPRESS_EOI_BROADCAST_DISABLED;
With a shorter field name, this can more comfortably be:
return kvm->arch.eoi_broadcast_mode != KVM_SUPPRESS_EOI_BROADCAST_DISABLED;
+}
+static inline bool kvm_lapic_ignore_suppress_eoi_broadcast(struct kvm *kvm) +{
- /*
* Returns true if KVM should ignore the suppress EOI broadcast bit set by* the guest and broadcast EOIs anyway.** Only returns false when the VMM explicitly enabled Suppress EOI* broadcast. If disabled by VMM, the bit should be ignored as it is not* supported. Legacy behavior was to ignore the bit and broadcast EOIs* anyway.*/- return kvm->arch.suppress_eoi_broadcast_mode !=
KVM_SUPPRESS_EOI_BROADCAST_ENABLED;
And then...
return kvm->arch.eoi_broadcast_mode != KVM_SUPPRESS_EOI_BROADCAST_ENABLED;
+}
__read_mostly DEFINE_STATIC_KEY_FALSE(kvm_has_noapic_vcpu); EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_has_noapic_vcpu); @@ -562,6 +590,7 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu) * IOAPIC. */ if (guest_cpu_cap_has(vcpu, X86_FEATURE_X2APIC) &&
kvm_lapic_advertise_suppress_eoi_broadcast(vcpu->kvm) &&
Align indentation.
!ioapic_in_kernel(vcpu->kvm)) v |= APIC_LVR_DIRECTED_EOI;kvm_lapic_set_reg(apic, APIC_LVR, v); @@ -1517,6 +1546,17 @@ static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector) /* Request a KVM exit to inform the userspace IOAPIC. */ if (irqchip_split(apic->vcpu->kvm)) {
/** Don't exit to userspace if the guest has enabled Directed* EOI, a.k.a. Suppress EOI Broadcasts, in which case the local* APIC doesn't broadcast EOIs (the guest must EOI the target* I/O APIC(s) directly). Ignore the suppression if userspace* has NOT explicitly enabled Suppress EOI broadcast.*/if ((kvm_lapic_get_reg(apic, APIC_SPIV) & APIC_SPIV_DIRECTED_EOI) &&!kvm_lapic_ignore_suppress_eoi_broadcast(apic->vcpu->kvm))return;- apic->vcpu->arch.pending_ioapic_eoi = vector; kvm_make_request(KVM_REQ_IOAPIC_EOI_EXIT, apic->vcpu); return;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c9c2aa6f4705..81b40fdb5f5f 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -121,8 +121,11 @@ static u64 __read_mostly efer_reserved_bits = ~((u64)EFER_SCE); #define KVM_CAP_PMU_VALID_MASK KVM_PMU_CAP_DISABLE -#define KVM_X2APIC_API_VALID_FLAGS (KVM_X2APIC_API_USE_32BIT_IDS | \
KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK)+#define KVM_X2APIC_API_VALID_FLAGS \
- (KVM_X2APIC_API_USE_32BIT_IDS | \
- KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK | \
- KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST | \
- KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST)
Unless someone feels strongly, I think I'd prefer to keep the existing style, e.g.
#define KVM_X2APIC_API_VALID_FLAGS (KVM_X2APIC_API_USE_32BIT_IDS | \ KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK | \ KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST | \ KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST)
static void update_cr8_intercept(struct kvm_vcpu *vcpu); static void process_nmi(struct kvm_vcpu *vcpu); @@ -6777,12 +6780,22 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, r = -EINVAL; if (cap->args[0] & ~KVM_X2APIC_API_VALID_FLAGS) break;
if ((cap->args[0] & KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST) &&(cap->args[0] & KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST))break;if (!irqchip_split(kvm) &&((cap->args[0] & KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST) ||(cap->args[0] & KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST)))break;
Again, unless someone feels strongly, I'd prefer to have some newlines here, i.e.
r = -EINVAL; if (cap->args[0] & ~KVM_X2APIC_API_VALID_FLAGS) break;
if ((cap->args[0] & KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST) && (cap->args[0] & KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST)) break;
if (!irqchip_split(kvm) && ((cap->args[0] & KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST) || (cap->args[0] & KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST))) break;
if (cap->args[0] & KVM_X2APIC_API_USE_32BIT_IDS)
On Thu, 2025-12-11 at 10:59 +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:
- Interrupt for L2 arrives.
- L1 APIC EOIs the interrupt.
- L1 resumes L2 and injects the interrupt.
- L2 EOIs after servicing.
- 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 while using split IRQCHIP mode. 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 a userspace opt-in. Not setting either flag will result in legacy quirky behavior for backward compatibility.
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@vger.kernel.org Suggested-by: David Woodhouse dwmw2@infradead.org Co-developed-by: Sean Christopherson seanjc@google.com Signed-off-by: Sean Christopherson seanjc@google.com Signed-off-by: Khushit Shah khushit.shah@nutanix.com
v4:
- 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 while using split IRQCHIP mode.
After the inclusion of David Woodhouse's patch to support IOAPIC version 0x20, we can tweak the uAPI to support kernel IRQCHIP mode as well.
I suppose adding such support to allow userspace to apply either flag is not considered as "breaking userspace ABI", so:
Acked-by: Kai Huang kai.huang@intel.com
On Thu, 2025-12-11 at 10:59 +0000, Khushit Shah wrote:
+Modern VMMs should either enable KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST or +KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST. If not, legacy quirky behavior will +be used by KVM, which is to advertise support for Suppress EOI Broadcasts but +not actually suppressing EOI broadcasts.
"which is to... not actually suppressing EOI broadcasts."
I think you want s/suppressing/suppress/ there.
+Currently, both KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST and
I don't much like the word 'both' at the start of the sentence. It sent my brain down a language-recognition path which thought it was about setting both bits at the same time, and may lead to misunderstandings or just slower parsing of the sentence.
+KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST must only be set when in split IRQCHIP +mode. Otherwise, the ioctl will fail with an EINVAL error.
Hm, I don't like that much. For a start, DISABLE should be fine with the in-kernel IRQCHIP right now (and is the only behaviour that truly makes sense right now).
And my intent was that the in-kernel I/O APIC patch gets included as *part* of this series, otherwise we're making a semantic change to the ENABLE behaviour later.
Also... how does userspace discover the availability of these flags?
(And if you don't include the I/O APIC patch as part of this series, we also need to understand how userspace will later discover that ENABLE can be applied to the in-kernel irqchip too.)
On Fri, Dec 12, 2025, David Woodhouse wrote:
On Thu, 2025-12-11 at 10:59 +0000, Khushit Shah wrote: And my intent was that the in-kernel I/O APIC patch gets included as *part* of this series, otherwise we're making a semantic change to the ENABLE behaviour later.
Hmm, my only concern on that front is that we'd be backporting effectively new functionality to stable@ kernels that isn't strictly necessary. But that's probably fine given that it requires userspace to opt-in.
Also... how does userspace discover the availability of these flags?
Ugh, yeah, I was reading too fast and overlooked that. This needs a CAP.
On Thu, 2025-12-11 at 16:10 -0800, Sean Christopherson wrote:
On Fri, Dec 12, 2025, David Woodhouse wrote:
On Thu, 2025-12-11 at 10:59 +0000, Khushit Shah wrote: And my intent was that the in-kernel I/O APIC patch gets included as *part* of this series, otherwise we're making a semantic change to the ENABLE behaviour later.
Hmm, my only concern on that front is that we'd be backporting effectively new functionality to stable@ kernels that isn't strictly necessary. But that's probably fine given that it requires userspace to opt-in.
Yeah. Arguably it's *all* a new capability and a 'new' (albeit 30 years old!) feature.
I guess you could make an argument for *only* backporting the DISABLE part to stable kernels and not the ENABLE part.... but don't :)
On 12 Dec 2025, at 12:29 AM, Sean Christopherson seanjc@google.com wrote:
A bunch of nits, but I'll fix them up when applying, assuming on one else has feedback.
On Thu, Dec 11, 2025, Khushit Shah wrote:
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 48598d017d6f..4a6d94dc7a2a 100644 --- 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 */
- KVM_SUPPRESS_EOI_BROADCAST_ENABLED, /* Enable Suppress EOI broadcast */
- KVM_SUPPRESS_EOI_BROADCAST_DISABLED /* Disable Suppress EOI broadcast */
+};
struct kvm_x86_msr_filter { u8 count; bool default_allow:1; @@ -1480,6 +1486,7 @@ struct kvm_arch {
bool x2apic_format; bool x2apic_broadcast_quirk_disabled;
- enum kvm_suppress_eoi_broadcast_mode suppress_eoi_broadcast_mode;
For brevity, I vote for eoi_broadcast_mode here, i.e.:
enum kvm_suppress_eoi_broadcast_mode eoi_broadcast_mode;
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 0ae7f913d782..1ef0bd3eff1e 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -105,6 +105,34 @@ bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector) apic_test_vector(vector, apic->regs + APIC_IRR); }
+static inline bool kvm_lapic_advertise_suppress_eoi_broadcast(struct kvm *kvm)
Formletter...
Do not use "inline" for functions that are visible only to the local compilation unit. "inline" is just a hint, and modern compilers are smart enough to inline functions when appropriate without a hint.
A longer explanation/rant here: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_ZAd...
+{
- /*
- Advertise Suppress EOI broadcast support to the guest unless the VMM
- explicitly disabled it.
- Historically, KVM advertised this capability even though it did not
- actually suppress EOIs.
- */
- return kvm->arch.suppress_eoi_broadcast_mode !=
- KVM_SUPPRESS_EOI_BROADCAST_DISABLED;
With a shorter field name, this can more comfortably be:
return kvm->arch.eoi_broadcast_mode != KVM_SUPPRESS_EOI_BROADCAST_DISABLED;
+}
+static inline bool kvm_lapic_ignore_suppress_eoi_broadcast(struct kvm *kvm) +{
- /*
- Returns true if KVM should ignore the suppress EOI broadcast bit set by
- the guest and broadcast EOIs anyway.
- Only returns false when the VMM explicitly enabled Suppress EOI
- broadcast. If disabled by VMM, the bit should be ignored as it is not
- supported. Legacy behavior was to ignore the bit and broadcast EOIs
- anyway.
- */
- return kvm->arch.suppress_eoi_broadcast_mode !=
- KVM_SUPPRESS_EOI_BROADCAST_ENABLED;
And then...
return kvm->arch.eoi_broadcast_mode != KVM_SUPPRESS_EOI_BROADCAST_ENABLED;
+}
__read_mostly DEFINE_STATIC_KEY_FALSE(kvm_has_noapic_vcpu); EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_has_noapic_vcpu);
@@ -562,6 +590,7 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu)
- IOAPIC.
*/ if (guest_cpu_cap_has(vcpu, X86_FEATURE_X2APIC) &&
- kvm_lapic_advertise_suppress_eoi_broadcast(vcpu->kvm) &&
Align indentation.
!ioapic_in_kernel(vcpu->kvm))v |= APIC_LVR_DIRECTED_EOI; kvm_lapic_set_reg(apic, APIC_LVR, v); @@ -1517,6 +1546,17 @@ static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector)
/* Request a KVM exit to inform the userspace IOAPIC. */ if (irqchip_split(apic->vcpu->kvm)) {
- /*
- Don't exit to userspace if the guest has enabled Directed
- EOI, a.k.a. Suppress EOI Broadcasts, in which case the local
- APIC doesn't broadcast EOIs (the guest must EOI the target
- I/O APIC(s) directly). Ignore the suppression if userspace
- has NOT explicitly enabled Suppress EOI broadcast.
- */
- if ((kvm_lapic_get_reg(apic, APIC_SPIV) & APIC_SPIV_DIRECTED_EOI) &&
!kvm_lapic_ignore_suppress_eoi_broadcast(apic->vcpu->kvm))- return;
apic->vcpu->arch.pending_ioapic_eoi = vector; kvm_make_request(KVM_REQ_IOAPIC_EOI_EXIT, apic->vcpu); return; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c9c2aa6f4705..81b40fdb5f5f 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -121,8 +121,11 @@ static u64 __read_mostly efer_reserved_bits = ~((u64)EFER_SCE);
#define KVM_CAP_PMU_VALID_MASK KVM_PMU_CAP_DISABLE
-#define KVM_X2APIC_API_VALID_FLAGS (KVM_X2APIC_API_USE_32BIT_IDS | \
KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK)+#define KVM_X2APIC_API_VALID_FLAGS \
- (KVM_X2APIC_API_USE_32BIT_IDS | \
- KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK | \
- KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST | \
- KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST)
Unless someone feels strongly, I think I'd prefer to keep the existing style, e.g.
#define KVM_X2APIC_API_VALID_FLAGS (KVM_X2APIC_API_USE_32BIT_IDS | \ KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK | \ KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST | \ KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST)
static void update_cr8_intercept(struct kvm_vcpu *vcpu); static void process_nmi(struct kvm_vcpu *vcpu); @@ -6777,12 +6780,22 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, r = -EINVAL; if (cap->args[0] & ~KVM_X2APIC_API_VALID_FLAGS) break;
- if ((cap->args[0] & KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST) &&
(cap->args[0] & KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST))- break;
- if (!irqchip_split(kvm) &&
((cap->args[0] & KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST) ||(cap->args[0] & KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST)))- break;
Again, unless someone feels strongly, I'd prefer to have some newlines here, i.e.
r = -EINVAL; if (cap->args[0] & ~KVM_X2APIC_API_VALID_FLAGS) break;
if ((cap->args[0] & KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST) && (cap->args[0] & KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST)) break;
if (!irqchip_split(kvm) && ((cap->args[0] & KVM_X2APIC_ENABLE_SUPPRESS_EOI_BROADCAST) || (cap->args[0] & KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST))) break;
if (cap->args[0] & KVM_X2APIC_API_USE_32BIT_IDS)
Seems like there will be v5, so I will incorporate these fixes.
On 12 Dec 2025, at 5:31 AM, David Woodhouse dwmw2@infradead.org wrote:
Hm, I don't like that much. For a start, DISABLE should be fine with the in-kernel IRQCHIP right now (and is the only behaviour that truly makes sense right now).
And my intent was that the in-kernel I/O APIC patch gets included as *part* of this series, otherwise we're making a semantic change to the ENABLE behaviour later.
Also... how does userspace discover the availability of these flags?
(And if you don't include the I/O APIC patch as part of this series, we also need to understand how userspace will later discover that ENABLE can be applied to the in-kernel irqchip too.)
That is a valid point, how about also including the IOAPIC version 0x20 (needs to be tested) and:
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 0ae7f913d782..7b368284ec0b 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -105,6 +105,43 @@ bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector) apic_test_vector(vector, apic->regs + APIC_IRR); }
+static bool kvm_lapic_advertise_suppress_eoi_broadcast(struct kvm *kvm) +{ + /* + * Returns true if KVM should advertise Suppress EOI broadcast support + * to the guest. + * + * In split IRQCHIP mode: advertise unless the VMM explicitly disabled + * it. This preserves legacy quirky behavior where KVM advertised the + * capability even though it did not actually suppress EOIs. + * + * In kernel IRQCHIP mode: only advertise if the VMM explicitly + * enabled it (and use the IOAPIC version 0x20). + */ + if (irqchip_split(kvm)) { + return kvm->arch.suppress_eoi_broadcast_mode != + KVM_SUPPRESS_EOI_BROADCAST_DISABLED; + } else { + return kvm->arch.suppress_eoi_broadcast_mode == + KVM_SUPPRESS_EOI_BROADCAST_ENABLED; + } +} + +static bool kvm_lapic_ignore_suppress_eoi_broadcast(struct kvm *kvm) +{ + /* + * Returns true if KVM should ignore the suppress EOI broadcast bit set by + * the guest and broadcast EOIs anyway. + * + * Only returns false when the VMM explicitly enabled Suppress EOI + * broadcast. If disabled by VMM, the bit should be ignored as it is not + * supported. Legacy behavior was to ignore the bit and broadcast EOIs + * anyway. + */ + return kvm->arch.suppress_eoi_broadcast_mode != + KVM_SUPPRESS_EOI_BROADCAST_ENABLED; +} + __read_mostly DEFINE_STATIC_KEY_FALSE(kvm_has_noapic_vcpu); EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_has_noapic_vcpu);
@@ -562,7 +599,7 @@ void kvm_apic_set_version(struct kvm_vcpu *vcpu) * IOAPIC. */ if (guest_cpu_cap_has(vcpu, X86_FEATURE_X2APIC) && - !ioapic_in_kernel(vcpu->kvm)) + kvm_lapic_advertise_suppress_eoi_broadcast(vcpu->kvm)) v |= APIC_LVR_DIRECTED_EOI; kvm_lapic_set_reg(apic, APIC_LVR, v); } @@ -1515,6 +1552,17 @@ static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector) if (apic->vcpu->arch.highest_stale_pending_ioapic_eoi == vector) kvm_make_request(KVM_REQ_SCAN_IOAPIC, apic->vcpu);
+ /* + * Don't send the EOI to the I/O APIC if the guest has enabled Directed + * EOI, a.k.a. Suppress EOI Broadcasts, in which case the local + * APIC doesn't broadcast EOIs (the guest must EOI the target + * I/O APIC(s) directly). Ignore the suppression if the guest has not + * explicitly enabled Suppress EOI broadcast. + */ + if ((kvm_lapic_get_reg(apic, APIC_SPIV) & APIC_SPIV_DIRECTED_EOI) && + !kvm_lapic_ignore_suppress_eoi_broadcast(apic->vcpu->kvm)) + return; + /* Request a KVM exit to inform the userspace IOAPIC. */ if (irqchip_split(apic->vcpu->kvm)) { apic->vcpu->arch.pending_ioapic_eoi = vector;
I am not entirely sure if returning from kvm_ioapic_send_eoi() early is correct for kernel IOAPIC. The original code (which is now redundant) does this very late in kvm_ioapic_update_eoi_one().
On Fri, 2025-12-12 at 07:08 +0000, Khushit Shah wrote:
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 0ae7f913d782..7b368284ec0b 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c
Surely the point of helper functions is that they live in a header file somewhere and are not *duplicated* in the different C files that use them?
@@ -105,6 +105,43 @@ bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector) apic_test_vector(vector, apic->regs + APIC_IRR); } +static bool kvm_lapic_advertise_suppress_eoi_broadcast(struct kvm *kvm) +{ + /* + * Returns true if KVM should advertise Suppress EOI broadcast support + * to the guest. + * + * In split IRQCHIP mode: advertise unless the VMM explicitly disabled + * it. This preserves legacy quirky behavior where KVM advertised the + * capability even though it did not actually suppress EOIs. + * + * In kernel IRQCHIP mode: only advertise if the VMM explicitly + * enabled it (and use the IOAPIC version 0x20). + */ + if (irqchip_split(kvm)) { + return kvm->arch.suppress_eoi_broadcast_mode != + KVM_SUPPRESS_EOI_BROADCAST_DISABLED; + } else { + return kvm->arch.suppress_eoi_broadcast_mode == + KVM_SUPPRESS_EOI_BROADCAST_ENABLED; + }
Ick, that makes my brain hurt, and obfuscates the nice clean simple ENABLED/DISABLED cases. How about:
switch(kvm->arch.suppress_eoi_broadcast_mode) { case KVM_SUPPRESS_EOI_BROADCAST_ENABLED: return true; case KVM_SUPPRESS_EOI_BROADCAST_DISABLED: return false; default: return !ioapic_in_kernel(kvm); }
(You can do it with if/else if you prefer; this was easier to type into email).
+}
+static bool kvm_lapic_ignore_suppress_eoi_broadcast(struct kvm *kvm) +{ + /* + * Returns true if KVM should ignore the suppress EOI broadcast bit set by + * the guest and broadcast EOIs anyway. + * + * Only returns false when the VMM explicitly enabled Suppress EOI + * broadcast. If disabled by VMM, the bit should be ignored as it is not + * supported. Legacy behavior was to ignore the bit and broadcast EOIs + * anyway. + */ + return kvm->arch.suppress_eoi_broadcast_mode != + KVM_SUPPRESS_EOI_BROADCAST_ENABLED; +}
Still kind of hate the inverse logic and the conjunction of 'ignore suppress' which is hard to parse as a double-negative. What was wrong with a 'kvm_lapic_implement_suppress_eoi_broadcast() which returns true if suppress_eoi_broadcast_mode == KVM_SUPPRESS_EOI_BROADCAST_ENABLED?
On 12 Dec 2025, at 1:01 PM, David Woodhouse dwmw2@infradead.org wrote:
Surely the point of helper functions is that they live in a header file somewhere and are not *duplicated* in the different C files that use them?
@@ -105,6 +105,43 @@ bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector) apic_test_vector(vector, apic->regs + APIC_IRR); } +static bool kvm_lapic_advertise_suppress_eoi_broadcast(struct kvm *kvm) +{
/** Returns true if KVM should advertise Suppress EOI broadcast support* to the guest.** In split IRQCHIP mode: advertise unless the VMM explicitly disabled* it. This preserves legacy quirky behavior where KVM advertised the* capability even though it did not actually suppress EOIs.** In kernel IRQCHIP mode: only advertise if the VMM explicitly* enabled it (and use the IOAPIC version 0x20).*/if (irqchip_split(kvm)) {return kvm->arch.suppress_eoi_broadcast_mode !=KVM_SUPPRESS_EOI_BROADCAST_DISABLED;} else {return kvm->arch.suppress_eoi_broadcast_mode ==KVM_SUPPRESS_EOI_BROADCAST_ENABLED;}Ick, that makes my brain hurt, and obfuscates the nice clean simple ENABLED/DISABLED cases. How about:
switch(kvm->arch.suppress_eoi_broadcast_mode) { case KVM_SUPPRESS_EOI_BROADCAST_ENABLED: return true; case KVM_SUPPRESS_EOI_BROADCAST_DISABLED: return false; default: return !ioapic_in_kernel(kvm); }
Thanks, the switch logic is indeed simpler.
On 12 Dec 2025, at 1:01 PM, David Woodhouse dwmw2@infradead.org wrote:
+}
+static bool kvm_lapic_ignore_suppress_eoi_broadcast(struct kvm *kvm) +{
/** Returns true if KVM should ignore the suppress EOI broadcast bit set by* the guest and broadcast EOIs anyway.** Only returns false when the VMM explicitly enabled Suppress EOI* broadcast. If disabled by VMM, the bit should be ignored as it is not* supported. Legacy behavior was to ignore the bit and broadcast EOIs* anyway.*/return kvm->arch.suppress_eoi_broadcast_mode !=KVM_SUPPRESS_EOI_BROADCAST_ENABLED;+}
Still kind of hate the inverse logic and the conjunction of 'ignore suppress' which is hard to parse as a double-negative. What was wrong with a 'kvm_lapic_implement_suppress_eoi_broadcast() which returns true if suppress_eoi_broadcast_mode == KVM_SUPPRESS_EOI_BROADCAST_ENABLED?
I thought the earlier discussion preferred kvm_lapic_ignore_suppress_eoi_broadcast(), but I’m not tied to it.
On Fri, 2025-12-12 at 08:16 +0000, Khushit Shah wrote:
I thought the earlier discussion preferred kvm_lapic_ignore_suppress_eoi_broadcast(), but I’m not tied to it.
I think some of that earlier discussion was 'informed' by me typing code into my mailer, and managing to type an example whose name implied the exact opposite of what the code actually returned. :)
On 12 Dec 2025, at 1:49 PM, David Woodhouse dwmw2@infradead.org wrote:
On Fri, 2025-12-12 at 08:16 +0000, Khushit Shah wrote:
I thought the earlier discussion preferred kvm_lapic_ignore_suppress_eoi_broadcast(), but I’m not tied to it.
I think some of that earlier discussion was 'informed' by me typing code into my mailer, and managing to type an example whose name implied the exact opposite of what the code actually returned. :)
Got it :), How about kvm_lapic_respect_suppress_eoi_broadcast()? It avoids the double-negative and keeps the semantics clear.
On Fri, 2025-12-12 at 08:27 +0000, Khushit Shah wrote:
On 12 Dec 2025, at 1:49 PM, David Woodhouse dwmw2@infradead.org wrote:
On Fri, 2025-12-12 at 08:16 +0000, Khushit Shah wrote:
I thought the earlier discussion preferred kvm_lapic_ignore_suppress_eoi_broadcast(), but I’m not tied to it.
I think some of that earlier discussion was 'informed' by me typing code into my mailer, and managing to type an example whose name implied the exact opposite of what the code actually returned. :)
Got it :), How about kvm_lapic_respect_suppress_eoi_broadcast()? It avoids the double-negative and keeps the semantics clear.
Sure. That gives us fairly clear separate helpers (in a .h file) which control whether we *advertise* it, and whether we really *do* it.
With them giving consistent 'true' and 'false' for 'ENABLE' and 'DISABLE' modes respectively, and being *inconsistent* for the 'QUIRK' mode.
You could even use the same switch() structure for both, with only the default: case being different, to make it crystal clear what's going on.
linux-stable-mirror@lists.linaro.org
-
David Woodhouse -
Huang, Kai -
Khushit Shah -
Sean Christopherson