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_iommu.c b/drivers/rknpu/rknpu_iommu.c index a3efb1ef02734..e9e9f3652d302 100644 --- a/drivers/rknpu/rknpu_iommu.c +++ b/drivers/rknpu/rknpu_iommu.c @@ -560,7 +560,25 @@ int rknpu_iommu_domain_get_and_switch(struct rknpu_device *rknpu_dev, int rknpu_iommu_domain_put(struct rknpu_device *rknpu_dev) { - atomic_dec(&rknpu_dev->iommu_domain_refcount); + /* + * Never let the reference count go negative. + * + * rknpu_iommu_domain_get_and_switch() proceeds only when this reads exactly + * zero, so a bare atomic_dec() turns a single unbalanced put into a permanent + * wedge: the count never reads zero again, every domain switch burns its full + * RKNPU_SWITCH_DOMAIN_WAIT_TIME_MS and fails, and because + * rknpu_gem_object_create() switches domains, every subsequent allocation then + * fails with -EINVAL until the machine is rebooted. + * + * Clamping turns an over-put into a bounded anomaly instead of a dead device. + * It treats the symptom, not the cause, so warn once per occurrence to keep any + * remaining imbalance visible. + */ + if (atomic_dec_return(&rknpu_dev->iommu_domain_refcount) < 0) { + atomic_set(&rknpu_dev->iommu_domain_refcount, 0); + LOG_DEV_ERROR(rknpu_dev->dev, + "iommu domain refcount underflow, clamped to 0\n"); + } return 0; } 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);