| Message ID | 20161216134348.16236-8-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/vmxon.c b/tests/vvmx/vmxon.c > index 31f074c..ca33b3c 100644 > --- a/tests/vvmx/vmxon.c > +++ b/tests/vvmx/vmxon.c > @@ -28,11 +28,42 @@ static bool test_vmxon_novmxe(void) > VMXERR_FAULT, EXINFO_SYM(UD, 0), 0); > } > > +static unsigned long vmxon_in_user(void) I'd name this user_vmxon() as it is slightly shorter, but I'm not terribly fussed. > +{ > + exinfo_t fault; > + unsigned long ret = vmxon((uint64_t)vmxon_region, &fault); > + > + return (ret << 32) | fault; > +} > + > +/** > + * vmxon in CPL=3 and out of VMX operation > + * > + * Expect: #GP(0) > + */ > +static bool test_vmxon_in_user(void) Similarly, test_user_vmxon() > +{ > + clear_vmcs(vmxon_region, get_vmcs_revid()); > + > + unsigned long ret = exec_user(vmxon_in_user); > + uint8_t err = (ret >> 32) & 0xff; > + exinfo_t fault = ret & 0xFFFFFFFF; > + > + return handle_vmxinsn_err(__func__, err, fault, > + VMXERR_FAULT, EXINFO_SYM(GP, 0), 0); > +} > + > bool test_vmxon(void) > { > if ( !test_vmxon_novmxe() ) > return false; Your subject says out of VMX operation, but the implementation is inside VMX operation. It would be worth testing both scenarios, as they should be distinguished by #UD vs #GP[0]. ~Andrew > > + unsigned long cr4 = read_cr4(); > + write_cr4(cr4 | X86_CR4_VMXE); > + > + if ( !test_vmxon_in_user() ) > + return false; > + > return true; > } >
On 12/16/16 20:33 +0000, Andrew Cooper wrote: >On 16/12/16 13:43, Haozhong Zhang wrote: >> diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c >> index 31f074c..ca33b3c 100644 >> --- a/tests/vvmx/vmxon.c >> +++ b/tests/vvmx/vmxon.c >> @@ -28,11 +28,42 @@ static bool test_vmxon_novmxe(void) >> VMXERR_FAULT, EXINFO_SYM(UD, 0), 0); >> } >> >> +static unsigned long vmxon_in_user(void) > >I'd name this user_vmxon() as it is slightly shorter, but I'm not >terribly fussed. > >> +{ >> + exinfo_t fault; >> + unsigned long ret = vmxon((uint64_t)vmxon_region, &fault); >> + >> + return (ret << 32) | fault; >> +} >> + >> +/** >> + * vmxon in CPL=3 and out of VMX operation >> + * >> + * Expect: #GP(0) >> + */ >> +static bool test_vmxon_in_user(void) > >Similarly, test_user_vmxon() > I'll turn to shorter names. >> +{ >> + clear_vmcs(vmxon_region, get_vmcs_revid()); >> + >> + unsigned long ret = exec_user(vmxon_in_user); >> + uint8_t err = (ret >> 32) & 0xff; >> + exinfo_t fault = ret & 0xFFFFFFFF; >> + >> + return handle_vmxinsn_err(__func__, err, fault, >> + VMXERR_FAULT, EXINFO_SYM(GP, 0), 0); >> +} >> + >> bool test_vmxon(void) >> { >> if ( !test_vmxon_novmxe() ) >> return false; > >Your subject says out of VMX operation, but the implementation is inside >VMX operation. > vmxon in test_vmxon_novmxe() fails if test_vmxon_novmxe() return true, so here we are still out of VMX operation. >It would be worth testing both scenarios, as they should be >distinguished by #UD vs #GP[0]. > Yes, patch 13 - 16 are for this purpose. Thanks, Haozhong
On 19/12/16 03:35, Haozhong Zhang wrote: > On 12/16/16 20:33 +0000, Andrew Cooper wrote: >> On 16/12/16 13:43, Haozhong Zhang wrote: >>> diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c >>> index 31f074c..ca33b3c 100644 >>> --- a/tests/vvmx/vmxon.c >>> +++ b/tests/vvmx/vmxon.c >>> @@ -28,11 +28,42 @@ static bool test_vmxon_novmxe(void) >>> VMXERR_FAULT, EXINFO_SYM(UD, 0), 0); >>> } >>> >>> +static unsigned long vmxon_in_user(void) >> >> I'd name this user_vmxon() as it is slightly shorter, but I'm not >> terribly fussed. >> >>> +{ >>> + exinfo_t fault; >>> + unsigned long ret = vmxon((uint64_t)vmxon_region, &fault); >>> + >>> + return (ret << 32) | fault; >>> +} >>> + >>> +/** >>> + * vmxon in CPL=3 and out of VMX operation >>> + * >>> + * Expect: #GP(0) >>> + */ >>> +static bool test_vmxon_in_user(void) >> >> Similarly, test_user_vmxon() >> > > I'll turn to shorter names. > >>> +{ >>> + clear_vmcs(vmxon_region, get_vmcs_revid()); >>> + >>> + unsigned long ret = exec_user(vmxon_in_user); >>> + uint8_t err = (ret >> 32) & 0xff; >>> + exinfo_t fault = ret & 0xFFFFFFFF; >>> + >>> + return handle_vmxinsn_err(__func__, err, fault, >>> + VMXERR_FAULT, EXINFO_SYM(GP, 0), 0); >>> +} >>> + >>> bool test_vmxon(void) >>> { >>> if ( !test_vmxon_novmxe() ) >>> return false; >> >> Your subject says out of VMX operation, but the implementation is inside >> VMX operation. >> > > vmxon in test_vmxon_novmxe() fails if test_vmxon_novmxe() return true, > so here we are still out of VMX operation. Very good point. Its obvious now you point it out. Sorry for the noise. ~andrew
On 19/12/16 03:35, Haozhong Zhang wrote: > >>> +{ >>> + clear_vmcs(vmxon_region, get_vmcs_revid()); >>> + >>> + unsigned long ret = exec_user(vmxon_in_user); >>> + uint8_t err = (ret >> 32) & 0xff; >>> + exinfo_t fault = ret & 0xFFFFFFFF; >>> + >>> + return handle_vmxinsn_err(__func__, err, fault, >>> + VMXERR_FAULT, EXINFO_SYM(GP, 0), 0); >>> +} >>> + >>> bool test_vmxon(void) >>> { >>> if ( !test_vmxon_novmxe() ) >>> return false; >> >> Your subject says out of VMX operation, but the implementation is inside >> VMX operation. >> > > vmxon in test_vmxon_novmxe() fails if test_vmxon_novmxe() return true, > so here we are still out of VMX operation. Very good point. Its obvious now you point it out. Sorry for the noise. ~Andrew
diff --git a/tests/vvmx/main.c b/tests/vvmx/main.c index cec9057..c1852fd 100644 --- a/tests/vvmx/main.c +++ b/tests/vvmx/main.c @@ -13,6 +13,8 @@ const char test_title[] = "Test vvmx"; +bool test_wants_user_mappings = true; + extern bool test_cpuid_vmx_feat(void); extern bool test_msr_vmx(void); extern bool test_vmxon(void); diff --git a/tests/vvmx/vmxon.c b/tests/vvmx/vmxon.c index 31f074c..ca33b3c 100644 --- a/tests/vvmx/vmxon.c +++ b/tests/vvmx/vmxon.c @@ -28,11 +28,42 @@ static bool test_vmxon_novmxe(void) VMXERR_FAULT, EXINFO_SYM(UD, 0), 0); } +static unsigned long vmxon_in_user(void) +{ + exinfo_t fault; + unsigned long ret = vmxon((uint64_t)vmxon_region, &fault); + + return (ret << 32) | fault; +} + +/** + * vmxon in CPL=3 and out of VMX operation + * + * Expect: #GP(0) + */ +static bool test_vmxon_in_user(void) +{ + clear_vmcs(vmxon_region, get_vmcs_revid()); + + unsigned long ret = exec_user(vmxon_in_user); + uint8_t err = (ret >> 32) & 0xff; + exinfo_t fault = ret & 0xFFFFFFFF; + + return handle_vmxinsn_err(__func__, err, fault, + VMXERR_FAULT, EXINFO_SYM(GP, 0), 0); +} + bool test_vmxon(void) { if ( !test_vmxon_novmxe() ) return false; + unsigned long cr4 = read_cr4(); + write_cr4(cr4 | X86_CR4_VMXE); + + if ( !test_vmxon_in_user() ) + return false; + return true; }
Fault #GP(0) is expected in this test. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- tests/vvmx/main.c | 2 ++ tests/vvmx/vmxon.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+)