Dear Albert ARIBAUD,
Hi Marek,
On Fri, 31 Aug 2012 01:29:05 +0200, Marek Vasut marex@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.
- 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).
- 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.
- 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