[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2] xen: add support for automatic debug key actions in case of crash
On 24.11.20 12:27, Jan Beulich wrote: On 20.11.2020 14:13, Juergen Gross wrote:@@ -507,6 +509,42 @@ void __init initialize_keytable(void) } }+#define CRASHACTION_SIZE 32+static char crash_debug_panic[CRASHACTION_SIZE]; +static char crash_debug_hwdom[CRASHACTION_SIZE]; +static char crash_debug_watchdog[CRASHACTION_SIZE]; +static char crash_debug_kexeccmd[CRASHACTION_SIZE]; +static char crash_debug_debugkey[CRASHACTION_SIZE]; + +static char *crash_action[CRASHREASON_N] = {Considering the sole use below, I think there can be two "const" added here. With this single use I also wonder whether this array wouldn't better be private to that function. Both fine with me. + [CRASHREASON_PANIC] = crash_debug_panic, + [CRASHREASON_HWDOM] = crash_debug_hwdom, + [CRASHREASON_WATCHDOG] = crash_debug_watchdog, + [CRASHREASON_KEXECCMD] = crash_debug_kexeccmd, + [CRASHREASON_DEBUGKEY] = crash_debug_debugkey, +}; + +string_runtime_param("crash-debug-panic", crash_debug_panic); +string_runtime_param("crash-debug-hwdom", crash_debug_hwdom); +string_runtime_param("crash-debug-watchdog", crash_debug_watchdog); +string_runtime_param("crash-debug-kexeccmd", crash_debug_kexeccmd);This one probably wants a CONFIG_KEXEC conditional around it, such that requests to set it won't appear to be "okay" on !KEXEC builds. At which point the doc probably also wants to mention the conditional availability of this option. Yes. +string_runtime_param("crash-debug-debugkey", crash_debug_debugkey); + +void keyhandler_crash_action(enum crash_reason reason) +{ + const char *action = crash_action[reason];In order to avoid cascade problems when the system's already in trouble, maybe better to bounds check "reason" before using as array index and, also with the CONFIG_KEXEC related adjustment requested above in mind, ...+ struct cpu_user_regs *regs = get_irq_regs() ? : guest_cpu_user_regs(); + + while ( *action )... perhaps also better to check action against NULL before de-referencing? Okay (to both). And I only now realized that get_irq_regs() is x86 only. I'll add a dummy Arm function always returning NULL. Juergen Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc Attachment:
OpenPGP_signature
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |