Re: [PATCH] x86/boot: Fix early exception handling with CONFIG_PERF_COUNTERS

[Julien Grall <julien@xxxxxxx>]

Hi Andrew,

On 15/04/2020 18:39, Andrew Cooper wrote:
> The PERFC_INCR() macro uses current->processor, but current is not valid
> during early boot.  This causes the following crash to occur if
> e.g. rdmsr_safe() has to recover from a #GP fault.
>
>    (XEN) Early fatal page fault at e008:ffff82d0803b1a39 (cr2=0000000000000004, 
> ec=0000)
>    (XEN) ----[ Xen-4.14-unstable  x86_64  debug=y   Not tainted ]----
>    (XEN) CPU:    0
>    (XEN) RIP:    e008:[<ffff82d0803b1a39>] 
> x86_64/entry.S#handle_exception_saved+0x64/0xb8
>    ...
>    (XEN) Xen call trace:
>    (XEN)    [<ffff82d0803b1a39>] R 
> x86_64/entry.S#handle_exception_saved+0x64/0xb8
>    (XEN)    [<ffff82d0806394fe>] F __start_xen+0x2cd/0x2980
>    (XEN)    [<ffff82d0802000ec>] F __high_start+0x4c/0x4e
>
> Furthermore, the PERFC_INCR() macro is wildly inefficient.  There has been a
> single caller for many releases now, so inline it and delete the macro
> completely.
>
> For the assembly, move entry_vector from %eax into %ecx.  There is no encoding
> benefit for movzbl, and it frees up %eax to be used inside the
> CONFIG_PERF_COUNTERS block where there is an encoding benefit.
>
> There is no need to reference current at all.  What is actually needed is the
> per_cpu_offset which can be obtained directly from the top-of-stack block.
> This simplifies the counter handling to 3 instructions and no spilling to the
> stack at all.
>
> The same breakage from above is now handled properly:
>
>    (XEN) traps.c:1591: GPF (0000): ffff82d0806394fe [__start_xen+0x2cd/0x2980] 
> -> ffff82d0803b3bfb
>
> Reported-by: Julien Grall <jgrall@xxxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
I can confirm the crash I have seen has now disappeared.

Tested-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall