Re: [PATCH v1 26/27] drm/panthor: Prevent missed post-yield cleanup due to reset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] drm: scheduler deadlock on reset due to missed post-reset hooks
- [High] drm: failed reset bypasses device unplug due to AW state transition
- [High] drm: TOCTOU race in reset causes invalid MMIO access
--

commit c5cc732c23a490baff27bd3657a378e7ad65a4e4
Author: Karunika Choo <karunika.choo@arm.com>

drm/panthor: Prevent missed post-yield cleanup due to reset

With v15 GPUs, FW can yield GPU access at any point. This commit introduces
pre_reset and post_reset helpers for panthor_aw which marks a granted AW as
reset-active, aiming to prevent a race condition between reset and FW yield
from suppressing post-yield cleanup.

> diff --git a/drivers/gpu/drm/panthor/panthor_aw.c b/drivers/gpu/drm/panthor/panthor_aw.c
> index 9ebb23eadfec5..7ef13db4acf50 100644
> --- a/drivers/gpu/drm/panthor/panthor_aw.c
> +++ b/drivers/gpu/drm/panthor/panthor_aw.c
[ ... ]
> @@ -579,5 +588,36 @@ bool panthor_aw_has_gpu_access(struct panthor_device *ptdev)
[ ... ]
> +void panthor_aw_pre_reset(struct panthor_device *ptdev)
> +{
> +	struct panthor_aw *aw = ptdev->aw;
> +
> +	if (!aw)
> +		return;
> +
> +	panthor_aw_state_try_set(ptdev->aw, PANTHOR_AW_STATE_GPU_GRANTED,
> +				 PANTHOR_AW_STATE_RESET_ACTIVE);
> +}
> +
> +void panthor_aw_post_reset(struct panthor_device *ptdev)
> +{
> +	struct panthor_aw *aw = ptdev->aw;
> +
> +	if (!aw)
> +		return;
> +
> +	panthor_aw_state_try_set(aw, PANTHOR_AW_STATE_RESET_ACTIVE,
> +				 PANTHOR_AW_STATE_GPU_GRANTED);
> +
> +	if (!panthor_aw_is_open(aw))
> +		panthor_aw_schedule_post_yield_work(aw);
>  }
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index ea35c5e1d87b4..86eb4e9a61789 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -138,18 +138,27 @@ static void panthor_device_reset_work(struct work_struct *work)
>  	if (!drm_dev_enter(&ptdev->base, &cookie))
>  		return;
>  
> +	if (!panthor_aw_has_gpu_access(ptdev))
> +		goto out_clear_pending;

[Severity: High]
If a GPU reset is scheduled (e.g., due to a job timeout), and just before
panthor_device_reset_work() runs, the firmware yields the GPU, this check
returns false and causes a jump to out_clear_pending.

Since out_clear_pending clears the reset.pending flag but skips calling
panthor_sched_post_reset(), does this leave the scheduler's tick work and
firmware watchdog permanently deadlocked, causing all jobs to hang forever
without recovery?

Additionally, does this introduce a Time-Of-Check to Time-Of-Use (TOCTOU)
race condition?

If an IRQ fires after panthor_aw_has_gpu_access(ptdev) is checked but before
panthor_aw_pre_reset(ptdev) is called, changing the state to STOPPED_IDLE,
panthor_aw_pre_reset() will silently fail to update the state since it
ignores the return value of panthor_aw_state_try_set().

Will this allow the reset sequence to continue against unpowered MMIO,
leading to bus faults because GPU access has been revoked?

> +
>  	panthor_sched_pre_reset(ptdev);
> +	panthor_aw_pre_reset(ptdev);
>  	panthor_fw_pre_reset(ptdev, true);
>  	panthor_mmu_pre_reset(ptdev);
> +
>  	panthor_hw_soft_reset(ptdev);
> +
>  	panthor_hw_l2_power_on(ptdev);
>  	panthor_mmu_post_reset(ptdev);
>  	ret = panthor_fw_post_reset(ptdev);
> -	atomic_set(&ptdev->reset.pending, 0);
> +	panthor_aw_post_reset(ptdev);
>  	panthor_sched_post_reset(ptdev, ret != 0);
> +
> +out_clear_pending:
> +	atomic_set(&ptdev->reset.pending, 0);
>  	drm_dev_exit(cookie);
>  
> -	if (ret) {
> +	if (ret && panthor_aw_has_gpu_access(ptdev)) {

[Severity: High]
If the MCU fails to boot during reset, panthor_fw_post_reset() will return an
error. Then panthor_aw_post_reset() executes and, seeing the window is not
open, calls panthor_aw_schedule_post_yield_work() which transitions the AW
state to PANTHOR_AW_STATE_GPU_STOPPED.

Since the state is now GPU_STOPPED, panthor_aw_has_gpu_access() will evaluate
to false. Will this completely bypass the device unplug operation, leaving a
broken device registered and accessible?

>  		panthor_device_unplug(ptdev);
>  		drm_err(&ptdev->base, "Failed to boot MCU after reset, making device unusable.");
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922204535.2850094-1-karunika.choo@arm.com?part=26