[PATCH 1/2] ARM: added Kconfig option for -mno-unaligned compiler flag

[PATCH 1/2] ARM: added Kconfig option for -mno-unaligned compiler flag

[Enrico Scholz]

With recent gcc, unaligned access is enabled by default on ARMv6+
CPUs.  This can cause problems when bootloader is located e.g. in SRAM
where such an access is not supported.

Patch adds a Kconfig option were the default behavior can be overridden.

Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
---
 arch/arm/Kconfig  | 8 ++++++++
 arch/arm/Makefile | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 3afd885..53c36d1 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -200,6 +200,14 @@ config ARM_UNWIND
 	  the performance is not affected. Currently, this feature
 	  only works with EABI compilers. If unsure say Y.
 
+config ARM_NOUNALIGNED
+	bool "disable unaligned access"
+        help
+          With recent gcc, unaligned access is enabled by default on ARMv6+
+          CPUs.  This can cause problems when bootloader is located e.g. in
+          SRAM were such an access is not supported. Selection this option
+          sets the '-mno-unaligned-access' compiler flag.
+
 endmenu
 
 source common/Kconfig
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 8cef771..9544cca 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -39,6 +39,10 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
 CFLAGS_ABI	+=-funwind-tables
 endif
 
+ifeq ($(CONFIG_ARM_NOUNALIGNED),y)
+CFLAGS_ABI	+=$(call cc-option,-mno-unaligned-access)
+endif
+
 ifeq ($(CONFIG_THUMB2_BAREBOX),y)
 AFLAGS_AUTOIT	:=$(call as-option,-Wa$(comma)-mimplicit-it=always,-Wa$(comma)-mauto-it)
 AFLAGS_NOWARN	:=$(call as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W)
-- 
1.7.11.7


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] OMAP: disable unaligned access when building the IFT

[Enrico Scholz]

MLO is located in SRAM and OMAP4 does not allow unaligned access in
this area:

| :/ md -w 0x40300000+2
| 40300000: 9001                                               ..
| :/ md -w 0x40300001+2
| unable to handle paging request at address 0x40300001

Patch sets the ARM_NOUNALIGNED option introduced by a previous patch.

Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
---
 arch/arm/mach-omap/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-omap/Kconfig b/arch/arm/mach-omap/Kconfig
index 81f6127..445a35a 100644
--- a/arch/arm/mach-omap/Kconfig
+++ b/arch/arm/mach-omap/Kconfig
@@ -68,6 +68,7 @@ config OMAP_GPMC
 
 config OMAP_BUILD_IFT
 	prompt "build ift binary"
+	select ARM_NOUNALIGNED
 	bool
 
 config OMAP_BUILD_SPI
-- 
1.7.11.7


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] OMAP: disable unaligned access when building the IFT

[Sascha Hauer]

On Tue, Dec 04, 2012 at 01:02:49PM +0100, Enrico Scholz wrote:
> MLO is located in SRAM and OMAP4 does not allow unaligned access in
> this area:
> 
> | :/ md -w 0x40300000+2
> | 40300000: 9001                                               ..
> | :/ md -w 0x40300001+2
> | unable to handle paging request at address 0x40300001
> 
> Patch sets the ARM_NOUNALIGNED option introduced by a previous patch.
> 
> Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
> ---
>  arch/arm/mach-omap/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-omap/Kconfig b/arch/arm/mach-omap/Kconfig
> index 81f6127..445a35a 100644
> --- a/arch/arm/mach-omap/Kconfig
> +++ b/arch/arm/mach-omap/Kconfig
> @@ -68,6 +68,7 @@ config OMAP_GPMC
>  
>  config OMAP_BUILD_IFT
>  	prompt "build ift binary"
> +	select ARM_NOUNALIGNED

This needs more investigation. Coupling this to OMAP_BUILD_IFT does not
seem to be correct. Unaligned accesses work for cached memory once the MMU
is enabled, it won't work with MMU disabled though. In barebox MMU
support is optional and even when the MMU is enabled in the config parts
of the initialization run with MMU disabled.

This U-Boot commit shows what's going on:

