Re: [PATCH v2 04/14] x86/traps: Implement #CP handler and extend #PF for shadow stacks

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.05.2020 21:18, Andrew Cooper wrote:
> For now, any #CP exception or shadow stack #PF indicate a bug in Xen, but
> attempt to recover from #CP if taken in guest context.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with one more question and a suggestion:

> @@ -1445,8 +1447,10 @@ void do_page_fault(struct cpu_user_regs *regs)
>       *
>       * Anything remaining is an error, constituting corruption of the
>       * pagetables and probably an L1TF vulnerable gadget.
> +     *
> +     * Any shadow stack access fault is a bug in Xen.
>       */
> -    if ( error_code & PFEC_reserved_bit )
> +    if ( error_code & (PFEC_reserved_bit | PFEC_shstk) )
>          goto fatal;

Wouldn't you saying "any" imply putting this new check higher up, in
particular ahead of the call to fixup_page_fault()?

> @@ -940,7 +944,8 @@ autogen_stubs: /* Automatically generated stubs. */
>          entrypoint 1b
>  
>          /* Reserved exceptions, heading towards do_reserved_trap(). */
> -        .elseif vec == TRAP_copro_seg || vec == TRAP_spurious_int || (vec > 
> TRAP_simd_error && vec < TRAP_nr)
> +        .elseif vec == X86_EXC_CSO || vec == X86_EXC_SPV || \
> +                vec == X86_EXC_VE  || (vec > X86_EXC_CP && vec < TRAP_nr)

Adding yet another || here adds to the fragility of the entire
construct. Wouldn't it be better to implement do_entry_VE at
this occasion, even its handling continues to end up in
do_reserved_trap()? This would have the benefit of avoiding the
pointless checking of %spl first thing in its handling. Feel
free to keep the R-b if you decide to go this route.

Jan