Message ID | 20161216134348.16236-7-haozhong.zhang@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 16/12/16 13:43, Haozhong Zhang wrote: > diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c > index 74b4d01..f30fc26 100644 > --- a/tests/vvmx/util.c > +++ b/tests/vvmx/util.c > @@ -1,7 +1,31 @@ > #include <xtf.h> > +#include <arch/x86/msr-index.h> > #include <arch/x86/hvm/vmx/vmcs.h> > #include "util.h" > > +#define INVALID_VMCS_REVID (~(uint32_t)0) > + > +static uint32_t vmcs_revid = INVALID_VMCS_REVID; > + > +uint32_t get_vmcs_revid(void) > +{ > + if ( vmcs_revid != INVALID_VMCS_REVID ) > + goto out; > + > + uint64_t vmx_basic = rdmsr(MSR_IA32_VMX_BASIC); > + > + vmcs_revid = (uint32_t)vmx_basic & VMX_BASIC_REVISION_MASK; > + > +out: > + return vmcs_revid; > +} > + > +void clear_vmcs(void *vmcs, uint32_t revid) Strictly speaking, this should be setup_vmcs() as it does more than just clearing it. > +{ > + memset(vmcs, 0, PAGE_SIZE); > + *((uint32_t *)vmcs) = revid; > +} > + > #define vvmx_failure(prefix, fmt, ...) \ > do { \ > xtf_failure("Fail: %s: "fmt, prefix, ##__VA_ARGS__); \ > diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c > index e69de29..31f074c 100644 > --- a/tests/vvmx/vmxon.c > +++ b/tests/vvmx/vmxon.c > @@ -0,0 +1,47 @@ > +#include <xtf.h> > + > +#include "util.h" > + > +static uint8_t vmxon_region[PAGE_SIZE] __aligned(PAGE_SIZE); > + > +/** > + * vmxon with CR4.VMXE cleared > + * > + * Expect: #UD > + */ > +static bool test_vmxon_novmxe(void) > +{ > + uint8_t ret; > + exinfo_t fault; > + > + if ( read_cr4() & X86_CR4_VMXE ) > + { > + xtf_skip("Skip: CR4.VMXE is already set, " > + "skip testing vmxon w/ CR4.VMXE=0\n"); > + return true; Tests should, wherever possible, not depend on the starting state of the microkernel. They are also entirely free to mess with any state they want to construct a test scenario. In this case, I would suggest... > + } > + > + clear_vmcs(vmxon_region, get_vmcs_revid()); > + ret = vmxon((uint64_t)vmxon_region, &fault); > + > + return handle_vmxinsn_err(__func__, ret, fault, > + VMXERR_FAULT, EXINFO_SYM(UD, 0), 0); > +} > + > +bool test_vmxon(void) > +{ unsigned long cr4 = read_cr4(); if ( cr4 & X86_CR4_VMXE ) write_cr4(cr4 & ~X86_CR4_VMXE); to explicitly set up state as intended. ~Andrew > + if ( !test_vmxon_novmxe() ) > + return false; > + > + return true; > +} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */
On 12/16/16 20:25 +0000, Andrew Cooper wrote: >On 16/12/16 13:43, Haozhong Zhang wrote: >> diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c >> index 74b4d01..f30fc26 100644 >> --- a/tests/vvmx/util.c >> +++ b/tests/vvmx/util.c >> @@ -1,7 +1,31 @@ >> #include <xtf.h> >> +#include <arch/x86/msr-index.h> >> #include <arch/x86/hvm/vmx/vmcs.h> >> #include "util.h" >> >> +#define INVALID_VMCS_REVID (~(uint32_t)0) >> + >> +static uint32_t vmcs_revid = INVALID_VMCS_REVID; >> + >> +uint32_t get_vmcs_revid(void) >> +{ >> + if ( vmcs_revid != INVALID_VMCS_REVID ) >> + goto out; >> + >> + uint64_t vmx_basic = rdmsr(MSR_IA32_VMX_BASIC); >> + >> + vmcs_revid = (uint32_t)vmx_basic & VMX_BASIC_REVISION_MASK; >> + >> +out: >> + return vmcs_revid; >> +} >> + >> +void clear_vmcs(void *vmcs, uint32_t revid) > >Strictly speaking, this should be setup_vmcs() as it does more than just >clearing it. > will change the name >> +{ >> + memset(vmcs, 0, PAGE_SIZE); >> + *((uint32_t *)vmcs) = revid; >> +} >> + >> #define vvmx_failure(prefix, fmt, ...) \ >> do { \ >> xtf_failure("Fail: %s: "fmt, prefix, ##__VA_ARGS__); \ >> diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c >> index e69de29..31f074c 100644 >> --- a/tests/vvmx/vmxon.c >> +++ b/tests/vvmx/vmxon.c >> @@ -0,0 +1,47 @@ >> +#include <xtf.h> >> + >> +#include "util.h" >> + >> +static uint8_t vmxon_region[PAGE_SIZE] __aligned(PAGE_SIZE); >> + >> +/** >> + * vmxon with CR4.VMXE cleared >> + * >> + * Expect: #UD >> + */ >> +static bool test_vmxon_novmxe(void) >> +{ >> + uint8_t ret; >> + exinfo_t fault; >> + >> + if ( read_cr4() & X86_CR4_VMXE ) >> + { >> + xtf_skip("Skip: CR4.VMXE is already set, " >> + "skip testing vmxon w/ CR4.VMXE=0\n"); >> + return true; > >Tests should, wherever possible, not depend on the starting state of the >microkernel. They are also entirely free to mess with any state they >want to construct a test scenario. > >In this case, I would suggest... > >> + } >> + >> + clear_vmcs(vmxon_region, get_vmcs_revid()); >> + ret = vmxon((uint64_t)vmxon_region, &fault); >> + >> + return handle_vmxinsn_err(__func__, ret, fault, >> + VMXERR_FAULT, EXINFO_SYM(UD, 0), 0); >> +} >> + >> +bool test_vmxon(void) >> +{ > >unsigned long cr4 = read_cr4(); > >if ( cr4 & X86_CR4_VMXE ) > write_cr4(cr4 & ~X86_CR4_VMXE); > >to explicitly set up state as intended. > will change to your suggestion. Thanks, Haozhong
diff --git a/include/arch/x86/msr-index.h b/include/arch/x86/msr-index.h index b7aeef0..ce5d068 100644 --- a/include/arch/x86/msr-index.h +++ b/include/arch/x86/msr-index.h @@ -17,6 +17,7 @@ #define MSR_MISC_FEATURES_CPUID_FAULTING (1ULL << _MSR_MISC_FEATURES_CPUID_FAULTING) #define MSR_IA32_VMX_BASIC 0x00000480 +#define VMX_BASIC_REVISION_MASK 0x7fffffff #define VMX_BASIC_VMCS_SIZE_MASK (0x1fffULL << 32) #define VMX_BASIC_32BIT_ADDRESSES (1ULL << 48) diff --git a/tests/vvmx/Makefile b/tests/vvmx/Makefile index 24d3720..fecb7c1 100644 --- a/tests/vvmx/Makefile +++ b/tests/vvmx/Makefile @@ -6,6 +6,6 @@ TEST-ENVS := hvm64 TEST-EXTRA-CFG := extra.cfg.in -obj-perenv += main.o cpuid.o msr.o util.o +obj-perenv += main.o cpuid.o msr.o util.o vmxon.o include $(ROOT)/build/gen.mk diff --git a/tests/vvmx/main.c b/tests/vvmx/main.c index bd36f10..cec9057 100644 --- a/tests/vvmx/main.c +++ b/tests/vvmx/main.c @@ -15,6 +15,7 @@ const char test_title[] = "Test vvmx"; extern bool test_cpuid_vmx_feat(void); extern bool test_msr_vmx(void); +extern bool test_vmxon(void); void test_main(void) { @@ -30,6 +31,9 @@ void test_main(void) if ( !test_msr_vmx() ) goto fail; + if ( !test_vmxon() ) + goto fail; + xtf_success(NULL); return; diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c index 74b4d01..f30fc26 100644 --- a/tests/vvmx/util.c +++ b/tests/vvmx/util.c @@ -1,7 +1,31 @@ #include <xtf.h> +#include <arch/x86/msr-index.h> #include <arch/x86/hvm/vmx/vmcs.h> #include "util.h" +#define INVALID_VMCS_REVID (~(uint32_t)0) + +static uint32_t vmcs_revid = INVALID_VMCS_REVID; + +uint32_t get_vmcs_revid(void) +{ + if ( vmcs_revid != INVALID_VMCS_REVID ) + goto out; + + uint64_t vmx_basic = rdmsr(MSR_IA32_VMX_BASIC); + + vmcs_revid = (uint32_t)vmx_basic & VMX_BASIC_REVISION_MASK; + +out: + return vmcs_revid; +} + +void clear_vmcs(void *vmcs, uint32_t revid) +{ + memset(vmcs, 0, PAGE_SIZE); + *((uint32_t *)vmcs) = revid; +} + #define vvmx_failure(prefix, fmt, ...) \ do { \ xtf_failure("Fail: %s: "fmt, prefix, ##__VA_ARGS__); \ diff --git a/tests/vvmx/util.h b/tests/vvmx/util.h index f18b9cc..c7b1c99 100644 --- a/tests/vvmx/util.h +++ b/tests/vvmx/util.h @@ -5,6 +5,24 @@ #include <arch/x86/hvm/vmx/vmcs.h> /** + * Get a valid VMCS revision ID from MSR_IA32_VMX_BASIC. + * + * Return: + * VMCS revision ID + */ +uint32_t get_vmcs_revid(void); + +/** + * Zero a VMXON region or VMCS and set the VMCS revision ID. + * + * Parameters: + * @vmcs: pointer to VMXON region or VMCS + * @revid: the VMCS revision ID to be used. It's not required + * to be a valid revision ID. + */ +void clear_vmcs(void *vmcs, uint32_t revid); + +/** * Flags for errors during the execution of a VMX instruction. * * NB. Besides VMXERR_NOERR, other flags are not mutually exclusive, diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c index e69de29..31f074c 100644 --- a/tests/vvmx/vmxon.c +++ b/tests/vvmx/vmxon.c @@ -0,0 +1,47 @@ +#include <xtf.h> + +#include "util.h" + +static uint8_t vmxon_region[PAGE_SIZE] __aligned(PAGE_SIZE); + +/** + * vmxon with CR4.VMXE cleared + * + * Expect: #UD + */ +static bool test_vmxon_novmxe(void) +{ + uint8_t ret; + exinfo_t fault; + + if ( read_cr4() & X86_CR4_VMXE ) + { + xtf_skip("Skip: CR4.VMXE is already set, " + "skip testing vmxon w/ CR4.VMXE=0\n"); + return true; + } + + clear_vmcs(vmxon_region, get_vmcs_revid()); + ret = vmxon((uint64_t)vmxon_region, &fault); + + return handle_vmxinsn_err(__func__, ret, fault, + VMXERR_FAULT, EXINFO_SYM(UD, 0), 0); +} + +bool test_vmxon(void) +{ + if ( !test_vmxon_novmxe() ) + return false; + + return true; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */
Fault #UD is expected in this test. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- include/arch/x86/msr-index.h | 1 + tests/vvmx/Makefile | 2 +- tests/vvmx/main.c | 4 ++++ tests/vvmx/util.c | 24 ++++++++++++++++++++++ tests/vvmx/util.h | 18 +++++++++++++++++ tests/vvmx/vmxon.c | 47 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 95 insertions(+), 1 deletion(-)