From 3af228c8d5f7bd2cefc53572828849e6163a4aa1 Mon Sep 17 00:00:00 2001 From: Michael Fischer Date: Thu, 27 Aug 2026 20:25:27 -0500 Subject: [PATCH 01/10] rknpu: do not access NPU registers in the IRQ handler after power-off rknpu_irq_handler() touches the core's registers unconditionally: the no-job path writes RKNPU_OFFSET_INT_CLEAR and the normal path reads RKNPU_OFFSET_INT_STATUS. rknpu_power_off() runs from a deferred work item, so an interrupt that is late or spurious can be delivered after the block has already been powered down. The register access then takes an external abort and the machine dies instantly, with no console output and no recovery short of a power cycle: Unable to handle kernel paging request at virtual address ... pc : readl+0x4/0x20 lr : rknpu_irq_handler.isra.0+0x94/0x2f0 Call trace: readl rknpu_core0_irq_handler __handle_irq_event_percpu ... el1_interrupt / cpuidle_enter / do_idle The trace was captured over netconsole with CPU 0 idle, i.e. the NPU had finished and powered down and the interrupt landed afterwards. Bail out early when the device is not powered. power_refcount is an atomic, so unlike power_lock (a mutex) it is safe to read from hard IRQ context. The check is racy in principle but closes the window that occurs in practice, and an unpowered device cannot be asserting an interrupt, so returning IRQ_NONE cannot cause a level-triggered interrupt storm. --- drivers/rknpu/rknpu_job.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/drivers/rknpu/rknpu_job.c b/drivers/rknpu/rknpu_job.c index 57d67d81f7d5e..7a4aee9f12f5f 100644 --- a/drivers/rknpu/rknpu_job.c +++ b/drivers/rknpu/rknpu_job.c @@ -646,6 +646,27 @@ static inline irqreturn_t rknpu_irq_handler(int irq, void *data, int core_index) uint32_t status = 0; unsigned long flags; + /* + * Never touch NPU registers while the block is powered down. + * + * Both paths below access registers unconditionally: the no-job path writes + * RKNPU_OFFSET_INT_CLEAR and the normal path reads RKNPU_OFFSET_INT_STATUS. + * rknpu_power_off() is driven by a deferred work item, so a late or spurious + * interrupt can arrive after power has gone. The register access then takes an + * external abort and the machine dies immediately, with no console output and + * no way back but a power cycle: + * + * pc : readl+0x4/0x20 + * lr : rknpu_irq_handler.isra.0+0x94/0x2f0 + * Call trace: readl / rknpu_core0_irq_handler / __handle_irq_event_percpu + * + * power_refcount is an atomic, so unlike power_lock (a mutex) it is safe to read + * from hard IRQ context. If the device is unpowered it cannot be asserting an + * interrupt, so IRQ_NONE is correct and cannot cause a level-IRQ storm. + */ + if (atomic_read(&rknpu_dev->power_refcount) <= 0) + return IRQ_NONE; + subcore_data = &rknpu_dev->subcore_datas[core_index]; spin_lock_irqsave(&rknpu_dev->irq_lock, flags); From aedcff95c87bc5af8dd1948ad1513342646d1701 Mon Sep 17 00:00:00 2001 From: Michael Fischer Date: Thu, 27 Aug 2026 20:25:42 -0500 Subject: [PATCH 02/10] rknpu: signal a job's fence when it is torn down without completing rknpu_job_free() drops the fence reference but never signals the fence. A job that completes normally is signalled from the completion path, but a job destroyed by rknpu_job_timeout_clean() or rknpu_job_abort() never gets there. Every waiter on such a fence therefore blocks until its own timeout expires rather than being woken when the job actually died, which defeats the purpose of waiting on the fence to detect a failed job. With CONFIG_ROCKCHIP_RKNPU_FENCE=y and a userspace consumer that waits on the fence fd, a single aborted job costs the full wait timeout instead of returning promptly. Signal the fence with -ETIMEDOUT before dropping the reference so waiters wake at once and can tell failure from completion with dma_fence_get_status(). The added dma_fence_is_signaled() test makes this a no-op on the normal completion path. --- drivers/rknpu/rknpu_job.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/rknpu/rknpu_job.c b/drivers/rknpu/rknpu_job.c index 7a4aee9f12f5f..edc383d1532e0 100644 --- a/drivers/rknpu/rknpu_job.c +++ b/drivers/rknpu/rknpu_job.c @@ -101,8 +101,23 @@ static void rknpu_job_free(struct rknpu_job *job) rknpu_gem_object_put(&task_obj->base); #endif - if (job->fence) + if (job->fence) { + /* + * A job torn down without completing (rknpu_job_timeout_clean() or + * rknpu_job_abort()) never reaches the RKNPU_JOB_DONE path, so its fence + * is never signalled and every waiter blocks until its own timeout + * expires -- which defeats the point of waiting on a fence to notice + * that a job has failed. Signal it with an error instead, so waiters + * wake immediately and can distinguish failure from completion via + * dma_fence_get_status(). No-op on the success path, where the + * completion interrupt has already signalled it. + */ + if (!dma_fence_is_signaled(job->fence)) { + dma_fence_set_error(job->fence, -ETIMEDOUT); + dma_fence_signal(job->fence); + } dma_fence_put(job->fence); + } if (job->args_owner) kfree(job->args); From f9d560fe4919062e98a46d1d5bd7fec0c043755c Mon Sep 17 00:00:00 2001 From: Michael Fischer Date: Thu, 27 Aug 2026 20:26:00 -0500 Subject: [PATCH 03/10] rknpu: clamp the IOMMU domain reference count at zero rknpu_iommu_domain_put() is a bare atomic_dec(), so an unbalanced put drives iommu_domain_refcount negative without bound. rknpu_iommu_domain_get_and_switch() decides a switch is safe using only if (atomic_read(&rknpu_dev->iommu_domain_refcount) == 0) so once the count has gone negative it never reads zero again. Every subsequent domain switch then waits the full RKNPU_SWITCH_DOMAIN_WAIT_TIME_MS and fails, and since rknpu_gem_object_create() switches domains, every allocation after that returns -EINVAL: RKNPU: switch iommu domain time out, failed to switch iommu domain, id: 1 RKNPU: rknpu_gem_object_create error Note this presents as MEM_CREATE failing with -EINVAL at essentially zero IOVA in use, which is easy to misread as memory exhaustion rather than a stuck reference count. Clamp at zero and log, so an unbalanced put is a bounded, visible anomaly instead of a device that needs a reboot. This is deliberately a safety net: the underlying imbalance is fixed separately. --- drivers/rknpu/rknpu_iommu.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/rknpu/rknpu_iommu.c b/drivers/rknpu/rknpu_iommu.c index a3efb1ef02734..e9e9f3652d302 100644 --- a/drivers/rknpu/rknpu_iommu.c +++ b/drivers/rknpu/rknpu_iommu.c @@ -560,7 +560,25 @@ int rknpu_iommu_domain_get_and_switch(struct rknpu_device *rknpu_dev, int rknpu_iommu_domain_put(struct rknpu_device *rknpu_dev) { - atomic_dec(&rknpu_dev->iommu_domain_refcount); + /* + * Never let the reference count go negative. + * + * rknpu_iommu_domain_get_and_switch() proceeds only when this reads exactly + * zero, so a bare atomic_dec() turns a single unbalanced put into a permanent + * wedge: the count never reads zero again, every domain switch burns its full + * RKNPU_SWITCH_DOMAIN_WAIT_TIME_MS and fails, and because + * rknpu_gem_object_create() switches domains, every subsequent allocation then + * fails with -EINVAL until the machine is rebooted. + * + * Clamping turns an over-put into a bounded anomaly instead of a dead device. + * It treats the symptom, not the cause, so warn once per occurrence to keep any + * remaining imbalance visible. + */ + if (atomic_dec_return(&rknpu_dev->iommu_domain_refcount) < 0) { + atomic_set(&rknpu_dev->iommu_domain_refcount, 0); + LOG_DEV_ERROR(rknpu_dev->dev, + "iommu domain refcount underflow, clamped to 0\n"); + } return 0; } From b3c37c1ab82aee1f0af9ccc910b0fb528012fbff Mon Sep 17 00:00:00 2001 From: Michael Fischer Date: Thu, 27 Aug 2026 20:26:37 -0500 Subject: [PATCH 04/10] rknpu: release the IOMMU domain reference exactly once per job rknpu_job_abort() releases the IOMMU domain reference unconditionally. The reference is acquired exactly once, in rknpu_job_commit(), but three teardown paths release it -- the completion path, rknpu_job_abort() and rknpu_job_timeout_clean() -- and nothing records whether a given job still holds one. A job that completes and is then aborted, or is aborted and then reaped, therefore releases twice. Because the count is device-wide rather than per job, a double release drives iommu_domain_refcount to zero while other cores are still executing. rknpu_iommu_domain_get_and_switch() decides a switch is safe using only if (atomic_read(&rknpu_dev->iommu_domain_refcount) == 0) so once the count reaches zero prematurely the IOMMU is reprogrammed underneath live work, and the result is a cascade that only a reboot clears: RKNPU: mismatch domain get from iommu_get_domain_for_dev RKNPU: failed to switch iommu domain, id: 1, ret: -22 RKNPU: rknpu_gem_get_pages: dma map 2097152 fail This only affects users of more than one IOMMU domain (rknpu_mem_create()'s iommu_domain_id); a single-domain workload cannot hit it. The premature zeros are silent: the count never goes negative, so an underflow check does not catch them. Instrumenting rknpu_iommu_domain_put() with __builtin_return_address(0) and reporting "reached zero while some core still has a job" separately from "went below zero" attributed 17 premature zeros in a single run to rknpu_job_abort(), against one underflow from rknpu_job_timeout_clean(). Track ownership per job and make every release a test-and-clear, so a job releases the reference at most once no matter which teardown path runs, and a second teardown of the same job is a no-op. With this applied the premature-zero count measured over the same workload drops to zero. Closes: https://github.com/rockchip-linux/kernel/issues/387 --- drivers/rknpu/include/rknpu_job.h | 11 +++++++++++ drivers/rknpu/rknpu_job.c | 7 +++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/rknpu/include/rknpu_job.h b/drivers/rknpu/include/rknpu_job.h index b4d40d96ffce4..86acb0540bf7e 100644 --- a/drivers/rknpu/include/rknpu_job.h +++ b/drivers/rknpu/include/rknpu_job.h @@ -32,6 +32,17 @@ struct rknpu_job { struct work_struct cleanup_work; bool irq_entry[RKNPU_MAX_CORES]; unsigned int flags; + /* + * Does this job currently hold the IOMMU domain reference? + * + * The reference is acquired exactly once, in rknpu_job_commit(), but three + * teardown paths release it -- the completion path, rknpu_job_abort() and + * rknpu_job_timeout_clean() -- with nothing recording whether this particular + * job still holds one. Its own word rather than a bit in ->flags because the + * releases run from both interrupt and process context, so the test-and-clear + * has to be atomic. + */ + unsigned long dom_held; int ret; struct rknpu_submit *args; bool args_owner; diff --git a/drivers/rknpu/rknpu_job.c b/drivers/rknpu/rknpu_job.c index edc383d1532e0..7a981a839413b 100644 --- a/drivers/rknpu/rknpu_job.c +++ b/drivers/rknpu/rknpu_job.c @@ -500,7 +500,8 @@ static void rknpu_job_done(struct rknpu_job *job, int ret, int core_index) if (atomic_dec_and_test(&job->interrupt_count)) { int use_core_num = job->use_core_num; - rknpu_iommu_domain_put(rknpu_dev); + if (test_and_clear_bit(0, &job->dom_held)) + rknpu_iommu_domain_put(rknpu_dev); job->flags |= RKNPU_JOB_DONE; job->ret = ret; @@ -556,6 +557,7 @@ static void rknpu_job_schedule(struct rknpu_job *job) job->ret = -EINVAL; return; } + set_bit(0, &job->dom_held); spin_lock_irqsave(&rknpu_dev->irq_lock, flags); for (i = 0; i < rknpu_dev->config->num_irqs; i++) { @@ -580,7 +582,8 @@ static void rknpu_job_abort(struct rknpu_job *job) unsigned long flags; int i = 0; - rknpu_iommu_domain_put(rknpu_dev); + if (test_and_clear_bit(0, &job->dom_held)) + rknpu_iommu_domain_put(rknpu_dev); msleep(100); From ccee542566b1ef98181d0e83bc557fdaf41fd2fc Mon Sep 17 00:00:00 2001 From: Michael Fischer Date: Thu, 27 Aug 2026 20:27:13 -0500 Subject: [PATCH 05/10] rknpu: release the domain reference held by a job reaped on timeout rknpu_job_timeout_clean() detaches a timed-out job from its core and queues its cleanup work, but never releases the IOMMU domain reference the job acquired in rknpu_job_commit(). The completion path and rknpu_job_abort() both release it; this path does not. The reference is therefore leaked for every job reaped here, and since iommu_domain_refcount is device-wide and rknpu_iommu_domain_get_and_switch() proceeds only when it reads exactly zero, one leaked reference is enough to make every subsequent domain switch fail for the lifetime of the boot -- and with it every allocation that has to switch domains. Release it, guarded by the same per-job ownership bit the other two paths use, so a job reaped after it has already been aborted does not double release. --- drivers/rknpu/rknpu_job.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/rknpu/rknpu_job.c b/drivers/rknpu/rknpu_job.c index 7a981a839413b..fc0d245802bc2 100644 --- a/drivers/rknpu/rknpu_job.c +++ b/drivers/rknpu/rknpu_job.c @@ -757,6 +757,17 @@ static void rknpu_job_timeout_clean(struct rknpu_device *rknpu_dev, spin_unlock_irqrestore(&rknpu_dev->irq_lock, flags); + /* + * Release the domain reference this job still + * holds. The completion and abort paths both do + * this, but reaping a timed-out job here did + * not, so the reference was leaked and the + * device-wide count never returned to zero -- + * after which no domain switch can ever succeed. + */ + if (test_and_clear_bit(0, &job->dom_held)) + rknpu_iommu_domain_put(rknpu_dev); + do { schedule_work(&job->cleanup_work); From 4aa7c465d98b1c666cc76e939c61c7c897c1c0a2 Mon Sep 17 00:00:00 2001 From: Michael Fischer Date: Fri, 28 Aug 2026 15:35:54 -0500 Subject: [PATCH 06/10] iommu/rockchip: add an in-place domain switch and an MMU reprogram helper Switching a master between IOMMU domains through the core -- iommu_detach_device() followed by iommu_attach_device() -- makes the attach run rk_iommu_enable(), which performs a full rk_iommu_force_reset() of every MMU bank. With work in flight on other cores that reset intermittently fails its DTE_ADDR readback verification, the attach then fails, and __iommu_attach_group() leaves the master attached to no domain at all: rk_iommu fdab9000.iommu: Error during raw reset. MMU_DTE_ADDR is not functioning iommu driver failed to attach a compatible domain WARNING: CPU: 5 PID: 7555 at drivers/iommu/iommu.c:2116 __iommu_attach_group+0x9c/0xac The hardware needs none of that to change page tables. On an already enabled MMU a page table swap is stall, write RK_MMU_DTE_ADDR on every bank, ZAP_CACHE, unstall, with paging left on throughout. rk_iommu_switch_domain() does exactly that, and also moves the rk_iommu between the two domains' iommus lists so rk_iommu_zap_iova() and the TLB flush paths still name the live domain. Because the core is not involved, iommu_get_domain_for_dev() does not track such a switch and a caller must track the live domain itself. rk_iommu_reprogram() re-establishes the MMU for the currently attached domain after a hardware reset has wiped it. It is deliberately unconditional: the domain has not changed, so a switch helper would short-circuit, but the hardware state is gone. Both are exported for the rknpu driver, which is the only in-tree user of multiple domains on one master. No behaviour changes until a caller uses them. --- drivers/iommu/rockchip-iommu.c | 119 +++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 5f86a0db4a2d3..1afe988c91833 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1323,6 +1323,125 @@ static void rk_iommu_detach_device(struct iommu_domain *domain, } } +/* + * Switch a master to a different domain by reprogramming the page table base in + * place, without going through the IOMMU core. + * + * The core path for this is iommu_detach_device() + iommu_attach_device(), and + * for rockchip-iommu the attach runs rk_iommu_enable(), i.e. a full + * rk_iommu_force_reset() of every MMU bank. With work in flight on other cores + * that reset intermittently fails its DTE_ADDR readback ("Error during raw + * reset. MMU_DTE_ADDR is not functioning"), the attach then fails, and the core + * leaves the master attached to no domain at all. + * + * The hardware does not need any of that to change page tables. On an already + * enabled MMU it is: + * + * enable_stall -> write DTE_ADDR on every bank -> ZAP_CACHE -> disable_stall + * + * with paging left on throughout. No reset, no paging off/on. + * + * Because the core is not involved, iommu_get_domain_for_dev() does NOT track + * this switch; a caller using it must track the live domain itself. The caller + * is also responsible for ensuring no DMA is in flight across the swap. + */ +int rk_iommu_switch_domain(struct device *dev, struct iommu_domain *domain) +{ + struct rk_iommu *iommu = rk_iommu_from_dev(dev); + struct rk_iommu_domain *rk_domain; + unsigned long flags; + int ret, i; + + if (!iommu || !domain) + return -ENODEV; + + rk_domain = to_rk_domain(domain); + + /* the third-party ops wrapper owns its own attach path */ + if (rk_domain->opt_ops) + return -EOPNOTSUPP; + + if (iommu->domain == domain) + return 0; + + /* + * Move this iommu between the two domains' iommus lists: rk_iommu_zap_iova() + * and the TLB flush paths walk that list, so it has to name the live domain. + */ + if (iommu->domain) { + struct rk_iommu_domain *old = to_rk_domain(iommu->domain); + + spin_lock_irqsave(&old->iommus_lock, flags); + list_del_init(&iommu->node); + spin_unlock_irqrestore(&old->iommus_lock, flags); + } + iommu->domain = domain; + spin_lock_irqsave(&rk_domain->iommus_lock, flags); + list_add_tail(&iommu->node, &rk_domain->iommus); + spin_unlock_irqrestore(&rk_domain->iommus_lock, flags); + rk_domain->shootdown_entire = iommu->shootdown_entire; + + ret = pm_runtime_get_if_in_use(iommu->dev); + if (!ret || WARN_ON_ONCE(ret < 0)) { + /* not runtime-active: rk_iommu_resume() programs DTE_ADDR from iommu->domain */ + return 0; + } + + ret = clk_bulk_enable(iommu->num_clocks, iommu->clocks); + if (ret) + goto out_pm_put; + + ret = rk_iommu_enable_stall(iommu); + if (ret) + goto out_disable_clocks; + + for (i = 0; i < iommu->num_mmu; i++) { + rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR, + rk_ops->mk_dtentries(rk_domain->dt_dma)); + rk_iommu_base_command(iommu->bases[i], RK_MMU_CMD_ZAP_CACHE); + } + + rk_iommu_disable_stall(iommu); + +out_disable_clocks: + clk_bulk_disable(iommu->num_clocks, iommu->clocks); +out_pm_put: + pm_runtime_put(iommu->dev); + return ret; +} +EXPORT_SYMBOL(rk_iommu_switch_domain); + +/* + * Re-establish the MMU for the currently attached domain, unconditionally. + * + * A hardware soft reset wipes the MMU (DTE_ADDR, paging), so the page table has + * to be reprogrammed afterwards. rk_iommu_switch_domain() deliberately + * short-circuits when the domain has not changed, which is exactly wrong here: + * the domain is the same, the hardware is not. + * + * iommu->domain is the live domain, so reprogramming from it is correct by + * construction and does not depend on what the IOMMU core believes. + */ +int rk_iommu_reprogram(struct device *dev) +{ + struct rk_iommu *iommu = rk_iommu_from_dev(dev); + int ret; + + if (!iommu || !iommu->domain) + return -ENODEV; + if (to_rk_domain(iommu->domain)->opt_ops) + return -EOPNOTSUPP; + + ret = pm_runtime_get_if_in_use(iommu->dev); + if (!ret || WARN_ON_ONCE(ret < 0)) + return 0; /* not runtime-active: rk_iommu_resume() will program it */ + + ret = rk_iommu_enable(iommu); + pm_runtime_put(iommu->dev); + return ret; +} +EXPORT_SYMBOL(rk_iommu_reprogram); + static int rk_iommu_attach_device(struct iommu_domain *domain, struct device *dev) { From 1a253e6968309e3a4d5857a8ddbba6d70c0f3e63 Mon Sep 17 00:00:00 2001 From: Michael Fischer Date: Fri, 28 Aug 2026 15:36:52 -0500 Subject: [PATCH 07/10] rknpu: resolve the live IOMMU domain from the driver's own state This driver is the only in-tree user of several IOMMU domains on one master, and it tracks which one is live itself, in rknpu_dev->iommu_domain_id. iommu_get_domain_for_dev() returns what the core last attached, which coincides with the live domain only while switching goes through the core. Introduce rknpu_iommu_live_domain() and use it everywhere a mapping, unmap or TLB flush has to target the domain the NPU is actually running in: - rknpu_iommu_dma_map_sg() / rknpu_iommu_dma_unmap_sg() - the post-map iommu_flush_iotlb_all() in rknpu_gem_get_pages() - the iommu_map()/iommu_unmap() of SRAM and NBUF cache buffers No functional change on its own: while switching still goes through the core the two answers are identical. This is preparation for the following commit, which takes the core out of the switch path -- after which they are not. --- drivers/rknpu/include/rknpu_iommu.h | 9 +++++++++ drivers/rknpu/rknpu_gem.c | 6 +++--- drivers/rknpu/rknpu_iommu.c | 17 +++++++++++++++-- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/drivers/rknpu/include/rknpu_iommu.h b/drivers/rknpu/include/rknpu_iommu.h index 225176650b605..0c8da94022bba 100644 --- a/drivers/rknpu/include/rknpu_iommu.h +++ b/drivers/rknpu/include/rknpu_iommu.h @@ -53,6 +53,15 @@ int rknpu_iommu_domain_get_and_switch(struct rknpu_device *rknpu_dev, int domain_id); int rknpu_iommu_domain_put(struct rknpu_device *rknpu_dev); +/* + * The domain the NPU is actually running in, from the driver's own state. + * + * iommu_get_domain_for_dev() returns what the IOMMU core last attached, which is + * only the same thing while domain switching goes through the core. Everything in + * this driver that must target the live domain uses this instead. + */ +struct iommu_domain *rknpu_iommu_live_domain(struct device *dev); + #if KERNEL_VERSION(5, 10, 0) < LINUX_VERSION_CODE int iommu_get_dma_cookie(struct iommu_domain *domain); #endif diff --git a/drivers/rknpu/rknpu_gem.c b/drivers/rknpu/rknpu_gem.c index f1346f98fff62..f07e8908b9132 100644 --- a/drivers/rknpu/rknpu_gem.c +++ b/drivers/rknpu/rknpu_gem.c @@ -72,7 +72,7 @@ static int rknpu_gem_get_pages(struct rknpu_gem_object *rknpu_obj) rknpu_obj->size); goto free_sgt; } - iommu_flush_iotlb_all(iommu_get_domain_for_dev(drm->dev)); + iommu_flush_iotlb_all(rknpu_iommu_live_domain(drm->dev)); if (rknpu_obj->flags & RKNPU_MEM_KERNEL_MAPPING) { rknpu_obj->cookie = vmap(rknpu_obj->pages, rknpu_obj->num_pages, @@ -501,7 +501,7 @@ static int rknpu_gem_alloc_buf_with_cache(struct rknpu_gem_object *rknpu_obj, } /* iova map to cache */ - domain = iommu_get_domain_for_dev(rknpu_dev->dev); + domain = rknpu_iommu_live_domain(rknpu_dev->dev); if (!domain) { LOG_ERROR("failed to get iommu domain!"); return -EINVAL; @@ -654,7 +654,7 @@ static void rknpu_gem_free_buf_with_cache(struct rknpu_gem_object *rknpu_obj, return; } - domain = iommu_get_domain_for_dev(rknpu_dev->dev); + domain = rknpu_iommu_live_domain(rknpu_dev->dev); if (domain) { iommu_unmap(domain, rknpu_obj->iova_start, cache_size); if (rknpu_obj->size > 0) diff --git a/drivers/rknpu/rknpu_iommu.c b/drivers/rknpu/rknpu_iommu.c index e9e9f3652d302..4683d79346caf 100644 --- a/drivers/rknpu/rknpu_iommu.c +++ b/drivers/rknpu/rknpu_iommu.c @@ -79,6 +79,19 @@ void rknpu_iommu_dma_free_iova(struct rknpu_iommu_dma_cookie *cookie, free_iova(iovad, iova_pfn(iovad, iova)); } +struct iommu_domain *rknpu_iommu_live_domain(struct device *dev) +{ + struct rknpu_device *rknpu_dev = dev_get_drvdata(dev); + int id; + + if (!rknpu_dev) + return NULL; + id = rknpu_dev->iommu_domain_id; + if (id < 0 || id >= RKNPU_MAX_IOMMU_DOMAIN_NUM) + return NULL; + return rknpu_dev->iommu_domains[id]; +} + static int rknpu_dma_info_to_prot(enum dma_data_direction dir, bool coherent) { int prot = coherent ? IOMMU_CACHE : 0; @@ -208,7 +221,7 @@ int rknpu_iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents, enum dma_data_direction dir, bool iova_aligned) { - struct iommu_domain *domain = iommu_get_domain_for_dev(dev); + struct iommu_domain *domain = rknpu_iommu_live_domain(dev); struct rknpu_iommu_dma_cookie *cookie = (void *)domain->iova_cookie; struct iova_domain *iovad = &cookie->iovad; struct scatterlist *s = NULL, *prev = NULL; @@ -298,7 +311,7 @@ void rknpu_iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents, enum dma_data_direction dir, bool iova_aligned) { - struct iommu_domain *domain = iommu_get_domain_for_dev(dev); + struct iommu_domain *domain = rknpu_iommu_live_domain(dev); struct rknpu_iommu_dma_cookie *cookie = (void *)domain->iova_cookie; struct iova_domain *iovad = &cookie->iovad; size_t iova_off = 0; From fe4671d77f741421f3ddf79e5a8140ef183ef131 Mon Sep 17 00:00:00 2001 From: Michael Fischer Date: Fri, 28 Aug 2026 15:38:30 -0500 Subject: [PATCH 08/10] rknpu: switch IOMMU domains without going through the IOMMU core rknpu_iommu_switch_domain() changed domains with iommu_detach_device() followed by iommu_attach_device(). For rockchip-iommu the attach runs rk_iommu_enable(), i.e. a full rk_iommu_force_reset() of every MMU bank, and with work in flight on other cores that reset intermittently fails its DTE_ADDR readback. The attach then fails and the master is left attached to no domain: rk_iommu fdab9000.iommu: Error during raw reset. MMU_DTE_ADDR is not functioning iommu driver failed to attach a compatible domain WARNING: CPU: 5 PID: 7555 at drivers/iommu/iommu.c:2116 __iommu_attach_group+0x9c/0xac Use rk_iommu_switch_domain() instead, which reprograms RK_MMU_DTE_ADDR in place and never resets. Three consequences follow, all handled here: - The mismatch check against iommu_get_domain_for_dev() goes away. That function no longer tracks these switches, so the comparison could only produce a spurious failure. The driver's own iommu_domains[] is the source of truth. - A new domain gets its IOVA allocator initialised directly with init_iova_domain() + iova_domain_init_rcaches(), instead of being made to look like a DMA-API domain (setting __IOMMU_DOMAIN_DMA_API on the type and calling iommu_setup_dma_ops()) purely so that iommu_dma_init_domain() would initialise the cookie's iovad. That route depends on the core reporting this domain as the device's, which is exactly what is no longer true. The domain stays a plain unmanaged domain, which is what iommu_map_sg() and iommu_unmap() want. - The permanent overwrite of iommu_group->default_domain is removed. It existed because dma-iommu resolves every DMA-API mapping through iommu_get_dma_domain() == group->default_domain, so it was the only way to make a DMA-API mapping land in a non-default domain. Leaving the core's default pointing at a domain the driver may later free means any core path that reattaches "the default" uses it. The NPU's own buffers are mapped explicitly against the domain the driver names, so nothing here needs it. Note the private copy of struct iommu_group is retained: a *scoped* override is still required for the dma-buf attachment window, because the DMA API offers no way to name a target domain. That is added in a later commit and is restored immediately, unlike the permanent overwrite removed here. rknpu_iommu_free_domains() correspondingly drops its iommu_detach_device() -- the core never attached these domains -- and releases the IOVA allocator this function created. --- drivers/rknpu/include/rknpu_iommu.h | 8 +++ drivers/rknpu/rknpu_iommu.c | 107 +++++++++++++++++++++------- 2 files changed, 90 insertions(+), 25 deletions(-) diff --git a/drivers/rknpu/include/rknpu_iommu.h b/drivers/rknpu/include/rknpu_iommu.h index 0c8da94022bba..8c0092c688ef9 100644 --- a/drivers/rknpu/include/rknpu_iommu.h +++ b/drivers/rknpu/include/rknpu_iommu.h @@ -62,6 +62,14 @@ int rknpu_iommu_domain_put(struct rknpu_device *rknpu_dev); */ struct iommu_domain *rknpu_iommu_live_domain(struct device *dev); +/* + * Provided by drivers/iommu/rockchip-iommu.c. Switching with these keeps the IOMMU + * core out of the path, which is what avoids the rk_iommu_force_reset() that an + * iommu_attach_device() would perform on every domain change. + */ +extern int rk_iommu_switch_domain(struct device *dev, struct iommu_domain *domain); +extern int rk_iommu_reprogram(struct device *dev); + #if KERNEL_VERSION(5, 10, 0) < LINUX_VERSION_CODE int iommu_get_dma_cookie(struct iommu_domain *domain); #endif diff --git a/drivers/rknpu/rknpu_iommu.c b/drivers/rknpu/rknpu_iommu.c index 4683d79346caf..cdfeccbb33211 100644 --- a/drivers/rknpu/rknpu_iommu.c +++ b/drivers/rknpu/rknpu_iommu.c @@ -5,6 +5,8 @@ */ #include +#include +#include #include #include @@ -456,29 +458,32 @@ int rknpu_iommu_switch_domain(struct rknpu_device *rknpu_dev, int domain_id) return 0; } - src_domain = iommu_get_domain_for_dev(rknpu_dev->dev); - if (src_domain != rknpu_dev->iommu_domains[src_domain_id]) { - LOG_DEV_ERROR( - rknpu_dev->dev, - "mismatch domain get from iommu_get_domain_for_dev\n"); - return -EINVAL; - } + /* + * The driver's own record is the source of truth for which domain is live. + * iommu_get_domain_for_dev() no longer tracks these switches, so comparing + * against it could only ever produce a spurious mismatch. + */ + src_domain = rknpu_dev->iommu_domains[src_domain_id]; dst_domain = rknpu_dev->iommu_domains[domain_id]; if (dst_domain != NULL) { - iommu_detach_device(src_domain, rknpu_dev->dev); - ret = iommu_attach_device(dst_domain, rknpu_dev->dev); + ret = rk_iommu_switch_domain(rknpu_dev->dev, dst_domain); if (ret) { LOG_DEV_ERROR( rknpu_dev->dev, - "failed to attach dst iommu domain, id: %d, ret: %d\n", + "failed to switch to iommu domain, id: %d, ret: %d\n", domain_id, ret); - if (iommu_attach_device(src_domain, rknpu_dev->dev)) { + /* + * Unlike the detach/attach pair this replaces, a failed + * switch leaves the previous page table live rather than + * leaving the master attached to nothing, so restoring is + * belt and braces. + */ + if (rk_iommu_switch_domain(rknpu_dev->dev, src_domain)) LOG_DEV_ERROR( rknpu_dev->dev, - "failed to reattach src iommu domain, id: %d\n", + "failed to restore src iommu domain, id: %d\n", src_domain_id); - } return ret; } rknpu_dev->iommu_domain_id = domain_id; @@ -491,32 +496,74 @@ int rknpu_iommu_switch_domain(struct rknpu_device *rknpu_dev, int domain_id) "failed to allocate iommu domain\n"); return -EIO; } - // init domain iova_cookie iommu_get_dma_cookie(dst_domain); - iommu_detach_device(src_domain, rknpu_dev->dev); - ret = iommu_attach_device(dst_domain, rknpu_dev->dev); + /* + * Initialise this domain's IOVA allocator directly. + * + * The previous route got one by pretending the domain was a DMA-API + * domain -- setting __IOMMU_DOMAIN_DMA_API on the type and calling + * iommu_setup_dma_ops(), whose only useful effect here was + * iommu_dma_init_domain() initialising the cookie's iovad. That + * depends on iommu_get_domain_for_dev() reporting this domain, i.e. + * on the default-domain overwrite removed below. Do the two init + * calls directly instead: same iovad, and the domain stays a plain + * unmanaged domain that iommu_map_sg() and iommu_unmap() serve. + * + * Granule and start_pfn mirror iommu_dma_init_domain(): 4 KiB pages, + * base_pfn 1 so IOVA 0 is never handed out. + */ + { + struct rknpu_iommu_dma_cookie *ck = + (struct rknpu_iommu_dma_cookie *)dst_domain->iova_cookie; + + if (!ck) { + LOG_DEV_ERROR(rknpu_dev->dev, + "no iova cookie for domain %d\n", + domain_id); + iommu_domain_free(dst_domain); + return -ENOMEM; + } + init_iova_domain(&ck->iovad, SZ_4K, 1); + if (iova_domain_init_rcaches(&ck->iovad)) { + LOG_DEV_ERROR(rknpu_dev->dev, + "failed to init iova rcaches, domain %d\n", + domain_id); + iommu_domain_free(dst_domain); + return -ENOMEM; + } + } + + ret = rk_iommu_switch_domain(rknpu_dev->dev, dst_domain); if (ret) { LOG_DEV_ERROR( rknpu_dev->dev, - "failed to attach iommu domain, id: %d, ret: %d\n", + "failed to switch to iommu domain, id: %d, ret: %d\n", domain_id, ret); iommu_domain_free(dst_domain); return ret; } - - // set domain type to dma domain - dst_domain->type |= __IOMMU_DOMAIN_DMA_API; - // iommu dma init domain - iommu_setup_dma_ops(rknpu_dev->dev, 0, dma_limit); + (void)dma_limit; rknpu_dev->iommu_domain_id = domain_id; rknpu_dev->iommu_domains[domain_id] = dst_domain; rknpu_dev->iommu_domain_num++; } - // reset default iommu domain - rknpu_dev->iommu_group->default_domain = dst_domain; + /* + * The default-domain overwrite that used to live here is gone. + * + * dma-iommu resolves every DMA-API mapping through + * iommu_get_dma_domain() == dev->iommu_group->default_domain, so the only + * way to make a DMA-API mapping land in domain N was to tell the core that + * N was the group's default -- reaching the field through a private copy of + * struct iommu_group. That leaves the core's idea of the default pointing at + * a domain this driver may later free, and any core path that reattaches + * "the default" then uses it. + * + * Nothing here needs it any more: the NPU's buffers are mapped explicitly + * with iommu_map_sg()/iommu_unmap() against the domain the driver names. + */ LOG_INFO("switch iommu domain from %d to %d\n", src_domain_id, domain_id); @@ -611,7 +658,17 @@ void rknpu_iommu_free_domains(struct rknpu_device *rknpu_dev) if (domain == NULL) continue; - iommu_detach_device(domain, rknpu_dev->dev); + /* + * No iommu_detach_device(): the core never attached these domains. + * The switch to domain 0 above already moved the hardware off this + * page table. Release the IOVA allocator initialised in the switch. + */ + if (domain->iova_cookie) { + struct rknpu_iommu_dma_cookie *ck = + (struct rknpu_iommu_dma_cookie *)domain->iova_cookie; + + put_iova_domain(&ck->iovad); + } iommu_domain_free(domain); rknpu_dev->iommu_domains[i] = NULL; From 51c2f16b07b2e31f8800b9f3cafbecfcc3e4bde9 Mon Sep 17 00:00:00 2001 From: Michael Fischer Date: Fri, 28 Aug 2026 15:38:31 -0500 Subject: [PATCH 09/10] rknpu: reprogram the live IOMMU domain after a soft reset A soft reset wipes the MMU, so the page table must be reprogrammed afterwards. That was done with iommu_detach_device() + iommu_attach_device() on iommu_get_domain_for_dev(), which is the group's default domain. That was only ever correct because the driver overwrote the default to follow its own switches. It no longer does, so this would re-attach domain 0 while the driver still believes domain N is live. Every IOVA the next job uses then resolves against the wrong page table: the job is committed and never completes, with no interrupt and no error, and the output buffer is left untouched. Use rk_iommu_reprogram(), which re-enables the MMU from the live domain rather than from whatever the core last attached. --- drivers/rknpu/rknpu_reset.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/rknpu/rknpu_reset.c b/drivers/rknpu/rknpu_reset.c index 65142368c9fd8..2974617151ed8 100644 --- a/drivers/rknpu/rknpu_reset.c +++ b/drivers/rknpu/rknpu_reset.c @@ -8,6 +8,7 @@ #include #include "rknpu_reset.h" +#include "rknpu_iommu.h" #ifndef FPGA_PLATFORM static inline struct reset_control *rknpu_reset_control_get(struct device *dev, @@ -138,13 +139,24 @@ int rknpu_soft_reset(struct rknpu_device *rknpu_dev) return ret; } - if (rknpu_dev->iommu_en) - domain = iommu_get_domain_for_dev(rknpu_dev->dev); - - if (domain) { - iommu_detach_device(domain, rknpu_dev->dev); - iommu_attach_device(domain, rknpu_dev->dev); + /* + * A soft reset wipes the MMU, so the page table has to be reprogrammed. This + * used to be a detach/attach of iommu_get_domain_for_dev(), which is the + * core's default domain -- correct only while the driver overwrote that to + * follow its switches. It no longer does, so re-establish the live domain + * explicitly instead; otherwise the reset silently leaves the NPU pointed at + * domain 0 while the driver believes domain N is live, and the next job is + * committed against a page table where its IOVAs do not exist. + */ + if (rknpu_dev->iommu_en) { + int rp = rk_iommu_reprogram(rknpu_dev->dev); + + if (rp) + LOG_DEV_ERROR(rknpu_dev->dev, + "failed to reprogram iommu after reset: %d\n", + rp); } + (void)domain; rknpu_dev->soft_reseting = false; From 23a11aeefad385fd5682e589f0151db6bae02b83 Mon Sep 17 00:00:00 2001 From: Michael Fischer Date: Fri, 28 Aug 2026 15:39:55 -0500 Subject: [PATCH 10/10] rknpu: map dma-buf imports into the domain they will be used in An imported dma-buf's sg is mapped in two places, and neither of them named the domain the buffer would actually be used in. First, dma_buf_map_attachment() runs inside DRM core's PRIME_FD_TO_HANDLE, before any rknpu hook, and maps through the DMA API -- which always targets group->default_domain. While this driver permanently overwrote that default the mapping happened to land in the live domain; with that overwrite gone it lands in domain 0, while rknpu_gem_prime_import_sg_table() records the object as belonging to the live domain. A submit against such a buffer then resolves an IOVA that exists only in another page table: the job is committed and never completes, with no interrupt, no fault and no error, and userspace sees an untouched output buffer. Second, PRIME_FD_TO_HANDLE carries no domain at all. The target is named only by the MEM_CREATE that follows, which does not re-map an already-imported handle, so the mapping lands wherever the device happened to be. Fix both ends: - Override the core's default domain for the duration of the attachment map, in the .gem_prime_import hook, and for the matching unmap at destroy. This is scoped to one call, taken under domain_lock so a concurrent switch cannot move the live domain underneath it, and restored immediately -- unlike the permanent overwrite removed earlier in this series. - Add RKNPU_ACT_SET_DOMAIN, a switch-only action with no allocation, so userspace can make the target domain live before PRIME_FD_TO_HANDLE. An alternative to the second half would be for MEM_CREATE to re-map an imported handle when the requested domain differs from the one it was mapped in, which would avoid new uABI. That is arguably cleaner, but it is not what was implemented and measured here, so it is offered as a note rather than a claim. Native (non-imported) allocations were never affected: their MEM_CREATE performs the switch and the mapping together. --- drivers/rknpu/include/rknpu_ioctl.h | 10 +++++++ drivers/rknpu/include/rknpu_iommu.h | 7 +++++ drivers/rknpu/rknpu_drv.c | 8 ++++++ drivers/rknpu/rknpu_gem.c | 42 +++++++++++++++++++++++++++-- drivers/rknpu/rknpu_iommu.c | 29 ++++++++++++++++++++ 5 files changed, 94 insertions(+), 2 deletions(-) diff --git a/drivers/rknpu/include/rknpu_ioctl.h b/drivers/rknpu/include/rknpu_ioctl.h index f4e3b83bfc5af..ed06ee03e64c2 100644 --- a/drivers/rknpu/include/rknpu_ioctl.h +++ b/drivers/rknpu/include/rknpu_ioctl.h @@ -122,6 +122,16 @@ enum e_rknpu_action { RKNPU_GET_BW_TW = 11, RKNPU_SET_BW_TW = 12, RKNPU_ACT_CLR_TOTAL_RW_AMOUNT = 13, + /* + * Make an IOMMU domain live, with no allocation. `value` is the domain id. + * + * A dma-buf import maps its sg during PRIME_FD_TO_HANDLE, into whichever + * domain is live at that instant, because the ioctl carries no domain. The + * target is named only by the later MEM_CREATE, which does not re-map an + * already-imported handle. Userspace issues this first so the mapping lands + * in the domain the buffer will actually be used in. + */ + RKNPU_ACT_SET_DOMAIN = 0x100, RKNPU_GET_DT_WR_AMOUNT = 14, RKNPU_GET_DT_RD_AMOUNT = 15, RKNPU_GET_WT_RD_AMOUNT = 16, diff --git a/drivers/rknpu/include/rknpu_iommu.h b/drivers/rknpu/include/rknpu_iommu.h index 8c0092c688ef9..c336600a717b4 100644 --- a/drivers/rknpu/include/rknpu_iommu.h +++ b/drivers/rknpu/include/rknpu_iommu.h @@ -70,6 +70,13 @@ struct iommu_domain *rknpu_iommu_live_domain(struct device *dev); extern int rk_iommu_switch_domain(struct device *dev, struct iommu_domain *domain); extern int rk_iommu_reprogram(struct device *dev); +/* + * Scoped override of the core's default domain, for the dma-buf attachment map and + * its matching unmap only. Returns the previous value; the caller restores it. + */ +struct iommu_domain *rknpu_iommu_default_swap(struct device *dev, + struct iommu_domain *dom); + #if KERNEL_VERSION(5, 10, 0) < LINUX_VERSION_CODE int iommu_get_dma_cookie(struct iommu_domain *domain); #endif diff --git a/drivers/rknpu/rknpu_drv.c b/drivers/rknpu/rknpu_drv.c index 9e6f1d28a55c2..b90afa8f83758 100644 --- a/drivers/rknpu/rknpu_drv.c +++ b/drivers/rknpu/rknpu_drv.c @@ -466,6 +466,14 @@ static int rknpu_action(struct rknpu_device *rknpu_dev, case RKNPU_SET_BW_TW: ret = rknpu_set_bw_priority(rknpu_dev, 0, 0, args->value); break; + case RKNPU_ACT_SET_DOMAIN: { + int _id = (int)args->value; + + ret = rknpu_iommu_domain_get_and_switch(rknpu_dev, _id); + if (!ret) + rknpu_iommu_domain_put(rknpu_dev); /* switch only, hold no reference */ + break; + } case RKNPU_ACT_CLR_TOTAL_RW_AMOUNT: ret = rknpu_clear_rw_amount(rknpu_dev); break; diff --git a/drivers/rknpu/rknpu_gem.c b/drivers/rknpu/rknpu_gem.c index f07e8908b9132..0d51afbb4f5e7 100644 --- a/drivers/rknpu/rknpu_gem.c +++ b/drivers/rknpu/rknpu_gem.c @@ -850,7 +850,22 @@ void rknpu_gem_object_destroy(struct rknpu_gem_object *rknpu_obj) * once dmabuf's refcount becomes 0. */ if (obj->import_attach) { - drm_prime_gem_destroy(obj, rknpu_obj->sgt); + { + /* + * dma_buf_unmap_attachment() unmaps through the DMA API + * too, so it must see the same default domain the map saw. + * The caller has already switched to this object's recorded + * domain, so the live domain is the right one. + */ + struct iommu_domain *l = rknpu_dev->iommu_en ? + rknpu_iommu_live_domain(rknpu_dev->dev) : NULL; + struct iommu_domain *sv = l ? + rknpu_iommu_default_swap(rknpu_dev->dev, l) : NULL; + + drm_prime_gem_destroy(obj, rknpu_obj->sgt); + if (sv) + rknpu_iommu_default_swap(rknpu_dev->dev, sv); + } rknpu_gem_free_page(rknpu_obj->pages); } else { if (IS_ENABLED(CONFIG_ROCKCHIP_RKNPU_SRAM) && @@ -1398,7 +1413,30 @@ int rknpu_gem_mmap(struct file *filp, struct vm_area_struct *vma) struct drm_gem_object *rknpu_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf) { - return drm_gem_prime_import_dev(dev, dma_buf, dev->dev); + struct rknpu_device *rknpu_dev = dev->dev_private; + struct drm_gem_object *obj; + struct iommu_domain *live, *saved = NULL; + + if (!rknpu_dev->iommu_en) + return drm_gem_prime_import_dev(dev, dma_buf, dev->dev); + + /* + * DRM core maps the attachment through the DMA API here, which always + * targets group->default_domain. Point that at the live domain for the + * duration of the import so the sg is mapped where the NPU will run, then + * restore it. domain_lock keeps a concurrent switch from moving the live + * domain underneath the map. + */ + mutex_lock(&rknpu_dev->domain_lock); + live = rknpu_iommu_live_domain(rknpu_dev->dev); + if (live) + saved = rknpu_iommu_default_swap(rknpu_dev->dev, live); + obj = drm_gem_prime_import_dev(dev, dma_buf, dev->dev); + if (saved) + rknpu_iommu_default_swap(rknpu_dev->dev, saved); + mutex_unlock(&rknpu_dev->domain_lock); + + return obj; } #endif diff --git a/drivers/rknpu/rknpu_iommu.c b/drivers/rknpu/rknpu_iommu.c index cdfeccbb33211..be3cdd57aee54 100644 --- a/drivers/rknpu/rknpu_iommu.c +++ b/drivers/rknpu/rknpu_iommu.c @@ -418,6 +418,35 @@ struct iommu_group { }; #endif +/* + * Temporarily point the IOMMU core's default domain at the live domain. + * + * dma-iommu resolves every DMA-API mapping through + * iommu_get_dma_domain() == group->default_domain, and dma_buf_map_attachment(), + * which DRM core runs during PRIME_FD_TO_HANDLE, goes through the DMA API. There + * is no way to tell it which domain to use. + * + * This is deliberately not the permanent overwrite that used to live in + * rknpu_iommu_switch_domain(). That left the core's default pointing at a domain + * the driver might later free, for the lifetime of the device. This override lasts + * only for one attachment map or unmap, is taken under domain_lock so no switch can + * move the live domain underneath it, and is restored immediately. + * + * Returns the previous default so the caller can restore it. + */ +struct iommu_domain *rknpu_iommu_default_swap(struct device *dev, + struct iommu_domain *dom) +{ + struct rknpu_device *rknpu_dev = dev_get_drvdata(dev); + struct iommu_domain *old; + + if (!rknpu_dev || !rknpu_dev->iommu_group || !dom) + return NULL; + old = rknpu_dev->iommu_group->default_domain; + rknpu_dev->iommu_group->default_domain = dom; + return old; +} + int rknpu_iommu_init_domain(struct rknpu_device *rknpu_dev) { // init domain 0