Re: [PATCH v12 11/15] drm/panfrost: Move perfcnt GPU disable sequence into a helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/panfrost: inverted hardware disable sequence risks spurious dump
--

commit 3efe7a19ba3bb185c762f241fc91b732bd783ed3
Author: Adrián Larumbe <adrian.larumbe@collabora.com>

drm/panfrost: Move perfcnt GPU disable sequence into a helper

This commit introduces a helper function to avoid repetition of the hardware
performance counter disable sequence.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index 7020c0192e189..f71534e741b65 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> @@ -35,6 +35,16 @@ struct panfrost_perfcnt {
>  	struct completion dump_comp;
>  };
>  
> +static void panfrost_perfcnt_hw_disable(struct panfrost_device *pfdev)
> +{
> +	gpu_write(pfdev, GPU_PERFCNT_CFG,
> +		  GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF));
> +	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x0);
> +	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0x0);
> +	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0x0);
> +	gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
> +}

[Severity: High]
Does this sequence introduce a risk of a spurious hardware dump?

The new helper turns off the GPU_PERFCNT_CFG register before disabling the
individual block counters, which reverses the previous safe sequence in
panfrost_perfcnt_disable_locked() where the EN registers were cleared first.

Modifying the CFG register while TILER_EN is still active might trigger a
spurious dump. This seems similar to hardware erratum PRLAM-8186, which
requires TILER_EN to be off before modifying the CFG register to enable
counters.

[ ... ]

> @@ -193,12 +203,7 @@ static int panfrost_perfcnt_disable_locked(struct panfrost_device *pfdev,
>  	if (user != perfcnt->user)
>  		return -EINVAL;
>  
> -	gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x0);
> -	gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0x0);
> -	gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0x0);
> -	gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
> -	gpu_write(pfdev, GPU_PERFCNT_CFG,
> -		  GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF));
> +	panfrost_perfcnt_hw_disable(pfdev);
>  
>  	perfcnt->user = NULL;
>  	drm_gem_vunmap(&perfcnt->mapping->obj->base.base, &map);

[Severity: High]
Could a spurious hardware dump here result in memory corruption?

Because the inverted hardware disable sequence is invoked while TILER_EN is
active, a spurious hardware dump would cause the GPU to write to the
performance counter buffer asynchronously.

Since panfrost_perfcnt_disable_locked() immediately tears down and unmaps
this memory via drm_gem_vunmap(), could the GPU perform a DMA write-after-free,
leading to an IOMMU fault or memory corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-claude-fixes-v12-0-62beb08de207@collabora.com?part=11