KVM: ioapic RTC EOI tracking wedges on non-FIXED delivery modes

KVM: ioapic RTC EOI tracking wedges on non-FIXED delivery modes

[David Ballesteros]

Hi Sean, Paolo,

It was your tripwire in cc3d0e1afd10 ("KVM: x86: WARN (once) if RTC pending
EOI tracking goes off the rails") that led me here. While auditing the
ioapic RTC EOI tracking logic with its arithmetic invariants in mind, I
found that the check cannot fire for the failure mode it was hunting for.
It turns out there's a third desync state, silent and permanent, where
rtc_status.pending_eoi > 0 while rtc_status.map is empty.

With in-kernel irqchip, a guest-kernel source that performs two MMIO writes
to the guest IOAPIC (0xFEC00000; reachable only from guest kernel mode) to
write a non-FIXED delivery mode into the RTE of the RTC GSI permanently
wedges the tracking. Every later RTC IRQ of that VM is then silently
dropped, and the VM loses its timer. To be fair though, no host boundary is
crossed since a guest can only freeze its own VM, so I'm reporting this as
a correctness bug on the normal channel.

The root cause lies in an asymmetry: ioapic_service() counts any accepted
delivery into pending_eoi (arch/x86/kvm/ioapic.c:491), but
__apic_accept_irq() only sets map bits for FIXED/LOWEST modes
(lapic.c:1433-1438). As a result, modes like NMI, INIT, STARTUP, SMI, and
EXTINT return result > 0 without touching rtc_status at all. By design none
of these receive an EOI, so no ISR bit is ever established for the paired
decrement.

When RTE 8 (RTC) has delivery_mode != FIXED/LOWEST, the next tick causes
ioapic_service() to count N accepted deliveries into pending_eoi while the
map stays empty. Both sides of the invariant hold from each side's own
bookkeeping; the system is simply counting two different things. Then every
later tick gets discarded in ioapic_set_irq() before ioapic_service() even
runs (ioapic.c:225-229: gate on rtc_irq_check_coalesced()), so pending_eoi
never gets re-assigned. The WARN_ON_ONCE sits inside the RTC branch, one
call below that gate, which means it can only fire if ioapic_service()
actually reaches pin 8 with a stale count. In the stuck state, the gate
closes the signal before the WARN can evaluate it.

I verified this statically on mainline 0fcb05b72344, v6.1.74, and a
hardened 7.2-rc5 build (the git history confirms no commit since cc3d0e1
touches this logic). Dynamically, the same reproducer showed the identical
stuck state (pending_eoi = 1, map = 0x0 observed live with kprobes) on both
hardened 7.2-rc5 and upstream v6.1.74. On the hardened build carrying
cc3d0e1, the warning never fired across 6 runs with clean dmesg. To keep
the claims straight on the other two trees: that observation is
dynamic-only on the hardened build, since that is the only tree carrying
cc3d0e1afd10 I could run on; on mainline it follows statically from the
gate analysis above (the WARN is unreachable in the stuck state), and
v6.1.74 predates the commit, so there is no WARN there. Rewriting the RTE
back to fixed-mode does not recover the timer. A same-harness run with a
fixed-mode RTE delivers 100/100 ticks normally. Causality closed when a
build with the fix delta made the same reproducer behave correctly (ticks
flow, no coalescing).

I'm keeping the reproducer off-list; the tarball is packaged and ready, so
I can send it in a single reply. All run hashes and a pre-registered
prediction record are available on request, and I can also re-run on an
upstream mainline build on request.

The fix I believe is right is to count into rtc_status only deliveries
that participate in EOI pairing. Concretely, in ioapic_service(), take the
RTC branch (and its pending_eoi = ret assignment) only when the RTE's
delivery_mode is FIXED or LOWEST. This preserves the current gate
semantics without forcing __apic_accept_irq() to populate the map for modes
that architecturally won't receive an EOI the kernel can associate back. I
validated it on the same rig: 3/3 runs post-fix show every tick delivered,
coalesced = 0, and the observed rtc_status stays consistent (map populated
whenever pending_eoi is armed). Normal fixed/lowest path remains
unaffected (100/100 delivered, EOIs paired).

Separately, the tripwire itself could be extended to catch this class
directly (WARN if pending_eoi != hweight(map)), though I left it as-is in
the patch.

The minimal patch with explanatory comments is inlined below. If this
direction works, I'll send it through the normal patch flow. It carries
Fixes: 2c2bf0113697 ("KVM: Use eoi to track RTC interrupt delivery status",
Yang Zhang, 2013-04-11). The coalescing gate, the pending_eoi = ret
assignment, and the paired decrement all trace back to that commit, so the
fixed/lowest-only asymmetry is as old as the tracking itself. I haven't
found any historical change that validated delivery_mode on this pin.

Thanks,
David

-- ----[ fix patch ]---- --

KVM: ioapic: only track RTC IRQs that can receive an EOI

Only count RTC IRQs into the RTC EOI tracking when they can actually
receive an EOI, i.e. those with the FIXED and LOWEST delivery modes;
treat every other delivery mode on the RTC GSI as a plain IRQ with no
RTC book-keeping.

The tracking counts every accepted IRQ into rtc_status.pending_eoi,
but __apic_accept_irq() populates rtc_status.map only for FIXED and
LOWEST delivery.  Any other mode (NMI, SMI, INIT, STARTUP, ExtINT)
accepts the IRQ without establishing an ISR bit, so no later EOI can
pair with the count armed at that moment.  A guest can trigger this
by writing a non-fixed delivery mode into the RTE of the RTC GSI
through MMIO; the tracking then wedges (pending_eoi > 0 with an empty
map) and every later RTC IRQ of the VM is coalesced, until the ioapic
is reset or the irqchip is reloaded.

Fixes: 2c2bf0113697 ("KVM: Use eoi to track RTC interrupt delivery status")
Assisted-by: Claude-Code:GLM-5.3-flash KASAN kprobes
Signed-off-by: David Ballesteros <linux@davidbq.me>
---
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -477,7 +477,23 @@
 	if (irqe.trig_mode == IOAPIC_EDGE_TRIG)
 		ioapic->irr_delivered |= 1 << irq;
 
-	if (irq == RTC_GSI && line_status) {
+	if (irq == RTC_GSI && line_status &&
+	    (irqe.delivery_mode == APIC_DM_FIXED ||
+	     irqe.delivery_mode == APIC_DM_LOWEST)) {
+		/*
+		 * Track only RTC IRQs that can actually receive an EOI.
+		 * The EOI tracking in __apic_accept_irq() sets a map bit
+		 * and a vector for FIXED and LOWEST delivery only; every
+		 * other delivery mode (NMI, SMI, INIT, STARTUP, ExtINT)
+		 * accepts the IRQ without establishing an ISR bit for the
+		 * vector, so no EOI can ever pair with the count armed
+		 * here.  Counting those accepted IRQs into pending_eoi
+		 * wedges the RTC status: pending_eoi stays > 0 with an
+		 * empty map, rtc_irq_check_coalesced() suppresses every
+		 * subsequent RTC IRQ, and rewriting the RTE back to fixed
+		 * delivery does not recover (restore_all runs only at
+		 * inject-all/reset/restore time, not on RTE rewrite).
+		 */
 		/*
 		 * pending_eoi cannot ever become negative (see
 		 * rtc_status_pending_eoi_check_valid) and the caller

-- ----[ end of fix patch ]---- --