Skip to content

Commit

Permalink
iommu: Keep dev->iommu state consistent
Browse files Browse the repository at this point in the history
At the moment, if of_iommu_configure() allocates dev->iommu itself via
iommu_fwspec_init(), then suffers a DT parsing failure, it cleans up the
fwspec but leaves the empty dev_iommu hanging around. So far this is
benign (if a tiny bit wasteful), but we'd like to be able to reason
about dev->iommu having a consistent and unambiguous lifecycle. Thus
make sure that the of_iommu cleanup undoes precisely whatever it did.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/r/d219663a3f23001f23d520a883ac622d70b4e642.1740753261.git.robin.murphy@arm.com
Signed-off-by: Joerg Roedel <jroedel@suse.de>
  • Loading branch information
Robin Murphy authored and Joerg Roedel committed Mar 11, 2025
1 parent fd598f7 commit 3832862
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 2 deletions.
2 changes: 2 additions & 0 deletions drivers/iommu/iommu-priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
return dev->iommu->iommu_dev->ops;
}

void dev_iommu_free(struct device *dev);

const struct iommu_ops *iommu_ops_from_fwnode(const struct fwnode_handle *fwnode);

static inline const struct iommu_ops *iommu_fwspec_ops(struct iommu_fwspec *fwspec)
Expand Down
2 changes: 1 addition & 1 deletion drivers/iommu/iommu.c
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ static struct dev_iommu *dev_iommu_get(struct device *dev)
return param;
}

static void dev_iommu_free(struct device *dev)
void dev_iommu_free(struct device *dev)
{
struct dev_iommu *param = dev->iommu;

Expand Down
6 changes: 5 additions & 1 deletion drivers/iommu/of_iommu.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ static void of_pci_check_device_ats(struct device *dev, struct device_node *np)
int of_iommu_configure(struct device *dev, struct device_node *master_np,
const u32 *id)
{
bool dev_iommu_present;
int err;

if (!master_np)
Expand All @@ -127,6 +128,7 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
mutex_unlock(&iommu_probe_device_lock);
return 0;
}
dev_iommu_present = dev->iommu;

/*
* We don't currently walk up the tree looking for a parent IOMMU.
Expand All @@ -147,8 +149,10 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
err = of_iommu_configure_device(master_np, dev, id);
}

if (err)
if (err && dev_iommu_present)
iommu_fwspec_free(dev);
else if (err && dev->iommu)
dev_iommu_free(dev);
mutex_unlock(&iommu_probe_device_lock);

if (!err && dev->bus)
Expand Down

0 comments on commit 3832862

Please sign in to comment.