Re: [PATCH v7 10/11] KVM: selftests: Test disabling NX hugepages on a VM

Ben Gardon
2022-05-03T18:34:37Z

On Tue, May 3, 2022 at 11:31 AM Ben Gardon <bgardon@google.com> wrote:
>
> Add an argument to the NX huge pages test to test disabling the feature
> on a VM using the new capability.
>
> Reviewed-by: David Matlack <dmatlack@google.com>
> Signed-off-by: Ben Gardon <bgardon@google.com>
> ---
> .../selftests/kvm/include/kvm_util_base.h | 2 +
> tools/testing/selftests/kvm/lib/kvm_util.c | 27 ++++++-
> .../selftests/kvm/x86_64/nx_huge_pages_test.c | 70 +++++++++++++++----
> .../kvm/x86_64/nx_huge_pages_test.sh | 12 +++-
> 4 files changed, 95 insertions(+), 16 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
> index 1dac3c6607f1..eee96189c1c4 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> @@ -414,4 +414,6 @@ uint64_t vm_get_stat(struct kvm_vm *vm, const char *stat_name);
>
> uint32_t guest_get_vcpuid(void);
>
> +int __vm_disable_nx_huge_pages(struct kvm_vm *vm);
> +
> #endif /* SELFTEST_KVM_UTIL_BASE_H */
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 27ffd2537df6..0ec7efc2900d 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -112,6 +112,11 @@ int vm_check_cap(struct kvm_vm *vm, long cap)
> return ret;
> }
>
> +static int __vm_enable_cap(struct kvm_vm *vm, struct kvm_enable_cap *cap)
> +{
> + return ioctl(vm->fd, KVM_ENABLE_CAP, cap);
> +}
> +
> /* VM Enable Capability
> *
> * Input Args:
> @@ -128,7 +133,7 @@ int vm_enable_cap(struct kvm_vm *vm, struct kvm_enable_cap *cap)
> {
> int ret;
>
> - ret = ioctl(vm->fd, KVM_ENABLE_CAP, cap);
> + ret = __vm_enable_cap(vm, cap);
> TEST_ASSERT(ret == 0, "KVM_ENABLE_CAP IOCTL failed,\n"
> " rc: %i errno: %i", ret, errno);
>
> @@ -2758,3 +2763,23 @@ uint64_t vm_get_stat(struct kvm_vm *vm, const char *stat_name)
> stat_name, ret);
> return data;
> }
> +
> +/* VM disable NX huge pages
> + *
> + * Input Args:
> + * vm - Virtual Machine
> + *
> + * Output Args: None
> + *
> + * Return: On success, 0 -ERRNO on failure.
> + *
> + * Disables NX huge pages for the VM.
> + */
> +int __vm_disable_nx_huge_pages(struct kvm_vm *vm)
> +{
> + struct kvm_enable_cap cap = { 0 };
> +
> + cap.cap = KVM_CAP_VM_DISABLE_NX_HUGE_PAGES;
> + cap.args[0] = 0;
> + return __vm_enable_cap(vm, &cap);
> +}
> 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 238a6047791c..1e7328dd33d2 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
> @@ -13,6 +13,8 @@
> #include <fcntl.h>
> #include <stdint.h>
> #include <time.h>
> +#include <linux/reboot.h>
> +#include <sys/syscall.h>
>
> #include <test_util.h>
> #include "kvm_util.h"
> @@ -89,18 +91,36 @@ static void check_split_count(struct kvm_vm *vm, int expected_splits)
> expected_splits, actual_splits);
> }
>
> -int main(int argc, char **argv)
> +void run_test(bool disable_nx_huge_pages, bool reboot_permissions)
> {
> struct kvm_vm *vm;
> struct timespec ts;
> + uint64_t pages;
> void *hva;
> -
> - if (argc != 2 || strtol(argv[1], NULL, 0) != MAGIC_TOKEN) {
> - printf("This test must be run through nx_huge_pages_test.sh");
> - return KSFT_SKIP;
> + int r;
> +
> + pages = vm_pages_needed(VM_MODE_DEFAULT, 1, DEFAULT_GUEST_PHY_PAGES,
> + 0, 0);
> + vm = vm_create_without_vcpus(VM_MODE_DEFAULT, pages);
> +
> + 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");
> + } else {
> + TEST_ASSERT(r == -EPERM, "This process should not have permission to disable NX huge pages");
> + return;
> + }
> }
>
> - vm = vm_create_default(0, 0, guest_code);
> + vm_vcpu_add_default(vm, 0, guest_code);
>
> vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS_HUGETLB,
> HPAGE_GPA, HPAGE_SLOT,
> @@ -133,23 +153,27 @@ int main(int argc, char **argv)
> /*
> * Next, the guest will execute from the first huge page, causing it
> * to be remapped at 4k.
> + *
> + * If NX huge pages are disabled, this should have no effect.
> */
> vcpu_run(vm, 0);
> - check_2m_page_count(vm, 1);
> - check_split_count(vm, 1);
> + check_2m_page_count(vm, disable_nx_huge_pages ? 2 : 1);
> + check_split_count(vm, disable_nx_huge_pages ? 0 : 1);
>
> /*
> * Executing from the third huge page (previously unaccessed) will
> * cause part to be mapped at 4k.
> + *
> + * If NX huge pages are disabled, it should be mapped at 2M.
> */
> vcpu_run(vm, 0);
> - check_2m_page_count(vm, 1);
> - check_split_count(vm, 2);
> + check_2m_page_count(vm, disable_nx_huge_pages ? 3 : 1);
> + check_split_count(vm, disable_nx_huge_pages ? 0 : 2);
>
> /* Reading from the first huge page again should have no effect. */
> vcpu_run(vm, 0);
> - check_2m_page_count(vm, 1);
> - check_split_count(vm, 2);
> + check_2m_page_count(vm, disable_nx_huge_pages ? 3 : 1);
> + check_split_count(vm, disable_nx_huge_pages ? 0 : 2);
>
> /*
> * Give recovery thread time to run. The wrapper script sets
> @@ -161,8 +185,11 @@ int main(int argc, char **argv)
>
> /*
> * Now that the reclaimer has run, all the split pages should be gone.
> + *
> + * If NX huge pages are disabled, the relaimer will not run, so
> + * nothing should change from here on.
> */
> - check_2m_page_count(vm, 1);
> + check_2m_page_count(vm, disable_nx_huge_pages ? 3 : 1);
> check_split_count(vm, 0);
>
> /*
> @@ -170,10 +197,25 @@ int main(int argc, char **argv)
> * reading from it causes a huge page mapping to be installed.
> */
> vcpu_run(vm, 0);
> - check_2m_page_count(vm, 2);
> + check_2m_page_count(vm, disable_nx_huge_pages ? 3 : 2);
> check_split_count(vm, 0);
>
> kvm_vm_free(vm);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + bool reboot_permissions;
> +
> + if (argc != 3 || strtol(argv[1], NULL, 0) != MAGIC_TOKEN) {
> + printf("This test must be run through nx_huge_pages_test.sh");
> + return KSFT_SKIP;
> + }
> +
> + reboot_permissions = strtol(argv[2], NULL, 0);
> +
> + run_test(false, reboot_permissions);
> + run_test(true, reboot_permissions);
>
> return 0;
> }
> diff --git a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.sh b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.sh
> index 60bfed8181b9..c21c1f639141 100755
> --- a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.sh
> +++ b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.sh
> @@ -16,6 +16,8 @@ HUGE_PAGES=$(sudo cat /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages)
>
> set +e
>
> +NXECUTABLE="$(dirname $0)/nx_huge_pages_test"
> +
> (
> set -e
>
> @@ -24,7 +26,15 @@ set +e
> sudo echo 100 > /sys/module/kvm/parameters/nx_huge_pages_recovery_period_ms
> sudo echo 3 > /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages
>
> - "$(dirname $0)"/nx_huge_pages_test 887563923
> + # Test with reboot permissions
> + sudo setcap cap_sys_boot+ep $NXECUTABLE
> + $NXECUTABLE 887563923 1
> +
> + # Test without reboot permissions
> + if [ $(whoami) != "root" ] ; then
> + sudo setcap cap_sys_boot-ep $NXECUTABLE
> + $NXECUTABLE 887563923 0
> + fi
> )
> RET=$?

I feel lazy and dumb for not testing this as a non-root user, but I've
been working on setting up an environment to do such testing for over
a week just to test this commit, with no success. Perhaps I am too
ensconced in Google's development tools. In any case, if anyone is
willing to run this test as a non-root user to make sure it doesn't
blow up, I'd appreciate it. I can confirm that the test works fine
when run as root.

>
> --
> 2.36.0.464.gb9c8b46e94-goog
>