Dear Albert ARIBAUD,
> Hi Marek,
>
> On Fri, 31 Aug 2012 18:15:30 +0200, Marek Vasut <marex(a)denx.de> wrote:
> > Dear Albert ARIBAUD,
> >
> > > Hi Marek,
> > >
> > > On Fri, 31 Aug 2012 01:29:05 +0200, Marek Vasut <marex(a)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).
>
> As a compiler bug, this can happen indeed. And we had our share of
> compiler alignment bugs ourselves. But a conforming compiler should not
> reorder fields.
I'm no compiler expert ... so I won't argue here
> > > 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:
> Someone please hold Wolfgang back while I correct myself: "as the *ARM*
> 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.
>
> I think you are talking about lumping small-sized accesses together
> into a bigger access possibly aligned.
This is exactly what I mean.
> If I am correct, then I don't
> think this is related to misaligned accesses.
Why won't it be? Such access can in the end turn out to be aligned, therefore
leveraging all the penalty.
> If I am not correct, can
> you please detail what you meant?
>
> > 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.
>
> I personally would let the compiler do it too, but I prefer it to be
> clearly indicated to the reader of the code when an access is
> known to be misaligned.
I'd enable enable the Alignment trapping in the CPU and die on an unaligned
access at runtime -- to indicate the user that he should fix his bloody
compiler.
> > $ 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
>
> Amicalement,
Best regards,
Marek Vasut
