Message ID | 20170707035611.6a9e60b6@roar.ozlabs.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
On 07/06/2017 11:26 PM, Nicholas Piggin wrote: > On Wed, 5 Jul 2017 14:04:19 +1000 > Nicholas Piggin <npiggin@gmail.com> 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. Looks good to me. Just one minor suggestion/comment below. > > --- > 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. > + */ > + 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 <mahesh@linux.vnet.ibm.com> wrote: > On 07/06/2017 11:26 PM, Nicholas Piggin wrote: > > On Wed, 5 Jul 2017 14:04:19 +1000 > > Nicholas Piggin <npiggin@gmail.com> wrote: > > + /* > > + * 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(); > > > + 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
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