* [PATCH] accel/ivpu: Disable recovery work on probe failure
@ 2026-11-07 17:42 Daksh Sahni
2026-11-07 18:00 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Daksh Sahni @ 2026-11-07 17:42 UTC (permalink / raw)
To: karol.wachowski, andrzej.kacprowski, ogabbay
Cc: jacek.lawrynowicz, jeff.hugo, dri-devel, linux-kernel,
Daksh Sahni
If the NPU raises an error interrupt while ivpu_dev_init() boots the
firmware, the Buttress, MMU or WDT IRQ handler calls
ivpu_pm_trigger_recovery(), which queues recovery_work. When ivpu_boot()
then fails, the err_ipc_fini path tears the device down without
disabling recovery_work and probe returns an error. devres unmaps the
register BARs while the work is still queued or running, and the work
faults on the first register access:
BUG: unable to handle page fault for address: ffffcfc6680200f4
Workqueue: events_unbound ivpu_pm_recovery_work [intel_vpu]
RIP: 0010:ivpu_hw_ip_ipc_rx_addr_get+0xc/0xb0 [intel_vpu]
Call Trace:
ivpu_wait_for_ready+0x155/0x220 [intel_vpu]
ivpu_boot+0x9e/0x270 [intel_vpu]
ivpu_resume+0xa4/0xd0 [intel_vpu]
ivpu_pm_reset_complete+0x2c/0x90 [intel_vpu]
ivpu_pm_recovery_work+0xf6/0x140 [intel_vpu]
process_one_work+0x1da/0x3c0
worker_thread+0x1af/0x340
kthread+0xfa/0x130
ret_from_fork+0x2b9/0x380
ret_from_fork_asm+0x1a/0x30
The work dies between pm_runtime_disable() in ivpu_pm_reset_begin() and
pm_runtime_enable() in ivpu_pm_reset_complete(), so the PCI device is
also left with runtime PM disabled and never autosuspends after a later
rebind.
Commit 5eaa49741119 ("accel/ivpu: Prevent recovery invocation during
probe and resume") stopped IPC failures during probe from triggering
recovery, but the IRQ handlers still can.
Call ivpu_pm_disable_recovery() at the start of the err_ipc_fini path,
as ivpu_dev_fini() already does, so the work is flushed and cannot be
queued again before the device is torn down.
Reproduced on Meteor Lake (Core Ultra 5 125H, vpu_37xx firmware
20260219) by kexec'ing with the NPU runtime-suspended: probe in the new
kernel gets an ATS_ERR Buttress IRQ during firmware boot. With this
change probe still fails with -ECANCELED, there is no oops, and an FLR
followed by a rebind brings the NPU back.
Fixes: 852be13f3bd3 ("accel/ivpu: Add PM support")
Cc: stable@vger.kernel.org
Signed-off-by: Daksh Sahni <swizzguy76@gmail.com>
---
Notes:
Tested on v7.4-rc1 (d48723711bae).
Two things this patch does not address:
The trigger is kexec with the NPU runtime-suspended in the old kernel.
The driver has no .shutdown hook, so the new kernel's probe sees stale
NPU state and gets ATS_ERR. A cold boot doesn't hit it. Whether that
wants a shutdown hook or a reset at probe seems like your call.
In one of four runs that hit ATS_ERR, probe succeeded while
recovery_work was running concurrently. Probe returned 0, but the
autosuspend request from ivpu_pm_enable() landed while recovery had
runtime PM disabled, so the NPU stayed active until first use. Not
allowing IRQ-triggered recovery until ivpu_pm_enable() would cover both
cases; I can send a v2 that does that if you prefer.
drivers/accel/ivpu/ivpu_drv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index 247ed6b2ff62..4fcccf78ec5d 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -852,6 +852,7 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
return 0;
err_ipc_fini:
+ ivpu_pm_disable_recovery(vdev);
ivpu_ipc_fini(vdev);
err_fw_fini:
ivpu_fw_fini(vdev);
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] accel/ivpu: Disable recovery work on probe failure
2026-11-07 17:42 [PATCH] accel/ivpu: Disable recovery work on probe failure Daksh Sahni
@ 2026-11-07 18:00 ` sashiko-bot
2026-11-10 8:33 ` Karol Wachowski
2026-11-10 8:50 ` [PATCH v2] accel/ivpu: Defer recovery until probe completes Karol Wachowski
2 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-11-07 18:00 UTC (permalink / raw)
To: Daksh Sahni; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] accel/ivpu: ivpu_pm_recovery_work() races with ivpu_boot()
during probe
--
commit 9885035bb27e3920959278569c3258e9ccfc3b7e
Author: Daksh Sahni <swizzguy76@gmail.com>
accel/ivpu: Disable recovery work on probe failure
This commit calls ivpu_pm_disable_recovery() on the err_ipc_fini error
path in ivpu_dev_init() to flush and disable recovery_work before device
teardown.
> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index 247ed6b2ff62a..4fcccf78ec5db 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
> @@ -852,6 +852,7 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
> return 0;
>
> err_ipc_fini:
> + ivpu_pm_disable_recovery(vdev);
> ivpu_ipc_fini(vdev);
> err_fw_fini:
> ivpu_fw_fini(vdev);
[Severity: Medium]
Can ivpu_pm_recovery_work() still race with ivpu_boot() before
ivpu_dev_init() reaches err_ipc_fini?
ivpu_pm_init() leaves recovery_work enabled before ivpu_boot() enables
IRQs. When an error IRQ queues recovery_work during ivpu_boot(), both the
probe thread and the worker tear down the device concurrently:
CPU0 (probe) CPU1 (kworker)
ivpu_dev_init()
ivpu_boot()
enable_irq(vdev->irq)
<error IRQ>
ivpu_pm_trigger_recovery()
queue_work(recovery_work)
ivpu_pm_recovery_work()
ivpu_suspend()
ivpu_prepare_for_reset()
disable_irq(vdev->irq)
ivpu_ipc_disable()
err_disable_ipc:
disable_irq(vdev->irq)
ivpu_mmu_evtq_dump()
ivpu_shutdown()
ivpu_pm_reset_complete()
ivpu_resume()
ivpu_boot()
enable_irq(vdev->irq)
...
ivpu_hw_power_down()
err_ipc_fini:
ivpu_pm_disable_recovery()
...
ivpu_shutdown() <-- already in PCI_D3hot
Because both ivpu_prepare_for_reset() and err_disable_ipc call
disable_irq(vdev->irq), the single enable_irq() in ivpu_resume() leaves
vdev->irq disabled, causing the recovery boot to time out and power down
the device into PCI_D3hot. Once disable_work_sync() in
ivpu_pm_disable_recovery() returns, err_shutdown calls ivpu_shutdown() a
second time while already in PCI_D3hot, accessing MMIO registers and
overwriting the saved PCI state.
Conversely, if ivpu_resume() in recovery_work succeeds, the NPU is left
powered on in PCI_D0 with IRQs, IPC, and MMU active while err_ipc_fini
frees the IPC, firmware, and MMU buffers without calling
ivpu_prepare_for_reset() first; and if ivpu_boot() in ivpu_dev_init()
finishes before recovery_work calls ivpu_ipc_disable(), probe returns 0
while recovery_work runs with runtime PM disabled, dropping the
autosuspend request from ivpu_pm_enable().
Initializing recovery_work in the disabled state in ivpu_pm_init() and
enabling it in ivpu_pm_enable() would prevent IRQ handlers from queuing
recovery until probe succeeds.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261107174215.15818-1-swizzguy76@gmail.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] accel/ivpu: Disable recovery work on probe failure
2026-11-07 17:42 [PATCH] accel/ivpu: Disable recovery work on probe failure Daksh Sahni
2026-11-07 18:00 ` sashiko-bot
@ 2026-11-10 8:33 ` Karol Wachowski
2026-11-10 8:50 ` [PATCH v2] accel/ivpu: Defer recovery until probe completes Karol Wachowski
2 siblings, 0 replies; 6+ messages in thread
From: Karol Wachowski @ 2026-11-10 8:33 UTC (permalink / raw)
To: Daksh Sahni
Cc: jacek.lawrynowicz, jeff.hugo, dri-devel, linux-kernel, ogabbay,
andrzej.kacprowski
On 07-Nov-26 18:42, Daksh Sahni wrote:
> If the NPU raises an error interrupt while ivpu_dev_init() boots the
> firmware, the Buttress, MMU or WDT IRQ handler calls
> ivpu_pm_trigger_recovery(), which queues recovery_work. When ivpu_boot()
> then fails, the err_ipc_fini path tears the device down without
> disabling recovery_work and probe returns an error. devres unmaps the
> register BARs while the work is still queued or running, and the work
> faults on the first register access:
>
> BUG: unable to handle page fault for address: ffffcfc6680200f4
> Workqueue: events_unbound ivpu_pm_recovery_work [intel_vpu]
> RIP: 0010:ivpu_hw_ip_ipc_rx_addr_get+0xc/0xb0 [intel_vpu]
> Call Trace:
> ivpu_wait_for_ready+0x155/0x220 [intel_vpu]
> ivpu_boot+0x9e/0x270 [intel_vpu]
> ivpu_resume+0xa4/0xd0 [intel_vpu]
> ivpu_pm_reset_complete+0x2c/0x90 [intel_vpu]
> ivpu_pm_recovery_work+0xf6/0x140 [intel_vpu]
> process_one_work+0x1da/0x3c0
> worker_thread+0x1af/0x340
> kthread+0xfa/0x130
> ret_from_fork+0x2b9/0x380
> ret_from_fork_asm+0x1a/0x30
>
> The work dies between pm_runtime_disable() in ivpu_pm_reset_begin() and
> pm_runtime_enable() in ivpu_pm_reset_complete(), so the PCI device is
> also left with runtime PM disabled and never autosuspends after a later
> rebind.
>
> Commit 5eaa49741119 ("accel/ivpu: Prevent recovery invocation during
> probe and resume") stopped IPC failures during probe from triggering
> recovery, but the IRQ handlers still can.
>
> Call ivpu_pm_disable_recovery() at the start of the err_ipc_fini path,
> as ivpu_dev_fini() already does, so the work is flushed and cannot be
> queued again before the device is torn down.
>
> Reproduced on Meteor Lake (Core Ultra 5 125H, vpu_37xx firmware
> 20260219) by kexec'ing with the NPU runtime-suspended: probe in the new
> kernel gets an ATS_ERR Buttress IRQ during firmware boot. With this
> change probe still fails with -ECANCELED, there is no oops, and an FLR
> followed by a rebind brings the NPU back.
>
> Fixes: 852be13f3bd3 ("accel/ivpu: Add PM support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daksh Sahni <swizzguy76@gmail.com>
Hi Daksh,
Thanks for the report and the thorough analysis, it's correct. An error
IRQ that fires once ivpu_boot() has enabled interrupts can queue
recovery_work during probe, and nothing on the probe error path stops
it.
As Sashiko pointed out, disabling the work on err_ipc_fini comes too
late. By then recovery_work may already have run concurrently with
ivpu_boot(). It can also race with a probe that succeeds, which you saw
yourself in one of your runs.
I'll send a v2 that addresses this along the lines you offered and
Sashiko suggested. recovery_work stays disabled for the whole of probe
and is enabled in ivpu_pm_enable(). A recovery requested during probe
is not lost. If ivpu_boot() fails, probe unwinds with nothing pending.
If probe succeeds, ivpu_pm_enable() runs the deferred recovery and
waits for it before issuing the initial autosuspend request. The NPU is
then reset before first use, and the autosuspend no longer lands while
runtime PM is disabled, which covers your second note too. The patch
will keep your Reported-by.
I'm running it through our internal CI first and will post it once that
passes. When it's out, could you test it on your Meteor Lake kexec
setup and reply with your Tested-by? Once I have it, I'll go ahead
and merge.
Thanks,
Karol
> ---
>
> Notes:
> Tested on v7.4-rc1 (d48723711bae).
>
> Two things this patch does not address:
>
> The trigger is kexec with the NPU runtime-suspended in the old kernel.
> The driver has no .shutdown hook, so the new kernel's probe sees stale
> NPU state and gets ATS_ERR. A cold boot doesn't hit it. Whether that
> wants a shutdown hook or a reset at probe seems like your call.
>
> In one of four runs that hit ATS_ERR, probe succeeded while
> recovery_work was running concurrently. Probe returned 0, but the
> autosuspend request from ivpu_pm_enable() landed while recovery had
> runtime PM disabled, so the NPU stayed active until first use. Not
> allowing IRQ-triggered recovery until ivpu_pm_enable() would cover both
> cases; I can send a v2 that does that if you prefer.
>
> drivers/accel/ivpu/ivpu_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index 247ed6b2ff62..4fcccf78ec5d 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
> @@ -852,6 +852,7 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
> return 0;
>
> err_ipc_fini:
> + ivpu_pm_disable_recovery(vdev);
> ivpu_ipc_fini(vdev);
> err_fw_fini:
> ivpu_fw_fini(vdev);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] accel/ivpu: Defer recovery until probe completes
2026-11-07 17:42 [PATCH] accel/ivpu: Disable recovery work on probe failure Daksh Sahni
2026-11-07 18:00 ` sashiko-bot
2026-11-10 8:33 ` Karol Wachowski
@ 2026-11-10 8:50 ` Karol Wachowski
2026-11-10 9:13 ` sashiko-bot
2 siblings, 1 reply; 6+ messages in thread
From: Karol Wachowski @ 2026-11-10 8:50 UTC (permalink / raw)
To: Daksh Sahni, dri-devel
Cc: Andrzej Kacprowski, Oded Gabbay, Jeff Hugo, dawid.osuchowski,
Karol Wachowski, stable
The Buttress, MMU and WDT IRQ handlers call ivpu_pm_trigger_recovery()
once ivpu_boot() enables interrupts. If such an IRQ fires during probe
and ivpu_boot() then fails, the probe error path tears the device down
while recovery_work is still queued or running. devres unmaps the
register BARs and the work faults on its first register access:
BUG: unable to handle page fault for address: ffffcfc6680200f4
Workqueue: events_unbound ivpu_pm_recovery_work [intel_vpu]
RIP: 0010:ivpu_hw_ip_ipc_rx_addr_get+0xc/0xb0 [intel_vpu]
Call Trace:
ivpu_wait_for_ready+0x155/0x220 [intel_vpu]
ivpu_boot+0x9e/0x270 [intel_vpu]
ivpu_resume+0xa4/0xd0 [intel_vpu]
ivpu_pm_reset_complete+0x2c/0x90 [intel_vpu]
ivpu_pm_recovery_work+0xf6/0x140 [intel_vpu]
If ivpu_boot() succeeds instead, recovery runs concurrently with the
rest of probe and can disable runtime PM while ivpu_pm_enable() issues
the initial autosuspend request, leaving the NPU active until first use.
Disable recovery_work in ivpu_pm_init() and enable it in
ivpu_pm_enable() before runtime PM is allowed. A trigger during probe
still records reset_pending, runs failure diagnostics and masks NPU
interrupts, but cannot queue the work. If ivpu_boot() fails, probe
unwinds with no work pending. If probe succeeds, ivpu_pm_enable() runs
the deferred recovery and waits for it to finish before issuing the
initial autosuspend request, so the NPU is reset before it is used and
the request is not lost. reset_pending is checked after enable_work(),
so a trigger racing with ivpu_pm_enable() cannot be lost either.
Fixes: 852be13f3bd3 ("accel/ivpu: Add PM support")
Cc: stable@vger.kernel.org # v6.10+
Reported-by: Daksh Sahni <swizzguy76@gmail.com>
Closes: https://lore.kernel.org/r/20261107174215.15818-1-swizzguy76@gmail.com
Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
---
Notes:
v1:
- Link: https://lore.kernel.org/r/20261107174215.15818-1-swizzguy76@gmail.com
v2:
- Rework of Daksh's "accel/ivpu: Disable recovery work on probe failure":
https://lore.kernel.org/r/20261107174215.15818-1-swizzguy76@gmail.com
- Keep recovery_work disabled from ivpu_pm_init() until ivpu_pm_enable()
instead of disabling it on the probe error path, which is too late as
recovery may already run concurrently with ivpu_boot() (Sashiko)
- Run a recovery requested during probe from ivpu_pm_enable() and wait
for it before the initial autosuspend request, so the NPU is reset
before use and the autosuspend is not lost when probe succeeds
- Add stable prerequisite 69a79ada8eb0
drivers/accel/ivpu/ivpu_pm.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
index aa813262253e..7aee728b19ef 100644
--- a/drivers/accel/ivpu/ivpu_pm.c
+++ b/drivers/accel/ivpu/ivpu_pm.c
@@ -415,6 +415,7 @@ void ivpu_pm_init(struct ivpu_device *vdev)
atomic_set(&pm->engine_reset_counter, 0);
INIT_WORK(&pm->recovery_work, ivpu_pm_recovery_work);
+ disable_work(&pm->recovery_work);
INIT_DELAYED_WORK(&pm->job_timeout_work, ivpu_job_timeout_work);
if (ivpu_disable_recovery)
@@ -439,6 +440,12 @@ void ivpu_pm_enable(struct ivpu_device *vdev)
{
struct device *dev = vdev->drm.dev;
+ enable_work(&vdev->pm->recovery_work);
+ if (atomic_read(&vdev->pm->reset_pending)) {
+ queue_work(system_dfl_wq, &vdev->pm->recovery_work);
+ flush_work(&vdev->pm->recovery_work);
+ }
+
pm_runtime_allow(dev);
pm_runtime_put_autosuspend(dev);
}
base-commit: 54d3ff1b5ac08d0e28029b93a1719ddd9ff670a4
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] accel/ivpu: Defer recovery until probe completes
2026-11-10 8:50 ` [PATCH v2] accel/ivpu: Defer recovery until probe completes Karol Wachowski
@ 2026-11-10 9:13 ` sashiko-bot
2026-11-10 11:41 ` Karol Wachowski
0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-11-10 9:13 UTC (permalink / raw)
To: Karol Wachowski, dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] accel/ivpu: race in ivpu_pm_enable() queues recovery_work
twice
--
commit 53feec71cd6cb0035da92ea965a242f974fa21d7
Author: Karol Wachowski <karol.wachowski@linux.intel.com>
accel/ivpu: Defer recovery until probe completes
This commit disables recovery_work in ivpu_pm_init() and re-enables and
flushes it in ivpu_pm_enable() to prevent recovery from running during
probe before device initialization finishes.
> diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
> index aa813262253e4..7aee728b19ef1 100644
> --- a/drivers/accel/ivpu/ivpu_pm.c
> +++ b/drivers/accel/ivpu/ivpu_pm.c
> @@ -415,6 +415,7 @@ void ivpu_pm_init(struct ivpu_device *vdev)
> atomic_set(&pm->engine_reset_counter, 0);
>
> INIT_WORK(&pm->recovery_work, ivpu_pm_recovery_work);
> + disable_work(&pm->recovery_work);
> INIT_DELAYED_WORK(&pm->job_timeout_work, ivpu_job_timeout_work);
>
> if (ivpu_disable_recovery)
> @@ -439,6 +440,12 @@ void ivpu_pm_enable(struct ivpu_device *vdev)
> {
> struct device *dev = vdev->drm.dev;
>
> + enable_work(&vdev->pm->recovery_work);
> + if (atomic_read(&vdev->pm->reset_pending)) {
> + queue_work(system_dfl_wq, &vdev->pm->recovery_work);
> + flush_work(&vdev->pm->recovery_work);
> + }
[Severity: Medium]
Can ivpu_pm_enable() race with ivpu_pm_trigger_recovery() and queue
recovery_work twice for a single fault?
In ivpu_pm_trigger_recovery(), reset_pending is set to 1 before
ivpu_hw_diagnose_failure(), ivpu_hw_irq_disable(), and queue_work():
drivers/accel/ivpu/ivpu_pm.c:ivpu_pm_trigger_recovery() {
...
if (atomic_cmpxchg(&vdev->pm->reset_pending, 0, 1) == 0) {
ivpu_hw_diagnose_failure(vdev);
ivpu_hw_irq_disable(vdev); /* Disable IRQ early to protect from IRQ storm */
queue_work(system_dfl_wq, &vdev->pm->recovery_work);
}
}
And reset_pending stays 1 until ivpu_pm_reset_complete() finishes, while
the workqueue clears WORK_STRUCT_PENDING_BIT as soon as a worker dequeues
recovery_work:
CPU0 CPU1 kworker
ivpu_pm_trigger_recovery()
atomic_cmpxchg(reset_pending, 0, 1)
ivpu_hw_diagnose_failure()
ivpu_pm_enable()
enable_work(&recovery_work)
if (atomic_read(reset_pending))
queue_work(&recovery_work)
flush_work(&recovery_work)
ivpu_pm_recovery_work()
ivpu_hw_irq_disable()
queue_work(&recovery_work) <-- queued a second time
The same double queueing happens if ivpu_pm_trigger_recovery() runs right
after enable_work() and a worker starts ivpu_pm_recovery_work() before
ivpu_pm_enable() checks reset_pending.
In the first interleaving, ivpu_pm_recovery_work() runs concurrently with
ivpu_hw_diagnose_failure() and ivpu_hw_irq_disable(), and flush_work()
only waits for the first execution. The second queued recovery_work then
runs after flush_work() returns, performing a second full NPU reset
concurrently with pm_runtime_allow() and pm_runtime_put_autosuspend() and
reintroducing the race where recovery disables runtime PM during the
initial autosuspend request.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261110085003.3527014-1-karol.wachowski@linux.intel.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] accel/ivpu: Defer recovery until probe completes
2026-11-10 9:13 ` sashiko-bot
@ 2026-11-10 11:41 ` Karol Wachowski
0 siblings, 0 replies; 6+ messages in thread
From: Karol Wachowski @ 2026-11-10 11:41 UTC (permalink / raw)
To: sashiko-reviews, dri-devel
On 10-Nov-26 10:13, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] accel/ivpu: race in ivpu_pm_enable() queues recovery_work
> twice
> --
>
> commit 53feec71cd6cb0035da92ea965a242f974fa21d7
> Author: Karol Wachowski <karol.wachowski@linux.intel.com>
>
> accel/ivpu: Defer recovery until probe completes
>
> This commit disables recovery_work in ivpu_pm_init() and re-enables and
> flushes it in ivpu_pm_enable() to prevent recovery from running during
> probe before device initialization finishes.
>
>> diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c
>> index aa813262253e4..7aee728b19ef1 100644
>> --- a/drivers/accel/ivpu/ivpu_pm.c
>> +++ b/drivers/accel/ivpu/ivpu_pm.c
>> @@ -415,6 +415,7 @@ void ivpu_pm_init(struct ivpu_device *vdev)
>> atomic_set(&pm->engine_reset_counter, 0);
>>
>> INIT_WORK(&pm->recovery_work, ivpu_pm_recovery_work);
>> + disable_work(&pm->recovery_work);
>> INIT_DELAYED_WORK(&pm->job_timeout_work, ivpu_job_timeout_work);
>>
>> if (ivpu_disable_recovery)
>> @@ -439,6 +440,12 @@ void ivpu_pm_enable(struct ivpu_device *vdev)
>> {
>> struct device *dev = vdev->drm.dev;
>>
>> + enable_work(&vdev->pm->recovery_work);
>> + if (atomic_read(&vdev->pm->reset_pending)) {
>> + queue_work(system_dfl_wq, &vdev->pm->recovery_work);
>> + flush_work(&vdev->pm->recovery_work);
>> + }
>
> [Severity: Medium]
> Can ivpu_pm_enable() race with ivpu_pm_trigger_recovery() and queue
> recovery_work twice for a single fault?
>
> In ivpu_pm_trigger_recovery(), reset_pending is set to 1 before
> ivpu_hw_diagnose_failure(), ivpu_hw_irq_disable(), and queue_work():
>
> drivers/accel/ivpu/ivpu_pm.c:ivpu_pm_trigger_recovery() {
> ...
> if (atomic_cmpxchg(&vdev->pm->reset_pending, 0, 1) == 0) {
> ivpu_hw_diagnose_failure(vdev);
> ivpu_hw_irq_disable(vdev); /* Disable IRQ early to protect from IRQ storm */
> queue_work(system_dfl_wq, &vdev->pm->recovery_work);
> }
> }
>
> And reset_pending stays 1 until ivpu_pm_reset_complete() finishes, while
> the workqueue clears WORK_STRUCT_PENDING_BIT as soon as a worker dequeues
> recovery_work:
>
> CPU0 CPU1 kworker
> ivpu_pm_trigger_recovery()
> atomic_cmpxchg(reset_pending, 0, 1)
> ivpu_hw_diagnose_failure()
> ivpu_pm_enable()
> enable_work(&recovery_work)
> if (atomic_read(reset_pending))
> queue_work(&recovery_work)
> flush_work(&recovery_work)
> ivpu_pm_recovery_work()
> ivpu_hw_irq_disable()
> queue_work(&recovery_work) <-- queued a second time
>
> The same double queueing happens if ivpu_pm_trigger_recovery() runs right
> after enable_work() and a worker starts ivpu_pm_recovery_work() before
> ivpu_pm_enable() checks reset_pending.
>
> In the first interleaving, ivpu_pm_recovery_work() runs concurrently with
> ivpu_hw_diagnose_failure() and ivpu_hw_irq_disable(), and flush_work()
> only waits for the first execution. The second queued recovery_work then
> runs after flush_work() returns, performing a second full NPU reset
> concurrently with pm_runtime_allow() and pm_runtime_put_autosuspend() and
> reintroducing the race where recovery disables runtime PM during the
> initial autosuspend request.
>
I'll send V3 that addresses recovery that leaves the device in D0.
Karol
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-11-10 11:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-07 17:42 [PATCH] accel/ivpu: Disable recovery work on probe failure Daksh Sahni
2026-11-07 18:00 ` sashiko-bot
2026-11-10 8:33 ` Karol Wachowski
2026-11-10 8:50 ` [PATCH v2] accel/ivpu: Defer recovery until probe completes Karol Wachowski
2026-11-10 9:13 ` sashiko-bot
2026-11-10 11:41 ` Karol Wachowski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.