[PATCH 0/4] machine check handling improvements

These are some improvements to recoverability and debugability
of machine check handling. Patch 2 depends on patch 1, but they
are otherwise independent.

Thanks,
Nick

Nicholas Piggin (4):
  powerpc/powernv: handle the platform error reboot in ppc_md.restart
  powerpc/powernv: machine check use kernel crash path
  powernv/pseries: machine check use kernel crash path
  powerpc: machine check interrupt is a non-maskable interrupt

 arch/powerpc/include/asm/opal.h           |  2 +-
 arch/powerpc/kernel/traps.c               |  9 +++--
 arch/powerpc/platforms/powernv/opal-hmi.c | 20 ++---------
 arch/powerpc/platforms/powernv/opal.c     | 57 ++++++++-----------------------
 arch/powerpc/platforms/powernv/powernv.h  |  2 ++
 arch/powerpc/platforms/powernv/setup.c    | 31 +++++++++++++++++
 arch/powerpc/platforms/pseries/ras.c      | 22 ++++++------
 7 files changed, 69 insertions(+), 74 deletions(-)

-- 
2.11.0

Unrecovered MCE and HMI errors are sent through a special restart
OPAL call to log the platform error. The downside is that they don't
go through normal crash paths, so they don't give much information
to the Linux console.

Change this by allowing them to set an error which then causes the
normal restart handler to use the platform error call. Have MCE and HMI
handlers set this and then use the normal panic path for unrecoverable
cases.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/opal.h           |  2 +-
 arch/powerpc/platforms/powernv/opal-hmi.c | 20 +++-------------
 arch/powerpc/platforms/powernv/opal.c     | 39 ++-----------------------------
 arch/powerpc/platforms/powernv/powernv.h  |  2 ++
 arch/powerpc/platforms/powernv/setup.c    | 31 ++++++++++++++++++++++++
 5 files changed, 39 insertions(+), 55 deletions(-)

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 588fb1c23af9..182dab435aad 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h

@@ -50,7 +50,7 @@ int64_t opal_tpo_write(uint64_t token, uint32_t year_mon_day,
 		       uint32_t hour_min);
 int64_t opal_cec_power_down(uint64_t request);
 int64_t opal_cec_reboot(void);
-int64_t opal_cec_reboot2(uint32_t reboot_type, char *diag);
+int64_t opal_cec_reboot2(uint32_t reboot_type, const char *diag);
 int64_t opal_read_nvram(uint64_t buffer, uint64_t size, uint64_t offset);
 int64_t opal_write_nvram(uint64_t buffer, uint64_t size, uint64_t offset);
 int64_t opal_handle_interrupt(uint64_t isn, __be64 *outstanding_event_mask);

diff --git a/arch/powerpc/platforms/powernv/opal-hmi.c b/arch/powerpc/platforms/powernv/opal-hmi.c
index 88f3c61eec95..dbb0e91058e9 100644
--- a/arch/powerpc/platforms/powernv/opal-hmi.c
+++ b/arch/powerpc/platforms/powernv/opal-hmi.c

@@ -30,6 +30,8 @@
 #include <asm/cputable.h>
 #include <asm/machdep.h>
 
+#include "powernv.h"
+
 static int opal_hmi_handler_nb_init;
 struct OpalHmiEvtNode {
 	struct list_head list;

@@ -267,8 +269,6 @@ static void hmi_event_handler(struct work_struct *work)
 	spin_unlock_irqrestore(&opal_hmi_evt_lock, flags);
 
 	if (unrecoverable) {
-		int ret;
-
 		/* Pull all HMI events from OPAL before we panic. */
 		while (opal_get_msg(__pa(&msg), sizeof(msg)) == OPAL_SUCCESS) {
 			u32 type;

@@ -284,22 +284,8 @@ static void hmi_event_handler(struct work_struct *work)
 			print_hmi_event_info(hmi_evt);
 		}
 
-		/*
-		 * Unrecoverable HMI exception. We need to inform BMC/OCC
-		 * about this error so that it can collect relevant data
-		 * for error analysis before rebooting.
-		 */
-		ret = opal_cec_reboot2(OPAL_REBOOT_PLATFORM_ERROR,
-			"Unrecoverable HMI exception");
-		if (ret == OPAL_UNSUPPORTED) {
-			pr_emerg("Reboot type %d not supported\n",
-						OPAL_REBOOT_PLATFORM_ERROR);
-		}
+		pnv_platform_error = "Unrecoverable HMI exception";
 
-		/*
-		 * Fall through and panic if opal_cec_reboot2() returns
-		 * OPAL_UNSUPPORTED.
-		 */
 		panic("Unrecoverable HMI exception");
 	}
 }

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 59684b4af4d1..4b2505d98eb8 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c

