Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] Incorrect handling of pm_runtime_get_if_in_use() return value in reset_set() leading to usage count underflow. Pre-existing issues: - [High] Incorrect handling of pm_runtime_get_if_in_use() return value in panfrost_gpu_get_latest_flush_id() causing unsafe MMIO reads and PM underflow. -- commit 6c0954bdc79e7702d546ce41292da37dbd1a90e1 Author: Adrián Larumbe <[email protected]> drm/panfrost: Add debugfs knob for manually triggering a GPU reset This patch adds a debugfs file for manually triggering a GPU reset to aid in testing potential races between the reset sequence and hardware register access. > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > b/drivers/gpu/drm/panfrost/panfrost_device.c > index d8acae9b8cfac..b6a48ae0d3a61 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c [ ... ] > @@ -600,9 +601,43 @@ EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = { > }; > > #ifdef CONFIG_DEBUG_FS [ ... ] > +static int reset_set(void *data, u64 val) > +{ > + struct panfrost_device *pfdev = > + container_of(data, struct panfrost_device, base); > + > + if (pm_runtime_get_if_in_use(pfdev->base.dev)) { [Severity: Medium] When pm_runtime_get_if_in_use() is called while runtime PM is disabled (such as during driver unbind or system suspend), it returns -EINVAL. Since -EINVAL evaluates to true in this condition, does this cause the block to execute without a successful get? If so, could this regression lead to an unbalanced pm_runtime_put() call at the end of the block and a PM usage counter underflow? > + panfrost_device_schedule_reset(pfdev); > + flush_work(&pfdev->reset.work); > + pm_runtime_put(pfdev->base.dev); > + } > + > + return 0; > +} [ ... ] [Severity: High] This isn't a bug introduced by this patch, but looking at a similar pre-existing pattern in drivers/gpu/drm/panfrost/panfrost_gpu.c: panfrost_gpu_get_latest_flush_id() { if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION)) { if (pm_runtime_get_if_in_use(pfdev->base.dev)) { flush_id = gpu_read(pfdev, GPU_LATEST_FLUSH_ID); pm_runtime_put(pfdev->base.dev); return flush_id; } } ... } If pm_runtime_get_if_in_use() returns -EINVAL during system suspend or teardown when disable_depth > 0, does this condition also evaluate to true? Could this result in an unsafe MMIO read on potentially powered-off hardware, followed by another unbalanced usage count decrement? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
