Re: [Xen-devel] [PATCH v2 1/4] x86/vmx: introduce vmwrite_safe()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/02/17 11:09, Jan Beulich wrote:
>
>> @@ -423,6 +429,29 @@ static inline bool_t __vmread_safe(unsigned long field, 
>> unsigned long *value)
>>      return okay;
>>  }
>>  
>> +static always_inline unsigned long vmwrite_safe(unsigned long field,
>> +                                                unsigned long value)
>> +{
>> +    unsigned long ret = 0;
>> +    bool fail_invalid, fail_valid;
>> +
>> +    asm volatile ( GAS_VMX_OP("vmwrite %[value], %[field]\n\t",
>> +                              VMWRITE_OPCODE MODRM_EAX_ECX)
>> +                   ASM_FLAG_OUT(, "setc %[invalid]\n\t")
>> +                   ASM_FLAG_OUT(, "setz %[valid]\n\t")
>> +                   : ASM_FLAG_OUT("=@ccc", [invalid] "=rm") (fail_invalid),
>> +                     ASM_FLAG_OUT("=@ccz", [valid] "=rm") (fail_valid)
>> +                   : [field] GAS_VMX_OP("r", "a") (field),
>> +                     [value] GAS_VMX_OP("rm", "c") (value));
>> +
>> +    if ( unlikely(fail_invalid) )
>> +        ret = VMX_INSN_FAIL_INVALID;
>> +    else if ( unlikely(fail_valid) )
>> +        __vmread(VM_INSTRUCTION_ERROR, &ret);
>> +
>> +    return ret;
>> +}
> ... allow the function to return enum vmx_insn_errno, and that
> to not be a 64-bit quantity. As you're presumably aware, dealing
> with 32-bit quantities is on the average slightly more efficient than
> dealing with 64-bit ones. The code above should imo still BUG() if
> the value read from VM_INSTRUCTION_ERROR doesn't fit in 32
> bits (as it's a 32-bit field only anyway).
>
> Also, looking at the entire asm() above another time, wouldn't it
> end up a bit less convoluted if you simply used CMOVC for the
> "invalid" code path? Similarly I wonder whether the second
> VMREAD wouldn't better be moved into the asm(), utilizing the
> UNLIKELY_START() et al constructs to get that code path
> entirely out of the main path. These aren't requirements though,
> just aspects to think about.

Embedding two vm*** instruction is substantially harder in the non
GAS_VMX_OP() case.  It either involves manual register scheduling, or a
separate ModRM and different explicit register fields.

As for extra logic, I have some further plans which would allow the
compiler to elide the __vmread() on some paths, which it can only for
logic exposed in C.  From this point of view, the less code in the asm
block, the better.

~Andrew

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