A page fault handler running while another CPU detaches the same IOMMU can read a cleared RK_MMU_DTE_ADDR, turn it into a linear-map pointer and dereference it, which panics the machine. A dummy or firmware-left address is as unsafe. Seen on PX30 at reboot: the display controller still fetched an unmapped framebuffer, its IOMMU sat in a page fault, ignored the stall and paging-disable commands, and had RK_MMU_DTE_ADDR cleared during the handler's second run. No wait works: the master shares the line, the RK3588 NPU driver detaches from its interrupt thread, where synchronize_irq() deadlocks, and synchronize_hardirq() skips a force-threaded handler. A spinlock orders the walk against the register clear. 6.1 vendor kernel log, rk_pagefault_done is the walk, per CPU: CPU 1, init, reboot path: [ 1069.789374][ T1] rk_iommu ff460f00.iommu: Enable stall request timed out, retry_count = 0, status: 0x0002cb [ 1069.791314][ T1] rk_iommu ff460f00.iommu: Disable paging request timed out, retry_count = 0, status: 0x0002cb [ 1069.830197][ T1] Call trace: [ 1069.830922][ T1] iommu_detach_device+0xd0/0xdc [ 1069.831483][ T1] rockchip_drm_dma_detach_device+0x1c/0x34 [ 1069.832033][ T1] vop_crtc_atomic_disable+0xb0c/0xc44 [ 1069.832751][ T1] drm_atomic_helper_commit_modeset_disables+0x348/0x690 ... [ 1069.836601][ T1] drm_atomic_helper_shutdown+0xa0/0x158 [ 1069.837152][ T1] rockchip_drm_platform_shutdown+0x18/0x2c [ 1069.837870][ T1] platform_shutdown+0x3c/0x54 [ 1069.838599][ T1] device_shutdown+0x194/0x260 [ 1069.839160][ T1] kernel_restart+0x40/0x128 CPU 0, page fault handler: [ 1069.790364][ C0] rk_iommu ff460f00.iommu: Page fault at 0x00000000ffee0000 of type read [ 1069.792129][ C0] rk_iommu ff460f00.iommu: Page fault while iommu not attached to domain? [ 1069.792184][ C0] rk_iommu ff460f00.iommu: Page fault at 0x00000000ffee0280 of type read [ 1069.794604][ C0] Unable to handle kernel paging request at virtual address ffffff8000000ffc [ 1069.805998][ C0] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP [ 1069.809484][ C0] pc : rk_pagefault_done+0x1b4/0x3c8 [ 1069.810548][ C0] lr : rk_pagefault_done+0x19c/0x3c8 [ 1069.832390][ C0] Call trace: [ 1069.833116][ C0] rk_pagefault_done+0x1b4/0x3c8 [ 1069.833844][ C0] rk_iommu_irq+0x64/0xc8 [ 1069.834395][ C0] __handle_irq_event_percpu+0xe8/0x2ac [ 1069.834945][ C0] handle_irq_event+0x40/0xbc [ 1069.835675][ C0] handle_fasteoi_irq+0x118/0x28c [ 1069.836404][ C0] generic_handle_domain_irq+0x44/0x6c [ 1069.836954][ C0] gic_handle_irq+0x60/0xf0 [ 1069.837505][ C0] call_on_irq_stack+0x3c/0x5c [ 1069.838235][ C0] do_interrupt_handler+0x4c/0x94 [ 1069.838963][ C0] el1_interrupt+0x44/0xa8 [ 1069.839446][ C0] el1h_64_irq_handler+0x18/0x2c [ 1069.839997][ C0] el1h_64_irq+0x78/0x7c [ 1071.215224][ C0] Kernel panic - not syncing: Oops: Fatal exception in interrupt Fixes: c68a292152d3 ("iommu/rockchip: rk3288 iommu driver") Cc: stable@vger.kernel.org Signed-off-by: Lucas Tanure --- drivers/iommu/rockchip-iommu.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 1fa3110b142a..0f40d9678344 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -115,6 +115,7 @@ struct rk_iommu { struct iommu_device iommu; struct list_head node; /* entry in rk_iommu_domain.iommus */ struct iommu_domain *domain; /* domain to which iommu is attached */ + spinlock_t lock; const struct rk_iommu_ops *rk_ops; }; @@ -543,7 +544,8 @@ static int rk_iommu_force_reset(struct rk_iommu *iommu) return 0; } -static void log_iova(struct rk_iommu *iommu, int index, dma_addr_t iova) +static void log_iova(struct rk_iommu *iommu, struct iommu_domain *domain, + int index, dma_addr_t iova) { void __iomem *base = iommu->bases[index]; u32 dte_index, pte_index, page_offset; @@ -563,6 +565,12 @@ static void log_iova(struct rk_iommu *iommu, int index, dma_addr_t iova) mmu_dte_addr = rk_iommu_read(base, RK_MMU_DTE_ADDR); mmu_dte_addr_phys = iommu->rk_ops->pt_address(mmu_dte_addr); + if (domain == &rk_identity_domain || + mmu_dte_addr_phys != to_rk_domain(domain)->dt_dma) { + dev_err(iommu->dev, "iova = %pad: page directory %#x is not the attached domain's, not walked\n", + &iova, mmu_dte_addr); + return; + } dte_addr_phys = mmu_dte_addr_phys + (4 * dte_index); dte_addr = phys_to_virt(dte_addr_phys); @@ -593,6 +601,7 @@ static void log_iova(struct rk_iommu *iommu, int index, dma_addr_t iova) static irqreturn_t rk_iommu_irq(int irq, void *dev_id) { struct rk_iommu *iommu = dev_id; + struct iommu_domain *domain; u32 status; u32 int_status; dma_addr_t iova; @@ -606,6 +615,8 @@ static irqreturn_t rk_iommu_irq(int irq, void *dev_id) if (WARN_ON(clk_bulk_enable(iommu->num_clocks, iommu->clocks))) goto out; + spin_lock(&iommu->lock); + domain = READ_ONCE(iommu->domain); for (i = 0; i < iommu->num_mmu; i++) { int_status = rk_iommu_read(iommu->bases[i], RK_MMU_INT_STATUS); if (int_status == 0) @@ -625,15 +636,15 @@ static irqreturn_t rk_iommu_irq(int irq, void *dev_id) &iova, str_write_read(flags == IOMMU_FAULT_WRITE)); - log_iova(iommu, i, iova); + log_iova(iommu, domain, i, iova); /* * Report page fault to any installed handlers. * Ignore the return code, though, since we always zap cache * and clear the page fault anyway. */ - if (iommu->domain != &rk_identity_domain) - report_iommu_fault(iommu->domain, iommu->dev, iova, + if (domain != &rk_identity_domain) + report_iommu_fault(domain, iommu->dev, iova, flags); else dev_err(iommu->dev, "Page fault while iommu not attached to domain?\n"); @@ -651,6 +662,7 @@ static irqreturn_t rk_iommu_irq(int irq, void *dev_id) rk_iommu_write(iommu->bases[i], RK_MMU_INT_CLEAR, int_status); } + spin_unlock(&iommu->lock); clk_bulk_disable(iommu->num_clocks, iommu->clocks); @@ -921,16 +933,19 @@ static struct rk_iommu *rk_iommu_from_dev(struct device *dev) /* Must be called with iommu powered on and attached */ static void rk_iommu_disable(struct rk_iommu *iommu) { + unsigned long flags; int i; /* Ignore error while disabling, just keep going */ WARN_ON(clk_bulk_enable(iommu->num_clocks, iommu->clocks)); rk_iommu_enable_stall(iommu); rk_iommu_disable_paging(iommu); + spin_lock_irqsave(&iommu->lock, flags); for (i = 0; i < iommu->num_mmu; i++) { rk_iommu_write(iommu->bases[i], RK_MMU_INT_MASK, 0); rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR, 0); } + spin_unlock_irqrestore(&iommu->lock, flags); rk_iommu_disable_stall(iommu); clk_bulk_disable(iommu->num_clocks, iommu->clocks); } @@ -997,7 +1012,7 @@ static int rk_iommu_identity_attach(struct iommu_domain *identity_domain, if (iommu->domain == identity_domain) return 0; - iommu->domain = identity_domain; + WRITE_ONCE(iommu->domain, identity_domain); spin_lock_irqsave(&rk_domain->iommus_lock, flags); list_del_init(&iommu->node); @@ -1051,7 +1066,7 @@ static int rk_iommu_attach_device(struct iommu_domain *domain, if (ret) return ret; - iommu->domain = domain; + WRITE_ONCE(iommu->domain, domain); spin_lock_irqsave(&rk_domain->iommus_lock, flags); list_add_tail(&iommu->node, &rk_domain->iommus); @@ -1227,6 +1242,7 @@ static int rk_iommu_probe(struct platform_device *pdev) return -ENOMEM; iommu->domain = &rk_identity_domain; + spin_lock_init(&iommu->lock); platform_set_drvdata(pdev, iommu); iommu->dev = dev; -- 2.56.1