Re: [Xen-devel] [PATCH 6/7] x86emul: vendor specific SYSCALL behavior

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 24/03/2020 16:28, Jan Beulich wrote:
> AMD CPUs permit the insn everywhere (even outside of protected mode),
> while Intel ones restrict it to 64-bit mode. While at it also add the
> so far missing CPUID bit check.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -1870,6 +1870,7 @@ amd_like(const struct x86_emulate_ctxt *
>  #define vcpu_has_f16c()        (ctxt->cpuid->basic.f16c)
>  #define vcpu_has_rdrand()      (ctxt->cpuid->basic.rdrand)
>  
> +#define vcpu_has_syscall()     (ctxt->cpuid->extd.syscall)
>  #define vcpu_has_mmxext()      (ctxt->cpuid->extd.mmxext || vcpu_has_sse())
>  #define vcpu_has_3dnow_ext()   (ctxt->cpuid->extd._3dnowext)
>  #define vcpu_has_3dnow()       (ctxt->cpuid->extd._3dnow)
> @@ -5897,13 +5898,13 @@ x86_emulate(
>          break;
>  
>      case X86EMUL_OPC(0x0f, 0x05): /* syscall */
> -        generate_exception_if(!in_protmode(ctxt, ops), EXC_UD);
> -
> +        vcpu_must_have(syscall);
>          /* Inject #UD if syscall/sysret are disabled. */
>          fail_if(ops->read_msr == NULL);
>          if ( (rc = ops->read_msr(MSR_EFER, &msr_val, ctxt)) != X86EMUL_OKAY )
>              goto done;
>          generate_exception_if((msr_val & EFER_SCE) == 0, EXC_UD);

The CPUID check isn't actually missing, but it is fairly well hidden
here in the validity check to enable EFER.SCE in the first place.

In my (still incomplete and unposed) XSA-204 followup, I just commented
the fact here rather than introducing vcpu_must_have().

~Andrew

> +        generate_exception_if(!amd_like(ctxt) && !mode_64bit(), EXC_UD);
>  
>          if ( (rc = ops->read_msr(MSR_STAR, &msr_val, ctxt)) != X86EMUL_OKAY )
>              goto done;
>