Skip to content

Commit 0edc78b

Browse files
committed
x86/msi: Make irq_retrigger() functional for posted MSI
Luigi reported that retriggering a posted MSI interrupt does not work correctly. The reason is that the retrigger happens at the vector domain by sending an IPI to the actual vector on the target CPU. That works correctly exactly once because the posted MSI interrupt chip does not issue an EOI as that's only required for the posted MSI notification vector itself. As a consequence the vector becomes stale in the ISR, which not only affects this vector but also any lower priority vector in the affected APIC because the ISR bit is not cleared. Luigi proposed to set the vector in the remap PIR bitmap and raise the posted MSI notification vector. That works, but that still does not cure a related problem: If there is ever a stray interrupt on such a vector, then the related APIC ISR bit becomes stale due to the lack of EOI as described above. Unlikely to happen, but if it happens it's not debuggable at all. So instead of playing games with the PIR, this can be actually solved for both cases by: 1) Keeping track of the posted interrupt vector handler state 2) Implementing a posted MSI specific irq_ack() callback which checks that state. If the posted vector handler is inactive it issues an EOI, otherwise it delegates that to the posted handler. This is correct versus affinity changes and concurrent events on the posted vector as the actual handler invocation is serialized through the interrupt descriptor lock. Fixes: ed1e48e ("iommu/vt-d: Enable posted mode for device MSIs") Reported-by: Luigi Rizzo <lrizzo@google.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Tested-by: Luigi Rizzo <lrizzo@google.com> Cc: stable@vger.kernel.org Link: https://patch.msgid.link/20251125214631.044440658@linutronix.de Closes: https://lore.kernel.org/lkml/20251124104836.3685533-1-lrizzo@google.com
1 parent 21433d3 commit 0edc78b

File tree

3 files changed

+34
-4
lines changed

3 files changed

+34
-4
lines changed

arch/x86/include/asm/irq_remapping.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,4 +87,11 @@ static inline void panic_if_irq_remap(const char *msg)
8787
}
8888

8989
#endif /* CONFIG_IRQ_REMAP */
90+
91+
#ifdef CONFIG_X86_POSTED_MSI
92+
void intel_ack_posted_msi_irq(struct irq_data *irqd);
93+
#else
94+
#define intel_ack_posted_msi_irq NULL
95+
#endif
96+
9097
#endif /* __X86_IRQ_REMAPPING_H */

arch/x86/kernel/irq.c

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,7 @@ DEFINE_IDTENTRY_SYSVEC_SIMPLE(sysvec_kvm_posted_intr_nested_ipi)
397397

398398
/* Posted Interrupt Descriptors for coalesced MSIs to be posted */
399399
DEFINE_PER_CPU_ALIGNED(struct pi_desc, posted_msi_pi_desc);
400+
static DEFINE_PER_CPU_CACHE_HOT(bool, posted_msi_handler_active);
400401

401402
void intel_posted_msi_init(void)
402403
{
@@ -414,6 +415,25 @@ void intel_posted_msi_init(void)
414415
this_cpu_write(posted_msi_pi_desc.ndst, destination);
415416
}
416417

418+
void intel_ack_posted_msi_irq(struct irq_data *irqd)
419+
{
420+
irq_move_irq(irqd);
421+
422+
/*
423+
* Handle the rare case that irq_retrigger() raised the actual
424+
* assigned vector on the target CPU, which means that it was not
425+
* invoked via the posted MSI handler below. In that case APIC EOI
426+
* is required as otherwise the ISR entry becomes stale and lower
427+
* priority interrupts are never going to be delivered after that.
428+
*
429+
* If the posted handler invoked the device interrupt handler then
430+
* the EOI would be premature because it would acknowledge the
431+
* posted vector.
432+
*/
433+
if (unlikely(!__this_cpu_read(posted_msi_handler_active)))
434+
apic_eoi();
435+
}
436+
417437
static __always_inline bool handle_pending_pir(unsigned long *pir, struct pt_regs *regs)
418438
{
419439
unsigned long pir_copy[NR_PIR_WORDS];
@@ -446,6 +466,8 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_posted_msi_notification)
446466

447467
pid = this_cpu_ptr(&posted_msi_pi_desc);
448468

469+
/* Mark the handler active for intel_ack_posted_msi_irq() */
470+
__this_cpu_write(posted_msi_handler_active, true);
449471
inc_irq_stat(posted_msi_notification_count);
450472
irq_enter();
451473

@@ -474,6 +496,7 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_posted_msi_notification)
474496

475497
apic_eoi();
476498
irq_exit();
499+
__this_cpu_write(posted_msi_handler_active, false);
477500
set_irq_regs(old_regs);
478501
}
479502
#endif /* X86_POSTED_MSI */

drivers/iommu/intel/irq_remapping.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1303,17 +1303,17 @@ static struct irq_chip intel_ir_chip = {
13031303
* irq_enter();
13041304
* handle_edge_irq()
13051305
* irq_chip_ack_parent()
1306-
* irq_move_irq(); // No EOI
1306+
* intel_ack_posted_msi_irq(); // No EOI
13071307
* handle_irq_event()
13081308
* driver_handler()
13091309
* handle_edge_irq()
13101310
* irq_chip_ack_parent()
1311-
* irq_move_irq(); // No EOI
1311+
* intel_ack_posted_msi_irq(); // No EOI
13121312
* handle_irq_event()
13131313
* driver_handler()
13141314
* handle_edge_irq()
13151315
* irq_chip_ack_parent()
1316-
* irq_move_irq(); // No EOI
1316+
* intel_ack_posted_msi_irq(); // No EOI
13171317
* handle_irq_event()
13181318
* driver_handler()
13191319
* apic_eoi()
@@ -1322,7 +1322,7 @@ static struct irq_chip intel_ir_chip = {
13221322
*/
13231323
static struct irq_chip intel_ir_chip_post_msi = {
13241324
.name = "INTEL-IR-POST",
1325-
.irq_ack = irq_move_irq,
1325+
.irq_ack = intel_ack_posted_msi_irq,
13261326
.irq_set_affinity = intel_ir_set_affinity,
13271327
.irq_compose_msi_msg = intel_ir_compose_msi_msg,
13281328
.irq_set_vcpu_affinity = intel_ir_set_vcpu_affinity,

0 commit comments

Comments
 (0)