> commit b823fd9ba56d56e3cbb5b05e7a4815fb0914204a
> Author: Albert ARIBAUD <albert.u.boot@aribaud.net>
> Date:   Tue Oct 9 09:28:15 2012 +0000
>     ARM: prevent misaligned array inits
>     
>     Under option -munaligned-access, gcc can perform local char
>     or 16-bit array initializations using misaligned native
>     accesses which will throw a data abort exception. Fix files
>     where these array initializations were unneeded, and for
>     files known to contain such initializations, enforce gcc
>     option -mno-unaligned-access.
>     
>     Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
>     [trini: Switch to usign call cc-option for -mno-unaligned-access as
>     Albert had done previously as that's really correct]
>     Signed-off-by: Tom Rini <trini@ti.com>
> 

This patch explicitely mentions char arrays initialized on the stack
like this:

function foo()
{
       char buffer[] = "initial value";
/* or */
       char buffer[] = { 'i', 'n', 'i', 't', 0 };
       ...
}

Is this the place where you see problems?

The U-Boot people work around this issue by converting the above to
const char *buffer = "initial value"; where possible and pass the
-mno-unaligned-access flag to files where this is not possible.

I really do not want to go the way to pass compiler flags to individual
files. Also the above is valid C code which should work.

barebox is written to not contain unaligned accesses, everything else
will fail on earlier ARM CPUs. I think we should pass the
-mno-unaligned-access unconditionally. Normally this will have no impact
as barebox doesn't do explicit unaligned accesses. For the rare cases
like the one above barebox should just work without having to pass
additional flags to files.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] OMAP: disable unaligned access when building the IFT

[Enrico Scholz]

Sascha Hauer <s.hauer@pengutronix.de> writes:

>>  config OMAP_BUILD_IFT
>>  	prompt "build ift binary"
>> +	select ARM_NOUNALIGNED
>
> This needs more investigation. Coupling this to OMAP_BUILD_IFT does
> not seem to be correct. Unaligned accesses work for cached memory once
> the MMU is enabled,

It depends on the type of accessed memory.  E.g.

OMAP4 SDRAM
| :/ md -w 0x80000001+2
| 80000001: 0000                                               ..

OMAP4 SRAM
| :/ md -w 0x40300000+2
| 40300000: 9001                                               
| 
| :/ md -w 0x40300001+2
| alignment fault at address 0x40300001
| pc : [<8f02a684>]    lr : [<8f02a654>]

I guess, the OMAP4 SRAM is strongly ordered memory which does not allow
unaligned access.

Similar problems might arise when accessing device memory unaligned.
But this should not be an issue because such operations are usually
expressed explicitly with readX() + writeX() macros.


Coupling the -mno-unaligned-access to the TEXT_BASE and the used processor
might be a better solution.  But I have only a very limited overview about
all the possible variants, so I decided to:

1. make it configurable

2. enable it for known cases (OMAP4 IFT is such a case because it is
   usually executed in SRAM)


> it won't work with MMU disabled though. 

no; unaligned access works on ARMv6+ without MMU too.


>> commit b823fd9ba56d56e3cbb5b05e7a4815fb0914204a
>> Author: Albert ARIBAUD <albert.u.boot@aribaud.net>
>> Date:   Tue Oct 9 09:28:15 2012 +0000
>>     ARM: prevent misaligned array inits
>>     
>>     Under option -munaligned-access, gcc can perform local char
>>     or 16-bit array initializations using misaligned native
>>     accesses which will throw a data abort exception. Fix files
>>     where these array initializations were unneeded, and for
>>     files known to contain such initializations, enforce gcc
>>     option -mno-unaligned-access.
> ...
> This patch explicitely mentions char arrays initialized on the stack
> like this:

I do not know the backgrounds of this patch but without additional
context, I think it is wrong.  It might fix problems of a specific
processor or in cases where u-boot is executed in SRAM or so.  But
unaligned access on ARMv6+ is generally ok.


> function foo()
> {
>        char buffer[] = "initial value";
> /* or */
>        char buffer[] = { 'i', 'n', 'i', 't', 0 };
>        ...
> }
>
> Is this the place where you see problems?

no; I saw the problem when the MLO accessed attributes of an oddly
located __packed structure with 'ldrh'.

"OMAP4: removed __packed__ annotation from pad_conf_entry" (see previous
postings) was an immediate (and for other reasons still correct) fix for
this issue.


