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)