@@ -424,7 +424,6 @@ static int opal_recover_mce(struct pt_regs *regs,
 int opal_machine_check(struct pt_regs *regs)
 {
 	struct machine_check_event evt;
-	int ret;
 
 	if (!get_mce_event(&evt, MCE_EVENT_RELEASE))
 		return 0;

@@ -440,43 +439,9 @@ int opal_machine_check(struct pt_regs *regs)
 	if (opal_recover_mce(regs, &evt))
 		return 1;
 
-	/*
-	 * Unrecovered machine check, we are heading to panic path.
-	 *
-	 * We may have hit this MCE in very early stage of kernel
-	 * initialization even before opal-prd has started running. If
-	 * this is the case then this MCE error may go un-noticed or
-	 * un-analyzed if we go down panic path. We need to inform
-	 * BMC/OCC about this error so that they can collect relevant
-	 * data for error analysis before rebooting.
-	 * Use opal_cec_reboot2(OPAL_REBOOT_PLATFORM_ERROR) to do so.
-	 * This function may not return on BMC based system.
-	 */
-	ret = opal_cec_reboot2(OPAL_REBOOT_PLATFORM_ERROR,
-			"Unrecoverable Machine Check exception");
-	if (ret == OPAL_UNSUPPORTED) {
-		pr_emerg("Reboot type %d not supported\n",
-					OPAL_REBOOT_PLATFORM_ERROR);
-	}
+	pnv_platform_error = "Unrecoverable Machine Check exception";
 
-	/*
-	 * We reached here. There can be three possibilities:
-	 * 1. We are running on a firmware level that do not support
-	 *    opal_cec_reboot2()
-	 * 2. We are running on a firmware level that do not support
-	 *    OPAL_REBOOT_PLATFORM_ERROR reboot type.
-	 * 3. We are running on FSP based system that does not need opal
-	 *    to trigger checkstop explicitly for error analysis. The FSP
-	 *    PRD component would have already got notified about this
-	 *    error through other channels.
-	 *
-	 * If hardware marked this as an unrecoverable MCE, we are
-	 * going to panic anyway. Even if it didn't, it's not safe to
-	 * continue at this point, so we should explicitly panic.
-	 */
-
-	panic("PowerNV Unrecovered Machine Check");
-	return 0;
+	panic("Unrecoverable Machine Check exception");
 }
 
 /* Early hmi handler called in real mode. */

diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h
index 6dbc0a1da1f6..998f19828b85 100644
--- a/arch/powerpc/platforms/powernv/powernv.h
+++ b/arch/powerpc/platforms/powernv/powernv.h

@@ -7,6 +7,8 @@ extern void pnv_smp_init(void);
 static inline void pnv_smp_init(void) { }
 #endif
 
+extern const char *pnv_platform_error;
+
 struct pci_dev;
 
 #ifdef CONFIG_PCI

diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 2dc7e5fb86c3..c4571788f1c9 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c

@@ -125,10 +125,41 @@ static void pnv_prepare_going_down(void)
 	opal_flash_term_callback();
 }
 
