[PATCH RFC 1/1] x86/irq/i8259: Ack PIC vectors in the LAPIC ISR

[Dongli Zhang <dongli.zhang@oracle.com>]

This is to fix a LAPIC ISR stall that can happen in a kdump kernel, but can
also happen in other scenarios.

The issue can be intermittently reproduced in a QEMU/KVM guest when there
are large number of VMs on the same host.

Device interrupts can use the PIC, IOAPIC, or MSI-X. Nowadays, most x86
device interrupts use MSI-X or the IOAPIC. However, the PIC handler does
not send a LAPIC EOI, even when the interrupt it handles has set a bit in
the LAPIC ISR.

For example, suppose there is a QEMU/KVM guest using virtio-net.

1. virtio-net is about to deliver vector 52 to CPU 2 (APIC ID 2), but the
guest suddenly panics on that CPU.

2. The kdump kernel boots on APIC ID 2, which becomes CPU 0 in the new
kernel.

3. Suppose delivery of the interrupt is delayed, so vector 52 has not
reached the CPU yet.

4. On the new CPU 0, init_ISA_irqs() initially sets up the legacy IRQs to
use the PIC. Vector 52 maps to IRQ4, which is normally used by the serial
console.

5. Before these legacy IRQs are handed over to the IOAPIC, suppose the
delayed vector 52 arrives through the LAPIC and sets its ISR bit.

6. The new kernel handles it as PIC IRQ4. Since mask_and_ack_8259A()
does not send a LAPIC EOI, the ISR bit for vector 52 remains set.

The below is an example when vector is 55.

(qemu) info lapic 2
dumping local APIC state for CPU 2

LVT0	 0x00010700 active-hi edge  masked                      ExtINT (vec 0)
LVT1	 0x00000400 active-hi edge                              NMI
LVTPC	 0x00000400 active-hi edge                              NMI
LVTERR	 0x000000fe active-hi edge                              Fixed  (vec 254)
LVTTHMR	 0x00010000 active-hi edge  masked                      Fixed  (vec 0)
LVTT	 0x000400ec active-hi edge                 tsc-deadline Fixed  (vec 236)
Timer	 DCR=0x0 (divide by 2) initial_count = 0 current_count = 0
SPIV	 0x000001ff APIC enabled, focus=off, spurious vec 255
ICR	 0x000c0400 physical edge de-assert all
ICR2	 0x00000000
ESR	 0x00000000
ISR	 55
IRR	 37 38 39 40 48 56

APR 0x00 TPR 0x10 DFR 0x0f LDR 0x00 PPR 0x30

This is painful when it happens in production. The kdump kernel stalls, so
we cannot collect a vmcore for analysis.

The issue is easier to reproduce with a customized KVM host kernel that
allows arbitrary vectors to be injected through debugfs and
kvm_apic_set_irq(). I can stall the guest kernel by injecting vector 50
into vCPU 0, even after the guest has booted successfully. Vector 50 maps
to IRQ2, the cascade interrupt between the two 8259A chips. IRQ2 is not
taken over by the IOAPIC.

For other vectors, I can reproduce the issue by adding mdelay() at the
beginning of setup_IO_APIC() and injecting the vectors during the delay
through the modified KVM host kernel.

In theory, a normal kexec reboot may also hit this issue if device
interrupts arrive late.

Fix this by checking the LAPIC ISR bit corresponding to the PIC vector and
sending a LAPIC EOI only if it is set. Cover both normal and auto-EOI
modes.

Assisted-by: OpenAI Codex
Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
 arch/x86/kernel/i8259.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
index f7a86b94a0dd..73939edb8c2b 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -144,6 +144,16 @@ static inline int i8259A_irq_real(unsigned int irq)
 	return value;
 }
 
+static void apic_eoi_8259A(unsigned int irq)
+{
+	unsigned int vector = ISA_IRQ_VECTOR(irq);
+	u32 isr;
+
+	isr = apic_read(APIC_ISR + APIC_VECTOR_TO_REG_OFFSET(vector));
+	if (isr & BIT(APIC_VECTOR_TO_BIT_NUMBER(vector)))
+		apic_eoi();
+}
+
 /*
  * Careful! The 8259A is a fragile beast, it pretty
  * much _has_ to be done exactly like this (mask it
@@ -190,6 +200,9 @@ static void mask_and_ack_8259A(struct irq_data *data)
 		outb(0x60+irq, PIC_MASTER_CMD);	/* 'Specific EOI to master */
 	}
 	raw_spin_unlock_irqrestore(&i8259A_lock, flags);
+
+	apic_eoi_8259A(irq);
+
 	return;
 
 spurious_8259A_irq:
@@ -224,6 +237,12 @@ static void mask_and_ack_8259A(struct irq_data *data)
 	}
 }
 
+static void mask_and_ack_8259A_auto(struct irq_data *data)
+{
+	disable_8259A_irq(data);
+	apic_eoi_8259A(data->irq);
+}
+
 struct irq_chip i8259A_chip = {
 	.name		= "XT-PIC",
 	.irq_mask	= disable_8259A_irq,
@@ -386,7 +405,7 @@ static void init_8259A(int auto_eoi)
 		 * In AEOI mode we just have to mask the interrupt
 		 * when acking.
 		 */
-		i8259A_chip.irq_mask_ack = disable_8259A_irq;
+		i8259A_chip.irq_mask_ack = mask_and_ack_8259A_auto;
 	else
 		i8259A_chip.irq_mask_ack = mask_and_ack_8259A;
 
-- 
2.43.7