[PATCH] drm/tegra: Remove gratuitous pad field

[PATCH] drm/tegra: Remove gratuitous pad field

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The version of the drm_tegra_submit structure that was merged all the
way back in 3.10 contains a pad field that was originally intended to
properly pad the following __u64 field. Unfortunately it seems like a
different field was dropped during review that caused this padding to
become unnecessary, but the pad field wasn't removed at that time.

One possible side-effect of this is that since the __u64 following the
pad is now no longer properly aligned, the compiler may (or may not)
introduce padding itself, which results in no predictable ABI.

Rectify this by removing the pad field so that all fields are again
naturally aligned. Technically this is breaking existing userspace ABI,
but given that there aren't any (released) userspace drivers that make
use of this yet, the fallout should be minimal.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 include/uapi/drm/tegra_drm.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
index b042b48495d9..b75482112428 100644
--- a/include/uapi/drm/tegra_drm.h
+++ b/include/uapi/drm/tegra_drm.h
@@ -120,7 +120,6 @@ struct drm_tegra_submit {
 	__u32 num_waitchks;
 	__u32 waitchk_mask;
 	__u32 timeout;
-	__u32 pad;
 	__u64 syncpts;
 	__u64 cmdbufs;
 	__u64 relocs;
-- 
1.9.1

Re: [PATCH] drm/tegra: Remove gratuitous pad field

[Daniel Vetter]

cc: stable to make sure that even once you release userspace you won't
get angry bug reports?
-Daniel

On Wed, Apr 9, 2014 at 2:39 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The version of the drm_tegra_submit structure that was merged all the
> way back in 3.10 contains a pad field that was originally intended to
> properly pad the following __u64 field. Unfortunately it seems like a
> different field was dropped during review that caused this padding to
> become unnecessary, but the pad field wasn't removed at that time.
>
> One possible side-effect of this is that since the __u64 following the
> pad is now no longer properly aligned, the compiler may (or may not)
> introduce padding itself, which results in no predictable ABI.
>
> Rectify this by removing the pad field so that all fields are again
> naturally aligned. Technically this is breaking existing userspace ABI,
> but given that there aren't any (released) userspace drivers that make
> use of this yet, the fallout should be minimal.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  include/uapi/drm/tegra_drm.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
> index b042b48495d9..b75482112428 100644
> --- a/include/uapi/drm/tegra_drm.h
> +++ b/include/uapi/drm/tegra_drm.h
> @@ -120,7 +120,6 @@ struct drm_tegra_submit {
>         __u32 num_waitchks;
>         __u32 waitchk_mask;
>         __u32 timeout;
> -       __u32 pad;
>         __u64 syncpts;
>         __u64 cmdbufs;
>         __u64 relocs;
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/tegra: Remove gratuitous pad field

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2259 bytes --]

On Wed, Apr 09, 2014 at 02:44:42PM +0200, Daniel Vetter wrote:
> cc: stable to make sure that even once you release userspace you won't
> get angry bug reports?

Yes, good point. I was just pondering whether doing that would be smart
or not. But I came to the same conclusion.

Thierry

> -Daniel
> 
> On Wed, Apr 9, 2014 at 2:39 PM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > The version of the drm_tegra_submit structure that was merged all the
> > way back in 3.10 contains a pad field that was originally intended to
> > properly pad the following __u64 field. Unfortunately it seems like a
> > different field was dropped during review that caused this padding to
> > become unnecessary, but the pad field wasn't removed at that time.
> >
> > One possible side-effect of this is that since the __u64 following the
> > pad is now no longer properly aligned, the compiler may (or may not)
> > introduce padding itself, which results in no predictable ABI.
> >
> > Rectify this by removing the pad field so that all fields are again
> > naturally aligned. Technically this is breaking existing userspace ABI,
> > but given that there aren't any (released) userspace drivers that make
> > use of this yet, the fallout should be minimal.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  include/uapi/drm/tegra_drm.h | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
> > index b042b48495d9..b75482112428 100644
> > --- a/include/uapi/drm/tegra_drm.h
> > +++ b/include/uapi/drm/tegra_drm.h
> > @@ -120,7 +120,6 @@ struct drm_tegra_submit {
> >         __u32 num_waitchks;
> >         __u32 waitchk_mask;
> >         __u32 timeout;
> > -       __u32 pad;
> >         __u64 syncpts;
> >         __u64 cmdbufs;
> >         __u64 relocs;
> > --
> > 1.9.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

[-- Attachment #1.2: Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/tegra: Remove gratuitous pad field

[Terje Bergström]

On 09.04.2014 15:39, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> The version of the drm_tegra_submit structure that was merged all the
> way back in 3.10 contains a pad field that was originally intended to
> properly pad the following __u64 field. Unfortunately it seems like a
> different field was dropped during review that caused this padding to
> become unnecessary, but the pad field wasn't removed at that time.
> 
> One possible side-effect of this is that since the __u64 following the
> pad is now no longer properly aligned, the compiler may (or may not)
> introduce padding itself, which results in no predictable ABI.
> 
> Rectify this by removing the pad field so that all fields are again
> naturally aligned. Technically this is breaking existing userspace ABI,
> but given that there aren't any (released) userspace drivers that make
> use of this yet, the fallout should be minimal.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  include/uapi/drm/tegra_drm.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
> index b042b48495d9..b75482112428 100644
> --- a/include/uapi/drm/tegra_drm.h
> +++ b/include/uapi/drm/tegra_drm.h
> @@ -120,7 +120,6 @@ struct drm_tegra_submit {
>  	__u32 num_waitchks;
>  	__u32 waitchk_mask;
>  	__u32 timeout;
> -	__u32 pad;
>  	__u64 syncpts;
>  	__u64 cmdbufs;
>  	__u64 relocs;
> 

The padding is hilarious. I added it to remove the possibility for
compiler to add implicit padding, but it does completely the opposite.

If we'd care about binary compatibility, we could have also just added
__packed to the definition. But as this is in staging, I don't think
that's necessary.

Acked-By: tbergstrom@nvidia.com <tbergstrom@nvidia.com>


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------