+/*
+ * This can be set to have ppc_md.restart to request a
+ * OPAL_REBOOT_PLATFORM_ERROR reboot, which logs the error reason.
+ */
+const char *pnv_platform_error = NULL;
+
 static void  __noreturn pnv_restart(char *cmd)
 {
 	long rc = OPAL_BUSY;
 
+	if (pnv_platform_error) {
+		/*
+		 * Don't bother to shut things down because this will
+		 * xstop the system.
+		 */
+		rc = opal_cec_reboot2(OPAL_REBOOT_PLATFORM_ERROR,
+					pnv_platform_error);
+		if (rc == OPAL_UNSUPPORTED) {
+			pr_emerg("Reboot type %d not supported\n",
+					OPAL_REBOOT_PLATFORM_ERROR);
+			rc = OPAL_BUSY;
+		}
+		/*
+		 * We reached here. There can be three possibilities:
+		 * 1. We are running on a firmware level that do not support
+		 *    opal_cec_reboot2()
+		 * 2. We are running on a firmware level that do not support
+		 *    OPAL_REBOOT_PLATFORM_ERROR reboot type.
+		 * 3. We are running on FSP based system that does not need
+		 *    opal to trigger checkstop explicitly for error analysis.
+		 *    The FSP PRD component would have already got notified
+		 *    about this error through other channels.
+		 */
+	}
+
 	pnv_prepare_going_down();
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {

-- 
2.11.0

Use the kernel crash path in cases of recovered machine checks that
occur in kernel mode, because it gives much better Linux crash
information, and can allow the offending process to be killed without
completely taking down the system.


As a test, when triggering an i-side 0111b error (ifetch from foreign
address) in kernel mode process context on POWER9, the kernel currently
dies quickly like this:

Severe Machine check interrupt [Not recovered]
  NIP [ffff000000000000]: 0xffff000000000000
  Initiator: CPU
  Error type: Real address [Instruction fetch (foreign)]
[  127.426651616,0] OPAL: Reboot requested due to Platform error.
    Effective[  127.426693712,3] OPAL: Reboot requested due to Platform error. address: ffff000000000000
opal: Reboot type 1 not supported
Kernel panic - not syncing: PowerNV Unrecovered Machine Check
CPU: 56 PID: 4425 Comm: syscall Tainted: G   M            4.12.0-rc1-13857-ga4700a261072-dirty #35
Call Trace:
[  128.017988928,4] IPMI: BUG: Dropping ESEL on the floor due to buggy/mising code in OPAL for this BMCRebooting in 10 seconds..
Trying to free IRQ 496 from IRQ context!


After this patch, the process is killed and the kernel continues with
this message, which gives enough information to identify the offending
branch (e.g., CFAR):

Severe Machine check interrupt [Not recovered]
  NIP [ffff000000000000]: 0xffff000000000000
  Initiator: CPU
  Error type: Real address [Instruction fetch (foreign)]
    Effective address: ffff000000000000
Oops: Machine check, sig: 7 [#1]
SMP NR_CPUS=2048
NUMA
PowerNV
Modules linked in: iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp tun bridge stp llc kvm_hv kvm iptable_filter binfmt_misc vmx_crypto ip_tables x_tables autofs4 crc32c_vpmsum
CPU: 22 PID: 4436 Comm: syscall Tainted: G   M            4.12.0-rc1-13857-ga4700a261072-dirty #36
task: c000000932300000 task.stack: c000000932380000
NIP: ffff000000000000 LR: 00000000217706a4 CTR: ffff000000000000
REGS: c00000000fc8fd80 TRAP: 0200   Tainted: G   M             (4.12.0-rc1-13857-ga4700a261072-dirty)
MSR: 90000000001c1003 <SF,HV,ME,RI,LE>
  CR: 24000484  XER: 20000000
CFAR: c000000000004c80 DAR: 0000000021770a90 DSISR: 0a000000 SOFTE: 1
GPR00: 0000000000001ebe 00007fffce4818b0 0000000021797f00 0000000000000000
GPR04: 00007fff8007ac24 0000000044000484 0000000000004000 00007fff801405e8
GPR08: 900000000280f033 0000000024000484 0000000000000000 0000000000000030
GPR12: 9000000000001003 00007fff801bc370 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR28: 00007fff801b0000 0000000000000000 00000000217707a0 00007fffce481918
NIP [ffff000000000000] 0xffff000000000000
LR [00000000217706a4] 0x217706a4
Call Trace:
Instruction dump:
XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
---[ end trace 32ae1dabb4f8dae6 ]---
---
 arch/powerpc/platforms/powernv/opal.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 4b2505d98eb8..92f00113227f 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c

@@ -30,6 +30,7 @@
 #include <asm/opal.h>
 #include <asm/firmware.h>
 #include <asm/mce.h>
+#include <asm/bug.h>
 
 #include "powernv.h"
 

@@ -407,15 +408,22 @@ static int opal_recover_mce(struct pt_regs *regs,
 		/* Fatal machine check */
 		pr_err("Machine check interrupt is fatal\n");
 		recovered = 0;
-	} else if ((evt->severity == MCE_SEV_ERROR_SYNC) &&
-			(user_mode(regs) && !is_global_init(current))) {
+	} else if (evt->severity == MCE_SEV_ERROR_SYNC) {
 		/*
-		 * For now, kill the task if we have received exception when
-		 * in userspace.
+		 * Try to kill processes if we get a synchronous machine check
+		 * (e.g., one caused by execution of this instruction). This
+		 * will devolve into a panic if we try to kill init or are in
+		 * an interrupt etc.
 		 *
 		 * TODO: Queue up this address for hwpoisioning later.
+		 * TODO: This is not quite right for d-side machine
+		 *       checks ->nip is not necessarily the important
+		 *       address.
 		 */
-		_exception(SIGBUS, regs, BUS_MCEERR_AR, regs->nip);
+		if ((user_mode(regs)))
+			_exception(SIGBUS, regs, BUS_MCEERR_AR, regs->nip);
+		else
+			die("Machine check", regs, SIGBUS);
 		recovered = 1;
 	}
 	return recovered;

-- 
2.11.0

Similarly to the powernv patch, pseries can recover from and/or
print more error details from a recovered machine check if it
goes via the kernel crash path rather than panic()ing in the case
of MCE in kernel mode.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/platforms/pseries/ras.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 4923ffe230cf..eeac00160de0 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c

@@ -436,19 +436,19 @@ static int recover_mce(struct pt_regs *regs, struct rtas_error_log *err)
 		       "be degraded\n");
 		recovered = 1;
 
-	} else if (user_mode(regs) && !is_global_init(current) &&
-		   rtas_error_severity(err) == RTAS_SEVERITY_ERROR_SYNC) {
-
+	} else if (rtas_error_severity(err) == RTAS_SEVERITY_ERROR_SYNC) {
 		/*
-		 * If we received a synchronous error when in userspace
-		 * kill the task. Firmware may report details of the fail
-		 * asynchronously, so we can't rely on the target and type
-		 * fields being valid here.
+		 * Firmware may report details of the fail asynchronously, so
+		 * we can't rely on the target and type fields being valid
+		 * here.
 		 */
-		printk(KERN_ERR "MCE: uncorrectable error, killing task "
-		       "%s:%d\n", current->comm, current->pid);
-
-		_exception(SIGBUS, regs, BUS_MCEERR_AR, regs->nip);
+		if ((user_mode(regs))) {
+			printk(KERN_ERR "MCE: uncorrectable error, killing task"
+				" %s:%d\n", current->comm, current->pid);
+			_exception(SIGBUS, regs, BUS_MCEERR_AR, regs->nip);
+		} else {
+			die("Machine check", regs, SIGBUS);
+		}
 		recovered = 1;
 	}
 

-- 
2.11.0

Use nmi_enter similarly to system reset interrupts. This uses NMI
printk NMI buffers and turns off various debugging facilities that
helps avoid tripping on ourselves or other CPUs.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/traps.c           | 9 ++++++---
 arch/powerpc/platforms/powernv/opal.c | 2 +-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 574e949f8db9..1933b74372a1 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c

@@ -767,8 +767,10 @@ int machine_check_generic(struct pt_regs *regs)
 
 void machine_check_exception(struct pt_regs *regs)
 {
-	enum ctx_state prev_state = exception_enter();
 	int recover = 0;
+	bool nested = in_nmi();
+	if (!nested)
+		nmi_enter();
 
 	__this_cpu_inc(irq_stat.mce_exceptions);
 

@@ -798,10 +800,11 @@ void machine_check_exception(struct pt_regs *regs)
 
 	/* Must die if the interrupt is not recoverable */
 	if (!(regs->msr & MSR_RI))
-		panic("Unrecoverable Machine check");
+		nmi_panic("Unrecoverable Machine check");
 
 bail:
-	exception_exit(prev_state);
+	if (!nested)
+		nmi_exit();
 }
 
 void SMIException(struct pt_regs *regs)

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 92f00113227f..29967de9823d 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c

@@ -449,7 +449,7 @@ int opal_machine_check(struct pt_regs *regs)
 
 	pnv_platform_error = "Unrecoverable Machine Check exception";
 
-	panic("Unrecoverable Machine Check exception");
+	nmi_panic("Unrecoverable Machine Check exception");
 }
 
 /* Early hmi handler called in real mode. */

