> On Sep 3, 2019, at 10:28 AM, Sean Christopherson <sean.j.christopherson@xxxxxxxxx> wrote:
>
> 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().
In the case of invvpid_test_v2() the use seems correct, as the call is not
encapsulated within a group of tests. You want to skip all the tests if
invvpid is not supported for some reason.
> 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.
I don’t know. test_skip() does seem “nice” in theory (as long as it is not
used improperly). Having said that, the fact that it uses HYPERCALL_VMABORT
does seem wrong. I think it should be a separate change though.
