KVM: x86: Add userspace control for Suppress EOI Broadcast

[PATCH v5 2/3] KVM: x86/ioapic: Implement support for I/O APIC version 0x20 with EOIR

Posted by Khushit Shah 1 month, 1 week ago

From: David Woodhouse <dwmw@amazon.co.uk>

Introduce support for I/O APIC version 0x20, which includes the EOI
Register (EOIR) for directed EOI.  The EOI register allows guests to
perform EOIs to individual I/O APICs instead of relying on broadcast EOIs
from the local APIC.

When Suppress EOI Broadcast (SEOIB) capability is advertised to the guest,
guests that enable it will EOI individual I/O APICs by writing to their
EOI register instead of relying on broadcast EOIs from the LAPIC.  Hence,
when SEOIB is advertised (so that guests can use it if they choose), use
I/O APIC version 0x20 to provide the EOI register.  This prepares for a
userspace API that will allow explicit control of SEOIB support, providing
a consistent interface for both in-kernel and split IRQCHIP mode.

Add a tracepoint (kvm_ioapic_directed_eoi) to track directed EOIs for
debugging and observability.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Khushit Shah <khushit.shah@nutanix.com>
---
 arch/x86/kvm/ioapic.c | 31 +++++++++++++++++++++++++++++--
 arch/x86/kvm/ioapic.h | 19 +++++++++++--------
 arch/x86/kvm/trace.h  | 17 +++++++++++++++++
 3 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 6bf8d110aece..eea1eb7845c4 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -48,8 +48,11 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic)
 
 	switch (ioapic->ioregsel) {
 	case IOAPIC_REG_VERSION:
-		result = ((((IOAPIC_NUM_PINS - 1) & 0xff) << 16)
-			  | (IOAPIC_VERSION_ID & 0xff));
+		if (kvm_lapic_advertise_suppress_eoi_broadcast(ioapic->kvm))
+			result = IOAPIC_VERSION_ID_EOIR;
+		else
+			result = IOAPIC_VERSION_ID;
+		result |= ((IOAPIC_NUM_PINS - 1) & 0xff) << 16;
 		break;
 
 	case IOAPIC_REG_APIC_ID:
@@ -57,6 +60,10 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic)
 		result = ((ioapic->id & 0xf) << 24);
 		break;
 
+	case IOAPIC_REG_BOOT_CONFIG:
+		result = 0x01; /* Processor bus */
+		break;
+
 	default:
 		{
 			u32 redir_index = (ioapic->ioregsel - 0x10) >> 1;
@@ -701,6 +708,26 @@ static int ioapic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 		ioapic_write_indirect(ioapic, data);
 		break;
 
+	case IOAPIC_REG_EOIR:
+		/*
+		 * The EOI register is supported (and version 0x20 advertised)
+		 * when userspace explicitly enables suppress EOI broadcast.
+		 */
+		if (kvm_lapic_advertise_suppress_eoi_broadcast(vcpu->kvm)) {
+			u8 vector = data & 0xff;
+			int i;
+
+			trace_kvm_ioapic_directed_eoi(vcpu, vector);
+			rtc_irq_eoi(ioapic, vcpu, vector);
+			for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+				union kvm_ioapic_redirect_entry *ent = &ioapic->redirtbl[i];
+
+				if (ent->fields.vector != vector)
+					continue;
+				kvm_ioapic_update_eoi_one(vcpu, ioapic, ent->fields.trig_mode, i);
+			}
+		}
+		break;
 	default:
 		break;
 	}
diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
index bf28dbc11ff6..f219577f738c 100644
--- a/arch/x86/kvm/ioapic.h
+++ b/arch/x86/kvm/ioapic.h
@@ -11,7 +11,8 @@ struct kvm_vcpu;
 
 #define IOAPIC_NUM_PINS  KVM_IOAPIC_NUM_PINS
 #define MAX_NR_RESERVED_IOAPIC_PINS KVM_MAX_IRQ_ROUTES
