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 <[email protected]>

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 <[email protected]>
> Cc: Francois Dugast <[email protected]>
> Cc: José Roberto de Souza <[email protected]>
> Cc: Matt Roper <[email protected]>
> Signed-off-by: Rodrigo Vivi <[email protected]>
> ---
>  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;
>  

Reply via email to