Rockchip IOMMU fixes

Hi all,

I was expecting my DT probing change to flush out issues, and it seems some weaknesses in rockchip-iommu were the first to show. The first two patches here get things working again for me on my RK3399 NanoPC-T4 (with a console on HDMI exercising the VOP MMU), the 3rd patch just finishes the cleanup since I was there and could test it.

Thanks,
Robin.

Robin Murphy (3):
iommu/rockchip: Allocate per-device data sensibly iommu/rockchip: Register in a sensible order iommu/rockchip: Retire global dma_dev workaround

 drivers/iommu/rockchip-iommu.c | 61 +++++++++++++++-------------------
 1 file changed, 26 insertions(+), 35 deletions(-)

2.39.2.101.g768bb238c484.dirty

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.

---
 drivers/iommu/rockchip-iommu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index 323cc665c357..48826d1ccfd8 100644
@@ -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);

2.39.2.101.g768bb238c484.dirty

Currently Rockchip calls iommu_device_register() before it's finished setting up the hardware and driver state, and as such it now gets unhappy in various ways when registration starts working the way it was always intended to, and probing client devices straight away. Reorder the operations to ensure that what we're registering is a prepared and functional IOMMU instance.

---
 drivers/iommu/rockchip-iommu.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

index 48826d1ccfd8..9d2120a9232a 100644
@@ -1256,14 +1256,6 @@ static int rk_iommu_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	err = iommu_device_sysfs_add(&iommu->iommu, dev, NULL, dev_name(dev));
-	if (err)
-		goto err_unprepare_clocks;
-
-	err = iommu_device_register(&iommu->iommu, &rk_iommu_ops, dev);
-	if (err)
-		goto err_remove_sysfs;
-
 	/*
 	 * Use the first registered IOMMU device for domain to use with DMA
 	 * API, since a domain might not physically correspond to a single

@@ -1290,12 +1282,19 @@ static int rk_iommu_probe(struct platform_device *pdev)
 
 	dma_set_mask_and_coherent(dev, rk_ops->dma_bit_mask);
 
+	err = iommu_device_sysfs_add(&iommu->iommu, dev, NULL, dev_name(dev));
+	if (err)
+		goto err_pm_disable;
+
+	err = iommu_device_register(&iommu->iommu, &rk_iommu_ops, dev);
+	if (err)
+		goto err_remove_sysfs;
+
 	return 0;
-err_pm_disable:
-	pm_runtime_disable(dev);
 err_remove_sysfs:
 	iommu_device_sysfs_remove(&iommu->iommu);
-err_unprepare_clocks:
+err_pm_disable:
+	pm_runtime_disable(dev);
 	clk_bulk_unprepare(iommu->num_clocks, iommu->clocks);
 	return err;
 }

2.39.2.101.g768bb238c484.dirty

The global dma_dev trick was mostly because the old domain_alloc op provided no context, so no way to know which IOMMU was to own the pagetable, or if a suitable one even existed at all. In the new multi-instance world with domain_alloc_paging this is no longer a concern - now we know that the given device must be associated with a valid IOMMU instance which provided the op to call in the first place, and therefore that instance can and should be the pagetable owner. To avoid worrying about the lifetime and stability of the rk_domain->iommus list, and keep the lookups simple and efficient, we'll still stash a dma_dev pointer, but now it's accurately per-domain.

---
 drivers/iommu/rockchip-iommu.c | 34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

index 9d2120a9232a..af4cc91b2bbf 100644
@@ -88,6 +88,7 @@ struct rk_iommu_domain {
 	dma_addr_t dt_dma;
 	spinlock_t iommus_lock; /* lock for iommus list */
 	spinlock_t dt_lock; /* lock for modifying page directory table */
+	struct device *dma_dev;
 
 	struct iommu_domain domain;
 };

@@ -123,7 +124,6 @@ struct rk_iommudata {
 	struct rk_iommu *iommu;
 };
 
-static struct device *dma_dev;
 static const struct rk_iommu_ops *rk_ops;
 static struct iommu_domain rk_identity_domain;
 

