Re: [PATCH 06/16] x86/traps: Implement #CP handler and extend #PF for shadow stacks

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.05.2020 00:58, Andrew Cooper wrote:
> @@ -1457,6 +1451,10 @@ void do_page_fault(struct cpu_user_regs *regs)
>      {
>          enum pf_type pf_type = spurious_page_fault(addr, regs);
>  
> +        /* Any fault on a shadow stack access is a bug in Xen. */
> +        if ( error_code & PFEC_shstk )
> +            goto fatal;

Not going through the full spurious_page_fault() in this case
would seem desirable, as would be at least a respective
adjustment to __page_fault_type(). Perhaps such an adjustment
could then avoid the change (and the need for goto) here?

> @@ -1906,6 +1905,43 @@ void do_debug(struct cpu_user_regs *regs)
>      pv_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
>  }
>  
> +void do_entry_CP(struct cpu_user_regs *regs)

If there's a plan to change other exception handlers to this naming
scheme too, I can live with the intermediate inconsistency.
Otherwise, though, I'd prefer a name closer to what other handlers
use, e.g. do_control_prot(). Same for the asm entry point then.

> @@ -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 == TRAP_copro_seg || vec == TRAP_spurious_int || \
> +                vec == TRAP_virtualisation || (vec > X86_EXC_CP && vec < 
> TRAP_nr)

Use the shorter X86_EXC_VE here, since you don't want/need to
retain what was there before? (I'd be fine with you changing
the two other TRAP_* too at this occasion.)

> --- a/xen/include/asm-x86/processor.h
> +++ b/xen/include/asm-x86/processor.h
> @@ -68,6 +68,7 @@
>  #define PFEC_reserved_bit   (_AC(1,U) << 3)
>  #define PFEC_insn_fetch     (_AC(1,U) << 4)
>  #define PFEC_prot_key       (_AC(1,U) << 5)
> +#define PFEC_shstk         (_AC(1,U) << 6)

One too few padding blanks?

Jan