-- 
2.11.0

On Wed,  5 Jul 2017 14:04:19 +1000
Nicholas Piggin [off-list ref] wrote:

Unrecovered MCE and HMI errors are sent through a special restart
OPAL call to log the platform error. The downside is that they don't
go through normal crash paths, so they don't give much information
to the Linux console.

Change this by allowing them to set an error which then causes the
normal restart handler to use the platform error call. Have MCE and HMI
handlers set this and then use the normal panic path for unrecoverable
cases.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---

This patch is a bit clunky, setting this global variable. But it's
difficult to get this through the normal crash/panic paths by any
other way that I've found.

A concern is that we would like to add the opal log as early as
possible, but also print some information to the Linux console. This
goes against a competing concern that the more we do before logging
and xstop, the larger window something might go wrong.

So I would like to be able to cause opal to log a platform error
ASAP in the machine check handler, but then do some Linux crash
dumping before xstopping. That would require a new opal logging API
or convention though. So maybe this is an improvement (which also
allows patch 2 to be implemented more easily).

Anyway, discussion and criticism welcome.


Thanks,
Nick

On Wed,  5 Jul 2017 14:04:19 +1000
Nicholas Piggin [off-list ref] wrote:

Unrecovered MCE and HMI errors are sent through a special restart
OPAL call to log the platform error. The downside is that they don't
go through normal crash paths, so they don't give much information
to the Linux console.

