All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] iommufd: Use accurate dev_id in the PRI forwarding path
@ 2025-03-05 13:03 Yi Liu
  2025-03-05 17:18 ` Jason Gunthorpe
  0 siblings, 1 reply; 12+ messages in thread
From: Yi Liu @ 2025-03-05 13:03 UTC (permalink / raw)
  To: kevin.tian, baolu.lu, jgg; +Cc: yi.l.liu, iommu

Hi Jason, Kevin, Baolu,

This is more a query, the patch here does not really work as there
is locking issue. So I'd like to hear from you if any good idea.

Detail as below:

Existing code has a problem when the PRI happens on a device that has
alias. The PRI reporting path uses the idev stored in the handle. While
this idev is the first idev that attachs to the domain.  If the PRI
happens on devices other than the first attached device, then the idev
stored in handle does not match.

To solve it, the PRI reporting path needs to loop the attached devices
to get the correct idev and its dev_id. The trouble is that the device_list
is protected by igroup->lock. While the PRI path hold the fault->mutex
lock. This fault->mutex lock might be used in the attach path or the
replace path. Both path would hold igroup->lock first, and then the
fault->mutex if auto_response needed. This will have A-B-B-A locking
issue.

Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 drivers/iommu/iommufd/device.c          | 31 +++++++++++++++++++------
 drivers/iommu/iommufd/fault.c           |  4 +++-
 drivers/iommu/iommufd/iommufd_private.h |  5 +++-
 3 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index bd50146e2ad0..586d008d6bac 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -390,26 +390,26 @@ int iommufd_sw_msi(struct iommu_domain *domain, struct msi_desc *desc,
 
 	handle = to_iommufd_handle(raw_handle);
 	/* No IOMMU_RESV_SW_MSI means no change to the msi_msg */
-	if (handle->idev->igroup->sw_msi_start == PHYS_ADDR_MAX)
+	if (handle->igroup->sw_msi_start == PHYS_ADDR_MAX)
 		return 0;
 
-	ictx = handle->idev->ictx;
+	ictx = handle->igroup->ictx;
 	guard(mutex)(&ictx->sw_msi_lock);
 	/*
 	 * The input msi_addr is the exact byte offset of the MSI doorbell, we
 	 * assume the caller has checked that it is contained with a MMIO region
 	 * that is secure to map at PAGE_SIZE.
 	 */
-	msi_map = iommufd_sw_msi_get_map(handle->idev->ictx,
+	msi_map = iommufd_sw_msi_get_map(ictx,
 					 msi_addr & PAGE_MASK,
-					 handle->idev->igroup->sw_msi_start);
+					 handle->igroup->sw_msi_start);
 	if (IS_ERR(msi_map))
 		return PTR_ERR(msi_map);
 
 	rc = iommufd_sw_msi_install(ictx, hwpt_paging, msi_map);
 	if (rc)
 		return rc;
-	__set_bit(msi_map->id, handle->idev->igroup->required_sw_msi.bitmap);
+	__set_bit(msi_map->id, handle->igroup->required_sw_msi.bitmap);
 
 	iova = msi_map->sw_msi_start + msi_map->pgoff * PAGE_SIZE;
 	msi_desc_set_iommu_msi_iova(desc, iova, PAGE_SHIFT);
@@ -482,6 +482,26 @@ static bool iommufd_device_is_attached(struct iommufd_device *idev)
 	return false;
 }
 
+struct iommufd_device *
+iommufd_group_find_device(struct iommufd_group *igroup, struct device *dev)
+{
+	struct iommufd_device *cur, *idev = NULL;
+
+	/* !!! Headsup !!!
+	 * taking igroup->lock has dead lock with fault->mutex
+	 */
+	mutex_lock(&igroup->lock);
+	list_for_each_entry(cur, &igroup->device_list, group_item) {
+		if (cur->dev == dev) {
+			idev = cur;
+			break;
+		}
+	}
+	mutex_unlock(&igroup->lock);
+
+	return idev;
+}
+
 static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
 				      struct iommufd_device *idev)
 {
@@ -500,7 +517,7 @@ static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
 			goto out_free_handle;
 	}
 
-	handle->idev = idev;
+	handle->igroup = idev->igroup;
 	rc = iommu_attach_group_handle(hwpt->domain, idev->igroup->group,
 				       &handle->handle);
 	if (rc)
@@ -562,7 +579,7 @@ static int iommufd_hwpt_replace_device(struct iommufd_device *idev,
 			goto out_free_handle;
 	}
 
-	handle->idev = idev;
+	handle->igroup = idev->igroup;
 	rc = iommu_replace_group_handle(idev->igroup->group, hwpt->domain,
 					&handle->handle);
 	if (rc)
diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c
index c48d72c9668c..b7e0283d024a 100644
--- a/drivers/iommu/iommufd/fault.c
+++ b/drivers/iommu/iommufd/fault.c
@@ -168,7 +168,11 @@ static ssize_t iommufd_fault_fops_read(struct file *filep, char __user *buf,
 			break;
 		}
-		idev = to_iommufd_handle(group->attach_handle)->idev;
+		/* Or just add a WARN_ON if the idev->dev does not match
+		   group->fault_param->dev ?*/
+		idev = iommufd_group_find_device(
+				to_iommufd_handle(group->attach_handle)->igroup,
+				group->fault_param->dev);
 		list_for_each_entry(iopf, &group->faults, list) {
 			iommufd_compose_fault_message(&iopf->fault,
 						      &data, idev,
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 246297452a44..df0db45deac0 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -433,6 +433,9 @@ iommufd_get_device(struct iommufd_ucmd *ucmd, u32 id)
 			    struct iommufd_device, obj);
 }
 
+struct iommufd_device *
+iommufd_group_find_device(struct iommufd_group *igroup, struct device *dev);
+
 void iommufd_device_destroy(struct iommufd_object *obj);
 int iommufd_get_hw_info(struct iommufd_ucmd *ucmd);
 
@@ -499,7 +502,7 @@ static inline void iommufd_fault_deliver_restore(struct iommufd_fault *fault,
 
 struct iommufd_attach_handle {
 	struct iommu_attach_handle handle;
-	struct iommufd_device *idev;
+	struct iommufd_group *igroup;
 };
 
 /* Convert an iommu attach handle to iommufd handle. */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-03-07  1:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-05 13:03 [RFC] iommufd: Use accurate dev_id in the PRI forwarding path Yi Liu
2025-03-05 17:18 ` Jason Gunthorpe
2025-03-06  3:10   ` Tian, Kevin
2025-03-06  3:19     ` Baolu Lu
2025-03-06  3:30     ` Yi Liu
2025-03-06  4:10       ` Tian, Kevin
2025-03-06  4:55         ` Baolu Lu
2025-03-06  5:01         ` Yi Liu
2025-03-06 12:13           ` Jason Gunthorpe
2025-03-06  7:09       ` Yi Liu
2025-03-06 19:06         ` Jason Gunthorpe
2025-03-07  1:32           ` Baolu Lu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.