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

[Adrián Larumbe]

On Fri, 25 Sep 2026 19:45:36 +0100, Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx> wrote:
> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index b3f71d7fd82a..96c3c41cd269 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> @@ -58,44 +70,154 @@ void panfrost_perfcnt_sample_done(struct panfrost_device *pfdev)
>  		complete(&pfdev->perfcnt->dump_comp);
>  }
>  
> -static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev)
> +static int panfrost_perfcnt_hw_enable(struct panfrost_device *pfdev)
> +{
> +	struct panfrost_perfcnt *perfcnt = pfdev->perfcnt;
> +	u32 cfg, as;
> +	int ret;
> +

I might add a lockdep_assert_held(&pfdev->reset.lock); here.

> [ ... skip 152 lines ... ]
> +		if (perfcnt->user != user)
> +			return -EBUSY;
> +
> +		if (perfcnt->state & PANFROST_PERFCNT_SESSION_DEAD)
> +			drm_WARN_ON(&pfdev->base,
> +				    panfrost_perfcnt_disable_locked(pfdev, file_priv));

Forgot to add an 'else return 0'. Also checking the state flag here is racy with the reset sequence,
which could keep coming through, because perfcnt->user hasn't changed.
Also, on a second thought, it seems going through  perfcnt_disable to recreate a whole new session
from scratch is an overkill, since the only thing we need is an MMU reference for the perfcnt
BO's AS. On top of that, panfrost_perfcnt_disable_locked() might power the device off and then
getting a PM reference right below might power it up again, a completely pointless cycle.

So maybe:

if (perfcnt->user == user) {
	scoped_guard(rwsem_read, &pfdev->reset.lock) {
		if (perfcnt->state & PANFROST_PERFCNT_SESSION_DEAD) {
			ret = panfrost_perfcnt_hw_enable(pfdev);
			if (!ret)
				perfcnt->state = 0;
			return ret;
		}
	}

	return 0;
}

if (perfcnt->user != NULL)
	return -EBUSY;

An alterantive would be expecting UM to issue a perfcnt_disable before being able to enable a new session.

-- 
Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx>