| Message ID | 20220503183045.978509-11-bgardon@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Add a cap to disable NX hugepages on a VM | expand |
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 >
On Tue, May 03, 2022, Ben Gardon wrote: > + 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"); This is wrong, the return value on ioctl() failure is -1, the error code is in errno and it's a positive value. LOL, but it passes because EPERM == 1, hilarious. To avoid confusion: TEST_ASSERT(r == -1 && errno == EPERM, "This process should not have permission to disable NX huge pages"); > 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 This leaves cap_sys_boot set on the executable if the script is run as root. Probably this? It's moderately user friendly without going too crazy on error handling. # Test with reboot permissions if [ $(whoami) != "root" ] ; then sudo setcap cap_sys_boot+ep $NXECUTABLE fi $NXECUTABLE 887563923 1 # Test without reboot permissions if [ $(whoami) != "root" ] ; then sudo setcap cap_sys_boot-ep $NXECUTABLE $NXECUTABLE 887563923 0 fi > + $NXECUTABLE 887563923 1 > + > + # Test without reboot permissions > + if [ $(whoami) != "root" ] ; then > + sudo setcap cap_sys_boot-ep $NXECUTABLE > + $NXECUTABLE 887563923 0 I would much prefer a proper flag, not a magic 0 vs. 1. > + fi > ) > RET=$? > > -- > 2.36.0.464.gb9c8b46e94-goog >
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=$?