[U-Boot] [PATCH] arm: enable unaligned access on ARMv7

[Aneesh V]

+Tom

Hi Lucas,

On 06/22/2012 04:47 AM, Lucas Stach wrote:
> Hi Albert,
>
> Am Freitag, den 22.06.2012, 13:16 +0200 schrieb Albert ARIBAUD:
>> Hi Lucas,
>>
>>>>> Linux in particular does reinitialize this state and I expect any
>>>>> reasonable OS to do so.
>>>>
>>>> Then what is the point of enabling it on U-Boot? Does it fix some
>>>> issue whereby some mis-aligned piece of data cannot be properly
>>>> aligned?
>>>>
>>> Yes, it fixes U-Boot USB on Tegra, when built with a recent toolchain.
>>> Fixing the alignment of some of the structures in the USB code should
>>> also be done, but this is a whole lot more invasive and requires some
>>> more thought, as the discussion about this on LKML shows. The issue
>>> doesn't show for older toolchains, as they by default emit code to
>>> work around unaligned accesses.
>>>
>>> This patch fixes all unaligned issues, that may appear with recent
>>> toolchains. We avoid having to instruct the toolchain to work around
>>> unaligned accesses and gain better performance in cases where it is
>>> needed.
>>
>> I am not too happy with enabling a lax behavior only to avoid an
>> issue which apparently is diagnosed and could / should be fixed at
>> its root. Can you point me to the relevant LKML thread
>> so that I get the details and understand what prevents fixing this by
>> 'simply' aligning the USB structures?
>
> I'm with you that the issue for this particular fault that I ran into
> should be fixed at it's root and I will do so as soon as I have enough
> time to do so, i.e. within the next three weeks.
> You can find a thread about this here:
> https://lkml.org/lkml/2011/4/27/278
> The problem here is that simply removing the packed attribute is not the
> right thing to do for structures that are used for accessing hardware
> registers. I have to read a bit more of the USB code to come up with the
> right thing to do for every structure there.
>
> But apart from this, we certainly have situations where we have
> unaligned accesses that are justified and could not be removed.
> Activating the aligned access hardware check is crippling a feature of
> the ARMv7 architecture that's apparently useful enough that all recent
> toolchains default to using it and not using compiler side workarounds.
> Furthermore setting the check bit doesn't even deactivate unaligned
> access (there is also a bit for this, which is forced to 1 by all v7
> implementations), but just traps the processor in case you care about
> such unaligned accesses. Linux for example only sets this check bit if
> you choose to install a trap handler for this.
>
> I cannot see how enabling a hardware feature can be seen as allowing of
> lax behaviour. As some of the USB structs are used to access hardware
> registers, we can not align every struct there. Our options are either:
> 1. instruct the toolchain to insert workaround code or
> 2. allow v7 hardware to do the unaligned access on it's own
> My comment about fixing the USB code applies only to part of the structs
> used there as some of them generate _unnecessary_ unaligned accesses,
> the issue that all unaligned accesses fail with current U-Boot built
> with a recent toolchain has to be fixed either way and is exactly what
> this patch does.

I think this issue was discussed before here(I haven't gone through all
the details of your problem, but it looks similar). And I think Tom
fixed this by wrapping the problematic accesses with get/set_unaligned().

Please look at this thread, especially starting from my post reporting
the OMAP4 regression:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/113347/

best regards,
Aneesh