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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.02.17 at 12:59, <andrew.cooper3@xxxxxxxxxx> wrote:
> 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.

Ah, right, that wouldn't be very nice indeed.

> 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.

Well, okay then.

Jan


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