Re: [PATCH v6 15/16] drm/panfrost: Fix races between perfcnt and reset sequence

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Hardware initialization is bypassed on new perfcnt sessions, breaking performance counters.
- [High] Unconditional MMU AS reference put causes reference count underflow.
- [Medium] Mixing goto-based error handling with scoped_guard violates the kernel cleanup API.
--

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

drm/panfrost: Fix races between perfcnt and reset sequence

Formerly, the reset sequence would race with panfrost_mmu_as_put()
when tearing down a perfcnt session. On top of that, poking GPU
registers to program a perfcnt session or obtaining a dump might lead to
undefined behaviour when done at the same time a reset was ongoing.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index b3f71d7fd82a9..7dcf27add7ae7 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
[ ... ]
> @@ -132,44 +206,20 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
>  	gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
>  	gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_CLEAR);
>  
> -	ret = panfrost_mmu_as_get(pfdev, perfcnt->mapping->mmu);
> -	if (ret < 0)
> -		goto err_vunmap;
> -
[ ... ]
> +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> +		if (perfcnt->reset_failed) {
> +			ret = panfrost_perfcnt_hw_enable(pfdev);
> +			if (ret)
> +				goto err_vunmap;
> +		}

[Severity: High]
Does this code inadvertently skip hardware initialization for new perfcnt
sessions?

Earlier in panfrost_perfcnt_enable_locked(), the function returns early if
perfcnt->user is not NULL. Because reset_failed is only set during a reset
while a session is already active, reset_failed is guaranteed to be false
when creating a new session.

Consequently, panfrost_perfcnt_hw_enable() is never called here, the MMU
address space is not acquired, and hardware performance counters are never
enabled.

[Severity: Medium]
Is it safe to mix goto-based error handling with scoped_guard() blocks here?

According to the kernel cleanup subsystem guidelines, the usage of "goto" and
cleanup helpers should never be mixed in the same function to prevent
confusing ownership semantics and potential resource leak bugs.

The jump to err_vunmap directly exits the scoped_guard block, breaking this
rule.

> +		perfcnt->reset_happened = false;
> +		perfcnt->reset_failed = false;
> +		perfcnt->user = user;
> +	}
>  
>  	/* The BO ref is retained by the mapping. */
>  	drm_gem_object_put(&bo->base);
[ ... ]
> @@ -195,13 +245,15 @@ static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev,
>  	if (user != perfcnt->user)
>  		return -EINVAL;
>  
> -	panfrost_perfcnt_hw_disable(pfdev);
> +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> +		panfrost_perfcnt_hw_disable(pfdev);
> +		panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu);

[Severity: High]
Can this unconditional call to panfrost_mmu_as_put() cause a reference count
underflow regression?

If panfrost_perfcnt_hw_enable() was skipped during initialization as noted
above, or if it failed to re-acquire the AS during reset recovery, the perfcnt
session does not actually hold an MMU AS reference.

When a userspace client closes the perfcnt session, panfrost_mmu_as_put() is
called unconditionally, which will drive the mmu->as_count reference count to
-1 and trigger a kernel warning.

> +		perfcnt->user = NULL;
> +	}
>  
> -	perfcnt->user = NULL;
>  	drm_gem_vunmap(&perfcnt->mapping->obj->base.base, &map);
>  	perfcnt->buf = NULL;
>  	panfrost_gem_close(&perfcnt->mapping->obj->base.base, file_priv);
> -	panfrost_mmu_as_put(pfdev, perfcnt->mapping->mmu);
>  	panfrost_gem_mapping_put(perfcnt->mapping);
>  	perfcnt->mapping = NULL;
>  	pm_runtime_put_autosuspend(pfdev->base.dev);

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