> barebox is written to not contain unaligned accesses, everything else
> will fail on earlier ARM CPUs. I think we should pass the
> -mno-unaligned-access unconditionally.

I do not see a reason to do this unconditionally.


Enrico

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] OMAP: disable unaligned access when building the IFT

[Enrico Scholz]

Enrico Scholz <enrico.scholz@sigma-chemnitz.de> writes:

>>> commit b823fd9ba56d56e3cbb5b05e7a4815fb0914204a
>>> Author: Albert ARIBAUD <albert.u.boot@aribaud.net>
>>> Date:   Tue Oct 9 09:28:15 2012 +0000
>>>     ARM: prevent misaligned array inits
>> ...
>> This patch explicitely mentions char arrays initialized on the stack
>> like this:
>
> I do not know the backgrounds of this patch but without additional
> context, I think it is wrong.

ah... found it:

 | be  actually unnecessary. In order to catch these accesses and remove
 | or optimize them, option -munaligned-access is explicitly set for all
 | versions of gcc which support it.

e.g. u-boot sets '-munaligned-access' unconditionally which is imo
silly. And now, they are trying to fix this mess by compiling single
files with '-mno-unaligned-access'...



Enrico

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] OMAP: disable unaligned access when building the IFT

[Sascha Hauer]

On Wed, Dec 05, 2012 at 12:33:26PM +0100, Enrico Scholz wrote:
> Sascha Hauer <s.hauer@pengutronix.de> writes:
> 
> >>  config OMAP_BUILD_IFT
> >>  	prompt "build ift binary"
> >> +	select ARM_NOUNALIGNED
> >
> > This needs more investigation. Coupling this to OMAP_BUILD_IFT does
> > not seem to be correct. Unaligned accesses work for cached memory once
> > the MMU is enabled,
> 
> It depends on the type of accessed memory.  E.g.
> 
> OMAP4 SDRAM
> | :/ md -w 0x80000001+2
> | 80000001: 0000                                               ..

Haven't tested this on OMAP4, but on OMAP3 I get the same alignment trap
when the MMU is disabled, so SRAM is no different from SDRAM.

> 
> OMAP4 SRAM
> | :/ md -w 0x40300000+2
> | 40300000: 9001                                               
> | 
> | :/ md -w 0x40300001+2
> | alignment fault at address 0x40300001
> | pc : [<8f02a684>]    lr : [<8f02a654>]
> 
> I guess, the OMAP4 SRAM is strongly ordered memory which does not allow
> unaligned access.
> 
> Similar problems might arise when accessing device memory unaligned.
> But this should not be an issue because such operations are usually
> expressed explicitly with readX() + writeX() macros.

Yes, everything else is a bug.

> 
> 
> Coupling the -mno-unaligned-access to the TEXT_BASE and the used processor
> might be a better solution.

No, unaligned accesses are handled by the cache. They won't work when
the MMU is disabled, but barebox has to work with MMU disabled.

> But I have only a very limited overview about
> all the possible variants, so I decided to:
> 
> 1. make it configurable
> 
> 2. enable it for known cases (OMAP4 IFT is such a case because it is
>    usually executed in SRAM)
> 
> 
> > it won't work with MMU disabled though. 
> 
> no; unaligned access works on ARMv6+ without MMU too.

I can't confirm this:

i.MX53 (Cortex-A8):

barebox@Freescale i.MX53 LOCO:/ cpuinfo 
implementer: ARM
architecture: v7
cache: no cache
Control register: W P D L I DT IT U XP 
barebox@Freescale i.MX53 LOCO:/ md 0x70000001
<hangs>

OMAP3 beagle:

barebox@Texas Instrument's Beagle:/ cpuinfo 
implementer: ARM
architecture: v7
cache: no cache
Control register: W P D L Z I DT IT U XP 
barebox@Texas Instrument's Beagle:/ md 0x80000001
data abort
pc : [<87e1cf38>]    lr : [<87e1cf15>]
sp : 85dff8e0  ip : 00000000  fp : 00000000
r10: ffffffff  r9 : 00000000  r8 : 85e03a10
r7 : 00000004  r6 : 85e03a10  r5 : 80000001  r4 : 00000100
r3 : 00000020  r2 : 00000002  r1 : 00000004  r0 : 00000040

