[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [XTF PATCH 03/16] vvmx: test whether MSR_IA32_VMX_BASIC is set correctly



On 12/16/16 17:19 +0000, Andrew Cooper wrote:
On 16/12/16 13:43, Haozhong Zhang wrote:
It tests whether bit 31 and bit 48 are 0, and VMCS size is in the
range (0, 4096].

Signed-off-by: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
---
 include/arch/x86/msr-index.h |  4 ++++
 tests/vvmx/msr.c             | 47 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/include/arch/x86/msr-index.h b/include/arch/x86/msr-index.h
index f9867d5..b7aeef0 100644
--- a/include/arch/x86/msr-index.h
+++ b/include/arch/x86/msr-index.h
@@ -16,6 +16,10 @@
 #define _MSR_MISC_FEATURES_CPUID_FAULTING        0
 #define MSR_MISC_FEATURES_CPUID_FAULTING         (1ULL << 
_MSR_MISC_FEATURES_CPUID_FAULTING)

+#define MSR_IA32_VMX_BASIC              0x00000480
+#define VMX_BASIC_VMCS_SIZE_MASK        (0x1fffULL << 32)
+#define VMX_BASIC_32BIT_ADDRESSES       (1ULL << 48)
+
 #define MSR_EFER                        0xc0000080 /* Extended Feature 
register. */
 #define _EFER_SCE                       0  /* SYSCALL Enable. */
 #define EFER_SCE                        (_AC(1, L) << _EFER_SCE)
diff --git a/tests/vvmx/msr.c b/tests/vvmx/msr.c
index 100491d..ad01f26 100644
--- a/tests/vvmx/msr.c
+++ b/tests/vvmx/msr.c
@@ -48,11 +48,58 @@ static bool test_msr_feature_control(void)
     return passed;
 }

+static bool test_msr_vmx_basic(void)
+{
+    bool passed = true;
+    uint64_t vmx_basic;
+    uint64_t vmcs_size;
+
+    if ( rdmsr_safe(MSR_IA32_VMX_BASIC, &vmx_basic) )
+    {
+        xtf_failure("Fail: fault when rdmsr MSR_IA32_VMX_BASIC\n");
+        passed = false;
+        goto out;
+    }
+
+    if ( vmx_basic & (1ULL << 31) )
+    {
+        xtf_failure("Fail: MSR_IA32_VMX_BASIC[31] is not 0\n");

Out of interest, what is the reason for requiring this bit to be 0?  I
can see that the manual insists that it is, but not why.


I'm not sure the exact reason, but guess it's because bit 31 of VMCS
revision id is reserved to indicate whether it's a shadow one.

+        passed = false;
+    }
+
+    vmcs_size = (vmx_basic & VMX_BASIC_VMCS_SIZE_MASK) >> 32;
+    if ( vmcs_size > PAGE_SIZE )
+    {
+        xtf_failure("Fail: "
+                    "VMCS size (%"PRIu64") in MSR_IA32_VMX_BASIC is > %ld\n",
+                    vmcs_size, PAGE_SIZE);
+        passed = false;
+    }
+    else if ( vmcs_size == 0 )
+    {
+        xtf_failure("Fail: VMCS size in MSR_IA32_VMX_BASIC cannot be 0\n");
+        passed = false;
+    }
+
+    /* test is running on hvm64, so this bit should be 0 */
+    if ( vmx_basic & VMX_BASIC_32BIT_ADDRESSES )

There is nothing in principle wrong with Xen setting this bit.  It would
be odd certainly, but not erroneous.


The reason I added this test is because Intel SDM Vol 3, Appendix A.1
Basic VMX Information says "This bit (bit 48) is always 0 for
processors that support Intel 64 architecture."

I don't know whether there is SW in real world that depends on the
consistency between this bit and Intel SDM, but if there is any, it
might be confused by an odd configuration. So I think it's still worth
to keep such test.

Thanks,
Haozhong

~Andrew

+    {
+        xtf_failure("Fail: MSR_IA32_VMX_BASIC[48] is not 0\n");
+        passed = false;
+    }
+
+out:
+    return passed;
+}
+
 bool test_msr_vmx(void)
 {
     if ( !test_msr_feature_control() )
         return false;

+    if ( !test_msr_vmx_basic() )
+        return false;
+
     return true;
 }



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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.