Re: [PATCH] drm/amdgpu/userq: fix struct drm_amdgpu_info_device padding for 32bit compile
Christian König <[email protected]>
| Newsgroups | org.freedesktop.lists.amd-gfx |
|---|---|
| Message-ID | <[email protected]> |
Hi guys, if it looks like this means that the UAPI is broken since: commit cf21e76a6005016474623a375ae2e90364a02168 Author: Marek Olšák <[email protected]> Date: Tue Sep 24 17:53:25 2019 -0400 drm/amdgpu: return tcc_disabled_mask to userspace UMDs need this for correct programming of harvested chips. Signed-off-by: Marek Olšák <[email protected]> Reviewed-by: Alex Deucher <[email protected]> Signed-off-by: Alex Deucher <[email protected]> It's just that nobody really noticed. *sigh* this is exactly one of the reasons why I don't wanted to extend the drm_amdgpu_info_device structure. If I'm not completely mistaken we have messed things up massively now. This currently works with 32bit kernels / 32bit userspace and 64bit kernel / 64bit userspace, but breaks as soon as you have 64bit kernel / 32bit userspace combination. We have now the choice to apply this patch and essentially break 32bit kernel and 32bit userspace combination or use some other hacky workaround. Alex and Yogesh let's discuss that on my Thursday meeting. Regards, Christian. On 8/21/26 05:51, Mohan Marimuthu, Yogesh wrote: > AMD General > > Hi Alex, > > With 32bit Kernel + patch and older userspace will be broken. It will need userspace to be recompiled. > The current situation is "64bit Kernel + 32bit userspace" already broken. It can be said that the current Kernel "struct drm_amdgpu_info_device" is broken. > > Though "64bit Kernel + 32bit userspace" is already broken but it has not been noticed until now, it was noticed with userq_ip_mask variable retuning zero. > > I think we have only two options, > > 1. > Probably "32bit Kernel + 32bit UMD" combination is not used and hence picking this patch might not trigger any compatibility issue. > 2. > Create AMDGPU_INFO_DEV_INFO_2 version 2 for DRM_AMDGPU_INFO ioctl to pass the fixed data structure, but this would keep the older structure that is already broken still broken. > > > Thank you, > Yogesh > > ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ > *From:* Alex Deucher <[email protected]> > *Sent:* Thursday, August 20, 2026 11:00 PM > *To:* Mohan Marimuthu, Yogesh <[email protected]> > *Cc:* [email protected] <[email protected]>; Deucher, Alexander <[email protected]>; Koenig, Christian <[email protected]>; Khatri, Sunil <[email protected]> > *Subject:* Re: [PATCH] drm/amdgpu/userq: fix struct drm_amdgpu_info_device padding for 32bit compile > > On Thu, Aug 20, 2026 at 11:28 AM Mohan Marimuthu, Yogesh > <[email protected]> wrote: >> >> AMD General >> >> Hi Alex, >> >> I am not changing the layout. >> >> For 64 bit gcc, default alignment for __u64 is 8bytes. >> For 32 bit gcc, default alignment for __u64 is 4bytes. >> >> For 32 bit gcc, I have manually padded 4bytes so that it is 8bytes aligned for __u64. This will make 64bit kernel compatible with 32bit mesa compiled using gcc. >> > > Won't this break 32 bit kernels? > > Alex > >> Thank you, >> Yogesh >> >> ________________________________ >> From: Alex Deucher <[email protected]> >> Sent: Thursday, August 20, 2026 8:51 PM >> To: Mohan Marimuthu, Yogesh <[email protected]> >> Cc: [email protected] <[email protected]>; Deucher, Alexander <[email protected]>; Koenig, Christian <[email protected]>; Khatri, Sunil <[email protected]> >> Subject: Re: [PATCH] drm/amdgpu/userq: fix struct drm_amdgpu_info_device padding for 32bit compile >> >> On Thu, Aug 20, 2026 at 11:18 AM Mohan Marimuthu, Yogesh >> <[email protected]> wrote: >> > >> > AMD General >> > >> > Hi Alex, >> > >> > In 32bit mesa, I did not get the correct userq_ip_mask value. >> > I checked for variable alignment and tcc_disabled_mask was not on 8 byte alignment. This patch fixes it. >> > >> > Can you please tell how it would break UAPI. What did I miss. >> >> You are changing the layout of the structure that existing apps expect. >> >> Alex >> >> > >> > Thank you, >> > Yogesh >> > >> > ________________________________ >> > From: Alex Deucher <[email protected]> >> > Sent: Thursday, August 20, 2026 7:23 PM >> > To: Mohan Marimuthu, Yogesh <[email protected]> >> > Cc: [email protected] <[email protected]>; Deucher, Alexander <[email protected]>; Koenig, Christian <[email protected]>; Khatri, Sunil <[email protected]> >> > Subject: Re: [PATCH] drm/amdgpu/userq: fix struct drm_amdgpu_info_device padding for 32bit compile >> > >> > On Thu, Aug 20, 2026 at 12:55 AM Yogesh Mohan Marimuthu >> > <[email protected]> wrote: >> > > >> > > need to pad before __u64 tcc_disabled_mask variable. >> > > >> > > Signed-off-by: Yogesh Mohan Marimuthu <[email protected]> >> > >> > This will break UAPI. >> > >> > Alex >> > >> > > --- >> > > include/uapi/drm/amdgpu_drm.h | 2 +- >> > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > >> > > diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h >> > > index 0a113529b..5beef3eaa 100644 >> > > --- a/include/uapi/drm/amdgpu_drm.h >> > > +++ b/include/uapi/drm/amdgpu_drm.h >> > > @@ -1517,6 +1517,7 @@ struct drm_amdgpu_info_device { >> > > __u64 high_va_max; >> > > /* gfx10 pa_sc_tile_steering_override */ >> > > __u32 pa_sc_tile_steering_override; >> > > + __u32 pad; >> > > /* disabled TCCs */ >> > > __u64 tcc_disabled_mask; >> > > __u64 min_engine_clock; >> > > @@ -1541,7 +1542,6 @@ struct drm_amdgpu_info_device { >> > > __u32 csa_alignment; >> > > /* Userq IP mask (1 << AMDGPU_HW_IP_*) */ >> > > __u32 userq_ip_mask; >> > > - __u32 pad; >> > > }; >> > > >> > > struct drm_amdgpu_info_hw_ip { >> > > -- >> > > 2.43.0 >> > >