Re: [PATCH] selftests/kvm: add test for KVM_GET_MSR_FEATURE_INDEX_LIST

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 17/03/2021 11:49, Paolo Bonzini wrote:
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).

Thank you for the feedback, the title is indeed a little bit misleading. My idea in this patch was to just add an additional check to all usages of KVM_GET_MSRS, since KVM_GET_MSR_FEATURE_INDEX_LIST is used only to probe host capabilities and processor features.
But you are right, a separate test would be better.

Thank you,
Emanuele




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux