Re: [PATCH 3/3] x86/treewide: Drop the TRAP_* legacy names

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.02.2023 12:59, Andrew Cooper wrote:
> We have two naming schemes for exceptions - X86_EXC_?? which use the
> archtiectural abbreviations, and TRAP_* which is a mix of terminology and
> nonstandard abbrevations.  Switch to X86_EXC_* uniformly.
> 
> No funcational change, confirmed by diffing the disassembly.  Only 7 binary
> changes, and they're all __LINE__ being passed into printk().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -2745,9 +2745,9 @@ static int cf_check sh_page_fault(
>           * stream under Xen's feet.
>           */
>          if ( emul_ctxt.ctxt.event.type == X86_EVENTTYPE_HW_EXCEPTION &&
> -             ((emul_ctxt.ctxt.event.vector == TRAP_page_fault) ||
> -              (((emul_ctxt.ctxt.event.vector == TRAP_gp_fault) ||
> -                (emul_ctxt.ctxt.event.vector == TRAP_stack_error)) &&
> +             ((emul_ctxt.ctxt.event.vector == X86_EXC_PF) ||
> +              (((emul_ctxt.ctxt.event.vector == X86_EXC_GP) ||
> +                (emul_ctxt.ctxt.event.vector == X86_EXC_SS)) &&
>                 emul_ctxt.ctxt.event.error_code == 0)) )
>              hvm_inject_event(&emul_ctxt.ctxt.event);
>          else

Entirely unrelated to your change, but seeing that this piece of code is
being touched: Aren't we too lax here with #PF? Shouldn't we refuse
propagation also for e.g. reserved bit faults and insn fetch ones? Maybe
even for anything that isn't a supervisor write?

Jan