diff mbox

[U-Boot] arm: armv7: add compile option -mno-unaligned-access if available

Message ID 4FF16CF2.9090808@kmckk.co.jp
State Accepted
Commit 5eb497429ef065083d75ca0bbc61d421a17fcec1
Delegated to: Albert ARIBAUD
Headers show

Commit Message

Tetsuyuki Kobayashi July 2, 2012, 9:42 a.m. UTC
Recent compiler generates unaligned memory access in armv7 default.
But current U-Boot does not allow unaligned memory access, so it causes
data abort exception.
This patch add compile option "-mno-unaligned-access" if it is available.

Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
---
 arch/arm/cpu/armv7/config.mk |    2 ++
 1 file changed, 2 insertions(+)

-- 1.7.9.5

Comments

Måns Rullgård July 2, 2012, 9:53 a.m. UTC | #1
Tetsuyuki Kobayashi <koba@kmckk.co.jp> writes:

> Recent compiler generates unaligned memory access in armv7 default.
> But current U-Boot does not allow unaligned memory access, so it causes
> data abort exception.
> This patch add compile option "-mno-unaligned-access" if it is available.

Why not allow unaligned accesses instead?
Lucas Stach July 2, 2012, 3:16 p.m. UTC | #2
Am Montag, den 02.07.2012, 10:53 +0100 schrieb Måns Rullgård:
> Tetsuyuki Kobayashi <koba@kmckk.co.jp> writes:
> 
> > Recent compiler generates unaligned memory access in armv7 default.
> > But current U-Boot does not allow unaligned memory access, so it causes
> > data abort exception.
> > This patch add compile option "-mno-unaligned-access" if it is available.
> 
> Why not allow unaligned accesses instead?
> 
IMHO, our recent discussion showed that both ways are wrong.
"-mno-unaligned-access" works around misaligned data on the software
level, while allowing unaligned access does on the hardware level.

What we really want is no unaligned access in U-Boot at all. Just
because "-mno-unaligned-access" is the default on ARMv5, we should not
consider it a gold standard.

Thanks,
Lucas
Måns Rullgård July 2, 2012, 4:14 p.m. UTC | #3
Lucas Stach <dev@lynxeye.de> writes:

> Am Montag, den 02.07.2012, 10:53 +0100 schrieb Måns Rullgård:
>> Tetsuyuki Kobayashi <koba@kmckk.co.jp> writes:
>> 
>> > Recent compiler generates unaligned memory access in armv7 default.
>> > But current U-Boot does not allow unaligned memory access, so it causes
>> > data abort exception.
>> > This patch add compile option "-mno-unaligned-access" if it is available.
>> 
>> Why not allow unaligned accesses instead?
>> 
> IMHO, our recent discussion showed that both ways are wrong.
> "-mno-unaligned-access" works around misaligned data on the software
> level, while allowing unaligned access does on the hardware level.
>
> What we really want is no unaligned access in U-Boot at all. Just
> because "-mno-unaligned-access" is the default on ARMv5, we should not
> consider it a gold standard.

It's slightly more complicated than that.  Data can be misaligned for a
variety of reasons:

1. Errors in software.
2. Specified by a file format or communication protocol.
3. Deliberately misaligned by the compiler.

Misaligned data of type 1 should of course be fixed properly, not worked
around in any way.

Type 2 happens all the time, and has to be dealt with one way or
another.  If the hardware supports unaligned accesses, this is usually
faster than reading a byte at a time.

When targeting ARMv6 and later, recent gcc versions have started issuing
deliberate unaligned accesses where previously byte by byte accesses
would have been done.  This happens with "packed" structs and sometimes
to write multiple smaller values at once, typically when
zero-initialising things.  These unaligned accesses are *good*.  They
make code smaller and faster.

The real problem here is that u-boot is setting the strict alignment
checking flag, invalidating the assumption of the compiler that the
system allows unaligned accesses.  For ARMv5 and earlier, setting this
flag is usually advisable since it makes finding accidental unaligned
accesses much easier.

This was debated in the context of the kernel a while ago, ultimately
leading to strict alignment being disabled for ARMv6 and up [1].

