[U-Boot] [PATCH] arm: Disable the strict alignment of data on armv7

[Michal Simek]

Dear Wolfgang,

On 9.3.2018 13:02, Wolfgang Denk wrote:
> Dear Michal,
> 
> In message <fc0a315b-4e51-a009-05e3-1a140bf82155 at xilinx.com> you wrote:
>>
>> We are not seeing any issue from u-boot code itself and I can believe
>> that structures and accesses are aligned properly.
>>
>> The initial reason was that u-boot allows you to run for example command
>> md 1 and it ends up in panic. Which is something what should be possible
>> to run or code should check that architecture has alignment restriction.
> 
> I'm not sure this needs any "fixing".  Actually I use this on a
> regular base as a simple means to find out if an architecutre allos
> unaligned accesses...  U-Boot is just a boot loader.  It has no
> "normal users" - users are usually developers who know what they are
> doing.  I think this is an area where this old quote applies nicely:
> 
> "UNIX was not designed to stop you from doing stupid things,  because
> that would also stop you from doing clever things."       - Doug Gwyn

I understand your opinion and no problem with it.


>> Definitely panic is not proper reaction on command like this.
> 
> Why not?  Yes, we can blow up the code to handle this in a better
> way, and add some kB of stricgs for nice and user-friendy warnings
> not to do this or that.  But is it really worth the effort?
> I consider this a pretty useful feature and would be sad if it went
> missing. If someone runs into it by accident, he learns a lesson -
> but htere is no real damage done.  Or am I missing anything?

It is not that the patch is removing or changing current behavior.
Default option is to use strict alignment and have an option to change
this behavior.


>> It means we have two options:
>> 1. enable non aligned accesses
>> 2. fix code/commands which enables to pass these non aligned addresses
>> which ends up in panic
> 
> My vorte is for option 3:
> 
> 3. Leave as is.  It's a boot loader, and like other sharp tools it
> can be dangerous if improperly used.

Sure that's also an option but it seems to me weird that by default
armv7 has this requirement and a53 not.


>>> But I guess they still come under a (severe?) performance penalty?
>>
>> It is really a question where that penalty is coming from and when it
>> can happen.
> 
> It comes from the memory and cache architecture.  Just assume an
> unaligned read of 2 bytes that happen to cross a cacheline boundary.
> This access causes a double cache miss; in the worst case this
> causes two cache lines to be flushed to memory and then two
> cachelines to be written from memory.  Instead of just reading two
> bytes, we write 128 bytes and read 128 bytes.  Yes, this is a
> pathological case, but you get the idea.

If this is WB case that write shouldn't be there but you are right that
this could happen.

>> But again the code is align and there is not an issue in code that's why
>> this penalty is not happening. And non align accesses are performed when
>> user asks for this.
> 
> Why not tell the user: don't perform unaligend accesses on this
> platform then?  I can't see the situation where he really _needs_ to
> do something which is not natively supported by his hardware?

It is supported by hardware. U-Boot/SW just doing this configuration not
to allow it.

>>> Also, I wonder if ARM still maintains atomicity for unaligned reads/
>>> writes ? [IIRC, x86 doesn't, while it's guaranteed for aligned
>>> accesses.]
>>
>> Don't know.
> 
> Well, I'm willing to bet a few beer this doesn't work at least in
> pathological cases like example of crossing cache lines above.

:-)

Thanks,
Michal