| Message ID | 8-v1-8aecc628b904+2f42-iommu_probe_jgg@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Consolidate the probe_device path | expand |
> From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Thursday, April 20, 2023 12:12 AM > > Have release fully clean up the iommu related parts of the struct device, > no matter what state they are in. > > POWER creates iommu_groups without drivers attached, and the next patch > removes the open-coding of this same cleanup from POWER. > > Split the logic so that the three things owned by the iommu core are > always cleaned up: > - Any attached iommu_group > - Any allocated dev->iommu, eg for fwsepc > - Any attached driver via a struct group_device > > This fixes a bug where a fwspec created without an iommu_group being > probed would not be freed. > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On Thu, Apr 20, 2023 at 12:23:11PM +0800, Baolu Lu wrote: > On 4/20/23 12:11 AM, Jason Gunthorpe wrote: > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > index dbaf3ed9012c45..a82516c8ea87ad 100644 > > --- a/drivers/iommu/iommu.c > > +++ b/drivers/iommu/iommu.c > > @@ -569,7 +569,6 @@ static void __iommu_group_remove_device(struct device *dev) > > dev->iommu_group = NULL; > > goto out; > > Nit, given that below line has been removed, can above simply be a > loop break? Yes, that is much nicer Thanks, Jason
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index dbaf3ed9012c45..a82516c8ea87ad 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -569,7 +569,6 @@ static void __iommu_group_remove_device(struct device *dev) dev->iommu_group = NULL; goto out; } - WARN(true, "Corrupted iommu_group device_list"); out: mutex_unlock(&group->mutex); @@ -581,10 +580,12 @@ static void iommu_release_device(struct device *dev) { struct iommu_group *group = dev->iommu_group; - if (!dev->iommu || !group) - return; + if (group) + __iommu_group_remove_device(dev); - __iommu_group_remove_device(dev); + /* Free any fwspec if no iommu_driver was ever attached */ + if (dev->iommu) + dev_iommu_free(dev); } static int __init iommu_set_def_domain_type(char *str)
Have release fully clean up the iommu related parts of the struct device, no matter what state they are in. POWER creates iommu_groups without drivers attached, and the next patch removes the open-coding of this same cleanup from POWER. Split the logic so that the three things owned by the iommu core are always cleaned up: - Any attached iommu_group - Any allocated dev->iommu, eg for fwsepc - Any attached driver via a struct group_device This fixes a bug where a fwspec created without an iommu_group being probed would not be freed. Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/iommu/iommu.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)