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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

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

~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