On Fri, Aug 12, 2022 at 05:53:01PM +0000, Oliver Upton wrote:
> Avoid boilerplate for checking test preconditions by using
> TEST_REQUIRE(). While at it, add a precondition for
> KVM_CAP_VM_DISABLE_NX_HUGE_PAGES to skip (instead of silently pass) on
> older kernels.
>
> Signed-off-by: Oliver Upton <oliver.upton@xxxxxxxxx>
> ---
> .../selftests/kvm/x86_64/nx_huge_pages_test.c | 24 +++++--------------
> 1 file changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
> index cc6421716400..e19933ea34ca 100644
> --- a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
> @@ -118,13 +118,6 @@ void run_test(int reclaim_period_ms, bool disable_nx_huge_pages,
> vm = vm_create(1);
>
> if (disable_nx_huge_pages) {
> - /*
> - * Cannot run the test without NX huge pages if the kernel
> - * does not support it.
> - */
> - if (!kvm_check_cap(KVM_CAP_VM_DISABLE_NX_HUGE_PAGES))
> - return;
> -
> r = __vm_disable_nx_huge_pages(vm);
> if (reboot_permissions) {
> TEST_ASSERT(!r, "Disabling NX huge pages should succeed if process has reboot permissions");
> @@ -248,18 +241,13 @@ int main(int argc, char **argv)
> }
> }
>
> - if (token != MAGIC_TOKEN) {
> - print_skip("This test must be run with the magic token %d.\n"
> - "This is done by nx_huge_pages_test.sh, which\n"
> - "also handles environment setup for the test.",
> - MAGIC_TOKEN);
> - exit(KSFT_SKIP);
> - }
> + TEST_REQUIRE(kvm_has_cap(KVM_CAP_VM_DISABLE_NX_HUGE_PAGES));
This cap is only needed for run_test(..., true, ...) below so I don't think we should require it for the entire test.
That being said, it still might be good to inform the user that the test is being skipped. So perhaps something like this:
...
run_test(reclaim_period_ms, false, reboot_permissions);
if (kvm_has_cap(KVM_CAP_VM_DISABLE_NX_HUGE_PAGES))
run_test(reclaim_period_ms, true, reboot_permissions);
else
print_skip("KVM_CAP_VM_DISABLE_NX_HUGE_PAGES not supported");
...
> + TEST_REQUIRE(reclaim_period_ms > 0);
>
> - if (!reclaim_period_ms) {
> - print_skip("The NX reclaim period must be specified and non-zero");
> - exit(KSFT_SKIP);
> - }
> + __TEST_REQUIRE(token == MAGIC_TOKEN,
> + "This test must be run with the magic token %d.\n"
> + "This is done by nx_huge_pages_test.sh, which\n"
> + "also handles environment setup for the test.");
>
> run_test(reclaim_period_ms, false, reboot_permissions);
> run_test(reclaim_period_ms, true, reboot_permissions);
>
> base-commit: 93472b79715378a2386598d6632c654a2223267b
> --
> 2.37.1.595.g718a3a8f04-goog
>
