Re: [Xen-devel] [XTF PATCH 06/16] vvmx: test vmxon with CR4.VMXE cleared

[Haozhong Zhang <haozhong.zhang@xxxxxxxxx>]

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

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