[PATCH 1/2] drm/panthor: Fix missing explicit padding in drm_panthor_gpu_info

[Liviu Dudau]

On Thu, Apr 17, 2025 at 12:05:02PM +0200, Boris Brezillon wrote:
> drm_panthor_gpu_info::shader_present is currently automatically offset
> by 4 byte to meet Arm's 32-bit/64-bit field alignment rules, but those
> constraints don't stand on 32-bit x86 and cause a mismatch when running
> an x86 binary in a user emulated environment like FEX. It's also
> generally agreed that uAPIs should explicitly pad their struct fields,
> which we originally intended to do, but a mistake slipped through during
> the submission process, leading drm_panthor_gpu_info::shader_present to
> be misaligned.
> 
> This uAPI change doesn't break any of the existing users of panthor
> which are either arm32 or arm64 where the 64-bit alignment of
> u64 fields is already enforced a the compiler level.
> 
> Fixes: 0f25e493a246 ("drm/panthor: Add uAPI")
> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> ---
>  include/uapi/drm/panthor_drm.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
> index 97e2c4510e69..1379a2d4548c 100644
> --- a/include/uapi/drm/panthor_drm.h
> +++ b/include/uapi/drm/panthor_drm.h
> @@ -293,6 +293,18 @@ struct drm_panthor_gpu_info {
>  	/** @as_present: Bitmask encoding the number of address-space exposed by the MMU. */
>  	__u32 as_present;
>  
> +	/**
> +	 * @garbage: Unused field that's not even zero-checked.

By whom? Kernel provides it and initializes it to zero, so I guess that's why it's not checked.

> +	 *
> +	 * This originates from a missing padding that leaked in the initial driver submission
> +	 * and was only found when testing the driver in a 32-bit x86 environment, where
> +	 * u64 field alignment rules are relaxed compared to aarch32.
> +	 *
> +	 * This field can't be repurposed, because it's never been checked by the driver and
> +	 * userspace is not guaranteed to zero it out.

The direction is the other way around, it's provided by kernel. I would loosen the restriction
on repurposing, maybe when 32bit x86 is no longer such a hot market we can reuse it for some
other chicken bits.

With the comment updated,

Acked-by: Liviu Dudau <liviu.dudau at arm.com>

Best regards,
Liviu

> +	 */
> +	__u32 garbage;
> +
>  	/** @shader_present: Bitmask encoding the shader cores exposed by the GPU. */
>  	__u64 shader_present;
>  
> -- 
> 2.49.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