On 13.03.2025 12:01, Robin Murphy wrote:
> On 2025-03-13 9:56 am, Marek Szyprowski wrote:
> [...]
>> This patch landed in yesterday's linux-next as commit bcb81ac6ae3c
>> ("iommu: Get DT/ACPI parsing into the proper probe path"). In my tests I
>> found it breaks booting of ARM64 RK3568-based Odroid-M1 board
>> (arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts). Here is the
>> relevant kernel log:
>
> ...and the bug-flushing-out begins!
>
>> Unable to handle kernel NULL pointer dereference at virtual address
>> 00000000000003e8
>> Mem abort info:
>> ESR = 0x0000000096000004
>> EC = 0x25: DABT (current EL), IL = 32 bits
>> SET = 0, FnV = 0
>> EA = 0, S1PTW = 0
>> FSC = 0x04: level 0 translation fault
>> Data abort info:
>> ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
>> CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>> GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>> [00000000000003e8] user address but active_mm is swapper
>> Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
>> Modules linked in:
>> CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.14.0-rc3+ #15533
>> Hardware name: Hardkernel ODROID-M1 (DT)
>> pstate: 00400009 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : devm_kmalloc+0x2c/0x114
>> lr : rk_iommu_of_xlate+0x30/0x90
>> ...
>> Call trace:
>> devm_kmalloc+0x2c/0x114 (P)
>> rk_iommu_of_xlate+0x30/0x90
>
> Yeah, looks like this is doing something a bit questionable which can't
> work properly. TBH the whole dma_dev thing could probably be cleaned up
> now that we have proper instances, but for now does this work?
Yes, this patch fixes the problem I've observed.
Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
BTW, this dma_dev idea has been borrowed from my exynos_iommu driver and
I doubt it can be cleaned up.
>
> (annoyingly none of my Rockchip boards are set up for testing right
> now, but I might have time to dig one out later)
>
> Thanks,
> Robin.
>
> ----->8-----
>
> Subject: [PATCH] iommu/rockchip: Allocate per-device data sensibly
>
> Now that DT-based probing is finally happening in the right order again,
> it reveals an issue in Rockchip's of_xlate, which can now be called
> during registration, but is using the global dma_dev which is only
> assigned later. However, this makes little sense when we're already
> looking up the correct IOMMU device, who should logically be the owner
> of the devm allocation anyway.
>
> Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe
> path")
> Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
> ---
> drivers/iommu/rockchip-iommu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/rockchip-iommu.c
> b/drivers/iommu/rockchip-iommu.c
> index 323cc665c357..48826d1ccfd8 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -1148,12 +1148,12 @@ static int rk_iommu_of_xlate(struct device *dev,
> struct platform_device *iommu_dev;
> struct rk_iommudata *data;
>
> - data = devm_kzalloc(dma_dev, sizeof(*data), GFP_KERNEL);
> + iommu_dev = of_find_device_by_node(args->np);
> +
> + data = devm_kzalloc(&iommu_dev->dev, sizeof(*data), GFP_KERNEL);
> if (!data)
> return -ENOMEM;
>
> - iommu_dev = of_find_device_by_node(args->np);
> -
> data->iommu = platform_get_drvdata(iommu_dev);
> data->iommu->domain = &rk_identity_domain;
> dev_iommu_priv_set(dev, data);
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
