Re: [PATCH v1 5/6] x86/vvmx: correctly report vvmcs size

[Sergey Dyasli <sergey.dyasli@citrix.com>]

On 01/11/2018 02:29, Tian, Kevin wrote:
>> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
>> Sent: Tuesday, October 30, 2018 8:36 PM
>>
>> On 30/10/2018 08:06, Tian, Kevin wrote:
>>>> From: Sergey Dyasli [mailto:sergey.dyasli@citrix.com]
>>>> Sent: Friday, October 12, 2018 11:28 PM
>>>>
>>>> The size of Xen's virtual vmcs region is 4096 bytes. Correctly report
>>>> it to the guest in case when VMCS shadowing is not available instead of
>>>> providing H/W value (which is usually smaller).
>>>
>>> what is the problem of reporting smaller size even when actual
>>> size is 4096? is L1 expected to access the portion beyond h/w
>>> reported size?
>>>
>>
>> Here's the code snippet from kvm-unit-tests:
>>
>> 	vmcs[0]->hdr.revision_id = basic.revision;
>> 	assert(!vmcs_clear(vmcs[0]));
>> 	assert(!make_vmcs_current(vmcs[0]));
>> 	set_all_vmcs_fields(0x86);
>>
>> 	assert(!vmcs_clear(vmcs[0]));
>> 	memcpy(vmcs[1], vmcs[0], basic.size);
>> 	assert(!make_vmcs_current(vmcs[1]));
>> 	report("test vmclear flush (current VMCS)",
>> check_all_vmcs_fields(0x86));
>>
>> set_all_vmcs_fields() vmwrites almost 4k, but memcpy() copies only 1024
>> bytes and vmreads get incorrect values.
>>
> 
> I didn't understand why set_all_vmcs_fields blindly touch 4k instead of
> following reported size. Also I didn't get the reason of this patch - whatever
> size reported, xen just needs to emulate hw behavior according to spec,
> i.e. do proper emulation if offset < size, otherwise just vmfail. Guest is
> not aware of shadow vmcs. why do we want to report different vmcs
> size based on presence of shadow vmcs?

Here's the detailed explanation (for when vmcs shadowing is not
available in H/W):

1. Guest reads vmcs region size as 1024 (from H/W), allocates it and
does vmptrld

2. Xen maps provided guest's memory and uses it as virtual vmcs (vmcs12)

3. Guest uses vmwrites to set up the vmcs

4. During emulation (set_vvmcs_virtual()), Xen writes values into
virtual vmcs but the resulting offset can be larger than 1024 (when
multiplied by sizeof(u64)).

There is even a comment in include/asm-x86/hvm/vmx/vvmx.h:

/*
 * Virtual VMCS layout
 *
 * Since physical VMCS layout is unknown, a custom layout is used
 * for virtual VMCS seen by guest. It occupies a 4k page, and the
 * field is offset by an 9-bit offset into u64[], The offset is as
 * follow, which means every <width, type> pair has a max of 32
 * fields available.
 *
 *             9       7      5               0
 *             --------------------------------
 *     offset: | width | type |     index     |
 *             --------------------------------
 *
 * Also, since the lower range <width=0, type={0,1}> has only one
 * field: VPID, it is moved to a higher offset (63), and leaves the
 * lower range to non-indexed field like VMCS revision.
 *
 */

--
Thanks,
Sergey

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