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 <[email protected]> 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/[email protected]?part=26
