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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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

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