Re: [Xen-devel] [XTF PATCH 01/16] vvmx: test whether VMX feature is present in CPUID

[Haozhong Zhang <haozhong.zhang@xxxxxxxxx>]

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

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