diff --git a/drivers/rknpu/include/rknpu_iommu.h b/drivers/rknpu/include/rknpu_iommu.h index 225176650..793cf9d3b 100644 --- a/drivers/rknpu/include/rknpu_iommu.h +++ b/drivers/rknpu/include/rknpu_iommu.h @@ -53,8 +53,17 @@ 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); + #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/rknpu_gem.c b/drivers/rknpu/rknpu_gem.c index f1346f98f..6ed8115b5 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,94 @@ 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) { + drm_prime_gem_destroy(obj, rknpu_obj->sgt); + 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 +928,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. * @@ -878,10 +983,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 +1015,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 +1051,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 +1086,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; @@ -1422,6 +1573,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 +1584,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..573eca1ef 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); @@ -443,28 +602,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 +685,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 +815,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 +838,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 +853,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 +913,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 +1065,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; diff --git a/drivers/rknpu/rknpu_reset.c b/drivers/rknpu/rknpu_reset.c index 65142368c..f5d44644e 100644 --- a/drivers/rknpu/rknpu_reset.c +++ b/drivers/rknpu/rknpu_reset.c @@ -8,6 +8,10 @@ #include #include "rknpu_reset.h" +#include "rknpu_iommu.h" + +/* #patchB8: provided by drivers/iommu/rockchip-iommu.c */ +extern int rk_iommu_reprogram(struct device *dev); #ifndef FPGA_PLATFORM static inline struct reset_control *rknpu_reset_control_get(struct device *dev, @@ -95,6 +99,101 @@ static int rknpu_reset_deassert(struct reset_control *rst) } #endif +/* #patch: PER-CORE soft reset. rknpu_soft_reset() asserts ALL SIX DT reset lines + * ("srst_a0","srst_a1","srst_a2","srst_h0","srst_h1","srst_h2" — AXI+AHB per core), + * sleeps 100 ms, and sets the global soft_reseting flag that makes rknpu_job_next() + * bail for EVERY core. Recovering one stalled core therefore resets the whole NPU and + * destroys the other two cores' warm state — most of the ~3.6 s a doorbell miss costs. + * + * The lines are per core, so reset only core N: indices N (AXI) and N+num_irqs (AHB). + * No msleep, no global flag, no IOMMU re-attach — the other cores keep running. */ +int rknpu_soft_reset_core(struct rknpu_device *rknpu_dev, int core) +{ +#ifndef FPGA_PLATFORM + int ret = 0, ia, ih; + + if (rknpu_dev->bypass_soft_reset) + return 0; + + ia = core; + ih = core + rknpu_dev->config->num_irqs; + if (core < 0 || ih >= rknpu_dev->num_srsts) + return -EINVAL; + + ret |= rknpu_reset_assert(rknpu_dev->srsts[ia]); + ret |= rknpu_reset_assert(rknpu_dev->srsts[ih]); + udelay(10); + ret |= rknpu_reset_deassert(rknpu_dev->srsts[ia]); + ret |= rknpu_reset_deassert(rknpu_dev->srsts[ih]); + udelay(10); + + wake_up(&rknpu_dev->subcore_datas[core].job_done_wq); + return ret; +#else + return 0; +#endif +} + +/* #patch: per-core reset + IOMMU re-programming — the minimal sequence measured to actually recover a + * stalled core. Shared by the RKNPU_ACT_RESET_CORE ioctl and the progress watchdog so both paths behave + * identically, mirroring how rknpu_soft_reset serves the device-wide action. */ +int rknpu_reset_core_and_remap(struct rknpu_device *rknpu_dev, int core) +{ +#ifndef FPGA_PLATFORM + struct iommu_domain *domain; + int ret; + + ret = rknpu_soft_reset_core(rknpu_dev, core); + if (ret) + return ret; + + if (!rknpu_dev->iommu_en) + return 0; + + /* the re-attach must not race a concurrent dma_map; quiesce as rknpu_soft_reset does */ + if (!mutex_trylock(&rknpu_dev->reset_lock)) + return -EBUSY; + rknpu_dev->soft_reseting = true; + /* #patchB9: reprogram the LIVE domain, exactly as rknpu_soft_reset() now does (#patchB8). + * + * This is the PER-CORE twin of that path (RKNPU_ACT_RESET_CORE + the progress watchdog) and it + * carried the identical bug: iommu_get_domain_for_dev() returns the group's DEFAULT domain + * under the light switch, so the detach/attach below re-attached domain 0 while the driver + * still believed domain N was live. B8 fixed the device-wide reset and missed this one -- and + * this is the path the watchdog drives during a multi-domain run. */ + domain = rknpu_iommu_live_domain(rknpu_dev->dev); + if (domain) { + /* #patch46: serialize against domain switching. + * + * rknpu_soft_reset() holds reset_lock; rknpu_iommu_domain_get_and_switch() holds + * domain_lock. Different locks, so this detach/attach had NO mutual exclusion against + * a concurrent switch -- and a switch that reads iommu_get_domain_for_dev() while we + * are detached sees a domain that does not match rknpu_dev->iommu_domains[], bails + * with "mismatch domain get from iommu_get_domain_for_dev", and fails the caller. + * + * Measured: 1600 mismatch lines in a single 300 s run once the fast-abort raised the + * reset rate ~30x. Low reset rates hid it; it was always latent. + * + * Ordering is reset_lock -> domain_lock. Safe: no path takes reset_lock while holding + * domain_lock (rknpu_iommu_switch_domain does not reset). */ + mutex_lock(&rknpu_dev->domain_lock); + { + int rp = rk_iommu_reprogram(rknpu_dev->dev); + + if (rp) + LOG_DEV_ERROR(rknpu_dev->dev, + "failed to reprogram iommu after per-core reset: %d\n", rp); + } + mutex_unlock(&rknpu_dev->domain_lock); + } + rknpu_dev->soft_reseting = false; + mutex_unlock(&rknpu_dev->reset_lock); + return 0; +#else + return 0; +#endif +} + int rknpu_soft_reset(struct rknpu_device *rknpu_dev) { #ifndef FPGA_PLATFORM @@ -107,8 +206,15 @@ int rknpu_soft_reset(struct rknpu_device *rknpu_dev) return 0; } + /* #patch: a failed trylock used to `return 0` — reporting SUCCESS without resetting anything. + * The caller (RKNPU_ACT_RESET from userspace recovery) then believes the NPU was reset when it + * was not, and spins on a still-wedged core. That was harmless while this was the only resetter; + * it stopped being harmless once the per-core recovery path also takes reset_lock, at which point + * userspace recovery silently became a no-op and the workload ran ~10x slower. + * Wait for the in-flight reset instead, so an ACT_RESET always means a reset actually happened. + * Legal: this function already msleep()s, so it is process context. */ if (!mutex_trylock(&rknpu_dev->reset_lock)) - return 0; + mutex_lock(&rknpu_dev->reset_lock); rknpu_dev->soft_reseting = true; @@ -138,13 +244,27 @@ int rknpu_soft_reset(struct rknpu_device *rknpu_dev) return ret; } - if (rknpu_dev->iommu_en) - domain = iommu_get_domain_for_dev(rknpu_dev->dev); - - if (domain) { - iommu_detach_device(domain, rknpu_dev->dev); - iommu_attach_device(domain, rknpu_dev->dev); + /* #patchB8: reprogram the MMU for the LIVE domain, not the core's default. + * + * This used to be iommu_detach_device(iommu_get_domain_for_dev(dev)) + iommu_attach_device(), + * which was only ever right because the old multi-domain code overwrote + * iommu_group->default_domain on every switch. With the light switch (#patchB1/B2) the IOMMU + * core is not in the loop, so iommu_get_domain_for_dev() returns the group's DEFAULT domain + * (domain 0) -- and this sequence re-attached domain 0 while rknpu_dev->iommu_domain_id still + * said domain N. Every IOVA the next job used then resolved against the wrong page table, so + * the job was committed and never completed: no IRQ, no error, an output buffer left holding + * its seed. That is the "first int8 op after an ACT_RESET is silently dropped" bug. + * + * rk_iommu_reprogram() re-enables the MMU from iommu->domain, which the light switch keeps + * pointed at the live domain, so the correct page table is restored. */ + if (rknpu_dev->iommu_en) { + int rp = rk_iommu_reprogram(rknpu_dev->dev); + + if (rp) + LOG_DEV_ERROR(rknpu_dev->dev, + "failed to reprogram iommu after soft reset: %d\n", rp); } + (void)domain; rknpu_dev->soft_reseting = false;