-#define IOAPIC_VERSION_ID 0x11	/* IOAPIC version */
+#define IOAPIC_VERSION_ID	0x11	/* Default IOAPIC version */
+#define IOAPIC_VERSION_ID_EOIR	0x20	/* IOAPIC version with EOIR support */
 #define IOAPIC_EDGE_TRIG  0
 #define IOAPIC_LEVEL_TRIG 1
 
@@ -19,13 +20,15 @@ struct kvm_vcpu;
 #define IOAPIC_MEM_LENGTH            0x100
 
 /* Direct registers. */
-#define IOAPIC_REG_SELECT  0x00
-#define IOAPIC_REG_WINDOW  0x10
-
-/* Indirect registers. */
-#define IOAPIC_REG_APIC_ID 0x00	/* x86 IOAPIC only */
-#define IOAPIC_REG_VERSION 0x01
-#define IOAPIC_REG_ARB_ID  0x02	/* x86 IOAPIC only */
+#define IOAPIC_REG_SELECT	0x00
+#define IOAPIC_REG_WINDOW	0x10
+#define IOAPIC_REG_EOIR	0x40	/* version 0x20+ only */
+
+/* INDIRECT registers. */
+#define IOAPIC_REG_APIC_ID	0x00	/* x86 IOAPIC only */
+#define IOAPIC_REG_VERSION	0x01
+#define IOAPIC_REG_ARB_ID	0x02	/* x86 IOAPIC only */
+#define IOAPIC_REG_BOOT_CONFIG	0x03	/* x86 IOAPIC only */
 
 /*ioapic delivery mode*/
 #define	IOAPIC_FIXED			0x0
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index e79bc9cb7162..6902758353a9 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -315,6 +315,23 @@ TRACE_EVENT(kvm_ioapic_delayed_eoi_inj,
 		  (__entry->e & (1<<15)) ? "level" : "edge",
 		  (__entry->e & (1<<16)) ? "|masked" : "")
 );
