| Message ID | 20260820134718.2176411-1-mkchauras@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V2] powerpc/entry: Fix irq_soft_mask corruption on replayed interrupt exit | expand |
On 8/20/26 7:17 PM, Mukesh Kumar Chaurasiya (IBM) wrote: > When __replay_soft_interrupts() replays a pending interrupt (e.g. > PACA_IRQ_DEC → timer_interrupt), it calls the handler directly with a > synthetic pt_regs. The DEFINE_INTERRUPT_HANDLER_ASYNC wrapper around > each handler calls arch_interrupt_async_exit_prepare() on the way out, > which calls arch_interrupt_exit_prepare() → local_irq_disable() → > arch_local_irq_disable(), which does: > > irq_soft_mask_set(IRQS_DISABLED) /* 0x1 */ > > This unconditionally overwrites irq_soft_mask with IRQS_DISABLED (0x1), > stripping the IRQS_PMI_DISABLED (0x2) bit. The result is that > irq_soft_mask is 0x1 instead of IRQS_ALL_DISABLED (0x3) when the > handler returns to __replay_soft_interrupts(). > > For a normally-taken interrupt this is harmless: the next interrupt > always enters through arch_interrupt_enter_prepare() which > unconditionally sets irq_soft_mask to IRQS_ALL_DISABLED. > But during replay, next_interrupt() is called > directly between replayed handlers without going back through > arch_interrupt_enter_prepare(), so the stripped bit is never restored. > next_interrupt() then fires a WARNING: > > WARNING: arch/powerpc/kernel/irq_64.c:75 > WARN_ON(irq_soft_mask_return() != IRQS_ALL_DISABLED) > > This was introduced by commit bee25f97ad24 ("powerpc: Enable > GENERIC_ENTRY feature"). Before that commit, the old > interrupt_async_exit_prepare() called irq_exit() followed by an empty > interrupt_exit_prepare() stub and never touched irq_soft_mask at all, > so the soft mask was left at IRQS_ALL_DISABLED throughout replay. > > The root cause: arch_interrupt_exit_prepare() uses local_irq_disable() > whose only job is to set the IRQS_DISABLED bit; it has no knowledge of > IRQS_PMI_DISABLED. It is there to satisfy irqentry_exit()'s > requirement that interrupts be disabled, but using the plain > irq_soft_mask_set(IRQS_ALL_DISABLED) is the right primitive: > > - irq_soft_mask_set(IRQS_ALL_DISABLED): sets soft mask to 0x3 > (both IRQS_DISABLED and IRQS_PMI_DISABLED). Touches only the soft > mask. MSR[EE] and PACA_IRQ_HARD_DIS are already correct because > hard interrupts were never re-enabled during replay > (PACA_IRQ_REPLAYING is in PACA_IRQ_MUST_HARD_MASK, which blocks > should_hard_irq_enable()). > > - local_irq_disable() / arch_local_irq_disable(): sets soft mask to > IRQS_DISABLED (0x1) only, silently dropping IRQS_PMI_DISABLED. > > - hard_irq_disable(): also issues __mtmsrd to clear MSR[EE] in > hardware and sets PACA_IRQ_HARD_DIS — redundant and wrong here > since both are already set. > > Fix by replacing local_irq_disable() with irq_soft_mask_set(IRQS_ALL_DISABLED) > in arch_interrupt_exit_prepare(), making the exit symmetric with the > entry path in arch_interrupt_enter_prepare() which always sets > IRQS_ALL_DISABLED. > > The warning was observed early in boot on a POWER10 pseries guest > during kmem_cache_init_late(), where a spinlock release triggers > interrupt replay that processes a pending timer interrupt. > > Debugger state confirming the bug: > Before timer_interrupt(®s): > irq_soft_mask = 0x3 (IRQS_ALL_DISABLED) correct > irq_happened = 0x41 (HARD_DIS|REPLAYING) correct > After timer_interrupt(®s) returns: > irq_soft_mask = 0x1 (IRQS_DISABLED) WRONG — PMI bit stripped > irq_happened = 0x41 unchanged > Indeed. Thanks for the fix and good explanation. Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com> > Fixes: 334f3f6d7a16 ("powerpc/entry: Disable interrupts before irqentry_exit") > Reported-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com> > Closes: https://lore.kernel.org/all/6f9bfb0f-b14c-468e-bb9f-c157d120d0dc@linux.ibm.com/ > Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com> > --- > Change log: > V1 -> V2: > - Instead of using hard_irq_disable use irq_soft_mask_set > V1: https://lore.kernel.org/all/20260812152035.1661781-1-mkchauras@gmail.com > > arch/powerpc/include/asm/entry-common.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h > index c5adb5006361..2a153dca962c 100644 > --- a/arch/powerpc/include/asm/entry-common.h > +++ b/arch/powerpc/include/asm/entry-common.h > @@ -270,7 +270,7 @@ static inline void arch_interrupt_exit_prepare(struct pt_regs *regs) > } > > /* irqentry_exit expects to be called with interrupts disabled */ > - local_irq_disable(); > + irq_soft_mask_set(IRQS_ALL_DISABLED); > } > > static inline void arch_interrupt_async_enter_prepare(struct pt_regs *regs)
On 20/08/26 8:17 pm, Shrikanth Hegde wrote: > > > On 8/20/26 7:17 PM, Mukesh Kumar Chaurasiya (IBM) wrote: >> When __replay_soft_interrupts() replays a pending interrupt (e.g. >> PACA_IRQ_DEC → timer_interrupt), it calls the handler directly with a >> synthetic pt_regs. The DEFINE_INTERRUPT_HANDLER_ASYNC wrapper around >> each handler calls arch_interrupt_async_exit_prepare() on the way out, >> which calls arch_interrupt_exit_prepare() → local_irq_disable() → >> arch_local_irq_disable(), which does: >> >> irq_soft_mask_set(IRQS_DISABLED) /* 0x1 */ >> >> This unconditionally overwrites irq_soft_mask with IRQS_DISABLED (0x1), >> stripping the IRQS_PMI_DISABLED (0x2) bit. The result is that >> irq_soft_mask is 0x1 instead of IRQS_ALL_DISABLED (0x3) when the >> handler returns to __replay_soft_interrupts(). >> >> For a normally-taken interrupt this is harmless: the next interrupt >> always enters through arch_interrupt_enter_prepare() which >> unconditionally sets irq_soft_mask to IRQS_ALL_DISABLED. >> But during replay, next_interrupt() is called >> directly between replayed handlers without going back through >> arch_interrupt_enter_prepare(), so the stripped bit is never restored. >> next_interrupt() then fires a WARNING: >> >> WARNING: arch/powerpc/kernel/irq_64.c:75 >> WARN_ON(irq_soft_mask_return() != IRQS_ALL_DISABLED) >> >> This was introduced by commit bee25f97ad24 ("powerpc: Enable >> GENERIC_ENTRY feature"). Before that commit, the old >> interrupt_async_exit_prepare() called irq_exit() followed by an empty >> interrupt_exit_prepare() stub and never touched irq_soft_mask at all, >> so the soft mask was left at IRQS_ALL_DISABLED throughout replay. >> >> The root cause: arch_interrupt_exit_prepare() uses local_irq_disable() >> whose only job is to set the IRQS_DISABLED bit; it has no knowledge of >> IRQS_PMI_DISABLED. It is there to satisfy irqentry_exit()'s >> requirement that interrupts be disabled, but using the plain >> irq_soft_mask_set(IRQS_ALL_DISABLED) is the right primitive: >> >> - irq_soft_mask_set(IRQS_ALL_DISABLED): sets soft mask to 0x3 >> (both IRQS_DISABLED and IRQS_PMI_DISABLED). Touches only the soft >> mask. MSR[EE] and PACA_IRQ_HARD_DIS are already correct because >> hard interrupts were never re-enabled during replay >> (PACA_IRQ_REPLAYING is in PACA_IRQ_MUST_HARD_MASK, which blocks >> should_hard_irq_enable()). >> >> - local_irq_disable() / arch_local_irq_disable(): sets soft mask to >> IRQS_DISABLED (0x1) only, silently dropping IRQS_PMI_DISABLED. >> >> - hard_irq_disable(): also issues __mtmsrd to clear MSR[EE] in >> hardware and sets PACA_IRQ_HARD_DIS — redundant and wrong here >> since both are already set. >> >> Fix by replacing local_irq_disable() with >> irq_soft_mask_set(IRQS_ALL_DISABLED) >> in arch_interrupt_exit_prepare(), making the exit symmetric with the >> entry path in arch_interrupt_enter_prepare() which always sets >> IRQS_ALL_DISABLED. >> >> The warning was observed early in boot on a POWER10 pseries guest >> during kmem_cache_init_late(), where a spinlock release triggers >> interrupt replay that processes a pending timer interrupt. >> >> Debugger state confirming the bug: >> Before timer_interrupt(®s): >> irq_soft_mask = 0x3 (IRQS_ALL_DISABLED) correct >> irq_happened = 0x41 (HARD_DIS|REPLAYING) correct >> After timer_interrupt(®s) returns: >> irq_soft_mask = 0x1 (IRQS_DISABLED) WRONG — PMI bit stripped >> irq_happened = 0x41 unchanged >> > > Indeed. Thanks for the fix and good explanation. > > Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com> >> Fixes: 334f3f6d7a16 ("powerpc/entry: Disable interrupts before >> irqentry_exit") >> Reported-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com> >> Closes: >> https://lore.kernel.org/all/6f9bfb0f-b14c-468e-bb9f-c157d120d0dc@linux.ibm.com/ >> Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com> >> --- This patch fixes the reported issue. Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com> Regards, Venkat. >> Change log: >> V1 -> V2: >> - Instead of using hard_irq_disable use irq_soft_mask_set >> V1: >> https://lore.kernel.org/all/20260812152035.1661781-1-mkchauras@gmail.com >> >> arch/powerpc/include/asm/entry-common.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/include/asm/entry-common.h >> b/arch/powerpc/include/asm/entry-common.h >> index c5adb5006361..2a153dca962c 100644 >> --- a/arch/powerpc/include/asm/entry-common.h >> +++ b/arch/powerpc/include/asm/entry-common.h >> @@ -270,7 +270,7 @@ static inline void >> arch_interrupt_exit_prepare(struct pt_regs *regs) >> } >> /* irqentry_exit expects to be called with interrupts >> disabled */ >> - local_irq_disable(); >> + irq_soft_mask_set(IRQS_ALL_DISABLED); >> } >> static inline void arch_interrupt_async_enter_prepare(struct >> pt_regs *regs) > >
diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h index c5adb5006361..2a153dca962c 100644 --- a/arch/powerpc/include/asm/entry-common.h +++ b/arch/powerpc/include/asm/entry-common.h @@ -270,7 +270,7 @@ static inline void arch_interrupt_exit_prepare(struct pt_regs *regs) } /* irqentry_exit expects to be called with interrupts disabled */ - local_irq_disable(); + irq_soft_mask_set(IRQS_ALL_DISABLED); } static inline void arch_interrupt_async_enter_prepare(struct pt_regs *regs)
When __replay_soft_interrupts() replays a pending interrupt (e.g. PACA_IRQ_DEC → timer_interrupt), it calls the handler directly with a synthetic pt_regs. The DEFINE_INTERRUPT_HANDLER_ASYNC wrapper around each handler calls arch_interrupt_async_exit_prepare() on the way out, which calls arch_interrupt_exit_prepare() → local_irq_disable() → arch_local_irq_disable(), which does: irq_soft_mask_set(IRQS_DISABLED) /* 0x1 */ This unconditionally overwrites irq_soft_mask with IRQS_DISABLED (0x1), stripping the IRQS_PMI_DISABLED (0x2) bit. The result is that irq_soft_mask is 0x1 instead of IRQS_ALL_DISABLED (0x3) when the handler returns to __replay_soft_interrupts(). For a normally-taken interrupt this is harmless: the next interrupt always enters through arch_interrupt_enter_prepare() which unconditionally sets irq_soft_mask to IRQS_ALL_DISABLED. But during replay, next_interrupt() is called directly between replayed handlers without going back through arch_interrupt_enter_prepare(), so the stripped bit is never restored. next_interrupt() then fires a WARNING: WARNING: arch/powerpc/kernel/irq_64.c:75 WARN_ON(irq_soft_mask_return() != IRQS_ALL_DISABLED) This was introduced by commit bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature"). Before that commit, the old interrupt_async_exit_prepare() called irq_exit() followed by an empty interrupt_exit_prepare() stub and never touched irq_soft_mask at all, so the soft mask was left at IRQS_ALL_DISABLED throughout replay. The root cause: arch_interrupt_exit_prepare() uses local_irq_disable() whose only job is to set the IRQS_DISABLED bit; it has no knowledge of IRQS_PMI_DISABLED. It is there to satisfy irqentry_exit()'s requirement that interrupts be disabled, but using the plain irq_soft_mask_set(IRQS_ALL_DISABLED) is the right primitive: - irq_soft_mask_set(IRQS_ALL_DISABLED): sets soft mask to 0x3 (both IRQS_DISABLED and IRQS_PMI_DISABLED). Touches only the soft mask. MSR[EE] and PACA_IRQ_HARD_DIS are already correct because hard interrupts were never re-enabled during replay (PACA_IRQ_REPLAYING is in PACA_IRQ_MUST_HARD_MASK, which blocks should_hard_irq_enable()). - local_irq_disable() / arch_local_irq_disable(): sets soft mask to IRQS_DISABLED (0x1) only, silently dropping IRQS_PMI_DISABLED. - hard_irq_disable(): also issues __mtmsrd to clear MSR[EE] in hardware and sets PACA_IRQ_HARD_DIS — redundant and wrong here since both are already set. Fix by replacing local_irq_disable() with irq_soft_mask_set(IRQS_ALL_DISABLED) in arch_interrupt_exit_prepare(), making the exit symmetric with the entry path in arch_interrupt_enter_prepare() which always sets IRQS_ALL_DISABLED. The warning was observed early in boot on a POWER10 pseries guest during kmem_cache_init_late(), where a spinlock release triggers interrupt replay that processes a pending timer interrupt. Debugger state confirming the bug: Before timer_interrupt(®s): irq_soft_mask = 0x3 (IRQS_ALL_DISABLED) correct irq_happened = 0x41 (HARD_DIS|REPLAYING) correct After timer_interrupt(®s) returns: irq_soft_mask = 0x1 (IRQS_DISABLED) WRONG — PMI bit stripped irq_happened = 0x41 unchanged Fixes: 334f3f6d7a16 ("powerpc/entry: Disable interrupts before irqentry_exit") Reported-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com> Closes: https://lore.kernel.org/all/6f9bfb0f-b14c-468e-bb9f-c157d120d0dc@linux.ibm.com/ Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com> --- Change log: V1 -> V2: - Instead of using hard_irq_disable use irq_soft_mask_set V1: https://lore.kernel.org/all/20260812152035.1661781-1-mkchauras@gmail.com arch/powerpc/include/asm/entry-common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)