i.MX35 (ARM-V6):

barebox@Phytec phyCORE-i.MX35:/ cpuinfo 
implementer: ARM
architecture: v6
I-cache: 16384 bytes (linelen = 32)
D-cache: 16384 bytes (linelen = 32)
Control register: W P D L Z I RR DT IT FI U 
barebox@Phytec phyCORE-i.MX35:/ md 0x80000001
80000001: feea0000 feeaffff feeaffff feeaffff ................
80000011: feeaffff feeaffff feeaffff 62eaffff ...............b
80000021: 62657261 0000786f 0087e000 0000045d arebox......]...
80000031: 1fe10f30 d3e3c330 03e38330 10e129f0 0...0...0....)..
80000041: 8eee113f 05e3c33d 01e3c330 01e38335 ?...=...0...5...
80000051: 02e3833a 10e3c330 00ee013f 95e3a030 :...0...?...0...

On Armv6 this indeed works.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] OMAP: disable unaligned access when building the IFT

[Enrico Scholz]

Sascha Hauer <s.hauer@pengutronix.de> writes:

>> Coupling the -mno-unaligned-access to the TEXT_BASE and the used processor
>> might be a better solution.
>
> No, unaligned accesses are handled by the cache. They won't work when
> the MMU is disabled, but barebox has to work with MMU disabled.

ok; this stuff seems to be really processor dependent then.  E.g. unaligned
access works on Cortex-M3 which is ARMv7 but has neither an MMU nor an MPU.

Perhaps 'CONFIG_ARM_NOUNALIGNED' should default to yes when !MMU, and/or
it should be selected by other problematic configurations.  Of course,
there is still the problem that some C code is executed before MMU will
be enabled and unaligned access can happen there.


While thinking about it... cause of the core problem (unaligned access
exception when executed in SRAM) might be that SRAM memory mapped as
strongly ordered memory:

| :/ mmuinfo 0x40300000
|   ...
|   Inner mem. attr. [6:4]:   0x1 (0b001 Strongly-ordered)

Perhaps, this memory can be remapped as normal memory.



Enrico

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] OMAP: disable unaligned access when building the IFT

[Enrico Scholz]

Enrico Scholz
<enrico.scholz-wttK6gPy29v+Hn7q9Vec/7NAH6kLmebB@public.gmane.org>
writes:

> Patch sets the ARM_NOUNALIGNED option introduced by a previous patch.

just for the record: difference between both aligned/unaligned variants
is

$ bloat-o-meter barebox barebox.unaligned 
add/remove: 0/0 grow/shrink: 1/32 up/down: 4/-2648 (-2644)
function                                     old     new   delta
show_progress                                292     296      +4
usb_new_device                              1980    1972      -8
usb_get_configuration_no                     156     148      -8
rpc_req                                      232     220     -12
ping_send                                    152     140     -12
parse_partition_table                        776     764     -12
net_icmp_send                                 64      52     -12
dhcp_handler                                 700     684     -16
net_udp_send                                  56      36     -20
usb_control_msg                              164     140     -24
ubi_io_write_vid_hdr                         140     116     -24
ubi_eba_read_leb                             640     616     -24
ubi_change_vtbl_record                       208     184     -24
net_udp_new                                  136     112     -24
net_ip_send                                  156     120     -36
erase_worker                                 900     856     -44
ubi_scan_erase_peb                           220     172     -48
tftp_handler                                 752     688     -64
validate_vid_hdr                             212     140     -72
ubi_io_write_ec_hdr                          208     136     -72
net_receive                                  920     848     -72
ubi_scan                                    1596    1504     -92
ubi_cdev_ioctl                               236     140     -96
ubi_read_volume_table                       2072    1968    -104
create_vtbl                                  664     560    -104
vtbl_check                                   816     696    -120
ubi_io_read_ec_hdr                           660     516    -144
ubi_create_volume                           1276    1108    -168
ubi_eba_copy_leb                             904     712    -192
ubi_io_read_vid_hdr                          856     660    -196
ubi_eba_write_leb                           1628    1412    -216
ubi_eba_write_leb_st                         944     704    -240
ubi_scan_add_used                           1872    1524    -348



Enrico

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox