linux-kernel - Re: [PATCH v3] KVM: x86: Add x2APIC "features" to control EOI broadcast suppression

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <A922DCC2-4CB4-4DE8-82FA-95B502B3FCD4@nutanix.com>
Date: Tue, 2 Dec 2025 12:58:52 +0000
From: Khushit Shah <khushit.shah@...anix.com>
To: David Woodhouse <dwmw2@...radead.org>
CC: "seanjc@...gle.com" <seanjc@...gle.com>,
        "pbonzini@...hat.com"
	<pbonzini@...hat.com>,
        "kai.huang@...el.com" <kai.huang@...el.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "x86@...nel.org" <x86@...nel.org>, "bp@...en8.de" <bp@...en8.de>,
        "hpa@...or.com" <hpa@...or.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "dave.hansen@...ux.intel.com"
	<dave.hansen@...ux.intel.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        Jon
 Kohler <jon@...anix.com>,
        Shaju Abraham <shaju.abraham@...anix.com>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH v3] KVM: x86: Add x2APIC "features" to control EOI
 broadcast suppression

Thanks for the review!

> On 2 Dec 2025, at 2:43 PM, David Woodhouse <dwmw2@...radead.org> wrote:
> 
> Firstly, excellent work debugging and diagnosing that!
> 
> On Tue, 2025-11-25 at 18:05 +0000, Khushit Shah wrote:
>> 
>> --- 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_API_DISABLE_IGNORE_SUPPRESS_EOI_BROADCAST_QUIRK (1ULL << 2)
>> +  #define KVM_X2APIC_API_DISABLE_SUPPRESS_EOI_BROADCAST              (1ULL << 3)
>> 
> 
> I kind of hate these names. This part right here is what we leave
> behind for future generations, to understand the weird behaviour of
> KVM. To have "IGNORE" "SUPPRESS" "QUIRK" all in the same flag, quite
> apart from the length of the token, makes my brain hurt.

Yes, I agree the original name is too wordy. How about renaming it to
KVM_X2APIC_API_ACTUALLY_SUPPRESS_EOI_BROADCASTS?
That makes the intended KVM behaviour clear.

I'm also not very keen on ENABLE_SUPPRESS_EOI_BROADCAST
it reads as if KVM is the one enabling the feature, which isn't the case.
The guest decides whether to enable suppression; KVM should just
advertise the capability correctly and then respect whatever the guest
chooses.

> On 2 Dec 2025, at 2:43 PM, David Woodhouse <dwmw2@...radead.org> wrote:
> 
>>  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,14 @@ 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_API_DISABLE_IGNORE_SUPPRESS_EOI_BROADCAST_QUIRK overrides
>> +KVM's quirky behavior of not actually suppressing EOI broadcasts for split IRQ
>> +chips when support for Suppress EOI Broadcasts is advertised to the guest.
> 
> This paragraph doesn't actually say what the flag *does*, only the old
> behaviour that it overrides?

Right, good point. I'll update the doc.