Re: [PATCH 3/4] VMX: use proper instruction mnemonics if assembler supports them

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

On 26/08/2013 10:06, Jan Beulich wrote:
>>>> On 25.08.13 at 00:18, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> I certainly like and encourage of the idea of getting rid of the fixed
>> registers due to the hex literals.  However, are macros really the
>> correct solution?
>>
>> __vmread_safe(field, value) now has non-function like semantics for value.
>>
>> To me, the more obvious (and as far as I can tell, functionally
>> identical) approach would be for
>>
>> unsigned long always_inline __vmread_safe(unsigned long field, unsigned
>> long *value)
>>
>> The compiler, given the correct constraints for value could resolve the
>> pointer back to the caller as it inlined the function, and calls would
>> look like "__vmread_safe(field, &value);" which looks rather more natural.
> The constraints aren't the problem - the use of sizeof(value) inside
> the macro is. You can't pass both pointers to 32-bit and 64-bit data
> items into the function, and then have the function guess what the
> type is to be.

Ok (wrt the explanation), but I am not sure the result is correct.

VMREAD has a single encoding, 0f 78, and the size of the reg/mem operand
depends on long mode or not (where compat mode forces a #UD).

>From SDM Vol 3 30.3 "VMREAD":

"If the VMCS field specified by the source operand is shorter than this
effective operand size, the high bits of the destination operand are
cleared to 0. If the VMCS field is longer, then the high bits of the
field are not read."

So passing a pointer to a 32bit value into this function/macro in long
mode will result in the adjacent 32 bits being clobbered, which should
best be avoided.

~Andrew

>
>> As for the memory clobbers, I see no indication from the VMX instruction
>> reference that vmread or inv{ept,vpid} have relevant side effects
>> requiring a clobber.  For the load functions and vmwrite, there is a
>> possibilty of a memory clobber if someone has peaked into the relevant
>> VMCS page(s).  However, doing so defeats the purpose of vmread/write, so
>> as far as I can tell, we could argue away the clobbers on those grounds
>> alone.
> Good; I'm still hoping to hear an opinion from Intel engineers too.
>
> Jan
>