powerpc/entry: Fix irq_soft_mask corruption on replayed interrupt exit
PATCH v3 1/1 powerpc/entry: Fix irq_soft_mask corruption on replayed interrupt exit
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)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 unchangedThe fix is to replace local_irq_disable() with hard_irq_disable().
hard_irq_disable() is the right primitive here for two reasons:
1. On PPC64 (hw_irq.h:301) it calls irq_soft_mask_set_return(IRQS_ALL_DISABLED), setting the soft mask to 0x3 (both IRQS_DISABLED and IRQS_PMI_DISABLED), which preserves the PMI bit and fixes the WARNING. The additional work it does (__hard_irq_disable(), PACA_IRQ_HARD_DIS |=) is redundant but safe since both are already set at this point in the exit path; the trace_hardirqs_off() inside is guarded by
if (!arch_irqs_disabled_flags(flags)) so it will not double-fire.
2. On PPC32 (hw_irq.h:467) hard_irq_disable() maps to arch_local_irq_disable() -> __hard_irq_disable(), which clears MSR[EE] in hardware. This is exactly correct: PPC32 has no soft-mask PACA mechanism, so the hardware disable is the right way to satisfy irqentry_exit()'s requirement. This also fixes a build error on PPC32 where irq_soft_mask_set() is only defined under CONFIG_PPC64:
arch/powerpc/include/asm/entry-common.h:273: error: implicit declaration of function 'irq_soft_mask_set'
Using hard_irq_disable() requires no #ifdef and is consistent with how the rest of the entry code (e.g. entry-common.h:463) handles the same PPC32/PPC64 split.
---
Change log:
V2 -> V3:
- Revert to hard_irq_disable
- Removed some dead code
V2: https://lore.kernel.org/all/[email protected]
V1 -> V2:
- Instead of using hard_irq_disable use irq_soft_mask_set
V1: https://lore.kernel.org/all/[email protected]
arch/powerpc/include/asm/entry-common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/powerpc/include/asm/entry-common.h +1 -1
@@ -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 (); +hard_irq_disable (); }static inline void arch_interrupt_async_enter_prepare (struct pt_regs *regs)
Signature · 1 line
2.55.0
Quoted 68 lines · Mukesh Kumar Chaurasiya (IBM)
When
__replay_soft_interrupts()replays a pending interrupt (e.g.PACA_IRQ_DEC->timer_interrupt), it calls the handler directly with a syntheticpt_regs. TheDEFINE_INTERRUPT_HANDLER_ASYNCwrapper around each handler callsarch_interrupt_async_exit_prepare()on the way out, which callsarch_interrupt_exit_prepare()->local_irq_disable()->arch_local_irq_disable(), which does:irq_soft_mask_set(IRQS_DISABLED) /* 0x1 */This unconditionally overwrites
irq_soft_maskwithIRQS_DISABLED(0x1), stripping theIRQS_PMI_DISABLED(0x2) bit. The result is thatirq_soft_maskis0x1instead ofIRQS_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 setsirq_soft_masktoIRQS_ALL_DISABLED. But during replay,next_interrupt()is called directly between replayed handlers without going back througharch_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)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:
Beforetimer_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 unchangedThe fix is to replace
local_irq_disable()withhard_irq_disable().
hard_irq_disable()is the right primitive here for two reasons:1. On PPC64 (hw_irq.h:301) it calls
irq_soft_mask_set_return(IRQS_ALL_DISABLED), setting the soft mask to0x3(bothIRQS_DISABLEDandIRQS_PMI_DISABLED), which preserves the PMI bit and fixes the WARNING. The additional work it does (__hard_irq_disable(),PACA_IRQ_HARD_DIS|=) is redundant but safe since both are already set at this point in the exit path; thetrace_hardirqs_off()inside is guarded by
if (!arch_irqs_disabled_flags(flags)) so it will not double-fire.2. On PPC32 (hw_irq.h:467)
hard_irq_disable()maps toarch_local_irq_disable()->__hard_irq_disable(), which clears MSR[EE] in hardware. This is exactly correct: PPC32 has no soft-mask PACA mechanism, so the hardware disable is the right way to satisfy irqentry_exit()'s requirement. This also fixes a build error on PPC32 whereirq_soft_mask_set()is only defined underCONFIG_PPC64:
arch/powerpc/include/asm/entry-common.h:273: error: implicit declaration of function 'irq_soft_mask_set'Using
hard_irq_disable()requires no #ifdef and is consistent with how the rest of the entry code (e.g. entry-common.h:463) handles the same PPC32/PPC64 split.Fixes 334f3f6d7a16 ("powerpc/entry: Disable interrupts before irqentry_exit")Reported-by Venkat Rao Bagalkote <[email protected]>Tested-by Venkat Rao Bagalkote <[email protected]>Reviewed-by Shrikanth Hegde <[email protected]>Signed-off-by Mukesh Kumar Chaurasiya (IBM) <[email protected]>
Quoted 5 lines
---
Change log:
V2 -> V3:
- Revert tohard_irq_disable
- Removed some dead code
Ignore this line.
Regards,
Mukesh
Quoted 23 lines
V2: https://lore.kernel.org/all/[email protected]
V1 -> V2:
- Instead of usinghard_irq_disableuseirq_soft_mask_set
V1: https://lore.kernel.org/all/[email protected]arch/powerpc/include/asm/entry-common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/powerpc/include/asm/entry-common.h +1
-1@@ -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 (); +hard_irq_disable (); }static inline void arch_interrupt_async_enter_prepare (struct pt_regs *regs)Signature · 1 line
2.55.0
Quoted 8 lines · Mukesh Kumar Chaurasiya (IBM)
When
__replay_soft_interrupts()replays a pending interrupt (e.g.PACA_IRQ_DEC->timer_interrupt), it calls the handler directly with a syntheticpt_regs. TheDEFINE_INTERRUPT_HANDLER_ASYNCwrapper around each handler callsarch_interrupt_async_exit_prepare()on the way out, which callsarch_interrupt_exit_prepare()->local_irq_disable()->arch_local_irq_disable(), which does:[...]
Applied to powerpc/fixes.
[1/1] powerpc/entry: Fix irq_soft_mask corruption on replayed interrupt exit
https://git.kernel.org/powerpc/c/63a7531ca31f9f097d9cc1cc3fe86ae683cdabdd
cheers