* [PATCH v2 2/2] KVM: selftests: Verify failed IOAPIC delivery preserves state
2026-08-10 6:17 [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery Hao Zhang
@ 2026-08-10 6:22 ` Hao Zhang
2026-08-10 6:33 ` sashiko-bot
2026-08-10 6:45 ` [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery sashiko-bot
2026-08-10 8:34 ` Huang, Kai
2 siblings, 1 reply; 6+ messages in thread
From: Hao Zhang @ 2026-08-10 6:22 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, kvm
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] KVM: selftests: Verify failed IOAPIC delivery preserves state
2026-08-10 6:22 ` [PATCH v2 2/2] KVM: selftests: Verify failed IOAPIC delivery preserves state Hao Zhang
@ 2026-08-10 6:33 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-10 6:33 UTC (permalink / raw)
To: Hao Zhang; +Cc: kvm
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery
2026-08-10 6:17 [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery Hao Zhang
2026-08-10 6:22 ` [PATCH v2 2/2] KVM: selftests: Verify failed IOAPIC delivery preserves state Hao Zhang
@ 2026-08-10 6:45 ` sashiko-bot
2026-08-10 8:34 ` Huang, Kai
2 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-10 6:45 UTC (permalink / raw)
To: Hao Zhang; +Cc: kvm
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery
2026-08-10 6:17 [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery Hao Zhang
2026-08-10 6:22 ` [PATCH v2 2/2] KVM: selftests: Verify failed IOAPIC delivery preserves state Hao Zhang
2026-08-10 6:45 ` [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery sashiko-bot
@ 2026-08-10 8:34 ` Huang, Kai
2026-08-10 9:39 ` hao_zhang_kdev
2 siblings, 1 reply; 6+ messages in thread
From: Huang, Kai @ 2026-08-10 8:34 UTC (permalink / raw)
To: seanjc@google.com, hao_zhang_kdev@163.com
Cc: kvm@vger.kernel.org, pbonzini@redhat.com
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. :-(
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] KVM: x86: ioapic: Update state only after successful delivery
2026-08-10 8:34 ` Huang, Kai
@ 2026-08-10 9:39 ` hao_zhang_kdev
0 siblings, 0 replies; 6+ messages in thread
From: hao_zhang_kdev @ 2026-08-10 9:39 UTC (permalink / raw)
To: Huang, Kai
Cc: seanjc@google.com, hao_zhang_kdev@163.com, kvm@vger.kernel.org,
pbonzini@redhat.com
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
^ permalink raw reply [flat|nested] 6+ messages in thread