On Sat, 15 Feb 2025 at 00:34, Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
>
> On Wed, Feb 12, 2025 at 07:31:26PM +1030, Gustavo A. R. Silva wrote:
> > -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> > getting ready to enable it, globally.
> >
> > So, in order to avoid ending up with flexible-array members in the
> > middle of other structs, we use the `struct_group_tagged()` helper
> > to separate the flexible arrays from the rest of the members in the
> > flexible structures. We then use the newly created tagged `struct
> > nvif_ioctl_v0_hdr` and `struct nvif_ioctl_mthd_v0_hdr` to replace the
> > type of the objects causing trouble in multiple structures.
> >
> > We also want to ensure that when new members need to be added to the
> > flexible structures, they are always included within the newly created
> > tagged structs. For this, we use `static_assert()`. This ensures that the
> > memory layout for both the flexible structure and the new tagged struct
> > is the same after any changes.
> >
> > So, with these changes, fix the following warnings:
> > drivers/gpu/drm/nouveau/nvif/object.c:60:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nvif/object.c:233:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nvif/object.c:214:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nvif/object.c:152:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nvif/object.c:138:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nvif/object.c:104:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nouveau_svm.c:83:35: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> > drivers/gpu/drm/nouveau/nouveau_svm.c:82:30: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> >
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>
>
> @Faith, Karol: Can I get an ACK from mesa for this one?
If we do reimport this to userspace we will have to figure it out, but for now,
Acked-by: Dave Airlie <airlied@xxxxxxxxxx>
>
> > ---
> > drivers/gpu/drm/nouveau/include/nvif/ioctl.h | 32 +++++++++++++-------
> > drivers/gpu/drm/nouveau/nouveau_svm.c | 4 +--
> > drivers/gpu/drm/nouveau/nvif/object.c | 12 ++++----
> > 3 files changed, 29 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/include/nvif/ioctl.h b/drivers/gpu/drm/nouveau/include/nvif/ioctl.h
> > index e825c8a1d9ca..00015412cb3e 100644
> > --- a/drivers/gpu/drm/nouveau/include/nvif/ioctl.h
> > +++ b/drivers/gpu/drm/nouveau/include/nvif/ioctl.h
> > @@ -3,25 +3,30 @@
> > #define __NVIF_IOCTL_H__
> >
> > struct nvif_ioctl_v0 {
> > - __u8 version;
> > + /* New members MUST be added within the struct_group() macro below. */
> > + struct_group_tagged(nvif_ioctl_v0_hdr, __hdr,
> > + __u8 version;
> > #define NVIF_IOCTL_V0_SCLASS 0x01
> > #define NVIF_IOCTL_V0_NEW 0x02
> > #define NVIF_IOCTL_V0_DEL 0x03
> > #define NVIF_IOCTL_V0_MTHD 0x04
> > #define NVIF_IOCTL_V0_MAP 0x07
> > #define NVIF_IOCTL_V0_UNMAP 0x08
> > - __u8 type;
> > - __u8 pad02[4];
> > + __u8 type;
> > + __u8 pad02[4];
> > #define NVIF_IOCTL_V0_OWNER_NVIF 0x00
> > #define NVIF_IOCTL_V0_OWNER_ANY 0xff
> > - __u8 owner;
> > + __u8 owner;
> > #define NVIF_IOCTL_V0_ROUTE_NVIF 0x00
> > #define NVIF_IOCTL_V0_ROUTE_HIDDEN 0xff
> > - __u8 route;
> > - __u64 token;
> > - __u64 object;
> > + __u8 route;
> > + __u64 token;
> > + __u64 object;
> > + );
> > __u8 data[]; /* ioctl data (below) */
> > };
> > +static_assert(offsetof(struct nvif_ioctl_v0, data) == sizeof(struct nvif_ioctl_v0_hdr),
> > + "struct member likely outside of struct_group()");
> >
> > struct nvif_ioctl_sclass_v0 {
> > /* nvif_ioctl ... */
> > @@ -51,12 +56,17 @@ struct nvif_ioctl_del {
> > };
> >
> > struct nvif_ioctl_mthd_v0 {
> > - /* nvif_ioctl ... */
> > - __u8 version;
> > - __u8 method;
> > - __u8 pad02[6];
> > + /* New members MUST be added within the struct_group() macro below. */
> > + struct_group_tagged(nvif_ioctl_mthd_v0_hdr, __hdr,
> > + /* nvif_ioctl ... */
> > + __u8 version;
> > + __u8 method;
> > + __u8 pad02[6];
> > + );
> > __u8 data[]; /* method data (class.h) */
> > };
> > +static_assert(offsetof(struct nvif_ioctl_mthd_v0, data) == sizeof(struct nvif_ioctl_mthd_v0_hdr),
> > + "struct member likely outside of struct_group()");
> >
> > struct nvif_ioctl_map_v0 {
> > /* nvif_ioctl ... */
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
> > index b4da82ddbb6b..fc64c3d3169e 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> > @@ -79,8 +79,8 @@ struct nouveau_svm {
> > #define SVM_ERR(s,f,a...) NV_WARN((s)->drm, "svm: "f"\n", ##a)
> >
> > struct nouveau_pfnmap_args {
> > - struct nvif_ioctl_v0 i;
> > - struct nvif_ioctl_mthd_v0 m;
> > + struct nvif_ioctl_v0_hdr i;
> > + struct nvif_ioctl_mthd_v0_hdr m;
> > struct nvif_vmm_pfnmap_v0 p;
> > };
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvif/object.c b/drivers/gpu/drm/nouveau/nvif/object.c
> > index 0b87278ac0f8..70af63d70976 100644
> > --- a/drivers/gpu/drm/nouveau/nvif/object.c
> > +++ b/drivers/gpu/drm/nouveau/nvif/object.c
> > @@ -57,7 +57,7 @@ int
> > nvif_object_sclass_get(struct nvif_object *object, struct nvif_sclass **psclass)
> > {
> > struct {
> > - struct nvif_ioctl_v0 ioctl;
> > + struct nvif_ioctl_v0_hdr ioctl;
> > struct nvif_ioctl_sclass_v0 sclass;
> > } *args = NULL;
> > int ret, cnt = 0, i;
> > @@ -101,7 +101,7 @@ int
> > nvif_object_mthd(struct nvif_object *object, u32 mthd, void *data, u32 size)
> > {
> > struct {
> > - struct nvif_ioctl_v0 ioctl;
> > + struct nvif_ioctl_v0_hdr ioctl;
> > struct nvif_ioctl_mthd_v0 mthd;
> > } *args;
> > u32 args_size;
> > @@ -135,7 +135,7 @@ void
> > nvif_object_unmap_handle(struct nvif_object *object)
> > {
> > struct {
> > - struct nvif_ioctl_v0 ioctl;
> > + struct nvif_ioctl_v0_hdr ioctl;
> > struct nvif_ioctl_unmap unmap;
> > } args = {
> > .ioctl.type = NVIF_IOCTL_V0_UNMAP,
> > @@ -149,7 +149,7 @@ nvif_object_map_handle(struct nvif_object *object, void *argv, u32 argc,
> > u64 *handle, u64 *length)
> > {
> > struct {
> > - struct nvif_ioctl_v0 ioctl;
> > + struct nvif_ioctl_v0_hdr ioctl;
> > struct nvif_ioctl_map_v0 map;
> > } *args;
> > u32 argn = sizeof(*args) + argc;
> > @@ -211,7 +211,7 @@ void
> > nvif_object_dtor(struct nvif_object *object)
> > {
> > struct {
> > - struct nvif_ioctl_v0 ioctl;
> > + struct nvif_ioctl_v0_hdr ioctl;
> > struct nvif_ioctl_del del;
> > } args = {
> > .ioctl.type = NVIF_IOCTL_V0_DEL,
> > @@ -230,7 +230,7 @@ nvif_object_ctor(struct nvif_object *parent, const char *name, u32 handle,
> > s32 oclass, void *data, u32 size, struct nvif_object *object)
> > {
> > struct {
> > - struct nvif_ioctl_v0 ioctl;
> > + struct nvif_ioctl_v0_hdr ioctl;
> > struct nvif_ioctl_new_v0 new;
> > } *args;
> > int ret = 0;
> > --
> > 2.43.0
> >
