On 2024/9/30 15:44, Tian, Kevin wrote:
From: Yi Liu <yi.l.liu@xxxxxxxxx>
Sent: Thursday, September 12, 2024 9:13 PM
iommufd plans to always pass in an iommu_attach_handle to the iommu
core, so it's no longer fault specific, hence move the helpers out
of the fault.c
again please put the reason for why iommufd plans to do so.
how about the below?
The iommu_attach_handle tracks the attached domains in the
group->pasid_array, it is optional so far. But to support the
domain replacement for a pasid, it needs to get the attached
domain, so making iommufd always pass a handle is a choice.
Before that, we would need to decouple the handle allocation
and fault code as non-fault capable hwpt attach would also
allocate handle.
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -458,6 +458,63 @@ struct iommufd_attach_handle {
/* Convert an iommu attach handle to iommufd handle. */
#define to_iommufd_handle(hdl) container_of(hdl, struct
iommufd_attach_handle, handle)
+static inline struct iommufd_attach_handle *
+iommufd_device_get_attach_handle(struct iommufd_device *idev)
+{
+ struct iommu_attach_handle *handle;
+
+ handle = iommu_attach_handle_get(idev->igroup->group,
IOMMU_NO_PASID, 0);
+ if (IS_ERR(handle))
+ return NULL;
+
+ return to_iommufd_handle(handle);
+}
+
+static inline int iommufd_dev_attach_handle(struct
iommufd_hw_pagetable *hwpt,
+ struct iommufd_device *idev)
+{
+ struct iommufd_attach_handle *handle;
+ int ret;
+
+ handle = kzalloc(sizeof(*handle), GFP_KERNEL);
+ if (!handle)
+ return -ENOMEM;
+
+ handle->idev = idev;
+ ret = iommu_attach_group_handle(hwpt->domain, idev->igroup-
group,
+ &handle->handle);
+ if (ret)
+ kfree(handle);
+
+ return ret;
+}
+
+/* Caller to free the old iommufd_attach_handle */
+static inline struct iommufd_attach_handle *
+iommufd_dev_replace_handle(struct iommufd_device *idev,
+ struct iommufd_hw_pagetable *hwpt,
+ struct iommufd_hw_pagetable *old)
+{
+ struct iommufd_attach_handle *handle, *curr;
+ int ret;
+
+ curr = iommufd_device_get_attach_handle(idev);
+
+ handle = kzalloc(sizeof(*handle), GFP_KERNEL);
+ if (!handle)
+ return ERR_PTR(-ENOMEM);
+
+ handle->idev = idev;
+ ret = iommu_replace_group_handle(idev->igroup->group,
+ hwpt->domain, &handle->handle);
+ if (ret) {
+ kfree(handle);
+ return ERR_PTR(ret);
+ }
+
+ return curr;
+}
+
why putting them in header file instead of C file?
put it in device.c is also fine. :) but not in the fault.c as the non
fault path also needs to use handle.
--
Regards,
Yi Liu
