| Newsgroups |
org.kernel.vger.kvm |
| Message-ID |
<[email protected]> |
On Mon, Aug 10, 2026, Huang, Kai wrote:
> On Mon, 2026-08-10 at 14:17 +0800, Hao Zhang wrote:
> > From: Hao Zhang <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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/[email protected]/
> >
> > 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