Re: [PATCH v4 09/19] vfio/pci: Accept device fd for hot reset

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

 



> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Friday, February 24, 2023 10:36 AM
> 
> On Fri, Feb 24, 2023 at 02:21:33AM +0000, Tian, Kevin wrote:
> 
> > Yi, while you are incorporating this change please also update the
> > uapi header. Rename 'group_fds[]' to 'fds[]' and add comment to
> > explain that it could be an array of group fds or a single iommufd.
> 
> Upon reflection we can probably make it even simpler and just have a 0
> length fd array mean to use the iommufd the vfio_device is already
> associated with
> 
> And the check for correctness can be simplified to simply see if each
> vfio_device in the dev_set is attached to the same iommufd ctx
> pointer instead of searching the xarray.

Sorry, it appears to me the below concern is not solved as above logic
still requires userspace to open and bind devices to the same iommufd.

"
  > Say a scenario where group happens to overlap with devset. Let's say
  > two devices in the group/devset.
  > 
  > An existing deployment assigns only dev1 to Qemu. In this case dev1
  > is resettable via group fd given dev2 cannot be opened by another
  > user.
"

Thus, I think we still need to search the xarray to check if a device is
bound to iommufd or not. And this check needs to be more relaxed.
E.g. dev1 is bound to iommufd, but dev2 has not. However, they have
the same group, so dev2 should be considered to be "bound" as well.
When 0 length fd is used, vfio just gets the iommufd_ctx from the device
that is to be reset, then check if all other devices in the dev_set are
considered as bound with the below interface.

+/**
+ * iommufd_ctx_has_group_for_device - True if any alias of struct device
+					is bound to this ictx
+ * @ictx: iommufd file descriptor
+ * @dev: Pointer to a physical device struct
+ *
+ * True if a iommufd_device_bind() is present for any alias of this dev
+ */
+bool iommufd_ctx_has_group_for_device(struct iommufd_ctx *ictx, struct device *dev)
+{
+	unsigned long index;
+	struct iommu_group *group;
+	struct iommufd_object *obj;
+
+	if (!ictx)
+		return false;
+
+	group = iommu_group_get(dev);
+	if (!group)
+		return false;
+
+	xa_lock(&ictx->objects);
+	xa_for_each(&ictx->objects, index, obj) {
+		if (obj->type == IOMMUFD_OBJ_DEVICE)  &&
+		    container_of(obj, struct iommufd_device, obj)->group == group) {
+			xa_unlock(&ictx->objects);
+			iommu_group_put(group);
+			return true;
+		}
+	}
+	xa_unlock(&ictx->objects);
+	iommu_group_put(group);
+	return false;
+}
+EXPORT_SYMBOL_NS_GPL(iommufd_ctx_has_group_for_device, IOMMUFD);

Regards
Yi Liu




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux