On Fri, Jun 15, 2012 at 11:02:55PM +0200, Geert Uytterhoeven wrote:
> Hi Greg,
>
> On Fri, Jun 15, 2012 at 10:10 PM, Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Fri, Jun 15, 2012 at 07:42:08PM +0200, Geert Uytterhoeven wrote:
> >> commit 81df2d594340dcb6d1a02191976be88a1ca8120c ("USB: allow match
> >> on bInterfaceNumber") added a byte to the interior of struct usb_device_id,
> >> enabling implicit padding:
> >>
> >> --- a/include/linux/mod_devicetable.h
> >> +++ b/include/linux/mod_devicetable.h
> >> @@ -115,6 +118,9 @@ struct usb_device_id {
> >> __u8 bInterfaceSubClass;
> >> __u8 bInterfaceProtocol;
> >>
> >> + /* Used for vendor-specific interface matches */
> >> + __u8 bInterfaceNumber;
> >> +
> >> /* not matched against */
> >> kernel_ulong_t driver_info;
> >> };
> >>
> >> On m68k, this causes failures like:
> >>
> >> | FATAL: drivers/gpu/drm/udl/udl: sizeof(struct usb_device_id)=24 is
> >> not a modulo of the size of section __mod_usb_device_table=44.
> >> | Fix definition of struct usb_device_id in mod_devicetable.h
> >>
> >> M68k is special in that it uses 2 for the alignment of 32-bit entities, hence
> >> sizeof(struct usb_device_id) = 22.
> >>
> >> However, when cross-compiling on amd64, sizeof(struct usb_device_id) = 24
> >> in scripts/mod/file2alias.c.
> >
> > I don't really understand, what is the problem here? The fact that we
> > added one byte to the structure? And now something breaks? How?
> >
> > What can we do to resolve it?
>
> As "kernel_ulong_t driver_info" is no longer naturally aligned, the
> compiler will
> add implicit padding. But the padding depends on the architecture.
Ah, so we were "lucky" before, nice.
> It can be fixed by adding explicit padding. Probably it should be padded by
> 7 bytes (not 3), as kernel_ulong_t may require 8-byte alignment on some 64-bit
> platforms. Or by an explicit alignment attribute.
>
> See also
> * commit 8175fe2dda1c93a9c596921c8ed4a0b4baccdefe ("HID: fix
> hid_device_id for cross compiling")
> * commit 7492d4a416d68ab4bd254b36ffcc4e0138daa8ff ("sdio: fix module
> device table definition for m68k")
> * commit 9e2d3cd34a159948dc753a14573e16bffc04dba8 ("[PATCH]
> mod_devicetable.h fixes")
So would the patch below fix this? It should force alignment of the
driver_data field, which is all you want here, right?
> Still, there's a bug in file2alias (which is compiled by the host
> compiler), in that
> it may use different padding than the target platform when cross-compiling.
That's not good, but outside of this specific issue, right? Have we
just been fortunate it hasn't really hit us yet?
thanks,
greg k-h
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 7771d45..6955045 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -122,7 +122,8 @@ struct usb_device_id {
__u8 bInterfaceNumber;
/* not matched against */
- kernel_ulong_t driver_info;
+ kernel_ulong_t driver_info
+ __attribute__((aligned(sizeof(kernel_ulong_t))));
};
/* Some useful macros to use to create struct usb_device_id */
--
To unsubscribe from this list: send the line "unsubscribe linux-next" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
