Re: [PATCH v3 4/5] drm/imagination: Limit access to the META firmware interface

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/imagination: inverted layout_mars_value check in layout_is_mars()
--

commit 4207fc7502d043d11f6874d92c1ba51c38ade0fe
Author: Alessio Belle <alessio.belle@imgtec.com>

drm/imagination: Limit access to the META firmware interface

This commit isolates access to the META firmware interface behind the
generic firmware processor table in struct pvr_fw_defs, removes
pvr_fw_meta.h, and renames pvr_rogue_meta.h to pvr_rogue_meta_internal.h.

> diff --git a/drivers/gpu/drm/imagination/pvr_fw_startstop.c b/drivers/gpu/drm/imagination/pvr_fw_startstop.c
> index 98ca2692239d0..ab637ba7d108c 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw_startstop.c
> +++ b/drivers/gpu/drm/imagination/pvr_fw_startstop.c

[ ... ]

> @@ -199,6 +197,30 @@ pvr_fw_start(struct pvr_device *pvr_dev)
>  	return err;
>  }
>  
> +static inline bool layout_is_mars(struct pvr_device *pvr_dev)
> +{
> +	u64 layout_mars_value = 0;
> +
> +	if (PVR_FEATURE_VALUE(pvr_dev, layout_mars, &layout_mars_value))
> +		return false;
> +
> +	return layout_mars_value == 0;
> +}

[Severity: High]
As reported by Sashiko on v2 [1], does layout_is_mars() invert the check on
layout_mars_value when PVR_FEATURE_VALUE() succeeds?

Prior to this change, pvr_fw_stop() checked:

	if (PVR_FEATURE_VALUE(pvr_dev, layout_mars, &layout_mars_value) == 0)
		layout_mars = layout_mars_value > 0;

In layout_is_mars(), when PVR_FEATURE_VALUE() returns 0 (feature present),
it evaluates layout_mars_value == 0 instead of layout_mars_value > 0.

On cores with the layout_mars feature enabled (layout_mars_value > 0),
layout_is_mars() will return false, causing pvr_fw_stop() to poll
ROGUE_CR_SIDEKICK_IDLE and ROGUE_CR_SLC_IDLE (which are already powered
down by the firmware on MARS cores) and skip polling ROGUE_CR_MARS_IDLE
before asserting ROGUE_CR_SOFT_RESET. Should this return
layout_mars_value > 0 instead?

[1] https://lore.kernel.org/r/20261029-prep-meta-fw-arch-independent-v2-4-b9f5d69b7a52@imgtec.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-prep-meta-fw-arch-independent-v3-0-563dbaf1b779@imgtec.com?part=4