Re: [kvm-unit-tests PATCH 1/2] x86: nVMX: Do not use test_skip() when multiple tests are run

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Fri, Aug 30, 2019 at 01:40:30PM -0700, Nadav Amit wrote:
> Using test_skip() when multiple tests are run causes all the following
> tests to be skipped. Instead, just print a message and return.
> 
> Fixes: 47cc3d85c2fe ("nVMX x86: Check PML and EPT on vmentry of L2 guests")
> Fixes: 7fd449f2ed2e ("nVMX x86: Check VPID value on vmentry of L2 guests")
> Fixes: 181219bfd76b ("x86: Add test for checking NMI controls on vmentry of L2 guests")
> Fixes: 1d70eb823e12 ("nVMX x86: Check EPTP on vmentry of L2 guests")
> Cc: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> Signed-off-by: Nadav Amit <namit@vmware.com>

invvpid_test_v2() also has a bunch of bad calls to test_skip().

What about removing test_skip() entirely?  The code for in_guest looks
suspect, e.g. at a glance it should use HYPERCALL_VMSKIP instead of
HYPERCALL_VMABORT.  The only somewhat legit usage is the ept tests, and
only then because the ept tests are all at the end of the array.
Returning success/failure from ept_access_test_setup() seems like a
better solution than test_skip.

> ---
>  x86/vmx_tests.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index f035f24..4ff1570 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -4040,7 +4040,7 @@ static void test_vpid(void)
>  
>  	if (!((ctrl_cpu_rev[0].clr & CPU_SECONDARY) &&
>  	    (ctrl_cpu_rev[1].clr & CPU_VPID))) {
> -		test_skip("Secondary controls and/or VPID not supported");
> +		printf("Secondary controls and/or VPID not supported\n");
>  		return;
>  	}
>  
> @@ -4544,7 +4544,7 @@ static void test_nmi_ctrls(void)
>  
>  	if ((ctrl_pin_rev.clr & (PIN_NMI | PIN_VIRT_NMI)) !=
>  	    (PIN_NMI | PIN_VIRT_NMI)) {
> -		test_skip("NMI exiting and Virtual NMIs are not supported !");
> +		printf("NMI exiting and Virtual NMIs are not supported !\n");
>  		return;
>  	}
>  
> @@ -4657,7 +4657,7 @@ static void test_ept_eptp(void)
>  
>  	if (!((ctrl_cpu_rev[0].clr & CPU_SECONDARY) &&
>  	    (ctrl_cpu_rev[1].clr & CPU_EPT))) {
> -		test_skip("\"CPU secondary\" and/or \"enable EPT\" execution controls are not supported !");
> +		printf("\"CPU secondary\" and/or \"enable EPT\" execution controls are not supported !\n");
>  		return;
>  	}
>  
> @@ -4844,7 +4844,7 @@ static void test_pml(void)
>  
>  	if (!((ctrl_cpu_rev[0].clr & CPU_SECONDARY) &&
>  	    (ctrl_cpu_rev[1].clr & CPU_EPT) && (ctrl_cpu_rev[1].clr & CPU_PML))) {
> -		test_skip("\"Secondary execution\" control or \"enable EPT\" control or \"enable PML\" control is not supported !");
> +		printf("\"Secondary execution\" control or \"enable EPT\" control or \"enable PML\" control is not supported !\n");
>  		return;
>  	}
>  
> -- 
> 2.17.1
>