Message ID | 3-v2-d2762acaf50a+16d-iommu_group_locking2_jgg@nvidia.com (mailing list archive) |
---|---|
State | Handled Elsewhere, archived |
Headers | show |
Series | Fix device_lock deadlock on two probe() paths | expand |
> From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Wednesday, August 9, 2023 10:44 PM > > When bus_iommu_probe() runs it can attempt to probe the iommu itself, for > instance if the iommu is located on a platform_bus. This will cause the > device_lock() to deadlock on itself as the device_driver probe() callback > for the device calling iommu_device_register() already holds the > device_lock(): > > probe_iommu_group+0x18/0x38 > bus_for_each_dev+0xe4/0x168 > bus_iommu_probe+0x8c/0x240 > iommu_device_register+0x120/0x1b0 > mtk_iommu_probe+0x494/0x7a0 > platform_probe+0x94/0x100 > really_probe+0x1e4/0x3e8 > __driver_probe_device+0xc0/0x1a0 > driver_probe_device+0x110/0x1f0 > __device_attach_driver+0xf0/0x1b0 > bus_for_each_drv+0xf0/0x170 > __device_attach+0x120/0x240 > device_initial_probe+0x1c/0x30 > bus_probe_device+0xdc/0xe8 > deferred_probe_work_func+0xf0/0x140 > process_one_work+0x3b0/0x910 > worker_thread+0x33c/0x610 > kthread+0x1dc/0x1f0 > ret_from_fork+0x10/0x20 > > Keep track of the iommu itself and do not attempt to relock the device > while doing the probe_iommu_group scan. > > To accommodate omap's use of unregistered struct iommu_device's add a > new > 'hwdev' member to keep track of the hwdev in all cases. Normally this > would be dev->parent, but since omap doesn't allocate that struct it won't > exist for it. > > Fixes: a16b5d1681ab ("iommu: Complete the locking for dev- > >iommu_group") > Reported-by: Chen-Yu Tsai <wenst@chromium.org> > Closes: https://lore.kernel.org/linux-iommu/CAGXv+5E- > f9AteAYkmXYzVDZFSA_royc7-bS5LcrzzuHDnXccwA@mail.gmail.com > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > Tested-by: Chen-Yu Tsai <wenst@chromium.org> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 19fdb1a220240f..3ff365c9117850 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); @@ -1802,9 +1803,18 @@ static int probe_iommu_group(struct device *dev, void *data) struct probe_iommu_args *args = data; int ret; + /* + * The iommu device itself is not probed, in part because no sane iommu + * should self-attach to its own HW, but specifically because we already + * hold the device_lock for the hwdev when calling here. + */ + if (args->iommu->hwdev == dev) + return 0; + device_lock(dev); ret = __iommu_probe_device(dev, args->group_list); device_unlock(dev); + if (ret == -ENODEV) ret = 0; diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c index 1e4a90ec64322b..20fcc8ebab6ae3 100644 --- a/drivers/iommu/omap-iommu.c +++ b/drivers/iommu/omap-iommu.c @@ -1241,6 +1241,7 @@ static int omap_iommu_probe(struct platform_device *pdev) * an iommu without registering it, so re-run probe again to try * to match any devices that are waiting for this iommu. */ + obj->iommu.hwdev = &pdev->dev; bus_iommu_probe(&platform_bus_type, &obj->iommu); } diff --git a/include/linux/iommu.h b/include/linux/iommu.h index cc47e4086d69ec..96782bfb384462 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; };