From b317cfd4c0d102045ef84dc045141de416f9d808 Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Thu, 1 Nov 2018 21:34:17 +0000 Subject: [PATCH] Don't enter DDB for fatal traps before panic by default. Add a new 'debugger_on_trap' knob separate from 'debugger_on_panic' and make the calls to kdb_trap() in MD fatal trap handlers prior to calling panic() conditional on this new knob instead of 'debugger_on_panic'. Disable the new knob by default. Developers who wish to recover from a fatal fault by adjusting saved register state and retrying the faulting instruction can still do so by enabling the new knob. However, for the more common case this makes the user experience for panics due to a fatal fault match the user experience for other panics, e.g. 'c' in DDB will generate a crash dump and reboot the system rather than being stuck in an infinite loop of fatal fault messages and DDB prompts. Reviewed by: kib, avg MFC after: 2 months Sponsored by: Chelsio Communications Differential Revision: https://reviews.freebsd.org/D17768 --- sys/amd64/amd64/trap.c | 2 +- sys/arm/arm/trap-v4.c | 2 +- sys/arm/arm/trap-v6.c | 2 +- sys/arm64/arm64/trap.c | 2 +- sys/i386/i386/trap.c | 2 +- sys/kern/kern_shutdown.c | 9 +++++++-- sys/mips/mips/trap.c | 2 +- sys/powerpc/powerpc/trap.c | 2 +- sys/sys/kdb.h | 2 +- 9 files changed, 15 insertions(+), 10 deletions(-) diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c index 27faa7c14fa..7a097302529 100644 --- a/sys/amd64/amd64/trap.c +++ b/sys/amd64/amd64/trap.c @@ -916,7 +916,7 @@ trap_fatal(frame, eva) curproc->p_pid, curthread->td_name); #ifdef KDB - if (debugger_on_panic) { + if (debugger_on_trap) { kdb_why = KDB_WHY_TRAP; handled = kdb_trap(type, 0, frame); kdb_why = KDB_WHY_UNSET; diff --git a/sys/arm/arm/trap-v4.c b/sys/arm/arm/trap-v4.c index cd701c95f6a..d2690ba46d6 100644 --- a/sys/arm/arm/trap-v4.c +++ b/sys/arm/arm/trap-v4.c @@ -456,7 +456,7 @@ dab_fatal(struct trapframe *tf, u_int fsr, u_int far, struct thread *td, printf(", pc =%08x\n\n", tf->tf_pc); #ifdef KDB - if (debugger_on_panic) { + if (debugger_on_trap) { kdb_why = KDB_WHY_TRAP; handled = kdb_trap(fsr, 0, tf); kdb_why = KDB_WHY_UNSET; diff --git a/sys/arm/arm/trap-v6.c b/sys/arm/arm/trap-v6.c index ae78401e91a..58941f06ae9 100644 --- a/sys/arm/arm/trap-v6.c +++ b/sys/arm/arm/trap-v6.c @@ -599,7 +599,7 @@ abort_fatal(struct trapframe *tf, u_int idx, u_int fsr, u_int far, printf(", pc =%08x\n\n", tf->tf_pc); #ifdef KDB - if (debugger_on_panic) { + if (debugger_on_trap) { kdb_why = KDB_WHY_TRAP; kdb_trap(fsr, 0, tf); kdb_why = KDB_WHY_UNSET; diff --git a/sys/arm64/arm64/trap.c b/sys/arm64/arm64/trap.c index 9de15479800..bb5924336b5 100644 --- a/sys/arm64/arm64/trap.c +++ b/sys/arm64/arm64/trap.c @@ -256,7 +256,7 @@ no_pmap_fault: printf(" esr: %.8lx\n", esr); #ifdef KDB - if (debugger_on_panic) { + if (debugger_on_trap) { kdb_why = KDB_WHY_TRAP; handled = kdb_trap(ESR_ELx_EXCEPTION(esr), 0, frame); diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c index 4b62484533c..a81ec6ee89d 100644 --- a/sys/i386/i386/trap.c +++ b/sys/i386/i386/trap.c @@ -977,7 +977,7 @@ trap_fatal(frame, eva) curproc->p_pid, curthread->td_name); #ifdef KDB - if (debugger_on_panic) { + if (debugger_on_trap) { kdb_why = KDB_WHY_TRAP; frame->tf_err = eva; /* smuggle fault address to ddb */ handled = kdb_trap(type, 0, frame); diff --git a/sys/kern/kern_shutdown.c b/sys/kern/kern_shutdown.c index c5a1780cb55..2ac99440e78 100644 --- a/sys/kern/kern_shutdown.c +++ b/sys/kern/kern_shutdown.c @@ -115,14 +115,19 @@ SYSCTL_INT(_kern, OID_AUTO, panic_reboot_wait_time, CTLFLAG_RWTUN, #ifdef KDB #ifdef KDB_UNATTENDED -int debugger_on_panic = 0; +static int debugger_on_panic = 0; #else -int debugger_on_panic = 1; +static int debugger_on_panic = 1; #endif SYSCTL_INT(_debug, OID_AUTO, debugger_on_panic, CTLFLAG_RWTUN | CTLFLAG_SECURE, &debugger_on_panic, 0, "Run debugger on kernel panic"); +int debugger_on_trap = 0; +SYSCTL_INT(_debug, OID_AUTO, debugger_on_trap, + CTLFLAG_RWTUN | CTLFLAG_SECURE, + &debugger_on_trap, 0, "Run debugger on kernel trap before panic"); + #ifdef KDB_TRACE static int trace_on_panic = 1; static bool trace_all_panics = true; diff --git a/sys/mips/mips/trap.c b/sys/mips/mips/trap.c index 746208e521f..042b6450b2e 100644 --- a/sys/mips/mips/trap.c +++ b/sys/mips/mips/trap.c @@ -1100,7 +1100,7 @@ err: #endif #ifdef KDB - if (debugger_on_panic) { + if (debugger_on_trap) { kdb_why = KDB_WHY_TRAP; kdb_trap(type, 0, trapframe); kdb_why = KDB_WHY_UNSET; diff --git a/sys/powerpc/powerpc/trap.c b/sys/powerpc/powerpc/trap.c index bf5b492ea29..815ea21d421 100644 --- a/sys/powerpc/powerpc/trap.c +++ b/sys/powerpc/powerpc/trap.c @@ -455,7 +455,7 @@ trap_fatal(struct trapframe *frame) printtrap(frame->exc, frame, 1, (frame->srr1 & PSL_PR)); #ifdef KDB - if (debugger_on_panic) { + if (debugger_on_trap) { kdb_why = KDB_WHY_TRAP; handled = kdb_trap(frame->exc, 0, frame); kdb_why = KDB_WHY_UNSET; diff --git a/sys/sys/kdb.h b/sys/sys/kdb.h index 8834b0fa817..75f8343b842 100644 --- a/sys/sys/kdb.h +++ b/sys/sys/kdb.h @@ -62,7 +62,7 @@ struct kdb_dbbe { DATA_SET(kdb_dbbe_set, name##_dbbe) extern u_char kdb_active; /* Non-zero while in debugger. */ -extern int debugger_on_panic; /* enter the debugger on panic. */ +extern int debugger_on_trap; /* enter the debugger on trap. */ extern struct kdb_dbbe *kdb_dbbe; /* Default debugger backend or NULL. */ extern struct trapframe *kdb_frame; /* Frame to kdb_trap(). */ extern struct pcb *kdb_thrctx; /* Current context. */