[v5] IOMMUFD: Deliver IO page faults to user space

[PATCH v5 3/9] iommu: Add attachment handle to struct iopf_group

Posted by Lu Baolu 1 year, 7 months ago

Previously, the domain that a page fault targets is stored in an
iopf_group, which represents a minimal set of page faults. With the
introduction of attachment handle, replace the domain with the handle
so that the fault handler can obtain more information as needed
when handling the faults.

iommu_report_device_fault() is currently used for SVA page faults,
which handles the page fault in an internal cycle. The domain is retrieved
with iommu_get_domain_for_dev_pasid() if the pasid in the fault message
is valid. This doesn't work in IOMMUFD case, where if the pasid table of
a device is wholly managed by user space, there is no domain attached to
the PASID of the device. Improve this code to try fetching the attachment
handle on a PASID if possible, otherwise try it on the RID.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 include/linux/iommu.h      |  1 +
 drivers/iommu/io-pgfault.c | 34 ++++++++++++++--------------------
 2 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index be9c9a10169d..4ff68d50ae39 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -128,6 +128,7 @@ struct iopf_group {
 	struct list_head pending_node;
 	struct work_struct work;
 	struct iommu_domain *domain;
+	struct iommu_attach_handle *attach_handle;
 	/* The device's fault data parameter. */
 	struct iommu_fault_param *fault_param;
 };
diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
index 06d78fcc79fd..f19215d4ffa2 100644
--- a/drivers/iommu/io-pgfault.c
+++ b/drivers/iommu/io-pgfault.c
@@ -59,28 +59,19 @@ void iopf_free_group(struct iopf_group *group)
 }
 EXPORT_SYMBOL_GPL(iopf_free_group);
 
-static struct iommu_domain *get_domain_for_iopf(struct device *dev,
-						struct iommu_fault *fault)
+static int get_attach_handle_for_iopf(struct device *dev, ioasid_t pasid,
+				      struct iopf_group *group)
 {
-	struct iommu_domain *domain;
+	struct iommu_attach_handle *handle;
 
-	if (fault->prm.flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID) {
-		domain = iommu_get_domain_for_dev_pasid(dev, fault->prm.pasid, 0);
-		if (IS_ERR(domain))
-			domain = NULL;
-	} else {
-		domain = iommu_get_domain_for_dev(dev);
-	}
+	handle = iommu_attach_handle_get(dev->iommu_group, pasid, 0);
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
 
-	if (!domain || !domain->iopf_handler) {
-		dev_warn_ratelimited(dev,
-			"iopf (pasid %d) without domain attached or handler installed\n",
-			 fault->prm.pasid);
+	group->attach_handle = handle;
+	group->domain = handle->domain;
 
-		return NULL;
-	}
-
-	return domain;
+	return 0;
 }
 
 /* Non-last request of a group. Postpone until the last one. */
@@ -206,8 +197,11 @@ void iommu_report_device_fault(struct device *dev, struct iopf_fault *evt)
 	if (group == &abort_group)
 		goto err_abort;
 
-	group->domain = get_domain_for_iopf(dev, fault);
-	if (!group->domain)
+	if (!(fault->prm.flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID) ||
+	    get_attach_handle_for_iopf(dev, fault->prm.pasid, group))
+		get_attach_handle_for_iopf(dev, IOMMU_NO_PASID, group);
+
+	if (!group->attach_handle || !group->domain->iopf_handler)
 		goto err_abort;
 
 	/*
-- 
2.34.1

RE: [PATCH v5 3/9] iommu: Add attachment handle to struct iopf_group

Posted by Tian, Kevin 1 year, 7 months ago

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Tuesday, April 30, 2024 10:57 PM
> 
> Previously, the domain that a page fault targets is stored in an
> iopf_group, which represents a minimal set of page faults. With the
> introduction of attachment handle, replace the domain with the handle

It's better to use 'attach handle' as the code does.

> +	handle = iommu_attach_handle_get(dev->iommu_group, pasid, 0);
> +	if (IS_ERR(handle))
> +		return PTR_ERR(handle);
> 
> -	if (!domain || !domain->iopf_handler) {
> -		dev_warn_ratelimited(dev,
> -			"iopf (pasid %d) without domain attached or handler
> installed\n",
> -			 fault->prm.pasid);
> +	group->attach_handle = handle;
> +	group->domain = handle->domain;

this change also removes the warning message. Is it desired?