Re: [PATCH v4 1/9] iommu: Introduce domain attachment handle

From: Baolu Lu
Date: Mon Apr 08 2024 - 21:35:31 EST


On 4/8/24 10:05 PM, Jason Gunthorpe wrote:
On Sat, Apr 06, 2024 at 12:34:14PM +0800, Baolu Lu wrote:
On 4/3/24 7:58 PM, Jason Gunthorpe wrote:
On Wed, Apr 03, 2024 at 09:15:11AM +0800, Lu Baolu wrote:
Currently, when attaching a domain to a device or its PASID, domain is
stored within the iommu group. It could be retrieved for use during the
window between attachment and detachment.

With new features introduced, there's a need to store more information
than just a domain pointer. This information essentially represents the
association between a domain and a device. For example, the SVA code
already has a custom struct iommu_sva which represents a bond between
sva domain and a PASID of a device. Looking forward, the IOMMUFD needs
a place to store the iommufd_device pointer in the core, so that the
device object ID could be quickly retrieved in the critical fault handling
path.

Introduce domain attachment handle that explicitly represents the
attachment relationship between a domain and a device or its PASID.
A caller-specific data field can be used by the caller to store additional
information beyond a domain pointer, depending on its specific use case.

Co-developed-by: Jason Gunthorpe<jgg@xxxxxxxxxx>
Signed-off-by: Jason Gunthorpe<jgg@xxxxxxxxxx>
Signed-off-by: Lu Baolu<baolu.lu@xxxxxxxxxxxxxxx>
---
drivers/iommu/iommu-priv.h | 9 +++
drivers/iommu/iommu.c | 158 +++++++++++++++++++++++++++++++++----
2 files changed, 153 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
index 5f731d994803..08c0667cef54 100644
--- a/drivers/iommu/iommu-priv.h
+++ b/drivers/iommu/iommu-priv.h
@@ -28,4 +28,13 @@ void iommu_device_unregister_bus(struct iommu_device *iommu,
const struct bus_type *bus,
struct notifier_block *nb);
+struct iommu_attach_handle {
+ struct iommu_domain *domain;
+ refcount_t users;
I don't understand how the refcounting can be generally useful. There
is no way to free this:

+ void *priv;
When the refcount goes to zero.
This field is set by the caller, so the caller ensures that the pointer
can only be freed after iommu domain detachment. For iopf, the caller
should automatically respond to all outstanding iopf's in the domain
detach path.

In the sva case, which uses the workqueue to handle iopf,
flush_workqueue() is called in the domain detach path to ensure that all
outstanding iopf's are completed before detach completion.
Which is back to what is the point of the refcount at all?

Yeah, refcount is not generally useful. It's context-specific, so it
needs to move out of the core.

The SVA code needs refcount because it allows multiple attachments
between a SVA domain and a PASID. This is not a common case.


+static void iommufd_auto_response_handle(struct iommufd_fault *fault,
+ struct iommu_attach_handle *handle)
+{
+ struct iommufd_device *idev = handle->priv;
The caller already has the iommufd_device, don't need the handler.

Yes.

Best regards,
baolu