ddb: have 'reset' command use normal reboot path

[mhorne]

Summary

This conditionally gives all registered shutdown handlers a chance to
perform the reboot, with cpu_reset() being the fallback. The
debug.ddb.full_reboot sysctl/tunable restores the previous behaviour.

The motivation is that some platforms may not be able do anything
meaningful via cpu_reset(), due to a lack of standardized reset
mechanism and/or firmware shortcomings. However, they may have a
separate device driver attached that normally performs the reboot. Such
is the case for some versions of the Raspberry Pi.

Test Plan

We get some new console output with this change:

debug.kdb.enter: 0KDB: enter: sysctl debug.kdb.enter
[ thread pid 812 tid 100118 ]
Stopped at      kdb_trap+0x448: sd      zero,0(s1)
db> reset
Waiting (max 60 seconds) for system process `vnlru' to stop... done
Uptime: 1m51s

[mhorne]

Note that kern_reboot() is called by vpanic(), so it is safe for this context. I'm providing the tunable out of caution, in case there is some unforeseen side effect. If the change seems agreeable I'll document the tunable in ddb(4) as well.

[imp]

Looks good to me. RB_NOSYNC is critical here...

[bz]

Can we grab a mtx, call eventhandlers, ... etc. when called from ddb (e.g. after a panic)? What about early boot panics - is this all setup then? Given kern_reboot will never return, we'll also never have the chance to try cpu_reset(). Can we, instead of making this a sysctl have it as a /f argument to db_reset? That way it'll be available if cpu_reset fails and one can just call reset /f and give it a try... but in either way, I'd rather not change the default until I am convinced that it won't make it worse in a lot of panic situations...

[mhorne]

In D37981#862886, @bz wrote:

Can we grab a mtx, call eventhandlers, ... etc. when called from ddb (e.g. after a panic)? What about early boot panics - is this all setup then? Given kern_reboot will never return, we'll also never have the chance to try cpu_reset(). Can we, instead of making this a sysctl have it as a /f argument to db_reset? That way it'll be available if cpu_reset fails and one can just call reset /f and give it a try... but in either way, I'd rather not change the default until I am convinced that it won't make it worse in a lot of panic situations...

Short answer: yes, this function is totally safe for this context. It is already in use today at the very end of vpanic(). Lock acquisitions and attempts to sleep will check if we are in a panic/debugger context and return early if that's the case.

cpu_reset() will still be executed by the kern_reboot() path. It is called by the very last shutdown handler, shutdown_reset().

For early-boot, we do have a small problem. Before any shutdown handlers are registered during SI_SUB_INTRINSIC, calling kern_reboot() will just end up spinning in the infinite loop. This can be observed today if you type continue instead of reset at the prompt after an early panic. To handle this edge-case I think it would be best for kern_reboot() to call shutdown_reset() directly rather than relying on the event handler machinery. That way, the most basic reset is always attempted.

I agree that a command modifier is much more natural than the tunable here, but I still propose the (unifying) change to the default behaviour of reset. I will update the review shortly.

[mhorne]

Switch from a tunable to a ddb command modifier 's'.

Ensure we always call shutdown_reset(), even before event handlers are registered. This bit can be committed separately but I'm including it in this review.

[markj]

sys/ddb/db_command.c

781

I'd elaborate a bit more on why this might be important, either here or in the man page.