On 26.11.2020 09:03, Juergen Gross wrote:
> When the host crashes it would sometimes be nice to have additional
> debug data available which could be produced via debug keys, but
> halting the server for manual intervention might be impossible due to
> the need to reboot/kexec rather sooner than later.
>
> Add support for automatic debug key actions in case of crashes which
> can be activated via boot- or runtime-parameter.
>
> Depending on the type of crash the desired data might be different, so
> support different settings for the possible types of crashes.
>
> The parameter is "crash-debug" with the following syntax:
>
> crash-debug-<type>=<string>
>
> with <type> being one of:
>
> panic, hwdom, watchdog, kexeccmd, debugkey
>
> and <string> a sequence of debug key characters with '+' having the
> special semantics of a 10 millisecond pause.
>
> So "crash-debug-watchdog=0+0qr" would result in special output in case
> of watchdog triggered crash (dom0 state, 10 ms pause, dom0 state,
> domain info, run queues).
>
> Signed-off-by: Juergen Gross <jgr...@suse.com>
> ---
> V2:
> - switched special character '.' to '+' (Jan Beulich)
> - 10 ms instead of 1 s pause (Jan Beulich)
> - added more text to the boot parameter description (Jan Beulich)
>
> V3:
> - added const (Jan Beulich)
> - thorough test of crash reason parameter (Jan Beulich)
> - kexeccmd case should depend on CONFIG_KEXEC (Jan Beulich)
> - added dummy get_irq_regs() helper on Arm
>
> Signed-off-by: Juergen Gross <jgr...@suse.com>
Except for the Arm aspect, where I'm not sure using
guest_cpu_user_regs() is correct in all cases,
Reviewed-by: Jan Beulich <jbeul...@suse.com>
Nevertheless ...
> @@ -507,6 +509,50 @@ void __init initialize_keytable(void)
> }
> }
>
> +#define CRASHACTION_SIZE 32
> +static char crash_debug_panic[CRASHACTION_SIZE];
> +string_runtime_param("crash-debug-panic", crash_debug_panic);
> +static char crash_debug_hwdom[CRASHACTION_SIZE];
> +string_runtime_param("crash-debug-hwdom", crash_debug_hwdom);
> +static char crash_debug_watchdog[CRASHACTION_SIZE];
> +string_runtime_param("crash-debug-watchdog", crash_debug_watchdog);
> +#ifdef CONFIG_KEXEC
> +static char crash_debug_kexeccmd[CRASHACTION_SIZE];
> +string_runtime_param("crash-debug-kexeccmd", crash_debug_kexeccmd);
> +#endif
... to limit #ifdef-ary I'd have suggested to put
#else
# define crash_debug_kexeccmd NULL
right above here and ...
> +void keyhandler_crash_action(enum crash_reason reason)
> +{
> + static const char *const crash_action[CRASHREASON_N] = {
> + [CRASHREASON_PANIC] = crash_debug_panic,
> + [CRASHREASON_HWDOM] = crash_debug_hwdom,
> + [CRASHREASON_WATCHDOG] = crash_debug_watchdog,
> +#ifdef CONFIG_KEXEC
> + [CRASHREASON_KEXECCMD] = crash_debug_kexeccmd,
> +#endif
> + [CRASHREASON_DEBUGKEY] = crash_debug_debugkey,
> + };
> + const char *action;
> + struct cpu_user_regs *regs = get_irq_regs() ? : guest_cpu_user_regs();
> +
> + if ( (unsigned int)reason >= CRASHREASON_N )
... I'd have preferred ARRAY_SIZE() here, at which point the
array dimension also wouldn't need explicitly specifying.
Jan