[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=8428e84d42179c2a00f5f6450866e70d802d1d05
Tetsuyuki Kobayashi July 3, 2012, 7:10 a.m. UTC | #4
Hello, Måns
Thank you for summarizing.

I am not against you.
I'm OK either "Allow unaligned access in U-Boot setting" or
"Specify compiler not to generate unaligned memory access"
or other.
I just want to solve hung-up by unaligned access.
I follow custodian's decision.

(2012/07/03 1:14), Måns Rullgård wrote:
> Lucas Stach<dev@lynxeye.de>  writes:
>
>> Am Montag, den 02.07.2012, 10:53 +0100 schrieb Måns Rullgård:
>>> Tetsuyuki Kobayashi<koba@kmckk.co.jp>  writes:
>>>
>>>> Recent compiler generates unaligned memory access in armv7 default.
>>>> But current U-Boot does not allow unaligned memory access, so it causes
>>>> data abort exception.
>>>> This patch add compile option "-mno-unaligned-access" if it is available.
>>>
>>> Why not allow unaligned accesses instead?
>>>
>> IMHO, our recent discussion showed that both ways are wrong.
>> "-mno-unaligned-access" works around misaligned data on the software
>> level, while allowing unaligned access does on the hardware level.
>>
>> What we really want is no unaligned access in U-Boot at all. Just
>> because "-mno-unaligned-access" is the default on ARMv5, we should not
>> consider it a gold standard.
>
> It's slightly more complicated than that.  Data can be misaligned for a
> variety of reasons:
>
> 1. Errors in software.
> 2. Specified by a file format or communication protocol.
> 3. Deliberately misaligned by the compiler.
>
> Misaligned data of type 1 should of course be fixed properly, not worked
> around in any way.
>
> Type 2 happens all the time, and has to be dealt with one way or
> another.  If the hardware supports unaligned accesses, this is usually
> faster than reading a byte at a time.
>
> When targeting ARMv6 and later, recent gcc versions have started issuing
> deliberate unaligned accesses where previously byte by byte accesses
> would have been done.  This happens with "packed" structs and sometimes
> to write multiple smaller values at once, typically when
> zero-initialising things.  These unaligned accesses are *good*.  They
> make code smaller and faster.
>
> The real problem here is that u-boot is setting the strict alignment
> checking flag, invalidating the assumption of the compiler that the
> system allows unaligned accesses.  For ARMv5 and earlier, setting this
> flag is usually advisable since it makes finding accidental unaligned
> accesses much easier.
>
> This was debated in the context of the kernel a while ago, ultimately
> leading to strict alignment being disabled for ARMv6 and up [1].
>
> [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=8428e84d42179c2a00f5f6450866e70d802d1d05
>
Albert ARIBAUD July 5, 2012, 7:57 a.m. UTC | #5
Hi Måns,

On Mon, 02 Jul 2012 17:14:40 +0100, Måns Rullgård <mans@mansr.com>
wrote:

> > IMHO, our recent discussion showed that both ways are wrong.
> > "-mno-unaligned-access" works around misaligned data on the software
> > level, while allowing unaligned access does on the hardware level.
> >
> > What we really want is no unaligned access in U-Boot at all. Just
> > because "-mno-unaligned-access" is the default on ARMv5, we should
> > not consider it a gold standard.
> 
> It's slightly more complicated than that.  Data can be misaligned for
> a variety of reasons:
> 
> 1. Errors in software.
> 2. Specified by a file format or communication protocol.
> 3. Deliberately misaligned by the compiler.
> 
> Misaligned data of type 1 should of course be fixed properly, not
> worked around in any way.

Agreed.

> Type 2 happens all the time, and has to be dealt with one way or
> another.  If the hardware supports unaligned accesses, this is usually
> faster than reading a byte at a time.

Sorry but I don't accept a systemic change as a good solution to a
specific issue. The best fix is "don't accept using a protocol or file
format which was not designed to avoid native misaligned accesses", and
the second best fix is "if you really must deal with a protocol or file
format which causes native alignment issues, then the fix should only
affect the protocol or file format's issue, not with your system which
is not the root cause".

But to be honest, I haven't seen such badly a designed protocol or file
format; their designers tend to make sure that (assuming the start of a
protocol or file 'content' (frame, record, etc) is aligned, then all
fields in it are aligned as well. Can someone point me to an example of
a protocol or file format which does present such a misalignment risk ?

> When targeting ARMv6 and later, recent gcc versions have started
> issuing deliberate unaligned accesses where previously byte by byte
> accesses would have been done.

Wrongly formulated: "mis-aligning deliberately" seems to imply that
GCC did away with the possibility of properly aligning, which they of
course did not. What they did is switch its default behavior regarding
native misaligned accesses from forbidding to allowing them. The change
here is of policy, a change which we may or may not want to follow. 

> This happens with "packed" structs
> and sometimes to write multiple smaller values at once, typically when
> zero-initialising things.  These unaligned accesses are *good*.  They
> make code smaller and faster.

Again, "good" is a policy, or subjective, statement, not an absolute.
Just as "good" is correctly aligning data in the first place (to begin
with, in protocols and file formats) and keeping the compiler's native
misaligned access policy set to "do not allow".

> The real problem here is that u-boot is setting the strict alignment
> checking flag, invalidating the assumption of the compiler that the
> system allows unaligned accesses.  For ARMv5 and earlier, setting this
> flag is usually advisable since it makes finding accidental unaligned
> accesses much easier.

Just as it is for ARMv6 and up. Again, just because the compiler folks
changed their default policy does not mean we should change ours,
which is not based on the same goals. 

> This was debated in the context of the kernel a while ago, ultimately
> leading to strict alignment being disabled for ARMv6 and up [1].
> 
> [1]
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=8428e84d42179c2a00f5f6450866e70d802d1d05

I'd rather have a link to the rationale than to the commit, but anyway,
the kernel folks' decision is theirs and does not necessarily apply to
us. I have mailed Catalin Marinas off-list to get details on the
rationale and context of the kernel patch; I will report conclusions
here.

Meanwhile, our policy regarding misalignment accesses is to only allow
them when externally required (by something other than a bad design).
Someone please show me such an external requirement for U-Boot, and I
will reconsider -- and then, other arch custodians may have a problem
with that too.

Regarding the origin of this patch, i.e. a mis-alignment of USB fields,
and unless U-Boot USB folks say otherwise, this issue should be fixed
by aligning said fields properly.

Amicalement,
Gary Thomas July 12, 2012, 3:12 p.m. UTC | #6
On 2012-07-02 03:42, Tetsuyuki Kobayashi wrote:
> Recent compiler generates unaligned memory access in armv7 default.
> But current U-Boot does not allow unaligned memory access, so it causes
> data abort exception.
> This patch add compile option "-mno-unaligned-access" if it is available.
>
> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> ---
>   arch/arm/cpu/armv7/config.mk |    2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/cpu/armv7/config.mk b/arch/arm/cpu/armv7/config.mk
> index 5407cb6..560c084 100644
> --- a/arch/arm/cpu/armv7/config.mk
> +++ b/arch/arm/cpu/armv7/config.mk
> @@ -26,6 +26,8 @@ PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float
>   # supported by more tool-chains
>   PF_CPPFLAGS_ARMV7 := $(call cc-option, -march=armv7-a, -march=armv5)
>   PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARMV7)
> +PF_CPPFLAGS_NO_UNALIGNED := $(call cc-option, -mno-unaligned-access,)
> +PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_NO_UNALIGNED)
>
>   # =========================================================================
>   #
> -- 1.7.9.5

Tested-by: Gary Thomas <gary@mlbassoc.com>
Albert ARIBAUD July 18, 2012, 9:37 p.m. UTC | #7
on Thu, 5 Jul 2012 09:57:19 +0200,
Albert ARIBAUD <albert.u.boot@aribaud.net> wrote :

> Hi Måns,
> 
> On Mon, 02 Jul 2012 17:14:40 +0100, Måns Rullgård <mans@mansr.com>
> wrote:
> 
> > > IMHO, our recent discussion showed that both ways are wrong.
> > > "-mno-unaligned-access" works around misaligned data on the software
> > > level, while allowing unaligned access does on the hardware level.
> > >
> > > What we really want is no unaligned access in U-Boot at all. Just
> > > because "-mno-unaligned-access" is the default on ARMv5, we should
> > > not consider it a gold standard.
> > 
> > It's slightly more complicated than that.  Data can be misaligned for
> > a variety of reasons:
> > 
> > 1. Errors in software.
> > 2. Specified by a file format or communication protocol.
> > 3. Deliberately misaligned by the compiler.
> > 
> > Misaligned data of type 1 should of course be fixed properly, not
> > worked around in any way.
> 
> Agreed.
> 
> > Type 2 happens all the time, and has to be dealt with one way or
> > another.  If the hardware supports unaligned accesses, this is usually
> > faster than reading a byte at a time.
> 
> Sorry but I don't accept a systemic change as a good solution to a
> specific issue. The best fix is "don't accept using a protocol or file
> format which was not designed to avoid native misaligned accesses", and
> the second best fix is "if you really must deal with a protocol or file
> format which causes native alignment issues, then the fix should only
> affect the protocol or file format's issue, not with your system which
> is not the root cause".
> 
> But to be honest, I haven't seen such badly a designed protocol or file
> format; their designers tend to make sure that (assuming the start of a
> protocol or file 'content' (frame, record, etc) is aligned, then all
> fields in it are aligned as well. Can someone point me to an example of
> a protocol or file format which does present such a misalignment risk ?
> 
> > When targeting ARMv6 and later, recent gcc versions have started
> > issuing deliberate unaligned accesses where previously byte by byte
> > accesses would have been done.
> 
> Wrongly formulated: "mis-aligning deliberately" seems to imply that
> GCC did away with the possibility of properly aligning, which they of
> course did not. What they did is switch its default behavior regarding
> native misaligned accesses from forbidding to allowing them. The change
> here is of policy, a change which we may or may not want to follow. 
> 
> > This happens with "packed" structs
> > and sometimes to write multiple smaller values at once, typically when
> > zero-initialising things.  These unaligned accesses are *good*.  They
> > make code smaller and faster.
> 
> Again, "good" is a policy, or subjective, statement, not an absolute.
> Just as "good" is correctly aligning data in the first place (to begin
> with, in protocols and file formats) and keeping the compiler's native
> misaligned access policy set to "do not allow".
> 
> > The real problem here is that u-boot is setting the strict alignment
> > checking flag, invalidating the assumption of the compiler that the
> > system allows unaligned accesses.  For ARMv5 and earlier, setting this
> > flag is usually advisable since it makes finding accidental unaligned
> > accesses much easier.
> 
> Just as it is for ARMv6 and up. Again, just because the compiler folks
> changed their default policy does not mean we should change ours,
> which is not based on the same goals. 
> 
> > This was debated in the context of the kernel a while ago, ultimately
> > leading to strict alignment being disabled for ARMv6 and up [1].
> > 
> > [1]
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=8428e84d42179c2a00f5f6450866e70d802d1d05
> 
> I'd rather have a link to the rationale than to the commit, but anyway,
> the kernel folks' decision is theirs and does not necessarily apply to
> us. I have mailed Catalin Marinas off-list to get details on the
> rationale and context of the kernel patch; I will report conclusions
> here.
> 
> Meanwhile, our policy regarding misalignment accesses is to only allow
> them when externally required (by something other than a bad design).
> Someone please show me such an external requirement for U-Boot, and I
> will reconsider -- and then, other arch custodians may have a problem
> with that too.
> 
> Regarding the origin of this patch, i.e. a mis-alignment of USB fields,
> and unless U-Boot USB folks say otherwise, this issue should be fixed
> by aligning said fields properly.

We are nearing the release, and we obviously won't have misalignments
fixed and tested in time for it.

So I suspect that if I want the ARM U-Boot release to work I'll have to
allow this patch in my master branch to be pulled in for 12.07, so that
the compiler keeps behaving as it did before gcc changed the default.

... but only for the upcoming release, i.e. I will revert the patch in
my 'next' branch, which will apply right after 12.07 is out. Therefore,
before next release, misalignments will have to be fixed at the root.

Amicalement,
Mike Frysinger July 19, 2012, 4:29 a.m. UTC | #8
On Monday 02 July 2012 12:14:40 Måns Rullgård wrote:
> It's slightly more complicated than that.  Data can be misaligned for a
> variety of reasons:
> 
> 1. Errors in software.
> 2. Specified by a file format or communication protocol.
> 3. Deliberately misaligned by the compiler.
> 
> Misaligned data of type 1 should of course be fixed properly, not worked
> around in any way.

it's also a reliability aspect.  people don't write bug free software, not bug 
free protocols, nor bug free compilers.  when misalignment does happen in the 
field, it's a hell of a lot better if the software continued to execute 
correctly rather than randomly triggered an exception.
-mike
Mike Frysinger July 19, 2012, 4:31 a.m. UTC | #9
On Thursday 05 July 2012 03:57:19 Albert ARIBAUD wrote:
> But to be honest, I haven't seen such badly a designed protocol or file
> format; their designers tend to make sure that (assuming the start of a
> protocol or file 'content' (frame, record, etc) is aligned, then all
> fields in it are aligned as well. Can someone point me to an example of
> a protocol or file format which does present such a misalignment risk ?

simply search the kernel for get_unaligned then.  there are plenty of examples 
in there.  granted, many apply to stacks that don't show up in u-boot (yet?) 
such as bluetooth, wireless, and irda, but i'm pretty sure TCP/IPv4 has a few 
edge cases too.
-mike
Albert ARIBAUD July 19, 2012, 6:28 a.m. UTC | #10
Hi Mike,

On Thu, 19 Jul 2012 00:29:23 -0400, Mike Frysinger <vapier@gentoo.org> wrote:
> On Monday 02 July 2012 12:14:40 Måns Rullgård wrote:
> > It's slightly more complicated than that.  Data can be misaligned for a
> > variety of reasons:
> > 
> > 1. Errors in software.
> > 2. Specified by a file format or communication protocol.
> > 3. Deliberately misaligned by the compiler.
> > 
> > Misaligned data of type 1 should of course be fixed properly, not worked
> > around in any way.
> 
> it's also a reliability aspect.  people don't write bug free software, not bug 
> free protocols, nor bug free compilers.  when misalignment does happen in the 
> field, it's a hell of a lot better if the software continued to execute 
> correctly rather than randomly triggered an exception.
> -mike

Nitpick: this is robustness, not reliability.

That being said, yes, this robustness is desirable when you do not control all
of the SW running on the product; Linux, for instance, will have to execute
processes which were built with any old (or new) compiler settings, thus the
Linux folks have to make sure the kernel won't fail running those.

But the only uncontrolled SW U-Boot runs is its payload -- typically the
kernel image -- which are usually very cautious in what they assume they can
do, thus are unlikely to perform unaligned accesses.

(pasting your other comment)

> simply search the kernel for get_unaligned then.  there are plenty of examples 
> in there.  granted, many apply to stacks that don't show up in u-boot (yet?) 
> such as bluetooth, wireless, and irda, but i'm pretty sure TCP/IPv4 has a few 
> edge cases too.

I'll have a look, if only to lament that protocol are not what they used to be
in the old days. :)

Anyway: as I said: performing *controlled* unaligned accesses for external reasons
other than bugs is fine with me. Having our own get_unaligned() in such places
would be fine with me.
 
Amicalement,
Mike Frysinger July 19, 2012, 2:27 p.m. UTC | #11
On Thursday 19 July 2012 02:28:05 Albert ARIBAUD wrote:
> On Thu, 19 Jul 2012 00:29:23 -0400, Mike Frysinger wrote:
> > On Monday 02 July 2012 12:14:40 Måns Rullgård wrote:
> > > It's slightly more complicated than that.  Data can be misaligned for a
> > > variety of reasons:
> > > 
> > > 1. Errors in software.
> > > 2. Specified by a file format or communication protocol.
> > > 3. Deliberately misaligned by the compiler.
> > > 
> > > Misaligned data of type 1 should of course be fixed properly, not
> > > worked around in any way.
> > 
> > it's also a reliability aspect.  people don't write bug free software,
> > not bug free protocols, nor bug free compilers.  when misalignment does
> > happen in the field, it's a hell of a lot better if the software
> > continued to execute correctly rather than randomly triggered an
> > exception.
> 
> Nitpick: this is robustness, not reliability.

useless pedantry: by increasing robustness, the system is more reliable

> That being said, yes, this robustness is desirable when you do not control
> all of the SW running on the product; Linux, for instance, will have to
> execute processes which were built with any old (or new) compiler
> settings, thus the Linux folks have to make sure the kernel won't fail
> running those.
> 
> But the only uncontrolled SW U-Boot runs is its payload -- typically the
> kernel image -- which are usually very cautious in what they assume they
> can do, thus are unlikely to perform unaligned accesses.

it isn't just that.  there is no way you can guarantee both the linux kernel 
and u-boot code bases themselves are perfect.  in fact, it's even worse when 
these are the ones that get tripped up because it means your system 
resets/hardlocks/kills a kitten.  when doing driver development under the 
linux kernel, we would come across parts of core stacks that lacked alignment 
checking and would panic the system.  sometimes it would always panic, other 
times it depended on factors that made life worse: the compiler version (newer 
ones always like to pack/optimize better), the actual data stream, or the 
execution paths.

> > simply search the kernel for get_unaligned then.  there are plenty of
> > examples in there.  granted, many apply to stacks that don't show up in
> > u-boot (yet?) such as bluetooth, wireless, and irda, but i'm pretty sure
> > TCP/IPv4 has a few edge cases too.
> 
> I'll have a look, if only to lament that protocol are not what they used to
> be in the old days. :)
> 
> Anyway: as I said: performing *controlled* unaligned accesses for external
> reasons other than bugs is fine with me. Having our own get_unaligned() in
> such places would be fine with me.

i have no problem adding put/get_unaligned() to all the right places.  that 
makes perfect sense.  but, as an orthogonal issue wrt ARMv7, i don't see any 
problem enabling hardware functionality: it increases robustness (:P), shrinks 
the code base (all the get/put unaligned macros expand into a single memory 
access as they no longer have to do alignment fixups in software), and speeds 
up the runtime (a single unaligned memory access is always faster than address 
masking/multiple loads/bit shifting/etc... -- obviously this ignores 
multimedia type code that does alignment adjustment at the start, then lets of 
memory accesses, then another adjustment at the end, but that's not what we're 
talking about here).

if you want to tell people that if they found an unaligned access in code they 
must fix that, then great.  make them fix it.  then once that bug has been fixed, 
let's merge the purely optimization patch that allows the hardware to do 
unaligned accesses.
-mike
Albert ARIBAUD July 20, 2012, 7:12 a.m. UTC | #12
Hi Mike,

On Thu, 19 Jul 2012 10:27:07 -0400, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday 19 July 2012 02:28:05 Albert ARIBAUD wrote:
> > On Thu, 19 Jul 2012 00:29:23 -0400, Mike Frysinger wrote:
> > > On Monday 02 July 2012 12:14:40 Måns Rullgård wrote:
> > > > It's slightly more complicated than that.  Data can be misaligned for a
> > > > variety of reasons:
> > > > 
> > > > 1. Errors in software.
> > > > 2. Specified by a file format or communication protocol.
> > > > 3. Deliberately misaligned by the compiler.
> > > > 
> > > > Misaligned data of type 1 should of course be fixed properly, not
> > > > worked around in any way.
> > > 
> > > it's also a reliability aspect.  people don't write bug free software,
> > > not bug free protocols, nor bug free compilers.  when misalignment does
> > > happen in the field, it's a hell of a lot better if the software
> > > continued to execute correctly rather than randomly triggered an
> > > exception.
> > 
> > Nitpick: this is robustness, not reliability.
> 
> useless pedantry: by increasing robustness, the system is more reliable

Your description confirms that robustness and reliability are not equivalent,
thereby goes against your statement about pedantry... :)
 
> > That being said, yes, this robustness is desirable when you do not control
> > all of the SW running on the product; Linux, for instance, will have to
> > execute processes which were built with any old (or new) compiler
> > settings, thus the Linux folks have to make sure the kernel won't fail
> > running those.
> > 
> > But the only uncontrolled SW U-Boot runs is its payload -- typically the
> > kernel image -- which are usually very cautious in what they assume they
> > can do, thus are unlikely to perform unaligned accesses.
> 
> it isn't just that.  there is no way you can guarantee both the linux kernel 
> and u-boot code bases themselves are perfect.  in fact, it's even worse when 
> these are the ones that get tripped up because it means your system 
> resets/hardlocks/kills a kitten.  when doing driver development under the 
> linux kernel, we would come across parts of core stacks that lacked alignment 
> checking and would panic the system.  sometimes it would always panic, other 
> times it depended on factors that made life worse: the compiler version (newer 
> ones always like to pack/optimize better), the actual data stream, or the 
> execution paths.

Correct; here I was considering the requirements / operating conditions for both
projects, I was not considering development issues -- bugs during development
happen (morethan they do in the field, hopefully).

Do you mean you'd like to catch misalignments as early as possible, and would
like to have both -munaligned-access and A=1 during dev?
 
> > > simply search the kernel for get_unaligned then.  there are plenty of
> > > examples in there.  granted, many apply to stacks that don't show up in
> > > u-boot (yet?) such as bluetooth, wireless, and irda, but i'm pretty sure
> > > TCP/IPv4 has a few edge cases too.
> > 
> > I'll have a look, if only to lament that protocol are not what they used to
> > be in the old days. :)
> > 
> > Anyway: as I said: performing *controlled* unaligned accesses for external
> > reasons other than bugs is fine with me. Having our own get_unaligned() in
> > such places would be fine with me.
> 
> i have no problem adding put/get_unaligned() to all the right places.  that 
> makes perfect sense.  but, as an orthogonal issue wrt ARMv7, i don't see any 
> problem enabling hardware functionality: it increases robustness (:P), shrinks 
> the code base (all the get/put unaligned macros expand into a single memory 
> access as they no longer have to do alignment fixups in software), and speeds 
> up the runtime (a single unaligned memory access is always faster than address 
> masking/multiple loads/bit shifting/etc... -- obviously this ignores 
> multimedia type code that does alignment adjustment at the start, then lets of 
> memory accesses, then another adjustment at the end, but that's not what we're 
> talking about here).

