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@xxxxxxxxx> 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@xxxxxxxxxxxxxxx> > Cc: Francois Dugast <francois.dugast@xxxxxxxxx> > Cc: José Roberto de Souza <jose.souza@xxxxxxxxx> > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > --- > 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; >
