Re: [PATCH v4 3/3] xen: add support for automatic debug key actions in case of crash

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.12.2020 08:56, Juergen Gross wrote:
> @@ -519,6 +521,59 @@ 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);
> +#else
> +#define crash_debug_kexeccmd NULL
> +#endif
> +static char crash_debug_debugkey[CRASHACTION_SIZE];
> +string_runtime_param("crash-debug-debugkey", crash_debug_debugkey);
> +
> +void keyhandler_crash_action(enum crash_reason reason)
> +{
> +    static const char *const crash_action[] = {
> +        [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,
> +    };
> +    static bool ignore;
> +    const char *action;
> +
> +    /* Some handlers are not functional too early. */
> +    if ( system_state < SYS_STATE_smp_boot )
> +        return;
> +
> +    /* Avoid recursion. */
> +    if ( ignore )
> +        return;
> +    ignore = true;
> +
> +    if ( (unsigned int)reason >= ARRAY_SIZE(crash_action) )
> +        return;
> +    action = crash_action[reason];
> +    if ( !action )
> +        return;

If we consider either of the last two "return"s to possibly be
taken, I think the "ignore" logic wants to live below them, not
above, avoiding no output at all when a recursive invocation
turns out to be a "good" one. Then, as before,
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan