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@xxxxxxxxxx>
> Signed-off-by: Nadav Amit <namit@xxxxxxxxxx>
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
>
