Re: [PATCH 3/8] x86emul: support BMI1 insns

["Jan Beulich" <JBeulich@suse.com>]

>>> On 13.01.17 at 18:40, <andrew.cooper3@citrix.com> wrote:
> On 13/01/17 15:31, Jan Beulich wrote:
>> @@ -5866,6 +5879,67 @@ x86_emulate(
>>          break;
>>  #endif
>>  
>> +    case X86EMUL_OPC_VEX(0x0f38, 0xf2):    /* andn r/m,r,r */
>> +    case X86EMUL_OPC_VEX(0x0f38, 0xf7):    /* bextr 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);
>> +        generate_exception_if(vex.l, EXC_UD);
> 
> The manual also states #UD if VEX.W is set.

This is very clearly a doc error: For one, is doesn't _also_ state this,
but says nothing about VEX.L. And the instruction encodings list .W1
variants (as expected) to encode 64-bit operations.

>> +
>> +        buf[0] = 0xc4;
>> +        *pvex = vex;
>> +        pvex->b = 1;
>> +        pvex->r = 1;
>> +        pvex->reg = ~0; /* rAX */
>> +        buf[3] = b;
>> +        buf[4] = 0x09; /* reg=rCX r/m=(%rCX) */
>> +        buf[5] = 0xc3;
>> +
>> +        src.reg = decode_register(~vex.reg & (mode_64bit() ? 0xf : 7),
>> +                                  &_regs, 0);
> 
> Given this construct, and several GPR-encoded vex instructions, how
> about a decode_vex_gpr() wrapper?

That's a good idea.

>> --- a/xen/include/asm-x86/cpufeature.h
>> +++ b/xen/include/asm-x86/cpufeature.h
>> @@ -57,6 +57,7 @@
>>  #define cpu_has_xsave           boot_cpu_has(X86_FEATURE_XSAVE)
>>  #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_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)
> 
> After trying this out, we clearly need to alter the position on VEX
> prefixes.  VEX encoded GPR instructions don't fall within the previous
> assumptions made about the dependences of VEX instructions.

Should I fold this in, or do you want to submit it as a separate
patch?

Jan

> --- a/xen/tools/gen-cpuid.py
> +++ b/xen/tools/gen-cpuid.py
> @@ -234,9 +234,11 @@ def crunch_numbers(state):
>          XSAVE: [XSAVEOPT, XSAVEC, XGETBV1, XSAVES,
>                  AVX, MPX, PKU, LWP],
>  
> -        # AVX is taken to mean hardware support for VEX encoded
> instructions,
> -        # 256bit registers, and the instructions themselves.  Each of these
> -        # subsequent instruction groups may only be VEX encoded.
> +        # AVX is taken to mean hardware support for 256bit registers,
> and the
> +        # instructions themselves.  It does not related to the VEX
> prefix (In
> +        # particular, most BMI{1,2} instructions may only be VEX
> encoded but
> +        # operate on GPRs rather than YMM register and can be used without
> +        # enabling xstate).
>          AVX: [FMA, FMA4, F16C, AVX2, XOP],
>  
>          # CX16 is only encodable in Long Mode.  LAHF_LM indicates that the




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