Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 9 Apr 2024 09:34:12 +0800 | Subject | Re: [PATCH v4 1/9] iommu: Introduce domain attachment handle | From | Baolu Lu <> |
| |
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@nvidia.com> >>>> Signed-off-by: Jason Gunthorpe<jgg@nvidia.com> >>>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com> >>>> --- >>>> 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
| ![\](/images/icornerr.gif) |