+
+TRACE_EVENT(kvm_ioapic_directed_eoi,
+	    TP_PROTO(struct kvm_vcpu *vcpu, u8 vector),
+	    TP_ARGS(vcpu, vector),
+
+	TP_STRUCT__entry(
+		__field(	__u32,		apicid		)
+		__field(	__u8,		vector		)
+	),
+
+	TP_fast_assign(
+		__entry->apicid		= vcpu->vcpu_id;
+		__entry->vector		= vector;
+	),
+
+	TP_printk("apicid %x vector %u", __entry->apicid, __entry->vector)
+);
 #endif
 
 TRACE_EVENT(kvm_msi_set_irq,
-- 
2.39.3

Re: [PATCH v5 2/3] KVM: x86/ioapic: Implement support for I/O APIC version 0x20 with EOIR

Posted by David Woodhouse 1 month, 1 week ago

On 29 December 2025 11:17:07 GMT, Khushit Shah <khushit.shah@nutanix.com> wrote:
>From: David Woodhouse <dwmw@amazon.co.uk>
>
>Introduce support for I/O APIC version 0x20, which includes the EOI
>Register (EOIR) for directed EOI.  The EOI register allows guests to
>perform EOIs to individual I/O APICs instead of relying on broadcast EOIs
>from the local APIC.
>
>When Suppress EOI Broadcast (SEOIB) capability is advertised to the guest,
>guests that enable it will EOI individual I/O APICs by writing to their
>EOI register instead of relying on broadcast EOIs from the LAPIC.  Hence,
>when SEOIB is advertised (so that guests can use it if they choose), use
>I/O APIC version 0x20 to provide the EOI register.  This prepares for a
>userspace API that will allow explicit control of SEOIB support, providing
>a consistent interface for both in-kernel and split IRQCHIP mode.
>
>Add a tracepoint (kvm_ioapic_directed_eoi) to track directed EOIs for
>debugging and observability.
>
>Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
>Signed-off-by: Khushit Shah <khushit.shah@nutanix.com>
>---
> arch/x86/kvm/ioapic.c | 31 +++++++++++++++++++++++++++++--
> arch/x86/kvm/ioapic.h | 19 +++++++++++--------
> arch/x86/kvm/trace.h  | 17 +++++++++++++++++
> 3 files changed, 57 insertions(+), 10 deletions(-)
>
>diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
>index 6bf8d110aece..eea1eb7845c4 100644
>--- a/arch/x86/kvm/ioapic.c
>+++ b/arch/x86/kvm/ioapic.c
>@@ -48,8 +48,11 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic)
> 
> 	switch (ioapic->ioregsel) {
> 	case IOAPIC_REG_VERSION:
>-		result = ((((IOAPIC_NUM_PINS - 1) & 0xff) << 16)
>-			  | (IOAPIC_VERSION_ID & 0xff));
>+		if (kvm_lapic_advertise_suppress_eoi_broadcast(ioapic->kvm))
>+			result = IOAPIC_VERSION_ID_EOIR;
>+		else
>+			result = IOAPIC_VERSION_ID;
>+		result |= ((IOAPIC_NUM_PINS - 1) & 0xff) << 16;

I think that wants to depend on _respect_ not _advertise_? Otherwise you're changing existing behaviour in the legacy/quirk case where the VMM neither explicitly enables not disables the feature.

> 		break;
> 
> 	case IOAPIC_REG_APIC_ID:
>@@ -57,6 +60,10 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic)
> 		result = ((ioapic->id & 0xf) << 24);
> 		break;
> 
>+	case IOAPIC_REG_BOOT_CONFIG:
>+		result = 0x01; /* Processor bus */
>+		break;
>+
> 	default:
> 		{
> 			u32 redir_index = (ioapic->ioregsel - 0x10) >> 1;
>@@ -701,6 +708,26 @@ static int ioapic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
> 		ioapic_write_indirect(ioapic, data);
> 		break;
> 
>+	case IOAPIC_REG_EOIR:
>+		/*
>+		 * The EOI register is supported (and version 0x20 advertised)
>+		 * when userspace explicitly enables suppress EOI broadcast.
>+		 */
>+		if (kvm_lapic_advertise_suppress_eoi_broadcast(vcpu->kvm)) {

I'm torn, but I suspect this one should be conditional on _respect_ too. A guest shouldn't be trying this register unless the version register suggests that it exists anyway.


>+			u8 vector = data & 0xff;
>+			int i;
>+
>+			trace_kvm_ioapic_directed_eoi(vcpu, vector);
>+			rtc_irq_eoi(ioapic, vcpu, vector);
>+			for (i = 0; i < IOAPIC_NUM_PINS; i++) {
>+				union kvm_ioapic_redirect_entry *ent = &ioapic->redirtbl[i];
>+
>+				if (ent->fields.vector != vector)
>+					continue;
>+				kvm_ioapic_update_eoi_one(vcpu, ioapic, ent->fields.trig_mode, i);
>+			}
>+		}
>+		break;
> 	default:
> 		break;
> 	}
>diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
>index bf28dbc11ff6..f219577f738c 100644
>--- a/arch/x86/kvm/ioapic.h
>+++ b/arch/x86/kvm/ioapic.h
>@@ -11,7 +11,8 @@ struct kvm_vcpu;
> 
> #define IOAPIC_NUM_PINS  KVM_IOAPIC_NUM_PINS
> #define MAX_NR_RESERVED_IOAPIC_PINS KVM_MAX_IRQ_ROUTES
>-#define IOAPIC_VERSION_ID 0x11	/* IOAPIC version */
>+#define IOAPIC_VERSION_ID	0x11	/* Default IOAPIC version */
>+#define IOAPIC_VERSION_ID_EOIR	0x20	/* IOAPIC version with EOIR support */
> #define IOAPIC_EDGE_TRIG  0
> #define IOAPIC_LEVEL_TRIG 1
> 
>@@ -19,13 +20,15 @@ struct kvm_vcpu;
> #define IOAPIC_MEM_LENGTH            0x100
> 
> /* Direct registers. */
>-#define IOAPIC_REG_SELECT  0x00
>-#define IOAPIC_REG_WINDOW  0x10
>-
>-/* Indirect registers. */
>-#define IOAPIC_REG_APIC_ID 0x00	/* x86 IOAPIC only */
>-#define IOAPIC_REG_VERSION 0x01
>-#define IOAPIC_REG_ARB_ID  0x02	/* x86 IOAPIC only */
>+#define IOAPIC_REG_SELECT	0x00
>+#define IOAPIC_REG_WINDOW	0x10
>+#define IOAPIC_REG_EOIR	0x40	/* version 0x20+ only */
>+
>+/* INDIRECT registers. */
>+#define IOAPIC_REG_APIC_ID	0x00	/* x86 IOAPIC only */
>+#define IOAPIC_REG_VERSION	0x01
>+#define IOAPIC_REG_ARB_ID	0x02	/* x86 IOAPIC only */
>+#define IOAPIC_REG_BOOT_CONFIG	0x03	/* x86 IOAPIC only */
> 
> /*ioapic delivery mode*/
> #define	IOAPIC_FIXED			0x0
>diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
>index e79bc9cb7162..6902758353a9 100644
>--- a/arch/x86/kvm/trace.h
>+++ b/arch/x86/kvm/trace.h
>@@ -315,6 +315,23 @@ TRACE_EVENT(kvm_ioapic_delayed_eoi_inj,
> 		  (__entry->e & (1<<15)) ? "level" : "edge",
> 		  (__entry->e & (1<<16)) ? "|masked" : "")
> );
>+
>+TRACE_EVENT(kvm_ioapic_directed_eoi,
>+	    TP_PROTO(struct kvm_vcpu *vcpu, u8 vector),
>+	    TP_ARGS(vcpu, vector),
>+
>+	TP_STRUCT__entry(
>+		__field(	__u32,		apicid		)
>+		__field(	__u8,		vector		)
>+	),
>+
>+	TP_fast_assign(
>+		__entry->apicid		= vcpu->vcpu_id;
>+		__entry->vector		= vector;
>+	),
>+
>+	TP_printk("apicid %x vector %u", __entry->apicid, __entry->vector)
>+);
> #endif
> 
> TRACE_EVENT(kvm_msi_set_irq,

Re: [PATCH v5 2/3] KVM: x86/ioapic: Implement support for I/O APIC version 0x20 with EOIR

Posted by Khushit Shah 1 month, 1 week ago

> On 29 Dec 2025, at 5:09 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> 
> On 29 December 2025 11:17:07 GMT, Khushit Shah <khushit.shah@nutanix.com> wrote:
>> From: David Woodhouse <dwmw@amazon.co.uk>
>> 
>> Introduce support for I/O APIC version 0x20, which includes the EOI
>> Register (EOIR) for directed EOI.  The EOI register allows guests to
>> perform EOIs to individual I/O APICs instead of relying on broadcast EOIs
>> from the local APIC.
>> 
>> When Suppress EOI Broadcast (SEOIB) capability is advertised to the guest,
>> guests that enable it will EOI individual I/O APICs by writing to their
>> EOI register instead of relying on broadcast EOIs from the LAPIC.  Hence,
>> when SEOIB is advertised (so that guests can use it if they choose), use
>> I/O APIC version 0x20 to provide the EOI register.  This prepares for a
>> userspace API that will allow explicit control of SEOIB support, providing
>> a consistent interface for both in-kernel and split IRQCHIP mode.
>> 
>> Add a tracepoint (kvm_ioapic_directed_eoi) to track directed EOIs for
>> debugging and observability.
>> 
>> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
>> Signed-off-by: Khushit Shah <khushit.shah@nutanix.com>
>> ---
>> arch/x86/kvm/ioapic.c | 31 +++++++++++++++++++++++++++++--
>> arch/x86/kvm/ioapic.h | 19 +++++++++++--------
>> arch/x86/kvm/trace.h  | 17 +++++++++++++++++
>> 3 files changed, 57 insertions(+), 10 deletions(-)
>> 
>> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
>> index 6bf8d110aece..eea1eb7845c4 100644
>> --- a/arch/x86/kvm/ioapic.c
>> +++ b/arch/x86/kvm/ioapic.c
>> @@ -48,8 +48,11 @@ static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic)
>> 
>> switch (ioapic->ioregsel) {
>> case IOAPIC_REG_VERSION:
>> - result = ((((IOAPIC_NUM_PINS - 1) & 0xff) << 16)
>> -   | (IOAPIC_VERSION_ID & 0xff));
>> + if (kvm_lapic_advertise_suppress_eoi_broadcast(ioapic->kvm))
>> + result = IOAPIC_VERSION_ID_EOIR;
>> + else
>> + result = IOAPIC_VERSION_ID;
>> + result |= ((IOAPIC_NUM_PINS - 1) & 0xff) << 16;
> 
> I think that wants to depend on _respect_ not _advertise_? Otherwise you're changing existing behaviour in the legacy/quirk case where the VMM neither explicitly enables not disables the feature.

I think _advertise_ is correct, as for legacy case, in kernel IRQCHIP mode, _advertise_ is false. For kernel IRQCHIP, _advertise_ is only true when *enabled*.

Re: [PATCH v5 2/3] KVM: x86/ioapic: Implement support for I/O APIC version 0x20 with EOIR

Posted by David Woodhouse 1 month, 1 week ago

On Mon, 2025-12-29 at 12:21 +0000, Khushit Shah wrote:
> 
> > On 29 Dec 2025, at 5:09 PM, David Woodhouse <dwmw2@infradead.org>
> > wrote:
> > 
> > On 29 December 2025 11:17:07 GMT, Khushit Shah
> > <khushit.shah@nutanix.com> wrote:
> > > From: David Woodhouse <dwmw@amazon.co.uk>
> > > 
> > > Introduce support for I/O APIC version 0x20, which includes the
> > > EOI
> > > Register (EOIR) for directed EOI.  The EOI register allows guests
> > > to
> > > perform EOIs to individual I/O APICs instead of relying on
> > > broadcast EOIs
> > > from the local APIC.
> > > 
> > > When Suppress EOI Broadcast (SEOIB) capability is advertised to
> > > the guest,
> > > guests that enable it will EOI individual I/O APICs by writing to
> > > their
> > > EOI register instead of relying on broadcast EOIs from the
> > > LAPIC.  Hence,
> > > when SEOIB is advertised (so that guests can use it if they
> > > choose), use
> > > I/O APIC version 0x20 to provide the EOI register.  This prepares
> > > for a
> > > userspace API that will allow explicit control of SEOIB support,
> > > providing
> > > a consistent interface for both in-kernel and split IRQCHIP mode.
> > > 
> > > Add a tracepoint (kvm_ioapic_directed_eoi) to track directed EOIs
> > > for
> > > debugging and observability.
> > > 
> > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > > Signed-off-by: Khushit Shah <khushit.shah@nutanix.com>
> > > ---
> > > arch/x86/kvm/ioapic.c | 31 +++++++++++++++++++++++++++++--
> > > arch/x86/kvm/ioapic.h | 19 +++++++++++--------
> > > arch/x86/kvm/trace.h  | 17 +++++++++++++++++
> > > 3 files changed, 57 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> > > index 6bf8d110aece..eea1eb7845c4 100644
> > > --- a/arch/x86/kvm/ioapic.c
> > > +++ b/arch/x86/kvm/ioapic.c
> > > @@ -48,8 +48,11 @@ static unsigned long
> > > ioapic_read_indirect(struct kvm_ioapic *ioapic)
> > > 
> > > switch (ioapic->ioregsel) {
> > > case IOAPIC_REG_VERSION:
> > > - result = ((((IOAPIC_NUM_PINS - 1) & 0xff) << 16)
> > > -   | (IOAPIC_VERSION_ID & 0xff));
> > > + if (kvm_lapic_advertise_suppress_eoi_broadcast(ioapic->kvm))
> > > + result = IOAPIC_VERSION_ID_EOIR;
> > > + else
> > > + result = IOAPIC_VERSION_ID;
> > > + result |= ((IOAPIC_NUM_PINS - 1) & 0xff) << 16;
> > 
> > I think that wants to depend on _respect_ not _advertise_?
> > Otherwise you're changing existing behaviour in the legacy/quirk
> > case where the VMM neither explicitly enables not disables the
> > feature.
> 
> I think _advertise_ is correct, as for legacy case, in kernel IRQCHIP
> mode, _advertise_ is false. For kernel IRQCHIP, _advertise_ is only
> true when *enabled*.

Hm? IIUC kvm_lapic_advertise_suppress_eoi_broadcast() is true whenever
userspace *hasn't* set KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST
(either userspace has explicitly *enabled* it instead, or userspace has
done neither and we should preserve the legacy behaviour).

If the kernel I/O APIC is enabled when userspace has not explicitly
either enabled or disabled EOI suppression, then the I/O APIC should
advertise precisely the same features as before. As far as I can tell,
this will make the kernel I/O APIC advertise the newer version and
support the EOI register in that legacy case, which it shouldn't?

Re: [PATCH v5 2/3] KVM: x86/ioapic: Implement support for I/O APIC version 0x20 with EOIR

Posted by Khushit Shah 1 month, 1 week ago

> On 29 Dec 2025, at 6:31 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> 
> Hm? IIUC kvm_lapic_advertise_suppress_eoi_broadcast() is true whenever
> userspace *hasn't* set KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST
> (either userspace has explicitly *enabled* it instead, or userspace has
> done neither and we should preserve the legacy behaviour).

The legacy behaviour for "kvm_lapic_advertise_suppress_eoi_broadcast()" is:
- true for split IRQCHIP (userspace I/O APIC)
- false for in-kernel IRQCHIP

The in-kernel IRQCHIP case was "fixed" by commit 0bcc3fb95b97 ("KVM: lapic:
stop advertising DIRECTED_EOI when in-kernel IOAPIC is in use"), which made
it return false when IOAPIC is in-kernel.

With this series, in QUIRKED mode the function still returns !ioapic_in_kernel(),
preserving that exact legacy behavior. The I/O APIC version 0x20 (with EOIR)
is only used when userspace explicitly sets the ENABLE flag.

The comments in patch 1 explain this in more detail ;)

Re: [PATCH v5 2/3] KVM: x86/ioapic: Implement support for I/O APIC version 0x20 with EOIR

Posted by David Woodhouse 1 month, 1 week ago

On 29 December 2025 15:16:40 GMT, Khushit Shah <khushit.shah@nutanix.com> wrote:
>
>
>> On 29 Dec 2025, at 6:31 PM, David Woodhouse <dwmw2@infradead.org> wrote:
>> 
>> Hm? IIUC kvm_lapic_advertise_suppress_eoi_broadcast() is true whenever
>> userspace *hasn't* set KVM_X2APIC_DISABLE_SUPPRESS_EOI_BROADCAST
>> (either userspace has explicitly *enabled* it instead, or userspace has
>> done neither and we should preserve the legacy behaviour).
>
>The legacy behaviour for "kvm_lapic_advertise_suppress_eoi_broadcast()" is:
>- true for split IRQCHIP (userspace I/O APIC)
>- false for in-kernel IRQCHIP
>
>The in-kernel IRQCHIP case was "fixed" by commit 0bcc3fb95b97 ("KVM: lapic:
>stop advertising DIRECTED_EOI when in-kernel IOAPIC is in use"), which made
>it return false when IOAPIC is in-kernel.
>
>With this series, in QUIRKED mode the function still returns !ioapic_in_kernel(),
>preserving that exact legacy behavior. The I/O APIC version 0x20 (with EOIR)
>is only used when userspace explicitly sets the ENABLE flag.
>
>The comments in patch 1 explain this in more detail ;)

Ah, OK. So in the case of in-kernel I/O APIC, kvm_lapic_advertise_suppress_eoi_broadcast() kvm_lapic_respect_suppress_eoi_broadcast() are the same. In that case we can choose the one which is easier to understand and doesn't need the reader to refer back to an earlier commit? I accept your correction; the patch is correct.

But I think I still prefer the check to be on _respect_ as it's clearer that it's part of the new behaviour that is only introduced with this series.

Re: [PATCH v5 2/3] KVM: x86/ioapic: Implement support for I/O APIC version 0x20 with EOIR

Posted by Khushit Shah 1 month, 1 week ago

> On 29 Dec 2025, at 9:06 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> 
> Ah, OK. So in the case of in-kernel I/O APIC, kvm_lapic_advertise_suppress_eoi_broadcast() kvm_lapic_respect_suppress_eoi_broadcast() are the same. In that case we can choose the one which is easier to understand and doesn't need the reader to refer back to an earlier commit? I accept your correction; the patch is correct.
> 
> But I think I still prefer the check to be on _respect_ as it's clearer that it's part of the new behaviour that is only introduced with this series.

We can't use `_respect_` here because in QUIRKED mode with in-kernel IRQCHIP:

  advertise = false  (version 0x11 advertised, no EOIR register)
  respect   = true   (legacy quirk: honor SPIV bit even if not advertised)

While it is true that when SEOIB is not advertised,  the bit should not
be respected. However, the legacy KVM implementation still respected the
SPIV bit in kvm_ioapic_update_eoi_one() even when not advertising SEOIB.
I've preserved that legacy behavior in `_respect_` for QUIRKED mode.

I think the logic is straightforward: if we advertise SEOIB while using in-kernel
IRQCHIP, use I/O APIC version 0x20.

Re: [PATCH v5 2/3] KVM: x86/ioapic: Implement support for I/O APIC version 0x20 with EOIR

Posted by David Woodhouse 1 month, 1 week ago

On Mon, 2025-12-29 at 15:57 +0000, Khushit Shah wrote:
> 
> 
> We can't use `_respect_` here because in QUIRKED mode with in-kernel IRQCHIP:
> 
>   advertise = false  (version 0x11 advertised, no EOIR register)
>   respect   = true   (legacy quirk: honor SPIV bit even if not advertised)

Oh wow, right. Since commit 0bcc3fb95b97a ("KVM: lapic: stop
advertising DIRECTED_EOI when in-kernel IOAPIC is in use"), KVM with
the in-kernel I/O APIC will *not* advertise the EOI suppression in the
local APIC version register… but does actually honour the DIRECTED_EOI
bit if the guest sets it anyway.

While with a userspace I/O APIC, KVM *will* advertise it, but not
honour it.

Yay.

So yes, your code is the best way to do it. Sorry for the noise.

Re: [PATCH v5 2/3] KVM: x86/ioapic: Implement support for I/O APIC version 0x20 with EOIR

Posted by Khushit Shah 4 weeks ago


> On 2 Jan 2026, at 9:47 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> 
> On Mon, 2025-12-29 at 15:57 +0000, Khushit Shah wrote:
>> 
>> 
>> We can't use `_respect_` here because in QUIRKED mode with in-kernel IRQCHIP:
>> 
>>   advertise = false  (version 0x11 advertised, no EOIR register)
>>   respect   = true   (legacy quirk: honor SPIV bit even if not advertised)
> 
> Oh wow, right. Since commit 0bcc3fb95b97a ("KVM: lapic: stop
> advertising DIRECTED_EOI when in-kernel IOAPIC is in use"), KVM with
> the in-kernel I/O APIC will *not* advertise the EOI suppression in the
> local APIC version register… but does actually honour the DIRECTED_EOI
> bit if the guest sets it anyway.
> 
> While with a userspace I/O APIC, KVM *will* advertise it, but not
> honour it.
> 
> Yay.
> 
> So yes, your code is the best way to do it. Sorry for the noise.


Thanks, for the review!