Re: [PATCH 5/5] x86/traps: Clean up diagnostics

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.11.2021 19:21, Andrew Cooper wrote:
> do{_reserved,}_trap() should use fatal_trap() rather than opencoding part of

Nit: That's do{_unhandled,}_trap() now.

> it.  This lets the remote stack trace logic work in more fatal error
> conditions.
> 
> With do_trap() converted, there is only one single user of trapstr()
> remaining.  Tweak the formatting in pv_inject_event(), and remove trapstr()
> entirely.
> 
> Take the opportunity of exporting vec_name() to improve the diagnostics in
> stub_selftest().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
albeit with one further aspect to consider:

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -759,21 +759,7 @@ static int nmi_show_execution_state(const struct 
> cpu_user_regs *regs, int cpu)
>      return 1;
>  }
>  
> -const char *trapstr(unsigned int trapnr)
> -{
> -    static const char * const strings[] = {
> -        "divide error", "debug", "nmi", "bkpt", "overflow", "bounds",
> -        "invalid opcode", "device not available", "double fault",
> -        "coprocessor segment", "invalid tss", "segment not found",
> -        "stack error", "general protection fault", "page fault",
> -        "spurious interrupt", "coprocessor error", "alignment check",
> -        "machine check", "simd error", "virtualisation exception"
> -    };
> -
> -    return trapnr < ARRAY_SIZE(strings) ? strings[trapnr] : "???";
> -}
> -
> -static const char *vec_name(unsigned int vec)
> +const char *vec_name(unsigned int vec)

Is this perhaps too ambiguous a name for a non-static function? exn_vec_name()
at least, maybe?

Jan