Re: [PATCH 03/10] iommu: Add generic_single_device_group()
From: Baolu Lu <baolu.lu@linux.intel.com>
Date: 2023-07-20 07:40:01
Also in:
linux-iommu, linux-rockchip, linux-sunxi
On 2023/7/19 3:05, Jason Gunthorpe wrote:
quoted hunk ↗ jump to hunk
This implements the common pattern seen in drivers of a single iommu_group for the entire iommu driver. Implement this in core code so the drivers that want this can select it from their ops. Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/iommu/iommu.c | 25 +++++++++++++++++++++++++ include/linux/iommu.h | 3 +++ 2 files changed, 28 insertions(+)diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 9e41ad4e3219b6..1e0c5d9a0370fb 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c@@ -289,6 +289,9 @@ void iommu_device_unregister(struct iommu_device *iommu) spin_lock(&iommu_device_lock); list_del(&iommu->list); spin_unlock(&iommu_device_lock); + + /* Pairs with the alloc in generic_single_device_group() */ + iommu_group_put(iommu->singleton_group); } EXPORT_SYMBOL_GPL(iommu_device_unregister);@@ -1595,6 +1598,28 @@ struct iommu_group *generic_device_group(struct device *dev) } EXPORT_SYMBOL_GPL(generic_device_group); +/* + * Generic device_group call-back function. It just allocates one + * iommu-group per iommu driver. + */ +struct iommu_group *generic_single_device_group(struct device *dev) +{ + struct iommu_device *iommu = dev->iommu->iommu_dev; + + lockdep_assert_held(&dev_iommu_group_lock); + + if (!iommu->singleton_group) { + struct iommu_group *group; + + group = iommu_group_alloc(); + if (IS_ERR(group)) + return group; + iommu->singleton_group = group; + } + return iommu_group_ref_get(iommu->singleton_group); +} +EXPORT_SYMBOL_GPL(generic_single_device_group);
When allocating the singleton group for the first time, the group's
refcount is taken twice. This can cause memory leaks even after calling
iommu_device_unregister(). Perhaps it can be adjusted as follows?
struct iommu_group *generic_single_device_group(struct device *dev)
{
struct iommu_device *iommu = dev->iommu->iommu_dev;
struct iommu_group *group;
lockdep_assert_held(&dev_iommu_group_lock);
if (iommu->singleton_group)
return iommu_group_ref_get(iommu->singleton_group);
group = iommu_group_alloc();
if (!IS_ERR(group))
iommu->singleton_group = group;
return group;
}
quoted hunk ↗ jump to hunk
+ /* * Use standard PCI bus topology, isolation features, and DMA alias quirks * to find or create an IOMMU group for a device.diff --git a/include/linux/iommu.h b/include/linux/iommu.h index b1dcb1b9b17040..f1e18e81fca78b 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h@@ -361,6 +361,7 @@ struct iommu_domain_ops { * @list: Used by the iommu-core to keep a list of registered iommus * @ops: iommu-ops for talking to this iommu * @dev: struct device for sysfs handling + * @singleton_group: Used internally for drivers that have only one group * @max_pasids: number of supported PASIDs */ struct iommu_device {@@ -368,6 +369,7 @@ struct iommu_device { const struct iommu_ops *ops; struct fwnode_handle *fwnode; struct device *dev; + struct iommu_group *singleton_group; u32 max_pasids; };@@ -640,6 +642,7 @@ extern struct iommu_group *pci_device_group(struct device *dev); extern struct iommu_group *generic_device_group(struct device *dev); /* FSL-MC device grouping function */ struct iommu_group *fsl_mc_device_group(struct device *dev); +extern struct iommu_group *generic_single_device_group(struct device *dev);
"extern" is not necessary.
/** * struct iommu_fwspec - per-device IOMMU instance data
Best regards, baolu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel