diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 5f86a0db4a2d3..1afe988c91833 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1323,6 +1323,125 @@ static void rk_iommu_detach_device(struct iommu_domain *domain, } } +/* + * Switch a master to a different domain by reprogramming the page table base in + * place, without going through the IOMMU core. + * + * The core path for this is iommu_detach_device() + iommu_attach_device(), and + * for rockchip-iommu the attach runs rk_iommu_enable(), i.e. a full + * rk_iommu_force_reset() of every MMU bank. With work in flight on other cores + * that reset intermittently fails its DTE_ADDR readback ("Error during raw + * reset. MMU_DTE_ADDR is not functioning"), the attach then fails, and the core + * leaves the master attached to no domain at all. + * + * The hardware does not need any of that to change page tables. On an already + * enabled MMU it is: + * + * enable_stall -> write DTE_ADDR on every bank -> ZAP_CACHE -> disable_stall + * + * with paging left on throughout. No reset, no paging off/on. + * + * Because the core is not involved, iommu_get_domain_for_dev() does NOT track + * this switch; a caller using it must track the live domain itself. The caller + * is also responsible for ensuring no DMA is in flight across the swap. + */ +int rk_iommu_switch_domain(struct device *dev, struct iommu_domain *domain) +{ + struct rk_iommu *iommu = rk_iommu_from_dev(dev); + struct rk_iommu_domain *rk_domain; + unsigned long flags; + int ret, i; + + if (!iommu || !domain) + return -ENODEV; + + rk_domain = to_rk_domain(domain); + + /* the third-party ops wrapper owns its own attach path */ + if (rk_domain->opt_ops) + return -EOPNOTSUPP; + + if (iommu->domain == domain) + return 0; + + /* + * Move this iommu between the two domains' iommus lists: rk_iommu_zap_iova() + * and the TLB flush paths walk that list, so it has to name the live domain. + */ + if (iommu->domain) { + struct rk_iommu_domain *old = to_rk_domain(iommu->domain); + + spin_lock_irqsave(&old->iommus_lock, flags); + list_del_init(&iommu->node); + spin_unlock_irqrestore(&old->iommus_lock, flags); + } + iommu->domain = domain; + spin_lock_irqsave(&rk_domain->iommus_lock, flags); + list_add_tail(&iommu->node, &rk_domain->iommus); + spin_unlock_irqrestore(&rk_domain->iommus_lock, flags); + rk_domain->shootdown_entire = iommu->shootdown_entire; + + ret = pm_runtime_get_if_in_use(iommu->dev); + if (!ret || WARN_ON_ONCE(ret < 0)) { + /* not runtime-active: rk_iommu_resume() programs DTE_ADDR from iommu->domain */ + return 0; + } + + ret = clk_bulk_enable(iommu->num_clocks, iommu->clocks); + if (ret) + goto out_pm_put; + + ret = rk_iommu_enable_stall(iommu); + if (ret) + goto out_disable_clocks; + + for (i = 0; i < iommu->num_mmu; i++) { + rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR, + rk_ops->mk_dtentries(rk_domain->dt_dma)); + rk_iommu_base_command(iommu->bases[i], RK_MMU_CMD_ZAP_CACHE); + } + + rk_iommu_disable_stall(iommu); + +out_disable_clocks: + clk_bulk_disable(iommu->num_clocks, iommu->clocks); +out_pm_put: + pm_runtime_put(iommu->dev); + return ret; +} +EXPORT_SYMBOL(rk_iommu_switch_domain); + +/* + * Re-establish the MMU for the currently attached domain, unconditionally. + * + * A hardware soft reset wipes the MMU (DTE_ADDR, paging), so the page table has + * to be reprogrammed afterwards. rk_iommu_switch_domain() deliberately + * short-circuits when the domain has not changed, which is exactly wrong here: + * the domain is the same, the hardware is not. + * + * iommu->domain is the live domain, so reprogramming from it is correct by + * construction and does not depend on what the IOMMU core believes. + */ +int rk_iommu_reprogram(struct device *dev) +{ + struct rk_iommu *iommu = rk_iommu_from_dev(dev); + int ret; + + if (!iommu || !iommu->domain) + return -ENODEV; + if (to_rk_domain(iommu->domain)->opt_ops) + return -EOPNOTSUPP; + + ret = pm_runtime_get_if_in_use(iommu->dev); + if (!ret || WARN_ON_ONCE(ret < 0)) + return 0; /* not runtime-active: rk_iommu_resume() will program it */ + + ret = rk_iommu_enable(iommu); + pm_runtime_put(iommu->dev); + return ret; +} +EXPORT_SYMBOL(rk_iommu_reprogram); + static int rk_iommu_attach_device(struct iommu_domain *domain, struct device *dev) { diff --git a/drivers/rknpu/include/rknpu_ioctl.h b/drivers/rknpu/include/rknpu_ioctl.h index f4e3b83bfc5af..ed06ee03e64c2 100644 --- a/drivers/rknpu/include/rknpu_ioctl.h +++ b/drivers/rknpu/include/rknpu_ioctl.h @@ -122,6 +122,16 @@ enum e_rknpu_action { RKNPU_GET_BW_TW = 11, RKNPU_SET_BW_TW = 12, RKNPU_ACT_CLR_TOTAL_RW_AMOUNT = 13, + /* + * Make an IOMMU domain live, with no allocation. `value` is the domain id. + * + * A dma-buf import maps its sg during PRIME_FD_TO_HANDLE, into whichever + * domain is live at that instant, because the ioctl carries no domain. The + * target is named only by the later MEM_CREATE, which does not re-map an + * already-imported handle. Userspace issues this first so the mapping lands + * in the domain the buffer will actually be used in. + */ + RKNPU_ACT_SET_DOMAIN = 0x100, RKNPU_GET_DT_WR_AMOUNT = 14, RKNPU_GET_DT_RD_AMOUNT = 15, RKNPU_GET_WT_RD_AMOUNT = 16, diff --git a/drivers/rknpu/include/rknpu_iommu.h b/drivers/rknpu/include/rknpu_iommu.h index 225176650b605..c336600a717b4 100644 --- a/drivers/rknpu/include/rknpu_iommu.h +++ b/drivers/rknpu/include/rknpu_iommu.h @@ -53,6 +53,30 @@ int rknpu_iommu_domain_get_and_switch(struct rknpu_device *rknpu_dev, int domain_id); int rknpu_iommu_domain_put(struct rknpu_device *rknpu_dev); +/* + * The domain the NPU is actually running in, from the driver's own state. + * + * iommu_get_domain_for_dev() returns what the IOMMU core last attached, which is + * only the same thing while domain switching goes through the core. Everything in + * this driver that must target the live domain uses this instead. + */ +struct iommu_domain *rknpu_iommu_live_domain(struct device *dev); + +/* + * Provided by drivers/iommu/rockchip-iommu.c. Switching with these keeps the IOMMU + * core out of the path, which is what avoids the rk_iommu_force_reset() that an + * iommu_attach_device() would perform on every domain change. + */ +extern int rk_iommu_switch_domain(struct device *dev, struct iommu_domain *domain); +extern int rk_iommu_reprogram(struct device *dev); + +/* + * Scoped override of the core's default domain, for the dma-buf attachment map and + * its matching unmap only. Returns the previous value; the caller restores it. + */ +struct iommu_domain *rknpu_iommu_default_swap(struct device *dev, + struct iommu_domain *dom); + #if KERNEL_VERSION(5, 10, 0) < LINUX_VERSION_CODE int iommu_get_dma_cookie(struct iommu_domain *domain); #endif diff --git a/drivers/rknpu/include/rknpu_job.h b/drivers/rknpu/include/rknpu_job.h index b4d40d96ffce4..86acb0540bf7e 100644 --- a/drivers/rknpu/include/rknpu_job.h +++ b/drivers/rknpu/include/rknpu_job.h @@ -32,6 +32,17 @@ struct rknpu_job { struct work_struct cleanup_work; bool irq_entry[RKNPU_MAX_CORES]; unsigned int flags; + /* + * Does this job currently hold the IOMMU domain reference? + * + * The reference is acquired exactly once, in rknpu_job_commit(), but three + * teardown paths release it -- the completion path, rknpu_job_abort() and + * rknpu_job_timeout_clean() -- with nothing recording whether this particular + * job still holds one. Its own word rather than a bit in ->flags because the + * releases run from both interrupt and process context, so the test-and-clear + * has to be atomic. + */ + unsigned long dom_held; int ret; struct rknpu_submit *args; bool args_owner; diff --git a/drivers/rknpu/rknpu_drv.c b/drivers/rknpu/rknpu_drv.c index 9e6f1d28a55c2..b90afa8f83758 100644 --- a/drivers/rknpu/rknpu_drv.c +++ b/drivers/rknpu/rknpu_drv.c @@ -466,6 +466,14 @@ static int rknpu_action(struct rknpu_device *rknpu_dev, case RKNPU_SET_BW_TW: ret = rknpu_set_bw_priority(rknpu_dev, 0, 0, args->value); break; + case RKNPU_ACT_SET_DOMAIN: { + int _id = (int)args->value; + + ret = rknpu_iommu_domain_get_and_switch(rknpu_dev, _id); + if (!ret) + rknpu_iommu_domain_put(rknpu_dev); /* switch only, hold no reference */ + break; + } case RKNPU_ACT_CLR_TOTAL_RW_AMOUNT: ret = rknpu_clear_rw_amount(rknpu_dev); break; diff --git a/drivers/rknpu/rknpu_gem.c b/drivers/rknpu/rknpu_gem.c index f1346f98fff62..0d51afbb4f5e7 100644 --- a/drivers/rknpu/rknpu_gem.c +++ b/drivers/rknpu/rknpu_gem.c @@ -72,7 +72,7 @@ static int rknpu_gem_get_pages(struct rknpu_gem_object *rknpu_obj) rknpu_obj->size); goto free_sgt; } - iommu_flush_iotlb_all(iommu_get_domain_for_dev(drm->dev)); + iommu_flush_iotlb_all(rknpu_iommu_live_domain(drm->dev)); if (rknpu_obj->flags & RKNPU_MEM_KERNEL_MAPPING) { rknpu_obj->cookie = vmap(rknpu_obj->pages, rknpu_obj->num_pages, @@ -501,7 +501,7 @@ static int rknpu_gem_alloc_buf_with_cache(struct rknpu_gem_object *rknpu_obj, } /* iova map to cache */ - domain = iommu_get_domain_for_dev(rknpu_dev->dev); + domain = rknpu_iommu_live_domain(rknpu_dev->dev); if (!domain) { LOG_ERROR("failed to get iommu domain!"); return -EINVAL; @@ -654,7 +654,7 @@ static void rknpu_gem_free_buf_with_cache(struct rknpu_gem_object *rknpu_obj, return; } - domain = iommu_get_domain_for_dev(rknpu_dev->dev); + domain = rknpu_iommu_live_domain(rknpu_dev->dev); if (domain) { iommu_unmap(domain, rknpu_obj->iova_start, cache_size); if (rknpu_obj->size > 0) @@ -850,7 +850,22 @@ void rknpu_gem_object_destroy(struct rknpu_gem_object *rknpu_obj) * once dmabuf's refcount becomes 0. */ if (obj->import_attach) { - drm_prime_gem_destroy(obj, rknpu_obj->sgt); + { + /* + * dma_buf_unmap_attachment() unmaps through the DMA API + * too, so it must see the same default domain the map saw. + * The caller has already switched to this object's recorded + * domain, so the live domain is the right one. + */ + struct iommu_domain *l = rknpu_dev->iommu_en ? + rknpu_iommu_live_domain(rknpu_dev->dev) : NULL; + struct iommu_domain *sv = l ? + rknpu_iommu_default_swap(rknpu_dev->dev, l) : NULL; + + drm_prime_gem_destroy(obj, rknpu_obj->sgt); + if (sv) + rknpu_iommu_default_swap(rknpu_dev->dev, sv); + } rknpu_gem_free_page(rknpu_obj->pages); } else { if (IS_ENABLED(CONFIG_ROCKCHIP_RKNPU_SRAM) && @@ -1398,7 +1413,30 @@ int rknpu_gem_mmap(struct file *filp, struct vm_area_struct *vma) struct drm_gem_object *rknpu_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf) { - return drm_gem_prime_import_dev(dev, dma_buf, dev->dev); + struct rknpu_device *rknpu_dev = dev->dev_private; + struct drm_gem_object *obj; + struct iommu_domain *live, *saved = NULL; + + if (!rknpu_dev->iommu_en) + return drm_gem_prime_import_dev(dev, dma_buf, dev->dev); + + /* + * DRM core maps the attachment through the DMA API here, which always + * targets group->default_domain. Point that at the live domain for the + * duration of the import so the sg is mapped where the NPU will run, then + * restore it. domain_lock keeps a concurrent switch from moving the live + * domain underneath the map. + */ + mutex_lock(&rknpu_dev->domain_lock); + live = rknpu_iommu_live_domain(rknpu_dev->dev); + if (live) + saved = rknpu_iommu_default_swap(rknpu_dev->dev, live); + obj = drm_gem_prime_import_dev(dev, dma_buf, dev->dev); + if (saved) + rknpu_iommu_default_swap(rknpu_dev->dev, saved); + mutex_unlock(&rknpu_dev->domain_lock); + + return obj; } #endif diff --git a/drivers/rknpu/rknpu_iommu.c b/drivers/rknpu/rknpu_iommu.c index a3efb1ef02734..be3cdd57aee54 100644 --- a/drivers/rknpu/rknpu_iommu.c +++ b/drivers/rknpu/rknpu_iommu.c @@ -5,6 +5,8 @@ */ #include +#include +#include #include #include @@ -79,6 +81,19 @@ void rknpu_iommu_dma_free_iova(struct rknpu_iommu_dma_cookie *cookie, free_iova(iovad, iova_pfn(iovad, iova)); } +struct iommu_domain *rknpu_iommu_live_domain(struct device *dev) +{ + struct rknpu_device *rknpu_dev = dev_get_drvdata(dev); + int id; + + if (!rknpu_dev) + return NULL; + id = rknpu_dev->iommu_domain_id; + if (id < 0 || id >= RKNPU_MAX_IOMMU_DOMAIN_NUM) + return NULL; + return rknpu_dev->iommu_domains[id]; +} + static int rknpu_dma_info_to_prot(enum dma_data_direction dir, bool coherent) { int prot = coherent ? IOMMU_CACHE : 0; @@ -208,7 +223,7 @@ int rknpu_iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents, enum dma_data_direction dir, bool iova_aligned) { - struct iommu_domain *domain = iommu_get_domain_for_dev(dev); + struct iommu_domain *domain = rknpu_iommu_live_domain(dev); struct rknpu_iommu_dma_cookie *cookie = (void *)domain->iova_cookie; struct iova_domain *iovad = &cookie->iovad; struct scatterlist *s = NULL, *prev = NULL; @@ -298,7 +313,7 @@ void rknpu_iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents, enum dma_data_direction dir, bool iova_aligned) { - struct iommu_domain *domain = iommu_get_domain_for_dev(dev); + struct iommu_domain *domain = rknpu_iommu_live_domain(dev); struct rknpu_iommu_dma_cookie *cookie = (void *)domain->iova_cookie; struct iova_domain *iovad = &cookie->iovad; size_t iova_off = 0; @@ -403,6 +418,35 @@ struct iommu_group { }; #endif +/* + * Temporarily point the IOMMU core's default domain at the live domain. + * + * dma-iommu resolves every DMA-API mapping through + * iommu_get_dma_domain() == group->default_domain, and dma_buf_map_attachment(), + * which DRM core runs during PRIME_FD_TO_HANDLE, goes through the DMA API. There + * is no way to tell it which domain to use. + * + * This is deliberately not the permanent overwrite that used to live in + * rknpu_iommu_switch_domain(). That left the core's default pointing at a domain + * the driver might later free, for the lifetime of the device. This override lasts + * only for one attachment map or unmap, is taken under domain_lock so no switch can + * move the live domain underneath it, and is restored immediately. + * + * Returns the previous default so the caller can restore it. + */ +struct iommu_domain *rknpu_iommu_default_swap(struct device *dev, + struct iommu_domain *dom) +{ + struct rknpu_device *rknpu_dev = dev_get_drvdata(dev); + struct iommu_domain *old; + + if (!rknpu_dev || !rknpu_dev->iommu_group || !dom) + return NULL; + old = rknpu_dev->iommu_group->default_domain; + rknpu_dev->iommu_group->default_domain = dom; + return old; +} + int rknpu_iommu_init_domain(struct rknpu_device *rknpu_dev) { // init domain 0 @@ -443,29 +487,32 @@ int rknpu_iommu_switch_domain(struct rknpu_device *rknpu_dev, int domain_id) return 0; } - src_domain = iommu_get_domain_for_dev(rknpu_dev->dev); - if (src_domain != rknpu_dev->iommu_domains[src_domain_id]) { - LOG_DEV_ERROR( - rknpu_dev->dev, - "mismatch domain get from iommu_get_domain_for_dev\n"); - return -EINVAL; - } + /* + * The driver's own record is the source of truth for which domain is live. + * iommu_get_domain_for_dev() no longer tracks these switches, so comparing + * against it could only ever produce a spurious mismatch. + */ + src_domain = rknpu_dev->iommu_domains[src_domain_id]; dst_domain = rknpu_dev->iommu_domains[domain_id]; if (dst_domain != NULL) { - iommu_detach_device(src_domain, rknpu_dev->dev); - ret = iommu_attach_device(dst_domain, rknpu_dev->dev); + ret = rk_iommu_switch_domain(rknpu_dev->dev, dst_domain); if (ret) { LOG_DEV_ERROR( rknpu_dev->dev, - "failed to attach dst iommu domain, id: %d, ret: %d\n", + "failed to switch to iommu domain, id: %d, ret: %d\n", domain_id, ret); - if (iommu_attach_device(src_domain, rknpu_dev->dev)) { + /* + * Unlike the detach/attach pair this replaces, a failed + * switch leaves the previous page table live rather than + * leaving the master attached to nothing, so restoring is + * belt and braces. + */ + if (rk_iommu_switch_domain(rknpu_dev->dev, src_domain)) LOG_DEV_ERROR( rknpu_dev->dev, - "failed to reattach src iommu domain, id: %d\n", + "failed to restore src iommu domain, id: %d\n", src_domain_id); - } return ret; } rknpu_dev->iommu_domain_id = domain_id; @@ -478,32 +525,74 @@ int rknpu_iommu_switch_domain(struct rknpu_device *rknpu_dev, int domain_id) "failed to allocate iommu domain\n"); return -EIO; } - // init domain iova_cookie iommu_get_dma_cookie(dst_domain); - iommu_detach_device(src_domain, rknpu_dev->dev); - ret = iommu_attach_device(dst_domain, rknpu_dev->dev); + /* + * Initialise this domain's IOVA allocator directly. + * + * The previous route got one by pretending the domain was a DMA-API + * domain -- setting __IOMMU_DOMAIN_DMA_API on the type and calling + * iommu_setup_dma_ops(), whose only useful effect here was + * iommu_dma_init_domain() initialising the cookie's iovad. That + * depends on iommu_get_domain_for_dev() reporting this domain, i.e. + * on the default-domain overwrite removed below. Do the two init + * calls directly instead: same iovad, and the domain stays a plain + * unmanaged domain that iommu_map_sg() and iommu_unmap() serve. + * + * Granule and start_pfn mirror iommu_dma_init_domain(): 4 KiB pages, + * base_pfn 1 so IOVA 0 is never handed out. + */ + { + struct rknpu_iommu_dma_cookie *ck = + (struct rknpu_iommu_dma_cookie *)dst_domain->iova_cookie; + + if (!ck) { + LOG_DEV_ERROR(rknpu_dev->dev, + "no iova cookie for domain %d\n", + domain_id); + iommu_domain_free(dst_domain); + return -ENOMEM; + } + init_iova_domain(&ck->iovad, SZ_4K, 1); + if (iova_domain_init_rcaches(&ck->iovad)) { + LOG_DEV_ERROR(rknpu_dev->dev, + "failed to init iova rcaches, domain %d\n", + domain_id); + iommu_domain_free(dst_domain); + return -ENOMEM; + } + } + + ret = rk_iommu_switch_domain(rknpu_dev->dev, dst_domain); if (ret) { LOG_DEV_ERROR( rknpu_dev->dev, - "failed to attach iommu domain, id: %d, ret: %d\n", + "failed to switch to iommu domain, id: %d, ret: %d\n", domain_id, ret); iommu_domain_free(dst_domain); return ret; } - - // set domain type to dma domain - dst_domain->type |= __IOMMU_DOMAIN_DMA_API; - // iommu dma init domain - iommu_setup_dma_ops(rknpu_dev->dev, 0, dma_limit); + (void)dma_limit; rknpu_dev->iommu_domain_id = domain_id; rknpu_dev->iommu_domains[domain_id] = dst_domain; rknpu_dev->iommu_domain_num++; } - // reset default iommu domain - rknpu_dev->iommu_group->default_domain = dst_domain; + /* + * The default-domain overwrite that used to live here is gone. + * + * dma-iommu resolves every DMA-API mapping through + * iommu_get_dma_domain() == dev->iommu_group->default_domain, so the only + * way to make a DMA-API mapping land in domain N was to tell the core that + * N was the group's default -- reaching the field through a private copy of + * struct iommu_group. That leaves the core's idea of the default pointing at + * a domain this driver may later free, and any core path that reattaches + * "the default" then uses it. + * + * Nothing here needs it any more: the NPU's buffers are mapped explicitly + * with iommu_map_sg()/iommu_unmap() against the domain the driver names. + */ LOG_INFO("switch iommu domain from %d to %d\n", src_domain_id, domain_id); @@ -560,7 +649,25 @@ int rknpu_iommu_domain_get_and_switch(struct rknpu_device *rknpu_dev, int rknpu_iommu_domain_put(struct rknpu_device *rknpu_dev) { - atomic_dec(&rknpu_dev->iommu_domain_refcount); + /* + * Never let the reference count go negative. + * + * rknpu_iommu_domain_get_and_switch() proceeds only when this reads exactly + * zero, so a bare atomic_dec() turns a single unbalanced put into a permanent + * wedge: the count never reads zero again, every domain switch burns its full + * RKNPU_SWITCH_DOMAIN_WAIT_TIME_MS and fails, and because + * rknpu_gem_object_create() switches domains, every subsequent allocation then + * fails with -EINVAL until the machine is rebooted. + * + * Clamping turns an over-put into a bounded anomaly instead of a dead device. + * It treats the symptom, not the cause, so warn once per occurrence to keep any + * remaining imbalance visible. + */ + if (atomic_dec_return(&rknpu_dev->iommu_domain_refcount) < 0) { + atomic_set(&rknpu_dev->iommu_domain_refcount, 0); + LOG_DEV_ERROR(rknpu_dev->dev, + "iommu domain refcount underflow, clamped to 0\n"); + } return 0; } @@ -580,7 +687,17 @@ void rknpu_iommu_free_domains(struct rknpu_device *rknpu_dev) if (domain == NULL) continue; - iommu_detach_device(domain, rknpu_dev->dev); + /* + * No iommu_detach_device(): the core never attached these domains. + * The switch to domain 0 above already moved the hardware off this + * page table. Release the IOVA allocator initialised in the switch. + */ + if (domain->iova_cookie) { + struct rknpu_iommu_dma_cookie *ck = + (struct rknpu_iommu_dma_cookie *)domain->iova_cookie; + + put_iova_domain(&ck->iovad); + } iommu_domain_free(domain); rknpu_dev->iommu_domains[i] = NULL; diff --git a/drivers/rknpu/rknpu_job.c b/drivers/rknpu/rknpu_job.c index 57d67d81f7d5e..fc0d245802bc2 100644 --- a/drivers/rknpu/rknpu_job.c +++ b/drivers/rknpu/rknpu_job.c @@ -101,8 +101,23 @@ static void rknpu_job_free(struct rknpu_job *job) rknpu_gem_object_put(&task_obj->base); #endif - if (job->fence) + if (job->fence) { + /* + * A job torn down without completing (rknpu_job_timeout_clean() or + * rknpu_job_abort()) never reaches the RKNPU_JOB_DONE path, so its fence + * is never signalled and every waiter blocks until its own timeout + * expires -- which defeats the point of waiting on a fence to notice + * that a job has failed. Signal it with an error instead, so waiters + * wake immediately and can distinguish failure from completion via + * dma_fence_get_status(). No-op on the success path, where the + * completion interrupt has already signalled it. + */ + if (!dma_fence_is_signaled(job->fence)) { + dma_fence_set_error(job->fence, -ETIMEDOUT); + dma_fence_signal(job->fence); + } dma_fence_put(job->fence); + } if (job->args_owner) kfree(job->args); @@ -485,7 +500,8 @@ static void rknpu_job_done(struct rknpu_job *job, int ret, int core_index) if (atomic_dec_and_test(&job->interrupt_count)) { int use_core_num = job->use_core_num; - rknpu_iommu_domain_put(rknpu_dev); + if (test_and_clear_bit(0, &job->dom_held)) + rknpu_iommu_domain_put(rknpu_dev); job->flags |= RKNPU_JOB_DONE; job->ret = ret; @@ -541,6 +557,7 @@ static void rknpu_job_schedule(struct rknpu_job *job) job->ret = -EINVAL; return; } + set_bit(0, &job->dom_held); spin_lock_irqsave(&rknpu_dev->irq_lock, flags); for (i = 0; i < rknpu_dev->config->num_irqs; i++) { @@ -565,7 +582,8 @@ static void rknpu_job_abort(struct rknpu_job *job) unsigned long flags; int i = 0; - rknpu_iommu_domain_put(rknpu_dev); + if (test_and_clear_bit(0, &job->dom_held)) + rknpu_iommu_domain_put(rknpu_dev); msleep(100); @@ -646,6 +664,27 @@ static inline irqreturn_t rknpu_irq_handler(int irq, void *data, int core_index) uint32_t status = 0; unsigned long flags; + /* + * Never touch NPU registers while the block is powered down. + * + * Both paths below access registers unconditionally: the no-job path writes + * RKNPU_OFFSET_INT_CLEAR and the normal path reads RKNPU_OFFSET_INT_STATUS. + * rknpu_power_off() is driven by a deferred work item, so a late or spurious + * interrupt can arrive after power has gone. The register access then takes an + * external abort and the machine dies immediately, with no console output and + * no way back but a power cycle: + * + * pc : readl+0x4/0x20 + * lr : rknpu_irq_handler.isra.0+0x94/0x2f0 + * Call trace: readl / rknpu_core0_irq_handler / __handle_irq_event_percpu + * + * power_refcount is an atomic, so unlike power_lock (a mutex) it is safe to read + * from hard IRQ context. If the device is unpowered it cannot be asserting an + * interrupt, so IRQ_NONE is correct and cannot cause a level-IRQ storm. + */ + if (atomic_read(&rknpu_dev->power_refcount) <= 0) + return IRQ_NONE; + subcore_data = &rknpu_dev->subcore_datas[core_index]; spin_lock_irqsave(&rknpu_dev->irq_lock, flags); @@ -718,6 +757,17 @@ static void rknpu_job_timeout_clean(struct rknpu_device *rknpu_dev, spin_unlock_irqrestore(&rknpu_dev->irq_lock, flags); + /* + * Release the domain reference this job still + * holds. The completion and abort paths both do + * this, but reaping a timed-out job here did + * not, so the reference was leaked and the + * device-wide count never returned to zero -- + * after which no domain switch can ever succeed. + */ + if (test_and_clear_bit(0, &job->dom_held)) + rknpu_iommu_domain_put(rknpu_dev); + do { schedule_work(&job->cleanup_work); diff --git a/drivers/rknpu/rknpu_reset.c b/drivers/rknpu/rknpu_reset.c index 65142368c9fd8..2974617151ed8 100644 --- a/drivers/rknpu/rknpu_reset.c +++ b/drivers/rknpu/rknpu_reset.c @@ -8,6 +8,7 @@ #include #include "rknpu_reset.h" +#include "rknpu_iommu.h" #ifndef FPGA_PLATFORM static inline struct reset_control *rknpu_reset_control_get(struct device *dev, @@ -138,13 +139,24 @@ int rknpu_soft_reset(struct rknpu_device *rknpu_dev) return ret; } - if (rknpu_dev->iommu_en) - domain = iommu_get_domain_for_dev(rknpu_dev->dev); - - if (domain) { - iommu_detach_device(domain, rknpu_dev->dev); - iommu_attach_device(domain, rknpu_dev->dev); + /* + * A soft reset wipes the MMU, so the page table has to be reprogrammed. This + * used to be a detach/attach of iommu_get_domain_for_dev(), which is the + * core's default domain -- correct only while the driver overwrote that to + * follow its switches. It no longer does, so re-establish the live domain + * explicitly instead; otherwise the reset silently leaves the NPU pointed at + * domain 0 while the driver believes domain N is live, and the next job is + * committed against a page table where its IOVAs do not exist. + */ + if (rknpu_dev->iommu_en) { + int rp = rk_iommu_reprogram(rknpu_dev->dev); + + if (rp) + LOG_DEV_ERROR(rknpu_dev->dev, + "failed to reprogram iommu after reset: %d\n", + rp); } + (void)domain; rknpu_dev->soft_reseting = false;