I wouldn't care about the ARMv7 implementing explicit unaligned accesses with
native instructions if it didn't mean it won't catch unwanted unaligned accesses
any more, and thus such unalignments will be found in another context and will
take some more time to trace back.  

> if you want to tell people that if they found an unaligned access in code they 
> must fix that, then great.  make them fix it.  then once that bug has been fixed, 
> let's merge the purely optimization patch that allows the hardware to do 
> unaligned accesses.

My problem is that as long as people start configuring their HW and compiler to
not care about unaligned accesses, they *won't* find such accesses when accidental,
because nothing will tell them.

Here's my suggestion: when building U-Boot as usual, strict aligment policy is
enforced, i.e. -mno-unaligned-access and A=1, and for platforms that could benefit
from native unaligned accesses, a run-time warning is emitted on the console.
However, with a specific command 'PRODUCTION' line option added, the constraint
is relaxed, i.e. -munaligned-access and A=0, no run-time message, but a build
warning is emitted stating that this build is afoul of the U-Boot strict policy.

This way, you get the robustness you want as you can easily build an ARMv7-
efficient binary, and I get the one I want as the build used for development is
slightly less MCPS-efficient but will catch potential issues.

Comments welcome.

> -mike

Amicalement,
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/config.mk b/arch/arm/cpu/armv7/config.mk
index 5407cb6..560c084 100644
--- a/arch/arm/cpu/armv7/config.mk
+++ b/arch/arm/cpu/armv7/config.mk
@@ -26,6 +26,8 @@  PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float
 # supported by more tool-chains
 PF_CPPFLAGS_ARMV7 := $(call cc-option, -march=armv7-a, -march=armv5)
 PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARMV7)
+PF_CPPFLAGS_NO_UNALIGNED := $(call cc-option, -mno-unaligned-access,)
+PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_NO_UNALIGNED)
 
 # =========================================================================
 #