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

[Jürgen Groß <jgross@xxxxxxxx>]

On 14.12.20 10:16, Jan Beulich wrote:
> 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>
Fine with me.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys

Attachment: OpenPGP_signature
Description: OpenPGP digital signature