Re: [PATCH v2 02/14] x86/traps: Factor out extable_fixup() and make printing consistent

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.05.2020 21:18, Andrew Cooper wrote:
> UD faults never had any diagnostics printed, and the others were inconsistent.
> 
> Don't use dprintk() because identifying traps.c is actively unhelpful in the
> message, as it is the location of the fixup, not the fault.  Use the new
> vec_name() infrastructure, rather than leaving raw numbers for the log.
> 
>   (XEN) Running stub recovery selftests...
>   (XEN) Fixup #UD[0000]: ffff82d07fffd040 [ffff82d07fffd040] -> 
> ffff82d0403ac9d6
>   (XEN) Fixup #GP[0000]: ffff82d07fffd041 [ffff82d07fffd041] -> 
> ffff82d0403ac9d6
>   (XEN) Fixup #SS[0000]: ffff82d07fffd040 [ffff82d07fffd040] -> 
> ffff82d0403ac9d6
>   (XEN) Fixup #BP[0000]: ffff82d07fffd041 [ffff82d07fffd041] -> 
> ffff82d0403ac9d6
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

As before
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
albeit I realize I have one more suggestion:

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -772,10 +772,31 @@ static void do_reserved_trap(struct cpu_user_regs *regs)
>            trapnr, vec_name(trapnr), regs->error_code);
>  }
>  
> +static bool extable_fixup(struct cpu_user_regs *regs, bool print)
> +{
> +    unsigned long fixup = search_exception_table(regs);
> +
> +    if ( unlikely(fixup == 0) )
> +        return false;
> +
> +    /*
> +     * Don't use dprintk() because the __FILE__ reference is unhelpful.
> +     * Can currently be triggered by guests.  Make sure we ratelimit.
> +     */
> +    if ( IS_ENABLED(CONFIG_DEBUG) && print )

How about pulling the IS_ENABLED(CONFIG_DEBUG) into the call sites
currently passing "true"?

Jan