Re: [PATCH v7 11/17] drm/panfrost: Add debugfs knob for manually triggering a GPU reset

[sashiko-bot@kernel.org]

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 <adrian.larumbe@collabora.com>

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/20260828-claude-fixes-v7-0-72a13b2c125d@collabora.com?part=11