[PATCH v2 17/17] iommu: Mark dev_iommu_priv_set() with a lockdep

Jason Gunthorpe
2023-11-15T14:06:39Z

A perfect driver would only call dev_iommu_priv_set() from its probe
callback. We've made it functionally correct to call it from the of_xlate
by adding a lock around that call.

lockdep assert that iommu_probe_device_lock is held to discourage misuse.

Exclude PPC kernels with CONFIG_FSL_PAMU turned on because FSL_PAMU uses a
global static for its priv and abuses priv for its domain.

Remove the pointless stores of NULL, all these are on paths where the core
code will free dev->iommu after the op returns.

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
drivers/iommu/amd/iommu.c | 2 --
drivers/iommu/apple-dart.c | 1 -
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 -
drivers/iommu/arm/arm-smmu/arm-smmu.c | 1 -
drivers/iommu/intel/iommu.c | 2 --
drivers/iommu/iommu.c | 9 +++++++++
drivers/iommu/omap-iommu.c | 1 -
include/linux/iommu.h | 5 +----
8 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index fcc987f5d4edc3..8199c678c2dc2a 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -551,8 +551,6 @@ static void amd_iommu_uninit_device(struct device *dev)
if (dev_data->domain)
detach_device(dev);

- dev_iommu_priv_set(dev, NULL);
-
/*
* We keep dev_data around for unplugged devices and reuse it when the
* device is re-plugged - not doing so would introduce a ton of races.
diff --git a/drivers/iommu/apple-dart.c b/drivers/iommu/apple-dart.c
index ee05f4824bfad1..56cfc33042e0b5 100644
--- a/drivers/iommu/apple-dart.c
+++ b/drivers/iommu/apple-dart.c
@@ -740,7 +740,6 @@ static void apple_dart_release_device(struct device *dev)
{
struct apple_dart_master_cfg *cfg = dev_iommu_priv_get(dev);

- dev_iommu_priv_set(dev, NULL);
kfree(cfg);
}

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index b1309f04ebc0d9..df81fcd25a75b0 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2698,7 +2698,6 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)

err_free_master:
kfree(master);
- dev_iommu_priv_set(dev, NULL);
return ERR_PTR(ret);
}

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 8c4a60d8e5d522..6fc040a4168aa3 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1423,7 +1423,6 @@ static void arm_smmu_release_device(struct device *dev)

arm_smmu_rpm_put(cfg->smmu);

- dev_iommu_priv_set(dev, NULL);
kfree(cfg);
}

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 3531b956556c7d..4a5792888e6433 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4457,7 +4457,6 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
ret = intel_pasid_alloc_table(dev);
if (ret) {
dev_err(dev, "PASID table allocation failed\n");
- dev_iommu_priv_set(dev, NULL);
kfree(info);
return ERR_PTR(ret);
}
@@ -4475,7 +4474,6 @@ static void intel_iommu_release_device(struct device *dev)
dmar_remove_one_dev_info(dev);
intel_pasid_free_table(dev);
intel_iommu_debugfs_remove_dev(info);
- dev_iommu_priv_set(dev, NULL);
kfree(info);
set_dma_ops(dev, NULL);
}
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 34c4b07a6aafae..fbfb9ba4da0ee2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -387,6 +387,15 @@ static u32 dev_iommu_get_max_pasids(struct device *dev)
return min_t(u32, max_pasids, dev->iommu->iommu_dev->max_pasids);
}

+void dev_iommu_priv_set(struct device *dev, void *priv)
+{
+ /* FSL_PAMU does something weird */
+ if (!IS_ENABLED(CONFIG_FSL_PAMU))
+ lockdep_assert_held(&iommu_probe_device_lock);
+ dev->iommu->priv = priv;
+}
+EXPORT_SYMBOL_GPL(dev_iommu_priv_set);
+
/*
* Init the dev->iommu and dev->iommu_group in the struct device and get the
* driver probed. Take ownership of fwspec, it always freed on error
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index c66b070841dd41..c9528065a59afa 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1719,7 +1719,6 @@ static void omap_iommu_release_device(struct device *dev)
if (!dev->of_node || !arch_data)
return;

- dev_iommu_priv_set(dev, NULL);
kfree(arch_data);

}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 3495db0c3e4631..8be153a54c8ca2 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -852,10 +852,7 @@ static inline void *dev_iommu_priv_get(struct device *dev)
return NULL;
}

-static inline void dev_iommu_priv_set(struct device *dev, void *priv)
-{
- dev->iommu->priv = priv;
-}
+void dev_iommu_priv_set(struct device *dev, void *priv);

int iommu_probe_device_fwspec(struct device *dev, struct iommu_fwspec *fwspec);
static inline int iommu_probe_device(struct device *dev)