Change this by allowing them to set an error which then causes the
normal restart handler to use the platform error call. Have MCE and HMI
handlers set this and then use the normal panic path for unrecoverable
cases.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Mahesh brought up a couple of good points about this offline.
Firstly that some HMI erorrs will stop the TB, second that if
crash dumps are registered then we will not get to the platform
reboot code from panic.

So it was a nice idea, but it seems to be just a bit too hard to
do exactly what we want in the panic path. So the other option is
put some of the printk and console flushing into the opal platform
error handler.

It's not really ideal to duplicate this code here... but it's better
than not printing anything.

Patch 2 won't be able to just call die() for kernel context now, but
it will have to check in_interrupt(), panic_on_oops, etc. to make
sure die() doesn't panic. But that should be okay.

This is what I have. If there are no great objections I'll repost
a v2 series with this.

---

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 588fb1c23af9..182dab435aad 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h

@@ -50,7 +50,7 @@ int64_t opal_tpo_write(uint64_t token, uint32_t year_mon_day,
 		       uint32_t hour_min);
 int64_t opal_cec_power_down(uint64_t request);
 int64_t opal_cec_reboot(void);
-int64_t opal_cec_reboot2(uint32_t reboot_type, char *diag);
+int64_t opal_cec_reboot2(uint32_t reboot_type, const char *diag);
 int64_t opal_read_nvram(uint64_t buffer, uint64_t size, uint64_t offset);
 int64_t opal_write_nvram(uint64_t buffer, uint64_t size, uint64_t offset);
 int64_t opal_handle_interrupt(uint64_t isn, __be64 *outstanding_event_mask);

diff --git a/arch/powerpc/platforms/powernv/opal-hmi.c b/arch/powerpc/platforms/powernv/opal-hmi.c
index 88f3c61eec95..d78fed728cdf 100644
--- a/arch/powerpc/platforms/powernv/opal-hmi.c
+++ b/arch/powerpc/platforms/powernv/opal-hmi.c

@@ -30,6 +30,8 @@
 #include <asm/cputable.h>
 #include <asm/machdep.h>
 
