Messages in this thread | | | From | Paolo Bonzini <> | Subject | Re: [PATCH] selftests/kvm: add test for KVM_GET_MSR_FEATURE_INDEX_LIST | Date | Wed, 17 Mar 2021 11:49:43 +0100 |
| |
On 17/03/21 08:45, Emanuele Giuseppe Esposito wrote: > + struct kvm_msr_list features_list; > > buffer.header.nmsrs = 1; > buffer.entry.index = msr_index; > + features_list.nmsrs = 1; > + > kvm_fd = open(KVM_DEV_PATH, O_RDONLY); > if (kvm_fd < 0) > exit(KSFT_SKIP); > > + r = ioctl(kvm_fd, KVM_GET_MSR_FEATURE_INDEX_LIST, &features_list); > + TEST_ASSERT(r < 0 && r != -E2BIG, "KVM_GET_MSR_FEATURE_INDEX_LIST IOCTL failed,\n" > + " rc: %i errno: %i", r, errno);
Careful: because this has nsmrs == 1, you are overwriting an u32 of the stack after struct kvm_msr_list. You need to use your own struct similar to what is done with "buffer.header" and "buffer.entry".
> r = ioctl(kvm_fd, KVM_GET_MSRS, &buffer.header); > TEST_ASSERT(r == 1, "KVM_GET_MSRS IOCTL failed,\n" > " rc: %i errno: %i", r, errno); >
More in general, this is not a test, but rather a library function used to read a single MSR.
If you would like to add a test for KVM_GET_MSR_FEATURE_INDEX_LIST that would be very welcome. That would be a new executable. Looking at the logic for the ioctl, the main purpose of the test should be:
- check that if features_list.nmsrs is too small it will set the nmsrs field and return -E2BIG.
- check that all MSRs returned by KVM_GET_MSR_FEATURE_INDEX_LIST can be accessed with KVM_GET_MSRS
So something like this:
set nmsrs to 0 and try the ioctl check that it returns -E2BIG and has changed nmsrs if nmsrs != 1 { set nmsrs to 1 and try the ioctl again check that it returns -E2BIG } malloc a buffer with room for struct kvm_msr_list and nmsrs indices set nmsrs in the malloc-ed buffer and try the ioctl again for each index invoke kvm_get_feature_msr to read it
(The test should also be skipped if KVM does not expose the KVM_CAP_GET_MSR_FEATURES capability).
Thanks,
Paolo
| |