@@ -132,7 +132,7 @@ static inline void rk_table_flush(struct rk_iommu_domain *dom, dma_addr_t dma,
 {
 	size_t size = count * sizeof(u32); /* count of u32 entry */
 
-	dma_sync_single_for_device(dma_dev, dma, size, DMA_TO_DEVICE);
+	dma_sync_single_for_device(dom->dma_dev, dma, size, DMA_TO_DEVICE);
 }
 
 static struct rk_iommu_domain *to_rk_domain(struct iommu_domain *dom)

@@ -734,9 +734,9 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain,
 	if (!page_table)
 		return ERR_PTR(-ENOMEM);
 
-	pt_dma = dma_map_single(dma_dev, page_table, SPAGE_SIZE, DMA_TO_DEVICE);
-	if (dma_mapping_error(dma_dev, pt_dma)) {
-		dev_err(dma_dev, "DMA mapping error while allocating page table\n");
+	pt_dma = dma_map_single(rk_domain->dma_dev, page_table, SPAGE_SIZE, DMA_TO_DEVICE);
+	if (dma_mapping_error(rk_domain->dma_dev, pt_dma)) {
+		dev_err(rk_domain->dma_dev, "DMA mapping error while allocating page table\n");
 		iommu_free_page(page_table);
 		return ERR_PTR(-ENOMEM);
 	}

@@ -1051,9 +1051,7 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
 static struct iommu_domain *rk_iommu_domain_alloc_paging(struct device *dev)
 {
 	struct rk_iommu_domain *rk_domain;
-
-	if (!dma_dev)
-		return NULL;
+	struct rk_iommu *iommu;
 
 	rk_domain = kzalloc(sizeof(*rk_domain), GFP_KERNEL);
 	if (!rk_domain)

@@ -1068,10 +1066,12 @@ static struct iommu_domain *rk_iommu_domain_alloc_paging(struct device *dev)
 	if (!rk_domain->dt)
 		goto err_free_domain;
 
-	rk_domain->dt_dma = dma_map_single(dma_dev, rk_domain->dt,
+	iommu = rk_iommu_from_dev(dev);
+	rk_domain->dma_dev = iommu->dev;
+	rk_domain->dt_dma = dma_map_single(rk_domain->dma_dev, rk_domain->dt,
 					   SPAGE_SIZE, DMA_TO_DEVICE);
-	if (dma_mapping_error(dma_dev, rk_domain->dt_dma)) {
-		dev_err(dma_dev, "DMA map error for DT\n");
+	if (dma_mapping_error(rk_domain->dma_dev, rk_domain->dt_dma)) {
+		dev_err(rk_domain->dma_dev, "DMA map error for DT\n");
 		goto err_free_dt;
 	}
 

@@ -1105,13 +1105,13 @@ static void rk_iommu_domain_free(struct iommu_domain *domain)
 		if (rk_dte_is_pt_valid(dte)) {
 			phys_addr_t pt_phys = rk_ops->pt_address(dte);
 			u32 *page_table = phys_to_virt(pt_phys);
-			dma_unmap_single(dma_dev, pt_phys,
+			dma_unmap_single(rk_domain->dma_dev, pt_phys,
 					 SPAGE_SIZE, DMA_TO_DEVICE);
 			iommu_free_page(page_table);
 		}
 	}
 
-	dma_unmap_single(dma_dev, rk_domain->dt_dma,
+	dma_unmap_single(rk_domain->dma_dev, rk_domain->dt_dma,
 			 SPAGE_SIZE, DMA_TO_DEVICE);
 	iommu_free_page(rk_domain->dt);
 

@@ -1256,14 +1256,6 @@ static int rk_iommu_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	/*
-	 * Use the first registered IOMMU device for domain to use with DMA
-	 * API, since a domain might not physically correspond to a single
-	 * IOMMU device..
-	 */
-	if (!dma_dev)
-		dma_dev = &pdev->dev;
-
 	pm_runtime_enable(dev);
 
 	for (i = 0; i < iommu->num_irq; i++) {

2.39.2.101.g768bb238c484.dirty

Tested this series on an RK3576, which indeed fixes the boot panic I get with next-20250314. After looking at the patches with the full function context, I can't see any obvious issues.

For the whole series:

Kind regards,
Nicolas Frattaroli

Tested this series on a RK3566 device and this fixed kernel panic/oops on boot since next-20250314.

For the whole series:

Hi Robin,

Fixes the issue seen on next-20250314 on my RK3588 Tiger Haikou:

"""

[    2.007506] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000298
[    2.017346] Mem abort info:
[    2.020465]   ESR = 0x0000000096000004
[    2.024654]   EC = 0x25: DABT (current EL), IL = 32 bits
[    2.030591]   SET = 0, FnV = 0
[    2.033999]   EA = 0, S1PTW = 0
[    2.037506]   FSC = 0x04: level 0 translation fault
[    2.042958] Data abort info:
[    2.046170]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[    2.052302]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[    2.057949]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[    2.063885] [0000000000000298] user address but active_mm is swapper
[    2.070989] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[    2.077994] Modules linked in:
[    2.081405] CPU: 7 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.14.0-rc6-next-20250314-dirty #10
[    2.091132] Hardware name: Theobroma Systems RK3588-Q7 SoM on Haikou devkit (DT)
[    2.099397] pstate: 00400009 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    2.107179] pc : devm_kmalloc+0x28/0x100
[    2.111568] lr : rk_iommu_of_xlate+0x30/0x90
[    2.116341] sp : ffff800081d0b840
[    2.120040] x29: ffff800081d0b840 x28: 0000000000000000 x27: ffff8000815d3e20
[    2.128010] x26: 0000000000000001 x25: ffff8000815d4ed8 x24: ffff0001f08e1f60
[    2.135989] x23: 0000000000000000 x22: ffff8000811b3f58 x21: 0000000000000000
[    2.143966] x20: 0000000000000010 x19: 0000000000000000 x18: 00000000ffffffff
[    2.151944] x17: ffff0001f14acc00 x16: ffff0001f14aca00 x15: ffff0001f148b27c
[    2.159922] x14: ffff800081d0b820 x13: ffff0001f148ba1c x12: 0101010101010101
[    2.167899] x11: 7f7f7f7f7f7f7f7f x10: 00000000000a2cc8 x9 : 0000000000000003
[    2.175877] x8 : ffff0001f08e3540 x7 : 0000000000000000 x6 : 1d150c11f5f38080
[    2.183855] x5 : ffff0001f0278000 x4 : 0000000000000000 x3 : ffff800081a77218
[    2.191832] x2 : 0000000000000dc0 x1 : 0000000000000010 x0 : 0000000000000090
[    2.199812] Call trace:
[    2.202538]  devm_kmalloc+0x28/0x100 (P)
[    2.206924]  rk_iommu_of_xlate+0x30/0x90
[    2.211307]  of_iommu_xlate+0x94/0xc0
[    2.215400]  of_iommu_configure+0x1cc/0x250
[    2.220075]  of_dma_configure_id+0x144/0x24c
[    2.224848]  platform_dma_configure+0x8c/0xb4
[    2.229716]  __iommu_probe_device+0x2e4/0x4a0
[    2.234583]  probe_iommu_group+0x3c/0x68
[    2.238965]  bus_for_each_dev+0x7c/0xe0
[    2.243244]  iommu_device_register+0xd8/0x228
[    2.248111]  rk_iommu_probe+0x204/0x360
[    2.252397]  platform_probe+0x68/0xdc
[    2.256488]  really_probe+0xbc/0x2c0
[    2.260484]  __driver_probe_device+0x78/0x120
[    2.265355]  driver_probe_device+0x3c/0x154
[    2.270032]  __driver_attach+0x90/0x1a0
[    2.274315]  bus_for_each_dev+0x7c/0xe0
[    2.278602]  driver_attach+0x24/0x30
[    2.282599]  bus_add_driver+0xe4/0x208
[    2.286781]  driver_register+0x68/0x130
[    2.291065]  __platform_driver_register+0x24/0x30
[    2.296322]  rk_iommu_driver_init+0x1c/0x28
[    2.300997]  do_one_initcall+0x60/0x1d4
[    2.305283]  kernel_init_freeable+0x210/0x274
[    2.310153]  kernel_init+0x20/0x140
[    2.314051]  ret_from_fork+0x10/0x20
[    2.318048] Code: a90153f3 b1020020 f9001bf7 aa0103f4 (b9429ab7)
[    2.324857] ---[ end trace 0000000000000000 ]---
[    2.330029] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[    2.338587] SMP: stopping secondary CPUs
[    2.342971] Kernel Offset: disabled
[    2.346862] CPU features: 0x0c00,000000e0,01202650,8200720b
[    2.353089] Memory Limit: none
[    2.356499] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---

"""

Therefore:

Thanks!
Quentin

Applied, thanks.