[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6DF6855A-3F9F-4C31-89FC-6905B11553AF@infradead.org>
Date: Tue, 13 Jan 2026 23:40:31 +0000
From: David Woodhouse <dwmw2@...radead.org>
To: Khushit Shah <khushit.shah@...anix.com>
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>
Subject: Re: [PATCH v5 1/3] KVM: x86: Refactor suppress EOI broadcast logic
On 12 January 2026 04:15:37 GMT, Khushit Shah <khushit.shah@...anix.com> wrote:
>
>
>> On 2 Jan 2026, at 9:53 PM, David Woodhouse <dwmw2@...radead.org> wrote:
>>
>> On Mon, 2025-12-29 at 11:17 +0000, Khushit Shah wrote:
>>> Extract the suppress EOI broadcast (Directed EOI) logic into helper
>>> functions and move the check from kvm_ioapic_update_eoi_one() to
>>> kvm_ioapic_update_eoi() (required for a later patch). Prepare
>>> kvm_ioapic_send_eoi() to honor Suppress EOI Broadcast in split IRQCHIP
>>> mode.
>>>
>>> Introduce two helper functions:
>>> - kvm_lapic_advertise_suppress_eoi_broadcast(): determines whether KVM
>>> should advertise Suppress EOI Broadcast support to the guest
>>> - kvm_lapic_respect_suppress_eoi_broadcast(): determines whether KVM should
>>> honor the guest's request to suppress EOI broadcasts
>>>
>>> This refactoring prepares for I/O APIC version 0x20 support and userspace
>>> control of suppress EOI broadcast behavior.
>>>
>>> Signed-off-by: Khushit Shah <khushit.shah@...anix.com>
>>
>> Looks good to me, thanks for pushing this through to completion!
>>
>>
>> Reviewed-by: David Woodhouse <dwmw@...zon.co.uk>
>>
>> Nit: Ideally I would would prefer to see an explicit 'no functional
>> change intended' and a reference to commit 0bcc3fb95b97a.
>
>
>I took another careful look at the refactor specifically through the
>“no functional change” lens.
This is, of course, exactly why I make people type those words explicitly :)
>The legacy behavior with the in-kernel IRQCHIP can be summarized as:
>- Suppress EOI Broadcast (SEOIB) is not advertised to the guest.
>- If the guest nevertheless enables SEOIB, it is honored (already in un-s
> upported territory).
>- Even in that case, the legacy code still ends up calling
> kvm_notify_acked_irq() in kvm_ioapic_update_eoi_one().
>
>With the refactor, kvm_notify_acked_irq() is no longer reached in this
>specific legacy scenario when the guest enables SEOIB despite it not
>being advertised. I believe this is acceptable, as the guest is relying
>on an unadvertised feature.
That sounds sensible as you describe it.
Note that we did advertise this in the past and then silently just stop doing so potentially underneath already running guests, but that (commit
0bcc3fb95b97a) was back in 2018 so I guess there won't be many "innocent victim" guests around any more who genuinely did see the feature advertised.
>For non-QUIRKED configurations, the behavior is also correct:
>- When SEOIB is ENABLED, kvm_notify_acked_irq() is called on EOIR write,
> when enabled by guest.
>- When SEOIB is DISABLED, kvm_notify_acked_irq() is called on EOI
> broadcast.
>
>I would appreciate others chiming in if they see a reason to preserve
>the legacy ack behavior even in the unsupported case.
LGTM. Thanks.
Powered by blists - more mailing lists