[Intel-gfx] [PATCH] drm/xe/uapi: Fix various struct padding for 64b alignment

Souza, Jose jose.souza at intel.com
Fri Nov 17 20:57:31 UTC 2023


On Fri, 2023-11-17 at 15:48 -0500, Rodrigo Vivi wrote:
> Let's respect Documentation/process/botching-up-ioctls.rst
> and add the proper padding for a 64b alignment with all as
> well as all the required checks and settings for the pads
> and the reserved entries.
> 
> v2: Fix remaining wholes and double check with pahole (Jose)
>     Ensure with pahole that both 32b and 64b have exact same
>     layout (Thomas)
>     Do not set query's pad and reserved bits to zero since it
>     is redundant and already done by kzalloc (Matt)


Reviewed-by: José Roberto de Souza <jose.souza at intel.com>

But this has changes that were done in https://patchwork.freedesktop.org/series/126535/ so it will not apply on top of current drm-xe-next.

> 
> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> Cc: Francois Dugast <francois.dugast at intel.com>
> Cc: José Roberto de Souza <jose.souza at intel.com>
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_query.c |  1 +
>  drivers/gpu/drm/xe/xe_vm.c    |  8 ++++++++
>  include/uapi/drm/xe_drm.h     | 19 +++++++++++--------
>  3 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> index 838f03795841..c5f2b3d67166 100644
> --- a/drivers/gpu/drm/xe/xe_query.c
> +++ b/drivers/gpu/drm/xe/xe_query.c
> @@ -377,6 +377,7 @@ static int query_gt_list(struct xe_device *xe, struct drm_xe_device_query *query
>  		return -ENOMEM;
>  
>  	gt_list->num_gt = xe->info.gt_count;
> +
>  	for_each_gt(gt, xe, id) {
>  		if (xe_gt_is_media_type(gt))
>  			gt_list->gt_list[id].type = DRM_XE_QUERY_GT_TYPE_MEDIA;
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index f8559ebad9bc..2f22f1feaf1c 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2850,6 +2850,10 @@ static int vm_bind_ioctl_check_args(struct xe_device *xe,
>  	int err;
>  	int i;
>  
> +	if (XE_IOCTL_DBG(xe, args->pad || args->pad2) ||
> +	    XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
> +		return -EINVAL;
> +
>  	if (XE_IOCTL_DBG(xe, args->extensions) ||
>  	    XE_IOCTL_DBG(xe, !args->num_binds) ||
>  	    XE_IOCTL_DBG(xe, args->num_binds > MAX_BINDS))
> @@ -2966,6 +2970,10 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
>  	if (err)
>  		return err;
>  
> +	if (XE_IOCTL_DBG(xe, args->pad || args->pad2) ||
> +	    XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
> +		return -EINVAL;
> +
>  	if (args->exec_queue_id) {
>  		q = xe_exec_queue_lookup(xef, args->exec_queue_id);
>  		if (XE_IOCTL_DBG(xe, !q)) {
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 8610ac461619..c07cd4b61baa 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -211,8 +211,6 @@ struct drm_xe_mem_region {
>  	 * a unique pair.
>  	 */
>  	__u16 instance;
> -	/** @pad: MBZ */
> -	__u32 pad;
>  	/**
>  	 * @min_page_size: Min page-size in bytes for this region.
>  	 *
> @@ -384,6 +382,8 @@ struct drm_xe_gt {
>  	__u16 tile_id;
>  	/** @gt_id: Unique ID of this GT within the PCI Device */
>  	__u16 gt_id;
> +	/** @pad: MBZ */
> +	__u16 pad[3];
>  	/** @clock_freq: A clock frequency for timestamp */
>  	__u32 clock_freq;
>  	/**
> @@ -723,6 +723,9 @@ struct drm_xe_vm_bind_op {
>  	 */
>  	__u32 prefetch_mem_region_instance;
>  
> +	/** @pad: MBZ */
> +	__u32 pad2;
> +
>  	/** @reserved: Reserved */
>  	__u64 reserved[2];
>  };
> @@ -741,12 +744,12 @@ struct drm_xe_vm_bind {
>  	 */
>  	__u32 exec_queue_id;
>  
> -	/** @num_binds: number of binds in this IOCTL */
> -	__u32 num_binds;
> -
>  	/** @pad: MBZ */
>  	__u32 pad;
>  
> +	/** @num_binds: number of binds in this IOCTL */
> +	__u32 num_binds;
> +
>  	union {
>  		/** @bind: used if num_binds == 1 */
>  		struct drm_xe_vm_bind_op bind;
> @@ -758,12 +761,12 @@ struct drm_xe_vm_bind {
>  		__u64 vector_of_binds;
>  	};
>  
> +	/** @pad: MBZ */
> +	__u32 pad2;
> +
>  	/** @num_syncs: amount of syncs to wait on */
>  	__u32 num_syncs;
>  
> -	/** @pad2: MBZ */
> -	__u32 pad2;
> -
>  	/** @syncs: pointer to struct drm_xe_sync array */
>  	__u64 syncs;
>  



More information about the Intel-gfx mailing list