Re: [PATCH v2 00/10] Refine the locking for dev->iommu_group

Jason Gunthorpe
2023-08-08T13:25:44Z

On Tue, Aug 08, 2023 at 03:08:30PM +0200, Marek Szyprowski wrote:
> > Any of the drivers that use platform device as the iommu_device will
> > have a problem, please try:
> >
> > https://lore.kernel.org/linux-iommu/ZNIz%2FNVLb6WqqvQx@nvidia.com/
>
> I've checked and it doesn't help in my case. I will soon check why.

Oh, I botched it. Forgot that the iommu_device->dev is the sysfs
handle not the HW device. Maybe this:

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 7dbbcffac21930..dbaace482861da 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -264,6 +264,7 @@ int iommu_device_register(struct iommu_device *iommu,
return -EBUSY;

iommu->ops = ops;
+ iommu->hwdev = hwdev;
if (hwdev)
iommu->fwnode = dev_fwnode(hwdev);

@@ -273,7 +274,7 @@ int iommu_device_register(struct iommu_device *iommu,

for (int i = 0; i < ARRAY_SIZE(iommu_buses) && !err; i++) {
iommu_buses[i]->iommu_ops = ops;
- err = bus_iommu_probe(iommu_buses[i]);
+ err = bus_iommu_probe(iommu_buses[i], iommu);
}
if (err)
iommu_device_unregister(iommu);
@@ -1784,12 +1785,21 @@ struct iommu_domain *iommu_group_default_domain(struct iommu_group *group)
return group->default_domain;
}

+struct probe_iommu_args {
+ struct list_head *group_list;
+ struct iommu_device *iommu;
+};
+
static int probe_iommu_group(struct device *dev, void *data)
{
- struct list_head *group_list = data;
+ struct probe_iommu_args *args = data;
int ret;

- ret = __iommu_probe_device(dev, group_list);
+ /* We never probe the iommu device itself */
+ if (args->iommu && args->iommu->hwdev == dev)
+ return 0;
+
+ ret = __iommu_probe_device(dev, args->group_list);
if (ret == -ENODEV)
ret = 0;

@@ -1858,13 +1868,16 @@ static void iommu_group_do_probe_finalize(struct device *dev)
ops->probe_finalize(dev);
}

-int bus_iommu_probe(const struct bus_type *bus)
+int bus_iommu_probe(const struct bus_type *bus, struct iommu_device *iommu)
{
struct iommu_group *group, *next;
+ struct probe_iommu_args args = {};
LIST_HEAD(group_list);
int ret;

- ret = bus_for_each_dev(bus, NULL, &group_list, probe_iommu_group);
+ args.group_list = &group_list;
+ args.iommu = iommu;
+ ret = bus_for_each_dev(bus, NULL, &args, probe_iommu_group);
if (ret)
return ret;

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 97c45f50bf4332..e8ab7cb832ba37 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1234,6 +1234,9 @@ static int omap_iommu_probe(struct platform_device *pdev)
if (err)
goto out_sysfs;
obj->has_iommu_driver = true;
+ } else {
+ /* Re-probe bus to probe device attached to this IOMMU */
+ bus_iommu_probe(&platform_bus_type, NULL);
}

pm_runtime_enable(obj->dev);
@@ -1242,9 +1245,6 @@ static int omap_iommu_probe(struct platform_device *pdev)

dev_info(&pdev->dev, "%s registered\n", obj->name);

- /* Re-probe bus to probe device attached to this IOMMU */
- bus_iommu_probe(&platform_bus_type);
-
return 0;

out_sysfs:
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f1e18e81fca78b..784e0fd91fd17e 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
+ * @hwdev: The device HW that controls the iommu
* @singleton_group: Used internally for drivers that have only one group
* @max_pasids: number of supported PASIDs
*/
@@ -369,6 +370,7 @@ struct iommu_device {
const struct iommu_ops *ops;
struct fwnode_handle *fwnode;
struct device *dev;
+ struct device *hwdev;
struct iommu_group *singleton_group;
u32 max_pasids;
};
@@ -465,7 +467,8 @@ static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
return dev->iommu->iommu_dev->ops;
}

-extern int bus_iommu_probe(const struct bus_type *bus);
+extern int bus_iommu_probe(const struct bus_type *bus,
+ struct iommu_device *iommu);
extern bool iommu_present(const struct bus_type *bus);
extern bool device_iommu_capable(struct device *dev, enum iommu_cap cap);
extern bool iommu_group_has_isolated_msi(struct iommu_group *group);