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

[PATCH v2] 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.

Cc: <stable@vger.kernel.org> # v7.1+
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>
---
Changes in v2:
 - add Cc: tag
 - link to v1: https://lore.kernel.org/dri-devel/20260914082800.892044-1-karol.wachowski@linux.intel.com/T/#u
---
 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 4c73dc8b3aaf..d7faabe0f676 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -539,6 +539,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);
@@ -734,7 +735,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 3b26a64ed04f..8762b8969d96 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 51d573ac4da8..245512bab523 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;
 	}
@@ -1271,10 +1270,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 1908375ff128..c846499d96ce 100644
--- a/drivers/accel/ivpu/ivpu_mmu.c
+++ b/drivers/accel/ivpu/ivpu_mmu.c
@@ -960,7 +960,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 v2] accel/ivpu: Use separate flag for job timeout

[sashiko-bot]

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

Pre-existing issues:
- [High] Lockless `xa_load` without RCU read lock and RCU-deferred freeing leads to a potential Use-After-Free of `file_priv`.
--

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

accel/ivpu: Use separate flag for job timeout
    
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.

> 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 is it possible for file_priv to be freed
concurrently while it is being accessed here?

xa_load() internally acquires and releases rcu_read_lock(), so the returned
file_priv pointer is unprotected once xa_load() returns. 

If a userspace application triggers an MMU fault and immediately closes the
file descriptor, could ivpu_postclose() drop the last reference and invoke
file_priv_release() before the hardware event queue is processed?

If file_priv_release() erases the entry from the xarray via xa_erase_irq()
and frees it synchronously via kfree(), does this lead to a use-after-free
when ivpu_mmu_irq_evtq_handler() subsequently accesses 
file_priv->has_mmu_faults?

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

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

[Wachowski, Karol]

On 14-Sep-26 10:58, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Lockless `xa_load` without RCU read lock and RCU-deferred freeing leads to a potential Use-After-Free of `file_priv`.
> --
> 
> commit 267f0542c58f023dad2c373ec173ea2fa96d1b24
> Author: Jakub Pawlak <jakub.pawlak@intel.com>
> 
> accel/ivpu: Use separate flag for job timeout
>      
> 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.
> 
>> 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 is it possible for file_priv to be freed
> concurrently while it is being accessed here?
> 
> xa_load() internally acquires and releases rcu_read_lock(), so the returned
> file_priv pointer is unprotected once xa_load() returns.
> 
> If a userspace application triggers an MMU fault and immediately closes the
> file descriptor, could ivpu_postclose() drop the last reference and invoke
> file_priv_release() before the hardware event queue is processed?
> 
> If file_priv_release() erases the entry from the xarray via xa_erase_irq()
> and frees it synchronously via kfree(), does this lead to a use-after-free
> when ivpu_mmu_irq_evtq_handler() subsequently accesses
> file_priv->has_mmu_faults?
> 

Thanks for the review.

Right, the race is real: xa_load() returns file_priv without
any protection, and the entry can be erased and the object freed before
has_mmu_faults is updated.

However, this is pre-existing and orthogonal to this patch: v2 only
removes atomic_set(&vdev->faults_detected, 1) from that block, so it
neither introduces nor worsens the race.

I will address it in a separate patch, with a proper Fixes: tag and
Cc: stable@vger.kernel.

At the same time I would like to proceed with this patch.

Thanks,
Karol

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

[Dawid Osuchowski]

On 2026-09-14 10:43 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.
> 
> Cc: <stable@vger.kernel.org> # v7.1+
> Fixes: ade00a6c903f ("accel/ivpu: Perform engine reset instead of device recovery on TDR")

Reviewed-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>

> Signed-off-by: Jakub Pawlak <jakub.pawlak@intel.com>
> Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
> ---
> Changes in v2:
>   - add Cc: tag
>   - link to v1: https://lore.kernel.org/dri-devel/20260914082800.892044-1-karol.wachowski@linux.intel.com/T/#u
> ---
>   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(-)

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

[Wachowski, Karol]

On 21-Sep-26 16:56, Dawid Osuchowski wrote:
> On 2026-09-14 10:43 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.
>>
>> Cc: <stable@vger.kernel.org> # v7.1+
>> Fixes: ade00a6c903f ("accel/ivpu: Perform engine reset instead of 
>> device recovery on TDR")
> 
> Reviewed-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>

Thanks, applied to drm-misc-fixes.

> 
>> Signed-off-by: Jakub Pawlak <jakub.pawlak@intel.com>
>> Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
>> ---
>> Changes in v2:
>>   - add Cc: tag
>>   - link to v1: https://lore.kernel.org/dri- 
>> devel/20260914082800.892044-1-karol.wachowski@linux.intel.com/T/#u
>> ---
>>   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(-)
>