Re: [Xen-devel] [PATCH 2/3] Nested VMX: Clear bit 31 of IA32_VMX_BASIC MSR

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Andrew Cooper wrote on 2013-09-05:
> On 05/09/13 03:57, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>> 
>> The bit 31 of revision_id will set to 1 if vmcs shadowing enabled.
>> And according intel SDM, the bit 31 of IA32_VMX_BASIC MSR is always
>> 0. So we cannot set low 32 bit of IA32_VMX_BASIC to revision_id directly.
>> Must clear the bit 31 to 0.
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>> ---
>>  xen/arch/x86/hvm/vmx/vvmx.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c
>> b/xen/arch/x86/hvm/vmx/vvmx.c index 2e0b7f7..8571002 100644
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -1846,7 +1846,7 @@ int nvmx_msr_read_intercept(unsigned int msr,
> u64 *msr_content)
>>      switch (msr) {
>>      case MSR_IA32_VMX_BASIC:
>>          data = (host_data & (~0ul << 32)) |
>> -               ((v->arch.hvm_vmx.vmcs)->vmcs_revision_id);
>> +               ((v->arch.hvm_vmx.vmcs)->vmcs_revision_id & ~(1ul <<
>> + 31));
> 
> What are the chances of vmcs_revision_id extending beyond 32 bits?
> 
> The SDM states that the bottom 31 bits of IA32_VMX_BASIC shall be the
> bottom 31 bits of the revision id, so
> 
> (v->arch.hvm_vmx.vmcs->vmcs_revision_id & 0x7fffffff);
> 
> would seem more obvious.  Also, the brackets were superfluous.

Right!

> 
> ~Andrew
> 
>>          break;
>>      case MSR_IA32_VMX_PINBASED_CTLS:
>>      case MSR_IA32_VMX_TRUE_PINBASED_CTLS:


Best regards,
Yang



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