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

Marek Szyprowski
2023-08-08T13:00:52Z

Hi All,

On 08.08.2023 14:32, Marek Szyprowski wrote:
> On 08.08.2023 12:31, Chen-Yu Tsai wrote:
>> On Mon, Aug 7, 2023 at 8:54 PM Joerg Roedel <joro@8bytes.org> wrote:
>>> On Mon, Jul 31, 2023 at 02:50:23PM -0300, Jason Gunthorpe wrote:
>>>> Jason Gunthorpe (10):
>>>>    iommu: Remove useless group refcounting
>>>>    iommu: Add a lockdep assertion for remaining dev->iommu_group reads
>>>>    iommu: Add generic_single_device_group()
>>>>    iommu/sun50i: Convert to generic_single_device_group()
>>>>    iommu/sprd: Convert to generic_single_device_group()
>>>>    iommu/rockchip: Convert to generic_single_device_group()
>>>>    iommu/ipmmu-vmsa: Convert to generic_single_device_group()
>>>>    iommu/omap: Convert to generic_single_device_group()
>>>>    iommu: Complete the locking for dev->iommu_group
>>>>    iommu/intel: Fix missing locking for
>>>> show_device_domain_translation()
>>>>
>>>>   drivers/iommu/intel/debugfs.c  |  34 ++++----
>>>>   drivers/iommu/iommu.c          | 155
>>>> +++++++++++++++++++++------------
>>>>   drivers/iommu/ipmmu-vmsa.c     |  22 ++---
>>>>   drivers/iommu/omap-iommu.c     |  30 +------
>>>>   drivers/iommu/omap-iommu.h     |   2 +-
>>>>   drivers/iommu/rockchip-iommu.c |  22 +----
>>>>   drivers/iommu/sprd-iommu.c     |  24 +----
>>>>   drivers/iommu/sun50i-iommu.c   |  29 ++----
>>>>   include/linux/iommu.h          |   3 +
>>>>   9 files changed, 138 insertions(+), 183 deletions(-)
>>> Applied, thanks for the nice cleanup!
>> This series seems to cause a hung task during boot on MediaTek
>> platforms.
>> It hangs with next-20230808. Reverting the 10 commits from this series
>> makes the system boot up again.
>
> I confirm that next-20230808 is broken on ARM 32bit based Exynos
> boards too. Boards lock up very early during boot. I will try to
> investigate this soon.

Hmm this turned to be Exynos IOMMU specific, but the issue is probably
somehow generic.

The deadlock happens early in __iommu_probe_device() on
device_lock(dev). Here is a stack dump of that call:

CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc5-next-20230808-dirty
#7013
Hardware name: Samsung Exynos (Flattened Device Tree)
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x58/0x70
dump_stack_lvl from __iommu_probe_device+0x3d8/0x4ac
__iommu_probe_device from probe_iommu_group+0x8/0x14
probe_iommu_group from bus_for_each_dev+0x60/0xb4
bus_for_each_dev from bus_iommu_probe+0x34/0x118
bus_iommu_probe from iommu_device_register+0x98/0x100
iommu_device_register from exynos_sysmmu_probe+0x238/0x3c0
exynos_sysmmu_probe from platform_probe+0x80/0xc0
platform_probe from really_probe+0x154/0x3d4
really_probe from __driver_probe_device+0xa0/0x1e8
__driver_probe_device from driver_probe_device+0x30/0xd0
driver_probe_device from __device_attach_driver+0xbc/0x11c
__device_attach_driver from bus_for_each_drv+0x74/0xc0
bus_for_each_drv from __device_attach+0xec/0x1b4
__device_attach from bus_probe_device+0x8c/0x90
bus_probe_device from device_add+0x5b8/0x78c
device_add from of_platform_device_create_pdata+0x94/0xcc
of_platform_device_create_pdata from of_platform_bus_create+0x1ac/0x4d8
of_platform_bus_create from of_platform_bus_create+0x214/0x4d8
of_platform_bus_create from of_platform_populate+0x80/0x114
of_platform_populate from of_platform_default_populate_init+0xcc/0xe4
of_platform_default_populate_init from do_one_initcall+0x6c/0x318
do_one_initcall from kernel_init_freeable+0x1c4/0x214
kernel_init_freeable from kernel_init+0x18/0x12c
kernel_init from ret_from_fork+0x14/0x2c

The problem here is that exynos_sysmmu_probe() is by design called under
device_lock, then it calls iommu_device_register(), which in turn
triggers calling __iommu_probe_device() on all platform devices in the
system, while the still probed sysmmu device is one of them.

Frankly speaking I have no idea how to defer calling
iommu_device_register() to avoid this deadlock. Any ideas?

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland