[PATCH] accel/ivpu: Use separate flag for job timeout

[PATCH] accel/ivpu: Use separate flag for job timeout

[Karol Wachowski]

From: Jakub Pawlak <jakub.pawlak@intel.com>

Use separate flag to mark a job timeout as a reason
of starting context_abort_work. This allows to distinguish
engine reset reason and clearly adjust reset procedure flow.

The flag is cleared in ivpu_prepare_for_reset(), which every
recovery and suspend path already funnels through, so that the
state is clean after recovery.

Fixes: ade00a6c903f ("accel/ivpu: Perform engine reset instead of device recovery on TDR")
Signed-off-by: Jakub Pawlak <jakub.pawlak@intel.com>
Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
---
 drivers/accel/ivpu/ivpu_drv.c | 3 ++-
 drivers/accel/ivpu/ivpu_drv.h | 2 +-
 drivers/accel/ivpu/ivpu_job.c | 7 +++----
 drivers/accel/ivpu/ivpu_mmu.c | 1 -
 drivers/accel/ivpu/ivpu_pm.c  | 1 +
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index 0f215392d42d..647684219e9d 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -528,6 +528,7 @@ void ivpu_prepare_for_reset(struct ivpu_device *vdev)
 {
 	ivpu_hw_irq_disable(vdev);
 	disable_irq(vdev->irq);
+	atomic_set(&vdev->job_timeout_detected, 0);
 	flush_work(&vdev->irq_dct_work);
 	flush_work(&vdev->context_abort_work);
 	flush_work(&vdev->job_destroy_work);
@@ -723,7 +724,7 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
 	vdev->context_xa_limit.max = IVPU_USER_CONTEXT_MAX_SSID;
 	atomic64_set(&vdev->unique_id_counter, 0);
 	atomic_set(&vdev->job_timeout_counter, 0);
-	atomic_set(&vdev->faults_detected, 0);
+	atomic_set(&vdev->job_timeout_detected, 0);
 	xa_init_flags(&vdev->context_xa, XA_FLAGS_ALLOC | XA_FLAGS_LOCK_IRQ);
 	xa_init_flags(&vdev->submitted_jobs_xa, XA_FLAGS_ALLOC1);
 	xa_init_flags(&vdev->db_xa, XA_FLAGS_ALLOC1);
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index 8f29f1dc52b9..37d5a6ac55f8 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -171,7 +171,7 @@ struct ivpu_device {
 	struct xarray submitted_jobs_xa;
 	struct ivpu_ipc_consumer job_done_consumer;
 	atomic_t job_timeout_counter;
-	atomic_t faults_detected;
+	atomic_t job_timeout_detected;
 
 	atomic64_t unique_id_counter;
 
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index b3de5dd29d1e..084d825f744d 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -626,7 +626,6 @@ bool ivpu_job_handle_engine_error(struct ivpu_device *vdev, u32 job_id, u32 job_
 		 * status and ensure both are handled in the same way
 		 */
 		job->file_priv->has_mmu_faults = true;
-		atomic_set(&vdev->faults_detected, 1);
 		queue_work(system_percpu_wq, &vdev->context_abort_work);
 		return true;
 	}
@@ -1260,10 +1259,10 @@ static int reset_engine_and_mark_faulty_contexts(struct ivpu_device *vdev)
 		return ret;
 
 	/*
-	 * If faults are detected, ignore guilty contexts from engine reset as NPU may not be stuck
-	 * and could return currently running good context and faulty contexts are already marked
+	 * If job timeout is detected, read guilty context from engine reset, for other reasons
+	 * faulty context is already known
 	 */
-	if (atomic_cmpxchg(&vdev->faults_detected, 1, 0) == 1)
+	if (atomic_cmpxchg(&vdev->job_timeout_detected, 1, 0) == 0)
 		return 0;
 
 	num_impacted_contexts = resp.payload.engine_reset_done.num_impacted_contexts;
diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
index 41efd8985fa6..b2025274f91d 100644
--- a/drivers/accel/ivpu/ivpu_mmu.c
+++ b/drivers/accel/ivpu/ivpu_mmu.c
@@ -964,7 +964,6 @@ void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
 		file_priv = xa_load(&vdev->context_xa, ssid);
 		if (file_priv) {
 			if (!READ_ONCE(file_priv->has_mmu_faults)) {
-				atomic_set(&vdev->faults_detected, 1);
 				ivpu_mmu_dump_event(vdev, event);
 				WRITE_ONCE(file_priv->has_mmu_faults, true);
 			}
diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
index c1ce8329790e..de0becbfdffb 100644
--- a/drivers/accel/ivpu/ivpu_pm.c
+++ b/drivers/accel/ivpu/ivpu_pm.c
@@ -229,6 +229,7 @@ static void ivpu_job_timeout_work(struct work_struct *work)
 
 	ivpu_jsm_state_dump(vdev);
 	ivpu_dev_coredump(vdev);
+	atomic_set(&vdev->job_timeout_detected, 1);
 	queue_work(system_percpu_wq, &vdev->context_abort_work);
 }
 
-- 
2.43.0

Re: [PATCH] accel/ivpu: Use separate flag for job timeout

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Premature clearing of `job_timeout_detected` flag before flushing `context_abort_work` silently drops the ban on the guilty context during a job timeout (TDR).

Pre-existing issues:
- [High] Lockless `xa_load` of `file_priv` in the MMU event queue IRQ handler leads to a Use-After-Free.
--

commit b8245a180aec634640c11be76ef2b20b2a900039
Author: Jakub Pawlak <jakub.pawlak@intel.com>

accel/ivpu: Use separate flag for job timeout

This patch introduces a separate flag, job_timeout_detected, to distinguish
engine resets caused by job timeouts from other reasons. The flag is
cleared in ivpu_prepare_for_reset() to ensure a clean state after recovery.

> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index 0f215392d42de..647684219e9d3 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
> @@ -528,6 +528,7 @@ void ivpu_prepare_for_reset(struct ivpu_device *vdev)
>  {
>  	ivpu_hw_irq_disable(vdev);
>  	disable_irq(vdev->irq);
> +	atomic_set(&vdev->job_timeout_detected, 0);
>  	flush_work(&vdev->irq_dct_work);
>  	flush_work(&vdev->context_abort_work);

[Severity: High]
Is it safe to clear the job_timeout_detected flag before flushing
context_abort_work?

If a concurrent recovery calls ivpu_prepare_for_reset() and sets the flag
to 0, and context_abort_work runs simultaneously, could it reach the
atomic_cmpxchg() in reset_engine_and_mark_faulty_contexts(), see a 0, and
silently skip banning the guilty context? 

Should this flag be cleared after flushing the work queues instead?

[ ... ]

> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index b3de5dd29d1e8..084d825f744dd 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
[ ... ]
> @@ -1260,10 +1259,10 @@ static int reset_engine_and_mark_faulty_contexts(struct ivpu_device *vdev)
>  		return ret;
>  
>  	/*
> -	 * If faults are detected, ignore guilty contexts from engine reset as NPU may not be stuck
> -	 * and could return currently running good context and faulty contexts are already marked
> +	 * If job timeout is detected, read guilty context from engine reset, for other reasons
> +	 * faulty context is already known
>  	 */
> -	if (atomic_cmpxchg(&vdev->faults_detected, 1, 0) == 1)
> +	if (atomic_cmpxchg(&vdev->job_timeout_detected, 1, 0) == 0)
>  		return 0;
>  
>  	num_impacted_contexts = resp.payload.engine_reset_done.num_impacted_contexts;

[ ... ]

> diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c
> index 41efd8985fa67..b2025274f91de 100644
> --- a/drivers/accel/ivpu/ivpu_mmu.c
> +++ b/drivers/accel/ivpu/ivpu_mmu.c
> @@ -964,7 +964,6 @@ void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev)
>  		file_priv = xa_load(&vdev->context_xa, ssid);
>  		if (file_priv) {
>  			if (!READ_ONCE(file_priv->has_mmu_faults)) {
> -				atomic_set(&vdev->faults_detected, 1);
>  				ivpu_mmu_dump_event(vdev, event);
>  				WRITE_ONCE(file_priv->has_mmu_faults, true);
>  			}

[Severity: High]
This is a pre-existing issue, but does the lockless xa_load() here risk a
use-after-free?

Looking at file_priv_release() in drivers/accel/ivpu/ivpu_drv.c, file_priv
is freed immediately via kfree() without waiting for an RCU grace period
after being removed from the xarray:

drivers/accel/ivpu/ivpu_drv.c:file_priv_release() {
    ...
    ivpu_user_limits_put(vdev, file_priv->user_limits);
    mutex_destroy(&file_priv->ms_lock);
    mutex_destroy(&file_priv->lock);
    kfree(file_priv);
}

If a user closes the file descriptor concurrently with this IRQ handler,
could file_priv be freed while we are still accessing
file_priv->has_mmu_faults via READ_ONCE() or WRITE_ONCE()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914082800.892044-1-karol.wachowski@linux.intel.com?part=1

Re: [PATCH] accel/ivpu: Use separate flag for job timeout

[Jeff Hugo]

On 9/14/2026 2:28 AM, Karol Wachowski wrote:
> From: Jakub Pawlak <jakub.pawlak@intel.com>
> 
> Use separate flag to mark a job timeout as a reason
> of starting context_abort_work. This allows to distinguish
> engine reset reason and clearly adjust reset procedure flow.
> 
> The flag is cleared in ivpu_prepare_for_reset(), which every
> recovery and suspend path already funnels through, so that the
> state is clean after recovery.
> 
> Fixes: ade00a6c903f ("accel/ivpu: Perform engine reset instead of device recovery on TDR")
> Signed-off-by: Jakub Pawlak <jakub.pawlak@intel.com>
> Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>

Reviewed-by: Jeff Hugo <jeff.hugo@oss.qualcomm.com>