+#include "powernv.h"
+
 static int opal_hmi_handler_nb_init;
 struct OpalHmiEvtNode {
 	struct list_head list;

@@ -267,8 +269,6 @@ static void hmi_event_handler(struct work_struct *work)
 	spin_unlock_irqrestore(&opal_hmi_evt_lock, flags);
 
 	if (unrecoverable) {
-		int ret;
-
 		/* Pull all HMI events from OPAL before we panic. */
 		while (opal_get_msg(__pa(&msg), sizeof(msg)) == OPAL_SUCCESS) {
 			u32 type;

@@ -284,23 +284,7 @@ static void hmi_event_handler(struct work_struct *work)
 			print_hmi_event_info(hmi_evt);
 		}
 
-		/*
-		 * Unrecoverable HMI exception. We need to inform BMC/OCC
-		 * about this error so that it can collect relevant data
-		 * for error analysis before rebooting.
-		 */
-		ret = opal_cec_reboot2(OPAL_REBOOT_PLATFORM_ERROR,
-			"Unrecoverable HMI exception");
-		if (ret == OPAL_UNSUPPORTED) {
-			pr_emerg("Reboot type %d not supported\n",
-						OPAL_REBOOT_PLATFORM_ERROR);
-		}
-
-		/*
-		 * Fall through and panic if opal_cec_reboot2() returns
-		 * OPAL_UNSUPPORTED.
-		 */
-		panic("Unrecoverable HMI exception");
+		pnv_platform_error_reboot(NULL, "Unrecoverable HMI exception");
 	}
 }
 

diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 59684b4af4d1..ccbcfa22bacf 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c

@@ -25,6 +25,10 @@
 #include <linux/memblock.h>
 #include <linux/kthread.h>
 #include <linux/freezer.h>
+#include <linux/printk.h>
+#include <linux/kmsg_dump.h>
+#include <linux/console.h>
+#include <linux/sched/debug.h>
 
 #include <asm/machdep.h>
 #include <asm/opal.h>

@@ -421,10 +425,57 @@ static int opal_recover_mce(struct pt_regs *regs,
 	return recovered;
 }
 
+void pnv_platform_error_reboot(struct pt_regs *regs, const char *msg)
+{
+	/*
+	 * This is mostly taken from kernel/panic.c, but tries to do
+	 * relatively minimal work. Don't use delay functions (TB may
+	 * be broken), don't crash dump (need to set a firmware log),
+	 * don't run notifiers. We do want to get some information to
+	 * Linux console.
+	 */
+	smp_send_stop();
+
+	console_verbose();
+	bust_spinlocks(1);
+	pr_emerg("Hardware platform error: %s\n", msg);
+	if (regs)
+		show_regs(regs);
+	printk_safe_flush_on_panic();
+	kmsg_dump(KMSG_DUMP_PANIC);
+	bust_spinlocks(0);
+	debug_locks_off();
+	console_flush_on_panic();
+
+	/*
+	 * Don't bother to shut things down because this will
+	 * xstop the system.
+	 */
+	if (opal_cec_reboot2(OPAL_REBOOT_PLATFORM_ERROR, msg)
+						== OPAL_UNSUPPORTED) {
+		pr_emerg("Reboot type %d not supported for %s\n",
+				OPAL_REBOOT_PLATFORM_ERROR, msg);
+	}
+
+	/*
+	 * We reached here. There can be three possibilities:
+	 * 1. We are running on a firmware level that do not support
+	 *    opal_cec_reboot2()
+	 * 2. We are running on a firmware level that do not support
+	 *    OPAL_REBOOT_PLATFORM_ERROR reboot type.
+	 * 3. We are running on FSP based system that does not need
+	 *    opal to trigger checkstop explicitly for error analysis.
+	 *    The FSP PRD component would have already got notified
+	 *    about this error through other channels.
+	 */
+
+	for (;;)
+		;
+}
+
 int opal_machine_check(struct pt_regs *regs)
 {
 	struct machine_check_event evt;
-	int ret;
 
 	if (!get_mce_event(&evt, MCE_EVENT_RELEASE))
 		return 0;

@@ -440,43 +491,7 @@ int opal_machine_check(struct pt_regs *regs)
 	if (opal_recover_mce(regs, &evt))
 		return 1;
 
-	/*
-	 * Unrecovered machine check, we are heading to panic path.
-	 *
-	 * We may have hit this MCE in very early stage of kernel
-	 * initialization even before opal-prd has started running. If
-	 * this is the case then this MCE error may go un-noticed or
-	 * un-analyzed if we go down panic path. We need to inform
-	 * BMC/OCC about this error so that they can collect relevant
-	 * data for error analysis before rebooting.
-	 * Use opal_cec_reboot2(OPAL_REBOOT_PLATFORM_ERROR) to do so.
-	 * This function may not return on BMC based system.
-	 */
-	ret = opal_cec_reboot2(OPAL_REBOOT_PLATFORM_ERROR,
-			"Unrecoverable Machine Check exception");
-	if (ret == OPAL_UNSUPPORTED) {
-		pr_emerg("Reboot type %d not supported\n",
-					OPAL_REBOOT_PLATFORM_ERROR);
-	}
-
-	/*
-	 * We reached here. There can be three possibilities:
-	 * 1. We are running on a firmware level that do not support
-	 *    opal_cec_reboot2()
-	 * 2. We are running on a firmware level that do not support
-	 *    OPAL_REBOOT_PLATFORM_ERROR reboot type.
-	 * 3. We are running on FSP based system that does not need opal
-	 *    to trigger checkstop explicitly for error analysis. The FSP
-	 *    PRD component would have already got notified about this
-	 *    error through other channels.
-	 *
-	 * If hardware marked this as an unrecoverable MCE, we are
-	 * going to panic anyway. Even if it didn't, it's not safe to
-	 * continue at this point, so we should explicitly panic.
-	 */
-
-	panic("PowerNV Unrecovered Machine Check");
-	return 0;
+	pnv_platform_error_reboot(regs, "Unrecoverable Machine Check exception");
 }
 
 /* Early hmi handler called in real mode. */

diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h
index 6dbc0a1da1f6..a159d48573d7 100644
--- a/arch/powerpc/platforms/powernv/powernv.h
+++ b/arch/powerpc/platforms/powernv/powernv.h

@@ -7,6 +7,8 @@ extern void pnv_smp_init(void);
 static inline void pnv_smp_init(void) { }
 #endif
 
+extern void pnv_platform_error_reboot(struct pt_regs *regs, const char *msg) __noreturn;
+
 struct pci_dev;
 
 #ifdef CONFIG_PCI

-- 
2.11.0

Hi Nicholas,

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.12 next-20170706]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Nicholas-Piggin/machine-check-handling-improvements/20170706-181843
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   arch/powerpc/kernel/traps.c: In function 'machine_check_exception':

quoted

arch/powerpc/kernel/traps.c:788:13: error: passing argument 1 of 'nmi_panic' from incompatible pointer type [-Werror=incompatible-pointer-types]

      nmi_panic("Unrecoverable Machine check");
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/asm-generic/bug.h:15:0,
                    from arch/powerpc/include/asm/bug.h:127,
                    from include/linux/bug.h:4,
                    from arch/powerpc/include/asm/mmu.h:125,
                    from arch/powerpc/include/asm/lppaca.h:36,
                    from arch/powerpc/include/asm/paca.h:21,
                    from arch/powerpc/include/asm/current.h:16,
                    from include/linux/sched.h:11,
                    from arch/powerpc/kernel/traps.c:19:
   include/linux/kernel.h:271:6: note: expected 'struct pt_regs *' but argument is of type 'char *'
    void nmi_panic(struct pt_regs *regs, const char *msg);
         ^~~~~~~~~

quoted

arch/powerpc/kernel/traps.c:788:3: error: too few arguments to function 'nmi_panic'

      nmi_panic("Unrecoverable Machine check");
      ^~~~~~~~~
   In file included from include/asm-generic/bug.h:15:0,
                    from arch/powerpc/include/asm/bug.h:127,
                    from include/linux/bug.h:4,
                    from arch/powerpc/include/asm/mmu.h:125,
                    from arch/powerpc/include/asm/lppaca.h:36,
                    from arch/powerpc/include/asm/paca.h:21,
                    from arch/powerpc/include/asm/current.h:16,
                    from include/linux/sched.h:11,
                    from arch/powerpc/kernel/traps.c:19:
   include/linux/kernel.h:271:6: note: declared here
    void nmi_panic(struct pt_regs *regs, const char *msg);
         ^~~~~~~~~
   cc1: all warnings being treated as errors
--
   arch/powerpc/platforms/powernv/opal.c: In function 'opal_machine_check':

quoted

arch/powerpc/platforms/powernv/opal.c:452:12: error: passing argument 1 of 'nmi_panic' from incompatible pointer type [-Werror=incompatible-pointer-types]

     nmi_panic("Unrecoverable Machine Check exception");
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/list.h:8:0,
                    from include/linux/kobject.h:20,
                    from include/linux/of.h:21,
                    from arch/powerpc/platforms/powernv/opal.c:16:
   include/linux/kernel.h:271:6: note: expected 'struct pt_regs *' but argument is of type 'char *'
    void nmi_panic(struct pt_regs *regs, const char *msg);
         ^~~~~~~~~

