diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index c48af6b2d..b21d36896 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -30,6 +30,23 @@ /** MMU register offsets */ #define RK_MMU_DTE_ADDR 0x00 /* Directory table address */ + +/* #patch29 (diagnostic): rk_iommu_attach_device() returns SUCCESS without touching hardware + * when the IOMMU is not runtime-active -- the domain is recorded in software but + * RK_MMU_DTE_ADDR still points at the PREVIOUS page table. rk_iommu_resume() is supposed to + * program it later. If a master (the NPU) issues DMA in the window between the two, it walks + * the WRONG table; because sibling domains allocate overlapping IOVAs the access RESOLVES + * instead of faulting, so the failure is completely silent. Count both halves so the window + * can be correlated with NPU dispatch stalls. */ +unsigned long rk_dbg_attach_deferred; +unsigned long rk_dbg_attach_programmed; +unsigned long rk_dbg_resume_programmed; +EXPORT_SYMBOL(rk_dbg_attach_deferred); +EXPORT_SYMBOL(rk_dbg_attach_programmed); +EXPORT_SYMBOL(rk_dbg_resume_programmed); +module_param_named(dbg_attach_deferred, rk_dbg_attach_deferred, ulong, 0444); +module_param_named(dbg_attach_programmed, rk_dbg_attach_programmed, ulong, 0444); +module_param_named(dbg_resume_programmed, rk_dbg_resume_programmed, ulong, 0444); #define RK_MMU_STATUS 0x04 #define RK_MMU_COMMAND 0x08 #define RK_MMU_PAGE_FAULT_ADDR 0x0C /* IOVA of last page fault */ @@ -1292,6 +1309,154 @@ static void rk_iommu_detach_device(struct iommu_domain *domain, } } +/* #patchB1: LIGHT DOMAIN SWITCH (no force-reset, no IOMMU-core involvement). + * + * Moving a master between IOMMU domains through the core (iommu_detach_device + + * iommu_attach_device) costs a FULL rk_iommu_force_reset() of every MMU bank. Worse, because + * __iommu_group_set_core_domain() reattaches the group's DEFAULT domain on detach, and + * __iommu_attach_group() then refuses with -EBUSY unless group->domain == group->default_domain, + * a naive detach/attach pair costs TWO force-resets per switch -- unless the caller overwrites + * iommu_group->default_domain, which is exactly what the vendor rknpu multi-domain code does and + * is the origin of its "mismatch domain get from iommu_get_domain_for_dev" failures. + * + * The hardware needs none of that. Swapping the page table on an already-enabled MMU is: + * + * enable_stall -> write DTE_ADDR on every bank -> ZAP_CACHE -> disable_stall + * + * with paging left ON throughout. That is all this does. No reset, no paging off/on, no core + * detach/attach -- so the caller owns which domain is live and never has to lie to the core about + * which one is "default". iommu_get_domain_for_dev() therefore does NOT track this switch; a + * caller using it must track the live domain itself. + * + * The caller is responsible for ensuring no DMA is in flight across the swap. + */ +/* #patchB6: how thoroughly a domain switch flushes the MMU. + * + * 1 (default) = full rk_iommu_enable(): FORCE_RESET (polled to completion) -> write DTE_ADDR -> + * ZAP_CACHE -> INT_MASK/AUTO_GATING -> enable_paging. + * 0 = light: stall -> write DTE_ADDR -> ZAP_CACHE -> unstall, paging left on. + * + * The light path assumed ZAP_CACHE alone retires the old translations. Suspected insufficient: a + * freshly switched domain returned WRONG DATA on its FIRST access and correct data on every access + * after, which is the signature of the walker still holding the previous page table for one + * translation and refilling correctly afterwards. ZAP_CACHE is issued with no completion poll, + * whereas FORCE_RESET is polled (rk_iommu_is_reset_done). + * + * Even at 1 this is still a large win over the stock path: the IOMMU core is not involved, so + * there is no detach/attach pair (which cost TWO force-resets per switch) and no need to overwrite + * iommu_group->default_domain. */ +static unsigned int rk_switch_reset = 1; +module_param_named(switch_reset, rk_switch_reset, uint, 0644); +MODULE_PARM_DESC(switch_reset, "domain switch flush: 1=full reset+paging (default), 0=light zap only"); + +/* #patchB8: re-establish the MMU for the CURRENTLY ATTACHED domain, unconditionally. + * + * A hardware soft reset clears the MMU (DTE_ADDR, paging), so the master's page table must be + * reprogrammed afterwards. The vendor rknpu reset did that with + * iommu_detach_device(iommu_get_domain_for_dev(dev)) + iommu_attach_device(...), which only worked + * because the old multi-domain code overwrote iommu_group->default_domain on every switch, so the + * core's idea of the domain happened to match the live one. With the light switch (#patchB1) the + * core is deliberately NOT in the loop, so iommu_get_domain_for_dev() reports the group's default + * (domain 0) and that sequence silently re-attaches the WRONG page table -- the driver still thinks + * domain N is live, its IOVAs resolve against domain 0, and the next job never completes. + * + * iommu->domain IS the live domain (rk_iommu_switch_domain maintains it), so reprogramming from it + * is correct by construction. Unconditional: rk_iommu_switch_domain() short-circuits when the domain + * is unchanged, which is exactly wrong here -- the domain has not changed, the HARDWARE has been + * wiped. */ +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); + +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; not ours to shortcut */ + 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 must reflect 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; + } + + if (rk_switch_reset) { + /* #patchB6: same MMU programming attach_device would do (reset + DTE + ZAP + + * paging), but WITHOUT the IOMMU-core detach/attach around it. rk_iommu_enable() + * manages its own clocks and stall, and reads iommu->domain, which is already the + * new domain here. */ + ret = rk_iommu_enable(iommu); + goto out_pm_put; + } + + 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); + static int rk_iommu_attach_device(struct iommu_domain *domain, struct device *dev) { @@ -1339,9 +1504,16 @@ static int rk_iommu_attach_device(struct iommu_domain *domain, rk_domain->shootdown_entire = iommu->shootdown_entire; ret = pm_runtime_get_if_in_use(iommu->dev); - if (!ret || WARN_ON_ONCE(ret < 0)) + if (!ret || WARN_ON_ONCE(ret < 0)) { + /* #patch29: HW page table NOT reprogrammed; deferred to rk_iommu_resume() */ + rk_dbg_attach_deferred++; + pr_warn_ratelimited( + "rk_iommu: attach DEFERRED (not runtime-active), DTE_ADDR still stale (n=%lu)\n", + rk_dbg_attach_deferred); return 0; + } + rk_dbg_attach_programmed++; ret = rk_iommu_enable(iommu); if (ret) rk_iommu_detach_device(iommu->domain, dev); @@ -1777,6 +1949,7 @@ static int __maybe_unused rk_iommu_resume(struct device *dev) if (iommu->dlr_disable) return 0; + rk_dbg_resume_programmed++; /* #patch29 */ return rk_iommu_enable(iommu); } diff --git a/drivers/rknpu/rknpu_reset.c b/drivers/rknpu/rknpu_reset.c index 65142368c..4418a76a4 100644 --- a/drivers/rknpu/rknpu_reset.c +++ b/drivers/rknpu/rknpu_reset.c @@ -9,6 +9,9 @@ #include "rknpu_reset.h" +/* #patchB8: provided by drivers/iommu/rockchip-iommu.c */ +extern int rk_iommu_reprogram(struct device *dev); + #ifndef FPGA_PLATFORM static inline struct reset_control *rknpu_reset_control_get(struct device *dev, const char *name) @@ -95,6 +98,89 @@ static int rknpu_reset_deassert(struct reset_control *rst) } #endif +/* #patch: PER-CORE soft reset. rknpu_soft_reset() asserts ALL SIX DT reset lines + * ("srst_a0","srst_a1","srst_a2","srst_h0","srst_h1","srst_h2" — AXI+AHB per core), + * sleeps 100 ms, and sets the global soft_reseting flag that makes rknpu_job_next() + * bail for EVERY core. Recovering one stalled core therefore resets the whole NPU and + * destroys the other two cores' warm state — most of the ~3.6 s a doorbell miss costs. + * + * The lines are per core, so reset only core N: indices N (AXI) and N+num_irqs (AHB). + * No msleep, no global flag, no IOMMU re-attach — the other cores keep running. */ +int rknpu_soft_reset_core(struct rknpu_device *rknpu_dev, int core) +{ +#ifndef FPGA_PLATFORM + int ret = 0, ia, ih; + + if (rknpu_dev->bypass_soft_reset) + return 0; + + ia = core; + ih = core + rknpu_dev->config->num_irqs; + if (core < 0 || ih >= rknpu_dev->num_srsts) + return -EINVAL; + + ret |= rknpu_reset_assert(rknpu_dev->srsts[ia]); + ret |= rknpu_reset_assert(rknpu_dev->srsts[ih]); + udelay(10); + ret |= rknpu_reset_deassert(rknpu_dev->srsts[ia]); + ret |= rknpu_reset_deassert(rknpu_dev->srsts[ih]); + udelay(10); + + wake_up(&rknpu_dev->subcore_datas[core].job_done_wq); + return ret; +#else + return 0; +#endif +} + +/* #patch: per-core reset + IOMMU re-programming — the minimal sequence measured to actually recover a + * stalled core. Shared by the RKNPU_ACT_RESET_CORE ioctl and the progress watchdog so both paths behave + * identically, mirroring how rknpu_soft_reset serves the device-wide action. */ +int rknpu_reset_core_and_remap(struct rknpu_device *rknpu_dev, int core) +{ +#ifndef FPGA_PLATFORM + struct iommu_domain *domain; + int ret; + + ret = rknpu_soft_reset_core(rknpu_dev, core); + if (ret) + return ret; + + if (!rknpu_dev->iommu_en) + return 0; + + /* the re-attach must not race a concurrent dma_map; quiesce as rknpu_soft_reset does */ + if (!mutex_trylock(&rknpu_dev->reset_lock)) + return -EBUSY; + rknpu_dev->soft_reseting = true; + domain = iommu_get_domain_for_dev(rknpu_dev->dev); + if (domain) { + /* #patch46: serialize against domain switching. + * + * rknpu_soft_reset() holds reset_lock; rknpu_iommu_domain_get_and_switch() holds + * domain_lock. Different locks, so this detach/attach had NO mutual exclusion against + * a concurrent switch -- and a switch that reads iommu_get_domain_for_dev() while we + * are detached sees a domain that does not match rknpu_dev->iommu_domains[], bails + * with "mismatch domain get from iommu_get_domain_for_dev", and fails the caller. + * + * Measured: 1600 mismatch lines in a single 300 s run once the fast-abort raised the + * reset rate ~30x. Low reset rates hid it; it was always latent. + * + * Ordering is reset_lock -> domain_lock. Safe: no path takes reset_lock while holding + * domain_lock (rknpu_iommu_switch_domain does not reset). */ + mutex_lock(&rknpu_dev->domain_lock); + iommu_detach_device(domain, rknpu_dev->dev); + iommu_attach_device(domain, rknpu_dev->dev); + mutex_unlock(&rknpu_dev->domain_lock); + } + rknpu_dev->soft_reseting = false; + mutex_unlock(&rknpu_dev->reset_lock); + return 0; +#else + return 0; +#endif +} + int rknpu_soft_reset(struct rknpu_device *rknpu_dev) { #ifndef FPGA_PLATFORM @@ -107,8 +193,15 @@ int rknpu_soft_reset(struct rknpu_device *rknpu_dev) return 0; } + /* #patch: a failed trylock used to `return 0` — reporting SUCCESS without resetting anything. + * The caller (RKNPU_ACT_RESET from userspace recovery) then believes the NPU was reset when it + * was not, and spins on a still-wedged core. That was harmless while this was the only resetter; + * it stopped being harmless once the per-core recovery path also takes reset_lock, at which point + * userspace recovery silently became a no-op and the workload ran ~10x slower. + * Wait for the in-flight reset instead, so an ACT_RESET always means a reset actually happened. + * Legal: this function already msleep()s, so it is process context. */ if (!mutex_trylock(&rknpu_dev->reset_lock)) - return 0; + mutex_lock(&rknpu_dev->reset_lock); rknpu_dev->soft_reseting = true; @@ -138,13 +231,27 @@ 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); + /* #patchB8: reprogram the MMU for the LIVE domain, not the core's default. + * + * This used to be iommu_detach_device(iommu_get_domain_for_dev(dev)) + iommu_attach_device(), + * which was only ever right because the old multi-domain code overwrote + * iommu_group->default_domain on every switch. With the light switch (#patchB1/B2) the IOMMU + * core is not in the loop, so iommu_get_domain_for_dev() returns the group's DEFAULT domain + * (domain 0) -- and this sequence re-attached domain 0 while rknpu_dev->iommu_domain_id still + * said domain N. Every IOVA the next job used then resolved against the wrong page table, so + * the job was committed and never completed: no IRQ, no error, an output buffer left holding + * its seed. That is the "first int8 op after an ACT_RESET is silently dropped" bug. + * + * rk_iommu_reprogram() re-enables the MMU from iommu->domain, which the light switch keeps + * pointed at the live domain, so the correct page table is restored. */ + 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 soft reset: %d\n", rp); } + (void)domain; rknpu_dev->soft_reseting = false;