| Message ID | 20200403132622.130394-14-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/64: machine check and system reset fixes | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (d0c12846a3a24cd6d68b608c866712bc7e471634) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 17 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Le 03/04/2020 à 15:26, Nicholas Piggin a écrit : > Similarly to the previous patch, do not trace system reset. This code > is used when there is a crash or hang, and tracing disturbs the system > more and has been known to crash in the crash handling path. > > Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > arch/powerpc/kernel/traps.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c > index 1845fd7e161a..ed7b7a6e2dc0 100644 > --- a/arch/powerpc/kernel/traps.c > +++ b/arch/powerpc/kernel/traps.c > @@ -443,6 +443,9 @@ void system_reset_exception(struct pt_regs *regs) > unsigned long hsrr0, hsrr1; > bool nested = in_nmi(); > bool saved_hsrrs = false; > + u8 ftrace_enabled = local_paca->ftrace_enabled; > + > + local_paca->ftrace_enabled = 0; I predict a build failure here in the near future ... See https://elixir.bootlin.com/linux/v5.6/source/arch/powerpc/kernel/head_8xx.S#L125 > > /* > * Avoid crashes in case of nested NMI exceptions. Recoverability > @@ -524,6 +527,8 @@ void system_reset_exception(struct pt_regs *regs) > if (!nested) > nmi_exit(); > > + local_paca->ftrace_enabled = ftrace_enabled; > + > /* What should we do here? We could issue a shutdown or hard reset. */ > } > > Christophe
Hi Nicholas,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on next-20200404]
[cannot apply to v5.6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64-machine-check-and-system-reset-fixes/20200405-030723
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
arch/powerpc/kernel/traps.c: In function 'system_reset_exception':
>> arch/powerpc/kernel/traps.c:446:22: error: 'local_paca' undeclared (first use in this function); did you mean 'local_dec'?
446 | u8 ftrace_enabled = local_paca->ftrace_enabled;
| ^~~~~~~~~~
| local_dec
arch/powerpc/kernel/traps.c:446:22: note: each undeclared identifier is reported only once for each function it appears in
vim +446 arch/powerpc/kernel/traps.c
440
441 void system_reset_exception(struct pt_regs *regs)
442 {
443 unsigned long hsrr0, hsrr1;
444 bool nested = in_nmi();
445 bool saved_hsrrs = false;
> 446 u8 ftrace_enabled = local_paca->ftrace_enabled;
447
448 local_paca->ftrace_enabled = 0;
449
450 /*
451 * Avoid crashes in case of nested NMI exceptions. Recoverability
452 * is determined by RI and in_nmi
453 */
454 if (!nested)
455 nmi_enter();
456
457 /*
458 * System reset can interrupt code where HSRRs are live and MSR[RI]=1.
459 * The system reset interrupt itself may clobber HSRRs (e.g., to call
460 * OPAL), so save them here and restore them before returning.
461 *
462 * Machine checks don't need to save HSRRs, as the real mode handler
463 * is careful to avoid them, and the regular handler is not delivered
464 * as an NMI.
465 */
466 if (cpu_has_feature(CPU_FTR_HVMODE)) {
467 hsrr0 = mfspr(SPRN_HSRR0);
468 hsrr1 = mfspr(SPRN_HSRR1);
469 saved_hsrrs = true;
470 }
471
472 hv_nmi_check_nonrecoverable(regs);
473
474 __this_cpu_inc(irq_stat.sreset_irqs);
475
476 /* See if any machine dependent calls */
477 if (ppc_md.system_reset_exception) {
478 if (ppc_md.system_reset_exception(regs))
479 goto out;
480 }
481
482 if (debugger(regs))
483 goto out;
484
485 kmsg_dump(KMSG_DUMP_OOPS);
486 /*
487 * A system reset is a request to dump, so we always send
488 * it through the crashdump code (if fadump or kdump are
489 * registered).
490 */
491 crash_fadump(regs, "System Reset");
492
493 crash_kexec(regs);
494
495 /*
496 * We aren't the primary crash CPU. We need to send it
497 * to a holding pattern to avoid it ending up in the panic
498 * code.
499 */
500 crash_kexec_secondary(regs);
501
502 /*
503 * No debugger or crash dump registered, print logs then
504 * panic.
505 */
506 die("System Reset", regs, SIGABRT);
507
508 mdelay(2*MSEC_PER_SEC); /* Wait a little while for others to print */
509 add_taint(TAINT_DIE, LOCKDEP_NOW_UNRELIABLE);
510 nmi_panic(regs, "System Reset");
511
512 out:
513 #ifdef CONFIG_PPC_BOOK3S_64
514 BUG_ON(get_paca()->in_nmi == 0);
515 if (get_paca()->in_nmi > 1)
516 nmi_panic(regs, "Unrecoverable nested System Reset");
517 #endif
518 /* Must die if the interrupt is not recoverable */
519 if (!(regs->msr & MSR_RI))
520 nmi_panic(regs, "Unrecoverable System Reset");
521
522 if (saved_hsrrs) {
523 mtspr(SPRN_HSRR0, hsrr0);
524 mtspr(SPRN_HSRR1, hsrr1);
525 }
526
527 if (!nested)
528 nmi_exit();
529
530 local_paca->ftrace_enabled = ftrace_enabled;
531
532 /* What should we do here? We could issue a shutdown or hard reset. */
533 }
534
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Christophe Leroy's on April 4, 2020 12:45 am: > > > Le 03/04/2020 à 15:26, Nicholas Piggin a écrit : >> Similarly to the previous patch, do not trace system reset. This code >> is used when there is a crash or hang, and tracing disturbs the system >> more and has been known to crash in the crash handling path. >> >> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> --- >> arch/powerpc/kernel/traps.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c >> index 1845fd7e161a..ed7b7a6e2dc0 100644 >> --- a/arch/powerpc/kernel/traps.c >> +++ b/arch/powerpc/kernel/traps.c >> @@ -443,6 +443,9 @@ void system_reset_exception(struct pt_regs *regs) >> unsigned long hsrr0, hsrr1; >> bool nested = in_nmi(); >> bool saved_hsrrs = false; >> + u8 ftrace_enabled = local_paca->ftrace_enabled; >> + >> + local_paca->ftrace_enabled = 0; > > I predict a build failure here in the near future ... Will fix. Naveen suggested some helper functions for this too. Thanks, Nick
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c index 1845fd7e161a..ed7b7a6e2dc0 100644 --- a/arch/powerpc/kernel/traps.c +++ b/arch/powerpc/kernel/traps.c @@ -443,6 +443,9 @@ void system_reset_exception(struct pt_regs *regs) unsigned long hsrr0, hsrr1; bool nested = in_nmi(); bool saved_hsrrs = false; + u8 ftrace_enabled = local_paca->ftrace_enabled; + + local_paca->ftrace_enabled = 0; /* * Avoid crashes in case of nested NMI exceptions. Recoverability @@ -524,6 +527,8 @@ void system_reset_exception(struct pt_regs *regs) if (!nested) nmi_exit(); + local_paca->ftrace_enabled = ftrace_enabled; + /* What should we do here? We could issue a shutdown or hard reset. */ }