quoted

arch/powerpc/platforms/powernv/opal.c:452:2: error: too few arguments to function 'nmi_panic'

     nmi_panic("Unrecoverable Machine Check exception");
     ^~~~~~~~~
   In file included from include/linux/list.h:8:0,
                    from include/linux/kobject.h:20,
                    from include/linux/of.h:21,
                    from arch/powerpc/platforms/powernv/opal.c:16:
   include/linux/kernel.h:271:6: note: declared here
    void nmi_panic(struct pt_regs *regs, const char *msg);
         ^~~~~~~~~
   arch/powerpc/platforms/powernv/opal.c:453:1: error: control reaches end of non-void function [-Werror=return-type]
    }
    ^
   cc1: all warnings being treated as errors

vim +/nmi_panic +788 arch/powerpc/kernel/traps.c

   782			goto bail;
   783	
   784		die("Machine check", regs, SIGBUS);
   785	
   786		/* Must die if the interrupt is not recoverable */
   787		if (!(regs->msr & MSR_RI))
 > 788			nmi_panic("Unrecoverable Machine check");
   789	
   790	bail:
   791		if (!nested)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

On 07/06/2017 11:26 PM, Nicholas Piggin wrote:

On Wed,  5 Jul 2017 14:04:19 +1000
Nicholas Piggin [off-list ref] wrote:

quoted

Unrecovered MCE and HMI errors are sent through a special restart
OPAL call to log the platform error. The downside is that they don't
go through normal crash paths, so they don't give much information
to the Linux console.

Change this by allowing them to set an error which then causes the
normal restart handler to use the platform error call. Have MCE and HMI
handlers set this and then use the normal panic path for unrecoverable
cases.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Mahesh brought up a couple of good points about this offline.
Firstly that some HMI erorrs will stop the TB, second that if
crash dumps are registered then we will not get to the platform
reboot code from panic.

So it was a nice idea, but it seems to be just a bit too hard to
do exactly what we want in the panic path. So the other option is
put some of the printk and console flushing into the opal platform
error handler.

It's not really ideal to duplicate this code here... but it's better
than not printing anything.

Patch 2 won't be able to just call die() for kernel context now, but
it will have to check in_interrupt(), panic_on_oops, etc. to make
sure die() doesn't panic. But that should be okay.

This is what I have. If there are no great objections I'll repost
a v2 series with this.

Looks good to me. Just one minor suggestion/comment below.

Not sure if looping forever unconditionally here is better idea. How
about we check panic_timeout and decide whether to reboot or fall
through for loop ?

	if (panic_timeout != 0)
		emergency_restart();

+	for (;;)
+		;> +}

Thanks,
-Mahesh.

On Mon, 10 Jul 2017 11:18:35 +0530
Mahesh Jagannath Salgaonkar [off-list ref] wrote:

On 07/06/2017 11:26 PM, Nicholas Piggin wrote:

quoted

On Wed,  5 Jul 2017 14:04:19 +1000
Nicholas Piggin [off-list ref] wrote:

quoted

+	/*
+	 * We reached here. There can be three possibilities:
+	 * 1. We are running on a firmware level that do not support
+	 *    opal_cec_reboot2()
+	 * 2. We are running on a firmware level that do not support
+	 *    OPAL_REBOOT_PLATFORM_ERROR reboot type.
+	 * 3. We are running on FSP based system that does not need
+	 *    opal to trigger checkstop explicitly for error analysis.
+	 *    The FSP PRD component would have already got notified
+	 *    about this error through other channels.
+	 */
+  

Not sure if looping forever unconditionally here is better idea. How
about we check panic_timeout and decide whether to reboot or fall
through for loop ?

	if (panic_timeout != 0)
		emergency_restart();

quoted

+	for (;;)
+		;> +}  

Yes that's a good point. What I've done is just to call ppc_md.restart().
To match BMC behaviour, particularly for point #3, I think we expect a
restart here. MCE is not quite the same thing as panic, so I think that's
okay to ignore the panic timeout.

The important thing is the MCE messages will have been sent to Linux
console.

Thanks,
Nick