On 2022/3/19 01:27, Jason Gunthorpe wrote:

+
+/**
+ * iommufd_device_attach - Connect a device to an iommu_domain
+ * @idev: device to attach
+ * @pt_id: Input a IOMMUFD_OBJ_IOAS, or IOMMUFD_OBJ_HW_PAGETABLE
+ *         Output the IOMMUFD_OBJ_HW_PAGETABLE ID
+ * @flags: Optional flags
+ *
+ * This connects the device to an iommu_domain, either automatically or 
manually
+ * selected. Once this completes the device could do DMA.
+ *
+ * The caller should return the resulting pt_id back to userspace.
+ * This function is undone by calling iommufd_device_detach().
+ */
+int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id,
+                         unsigned int flags)
+{
+       struct iommufd_hw_pagetable *hwpt;
+       int rc;
+
+       refcount_inc(&idev->obj.users);
+
+       hwpt = iommufd_hw_pagetable_from_id(idev->ictx, *pt_id, idev->dev);
+       if (IS_ERR(hwpt)) {
+               rc = PTR_ERR(hwpt);
+               goto out_users;
+       }
+
+       mutex_lock(&hwpt->devices_lock);
+       /* FIXME: Use a device-centric iommu api. For now check if the
+        * hw_pagetable already has a device of the same group joined to tell if
+        * we are the first and need to attach the group. */
+       if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
+               phys_addr_t sw_msi_start = 0;
+
+               rc = iommu_attach_group(hwpt->domain, idev->group);
+               if (rc)
+                       goto out_unlock;
+
+               /*
+                * hwpt is now the exclusive owner of the group so this is the
+                * first time enforce is called for this group.
+                */
+               rc = iopt_table_enforce_group_resv_regions(
+                       &hwpt->ioas->iopt, idev->group, &sw_msi_start);
+               if (rc)
+                       goto out_detach;
+               rc = iommufd_device_setup_msi(idev, hwpt, sw_msi_start, flags);
+               if (rc)
+                       goto out_iova;
+       }
+
+       idev->hwpt = hwpt;

could the below list_empty check be moved to the above "if branch"? If
above "if branch" is false, that means there is already group attached with
the hwpt->domain. So the hwpt->devices should be non-empty. Only if the above "if branch" is true, should the hwpt->devices possible to be empty.
So moving it into above "if branch" may be better?

+       if (list_empty(&hwpt->devices)) {
+               rc = iopt_table_add_domain(&hwpt->ioas->iopt, hwpt->domain);
+               if (rc)
+                       goto out_iova;
+       }
+       list_add(&idev->devices_item, &hwpt->devices);
+       mutex_unlock(&hwpt->devices_lock);
+
+       *pt_id = idev->hwpt->obj.id;
+       return 0;
+
+out_iova:
+       iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->group);
+out_detach:
+       iommu_detach_group(hwpt->domain, idev->group);
+out_unlock:
+       mutex_unlock(&hwpt->devices_lock);
+       iommufd_hw_pagetable_put(idev->ictx, hwpt);
+out_users:
+       refcount_dec(&idev->obj.users);
+       return rc;
+}
+EXPORT_SYMBOL_GPL(iommufd_device_attach);

--
Regards,
Yi Liu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to