| Message ID | 20161216134348.16236-2-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/cpuid.c b/tests/vvmx/cpuid.c > new file mode 100644 > index 0000000..9a4cdae > --- /dev/null > +++ b/tests/vvmx/cpuid.c > @@ -0,0 +1,24 @@ > +#include <xtf.h> > + > +bool test_cpuid_vmx_feat(void) > +{ > + uint32_t ecx = cpuid_ecx(1); > + > + if ( !(ecx & X86_FEATURE_VMX) ) > + { > + xtf_failure("Fail: cpuid.1:ecx[5] is not set.\n"); > + return false; > + } Some cpuid information is cached at boot. It turns out that there is already a suitable cpu_has_vmx define. > +#include <xtf.h> > + > +const char test_title[] = "Test vvmx"; > + > +extern bool test_cpuid_vmx_feat(void); > + > +void test_main(void) > +{ > + if ( !vendor_is(X86_VENDOR_INTEL) ) There is a slightly shorter vendor_is_intel which you can use. > + { > + xtf_skip("Skip: non-Intel processors\n"); "processor" > + return; Where it makes the code easier to read, I tend to use return xtf_skip("Skip: non-Intel processor\n"), which in this case allows the braces to be dropped. However, I am not overly fussed if you prefer this style. > + } > + > + if ( !test_cpuid_vmx_feat() ) > + goto fail; Are you intending to do converse tests? We have had security issues in the past where some of the nested-virt code in Xen was reachable from a guest even through the feature was intended to be fully disabled. ~Andrew
On 12/16/16 14:40 +0000, Andrew Cooper wrote: >On 16/12/16 13:43, Haozhong Zhang wrote: >> diff --git a/tests/vvmx/cpuid.c b/tests/vvmx/cpuid.c >> new file mode 100644 >> index 0000000..9a4cdae >> --- /dev/null >> +++ b/tests/vvmx/cpuid.c >> @@ -0,0 +1,24 @@ >> +#include <xtf.h> >> + >> +bool test_cpuid_vmx_feat(void) >> +{ >> + uint32_t ecx = cpuid_ecx(1); >> + >> + if ( !(ecx & X86_FEATURE_VMX) ) >> + { >> + xtf_failure("Fail: cpuid.1:ecx[5] is not set.\n"); >> + return false; >> + } > >Some cpuid information is cached at boot. > >It turns out that there is already a suitable cpu_has_vmx define. > Ah yes, it's what I want. >> +#include <xtf.h> >> + >> +const char test_title[] = "Test vvmx"; >> + >> +extern bool test_cpuid_vmx_feat(void); >> + >> +void test_main(void) >> +{ >> + if ( !vendor_is(X86_VENDOR_INTEL) ) > >There is a slightly shorter vendor_is_intel which you can use. > ditto >> + { >> + xtf_skip("Skip: non-Intel processors\n"); > >"processor" > will change >> + return; > >Where it makes the code easier to read, I tend to use return >xtf_skip("Skip: non-Intel processor\n"), which in this case allows the >braces to be dropped. However, I am not overly fussed if you prefer >this style. > Your suggestion is more clear and I'll change. >> + } >> + >> + if ( !test_cpuid_vmx_feat() ) >> + goto fail; > >Are you intending to do converse tests? We have had security issues in >the past where some of the nested-virt code in Xen was reachable from a >guest even through the feature was intended to be fully disabled. > I should follow the pseudo code in Intel SDM more closely, as the pseudo code has a branch saying if vmxon is executed w/o VMX support, there will be some errors. I'll add test cases for such circumstance. Haozhong
diff --git a/tests/vvmx/Makefile b/tests/vvmx/Makefile new file mode 100644 index 0000000..80a6629 --- /dev/null +++ b/tests/vvmx/Makefile @@ -0,0 +1,11 @@ +include $(ROOT)/build/common.mk + +NAME := vvmx +CATEGORY := functional +TEST-ENVS := hvm64 + +TEST-EXTRA-CFG := extra.cfg.in + +obj-perenv += main.o cpuid.o + +include $(ROOT)/build/gen.mk diff --git a/tests/vvmx/cpuid.c b/tests/vvmx/cpuid.c new file mode 100644 index 0000000..9a4cdae --- /dev/null +++ b/tests/vvmx/cpuid.c @@ -0,0 +1,24 @@ +#include <xtf.h> + +bool test_cpuid_vmx_feat(void) +{ + uint32_t ecx = cpuid_ecx(1); + + if ( !(ecx & X86_FEATURE_VMX) ) + { + xtf_failure("Fail: cpuid.1:ecx[5] is not set.\n"); + return false; + } + + return true; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/tests/vvmx/extra.cfg.in b/tests/vvmx/extra.cfg.in new file mode 100644 index 0000000..ae494f8 --- /dev/null +++ b/tests/vvmx/extra.cfg.in @@ -0,0 +1 @@ +nestedhvm = 1 diff --git a/tests/vvmx/main.c b/tests/vvmx/main.c new file mode 100644 index 0000000..8506b7b --- /dev/null +++ b/tests/vvmx/main.c @@ -0,0 +1,44 @@ +/** + * @file tests/vvmx/main.c + * @ref test-vvmx + * + * @page test-vvmx vvmx + * + * Test VMX features (CPUID, MSR, VMX instructions, EPT, APIC etc.) in + * the nested VMX environment. + * + * @see tests/vvmx/main.c + */ +#include <xtf.h> + +const char test_title[] = "Test vvmx"; + +extern bool test_cpuid_vmx_feat(void); + +void test_main(void) +{ + if ( !vendor_is(X86_VENDOR_INTEL) ) + { + xtf_skip("Skip: non-Intel processors\n"); + return; + } + + if ( !test_cpuid_vmx_feat() ) + goto fail; + + xtf_success(NULL); + return; + +fail: + xtf_failure(NULL); +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */
cpuid.1:ecx[5] is expected to be set in this test. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- tests/vvmx/Makefile | 11 +++++++++++ tests/vvmx/cpuid.c | 24 ++++++++++++++++++++++++ tests/vvmx/extra.cfg.in | 1 + tests/vvmx/main.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+) create mode 100644 tests/vvmx/Makefile create mode 100644 tests/vvmx/cpuid.c create mode 100644 tests/vvmx/extra.cfg.in create mode 100644 tests/vvmx/main.c