From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-10-04 14:58:12
This fixes a number of bugs found mostly looking at a MCE handler issue,
which should be fixed in patch 5 of the series, previous attempt here
which Ganesh found to be wrong.
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210922020247.209409-1-npiggin@gmail.com/
I didn't increment to patch v2 because it's a different approach (so I
gave it a different title).
Thanks,
Nick
Nicholas Piggin (5):
powerpc/64s: fix program check interrupt emergency stack path
powerpc/traps: do not enable irqs in _exception
powerpc/64: warn if local irqs are enabled in NMI or hardirq context
powerpc/64/interrupt: Reconcile soft-mask state in NMI and fix false
BUG
powerpc/64s: Fix unrecoverable MCE calling async handler from NMI
arch/powerpc/include/asm/interrupt.h | 18 ++++++------
arch/powerpc/kernel/exceptions-64s.S | 25 ++++++++++------
arch/powerpc/kernel/irq.c | 6 ++++
arch/powerpc/kernel/traps.c | 43 +++++++++++++++++-----------
4 files changed, 59 insertions(+), 33 deletions(-)
--
2.23.0
From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-10-04 14:58:50
_exception can be called by machine check handlers when the MCE hits
user code (e.g., pseries and powernv). This will enable local irqs
because, which is a dicey thing to do in NMI or hard irq context.
This seemed to worked out okay because a userspace MCE can basically be
treated like a synchronous interrupt (after async / imprecise MCEs are
filtered out). Since NMI and hard irq handlers have started growing
nmi_enter / irq_enter, and more irq state sanity checks, this has
started to cause problems (or at least trigger warnings).
The Fixes tag to the commit which introduced this rather than try to
work out exactly which commit was the first that could possibly cause a
problem because that may be difficult to prove.
Fixes: 9f2f79e3a3c1 ("powerpc: Disable interrupts in 64-bit kernel FP and vector faults")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/traps.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-10-04 14:59:27
This can help catch bugs such as the one fixed by the previous change
to prevent _exception() from enabling irqs.
ppc32 could have a similar warning but it has no good config option to
debug this stuff (the test may be overkill to add for production
kernels).
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/irq.c | 6 ++++++
1 file changed, 6 insertions(+)
From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-10-04 15:00:05
If a NMI hits early in an interrupt handler before the irq soft-mask
state is reconciled, that can cause a false-positive BUG with a
CONFIG_PPC_IRQ_SOFT_MASK_DEBUG assertion.
Remove that assertion and instead check the case that if regs->msr has
EE clear, then regs->softe should be marked as disabled so the irq state
looks correct to NMI handlers, the same as how it's fixed up in the
case it was implicit soft-masked.
This doesn't fix a known problem -- the change that was fixed by commit
4ec5feec1ad02 ("powerpc/64s: Make NMI record implicitly soft-masked code
as irqs disabled") was the addition of a warning in the soft-nmi
watchdog interrupt which can never actually fire when MSR[EE]=0. However
it may be important if NMI handlers grow more code, and it's less
surprising to anything using 'regs' - (I tripped over this when working
in the area).
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/interrupt.h | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-10-04 15:00:40
The machine check handler is not considered NMI on 64s. The early
handler is the true NMI handler, and then it schedules the
machine_check_exception handler to run when interrupts are enabled.
This works fine except the case of an unrecoverable MCE, where the true
NMI is taken when MSR[RI] is clear, it can not recover, so it calls
machine_check_exception directly so something might be done about it.
Calling an async handler from NMI context can result in irq state and
other things getting corrupted. This can also trigger the BUG at
arch/powerpc/include/asm/interrupt.h:168
BUG_ON(!arch_irq_disabled_regs(regs) && !(regs->msr & MSR_EE));
Fix this by making an _async version of the handler which is called
in the normal case, and a NMI version that is called for unrecoverable
interrupts.
Fixes: 2b43dd7653cc ("powerpc/64: enable MSR[EE] in irq replay pt_regs")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/interrupt.h | 5 ++---
arch/powerpc/kernel/exceptions-64s.S | 8 +++++--
arch/powerpc/kernel/traps.c | 31 ++++++++++++++++------------
3 files changed, 26 insertions(+), 18 deletions(-)
@@ -847,12 +845,19 @@ DEFINE_INTERRUPT_HANDLER_NMI(machine_check_exception)/* Must die if the interrupt is not recoverable */if(regs_is_unrecoverable(regs))die_mce("Unrecoverable Machine check",regs,SIGBUS);+}#ifdef CONFIG_PPC_BOOK3S_64-return;-#else-return0;+DEFINE_INTERRUPT_HANDLER_ASYNC(machine_check_exception_async)+{+__machine_check_exception(regs);+}#endif+DEFINE_INTERRUPT_HANDLER_NMI(machine_check_exception)+{+__machine_check_exception(regs);++return0;}DEFINE_INTERRUPT_HANDLER(SMIException)/* async? */
From: Cédric Le Goater <clg@kaod.org> Date: 2021-10-04 15:36:37
On 10/4/21 16:56, Nicholas Piggin wrote:
The machine check handler is not considered NMI on 64s. The early
handler is the true NMI handler, and then it schedules the
machine_check_exception handler to run when interrupts are enabled.
This works fine except the case of an unrecoverable MCE, where the true
NMI is taken when MSR[RI] is clear, it can not recover, so it calls
machine_check_exception directly so something might be done about it.
Calling an async handler from NMI context can result in irq state and
other things getting corrupted. This can also trigger the BUG at
arch/powerpc/include/asm/interrupt.h:168
BUG_ON(!arch_irq_disabled_regs(regs) && !(regs->msr & MSR_EE));
I was hitting this problem when I rebooted a P8 tuleta system and
this series fixes it.
Tested-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
quoted hunk
Fix this by making an _async version of the handler which is called
in the normal case, and a NMI version that is called for unrecoverable
interrupts.
Fixes: 2b43dd7653cc ("powerpc/64: enable MSR[EE] in irq replay pt_regs")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>> ---
arch/powerpc/include/asm/interrupt.h | 5 ++---
arch/powerpc/kernel/exceptions-64s.S | 8 +++++--
arch/powerpc/kernel/traps.c | 31 ++++++++++++++++------------
3 files changed, 26 insertions(+), 18 deletions(-)
@@ -847,12 +845,19 @@ DEFINE_INTERRUPT_HANDLER_NMI(machine_check_exception)/* Must die if the interrupt is not recoverable */if(regs_is_unrecoverable(regs))die_mce("Unrecoverable Machine check",regs,SIGBUS);+}#ifdef CONFIG_PPC_BOOK3S_64-return;-#else-return0;+DEFINE_INTERRUPT_HANDLER_ASYNC(machine_check_exception_async)+{+__machine_check_exception(regs);+}#endif+DEFINE_INTERRUPT_HANDLER_NMI(machine_check_exception)+{+__machine_check_exception(regs);++return0;}DEFINE_INTERRUPT_HANDLER(SMIException)/* async? */