Re: [Xen-devel] [PATCH 4/8] x86emul: support BMI2 insns

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/01/17 15:32, Jan Beulich wrote:
> Note that the adjustment to the mode_64bit() definition is so that we
> can avoid "#ifdef __x86_64__" around the 64-bit asm() portions. An
> alternative would be single asm()s with a conditional branch over the
> (manually encoded) REX64 prefix.

This presumably relying on sensible dead-code-elimitation to compile? 
Does this offer any further opportunities with removing other ifdefs?

(Either way, this seems cleaner than embedding a jmp in asm).

> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -707,7 +707,11 @@ do{ asm volatile (
>  })
>  #define truncate_ea(ea) truncate_word((ea), ad_bytes)
>  
> -#define mode_64bit() (ctxt->addr_size == 64)
> +#ifdef __x86_64__
> +# define mode_64bit() (ctxt->addr_size == 64)
> +#else
> +# define mode_64bit() false
> +#endif
>  
>  #define fail_if(p)                                      \
>  do {                                                    \
> @@ -1353,6 +1357,7 @@ static bool vcpu_has(
>  #define vcpu_has_misalignsse() vcpu_has(0x80000001, ECX,  7, ctxt, ops)
>  #define vcpu_has_bmi1()        vcpu_has(         7, EBX,  3, ctxt, ops)
>  #define vcpu_has_hle()         vcpu_has(         7, EBX,  4, ctxt, ops)
> +#define vcpu_has_bmi2()        vcpu_has(         7, EBX,  8, ctxt, ops)
>  #define vcpu_has_rtm()         vcpu_has(         7, EBX, 11, ctxt, ops)
>  #define vcpu_has_mpx()         vcpu_has(         7, EBX, 14, ctxt, ops)
>  #define vcpu_has_adx()         vcpu_has(         7, EBX, 19, ctxt, ops)
> @@ -5880,12 +5885,21 @@ x86_emulate(
>  #endif
>  
>      case X86EMUL_OPC_VEX(0x0f38, 0xf2):    /* andn r/m,r,r */
> +    case X86EMUL_OPC_VEX(0x0f38, 0xf5):    /* bzhi r,r/m,r */
> +    case X86EMUL_OPC_VEX_F3(0x0f38, 0xf5): /* pext r/m,r,r */
> +    case X86EMUL_OPC_VEX_F2(0x0f38, 0xf5): /* pdep r/m,r,r */
>      case X86EMUL_OPC_VEX(0x0f38, 0xf7):    /* bextr r,r/m,r */
> +    case X86EMUL_OPC_VEX_66(0x0f38, 0xf7): /* shlx r,r/m,r */
> +    case X86EMUL_OPC_VEX_F3(0x0f38, 0xf7): /* sarx r,r/m,r */
> +    case X86EMUL_OPC_VEX_F2(0x0f38, 0xf7): /* shrx r,r/m,r */
>      {
>          uint8_t *buf = get_stub(stub);
>          typeof(vex) *pvex = container_of(buf + 1, typeof(vex), raw[0]);
>  
> -        host_and_vcpu_must_have(bmi1);
> +        if ( b == 0xf5 || vex.pfx )
> +            host_and_vcpu_must_have(bmi2);
> +        else
> +            host_and_vcpu_must_have(bmi1);
>          generate_exception_if(vex.l, EXC_UD);
>  
>          buf[0] = 0xc4;
> @@ -5973,6 +5987,33 @@ x86_emulate(
>          break;
>      }
>  
> +    case X86EMUL_OPC_VEX_F2(0x0f38, 0xf6): /* mulx r/m,r,r */
> +        vcpu_must_have(bmi2);
> +        generate_exception_if(vex.l, EXC_UD);

vex.w again.

> +        ea.reg = decode_register(~vex.reg & (mode_64bit() ? 0xf : 7),
> +                                 &_regs, 0);
> +        if ( mode_64bit() && vex.w )
> +            asm ( "mulq %3" : "=a" (*ea.reg), "=d" (dst.val)
> +                            : "0" (src.val), "rm" (_regs.r(dx)) );
> +        else
> +            asm ( "mull %3" : "=a" (*ea.reg), "=d" (dst.val)
> +                            : "0" ((uint32_t)src.val), "rm" (_regs._edx) );
> +        break;
> +
> +    case X86EMUL_OPC_VEX_F2(0x0f3a, 0xf0): /* rorx imm,r/m,r */
> +        vcpu_must_have(bmi2);
> +        generate_exception_if(vex.l || vex.reg != 0xf, EXC_UD);

What does this vex.reg check correspond to?  I can't locate anything
relevant in the manuals.

~Andrew

> +        if ( ea.type == OP_REG )
> +            src.val = *ea.reg;
> +        else if ( (rc = read_ulong(ea.mem.seg, ea.mem.off, &src.val, 
> op_bytes,
> +                                   ctxt, ops)) != X86EMUL_OKAY )
> +            goto done;
> +        if ( mode_64bit() && vex.w )
> +            asm ( "rorq %b1,%0" : "=g" (dst.val) : "c" (imm1), "0" (src.val) 
> );
> +        else
> +            asm ( "rorl %b1,%k0" : "=g" (dst.val) : "c" (imm1), "0" 
> (src.val) );
> +        break;
> +
>      default:
>          goto cannot_emulate;
>      }
> --- a/xen/include/asm-x86/cpufeature.h
> +++ b/xen/include/asm-x86/cpufeature.h
> @@ -58,6 +58,7 @@
>  #define cpu_has_avx             boot_cpu_has(X86_FEATURE_AVX)
>  #define cpu_has_lwp             boot_cpu_has(X86_FEATURE_LWP)
>  #define cpu_has_bmi1            boot_cpu_has(X86_FEATURE_BMI1)
> +#define cpu_has_bmi2            boot_cpu_has(X86_FEATURE_BMI2)
>  #define cpu_has_mpx             boot_cpu_has(X86_FEATURE_MPX)
>  #define cpu_has_arch_perfmon    boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
>  #define cpu_has_rdtscp          boot_cpu_has(X86_FEATURE_RDTSCP)
>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel