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/include/rknpu_drv.h b/drivers/rknpu/include/rknpu_drv.h index 3f3804ad7..478c30a8a 100644 --- a/drivers/rknpu/include/rknpu_drv.h +++ b/drivers/rknpu/include/rknpu_drv.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -100,6 +101,11 @@ struct rknpu_subcore_data { struct rknpu_job *job; int64_t task_num; struct rknpu_timer timer; + /* #patch: progress watchdog. wd_last = last PC completed-task counter seen, + * wd_flat = consecutive samples with no advance. */ + void *wd_job; /* #patch44: job the wd_flat run belongs to */ + u32 wd_last; + u32 wd_flat; }; /** @@ -159,6 +165,11 @@ struct rknpu_device { struct rknpu_debugger debugger; struct hrtimer timer; ktime_t kt; + /* #patch: progress watchdog timer (armed on submit, self-disarms when idle) */ + struct hrtimer wd_timer; + struct work_struct wd_work; + ktime_t wd_kt; + int wd_on; phys_addr_t sram_start; phys_addr_t sram_end; phys_addr_t nbuf_start; @@ -186,4 +197,6 @@ int rknpu_power_get(struct rknpu_device *rknpu_dev); int rknpu_power_put(struct rknpu_device *rknpu_dev); int rknpu_power_put_delay(struct rknpu_device *rknpu_dev); +void rknpu_wd_arm(struct rknpu_device *rknpu_dev); + #endif /* __LINUX_RKNPU_DRV_H_ */ diff --git a/drivers/rknpu/include/rknpu_gem.h b/drivers/rknpu/include/rknpu_gem.h index 61e8fa22c..86300b21f 100644 --- a/drivers/rknpu/include/rknpu_gem.h +++ b/drivers/rknpu/include/rknpu_gem.h @@ -44,6 +44,9 @@ * user can access the buffer through kms_bo.handle. */ struct rknpu_gem_object { + /* #patch70: queue link used when destroy could not switch domains — see + * rknpu_gem_object_destroy(). The object is RETAINED here rather than leaked. */ + struct list_head deferred; struct drm_gem_object base; unsigned int flags; unsigned long size; @@ -121,7 +124,12 @@ static inline void rknpu_gem_object_put(struct drm_gem_object *obj) /* * get rknpu drm object from gem handle, this function could be used for * other drivers such as 2d/3d acceleration drivers. - * with this function call, gem object reference count would be increased. + * + * #patch37 WARNING: this does NOT give you a reference. It looks the object up (which takes one) + * and immediately puts it back, so the returned pointer is kept alive ONLY by the handle table and + * is valid only while that handle cannot be closed concurrently. Do NOT hold it across anything + * that sleeps -- see rknpu_gem_destroy_ioctl(), which used to hold it across an ~18 s domain-switch + * wait. The previous version of this comment claimed the refcount "would be increased"; it does not. */ static inline struct rknpu_gem_object * rknpu_gem_object_find(struct drm_file *filp, unsigned int handle) diff --git a/drivers/rknpu/include/rknpu_ioctl.h b/drivers/rknpu/include/rknpu_ioctl.h index f4e3b83bf..b0716d34a 100644 --- a/drivers/rknpu/include/rknpu_ioctl.h +++ b/drivers/rknpu/include/rknpu_ioctl.h @@ -134,6 +134,41 @@ enum e_rknpu_action { RKNPU_GET_FREE_SRAM_SIZE = 23, RKNPU_GET_IOMMU_DOMAIN_ID = 24, RKNPU_SET_IOMMU_DOMAIN_ID = 25, + + /* #patch: per-core reset, the symmetric counterpart of RKNPU_ACT_RESET (which is device-wide). + * args->value = core index. Value 0x100 is deliberately outside the vendor's range so a future + * vendor action cannot collide with it. + * + * Does per-core AXI+AHB reset AND the IOMMU re-attach: the re-attach is REQUIRED, because + * rk_iommu shares the NPU reset domain, so resetting a core wipes its MMU programming and any + * subsequent submit has a valid IOVA with no translation behind it. Measured: reset alone leaves + * the job unrecoverable (deficit 2.0/stall); reset + re-attach recovers it (1.0/stall). */ + RKNPU_ACT_RESET_CORE = 0x100, + /* #patch61: tear down and rebuild ALL iommu domain state — an explicit shortcut for the + * reboot that is currently the only way to clear it. Outside the vendor range (0-25). + * + * WHY: unbalanced domain puts accumulate across runs (rknpu_job_abort drives the refcount to + * zero while other cores still hold work) until every switch fails with + * "mismatch domain get from iommu_get_domain_for_dev" and then every allocation fails with + * "rknpu_gem_get_pages: dma map ... fail". Nothing clears it short of a reboot (~90 s, and it + * destroys any in-RAM diagnostics). + * + * MANUAL and EXPLICIT — not fired automatically. The driver refuses with -EBUSY if any core + * still has a job, so it cannot run underneath live work. CALLER CONTRACT: this changes the + * IOVAs, so every existing buffer AND every regcmd built against those IOVAs is invalid + * afterwards and must be rebuilt. */ + RKNPU_ACT_REINIT = 0x101, + + /* #patchB11: make an IOMMU domain LIVE, with no allocation. + * + * A dma-buf import maps its sg during PRIME_FD_TO_HANDLE, into whichever domain is live at that + * moment — the ioctl carries no domain. The target domain is only named later, by MEM_CREATE, + * which does NOT re-map an already-imported handle. So an import destined for domain N can land + * in domain M, and every submit against it is committed and never completes. + * + * Userspace issues this before PRIME_FD_TO_HANDLE so the mapping lands where the weight will be + * used. `value` = domain id. Switch and release: the caller holds no reference afterwards. */ + RKNPU_ACT_SET_DOMAIN = 0x102, }; /** diff --git a/drivers/rknpu/include/rknpu_iommu.h b/drivers/rknpu/include/rknpu_iommu.h index 225176650..1d0b18083 100644 --- a/drivers/rknpu/include/rknpu_iommu.h +++ b/drivers/rknpu/include/rknpu_iommu.h @@ -53,8 +53,21 @@ 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); +/* #patchB9: THE LIVE DOMAIN. Under the light switch (#patchB1/B2) the IOMMU core is not involved in + * a domain change, so iommu_get_domain_for_dev() reports the group's DEFAULT domain (domain 0) + * forever. Every map/unmap/flush that must target the domain the NPU is actually running in has to + * use this instead. Using the core accessor silently targets domain 0: the mapping lands in the + * wrong page table and the next job is committed but never completes. */ +struct iommu_domain *rknpu_iommu_live_domain(struct device *dev); + +/* #patchB10: scoped override of the core's DEFAULT domain, for the dma-buf attachment map/unmap only + * (the DMA API cannot be told which domain to use). Returns the previous value; restore 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 #endif + +int rknpu_iommu_reinit(struct rknpu_device *rknpu_dev); /* #patch61 */ diff --git a/drivers/rknpu/include/rknpu_job.h b/drivers/rknpu/include/rknpu_job.h index b4d40d96f..f625d547c 100644 --- a/drivers/rknpu/include/rknpu_job.h +++ b/drivers/rknpu/include/rknpu_job.h @@ -19,6 +19,11 @@ #define RKNPU_JOB_DONE (1 << 0) #define RKNPU_JOB_ASYNC (1 << 1) +/* #patch41: the progress watchdog has declared this job stalled. INTERNAL to job->flags (which is + * never assigned from the uABI args->flags), so it cannot collide with RKNPU_JOB_PC/NONBLOCK/etc. */ +#define RKNPU_JOB_STALLED (1 << 8) + +/* #patch47 profiler fields (filled at commit; free -- derived from existing timestamps) */ #define RKNPU_JOB_DETACHED (1 << 2) #define RKNPU_CORE_AUTO_MASK 0x00 @@ -32,6 +37,19 @@ struct rknpu_job { struct work_struct cleanup_work; bool irq_entry[RKNPU_MAX_CORES]; unsigned int flags; + /* #patch64: does THIS job currently hold the iommu domain reference? + * + * The reference is acquired exactly once (rknpu_job_commit) but THREE teardown paths release + * it -- rknpu_job_done(), rknpu_job_abort() and rknpu_job_timeout_clean() -- with nothing + * recording whether this job still holds one. done-then-aborted, or aborted-then-reaped, + * releases twice, driving the device-wide count to zero while OTHER cores are still executing; + * rknpu_iommu_domain_get_and_switch() then reads zero as "safe to switch" and reprograms the + * IOMMU underneath live work. Caller-tagging measured 17 such premature zeros in one run, + * attributed to rknpu_job_abort+0x40. Upstream: rockchip-linux/kernel#387. + * + * Its own word (not a bit in ->flags) because the releases run from both IRQ and process + * context, so the test-and-clear must be atomic. */ + unsigned long dom_held; int ret; struct rknpu_submit *args; bool args_owner; @@ -49,6 +67,19 @@ struct rknpu_job { ktime_t hw_elapse_time; atomic_t submit_count[RKNPU_MAX_CORES]; int iommu_domain_id; + s64 prof_gap_us; /* #patch47: since previous commit on this core */ + s64 prof_queue_us; + unsigned int prof_since_switch; + s64 prof_tsince_us; /* #patch52: us since the last domain switch */ /* #patch51 */ /* #patch47: submit -> commit */ + /* #patch (experiment): PC register snapshot taken at commit — what we WROTE and what + * reads BACK immediately after. Reported by the progress watchdog when a job stalls, so + * a stalled commit can be compared against a healthy one with no per-commit logging. */ + uint32_t dbg_blk[17]; /* #patch: full PC/INT register block sampled after the start pulse */ + ktime_t dbg_kick_t; /* #patch: when recovery kicked this job (0 = never) */ + uint32_t dbg_seq; /* #patch: userspace task-descriptor stamp seen at commit */ + uint32_t dbg_wr[6]; + uint32_t dbg_rd[7]; + bool dbg_valid; }; irqreturn_t rknpu_core0_irq_handler(int irq, void *data); @@ -79,3 +110,5 @@ int rknpu_get_rw_amount(struct rknpu_device *rknpu_dev, uint32_t *dt_wr, int rknpu_get_total_rw_amount(struct rknpu_device *rknpu_dev, uint32_t *amount); #endif /* __LINUX_RKNPU_JOB_H_ */ + +void rknpu_reap_all_cores(struct rknpu_device *rknpu_dev); /* #patch54 */ diff --git a/drivers/rknpu/include/rknpu_reset.h b/drivers/rknpu/include/rknpu_reset.h index 5e3094871..dead4f74b 100644 --- a/drivers/rknpu/include/rknpu_reset.h +++ b/drivers/rknpu/include/rknpu_reset.h @@ -15,4 +15,8 @@ int rknpu_reset_get(struct rknpu_device *rknpu_dev); int rknpu_soft_reset(struct rknpu_device *rknpu_dev); +int rknpu_soft_reset_core(struct rknpu_device *rknpu_dev, int core); + +int rknpu_reset_core_and_remap(struct rknpu_device *rknpu_dev, int core); + #endif diff --git a/drivers/rknpu/rknpu_drv.c b/drivers/rknpu/rknpu_drv.c index 4090344b4..852c24988 100644 --- a/drivers/rknpu/rknpu_drv.c +++ b/drivers/rknpu/rknpu_drv.c @@ -37,6 +37,7 @@ #include #endif +#include #include "rknpu_ioctl.h" #include "rknpu_reset.h" #include "rknpu_fence.h" @@ -417,6 +418,27 @@ static int rknpu_action(struct rknpu_device *rknpu_dev, break; case RKNPU_SET_VOLT: break; + case RKNPU_ACT_RESET_CORE: { + /* #patch: per-core counterpart of RKNPU_ACT_RESET. Cheaper than the device-wide path — no + * msleep(100), only two reset lines — but it still needs the same quiesce for the IOMMU + * re-attach, which is unsafe asynchronously (it raced rknpu_iommu_dma_map_sg and panicked). */ + int _core = (int)args->value; + if (_core < 0 || _core >= rknpu_dev->config->num_irqs) + return -EINVAL; + ret = rknpu_reset_core_and_remap(rknpu_dev, _core); + break; + } + case RKNPU_ACT_SET_DOMAIN: { /* #patchB11: make a domain live before an import's PRIME ioctl */ + 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_REINIT: /* #patch61 */ + ret = rknpu_iommu_reinit(rknpu_dev); + break; case RKNPU_ACT_RESET: ret = rknpu_soft_reset(rknpu_dev); break; @@ -778,17 +800,502 @@ static enum hrtimer_restart hrtimer_handler(struct hrtimer *timer) return HRTIMER_RESTART; } +/* ================= PROGRESS WATCHDOG (#patch) ========================================== + * WHY. rknpu_job_timeout_clean is not a watchdog: it runs only when ANOTHER submit arrives on + * the same core, so a stalled job with no follow-up submit is never noticed (which is why + * userspace has to fire "reap dummy" submits). It also measures TOTAL JOB AGE, meaningless for + * a PC-chained job whose task list is extensible at runtime. "No task completed in N samples" + * is well defined for a chain of any length. + * + * CONFIRMED FINDING: on a doorbell miss the PC completed-task counter reads 0 with the job many + * ms old -- the job is accepted and NEVER DISPATCHES. (Control: at an 8 ms threshold a healthy + * ~30 ms job does NOT report, so the counter genuinely advances and the offset is right.) + * + * POWER SAFETY -- the hard part, and v1/v2 got it wrong. + * v1 read NPU MMIO straight from the hrtimer. A register read on a powered-down block is an + * external abort => instant HARD hang (no ping, power cycle required). It hung the board on + * its first NPU workload. + * v2 guarded with pm_runtime_get_if_in_use(rknpu_dev->dev). STILL WRONG on RK3588: this driver + * keeps its OWN refcount (power_refcount/power_lock) and rknpu_power_off() puts the three + * PER-CORE genpds (genpd_dev_npu0/1/2) directly. A reference on the parent does not hold those + * up, so core i's domain can still drop between the check and the readl on base[i]. + * v3 (this): sample in PROCESS context and hold power_lock across the read. rknpu_power_off() + * takes the same mutex, so the genpds cannot go down while we read. We deliberately do NOT call + * rknpu_power_get(): that would POWER THE NPU ON just to read a counter and pin it up forever. + * mutex_trylock means the watchdog can never delay a submit; a missed sample is simply skipped + * rather than counted as a no-advance. + * + * The hrtimer only decides cadence and queues the work; all MMIO happens in the worker. */ +/* DEFAULT OFF. This watchdog has destabilised the board three times: v1 hard-hung it (unguarded MMIO, + * fixed), and even the power_lock-guarded v3 COLD-RESET it twice mid-run (instant SoC reset, no panic, + * no console, ramoops cleared). Discriminator, same workload: wd_period_us=1000 -> reset; + * wd_period_us=0 -> 4000 reps clean, same boot_id. It is a DIAGNOSTIC: set wd_period_us=1000 only for + * a deliberate investigation, accepting that the board may reset. */ +/* #patch40: the progress watchdog is ON BY DEFAULT. + * + * It was opt-in because two earlier versions killed the board (v1 read NPU MMIO straight from the + * hrtimer; v2's pm_runtime_get_if_in_use on the parent missed the per-core genpds). v3 samples in + * process context holding power_lock and has since run clean. The other reason -- "it caused board + * resets" -- was RETRACTED; that was the vendor IRQ bug fixed as change 6. + * + * Opt-in made it useless in practice: the one time it was needed, it was enabled by hand and + * recorded wd_ticks=0 across 8 stalls, because the arm site was NONBLOCK-only (fixed in #patch39). + * A stalled BLOCKING submit otherwise sits in rknpu_job_wait() for its whole timeout -- 60.6 s + * measured -- with nothing reporting it. Detection at ~120 ms is a ~500x cut in time-to-visibility. + * + * Cost, measured A/B on this board (2000-rep int4 probe, ndom=1, interleaved x3, warmup discarded): + * OFF 133/133/133 commits/s ON 131/133/129 commits/s => ~1.5% + * Paid on every submit since #patch39. Judged worth it: the stalls it exposes cost ~23% of 27B + * prefill and were previously invisible. + * + * Detection latency is wd_samples * wd_period_us ~= 120 ms. Do NOT drop the period to 1000 without + * also dropping wd_samples: 1 kHz sampling costs ~100x throughput (2.8 vs 283-490 commits/s). */ +static unsigned int rknpu_wd_period_us = 10000; +module_param_named(wd_period_us, rknpu_wd_period_us, uint, 0644); +MODULE_PARM_DESC(wd_period_us, "progress watchdog sample period in us (0 disables)"); + +/* diagnostics: is the watchdog actually sampling, and why not when it isn't */ +static unsigned long rknpu_wd_ticks, rknpu_wd_seen_job, rknpu_wd_pm_skip, rknpu_wd_stalls; +module_param_named(wd_ticks, rknpu_wd_ticks, ulong, 0444); +module_param_named(wd_seen_job, rknpu_wd_seen_job, ulong, 0444); +module_param_named(wd_pm_skip, rknpu_wd_pm_skip, ulong, 0444); +module_param_named(wd_stalls, rknpu_wd_stalls, ulong, 0444); + +/* START-CONFIRM-AND-RETRY (wd_kick=1). + * A stalled job is committed with byte-identical registers, a valid mapping, a fresh descriptor and + * verified-correct regcmd bytes in DRAM — it simply never begins, and the completed-task counter stays 0. + * The registers are still correct at that point, so rather than the current recovery (userspace waits out + * a ~614 ms window, then issues a DEVICE-WIDE ACT_RESET that destroys warm state on all three cores, then + * resubmits — ~3.6 s per miss), just RE-PULSE the start trigger for that one core. + * + * Cheap and targeted: two register writes, no reset, no warm-state loss, no resubmit. If the PC merely + * missed its start, this restarts it; if it does not, we have lost nothing and learned that the fault is + * not a missed start trigger. Fires ONCE per stall episode, on the same condition as the report, and only + * with the counter still reading 0 (i.e. nothing has executed, so re-pulsing cannot disturb work in + * flight). Opt-in while under evaluation. */ +static unsigned long rknpu_cnt_kick; +module_param_named(cnt_kick, rknpu_cnt_kick, ulong, 0444); + +static unsigned int rknpu_wd_samples = 12; +/* #patch41: FAST-ABORT. When a BLOCKING submit stalls, its caller sits in rknpu_job_wait() for + * args->timeout x 3 -- 60.8 s measured on a real 1.5B-Q8 run -- before getting -ETIMEDOUT and + * self-healing. Waking it as soon as the stall is established turns that into ~wd_abort_ms. + * + * DELIBERATELY NOT the logging threshold. The PC counter advances per COMPLETED TASK, so a single + * long-running task is indistinguishable from a stall by this signal alone; aborting at the ~120 ms + * log threshold would kill slow-but-healthy jobs. This is a separate, much larger bound: still + * ~30x faster than 60 s while leaving any plausible single task room to finish. + * + * 0 disables. OPT-IN on purpose -- the previous automatic stall intervention (wd_kick) was deleted + * for having no measurable benefit, so this one earns its default only once measured. */ +static unsigned int rknpu_wd_abort_ms; +module_param_named(wd_abort_ms, rknpu_wd_abort_ms, uint, 0644); +MODULE_PARM_DESC(wd_abort_ms, "fail a stalled BLOCKING submit after this many ms (0 = off)"); +/* #patch47: opt-in timing profiler. prof_mask != 0 enables it (bit0 = per-stall breakdown, + * bit1 = also log healthy commits). Everything it reports is derived from timestamps the driver + * already takes, so the cost when disabled is one branch and when enabled is one printk. */ +/* #patch49: CAUSAL TEST for the change-16 correlation. + * + * Every stalled job was committed 0.24-0.9 ms after the previous commit on the same core, against a + * healthy mean three orders of magnitude larger. That is correlation. This enforces a MINIMUM spacing + * between commits on a core so the hypothesis can be tested directly: if stalls vanish as this rises, + * back-to-back commits cause them; if they persist, the short gap is a symptom of something else. + * + * 0 = off (default). Clamped to 5000 us because this MUST busy-wait: rknpu_job_next() can be reached + * from the completion IRQ, where sleeping is illegal. A diagnostic knob, not a shipping fix -- a real + * fix would reshape submission rather than burn CPU in udelay(). */ +static unsigned int rknpu_min_commit_gap_us; +module_param_named(min_commit_gap_us, rknpu_min_commit_gap_us, uint, 0644); +MODULE_PARM_DESC(min_commit_gap_us, "enforce >= this gap between commits on a core (0=off, max 5000)"); +unsigned long rknpu_gap_enforced_n, rknpu_gap_enforced_us; +module_param_named(gap_enforced_n, rknpu_gap_enforced_n, ulong, 0444); +module_param_named(gap_enforced_us_sum, rknpu_gap_enforced_us, ulong, 0444); +unsigned int rknpu_min_commit_gap_get(void) { return rknpu_min_commit_gap_us; } + +/* #patch55: SUB-BLOCK register dump — is the NPU stuck mid-read, or did it never issue one? + * + * At stall time the PC completed-task counter reads 0x0 and all four MMU banks read IDLE + * (0x19 = paging|idle|replay-empty). Those two facts pull in opposite directions: MMU-idle says no + * translation is outstanding, i.e. nothing was ever requested; but the job WAS triggered, so + * something should have been. The missing instrument is the NPU's own sub-blocks — we only ever + * dumped the PC block. + * + * CNA 0x1000 does the convolution feeder think it is running? + * DPU 0x4000 output stage state + * CDMA 0x5000 IS A DMA READ OUTSTANDING? <- the discriminator + * + * CDMA busy while the MMU is idle == a transaction stuck UPSTREAM of translation (a hung read). + * CDMA idle too == the trigger never took and nothing was issued. + * + * Deliberately dumped BLIND rather than by name: mainline drivers/accel/rocket names these, but it + * is a much newer kernel than this tree, and we do not need names to diff a stalled snapshot against + * a healthy one. Names can follow once we know which words move. + * + * Safety: each core maps 0x10000 (rk3588s.dtsi), so 0x1000/0x4000/0x5000 are in range; and this runs + * only from the watchdog worker, which is process context holding power_lock with power_refcount > 0 + * (patch06: an MMIO read of a powered-down block kills the machine instantly). Capped so a stall + * storm cannot flood the log. */ +/* #patch58: does the PC completed-task counter EVER read non-zero while a job is in flight? + * + * The cnt>0 healthy control never fired across a whole run despite tens of thousands of samples. + * If cnt is always 0 for in-flight jobs then `cnt == wd_last` is trivially true and the watchdog is + * not a progress detector at all — it is a "job present for > wd_samples*period" duration timer, and + * the "counter 0x0" in every stall report is NOT evidence that the hardware never started, because + * healthy jobs would report it too. That would invalidate the framing this whole investigation has + * carried. Record the maximum ever observed and settle it. */ +static unsigned int rknpu_wd_max_cnt; +module_param_named(wd_max_cnt, rknpu_wd_max_cnt, uint, 0444); +static unsigned long rknpu_wd_cnt_nonzero; +module_param_named(wd_cnt_nonzero, rknpu_wd_cnt_nonzero, ulong, 0444); + +/* #patch67: TIME SERIES of CNA/DPU S_STATUS across in-flight jobs (prof_mask bit 3). + * + * The one-shot healthy dump showed CNA/DPU S_STATUS = 0x00000000 healthy vs 0x00000005 stalled, which + * suggests the PC DID start the downstream units and they are parked. But `cnt > 0` only proves the + * job executed a task — the single sample may have landed BETWEEN tasks, so "healthy = 0" could be a + * quiet moment rather than the running state, which would invalidate the comparison. + * + * Sample every watchdog tick for every in-flight job and log wd_flat alongside, so healthy + * (flat small, still progressing) and stalling (flat large) samples sit in one stream and can be + * separated offline. If 0x5 ever appears transiently while healthy, it means RUNNING and the units are + * merely stuck mid-op; if it never does, 0x5 is specific to the failure. */ +static unsigned long rknpu_prof_stat_n; +module_param_named(prof_stat_n, rknpu_prof_stat_n, ulong, 0444); +static unsigned long rknpu_prof_stat_cap = 400; +module_param_named(prof_stat_cap, rknpu_prof_stat_cap, ulong, 0644); + +static unsigned int rknpu_prof_blk_cap = 4; +module_param_named(prof_blk_cap, rknpu_prof_blk_cap, uint, 0644); +static unsigned int rknpu_prof_blk_done; +static unsigned int rknpu_prof_blk_healthy_done; + +static void rknpu_blk_dump(void __iomem *base, int core, const char *tag) +{ + /* #patch65: include the PC block (0x0000) — the one we never compared. + * + * The CPU->NPU protocol is a doorbell plus a DMA-fetched program: write PC_DATA_ADDR (0x10) + * with the regcmd's IOVA, PC_DATA_AMOUNT (0x14), the interrupt regs, then PC_TASK_CONTROL + * (0x30) whose 0x6 field is the trigger; the NPU then DMA-reads the regcmd and executes it. + * At a stall the trigger IS consumed (0x7002 written, 0x1002 read back) but the MMU stays + * idle, so the fetch never happens — the doorbell is acknowledged and the program is never + * collected. + * + * A stall dump showed PC_OP_EN (0x08) reading 0x00000000 with INT_STATUS and INT_RAW_STATUS + * both 0. The commit sequence never writes PC_OP_EN, and one watchdog kick mode was + * "re-pulse PC_OP_EN", so it is plausibly the run-enable — but we have never dumped this + * block for a HEALTHY job, so there is nothing to compare against. Add it. */ + static const u32 blk[] = { 0x0000, 0x1000, 0x4000, 0x5000 }; + static const char *nm[] = { "PC", "CNA", "DPU", "CDMA" }; + int b, i; + + /* #patch57: 16 words per block, not 8. The first 8 were byte-identical between healthy and + * stalled, which may simply mean they are static config. Widen so a word that actually moves + * has a chance to appear. */ + for (b = 0; b < 4; b++) { + u32 v[16]; + int h; + + for (i = 0; i < 16; i++) + v[i] = readl(base + blk[b] + i * 4); + for (h = 0; h < 2; h++) + LOG_ERROR("RKNPU: BLK[%s] core%d %-4s +0x%04x: %08x %08x %08x %08x %08x %08x %08x %08x\n", + tag, core, nm[b], blk[b] + h * 32, + v[h*8+0], v[h*8+1], v[h*8+2], v[h*8+3], + v[h*8+4], v[h*8+5], v[h*8+6], v[h*8+7]); + } +} + +extern unsigned long rknpu_dbg_destroy_bailed; /* #patch69 */ +module_param_named(dbg_destroy_bailed, rknpu_dbg_destroy_bailed, ulong, 0444); + +static unsigned int rknpu_prof_mask; +module_param_named(prof_mask, rknpu_prof_mask, uint, 0644); +MODULE_PARM_DESC(prof_mask, "timing profiler: 0=off, 1=stall breakdown, 3=+healthy commits"); +/* #patch50: shape-axis profiling. bit1 = log every stall with its shape (already on), + * bit2 = ALSO log every commit with its shape, so the HEALTHY distribution can be built. + * ~1100 lines per run — trivial for the persistent disk log, and it keeps all the binning + * offline in awk instead of hard-coding a histogram in the kernel. */ +unsigned int rknpu_prof_mask_get(void) { return rknpu_prof_mask; } +/* #patch48: non-static — rknpu_job_done() (other TU) accumulates the healthy side */ +unsigned long rknpu_prof_ok_n, rknpu_prof_ok_gap, rknpu_prof_ok_queue; +module_param_named(prof_ok_n, rknpu_prof_ok_n, ulong, 0444); +module_param_named(prof_ok_gap_us_sum, rknpu_prof_ok_gap, ulong, 0444); +module_param_named(prof_ok_queue_us_sum, rknpu_prof_ok_queue, ulong, 0444); +static unsigned long rknpu_prof_bad_n, rknpu_prof_bad_gap, rknpu_prof_bad_queue; +module_param_named(prof_bad_n, rknpu_prof_bad_n, ulong, 0444); +module_param_named(prof_bad_gap_us_sum, rknpu_prof_bad_gap, ulong, 0444); +module_param_named(prof_bad_queue_us_sum, rknpu_prof_bad_queue, ulong, 0444); + +static unsigned long rknpu_wd_queued_aborts; +module_param_named(wd_queued_aborts, rknpu_wd_queued_aborts, ulong, 0444); +static unsigned long rknpu_wd_aborts; +module_param_named(wd_aborts, rknpu_wd_aborts, ulong, 0444); /* #patch40: 12 x 10 ms ~= 120 ms detection */ +module_param_named(wd_samples, rknpu_wd_samples, uint, 0644); +MODULE_PARM_DESC(wd_samples, "consecutive no-advance samples before reporting a stall"); + +/* Process context: safe to take power_lock, so safe to touch MMIO. */ +static void rknpu_wd_work_fn(struct work_struct *work) +{ + struct rknpu_device *rknpu_dev = + container_of(work, struct rknpu_device, wd_work); + struct rknpu_subcore_data *subcore_data = NULL; + struct rknpu_job *job = NULL; + unsigned long flags; + int i; + + /* never block a submit: power_lock is held by rknpu_power_get/put on the hot path */ + if (!mutex_trylock(&rknpu_dev->power_lock)) { + rknpu_wd_pm_skip++; + return; + } + /* powered off => nothing is running and MMIO is unsafe; skip without penalising wd_flat */ + if (atomic_read(&rknpu_dev->power_refcount) <= 0) { + rknpu_wd_pm_skip++; + mutex_unlock(&rknpu_dev->power_lock); + return; + } + + for (i = 0; i < rknpu_dev->config->num_irqs; i++) { + void __iomem *rknpu_core_base = rknpu_dev->base[i]; + u32 cnt; + + subcore_data = &rknpu_dev->subcore_datas[i]; + + spin_lock_irqsave(&rknpu_dev->irq_lock, flags); + job = subcore_data->job; + spin_unlock_irqrestore(&rknpu_dev->irq_lock, flags); + + if (!job) { + subcore_data->wd_flat = 0; + subcore_data->wd_job = NULL; /* #patch44 */ + continue; + } + rknpu_wd_seen_job++; + + cnt = readl(rknpu_core_base + + rknpu_dev->config->pc_task_status_offset) & + rknpu_dev->config->pc_task_number_mask; + + /* #patch44: a stall run must be attributed to a JOB, not just to a counter value. + * + * wd_flat only reset when the PC counter CHANGED. Consecutive jobs on one core each + * start their counter at 0, so across a job boundary the counter looks unchanged and + * wd_flat keeps climbing. Both the stall report and the fast-abort fire on EQUALITY + * (== wd_samples / == an), so only the FIRST stall in a run of them was ever seen and + * every later one was silently absorbed. + * + * Measured before this fix: 10 timed-out waits, 5 detected+aborted at 2.0 s, 5 missed + * entirely at ~61 s -- exactly 50% coverage, with the misses in a clean second mode. + * Reset the run whenever the job identity changes. */ + if (subcore_data->wd_job != job) { + subcore_data->wd_job = job; + subcore_data->wd_last = cnt; + subcore_data->wd_flat = 0; + } + + if ((rknpu_prof_mask & 8) && + rknpu_prof_stat_n < rknpu_prof_stat_cap) { + rknpu_prof_stat_n++; + /* #patch68: INT_RAW_STATUS (PC block, 0x2c) in the same series. + * + * It read 0x00000008 healthy vs 0xc0000000 stalled — but from a SINGLE sample + * of each, the same one-shot method that made CNA/DPU 0x5 look like a failure + * signature when it is really the RUNNING state. Sample it across both + * populations before believing it. */ + LOG_ERROR("RKNPU: SSTAT core%d cnt=%u flat=%u CNA=%08x DPU=%08x RAW=%08x\n", + i, cnt, subcore_data->wd_flat, + readl(rknpu_core_base + 0x1000), + readl(rknpu_core_base + 0x4000), + readl(rknpu_core_base + 0x2c)); + } + + if (cnt > rknpu_wd_max_cnt) + rknpu_wd_max_cnt = cnt; /* #patch58 */ + if (cnt) + rknpu_wd_cnt_nonzero++; + + /* #patch57: a VALID healthy control. + * + * The previous baseline fired at wd_flat == 0, which only means RECENTLY COMMITTED — + * plausibly the identical register state to a job that is about to be reported + * stalled. Comparing that against a stall compares a state with itself, and indeed + * every word matched. Gate on cnt > 0 instead: the PC completed-task counter has + * advanced, so this job has provably executed at least one task and cannot be in the + * failure state (whose defining symptom is counter stuck at 0x0). Also far more + * catchable than requiring the counter to CHANGE between two 10 ms samples. */ + if ((rknpu_prof_mask & 4) && cnt > 0 && + rknpu_prof_blk_healthy_done < rknpu_prof_blk_cap) { + rknpu_prof_blk_healthy_done++; + rknpu_blk_dump(rknpu_core_base, i, "OK"); + } + + if (cnt == subcore_data->wd_last) { + /* fire ONCE per stall episode (equality, not >=) */ + if (++subcore_data->wd_flat == rknpu_wd_samples) { + rknpu_wd_stalls++; + if ((rknpu_prof_mask & 4) && + rknpu_prof_blk_done < rknpu_prof_blk_cap) { + rknpu_prof_blk_done++; + rknpu_blk_dump(rknpu_core_base, i, "STALL"); /* #patch55 */ + } + /* #patch47: attribute this stall's timeline */ + rknpu_prof_bad_n++; + rknpu_prof_bad_gap += (unsigned long)job->prof_gap_us; + rknpu_prof_bad_queue += (unsigned long)job->prof_queue_us; + if (rknpu_prof_mask & 1) + LOG_ERROR("RKNPU: PROF stalled core %d: tn=%u cm=%#x dom=%d sinceswitch=%u tsince=%lldus gap_since_prev_commit=%lldus queue(submit->commit)=%lldus | healthy mean gap=%luus queue=%luus over n=%lu\n", + i, job->args->task_number, + job->args->core_mask, + job->iommu_domain_id, + job->prof_since_switch, + job->prof_tsince_us, + job->prof_gap_us, job->prof_queue_us, + rknpu_prof_ok_n ? rknpu_prof_ok_gap / rknpu_prof_ok_n : 0UL, + rknpu_prof_ok_n ? rknpu_prof_ok_queue / rknpu_prof_ok_n : 0UL, + rknpu_prof_ok_n); + /* #patch: report COMPLETED vs SUBMITTED so the line is self-interpreting. + * The kernel has both: the PC counter (completed) and the submit's task + * counts. "0/21" says the job never started; "17/21" would say it stalled + * partway. core_mask is echoed so this pairs with the userspace + * ORK_KMSG_SUBMIT stamp (`ork: sub#N dom=D cm=0xM tasks=T`). */ + LOG_ERROR("RKNPU: core %d NO TASK PROGRESS for %u samples (%u us), tasks=%u/%u done (counter %#x), cm=0x%x dom=%u, job age %lld us\n", + i, subcore_data->wd_flat, + subcore_data->wd_flat * rknpu_wd_period_us, + cnt, + (job->args->subcore_task[i].task_number ? + job->args->subcore_task[i].task_number : + job->args->task_number), + cnt, + job->args->core_mask, + job->args->iommu_domain_id, + ktime_us_delta(ktime_get(), job->timestamp)); + if (job->dbg_valid) + LOG_ERROR("RKNPU: commit snapshot: seq=%u WROTE data_addr=%#x amount=%#x int_mask=%#x task_ctrl=%#x dma_base=%#x tasks=%u | READBACK data_addr=%#x amount=%#x int_mask=%#x task_ctrl=%#x dma_base=%#x status(pre)=%#x status(post)=%#x\n", + job->dbg_seq, job->dbg_wr[0], job->dbg_wr[1], job->dbg_wr[2], + job->dbg_wr[3], job->dbg_wr[4], job->dbg_wr[5], + job->dbg_rd[0], job->dbg_rd[1], job->dbg_rd[2], + job->dbg_rd[3], job->dbg_rd[4], job->dbg_rd[5], + job->dbg_rd[6]); + LOG_ERROR("RKNPU: PCBLK %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x | en=%08x\n", + job->dbg_blk[0], job->dbg_blk[1], job->dbg_blk[2], job->dbg_blk[3], + job->dbg_blk[4], job->dbg_blk[5], job->dbg_blk[6], job->dbg_blk[7], + job->dbg_blk[8], job->dbg_blk[9], job->dbg_blk[10], job->dbg_blk[11], + job->dbg_blk[12], job->dbg_blk[13], job->dbg_blk[14], job->dbg_blk[15], + job->dbg_blk[16]); + } + + /* #patch41: fast-abort a stalled BLOCKING submit. ASYNC jobs have no + * waiter, so there is nothing to wake. Equality fires it once. */ + if (rknpu_wd_abort_ms && !(job->flags & RKNPU_JOB_ASYNC)) { + unsigned int an = (rknpu_wd_abort_ms * 1000u) / + (rknpu_wd_period_us ? + rknpu_wd_period_us : 1u); + + if (an && subcore_data->wd_flat == an) { + struct rknpu_job *entry = NULL; + unsigned int qn = 0; + + spin_lock_irqsave(&rknpu_dev->irq_lock, flags); + job->flags |= RKNPU_JOB_STALLED; + /* #patch43: also abort everything QUEUED behind this head. + * + * This is the detection-coverage gap: the sampler only ever + * inspects subcore_data->job, the COMMITTED job, so a submit + * waiting in todo_list is invisible and its waiter burns the + * whole timeout for a job that never even started. Measured: + * 14 userspace submit failures produced only 2 watchdog + * episodes, and fast-abort therefore only helped 2 of 14. + * + * Safe to declare these stalled: rknpu_job_next() promotes + * from todo_list ONLY when subcore_data->job is NULL, so while + * this head is stuck nothing behind it can run. A woken job + * that never committed has last_task == NULL, and + * rknpu_job_wait() already handles exactly that -- it unlinks + * itself from todo_list and returns. No new path needed. */ + list_for_each_entry(entry, + &subcore_data->todo_list, + head[i]) { + if (entry->flags & RKNPU_JOB_ASYNC) + continue; + entry->flags |= RKNPU_JOB_STALLED; + qn++; + } + spin_unlock_irqrestore(&rknpu_dev->irq_lock, flags); + /* one wake serves the whole queue: every waiter sleeps on this + * same job_done_wq and re-tests its OWN job->flags */ + wake_up(&subcore_data->job_done_wq); + rknpu_wd_aborts++; + rknpu_wd_queued_aborts += qn; + LOG_ERROR("RKNPU: core %d FAST-ABORT after %u ms, +%u queued behind it (would have waited %u ms x3)\n", + i, rknpu_wd_abort_ms, qn, + job->args->timeout); + } + } + } else { + subcore_data->wd_last = cnt; + subcore_data->wd_flat = 0; + } + } + + mutex_unlock(&rknpu_dev->power_lock); +} + +/* Cadence only — NO MMIO here. Self-disarms once no core has an outstanding job. */ +static enum hrtimer_restart rknpu_wd_handler(struct hrtimer *timer) +{ + struct rknpu_device *rknpu_dev = + container_of(timer, struct rknpu_device, wd_timer); + unsigned long flags; + int i, active = 0; + + rknpu_wd_ticks++; + + spin_lock_irqsave(&rknpu_dev->irq_lock, flags); + for (i = 0; i < rknpu_dev->config->num_irqs; i++) + if (rknpu_dev->subcore_datas[i].job) + active++; + spin_unlock_irqrestore(&rknpu_dev->irq_lock, flags); + + if (!active) { + rknpu_dev->wd_on = 0; + return HRTIMER_NORESTART; + } + /* a no-op if the previous sample is still queued, which self-throttles */ + queue_work(system_wq, &rknpu_dev->wd_work); + + hrtimer_forward_now(timer, rknpu_dev->wd_kt); + return HRTIMER_RESTART; +} + +/* Armed from the submit path; self-disarms in the handler once no core has a job. Re-arming an + * already-running hrtimer is harmless, so the wd_on race is benign. */ +void rknpu_wd_arm(struct rknpu_device *rknpu_dev) +{ + if (!rknpu_wd_period_us || rknpu_dev->wd_on) + return; + rknpu_dev->wd_on = 1; + rknpu_dev->wd_kt = ktime_set(0, (u64)rknpu_wd_period_us * 1000); + hrtimer_start(&rknpu_dev->wd_timer, rknpu_dev->wd_kt, HRTIMER_MODE_REL); +} + static void rknpu_init_timer(struct rknpu_device *rknpu_dev) { rknpu_dev->kt = ktime_set(0, RKNPU_LOAD_INTERVAL); hrtimer_init(&rknpu_dev->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); rknpu_dev->timer.function = hrtimer_handler; hrtimer_start(&rknpu_dev->timer, rknpu_dev->kt, HRTIMER_MODE_REL); + /* #patch: watchdog timer is initialised but NOT started — armed on submit */ + hrtimer_init(&rknpu_dev->wd_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); + rknpu_dev->wd_timer.function = rknpu_wd_handler; + rknpu_dev->wd_on = 0; + INIT_WORK(&rknpu_dev->wd_work, rknpu_wd_work_fn); } static void rknpu_cancel_timer(struct rknpu_device *rknpu_dev) { hrtimer_cancel(&rknpu_dev->timer); + hrtimer_cancel(&rknpu_dev->wd_timer); /* #patch */ + cancel_work_sync(&rknpu_dev->wd_work); /* #patch */ } static bool rknpu_is_iommu_enable(struct device *dev) diff --git a/drivers/rknpu/rknpu_gem.c b/drivers/rknpu/rknpu_gem.c index f1346f98f..b9311a8f9 100644 --- a/drivers/rknpu/rknpu_gem.c +++ b/drivers/rknpu/rknpu_gem.c @@ -72,7 +72,9 @@ 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)); + /* #patchB9: flush the LIVE domain's IOTLB, not the core's default. Flushing domain 0 here left + * stale translations for the pages just mapped into domain N. */ + 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 +503,10 @@ 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); + /* #patchB9: map into the LIVE domain. iommu_get_domain_for_dev() is the core's default + * (domain 0) under the light switch, so this used to map the SRAM/NBUF cache buffer into a + * page table the NPU is not running in. */ + domain = rknpu_iommu_live_domain(rknpu_dev->dev); if (!domain) { LOG_ERROR("failed to get iommu domain!"); return -EINVAL; @@ -654,7 +659,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); /* #patchB9: unmap from the LIVE domain */ if (domain) { iommu_unmap(domain, rknpu_obj->iova_start, cache_size); if (rknpu_obj->size > 0) @@ -818,6 +823,102 @@ rknpu_gem_object_create(struct drm_device *drm, unsigned int flags, return ERR_PTR(ret); } +/* #patch69: how many GEM objects were LEAKED by the destroy path bailing out. + * + * When a stalled job makes the domain switch time out, the loop below gives up after 3 attempts and + * returns without freeing — the object and its IOVA are leaked permanently. Recorded originally as + * "leak rather than corrupt", which is safe per event but accumulates. */ +unsigned long rknpu_dbg_destroy_bailed; +EXPORT_SYMBOL(rknpu_dbg_destroy_bailed); + +/* #patch70: reclaim objects whose destroy could not switch domains. + * + * rknpu_gem_object_destroy() used to give up after 3 failed switches and RETURN WITHOUT FREEING, + * leaking the object and its IOVA permanently. Measured: 20 such bails and ~49 MB of outstanding IOVA + * in a SINGLE run. (That leak is not what wedges the device — the wedge is the domain mismatch — but + * it is a genuine resource leak on its own.) + * + * Instead of leaking, the object is queued here and reclaimed later. The drain deliberately only takes + * entries whose domain is ALREADY current: rknpu_iommu_domain_get_and_switch() then hits its + * same-domain fast path (increment, no switch, no 6 s timeout), so draining can never stall the caller. + * Objects for other domains simply wait until that domain comes around again. + * + * The acquire matters for balance: a deferred object never acquired a reference (its switch failed), + * and the free path below ends with rknpu_iommu_domain_put(). Taking one here keeps that paired. */ +static LIST_HEAD(rknpu_deferred_destroy); +static DEFINE_MUTEX(rknpu_deferred_lock); +unsigned long rknpu_dbg_destroy_reclaimed; +EXPORT_SYMBOL(rknpu_dbg_destroy_reclaimed); + +static void rknpu_gem_drain_deferred(struct rknpu_device *rknpu_dev); + +/* The free half of rknpu_gem_object_destroy(), reachable both from the normal path and from the + * deferred reclaim. Assumes the caller has already made the object's domain current AND holds a + * domain reference — the trailing rknpu_iommu_domain_put() releases it. */ +static void rknpu_gem_free_body(struct rknpu_gem_object *rknpu_obj) +{ + struct drm_gem_object *obj = &rknpu_obj->base; + struct rknpu_device *rknpu_dev = obj->dev->dev_private; + + /* + * do not release memory region from exporter. + * + * the region will be released by exporter + * once dmabuf's refcount becomes 0. + */ + if (obj->import_attach) { + /* #patchB10: 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) && + rknpu_obj->sram_size > 0) { + if (rknpu_obj->sram_obj != NULL) + rknpu_mm_free(rknpu_dev->sram_mm, + rknpu_obj->sram_obj); + rknpu_gem_free_buf_with_cache(rknpu_obj, + RKNPU_CACHE_SRAM); + } else if (IS_ENABLED(CONFIG_NO_GKI) && + rknpu_obj->nbuf_size > 0) { + rknpu_gem_free_buf_with_cache(rknpu_obj, + RKNPU_CACHE_NBUF); + } else { + rknpu_gem_free_buf(rknpu_obj); + } + } + + rknpu_gem_release(rknpu_obj); + rknpu_iommu_domain_put(rknpu_dev); +} + +static void rknpu_gem_drain_deferred(struct rknpu_device *rknpu_dev) +{ + struct rknpu_gem_object *o, *tmp; + + if (list_empty(&rknpu_deferred_destroy)) + return; + + mutex_lock(&rknpu_deferred_lock); + list_for_each_entry_safe(o, tmp, &rknpu_deferred_destroy, deferred) { + if (o->iommu_domain_id != rknpu_dev->iommu_domain_id) + continue; /* not this domain's turn — no switch, no timeout */ + if (rknpu_iommu_domain_get_and_switch(rknpu_dev, + o->iommu_domain_id)) + continue; /* same-domain fast path failed; try again later */ + list_del(&o->deferred); + rknpu_dbg_destroy_reclaimed++; + rknpu_gem_free_body(o); + } + mutex_unlock(&rknpu_deferred_lock); +} + void rknpu_gem_object_destroy(struct rknpu_gem_object *rknpu_obj) { struct drm_gem_object *obj = &rknpu_obj->base; @@ -835,14 +936,26 @@ void rknpu_gem_object_destroy(struct rknpu_gem_object *rknpu_obj) rknpu_dev, rknpu_obj->iommu_domain_id); if (ret && ++wait_count >= 3) { + /* #patch70: QUEUE instead of leaking. Reclaimed by + * rknpu_gem_drain_deferred() once this domain is current again. */ + rknpu_dbg_destroy_bailed++; LOG_DEV_ERROR( rknpu_dev->dev, - "failed to destroy dma addr: %pad, size: %lu\n", - &rknpu_obj->dma_addr, rknpu_obj->size); + "cannot switch to domain %d to destroy dma addr %pad (%lu bytes) — DEFERRED, not leaked (defer #%lu)\n", + rknpu_obj->iommu_domain_id, + &rknpu_obj->dma_addr, rknpu_obj->size, + rknpu_dbg_destroy_bailed); + mutex_lock(&rknpu_deferred_lock); + list_add_tail(&rknpu_obj->deferred, + &rknpu_deferred_destroy); + mutex_unlock(&rknpu_deferred_lock); return; } } while (ret); + /* #patch70: this domain is current and referenced — reclaim anything queued for it. */ + rknpu_gem_drain_deferred(rknpu_dev); + /* * do not release memory region from exporter. * @@ -850,7 +963,15 @@ void rknpu_gem_object_destroy(struct rknpu_gem_object *rknpu_obj) * once dmabuf's refcount becomes 0. */ if (obj->import_attach) { + /* #patchB10: 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) && @@ -878,10 +999,25 @@ int rknpu_gem_create_ioctl(struct drm_device *drm, void *data, { struct rknpu_mem_create *args = data; struct rknpu_gem_object *rknpu_obj = NULL; + struct drm_gem_object *obj = NULL; int ret = -EINVAL; - rknpu_obj = rknpu_gem_object_find(file_priv, args->handle); - if (!rknpu_obj) { + /* #patch38: hold a real reference, same reasoning as #patch37 in + * rknpu_gem_destroy_ioctl(). rknpu_gem_object_find() is lookup-then-put and returns an + * UNREFERENCED pointer kept alive only by the handle table, so every dereference below + * raced a concurrent close of that handle. The window here is far shorter than the + * destroy path's (nothing sleeps between the lookup and the last use), which is why this + * one is a hardening fix rather than a suspected crash source -- but the object is read + * four times after the lookup, and correctness should not rest on "probably not + * preempted there". + * + * Both branches below leave exactly ONE reference owned by us, dropped at the end. Net + * refcount across the ioctl is unchanged from before: the handle keeps the object alive + * afterwards, exactly as it always did. */ + obj = drm_gem_object_lookup(file_priv, args->handle); + if (obj) { + rknpu_obj = to_rknpu_obj(obj); + } else { rknpu_obj = rknpu_gem_object_create(drm, args->flags, args->size, args->sram_size, args->iommu_domain_id, @@ -895,15 +1031,19 @@ int rknpu_gem_create_ioctl(struct drm_device *drm, void *data, rknpu_gem_object_destroy(rknpu_obj); return ret; } + /* rknpu_gem_handle_create() dropped the allocation reference (the handle owns it + * now), so take our own to match the lookup branch. */ + obj = &rknpu_obj->base; + drm_gem_object_get(obj); } - // rknpu_gem_object_get(&rknpu_obj->base); - args->size = rknpu_obj->size; args->sram_size = rknpu_obj->sram_size; args->obj_addr = (__u64)(uintptr_t)rknpu_obj; args->dma_addr = rknpu_obj->dma_addr; + rknpu_gem_object_put(obj); + return 0; } @@ -927,12 +1067,33 @@ int rknpu_gem_destroy_ioctl(struct drm_device *drm, void *data, struct rknpu_device *rknpu_dev = drm->dev_private; struct rknpu_gem_object *rknpu_obj = NULL; struct rknpu_mem_destroy *args = data; + struct drm_gem_object *obj = NULL; int ret = 0; int wait_count = 0; - rknpu_obj = rknpu_gem_object_find(file_priv, args->handle); - if (!rknpu_obj) + /* #patch37: hold a REAL reference for the whole ioctl. + * + * rknpu_gem_object_find() is lookup-then-put -- it takes a reference via + * drm_gem_object_lookup() and drops it again before returning, so the caller receives an + * UNREFERENCED pointer kept alive only by the handle table. (Its kerneldoc, which claims + * the refcount "would be increased", is wrong; that comment is how this was missed.) + * + * Survivable for a short ioctl. Not for this one: it then blocks in + * rknpu_iommu_domain_get_and_switch() for up to 6 s per attempt, three attempts -- ~18 s -- + * and dereferences rknpu_obj->iommu_domain_id inside the loop. If the owning process dies + * in that window (SIGTERM -> DRM file close -> every handle released -> objects freed) the + * pointer dangles and we walk freed memory. + * + * The window is wide open in practice, not theoretical: a stalled NPU job is precisely what + * makes the domain switch time out, and a stuck run is precisely when the process is killed. + * Suspected source of the "BUG: Bad page cache ... still mapped when deleted" corruption + * (rknpu maps GEM shmem pages with vm_insert_page() and clears VM_PFNMAP, and vma->vm_file + * stays the DRM node -- so the shmem inode's i_mmap tree cannot find those VMAs to unmap at + * evict, and the GEM refcount is the ONLY thing preventing a premature free). */ + obj = drm_gem_object_lookup(file_priv, args->handle); + if (!obj) return -EINVAL; + rknpu_obj = to_rknpu_obj(obj); do { ret = rknpu_iommu_domain_get_and_switch( @@ -941,12 +1102,18 @@ int rknpu_gem_destroy_ioctl(struct drm_device *drm, void *data, if (ret && ++wait_count >= 3) { LOG_DEV_ERROR(rknpu_dev->dev, "failed to destroy memory\n"); + rknpu_gem_object_put(obj); return ret; } } while (ret); ret = rknpu_gem_handle_destroy(file_priv, args->handle); + /* Drop ours BEFORE the domain put: if this was the last reference, the object's teardown + * runs here with the domain still held -- the same ordering the original code had, where + * handle_destroy itself could free the object at exactly this point. */ + rknpu_gem_object_put(obj); + rknpu_iommu_domain_put(rknpu_dev); return ret; @@ -1398,7 +1565,29 @@ 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); + /* #patchB10: 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 actually run, then restore. Without this an imported weight + * lands in domain 0 while being recorded as domain N, and every submit against it is committed + * and never completes. domain_lock keeps a concurrent switch from moving the live domain + * underneath the map. */ + 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); + + 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 @@ -1422,6 +1611,7 @@ rknpu_gem_prime_import_sg_table(struct drm_device *dev, struct sg_table *sgt) { struct rknpu_gem_object *rknpu_obj = NULL; + struct rknpu_device *rknpu_dev = dev->dev_private; int npages = 0; int ret = -EINVAL; @@ -1432,6 +1622,9 @@ rknpu_gem_prime_import_sg_table(struct drm_device *dev, } rknpu_obj->dma_addr = sg_dma_address(sgt->sgl); + /* #patch: the sg above was mapped against the CURRENTLY attached domain. + * Record it, or destroy unmaps from domain 0 and strands this IOVA. */ + rknpu_obj->iommu_domain_id = rknpu_dev->iommu_domain_id; npages = rknpu_obj->size >> PAGE_SHIFT; rknpu_obj->pages = rknpu_gem_alloc_page(npages); diff --git a/drivers/rknpu/rknpu_iommu.c b/drivers/rknpu/rknpu_iommu.c index 3e05fd5e5..23a1d0744 100644 --- a/drivers/rknpu/rknpu_iommu.c +++ b/drivers/rknpu/rknpu_iommu.c @@ -10,21 +10,167 @@ #include "rknpu_iommu.h" +/* #patch30 diagnostics */ +unsigned long rknpu_dbg_switch_inflight; +unsigned long rknpu_dbg_switch_idle; +int rknpu_dbg_switch_verbose; +module_param_named(dbg_switch_inflight, rknpu_dbg_switch_inflight, ulong, 0444); +module_param_named(dbg_switch_idle, rknpu_dbg_switch_idle, ulong, 0444); +module_param_named(dbg_switch_verbose, rknpu_dbg_switch_verbose, int, 0644); + #define RKNPU_SWITCH_DOMAIN_WAIT_TIME_MS 6000 + +/* #patch59: what RATCHETS across runs? + * + * The mismatch wedge is cumulative — it appears on roughly the 4th run of a boot and clears only on + * reboot, so something is persistent kernel state. dbg_domain_underflow stays 0, so it is not the + * refcount going negative. The two remaining candidates are a leaked domain REFERENCE (refcount + * never returns to 0, so every switch has to wait) and a leaked MAPPING (IOVA space exhausted, so + * dma map eventually fails — which is the observed proximate cause, + * "rknpu_gem_get_pages: dma map fail"). + * + * Sample all three after each run: whichever monotonically climbs is the leak. refcount_now is a + * shadow of the atomic, updated wherever it is touched, so it can be read from sysfs. */ +unsigned long rknpu_dbg_refcnt_now; +module_param_named(dbg_refcnt_now, rknpu_dbg_refcnt_now, ulong, 0444); +/* #patch69: outstanding IOVA, and the destroy-path leak that is suspected of holding it. + * + * REINIT frees domains 1..15 but NOT domain 0 (the IOMMU core's default), so anything leaked there + * survives everything short of a reboot — and the wedge presents as dma map failures in domain 0. + * The suspected holder is rknpu_gem_object_destroy(), which bails after 3 failed domain switches and + * returns WITHOUT freeing, leaking the object and its IOVA. Count the bytes in and out, and the bails, + * and see whether they ratchet together across runs. */ +unsigned long rknpu_dbg_iova_alloc_kb, rknpu_dbg_iova_free_kb; +module_param_named(dbg_iova_alloc_kb, rknpu_dbg_iova_alloc_kb, ulong, 0444); +module_param_named(dbg_iova_free_kb, rknpu_dbg_iova_free_kb, ulong, 0444); +unsigned long rknpu_dbg_map_n, rknpu_dbg_unmap_n; +module_param_named(dbg_map_n, rknpu_dbg_map_n, ulong, 0444); +module_param_named(dbg_unmap_n, rknpu_dbg_unmap_n, ulong, 0444); + + +/* #patch51: distinguish "this DOMAIN is bad" from "the first commit after a SWITCH is bad". + * Per-domain stall rate alone cannot separate them: a domain that is visited briefly has a much + * larger FRACTION of its commits sitting immediately after a switch, so it would look bad either + * way. Reset on every switch, incremented per commit; logged per commit so the two can be + * disentangled by controlling for it. */ +unsigned int rknpu_commits_since_switch; +/* #patch52: COUNT vs TIME. sinceswitch alone cannot tell "the first N commits are poisoned" from + * "there is a settling window of T microseconds", because the first jobs into a new domain arrive in + * a burst -- the two describe the same events. Stamp the switch so each commit can also record how + * long ago it happened; then the stall boundary can be checked against both axes independently. + * They imply different fixes: a count means prime/quiesce, a time means a cooldown. */ +ktime_t rknpu_last_switch_time; +/* #patch53 */ +/* #patch54: ESCALATION for the domain-switch livelock. + * + * get_and_switch() only proceeds when iommu_domain_refcount reads 0. A job that never completes + * pins that reference, so every switch then times out after RKNPU_SWITCH_DOMAIN_WAIT_TIME_MS (6 s) + * and returns -EINVAL -- and userspace simply retries, forever. Nothing breaks the loop: + * rknpu_job_timeout_clean() is only invoked when a NEW submit arrives, which is exactly what cannot + * happen while allocation is failing. + * + * Measured cost of having no bound: a run wedged the board hard enough to need a power cycle, with + * "switch iommu domain time out" repeating every 6 s and one job abort at 184 s. None of the kernel's + * own detectors can see it -- panic_on_oops needs a fault, softlockup needs a spinning CPU, and + * hung_task needs CONTINUOUS uninterruptible sleep whereas this loop wakes every 6-20 s. It is a + * livelock, invisible by construction. + * + * So bound it here: after N consecutive timeouts, reap the stuck jobs, which releases the references + * they hold. Reaping (not forcing the refcount to 0) is deliberate -- if a job really is still live, + * zeroing the count would let a switch happen underneath it and corrupt its translations. */ +static unsigned int rknpu_switch_escalate = 3; +module_param_named(switch_escalate, rknpu_switch_escalate, uint, 0644); +MODULE_PARM_DESC(switch_escalate, "consecutive domain-switch timeouts before reaping stuck jobs (0=off)"); +static unsigned int rknpu_switch_consec; +unsigned long rknpu_switch_escalations; +module_param_named(switch_escalations, rknpu_switch_escalations, ulong, 0444); +void rknpu_reap_all_cores(struct rknpu_device *rknpu_dev); + +unsigned int rknpu_dom_cooldown_us; +module_param_named(dom_cooldown_us, rknpu_dom_cooldown_us, uint, 0644); +MODULE_PARM_DESC(dom_cooldown_us, "settle this many us after an iommu domain switch (0=off)"); +unsigned long rknpu_dom_cooldowns; +module_param_named(dom_cooldowns, rknpu_dom_cooldowns, ulong, 0444); + +/* #patch33: never dereference a NULL iommu domain. + * + * rknpu_iommu_switch_domain() does detach(src) then attach(dst); if the attach fails it tries to + * re-attach src, and if THAT fails too the device is left attached to NO domain. The switch does + * fail in practice: a stalled job cannot be drained, so the switch times out + * ("switch iommu domain time out") and the abort path runs. The next GEM allocation then called + * iommu_get_domain_for_dev() and dereferenced the NULL result one line later: + * + * Unable to handle kernel NULL pointer dereference at virtual address 0000000000000040 + * pc : rknpu_iommu_dma_map_sg+0x48 Comm: i4_widek_stall_ + * rknpu_gem_object_create / rknpu_gem_create_ioctl + * + * which kills the board (this is a strong candidate for the long-standing "NPU wedge needs a + * power cycle"). Turn an unrecoverable panic into an ordinary error return. */ +/* #patchB2: provided by drivers/iommu/rockchip-iommu.c (#patchB1). Reprograms the MMU page-table + * base in place -- stall / write DTE_ADDR / ZAP_CACHE / unstall -- with no force-reset and WITHOUT + * involving the IOMMU core, so no detach/attach pair and no need to overwrite the group's default + * domain. Consequence: iommu_get_domain_for_dev() no longer tracks our switches. */ +extern int rk_iommu_switch_domain(struct device *dev, struct iommu_domain *domain); + +/* #patchB2: THE LIVE DOMAIN, from the driver's own bookkeeping. + * + * This replaces every iommu_get_domain_for_dev() call in this driver. Since the light switch keeps + * the IOMMU core out of the loop, the core keeps reporting the group's default domain (domain 0) + * forever -- which was only ever "right" before because the vendor code overwrote + * iommu_group->default_domain on each switch. That overwrite is gone; the driver owns the answer. */ +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 struct rknpu_iommu_dma_cookie *rknpu_iommu_cookie(struct device *dev, + const char *who) +{ + struct iommu_domain *domain = rknpu_iommu_live_domain(dev); + + if (!domain) { + dev_err_ratelimited(dev, "%s: NPU attached to NO iommu domain (a domain switch left it detached)\n", + who); + return NULL; + } + if (!domain->iova_cookie) { + dev_err_ratelimited(dev, "%s: iommu domain has no iova cookie\n", who); + return NULL; + } + return (struct rknpu_iommu_dma_cookie *)domain->iova_cookie; +} + + dma_addr_t rknpu_iommu_dma_alloc_iova(struct iommu_domain *domain, size_t size, u64 dma_limit, struct device *dev, bool size_aligned) { - struct rknpu_iommu_dma_cookie *cookie = (void *)domain->iova_cookie; - struct iova_domain *iovad = &cookie->iovad; + struct rknpu_iommu_dma_cookie *cookie; + struct iova_domain *iovad; unsigned long shift, iova_len, iova = 0; unsigned long limit_pfn; struct iova *new_iova = NULL; bool alloc_fast = size_aligned; + if (!domain || !domain->iova_cookie) { /* #patch34 */ + dev_err_ratelimited(dev, "%s: no iommu domain/cookie\n", __func__); + return 0; + } + cookie = (struct rknpu_iommu_dma_cookie *)domain->iova_cookie; + iovad = &cookie->iovad; + shift = iova_shift(iovad); iova_len = size >> shift; + rknpu_dbg_iova_alloc_kb += (unsigned long)(size >> 10); /* #patch69 */ #if KERNEL_VERSION(6, 1, 0) > LINUX_VERSION_CODE /* @@ -69,6 +215,7 @@ dma_addr_t rknpu_iommu_dma_alloc_iova(struct iommu_domain *domain, size_t size, void rknpu_iommu_dma_free_iova(struct rknpu_iommu_dma_cookie *cookie, dma_addr_t iova, size_t size, bool size_aligned) { + rknpu_dbg_iova_free_kb += (unsigned long)(size >> 10); /* #patch69 */ struct iova_domain *iovad = &cookie->iovad; bool alloc_fast = size_aligned; @@ -208,9 +355,10 @@ 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 rknpu_iommu_dma_cookie *cookie = (void *)domain->iova_cookie; - struct iova_domain *iovad = &cookie->iovad; + struct iommu_domain *domain = rknpu_iommu_live_domain(dev); + struct rknpu_iommu_dma_cookie *cookie = + rknpu_iommu_cookie(dev, __func__); /* #patch33 */ + struct iova_domain *iovad; struct scatterlist *s = NULL, *prev = NULL; int prot = rknpu_dma_info_to_prot(dir, dev_is_dma_coherent(dev)); dma_addr_t iova; @@ -219,6 +367,11 @@ int rknpu_iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, ssize_t ret = -EINVAL; int i = 0; + if (!cookie) + return 0; /* #patch33: 0 == mapping failed, per this function's convention */ + rknpu_dbg_map_n++; /* #patch59 */ + iovad = &cookie->iovad; + if (iova_aligned) return dma_map_sg(dev, sg, nents, dir); @@ -298,9 +451,10 @@ 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 rknpu_iommu_dma_cookie *cookie = (void *)domain->iova_cookie; - struct iova_domain *iovad = &cookie->iovad; + struct iommu_domain *domain = rknpu_iommu_live_domain(dev); + struct rknpu_iommu_dma_cookie *cookie = + rknpu_iommu_cookie(dev, __func__); /* #patch34 */ + struct iova_domain *iovad; size_t iova_off = 0; dma_addr_t end = 0, start = 0; struct scatterlist *tmp = NULL; @@ -308,6 +462,11 @@ void rknpu_iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, size_t size = 0; int i = 0; + if (!cookie) + return; /* #patch34: nothing mapped through a domain that is gone */ + rknpu_dbg_unmap_n++; /* #patch59 */ + iovad = &cookie->iovad; + if (iova_aligned) return dma_unmap_sg(dev, sg, nents, dir); @@ -403,6 +562,33 @@ struct iommu_group { }; #endif +/* #patchB10: temporarily point the IOMMU core's DEFAULT domain at the LIVE domain. + * + * The DMA API resolves every mapping through iommu_get_dma_domain() == group->default_domain, and + * dma_buf_map_attachment() -- which DRM core runs during PRIME_FD_TO_HANDLE, before any rknpu hook + * -- goes through the DMA API. So an imported weight's sg is mapped into domain 0 while the object + * is recorded as domain N: the NPU then resolves an IOVA that exists only in domain 0's page table, + * and the job is committed but never completes. + * + * This is deliberately NOT the vendor's overwrite. That one was PERMANENT -- every switch left the + * core's default pointing at a domain the driver might later free, so any core path that reattaches + * "the default" (reset, release, error recovery) used it, which is where the mismatch-domain class + * came from. Here the override lives only for the duration of one attachment map/unmap, 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 @@ -443,28 +629,73 @@ 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; + /* #patch30 (diagnostic): a domain switch calls iommu_attach_device(), and for + * rockchip-iommu that runs rk_iommu_enable() -> enable_stall + FORCE_RESET of every + * MMU bank + ZAP_CACHE + paging off/on. If an NPU job is still in flight, its + * descriptor/regcmd fetches are cut mid-dispatch: the job is committed but the PC + * completed-task counter never advances, and nothing faults. Count switches that + * race in-flight work so it can be correlated with the observed dispatch stalls. */ + { + int _i, _busy = 0; + unsigned long _f; + spin_lock_irqsave(&rknpu_dev->irq_lock, _f); + for (_i = 0; _i < rknpu_dev->config->num_irqs; _i++) + if (rknpu_dev->subcore_datas[_i].job) + _busy++; + spin_unlock_irqrestore(&rknpu_dev->irq_lock, _f); + if (_busy) { + rknpu_dbg_switch_inflight++; + if (rknpu_dbg_switch_verbose) + LOG_WARN("switch domain %d->%d with %d job(s) IN FLIGHT (n=%lu)\n", + src_domain_id, domain_id, _busy, + rknpu_dbg_switch_inflight); + } else { + rknpu_dbg_switch_idle++; + } } + /* #patchB2: the "mismatch domain" check is gone with the core detach/attach it guarded. + * iommu_get_domain_for_dev() reports the group's default domain, which no longer tracks our + * switches -- so the check could only ever fire spuriously. The driver's own + * iommu_domains[src_domain_id] IS the source of truth now. */ + 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); /* #patchB2 */ if (ret) { LOG_DEV_ERROR( rknpu_dev->dev, "failed to attach dst iommu domain, id: %d, ret: %d\n", domain_id, ret); - if (iommu_attach_device(src_domain, rknpu_dev->dev)) { + /* #patchB3: recover with the light switch too. + * + * Note the failure mode is much weaker than it used to be: the old code had + * ALREADY executed iommu_detach_device(src) before the failing attach, so a + * failure left the device attached to NO domain and panicked the box on the + * next map (#patch34). rk_iommu_switch_domain() is atomic from the caller's + * point of view -- it either reprogrammed DTE_ADDR or it did not -- so on + * failure the previous page table is still live and this is belt-and-braces. */ + if (rk_iommu_switch_domain(rknpu_dev->dev, src_domain)) { + int _d; + 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); + for (_d = 0; _d < RKNPU_MAX_IOMMU_DOMAIN_NUM; _d++) { + if (!rknpu_dev->iommu_domains[_d]) + continue; + if (!rk_iommu_switch_domain( + rknpu_dev->dev, + rknpu_dev->iommu_domains[_d])) { + rknpu_dev->iommu_domain_id = _d; + LOG_DEV_ERROR(rknpu_dev->dev, + "recovered onto iommu domain %d\n", + _d); + break; + } + } } return ret; } @@ -481,36 +712,129 @@ int rknpu_iommu_switch_domain(struct rknpu_device *rknpu_dev, int domain_id) // 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); + /* #patchB2: initialise OUR OWN IOVA allocator for this domain. + * + * The vendor code got an iovad by faking a DMA-API domain: it set + * __IOMMU_DOMAIN_DMA_API on the type and called iommu_setup_dma_ops(), whose only + * useful effect here was iommu_dma_init_domain() initialising the cookie's iovad. + * That route needs iommu_get_domain_for_dev() to report this domain, i.e. it needs + * the default_domain overwrite. Do the two init calls directly instead -- same + * iovad, no lie, and this domain stays a plain UNMANAGED domain that iommu_map_sg() + * and iommu_unmap() are happy to serve. + * + * Granule/start_pfn mirror iommu_dma_init_domain(): 4 KiB pages, base_pfn 1 so that + * 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); /* #patchB2 */ 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; + /* #patchB2: the default_domain overwrite is DELETED. + * + * It existed because dma-iommu resolves every DMA-API mapping through + * iommu_get_dma_domain() == dev->iommu_group->default_domain, so the only way to make + * dma_map_sg() land in domain N was to tell the core that N was the default. That lie is + * the origin of the "mismatch domain" errors and of attaching a domain the driver may + * later free. Nothing here uses the DMA API for NPU buffers any more: allocations are + * NON_CONTIGUOUS + IOMMU_LIMIT_IOVA_ALIGNMENT, which take the explicit + * iommu_map_sg()/iommu_unmap() path against the domain the driver names. */ + + /* #patch53: COOLDOWN after a domain switch. + * + * Measured, replicated across two unrelated code paths (blocking colsplit on a 1.5B model, + * and the async int4 doorbell probe): EVERY observed stall was a commit landing within + * ~1 ms of an IOMMU domain switch, and there were ZERO stalls beyond that window across + * ~10,000 commits. Being early by COUNT is not the predictor -- 9004 commits at position + * 0-2 produced 5 stalls, all of them in the sub-1 ms subset while 8993 early-but->=1ms + * commits were clean. Time is the operative variable. + * + * Sleeping here is legal and correct: rknpu_iommu_switch_domain() is only ever reached from + * process context (gem create/destroy/sync ioctls and the submit path), never from the + * completion IRQ. That is why this can usleep_range() where the earlier per-commit spacing + * experiment was forced into udelay() -- and it is charged once per SWITCH, not once per + * commit, so the cost is ~1 ms x the switch count (~56 in a 1.5B run) rather than per job. + * + * NOT a claim about mechanism: the window is necessary but not sufficient (inside it the + * rate ranges 0.2%-90.9% by workload), so something else modulates it. This removes the + * necessary condition, which is enough to eliminate every stall observed so far. + * + * 0 = off (default, opt-in until measured). */ + if (rknpu_dom_cooldown_us) { + usleep_range(rknpu_dom_cooldown_us, rknpu_dom_cooldown_us + 100); + rknpu_dom_cooldowns++; + } + rknpu_commits_since_switch = 0; /* #patch51 */ + rknpu_last_switch_time = ktime_get(); /* #patch52 -- stamp AFTER the cooldown */ LOG_INFO("switch iommu domain from %d to %d\n", src_domain_id, domain_id); return ret; } +/* #patchB5: RECLAIM-AND-RETRY on a domain-switch timeout (default ON). + * + * A stalled job (dispatch stall: committed, PC task counter stuck at 0, no interrupt) holds its + * iommu domain reference forever. rknpu_iommu_domain_get_and_switch() only proceeds when the + * refcount reads 0, so from the first stall onward EVERY switch burns its full 6 s and fails -- + * and because rknpu_gem_object_create() switches domains, every later allocation fails -EINVAL + * ("MEM_CREATE errno=22" at zero IOVA used). One stall therefore kills the whole run, and for a + * >4 GiB model spread over several domains that is fatal rather than merely slow. + * + * patch54 already reaped on timeout, but it reaped and then RETURNED -EINVAL anyway, so the + * caller that triggered the reap still failed; only some later call could benefit. It also + * required `switch_escalate` CONSECUTIVE timeouts (3 x 6 s = 18 s), and any success in between + * reset the counter. Measured: the reproducer still died with escalate=3. + * + * So: reap on the FIRST timeout, then RETRY the switch. If the reap cleared every in-flight job + * but the refcount is still non-zero, that count is by definition leaked -- no job owns it -- so + * force it to 0 rather than stay wedged forever. + * + * Note the reap only runs after the full 6 s wait has already elapsed, so anything still in + * flight has been running for >= 6 s. A real NPU job here is microseconds to milliseconds, so + * that set is exactly the stuck jobs. A submit landing in the last instant before the reap could + * be caught with them; that is a far better outcome than a permanently dead domain. */ +unsigned int rknpu_dom_reclaim = 1; +module_param_named(dom_reclaim, rknpu_dom_reclaim, uint, 0644); +MODULE_PARM_DESC(dom_reclaim, "on a domain-switch timeout, reap stuck jobs and retry (0=off)"); +unsigned long rknpu_dbg_reclaim_tries, rknpu_dbg_reclaim_ok, rknpu_dbg_reclaim_forced, + rknpu_dbg_reclaim_fail; +module_param_named(dbg_reclaim_tries, rknpu_dbg_reclaim_tries, ulong, 0444); +module_param_named(dbg_reclaim_ok, rknpu_dbg_reclaim_ok, ulong, 0444); +module_param_named(dbg_reclaim_forced, rknpu_dbg_reclaim_forced, ulong, 0444); +module_param_named(dbg_reclaim_fail, rknpu_dbg_reclaim_fail, ulong, 0444); + int rknpu_iommu_domain_get_and_switch(struct rknpu_device *rknpu_dev, int domain_id) { @@ -518,12 +842,15 @@ int rknpu_iommu_domain_get_and_switch(struct rknpu_device *rknpu_dev, msecs_to_jiffies(RKNPU_SWITCH_DOMAIN_WAIT_TIME_MS); unsigned long start = jiffies; int ret = -EINVAL; + int reclaimed = 0; /* #patchB5 */ while (true) { mutex_lock(&rknpu_dev->domain_lock); if (domain_id == rknpu_dev->iommu_domain_id) { + rknpu_switch_consec = 0; /* #patch54 */ atomic_inc(&rknpu_dev->iommu_domain_refcount); + rknpu_dbg_refcnt_now = atomic_read(&rknpu_dev->iommu_domain_refcount); /* #patch59 */ mutex_unlock(&rknpu_dev->domain_lock); break; } @@ -538,7 +865,9 @@ int rknpu_iommu_domain_get_and_switch(struct rknpu_device *rknpu_dev, mutex_unlock(&rknpu_dev->domain_lock); return ret; } + rknpu_switch_consec = 0; /* #patch54 */ atomic_inc(&rknpu_dev->iommu_domain_refcount); + rknpu_dbg_refcnt_now = atomic_read(&rknpu_dev->iommu_domain_refcount); /* #patch59 */ mutex_unlock(&rknpu_dev->domain_lock); break; } @@ -551,6 +880,59 @@ int rknpu_iommu_domain_get_and_switch(struct rknpu_device *rknpu_dev, rknpu_dev->dev, "switch iommu domain time out, failed to switch iommu domain, id: %d\n", domain_id); + /* #patchB5: reap the stuck job(s), then RETRY -- do not fail the caller + * that is trying to make progress. Once only: if the switch still times + * out after a successful reclaim, something is wrong beyond a stuck job. */ + if (rknpu_dom_reclaim && !reclaimed) { + unsigned long f; + int k, busy = 0; + + reclaimed = 1; + rknpu_dbg_reclaim_tries++; + rknpu_reap_all_cores(rknpu_dev); + + spin_lock_irqsave(&rknpu_dev->irq_lock, f); + for (k = 0; k < rknpu_dev->config->num_irqs; k++) + if (rknpu_dev->subcore_datas[k].job) + busy++; + spin_unlock_irqrestore(&rknpu_dev->irq_lock, f); + + if (!busy) { + int rc = atomic_read(&rknpu_dev->iommu_domain_refcount); + + if (rc != 0) { + /* nothing is in flight, so this count is leaked */ + LOG_DEV_ERROR(rknpu_dev->dev, + "reclaim: no jobs in flight but refcount=%d -- forcing to 0\n", + rc); + atomic_set(&rknpu_dev->iommu_domain_refcount, 0); + rknpu_dbg_refcnt_now = 0; + rknpu_dbg_reclaim_forced++; + } + LOG_DEV_ERROR(rknpu_dev->dev, + "reclaim: domain reference released, retrying switch to id %d\n", + domain_id); + rknpu_dbg_reclaim_ok++; + start = jiffies; + continue; + } + + rknpu_dbg_reclaim_fail++; + LOG_DEV_ERROR(rknpu_dev->dev, + "reclaim: %d job(s) survived the reap -- cannot release the domain\n", + busy); + } + + /* #patch54: break the livelock rather than returning into an infinite retry */ + if (rknpu_switch_escalate && + ++rknpu_switch_consec >= rknpu_switch_escalate) { + rknpu_switch_consec = 0; + rknpu_switch_escalations++; + LOG_DEV_ERROR(rknpu_dev->dev, + "ESCALATING after %u consecutive switch timeouts: reaping stuck jobs to release the domain reference\n", + rknpu_switch_escalate); + rknpu_reap_all_cores(rknpu_dev); + } return -EINVAL; } } @@ -558,9 +940,139 @@ int rknpu_iommu_domain_get_and_switch(struct rknpu_device *rknpu_dev, return 0; } +/* #patch45 diagnostics: the live refcount, and how often a put tried to take it negative. */ +unsigned long rknpu_dbg_premature_zero; +module_param_named(dbg_premature_zero, rknpu_dbg_premature_zero, ulong, 0444); +unsigned long rknpu_dbg_domain_underflow; +module_param_named(dbg_domain_underflow, rknpu_dbg_domain_underflow, ulong, 0444); + int rknpu_iommu_domain_put(struct rknpu_device *rknpu_dev) { - atomic_dec(&rknpu_dev->iommu_domain_refcount); + /* #patch45: NEVER let the refcount go negative. + * + * rknpu_iommu_domain_get_and_switch() proceeds only when this reads exactly 0 + * (`if (atomic_read(&iommu_domain_refcount) == 0)`). A bare atomic_dec() therefore turns a + * single unbalanced put into a PERMANENT wedge: the count never reads 0 again, every domain + * switch burns its 6 s RKNPU_SWITCH_DOMAIN_WAIT_TIME_MS and fails, and because + * rknpu_gem_object_create() switches domains, EVERY allocation then fails with -EINVAL + * ("rknpu_gem_object_create error") until reboot. + * + * Unbalanced puts are a live risk here, not a hypothetical: our own kernel change 02 ADDED a + * put to rknpu_job_timeout_clean() to reclaim a leaked reference, which double-puts on any + * path that already released it. That is precisely the wedge seen repeatedly on this board + * (59 'mismatch domain'/'switch iommu domain time out' lines in a single 300 s run), and it + * invalidated several measurements before it was understood. + * + * Clamping is strictly better than wedging: an over-put becomes a counted anomaly instead of + * a dead NPU. The clamp is racy against a concurrent get, but the alternative it replaces is + * an unrecoverable device, so the trade is not close. dbg_domain_underflow != 0 means a real + * refcount bug remains to be found -- the clamp treats the symptom, not the cause. */ + /* #patch60: NAME the over-putting caller. + * + * Six runs showed dbg_domain_underflow reaching 5 — excess puts — but the counter alone + * cannot say WHICH of the ~10 call sites is unbalanced, and change 02 (our added put in + * rknpu_job_timeout_clean) is only a suspect, not a finding. %pS resolves the return address + * to a symbol, so the offending site names itself. + * + * Two conditions are reported, and the SECOND is the one that actually causes damage: + * - underflow: the count went below zero. Loud, but already too late. + * - premature zero: the count hit exactly 0 while jobs are still IN FLIGHT. No underflow, + * no warning, but rknpu_iommu_domain_get_and_switch() tests only `refcount == 0`, so it + * will now switch domains out from under live work — which is the mismatch-domain + * signature. These are invisible to the underflow counter and must be more numerous. */ + { + void *caller = __builtin_return_address(0); + int after; + + rknpu_dbg_refcnt_now = atomic_read(&rknpu_dev->iommu_domain_refcount) - 1; + after = atomic_dec_return(&rknpu_dev->iommu_domain_refcount); + if (after < 0) { + atomic_set(&rknpu_dev->iommu_domain_refcount, 0); + rknpu_dbg_refcnt_now = 0; + rknpu_dbg_domain_underflow++; + LOG_DEV_ERROR(rknpu_dev->dev, + "domain_put UNDERFLOW (clamped) from %pS\n", caller); + } else if (after == 0) { + unsigned long f; + int k, busy = 0; + + spin_lock_irqsave(&rknpu_dev->irq_lock, f); + for (k = 0; k < rknpu_dev->config->num_irqs; k++) + if (rknpu_dev->subcore_datas[k].job) + busy++; + spin_unlock_irqrestore(&rknpu_dev->irq_lock, f); + if (busy) { + rknpu_dbg_premature_zero++; + LOG_DEV_ERROR(rknpu_dev->dev, + "domain refcount hit 0 with %d job(s) IN FLIGHT — put from %pS\n", + busy, caller); + } + } + } + + return 0; +} + +/* #patch61: explicit state reset — the software equivalent of the reboot. + * + * Refuses while any core has a job, because freeing a domain out from under live work would strand + * its translations. Beyond freeing the domains it also zeroes the refcount and the diagnostic + * counters, so a fresh run starts from a known state rather than inheriting a skewed one. Note the + * caller must treat every existing buffer AND every regcmd it has built as invalid afterwards: the + * IOVAs change, and userspace bakes IOVAs into its programs. */ +int rknpu_iommu_reinit(struct rknpu_device *rknpu_dev) +{ + unsigned long flags; + int i, busy = 0; + + spin_lock_irqsave(&rknpu_dev->irq_lock, flags); + for (i = 0; i < rknpu_dev->config->num_irqs; i++) + if (rknpu_dev->subcore_datas[i].job) + busy++; + spin_unlock_irqrestore(&rknpu_dev->irq_lock, flags); + /* #patch62: ABORT in-flight work rather than refusing. + * + * The first cut returned -EBUSY whenever any core had a job. That made the call useless for + * its entire purpose: a device in the bad state almost always HAS a stuck job — that is what + * being stuck means — so REINIT refused with -EBUSY at exactly the moment it was needed + * (measured: idle device -> ok; wedged device -> -EBUSY). The guard was protecting the caller + * from the case the caller was trying to fix. + * + * So reap first, then re-check. rknpu_reap_all_cores() runs the driver's own timeout_clean + * path, which aborts the jobs and releases the references they hold. If a core STILL reports a + * job after that, something is wedged beyond what this call can safely unpick, and -EBUSY is + * then a truthful answer rather than an unhelpful one. */ + if (busy) { + LOG_INFO("RKNPU: REINIT — %d job(s) in flight, reaping before teardown\n", busy); + rknpu_reap_all_cores(rknpu_dev); + + busy = 0; + spin_lock_irqsave(&rknpu_dev->irq_lock, flags); + for (i = 0; i < rknpu_dev->config->num_irqs; i++) + if (rknpu_dev->subcore_datas[i].job) + busy++; + spin_unlock_irqrestore(&rknpu_dev->irq_lock, flags); + + if (busy) { + LOG_DEV_ERROR(rknpu_dev->dev, + "REINIT: %d job(s) survived the reap — refusing\n", + busy); + return -EBUSY; + } + } + + LOG_INFO("RKNPU: REINIT — freeing all iommu domains and resetting state (was: refcnt=%d underflow=%lu premature_zero=%lu)\n", + atomic_read(&rknpu_dev->iommu_domain_refcount), + rknpu_dbg_domain_underflow, rknpu_dbg_premature_zero); + + rknpu_iommu_free_domains(rknpu_dev); + atomic_set(&rknpu_dev->iommu_domain_refcount, 0); + rknpu_dbg_refcnt_now = 0; + rknpu_dbg_domain_underflow = 0; + rknpu_dbg_premature_zero = 0; + rknpu_dbg_map_n = 0; + rknpu_dbg_unmap_n = 0; + rknpu_switch_consec = 0; return 0; } @@ -580,7 +1092,16 @@ void rknpu_iommu_free_domains(struct rknpu_device *rknpu_dev) if (domain == NULL) continue; - iommu_detach_device(domain, rknpu_dev->dev); + /* #patchB3: no iommu_detach_device() -- the core never attached these domains. + * The switch to domain 0 above already moved the hardware off this page table, so + * freeing it here is safe. Release the IOVA allocator we initialised in the switch + * (init_iova_domain + iova_domain_init_rcaches) before the domain goes. */ + 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;