Re: [PATCH v18 08/18] s390/vfio-ap: allow assignment of unavailable AP queues to mdev device

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

 





On 3/3/22 10:39, Jason J. Herne wrote:
On 2/14/22 19:50, Tony Krowiak wrote:
  /**
- * vfio_ap_mdev_verify_no_sharing - verifies that the AP matrix is not configured + * vfio_ap_mdev_verify_no_sharing - verify APQNs are not shared by matrix mdevs
   *
- * @matrix_mdev: the mediated matrix device
+ * @mdev_apm: mask indicating the APIDs of the APQNs to be verified
+ * @mdev_aqm: mask indicating the APQIs of the APQNs to be verified
   *
- * Verifies that the APQNs derived from the cross product of the AP adapter IDs - * and AP queue indexes comprising the AP matrix are not configured for another + * Verifies that each APQN derived from the Cartesian product of a bitmap of
+ * AP adapter IDs and AP queue indexes is not configured for any matrix
   * mediated device. AP queue sharing is not allowed.
   *
- * Return: 0 if the APQNs are not shared; otherwise returns -EADDRINUSE.
+ * Return: 0 if the APQNs are not shared; otherwise return -EADDRINUSE.
   */
-static int vfio_ap_mdev_verify_no_sharing(struct ap_matrix_mdev *matrix_mdev)
+static int vfio_ap_mdev_verify_no_sharing(unsigned long *mdev_apm,
+                      unsigned long *mdev_aqm)
  {
-    struct ap_matrix_mdev *lstdev;
+    struct ap_matrix_mdev *matrix_mdev;
      DECLARE_BITMAP(apm, AP_DEVICES);
      DECLARE_BITMAP(aqm, AP_DOMAINS);
  -    list_for_each_entry(lstdev, &matrix_dev->mdev_list, node) {
-        if (matrix_mdev == lstdev)
+    list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
+        /*
+         * If the input apm and aqm belong to the matrix_mdev's matrix,
+         * then move on to the next.
+         */
+        if (mdev_apm == matrix_mdev->matrix.apm &&
+            mdev_aqm == matrix_mdev->matrix.aqm)
              continue;

We may have a problem here. This check seems like it exists to stop you from comparing an mdev's apm/aqm with itself. Obviously comparing an mdev's newly updated apm/aqm with itself would cause a false positive sharing check, right? If this is the case, I think the comment should be changed to reflect that.

You are correct, this check is performed to prevent comparing an mdev to
itself, I'll improve the comment.


Aside from the comment, what stops this particular series of if statements from allowing us to configure a second mdev with the exact same apm/aqm values as an existing mdev? If we do, then this check's continue will short circuit the rest
of the function thereby allowing that 2nd mdev even though it should be a
sharing violation.

I don't see how this is possible.

The function above is called from two places: the vfio_ap_mdev_validate_masks() function which is invoked when an adapter or domain is assigned to the mdev; and the vfio_ap_mdev_resource_in_use() function which is a callback registered with the
AP bus and is called by the bus when the apmask/aqmask are changed.

In the former case, the addresses passed in are from the apm/aqm fields within the ap_matrix structure. Each ap_matrix structure is a field contained within an ap_matrix_mdev structure, it is not a pointer to storage allocated external to
the matrix_mdev, so the apm/aqm addresses passed in from the
vfio_ap_mdev_validate_masks() function will be unique to each ap_matrix_mdev
structure.

In the latter case, the addresses are passed in by the AP bus and are allocated by the bus and would definitely not be contained within an ap_matrix_mdev since the AP bus
doesn't even have access to that structure.







[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