* [RFC PATCH 1/4] powerpc/64s: return more carefully from sreset NMI
2018-03-16 10:02 [RFC PATCH 0/4] more sreset debugging improvements Nicholas Piggin
@ 2018-03-16 10:02 ` Nicholas Piggin
2018-03-16 10:02 ` [RFC PATCH 2/4] powerpc/64s: sreset panic if there is no debugger or crash dump handlers Nicholas Piggin
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2018-03-16 10:02 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
System Reset, being an NMI, must return more carefully than other
interrupts. It has traditionally returned via the nromal return
from exception path, but that has a number of problems.
- r13 does not get restored if returning to kernel. This is for
interrupts which may cause a context switch, which sreset will
never do. Interrupting OPAL (which uses a different r13) is one
place where this causes breakage.
- It may cause several other problems returning to kernel with
preempt or TIF_EMULATE_STACK_STORE if it hits at the wrong time.
It's safer just to have a simple restore and return, like machine
check which is the other NMI.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64s.S | 61 ++++++++++++++++++++++++++++++++++--
1 file changed, 58 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 3ac87e53b3da..114ff7170562 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -139,6 +139,21 @@ EXC_COMMON_BEGIN(system_reset_idle_common)
b pnv_powersave_wakeup
#endif
+/*
+ * Set IRQS_ALL_DISABLED unconditionally so arch_irqs_disabled does
+ * the right thing. We do not want to reconcile because that goes
+ * through irq tracing which we don't want in NMI.
+ *
+ * Save PACAIRQHAPPENED because some code will do a hard disable
+ * (e.g., xmon). So we want to restore this back to where it was
+ * when we return. DAR is unused in the stack, so save it there.
+ */
+#define ADD_RECONCILE_NMI \
+ li r10,IRQS_ALL_DISABLED; \
+ stb r10,PACAIRQSOFTMASK(r13); \
+ lbz r10,PACAIRQHAPPENED(r13); \
+ std r10,_DAR(r1)
+
EXC_COMMON_BEGIN(system_reset_common)
/*
* Increment paca->in_nmi then enable MSR_RI. SLB or MCE will be able
@@ -157,16 +172,56 @@ EXC_COMMON_BEGIN(system_reset_common)
subi r1,r1,INT_FRAME_SIZE
EXCEPTION_COMMON_NORET_STACK(PACA_EXNMI, 0x100,
system_reset, system_reset_exception,
- ADD_NVGPRS;ADD_RECONCILE)
+ ADD_NVGPRS;ADD_RECONCILE_NMI)
+
+ /* This (and MCE) can be simplified with mtmsrd L=1 */
+ /* Clear MSR_RI before setting SRR0 and SRR1. */
+ li r0,MSR_RI
+ mfmsr r9
+ andc r9,r9,r0
+ mtmsrd r9,1
/*
- * The stack is no longer in use, decrement in_nmi.
+ * MSR_RI is clear, now we can decrement paca->in_nmi.
*/
lhz r10,PACA_IN_NMI(r13)
subi r10,r10,1
sth r10,PACA_IN_NMI(r13)
- b ret_from_except
+ /*
+ * Restore soft mask settings.
+ */
+ ld r10,_DAR(r1)
+ stb r10,PACAIRQHAPPENED(r13)
+ ld r10,SOFTE(r1)
+ stb r10,PACAIRQSOFTMASK(r13)
+
+ /*
+ * Keep below code in synch with MACHINE_CHECK_HANDLER_WINDUP.
+ * Should share common bits...
+ */
+
+ /* Move original SRR0 and SRR1 into the respective regs */
+ ld r9,_MSR(r1)
+ mtspr SPRN_SRR1,r9
+ ld r3,_NIP(r1)
+ mtspr SPRN_SRR0,r3
+ ld r9,_CTR(r1)
+ mtctr r9
+ ld r9,_XER(r1)
+ mtxer r9
+ ld r9,_LINK(r1)
+ mtlr r9
+ REST_GPR(0, r1)
+ REST_8GPRS(2, r1)
+ REST_GPR(10, r1)
+ ld r11,_CCR(r1)
+ mtcr r11
+ REST_GPR(11, r1)
+ REST_2GPRS(12, r1)
+ /* restore original r1. */
+ ld r1,GPR1(r1)
+ RFI_TO_USER_OR_KERNEL
#ifdef CONFIG_PPC_PSERIES
/*
--
2.16.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [RFC PATCH 2/4] powerpc/64s: sreset panic if there is no debugger or crash dump handlers
2018-03-16 10:02 [RFC PATCH 0/4] more sreset debugging improvements Nicholas Piggin
2018-03-16 10:02 ` [RFC PATCH 1/4] powerpc/64s: return more carefully from sreset NMI Nicholas Piggin
@ 2018-03-16 10:02 ` Nicholas Piggin
2018-03-16 10:02 ` [RFC PATCH 3/4] powerpc/powernv/nvram: opal_nvram_write handle unknown OPAL errors Nicholas Piggin
2018-03-16 10:02 ` [RFC PATCH 4/4] powerpc/xmon: Detect if OPAL was interrupted and mark unrecoverable Nicholas Piggin
3 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2018-03-16 10:02 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
system_reset_exception does most of its own crash handling now,
invoking the debugger or crash dumps if they are registered. If not,
then it goes through to die() to print stack traces, and then is
supposed to panic (according to comments).
However after die() prints oopses, it does its own handling which
doesn't allow system_reset_exception to panic (e.g., it may just
kill the current process). This patch causes sreset exceptions to
return from die after it prints messages but before acting.
This also stops die from invoking the debugger on 0x100 crashes.
system_reset_exception similarly calls the debugger. It had been
thought this was harmless (because if the debugger was disabled,
neither call would fire, and if it was enabled the first call
would return). However in some cases like xmon 'X' command, the
debugger returns 0, which currently causes it to be entered
again (first in system_reset_exception, then in die), which is
confusing.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/traps.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 1e48d157196a..b0ac57fa5056 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -208,6 +208,12 @@ static void oops_end(unsigned long flags, struct pt_regs *regs,
}
raw_local_irq_restore(flags);
+ /*
+ * system_reset_excption handles debugger, crash dump, panic, for 0x100
+ */
+ if (TRAP(regs) == 0x100)
+ return;
+
crash_fadump(regs, "die oops");
if (kexec_should_crash(current))
@@ -272,8 +278,13 @@ void die(const char *str, struct pt_regs *regs, long err)
{
unsigned long flags;
- if (debugger(regs))
- return;
+ /*
+ * system_reset_excption handles debugger, crash dump, panic, for 0x100
+ */
+ if (TRAP(regs) != 0x100) {
+ if (debugger(regs))
+ return;
+ }
flags = oops_begin(regs);
if (__die(str, regs, err))
--
2.16.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [RFC PATCH 3/4] powerpc/powernv/nvram: opal_nvram_write handle unknown OPAL errors
2018-03-16 10:02 [RFC PATCH 0/4] more sreset debugging improvements Nicholas Piggin
2018-03-16 10:02 ` [RFC PATCH 1/4] powerpc/64s: return more carefully from sreset NMI Nicholas Piggin
2018-03-16 10:02 ` [RFC PATCH 2/4] powerpc/64s: sreset panic if there is no debugger or crash dump handlers Nicholas Piggin
@ 2018-03-16 10:02 ` Nicholas Piggin
2018-03-16 10:02 ` [RFC PATCH 4/4] powerpc/xmon: Detect if OPAL was interrupted and mark unrecoverable Nicholas Piggin
3 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2018-03-16 10:02 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
opal_nvram_write currently just assumes success if it encounters an
error other than OPAL_BUSY or OPAL_BUSY_EVENT. Have it return -EIO
on other errors instead.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/platforms/powernv/opal-nvram.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/opal-nvram.c b/arch/powerpc/platforms/powernv/opal-nvram.c
index 9db4398ded5d..13bf625dc3e8 100644
--- a/arch/powerpc/platforms/powernv/opal-nvram.c
+++ b/arch/powerpc/platforms/powernv/opal-nvram.c
@@ -59,6 +59,8 @@ static ssize_t opal_nvram_write(char *buf, size_t count, loff_t *index)
if (rc == OPAL_BUSY_EVENT)
opal_poll_events(NULL);
}
+ if (rc)
+ return -EIO;
*index += count;
return count;
}
--
2.16.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC PATCH 4/4] powerpc/xmon: Detect if OPAL was interrupted and mark unrecoverable
2018-03-16 10:02 [RFC PATCH 0/4] more sreset debugging improvements Nicholas Piggin
` (2 preceding siblings ...)
2018-03-16 10:02 ` [RFC PATCH 3/4] powerpc/powernv/nvram: opal_nvram_write handle unknown OPAL errors Nicholas Piggin
@ 2018-03-16 10:02 ` Nicholas Piggin
3 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2018-03-16 10:02 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
xmon can be entered via sreset NMI (from a management sreset, or an
NMI IPI), which can interrupt OPAL. xmon will then issue OPAL calls
to read and write the console, which re-enter OPAL and will destroy
the OPAL stack. So xmon must not attempt to recover in this case.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
--
Skiboot at the moment will spew messages about re-entrant call
detected and things will go bad from there. I have some patches
for it that allow a few calls through that allow the console to
work from xmon, obviously still not recoverable.
---
arch/powerpc/include/asm/opal.h | 2 ++
arch/powerpc/platforms/powernv/opal.c | 5 +++++
arch/powerpc/xmon/xmon.c | 14 ++++++++++++++
3 files changed, 21 insertions(+)
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 12e70fb58700..b7efd9b0c720 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -27,6 +27,8 @@ extern struct kobject *opal_kobj;
/* /ibm,opal */
extern struct device_node *opal_node;
+bool in_opal_text(u64 address);
+
/* API functions */
int64_t opal_invalid_call(void);
int64_t opal_npu_destroy_context(uint64_t phb_id, uint64_t pid, uint64_t bdf);
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index c15182765ff5..88627b59ea2e 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -64,6 +64,11 @@ static struct atomic_notifier_head opal_msg_notifier_head[OPAL_MSG_TYPE_MAX];
static uint32_t opal_heartbeat;
static struct task_struct *kopald_tsk;
+bool in_opal_text(u64 address)
+{
+ return (address >= opal.base && address < opal.base + opal.size);
+}
+
void opal_configure_cores(void)
{
u64 reinit_flags = 0;
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 82e1a3ee6e0f..482709933c46 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -510,6 +510,20 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
xmon_fault_jmp[cpu] = recurse_jmp;
+ if (firmware_has_feature(FW_FEATURE_OPAL)) {
+ if (in_opal_text(regs->nip)) {
+ printf("WARNING: cpu 0x%x stopped in OPAL, cannot recover\n", cpu);
+ regs->msr &= ~MSR_RI;
+ /*
+ * It should be possible to return to OPAL if we
+ * didn't do anything else here, but xmon makes
+ * re-entrant OPAL calls to print console, which will
+ * trash the OPAL stack. So we have to mark ourselves
+ * as non-recoverable here.
+ */
+ }
+ }
+
bp = NULL;
if ((regs->msr & (MSR_IR|MSR_PR|MSR_64BIT)) == (MSR_IR|MSR_64BIT))
bp = at_breakpoint(regs->nip);
--
2.16.1
^ permalink raw reply related [flat|nested] 5+ messages in thread