[U-Boot] [PATCH] usb: do explicit unaligned accesses

[Marek Vasut]

Dear Albert ARIBAUD,

> Hi Marek,
> 
> On Fri, 31 Aug 2012 01:29:05 +0200, Marek Vasut <marex at denx.de> wrote:
> > Dear Lucas Stach,
> > 
> > > usb_hub_descriptor has to be packed as it's used for
> > > communication with the device. Member wHubCharacteristics
> > > violates the natural alignment rules.
> > > 
> > > Use explicit unaligned access functions for this member.
> > > Fixes ARMv7 traping while using USB.
> > 
> > Shouldn't a properly configured compiler prevent such behavior?
> 
> This has been discussed before. As a summary:

Argh, I must have missed this.

> 1. A properly configured compiler can pad a structure so that the
> fields always start at an aligned address (assuming the structure base
> address is itself aligned). But that alters the structure, and here
> there must be no alterations to the structure.

There can be ... the compiler can even reorder the structures (that happened in 
Linux kernel, as I'm on a train now with shitty internet connection,  I can't 
find it ... search LWN for that please).

> 2. A properly (and differently) configured compiler can automatically
> generate native unaligned accesses to unaligned fields. This is
> acceptable on armv6+ architectures, has a performance penalty on earlier
> architectures, and does not necessarily work depending on the hardware
> configuration.

Certainly, agreed.

> 3. A properly (and differently yet) configured compiler can
> automatically generate non-native unaligned accesses to unaligned
> fields. This is acceptable on all architectures, has a performance
> penalty on pre-armV6 architectures for all misaligned accesses, whether
> voluntary or not.

Correct, I'd vote for this solution -- let the compiler handle such unaligned 
cases.

> The conclusion of the discussion is as follows -- or to be more exact,
> following this discussion, this is my stance as the U-Boot custodian:
> 
> i) All the code intended to run 'close to' U-Boot (i.e., U-Boot code
> itself and application code) is controlled enough that we should be
> able to know and limit which code requires misaligned access (such as
> here for this USB structure field);
> 
> ii) On some ARM architectures, and possibly some non-ARM architectures
> as well, native misaligned access incur a performance hit, and may
> even simply be impossible or forbidden by a hardware or system design
> decision.
> 
> iii) Thus, U-Boot should follow a strict policy of using native aligned
> accesses only, possibly enforcing misaligned native access prevention in
> hardware, and of explicitly emulating misaligned accesses when they
> cannot be avoided.

I do agree with i) and ii), but why not just let compiler handle the unaligned 
access for us? The compiler can optimize across the whole code, not only locally 
over one access, therefore it might be able to punt some of the unaligned 
accesses altogether if the code allows it. Besides, right now, the code is much 
more readable. So I really don't like adding some strange macros to force crazy 
aligned access if the compiler can do it for us and can do it better.

$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- ./MAKEALL m28evk
Configuring for m28evk board...
   text    data     bss     dec     hex filename
 415964    7688  288740  712392   adec8 ./u-boot
  11754     788      12   12554    310a ./spl/u-boot-spl

--------------------- SUMMARY ----------------------------
Boards compiled: 1
----------------------------------------------------------
$ patch -Np1 -i /tmp/\[PATCH\ v2\]\ usb_do\ explicit\ unaligned\ accesses.mbox 
patching file common/usb_hub.c
patching file drivers/usb/host/ehci-hcd.c
Hunk #2 succeeded at 867 with fuzz 1 (offset -10 lines).
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- ./MAKEALL m28evk
Configuring for m28evk board...
   text    data     bss     dec     hex filename
 415968    7688  288736  712392   adec8 ./u-boot
  11754     788      12   12554    310a ./spl/u-boot-spl

--------------------- SUMMARY ----------------------------
Boards compiled: 1
----------------------------------------------------------

Notice the text section grew a bit too, I dunno why, does anyone care enough to 
clarify please?

> Hope this clarifies.
> 
> Amicalement,

Best regards,
Marek Vasut