[PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery

[PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery

[Hao Zhang]

From: Hao Zhang <zhanghao1@kylinos.cn>

The I/O APIC tracks delivered interrupts in state that is later used to
decide whether an interrupt is still pending or blocked waiting for an
EOI.

For level-triggered interrupts, remote_irr means that a local APIC
accepted the interrupt and that the I/O APIC must wait for the
corresponding EOI before delivering the interrupt again.  For
edge-triggered interrupts, irr_delivered is used to hide delivered
interrupts from KVM_GET_IRQCHIP so that userspace does not reinject an
interrupt that has already left the I/O APIC.

But ioapic_service() currently updates that state before or without
checking that interrupt delivery actually succeeded.
kvm_irq_delivery_to_apic() can return -1 when no destination is found.
Treating failed delivery as success can either leave a level-triggered
pin blocked forever waiting for an EOI that will never be generated, or
cause KVM_GET_IRQCHIP to drop an undelivered edge-triggered interrupt
from the saved IRR state.

Update I/O APIC delivery state only when the delivery result is positive,
i.e. when at least one local APIC accepted the interrupt.

Fixes: 4925663a079c ("KVM: Report IRQ injection status to userspace.")
Fixes: 5bda6eed2e36 ("KVM: ioapic: Record edge-triggered interrupts delivery status")
Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
---
Changes in v2:
 - Address Kai Huang's review by deferring both remote_irr and
   irr_delivered updates until interrupt delivery succeeds.
 - Extend the selftest to cover failed edge-triggered delivery and verify
   that the interrupt remains pending in IRR.

Link to v1: https://lore.kernel.org/all/anSOdijwS6LBbfYJ@192.168.1.215/

 arch/x86/kvm/ioapic.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 757667fb2bfa..540e5665fbe4 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -474,9 +474,6 @@ static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
 	irqe.shorthand = APIC_DEST_NOSHORT;
 	irqe.msi_redir_hint = false;
 
-	if (irqe.trig_mode == IOAPIC_EDGE_TRIG)
-		ioapic->irr_delivered |= 1 << irq;
-
 	if (irq == RTC_GSI && line_status) {
 		/*
 		 * pending_eoi cannot ever become negative (see
@@ -491,8 +488,12 @@ static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
 	} else
 		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
 
-	if (ret && irqe.trig_mode == IOAPIC_LEVEL_TRIG)
-		entry->fields.remote_irr = 1;
+	if (ret > 0) {
+		if (irqe.trig_mode == IOAPIC_EDGE_TRIG)
+			ioapic->irr_delivered |= 1 << irq;
+		else if (irqe.trig_mode == IOAPIC_LEVEL_TRIG)
+			entry->fields.remote_irr = 1;
+	}
 
 	return ret;
 }

base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86
-- 
2.15.0

[PATCH v2 2/2] KVM: selftests: Verify failed IOAPIC delivery preserves state

[Hao Zhang]

From: Hao Zhang <zhanghao1@kylinos.cn>

Add regression coverage for I/O APIC interrupt delivery when no local
APIC can accept the interrupt.

For level-triggered interrupts, verify that failed delivery does not set
remote_irr, because no local APIC accepted the interrupt and no EOI will
ever be generated.

For edge-triggered interrupts, verify that failed delivery does not cause
KVM_GET_IRQCHIP to drop the pending IRR bit.  Edge interrupts are hidden
from the saved IRR state only after they have actually been delivered.

Both cases use an I/O APIC-only GSI and inject the interrupt before
creating any vCPUs, forcing KVM_IRQ_LINE_STATUS to report failed
delivery.

Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
---
 tools/testing/selftests/kvm/Makefile.kvm           |   1 +
 .../testing/selftests/kvm/x86/ioapic_state_test.c  | 108 +++++++++++++++++++++
 2 files changed, 109 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86/ioapic_state_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 6fc34e9bf8e1..89e3f82e75b0 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -86,6 +86,7 @@ TEST_GEN_PROGS_x86 += x86/hyperv_features
 TEST_GEN_PROGS_x86 += x86/hyperv_ipi
 TEST_GEN_PROGS_x86 += x86/hyperv_svm_test
 TEST_GEN_PROGS_x86 += x86/hyperv_tlb_flush
+TEST_GEN_PROGS_x86 += x86/ioapic_state_test
 TEST_GEN_PROGS_x86 += x86/kvm_clock_test
 TEST_GEN_PROGS_x86 += x86/kvm_pv_test
 TEST_GEN_PROGS_x86 += x86/kvm_buslock_test
diff --git a/tools/testing/selftests/kvm/x86/ioapic_state_test.c b/tools/testing/selftests/kvm/x86/ioapic_state_test.c
new file mode 100644
index 000000000000..3613a67f4b21
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/ioapic_state_test.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Regression tests for in-kernel I/O APIC state.
+ */
+
+#include "kvm_util.h"
+#include "test_util.h"
+
+#define TEST_IOAPIC_PIN		16
+#define TEST_VECTOR		0x50
+#define NO_SUCH_APIC_ID		0xfe
+#define TEST_IOAPIC_EDGE_TRIG	0
+#define TEST_IOAPIC_LEVEL_TRIG	1
+
+static void get_ioapic(struct kvm_vm *vm, struct kvm_irqchip *irqchip)
+{
+	int r;
+
+	irqchip->chip_id = KVM_IRQCHIP_IOAPIC;
+	r = __vm_ioctl(vm, KVM_GET_IRQCHIP, irqchip);
+	if (r && errno == ENXIO)
+		__TEST_REQUIRE(0, "In-kernel I/O APIC not available");
+
+	TEST_ASSERT(!r, KVM_IOCTL_ERROR(KVM_GET_IRQCHIP, r));
+}
+
+static void set_ioapic(struct kvm_vm *vm, struct kvm_irqchip *irqchip)
+{
+	irqchip->chip_id = KVM_IRQCHIP_IOAPIC;
+	vm_ioctl(vm, KVM_SET_IRQCHIP, irqchip);
+}
+
+static void set_undeliverable_ioapic_entry(struct kvm_vm *vm,
+					   bool level_triggered)
+{
+	struct kvm_irqchip irqchip;
+
+	get_ioapic(vm, &irqchip);
+
+	irqchip.chip.ioapic.redirtbl[TEST_IOAPIC_PIN].fields.vector = TEST_VECTOR;
+	irqchip.chip.ioapic.redirtbl[TEST_IOAPIC_PIN].fields.dest_id = NO_SUCH_APIC_ID;
+	irqchip.chip.ioapic.redirtbl[TEST_IOAPIC_PIN].fields.dest_mode = 0;
+	irqchip.chip.ioapic.redirtbl[TEST_IOAPIC_PIN].fields.trig_mode =
+		level_triggered ? TEST_IOAPIC_LEVEL_TRIG :
+				  TEST_IOAPIC_EDGE_TRIG;
+	irqchip.chip.ioapic.redirtbl[TEST_IOAPIC_PIN].fields.mask = 0;
+	irqchip.chip.ioapic.redirtbl[TEST_IOAPIC_PIN].fields.remote_irr = 0;
+
+	set_ioapic(vm, &irqchip);
+}
+
+static void test_no_remote_irr_for_undelivered_interrupt(void)
+{
+	struct kvm_irq_level irq = {
+		.irq = TEST_IOAPIC_PIN,
+		.level = 1,
+	};
+	struct kvm_irqchip irqchip;
+	struct kvm_vm *vm;
+
+	vm = vm_create_barebones();
+	vm_create_irqchip(vm);
+
+	set_undeliverable_ioapic_entry(vm, true);
+
+	vm_ioctl(vm, KVM_IRQ_LINE_STATUS, &irq);
+	TEST_ASSERT(irq.status == -1,
+		    "Expected failed interrupt delivery, got %d", irq.status);
+
+	get_ioapic(vm, &irqchip);
+	TEST_ASSERT(!irqchip.chip.ioapic.redirtbl[TEST_IOAPIC_PIN].fields.remote_irr,
+		    "KVM set remote_irr for a level-triggered interrupt that wasn't delivered");
+
+	kvm_vm_free(vm);
+}
+
+static void test_undelivered_edge_interrupt_stays_pending(void)
+{
+	struct kvm_irq_level irq = {
+		.irq = TEST_IOAPIC_PIN,
+		.level = 1,
+	};
+	struct kvm_irqchip irqchip;
+	struct kvm_vm *vm;
+
+	vm = vm_create_barebones();
+	vm_create_irqchip(vm);
+
+	set_undeliverable_ioapic_entry(vm, false);
+
+	vm_ioctl(vm, KVM_IRQ_LINE_STATUS, &irq);
+	TEST_ASSERT(irq.status == -1,
+		    "Expected failed interrupt delivery, got %d", irq.status);
+
+	get_ioapic(vm, &irqchip);
+	TEST_ASSERT(irqchip.chip.ioapic.irr & (1 << TEST_IOAPIC_PIN),
+		    "KVM dropped an undelivered edge-triggered interrupt from IRR");
+
+	kvm_vm_free(vm);
+}
+
+int main(void)
+{
+	test_no_remote_irr_for_undelivered_interrupt();
+	test_undelivered_edge_interrupt_stays_pending();
+
+	return 0;
+}
-- 
2.15.0

Re: [PATCH v2 2/2] KVM: selftests: Verify failed IOAPIC delivery preserves state

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Uninitialized `struct kvm_irqchip` passed to `KVM_GET_IRQCHIP` ioctl leaks stack memory to the kernel, triggering memory sanitizer errors.
--

--- Patch [2]: [PATCH v2 2/2] KVM: selftests: Verify failed IOAPIC delivery preserves state ---
commit bdb3cd0c37699df03cd34fd3d221e67589076c39
Author: Hao Zhang <zhanghao1@kylinos.cn>

KVM: selftests: Verify failed IOAPIC delivery preserves state

This commit adds regression coverage for I/O APIC interrupt delivery when no
local APIC can accept the interrupt. It verifies that failed delivery behaves
correctly for both level-triggered and edge-triggered interrupts.

> diff --git a/tools/testing/selftests/kvm/x86/ioapic_state_test.c b/tools/testing/selftests/kvm/x86/ioapic_state_test.c
> new file mode 100644
> index 0000000000000..3613a67f4b217
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/ioapic_state_test.c
[ ... ]
> +static void set_undeliverable_ioapic_entry(struct kvm_vm *vm,
> +					   bool level_triggered)
> +{
> +	struct kvm_irqchip irqchip;
> +
> +	get_ioapic(vm, &irqchip);

[Severity: Medium]
Will this leak uninitialized stack memory to the kernel?

Since KVM_GET_IRQCHIP is defined as _IOWR, the kernel uses memdup_user() to
copy the entire struct kvm_irqchip from userspace. Because the struct contains
a 512-byte union and is declared uninitialized on the stack, passing it to the
kernel will trigger memory sanitizer or Valgrind errors during testing.

Could this be zero-initialized?

[ ... ]
> +static void test_no_remote_irr_for_undelivered_interrupt(void)
> +{
> +	struct kvm_irq_level irq = {
> +		.irq = TEST_IOAPIC_PIN,
> +		.level = 1,
> +	};
> +	struct kvm_irqchip irqchip;
> +	struct kvm_vm *vm;
> +
> +	vm = vm_create_barebones();
> +	vm_create_irqchip(vm);
> +
> +	set_undeliverable_ioapic_entry(vm, true);
> +
> +	vm_ioctl(vm, KVM_IRQ_LINE_STATUS, &irq);
> +	TEST_ASSERT(irq.status == -1,
> +		    "Expected failed interrupt delivery, got %d", irq.status);
> +
> +	get_ioapic(vm, &irqchip);

[Severity: Medium]
Does this struct also need to be zero-initialized to prevent memory
sanitizer errors when calling the KVM_GET_IRQCHIP ioctl?

[ ... ]
> +static void test_undelivered_edge_interrupt_stays_pending(void)
> +{
> +	struct kvm_irq_level irq = {
> +		.irq = TEST_IOAPIC_PIN,
> +		.level = 1,
> +	};
> +	struct kvm_irqchip irqchip;
> +	struct kvm_vm *vm;
> +
> +	vm = vm_create_barebones();
> +	vm_create_irqchip(vm);
> +
> +	set_undeliverable_ioapic_entry(vm, false);
> +
> +	vm_ioctl(vm, KVM_IRQ_LINE_STATUS, &irq);
> +	TEST_ASSERT(irq.status == -1,
> +		    "Expected failed interrupt delivery, got %d", irq.status);
> +
> +	get_ioapic(vm, &irqchip);

[Severity: Medium]
Does this uninitialized struct also need to be zero-initialized for the
same reason?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/anls-aUppuiFolpS@192.168.1.215?part=2

Re: [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Clearing `irr_delivered` for duplicate edge interrupts causes spurious injections upon VM migration.
--

--- Patch [1]: [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery ---
commit 709ff05e7e6d1df68c0523d442968791b55f37d1
Author: Hao Zhang <zhanghao1@kylinos.cn>

KVM: x86: ioapic: Update state only after successful delivery

This commit defers updating I/O APIC delivery state until interrupt
delivery actually succeeds. It ensures level-triggered pins aren't blocked
forever on a failed delivery and edge-triggered interrupts aren't incorrectly
dropped from the saved IRR state.

> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index 757667fb2bfa0..540e5665fbe4f 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c

[ ... ]

> @@ -491,8 +488,12 @@ static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
>  	} else
>  		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
>  
> -	if (ret && irqe.trig_mode == IOAPIC_LEVEL_TRIG)
> -		entry->fields.remote_irr = 1;
> +	if (ret > 0) {
> +		if (irqe.trig_mode == IOAPIC_EDGE_TRIG)
> +			ioapic->irr_delivered |= 1 << irq;
> +		else if (irqe.trig_mode == IOAPIC_LEVEL_TRIG)
> +			entry->fields.remote_irr = 1;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but while looking at irr_delivered
tracking, does ioapic_set_irq() have an issue with duplicate edge interrupts?

If a device emulator calls KVM_IRQ_LINE multiple times for an edge-triggered
interrupt without an intervening level=0, the first call delivers the
interrupt and sets irr_delivered here in ioapic_service().

However, on subsequent calls, ioapic_set_irq() does this:

arch/x86/kvm/ioapic.c:ioapic_set_irq() {
	...
	old_irr = ioapic->irr;
	ioapic->irr |= mask;
	if (edge) {
		ioapic->irr_delivered &= ~mask;
		if (old_irr == ioapic->irr) {
			ret = 0;
			goto out;
		}
	}
	...
}

It unsets irr_delivered but then detects the duplicate edge and returns early.

Could this leave irr_delivered incorrectly cleared?

If a VM migration occurs after this, KVM_GET_IRQCHIP might read the interrupt
as pending instead of delivered.

Would this cause a spurious injection on the destination?

>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/anls-aUppuiFolpS@192.168.1.215?part=1

Re: [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery

[Huang, Kai]

On Mon, 2026-08-10 at 14:17 +0800, Hao Zhang wrote:
> From: Hao Zhang <zhanghao1@kylinos.cn>
> 
> The I/O APIC tracks delivered interrupts in state that is later used to
> decide whether an interrupt is still pending or blocked waiting for an
> EOI.
> 
> For level-triggered interrupts, remote_irr means that a local APIC
> accepted the interrupt and that the I/O APIC must wait for the
> corresponding EOI before delivering the interrupt again.  For
> edge-triggered interrupts, irr_delivered is used to hide delivered
> interrupts from KVM_GET_IRQCHIP so that userspace does not reinject an
> interrupt that has already left the I/O APIC.
> 
> But ioapic_service() currently updates that state before or without
> checking that interrupt delivery actually succeeded.
> kvm_irq_delivery_to_apic() can return -1 when no destination is found.
> Treating failed delivery as success can either leave a level-triggered
> pin blocked forever waiting for an EOI that will never be generated, or
> cause KVM_GET_IRQCHIP to drop an undelivered edge-triggered interrupt
> from the saved IRR state.
> 
> Update I/O APIC delivery state only when the delivery result is positive,
> i.e. when at least one local APIC accepted the interrupt.
> 
> Fixes: 4925663a079c ("KVM: Report IRQ injection status to userspace.")
> Fixes: 5bda6eed2e36 ("KVM: ioapic: Record edge-triggered interrupts delivery status")
> Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
> ---
> Changes in v2:
>  - Address Kai Huang's review by deferring both remote_irr and
>    irr_delivered updates until interrupt delivery succeeds.
>  - Extend the selftest to cover failed edge-triggered delivery and verify
>    that the interrupt remains pending in IRR.
> 
> Link to v1: https://lore.kernel.org/all/anSOdijwS6LBbfYJ@192.168.1.215/
> 
>  arch/x86/kvm/ioapic.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index 757667fb2bfa..540e5665fbe4 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c
> @@ -474,9 +474,6 @@ static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
>  	irqe.shorthand = APIC_DEST_NOSHORT;
>  	irqe.msi_redir_hint = false;
>  
> -	if (irqe.trig_mode == IOAPIC_EDGE_TRIG)
> -		ioapic->irr_delivered |= 1 << irq;
> -
>  	if (irq == RTC_GSI && line_status) {
>  		/*
>  		 * pending_eoi cannot ever become negative (see
> @@ -491,8 +488,12 @@ static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
>  	} else
>  		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
>  
> -	if (ret && irqe.trig_mode == IOAPIC_LEVEL_TRIG)
> -		entry->fields.remote_irr = 1;
> +	if (ret > 0) {
> +		if (irqe.trig_mode == IOAPIC_EDGE_TRIG)
> +			ioapic->irr_delivered |= 1 << irq;
> +		else if (irqe.trig_mode == IOAPIC_LEVEL_TRIG)
> +			entry->fields.remote_irr = 1;
> +	}
> 

Ah looking at this I immediately realized I misread the code, that I thought the
irr_delivered was for level triggered as well, but actually it is for edge
triggered.  I guess we both got it wrong :-)

For edge triggered IRQ the existing code is correct I believe, since once the
function is called the IRQ is considered delivered no matter whether it is
actually accepted by LAPIC.  I think this exactly reflects the hardware
behaviour.  (In fact, in ioapic_set_irq() you can see the IRQ is removed from
irr_delivered for edge triggered right before ioapic_service() is called.)

For level triggered, I don't see ioapic->irr is cleared by KVM, but only cleared
when ioapic_set_irq() is called with irq_level == 0.  I now (AFAICT) realize it
is the right behaviour since it should be the driver which dessert the level to
stop the level-triggered IRQ.

So in short, I think your v1 is correct, and sorry about my noise. :-(

Re: [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery

[hao_zhang_kdev]

On Mon, Aug 10, 2026, Huang, Kai wrote:
> On Mon, 2026-08-10 at 14:17 +0800, Hao Zhang wrote:
> > From: Hao Zhang <zhanghao1@kylinos.cn>
> > 
> > The I/O APIC tracks delivered interrupts in state that is later used to
> > decide whether an interrupt is still pending or blocked waiting for an
> > EOI.
> > 
> > For level-triggered interrupts, remote_irr means that a local APIC
> > accepted the interrupt and that the I/O APIC must wait for the
> > corresponding EOI before delivering the interrupt again.  For
> > edge-triggered interrupts, irr_delivered is used to hide delivered
> > interrupts from KVM_GET_IRQCHIP so that userspace does not reinject an
> > interrupt that has already left the I/O APIC.
> > 
> > But ioapic_service() currently updates that state before or without
> > checking that interrupt delivery actually succeeded.
> > kvm_irq_delivery_to_apic() can return -1 when no destination is found.
> > Treating failed delivery as success can either leave a level-triggered
> > pin blocked forever waiting for an EOI that will never be generated, or
> > cause KVM_GET_IRQCHIP to drop an undelivered edge-triggered interrupt
> > from the saved IRR state.
> > 
> > Update I/O APIC delivery state only when the delivery result is positive,
> > i.e. when at least one local APIC accepted the interrupt.
> > 
> > Fixes: 4925663a079c ("KVM: Report IRQ injection status to userspace.")
> > Fixes: 5bda6eed2e36 ("KVM: ioapic: Record edge-triggered interrupts delivery status")
> > Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
> > ---
> > Changes in v2:
> >  - Address Kai Huang's review by deferring both remote_irr and
> >    irr_delivered updates until interrupt delivery succeeds.
> >  - Extend the selftest to cover failed edge-triggered delivery and verify
> >    that the interrupt remains pending in IRR.
> > 
> > Link to v1: https://lore.kernel.org/all/anSOdijwS6LBbfYJ@192.168.1.215/
> > 
> >  arch/x86/kvm/ioapic.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> > index 757667fb2bfa..540e5665fbe4 100644
> > --- a/arch/x86/kvm/ioapic.c
> > +++ b/arch/x86/kvm/ioapic.c
> > @@ -474,9 +474,6 @@ static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
> >  	irqe.shorthand = APIC_DEST_NOSHORT;
> >  	irqe.msi_redir_hint = false;
> >  
> > -	if (irqe.trig_mode == IOAPIC_EDGE_TRIG)
> > -		ioapic->irr_delivered |= 1 << irq;
> > -
> >  	if (irq == RTC_GSI && line_status) {
> >  		/*
> >  		 * pending_eoi cannot ever become negative (see
> > @@ -491,8 +488,12 @@ static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
> >  	} else
> >  		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe);
> >  
> > -	if (ret && irqe.trig_mode == IOAPIC_LEVEL_TRIG)
> > -		entry->fields.remote_irr = 1;
> > +	if (ret > 0) {
> > +		if (irqe.trig_mode == IOAPIC_EDGE_TRIG)
> > +			ioapic->irr_delivered |= 1 << irq;
> > +		else if (irqe.trig_mode == IOAPIC_LEVEL_TRIG)
> > +			entry->fields.remote_irr = 1;
> > +	}
> > 
> 
> Ah looking at this I immediately realized I misread the code, that I thought the
> irr_delivered was for level triggered as well, but actually it is for edge
> triggered.  I guess we both got it wrong :-)
> 
> For edge triggered IRQ the existing code is correct I believe, since once the
> function is called the IRQ is considered delivered no matter whether it is
> actually accepted by LAPIC.  I think this exactly reflects the hardware
> behaviour.  (In fact, in ioapic_set_irq() you can see the IRQ is removed from
> irr_delivered for edge triggered right before ioapic_service() is called.)
> 
> For level triggered, I don't see ioapic->irr is cleared by KVM, but only cleared
> when ioapic_set_irq() is called with irq_level == 0.  I now (AFAICT) realize it
> is the right behaviour since it should be the driver which dessert the level to
> stop the level-triggered IRQ.
> 
> So in short, I think your v1 is correct, and sorry about my noise. :-(

Thanks Kai,

I checked the Intel I/O APIC redirection table documentation(https://edc.intel.com/content/www/it/it/design/products-and-solutions/processors-and-chipsets/comet-lake-u/intel-400-series-chipset-on-package-platform-controller-hub-register-database/1.2/redirection-table-entry-0-rte0-offset-10/).  
The Remote IRR bit is defined for level-triggered interrupts; for edge-triggered
interrupts its meaning is undefined. So yes, treating irr_delivered like
remote_irr was wrong.

For edge-triggered interrupts, once the I/O APIC starts delivering the
interrupt, the interrupt should no longer be considered pending in the I/O
APIC state that is saved through KVM_GET_IRQCHIP, regardless of whether a
local APIC eventually accepts it.

So I agree that v2 went too far by moving the irr_delivered update behind
the delivery result check.  I will drop that part and restore the patch to
only fix the level-triggered remote_irr case.

Thanks,
Hao