Re: [PATCH v6 07/16] drm/panfrost: Split subsystem init/reset from interrupt enablement

[sashiko-bot@kernel.org]

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

New issues:
- [High] Masking GPU interrupts during device teardown creates an interrupt storm deadlock if jobs are actively running.

Pre-existing issues:
- [High] Race condition during device teardown where a pending reset work item executes concurrently with or after scheduler destruction, causing a Use-After-Free and erroneously re-enabling interrupts.
--

commit a45fd41d29f7c5d2faeeffea961ca1caad97ff77
Author: Adrián Larumbe <adrian.larumbe@collabora.com>

drm/panfrost: Split subsystem init/reset from interrupt enablement

Because MMU interrupts are only enabled when the device is reset, it
happened that after DRM device registration, the very first job targeting
the tiler heap BO would always time out. The reason is the reset sequence
is only part of PM runtime resume, which is not called explicitly at driver
probe time, and an actual reset work item manually triggered after a HW
error.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 9549ddb7854a3..0cc277efb5850 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
[ ... ]
> @@ -341,6 +365,8 @@ void panfrost_device_fini(struct panfrost_device *pfdev)
>  	pm_runtime_get_sync(pfdev->base.dev);
>  	pm_runtime_disable(pfdev->base.dev);
>  
> +	panfrost_device_disable_hw(pfdev);
> +

[Severity: High]
Does calling panfrost_device_disable_hw() unconditionally on driver unload
create an interrupt storm risk?

Since jobs are not guaranteed to be stopped yet, the GPU may generate a fault
or completion interrupt in panfrost_gpu_suspend_irq() right after setting
the software flag but before masking the hardware interrupt:

drivers/gpu/drm/panfrost/panfrost_gpu.c:panfrost_gpu_suspend_irq() {
    set_bit(PANFROST_COMP_BIT_GPU, pfdev->is_suspended);
    <-- interrupt fires here
    gpu_write(pfdev, GPU_INT_MASK, 0);
}

The panfrost_gpu_irq_handler() sees is_suspended set and returns IRQ_NONE
without clearing the hardware interrupt. Because the IRQ is level-triggered,
it would re-fire immediately upon exit, leading to a soft lockup and forcing
the kernel to permanently disable the shared IRQ line.

>  	panfrost_gem_fini(pfdev);
>  	panfrost_perfcnt_fini(pfdev);
>  	panfrost_jm_fini(pfdev);
[ ... ]
> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
> index 5016d2d53da26..087905cd347a5 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
[ ... ]
> @@ -753,8 +753,8 @@ panfrost_reset(struct panfrost_device *pfdev,
>  	for (i = 0; i < NUM_JOB_SLOTS; i++)
>  		drm_sched_start(&pfdev->js->queue[i].sched, 0);
>  
> -	/* Re-enable job interrupts now that everything has been restarted. */
> -	panfrost_jm_enable_interrupts(pfdev);
> +	/* Re-enable interrupts now that everything has been restarted. */
> +	panfrost_device_enable_int(pfdev);

[Severity: High]
This is a pre-existing issue, but does adding panfrost_device_enable_int()
here exacerbate a teardown race with the reset workqueue?

During teardown, panfrost_device_fini() disables hardware and then calls
panfrost_jm_fini(), which destroys the schedulers before synchronizing the
reset workqueue:

drivers/gpu/drm/panfrost/panfrost_job.c:panfrost_jm_fini() {
    for (j = 0; j < NUM_JOB_SLOTS; j++) {
        drm_sched_fini(&js->queue[j].sched);
    }
    cancel_work_sync(&pfdev->reset.work);
}

If a job timeout occurs before or during panfrost_device_fini(), the
concurrent reset work item can execute after scheduler destruction, causing
a use-after-free.

Furthermore, because this patch adds panfrost_device_enable_int() to the end
of the reset sequence, the racing reset work could erroneously clear the
is_suspended bits and re-enable hardware interrupts right before the clocks
are turned off in teardown.

>  
>  	dma_fence_end_signalling(cookie);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-claude-fixes-v6-0-78119af409e7@collabora.com?part=7