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.