| Message ID | 20210507190559.425518-1-dmatlack@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: selftests: Print a message if /dev/kvm is missing | expand |
On Fri, May 07, 2021 at 07:05:59PM +0000, David Matlack wrote: > If a KVM selftest is run on a machine without /dev/kvm, it will exit > silently. Make it easy to tell what's happening by printing an error > message. > > Opportunistically consolidate all codepaths that open /dev/kvm into a > single function so they all print the same message. > > This slightly changes the semantics of vm_is_unrestricted_guest() by > changing a TEST_ASSERT() to exit(KSFT_SKIP). However > vm_is_unrestricted_guest() is only called in one place > (x86_64/mmio_warning_test.c) and that is to determine if the test should > be skipped or not. > > Signed-off-by: David Matlack <dmatlack@google.com> > --- > .../testing/selftests/kvm/include/kvm_util.h | 1 + > tools/testing/selftests/kvm/lib/kvm_util.c | 35 +++++++++++-------- > .../selftests/kvm/lib/x86_64/processor.c | 16 +++------ > .../kvm/x86_64/get_msr_index_features.c | 8 ++--- > 4 files changed, 28 insertions(+), 32 deletions(-) > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > index a8f022794ce3..84982eb02b29 100644 > --- a/tools/testing/selftests/kvm/include/kvm_util.h > +++ b/tools/testing/selftests/kvm/include/kvm_util.h > @@ -77,6 +77,7 @@ struct vm_guest_mode_params { > }; > extern const struct vm_guest_mode_params vm_guest_mode_params[]; > > +int open_kvm_dev_path_or_exit(void); > int kvm_check_cap(long cap); > int vm_enable_cap(struct kvm_vm *vm, struct kvm_enable_cap *cap); > int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id, > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > index fc83f6c5902d..bb7dc65d7fb5 100644 > --- a/tools/testing/selftests/kvm/lib/kvm_util.c > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > @@ -31,6 +31,23 @@ static void *align(void *x, size_t size) > return (void *) (((size_t) x + mask) & ~mask); > } > > +/* Open KVM_DEV_PATH if available, otherwise exit the entire program. > + * > + * Return: > + * The opened file descriptor of /dev/kvm. > + */ > +int open_kvm_dev_path_or_exit(void) { > + int fd; > + > + fd = open(KVM_DEV_PATH, O_RDONLY); > + if (fd < 0) { > + print_skip("%s not available", KVM_DEV_PATH); > + exit(KSFT_SKIP); > + } > + > + return fd; > +} Style issues in the function above '{' and 2 spaces vs. 1 tab. > + > /* > * Capability > * > @@ -52,10 +69,7 @@ int kvm_check_cap(long cap) > int ret; > int kvm_fd; > > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > - if (kvm_fd < 0) > - exit(KSFT_SKIP); > - > + kvm_fd = open_kvm_dev_path_or_exit(); > ret = ioctl(kvm_fd, KVM_CHECK_EXTENSION, cap); > TEST_ASSERT(ret != -1, "KVM_CHECK_EXTENSION IOCTL failed,\n" > " rc: %i errno: %i", ret, errno); > @@ -128,9 +142,7 @@ void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size) > > static void vm_open(struct kvm_vm *vm, int perm) > { > - vm->kvm_fd = open(KVM_DEV_PATH, perm); I don't think we should change this one, otherwise the user provided perms are ignored. > - if (vm->kvm_fd < 0) > - exit(KSFT_SKIP); > + vm->kvm_fd = open_kvm_dev_path_or_exit(); > > if (!kvm_check_cap(KVM_CAP_IMMEDIATE_EXIT)) { > print_skip("immediate_exit not available"); > @@ -925,9 +937,7 @@ static int vcpu_mmap_sz(void) > { > int dev_fd, ret; > > - dev_fd = open(KVM_DEV_PATH, O_RDONLY); > - if (dev_fd < 0) > - exit(KSFT_SKIP); > + dev_fd = open_kvm_dev_path_or_exit(); spaces vs. tab here > > ret = ioctl(dev_fd, KVM_GET_VCPU_MMAP_SIZE, NULL); > TEST_ASSERT(ret >= sizeof(struct kvm_run), > @@ -2015,10 +2025,7 @@ bool vm_is_unrestricted_guest(struct kvm_vm *vm) > > if (vm == NULL) { > /* Ensure that the KVM vendor-specific module is loaded. */ > - f = fopen(KVM_DEV_PATH, "r"); > - TEST_ASSERT(f != NULL, "Error in opening KVM dev file: %d", > - errno); > - fclose(f); > + close(open_kvm_dev_path_or_exit()); spaces > } > > f = fopen("/sys/module/kvm_intel/parameters/unrestricted_guest", "r"); > diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c > index a8906e60a108..efe235044421 100644 > --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c > +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c > @@ -657,9 +657,7 @@ struct kvm_cpuid2 *kvm_get_supported_cpuid(void) > return cpuid; > > cpuid = allocate_kvm_cpuid2(); > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > - if (kvm_fd < 0) > - exit(KSFT_SKIP); > + kvm_fd = open_kvm_dev_path_or_exit(); > > ret = ioctl(kvm_fd, KVM_GET_SUPPORTED_CPUID, cpuid); > TEST_ASSERT(ret == 0, "KVM_GET_SUPPORTED_CPUID failed %d %d\n", > @@ -691,9 +689,7 @@ uint64_t kvm_get_feature_msr(uint64_t msr_index) > > buffer.header.nmsrs = 1; > buffer.entry.index = msr_index; > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > - if (kvm_fd < 0) > - exit(KSFT_SKIP); > + kvm_fd = open_kvm_dev_path_or_exit(); > > r = ioctl(kvm_fd, KVM_GET_MSRS, &buffer.header); > TEST_ASSERT(r == 1, "KVM_GET_MSRS IOCTL failed,\n" > @@ -986,9 +982,7 @@ struct kvm_msr_list *kvm_get_msr_index_list(void) > struct kvm_msr_list *list; > int nmsrs, r, kvm_fd; > > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > - if (kvm_fd < 0) > - exit(KSFT_SKIP); > + kvm_fd = open_kvm_dev_path_or_exit(); > > nmsrs = kvm_get_num_msrs_fd(kvm_fd); > list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0])); > @@ -1312,9 +1306,7 @@ struct kvm_cpuid2 *kvm_get_supported_hv_cpuid(void) > return cpuid; > > cpuid = allocate_kvm_cpuid2(); > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > - if (kvm_fd < 0) > - exit(KSFT_SKIP); > + kvm_fd = open_kvm_dev_path_or_exit(); > > ret = ioctl(kvm_fd, KVM_GET_SUPPORTED_HV_CPUID, cpuid); > TEST_ASSERT(ret == 0, "KVM_GET_SUPPORTED_HV_CPUID failed %d %d\n", > diff --git a/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c > index cb953df4d7d0..8aed0db1331d 100644 > --- a/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c > +++ b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c > @@ -37,9 +37,7 @@ static void test_get_msr_index(void) > int old_res, res, kvm_fd, r; > struct kvm_msr_list *list; > > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > - if (kvm_fd < 0) > - exit(KSFT_SKIP); > + kvm_fd = open_kvm_dev_path_or_exit(); > > old_res = kvm_num_index_msrs(kvm_fd, 0); > TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0"); > @@ -101,9 +99,7 @@ static void test_get_msr_feature(void) > int res, old_res, i, kvm_fd; > struct kvm_msr_list *feature_list; > > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > - if (kvm_fd < 0) > - exit(KSFT_SKIP); > + kvm_fd = open_kvm_dev_path_or_exit(); > > old_res = kvm_num_feature_msrs(kvm_fd, 0); > TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0"); > -- > 2.31.1.607.g51e8a6a459-goog > Thanks, drew
On Fri, May 7, 2021 at 1:14 PM Andrew Jones <drjones@redhat.com> wrote: > > On Fri, May 07, 2021 at 07:05:59PM +0000, David Matlack wrote: > > If a KVM selftest is run on a machine without /dev/kvm, it will exit > > silently. Make it easy to tell what's happening by printing an error > > message. > > > > Opportunistically consolidate all codepaths that open /dev/kvm into a > > single function so they all print the same message. > > > > This slightly changes the semantics of vm_is_unrestricted_guest() by > > changing a TEST_ASSERT() to exit(KSFT_SKIP). However > > vm_is_unrestricted_guest() is only called in one place > > (x86_64/mmio_warning_test.c) and that is to determine if the test should > > be skipped or not. > > > > Signed-off-by: David Matlack <dmatlack@google.com> > > --- > > .../testing/selftests/kvm/include/kvm_util.h | 1 + > > tools/testing/selftests/kvm/lib/kvm_util.c | 35 +++++++++++-------- > > .../selftests/kvm/lib/x86_64/processor.c | 16 +++------ > > .../kvm/x86_64/get_msr_index_features.c | 8 ++--- > > 4 files changed, 28 insertions(+), 32 deletions(-) > > > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > > index a8f022794ce3..84982eb02b29 100644 > > --- a/tools/testing/selftests/kvm/include/kvm_util.h > > +++ b/tools/testing/selftests/kvm/include/kvm_util.h > > @@ -77,6 +77,7 @@ struct vm_guest_mode_params { > > }; > > extern const struct vm_guest_mode_params vm_guest_mode_params[]; > > > > +int open_kvm_dev_path_or_exit(void); > > int kvm_check_cap(long cap); > > int vm_enable_cap(struct kvm_vm *vm, struct kvm_enable_cap *cap); > > int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id, > > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > > index fc83f6c5902d..bb7dc65d7fb5 100644 > > --- a/tools/testing/selftests/kvm/lib/kvm_util.c > > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > > @@ -31,6 +31,23 @@ static void *align(void *x, size_t size) > > return (void *) (((size_t) x + mask) & ~mask); > > } > > > > +/* Open KVM_DEV_PATH if available, otherwise exit the entire program. > > + * > > + * Return: > > + * The opened file descriptor of /dev/kvm. > > + */ > > +int open_kvm_dev_path_or_exit(void) { > > + int fd; > > + > > + fd = open(KVM_DEV_PATH, O_RDONLY); > > + if (fd < 0) { > > + print_skip("%s not available", KVM_DEV_PATH); > > + exit(KSFT_SKIP); > > + } > > + > > + return fd; > > +} > > Style issues in the function above '{' and 2 spaces vs. 1 tab. Well that's sure embarrassing! Thanks for catching. > > > + > > /* > > * Capability > > * > > @@ -52,10 +69,7 @@ int kvm_check_cap(long cap) > > int ret; > > int kvm_fd; > > > > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > > - if (kvm_fd < 0) > > - exit(KSFT_SKIP); > > - > > + kvm_fd = open_kvm_dev_path_or_exit(); > > ret = ioctl(kvm_fd, KVM_CHECK_EXTENSION, cap); > > TEST_ASSERT(ret != -1, "KVM_CHECK_EXTENSION IOCTL failed,\n" > > " rc: %i errno: %i", ret, errno); > > @@ -128,9 +142,7 @@ void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size) > > > > static void vm_open(struct kvm_vm *vm, int perm) > > { > > - vm->kvm_fd = open(KVM_DEV_PATH, perm); > > I don't think we should change this one, otherwise the user provided > perms are ignored. Good catch. I don't see any reason to exclude this case, but we do need to pass `perm` down to open_kvm_dev_path_or_exit(). > > > - if (vm->kvm_fd < 0) > > - exit(KSFT_SKIP); > > + vm->kvm_fd = open_kvm_dev_path_or_exit(); > > > > if (!kvm_check_cap(KVM_CAP_IMMEDIATE_EXIT)) { > > print_skip("immediate_exit not available"); > > @@ -925,9 +937,7 @@ static int vcpu_mmap_sz(void) > > { > > int dev_fd, ret; > > > > - dev_fd = open(KVM_DEV_PATH, O_RDONLY); > > - if (dev_fd < 0) > > - exit(KSFT_SKIP); > > + dev_fd = open_kvm_dev_path_or_exit(); > > spaces vs. tab here > > > > > ret = ioctl(dev_fd, KVM_GET_VCPU_MMAP_SIZE, NULL); > > TEST_ASSERT(ret >= sizeof(struct kvm_run), > > @@ -2015,10 +2025,7 @@ bool vm_is_unrestricted_guest(struct kvm_vm *vm) > > > > if (vm == NULL) { > > /* Ensure that the KVM vendor-specific module is loaded. */ > > - f = fopen(KVM_DEV_PATH, "r"); > > - TEST_ASSERT(f != NULL, "Error in opening KVM dev file: %d", > > - errno); > > - fclose(f); > > + close(open_kvm_dev_path_or_exit()); > > spaces > > > } > > > > f = fopen("/sys/module/kvm_intel/parameters/unrestricted_guest", "r"); > > diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c > > index a8906e60a108..efe235044421 100644 > > --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c > > +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c > > @@ -657,9 +657,7 @@ struct kvm_cpuid2 *kvm_get_supported_cpuid(void) > > return cpuid; > > > > cpuid = allocate_kvm_cpuid2(); > > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > > - if (kvm_fd < 0) > > - exit(KSFT_SKIP); > > + kvm_fd = open_kvm_dev_path_or_exit(); > > > > ret = ioctl(kvm_fd, KVM_GET_SUPPORTED_CPUID, cpuid); > > TEST_ASSERT(ret == 0, "KVM_GET_SUPPORTED_CPUID failed %d %d\n", > > @@ -691,9 +689,7 @@ uint64_t kvm_get_feature_msr(uint64_t msr_index) > > > > buffer.header.nmsrs = 1; > > buffer.entry.index = msr_index; > > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > > - if (kvm_fd < 0) > > - exit(KSFT_SKIP); > > + kvm_fd = open_kvm_dev_path_or_exit(); > > > > r = ioctl(kvm_fd, KVM_GET_MSRS, &buffer.header); > > TEST_ASSERT(r == 1, "KVM_GET_MSRS IOCTL failed,\n" > > @@ -986,9 +982,7 @@ struct kvm_msr_list *kvm_get_msr_index_list(void) > > struct kvm_msr_list *list; > > int nmsrs, r, kvm_fd; > > > > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > > - if (kvm_fd < 0) > > - exit(KSFT_SKIP); > > + kvm_fd = open_kvm_dev_path_or_exit(); > > > > nmsrs = kvm_get_num_msrs_fd(kvm_fd); > > list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0])); > > @@ -1312,9 +1306,7 @@ struct kvm_cpuid2 *kvm_get_supported_hv_cpuid(void) > > return cpuid; > > > > cpuid = allocate_kvm_cpuid2(); > > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > > - if (kvm_fd < 0) > > - exit(KSFT_SKIP); > > + kvm_fd = open_kvm_dev_path_or_exit(); > > > > ret = ioctl(kvm_fd, KVM_GET_SUPPORTED_HV_CPUID, cpuid); > > TEST_ASSERT(ret == 0, "KVM_GET_SUPPORTED_HV_CPUID failed %d %d\n", > > diff --git a/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c > > index cb953df4d7d0..8aed0db1331d 100644 > > --- a/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c > > +++ b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c > > @@ -37,9 +37,7 @@ static void test_get_msr_index(void) > > int old_res, res, kvm_fd, r; > > struct kvm_msr_list *list; > > > > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > > - if (kvm_fd < 0) > > - exit(KSFT_SKIP); > > + kvm_fd = open_kvm_dev_path_or_exit(); > > > > old_res = kvm_num_index_msrs(kvm_fd, 0); > > TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0"); > > @@ -101,9 +99,7 @@ static void test_get_msr_feature(void) > > int res, old_res, i, kvm_fd; > > struct kvm_msr_list *feature_list; > > > > - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > > - if (kvm_fd < 0) > > - exit(KSFT_SKIP); > > + kvm_fd = open_kvm_dev_path_or_exit(); > > > > old_res = kvm_num_feature_msrs(kvm_fd, 0); > > TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0"); > > -- > > 2.31.1.607.g51e8a6a459-goog > > > > Thanks, > drew >
On Fri, May 07, 2021 at 01:51:45PM -0700, David Matlack wrote: > > > static void vm_open(struct kvm_vm *vm, int perm) > > > { > > > - vm->kvm_fd = open(KVM_DEV_PATH, perm); > > > > I don't think we should change this one, otherwise the user provided > > perms are ignored. > > Good catch. I don't see any reason to exclude this case, but we do need > to pass `perm` down to open_kvm_dev_path_or_exit(). > I've reviewed v2 and gave it an r-b, since I don't have overly strong opinion about this, but I actually liked that open_kvm_dev_path_or_exit() didn't take any arguments. To handle this case I would have either left it open coded, like it was, or created something like int _open_kvm_dev_path_or_exit(int flags) { int fd = open(KVM_DEV_PATH, flags); if (fd < 0) ...exit skip... return fd; } int open_kvm_dev_path_or_exit(void) { return _open_kvm_dev_path_or_exit(O_RDONLY); } Thanks, drew
On Sun, May 9, 2021 at 11:32 PM Andrew Jones <drjones@redhat.com> wrote: > > On Fri, May 07, 2021 at 01:51:45PM -0700, David Matlack wrote: > > > > static void vm_open(struct kvm_vm *vm, int perm) > > > > { > > > > - vm->kvm_fd = open(KVM_DEV_PATH, perm); > > > > > > I don't think we should change this one, otherwise the user provided > > > perms are ignored. > > > > Good catch. I don't see any reason to exclude this case, but we do need > > to pass `perm` down to open_kvm_dev_path_or_exit(). > > > > I've reviewed v2 and gave it an r-b, since I don't have overly strong > opinion about this, but I actually liked that open_kvm_dev_path_or_exit() > didn't take any arguments. To handle this case I would have either left > it open coded, like it was, or created something like > > int _open_kvm_dev_path_or_exit(int flags) > { > int fd = open(KVM_DEV_PATH, flags); > if (fd < 0) > ...exit skip... > return fd; > } > > int open_kvm_dev_path_or_exit(void) > { > return _open_kvm_dev_path_or_exit(O_RDONLY); > } I agree so long as hiding O_RDONLY does not decrease code readability. I could not find any place in KVM where the R/W permissions on /dev/kvm played a role, so I'm inclined to agree it's better to hide the permissions. I'll send another version with this change along with the comment fix. Thanks. > > Thanks, > drew >
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index a8f022794ce3..84982eb02b29 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -77,6 +77,7 @@ struct vm_guest_mode_params { }; extern const struct vm_guest_mode_params vm_guest_mode_params[]; +int open_kvm_dev_path_or_exit(void); int kvm_check_cap(long cap); int vm_enable_cap(struct kvm_vm *vm, struct kvm_enable_cap *cap); int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id, diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index fc83f6c5902d..bb7dc65d7fb5 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -31,6 +31,23 @@ static void *align(void *x, size_t size) return (void *) (((size_t) x + mask) & ~mask); } +/* Open KVM_DEV_PATH if available, otherwise exit the entire program. + * + * Return: + * The opened file descriptor of /dev/kvm. + */ +int open_kvm_dev_path_or_exit(void) { + int fd; + + fd = open(KVM_DEV_PATH, O_RDONLY); + if (fd < 0) { + print_skip("%s not available", KVM_DEV_PATH); + exit(KSFT_SKIP); + } + + return fd; +} + /* * Capability * @@ -52,10 +69,7 @@ int kvm_check_cap(long cap) int ret; int kvm_fd; - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); - if (kvm_fd < 0) - exit(KSFT_SKIP); - + kvm_fd = open_kvm_dev_path_or_exit(); ret = ioctl(kvm_fd, KVM_CHECK_EXTENSION, cap); TEST_ASSERT(ret != -1, "KVM_CHECK_EXTENSION IOCTL failed,\n" " rc: %i errno: %i", ret, errno); @@ -128,9 +142,7 @@ void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size) static void vm_open(struct kvm_vm *vm, int perm) { - vm->kvm_fd = open(KVM_DEV_PATH, perm); - if (vm->kvm_fd < 0) - exit(KSFT_SKIP); + vm->kvm_fd = open_kvm_dev_path_or_exit(); if (!kvm_check_cap(KVM_CAP_IMMEDIATE_EXIT)) { print_skip("immediate_exit not available"); @@ -925,9 +937,7 @@ static int vcpu_mmap_sz(void) { int dev_fd, ret; - dev_fd = open(KVM_DEV_PATH, O_RDONLY); - if (dev_fd < 0) - exit(KSFT_SKIP); + dev_fd = open_kvm_dev_path_or_exit(); ret = ioctl(dev_fd, KVM_GET_VCPU_MMAP_SIZE, NULL); TEST_ASSERT(ret >= sizeof(struct kvm_run), @@ -2015,10 +2025,7 @@ bool vm_is_unrestricted_guest(struct kvm_vm *vm) if (vm == NULL) { /* Ensure that the KVM vendor-specific module is loaded. */ - f = fopen(KVM_DEV_PATH, "r"); - TEST_ASSERT(f != NULL, "Error in opening KVM dev file: %d", - errno); - fclose(f); + close(open_kvm_dev_path_or_exit()); } f = fopen("/sys/module/kvm_intel/parameters/unrestricted_guest", "r"); diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c index a8906e60a108..efe235044421 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c @@ -657,9 +657,7 @@ struct kvm_cpuid2 *kvm_get_supported_cpuid(void) return cpuid; cpuid = allocate_kvm_cpuid2(); - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); - if (kvm_fd < 0) - exit(KSFT_SKIP); + kvm_fd = open_kvm_dev_path_or_exit(); ret = ioctl(kvm_fd, KVM_GET_SUPPORTED_CPUID, cpuid); TEST_ASSERT(ret == 0, "KVM_GET_SUPPORTED_CPUID failed %d %d\n", @@ -691,9 +689,7 @@ uint64_t kvm_get_feature_msr(uint64_t msr_index) buffer.header.nmsrs = 1; buffer.entry.index = msr_index; - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); - if (kvm_fd < 0) - exit(KSFT_SKIP); + kvm_fd = open_kvm_dev_path_or_exit(); r = ioctl(kvm_fd, KVM_GET_MSRS, &buffer.header); TEST_ASSERT(r == 1, "KVM_GET_MSRS IOCTL failed,\n" @@ -986,9 +982,7 @@ struct kvm_msr_list *kvm_get_msr_index_list(void) struct kvm_msr_list *list; int nmsrs, r, kvm_fd; - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); - if (kvm_fd < 0) - exit(KSFT_SKIP); + kvm_fd = open_kvm_dev_path_or_exit(); nmsrs = kvm_get_num_msrs_fd(kvm_fd); list = malloc(sizeof(*list) + nmsrs * sizeof(list->indices[0])); @@ -1312,9 +1306,7 @@ struct kvm_cpuid2 *kvm_get_supported_hv_cpuid(void) return cpuid; cpuid = allocate_kvm_cpuid2(); - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); - if (kvm_fd < 0) - exit(KSFT_SKIP); + kvm_fd = open_kvm_dev_path_or_exit(); ret = ioctl(kvm_fd, KVM_GET_SUPPORTED_HV_CPUID, cpuid); TEST_ASSERT(ret == 0, "KVM_GET_SUPPORTED_HV_CPUID failed %d %d\n", diff --git a/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c index cb953df4d7d0..8aed0db1331d 100644 --- a/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c +++ b/tools/testing/selftests/kvm/x86_64/get_msr_index_features.c @@ -37,9 +37,7 @@ static void test_get_msr_index(void) int old_res, res, kvm_fd, r; struct kvm_msr_list *list; - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); - if (kvm_fd < 0) - exit(KSFT_SKIP); + kvm_fd = open_kvm_dev_path_or_exit(); old_res = kvm_num_index_msrs(kvm_fd, 0); TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0"); @@ -101,9 +99,7 @@ static void test_get_msr_feature(void) int res, old_res, i, kvm_fd; struct kvm_msr_list *feature_list; - kvm_fd = open(KVM_DEV_PATH, O_RDONLY); - if (kvm_fd < 0) - exit(KSFT_SKIP); + kvm_fd = open_kvm_dev_path_or_exit(); old_res = kvm_num_feature_msrs(kvm_fd, 0); TEST_ASSERT(old_res != 0, "Expecting nmsrs to be > 0");
If a KVM selftest is run on a machine without /dev/kvm, it will exit silently. Make it easy to tell what's happening by printing an error message. Opportunistically consolidate all codepaths that open /dev/kvm into a single function so they all print the same message. This slightly changes the semantics of vm_is_unrestricted_guest() by changing a TEST_ASSERT() to exit(KSFT_SKIP). However vm_is_unrestricted_guest() is only called in one place (x86_64/mmio_warning_test.c) and that is to determine if the test should be skipped or not. Signed-off-by: David Matlack <dmatlack@google.com> --- .../testing/selftests/kvm/include/kvm_util.h | 1 + tools/testing/selftests/kvm/lib/kvm_util.c | 35 +++++++++++-------- .../selftests/kvm/lib/x86_64/processor.c | 16 +++------ .../kvm/x86_64/get_msr_index_features.c | 8 ++--- 4 files changed, 28 insertions(+), 32 deletions(-)