From: Tom Rini Date: Tue, 25 Feb 2014 15:27:01 +0000 (-0500) Subject: arm: Switch to -mno-unaligned-access when supported by the compiler X-Git-Tag: submit/tizen/20160318.071304~318 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d9ac643d7fca72630b3dcebee2c12b7814753ec0;p=profile%2Fcommon%2Fplatform%2Fkernel%2Fu-boot-artik.git arm: Switch to -mno-unaligned-access when supported by the compiler When we tell the compiler to optimize for ARMv7 (and ARMv6 for that matter) it assumes a default of SCTRL.A being cleared and unaligned accesses being allowed and fast at the hardware level. We set this bit and must pass along -mno-unaligned-access so that the compiler will still breakdown accesses and not trigger a data abort. To better help understand the requirements of the project with respect to unaligned memory access, the Documentation/unaligned-memory-access.txt file has been added as doc/README.unaligned-memory-access.txt and is taken from the v3.14-rc1 tag of the kernel. Cc: Albert ARIBAUD Cc: Mans Rullgard Signed-off-by: Tom Rini Conflicts: README arch/arm/cpu/armv8/config.mk common/Makefile fs/ubifs/Makefile lib/Makefile --- diff --git a/arch/arm/cpu/armv7/config.mk b/arch/arm/cpu/armv7/config.mk index 9c3e2f3ce..824150d46 100644 --- a/arch/arm/cpu/armv7/config.mk +++ b/arch/arm/cpu/armv7/config.mk @@ -35,9 +35,12 @@ PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARMV7) PF_RELFLAGS_SLB_AT := $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,)) PLATFORM_RELFLAGS += $(PF_RELFLAGS_SLB_AT) -# SEE README.arm-unaligned-accesses +# On supported platforms we set the bit which causes us to trap on unaligned +# memory access. This is the opposite of what the compiler expects to be +# the default so we must pass in -mno-unaligned-access so that it is aware +# of our decision. PF_NO_UNALIGNED := $(call cc-option, -mno-unaligned-access,) -PLATFORM_NO_UNALIGNED := $(PF_NO_UNALIGNED) +PLATFORM_CPPFLAGS += $(PF_NO_UNALIGNED) ifneq ($(CONFIG_IMX_CONFIG),) ALL-y += $(obj)u-boot.imx diff --git a/arch/arm/lib/interrupts.c b/arch/arm/lib/interrupts.c index 02124a704..74ff5ce1c 100644 --- a/arch/arm/lib/interrupts.c +++ b/arch/arm/lib/interrupts.c @@ -169,7 +169,7 @@ void do_prefetch_abort (struct pt_regs *pt_regs) void do_data_abort (struct pt_regs *pt_regs) { - printf ("data abort\n\n MAYBE you should read doc/README.arm-unaligned-accesses\n\n"); + printf ("data abort\n"); show_regs (pt_regs); bad_mode (); } diff --git a/common/Makefile b/common/Makefile index ec51fa213..a390666ee 100644 --- a/common/Makefile +++ b/common/Makefile @@ -233,10 +233,6 @@ $(obj)env_embedded.o: $(src)env_embedded.c $(obj)../tools/envcrc $(obj)../tools/envcrc: $(MAKE) -C ../tools -# SEE README.arm-unaligned-accesses -$(obj)hush.o: CFLAGS += $(PLATFORM_NO_UNALIGNED) -$(obj)fdt_support.o: CFLAGS += $(PLATFORM_NO_UNALIGNED) - ######################################################################### # defines $(obj).depend target diff --git a/doc/README.arm-unaligned-accesses b/doc/README.arm-unaligned-accesses deleted file mode 100644 index c37d13585..000000000 --- a/doc/README.arm-unaligned-accesses +++ /dev/null @@ -1,122 +0,0 @@ -If you are reading this because of a data abort: the following MIGHT -be relevant to your abort, if it was caused by an alignment violation. -In order to determine this, use the PC from the abort dump along with -an objdump -s -S of the u-boot ELF binary to locate the function where -the abort happened; then compare this function with the examples below. -If they match, then you've been hit with a compiler generated unaligned -access, and you should rewrite your code or add -mno-unaligned-access -to the command line of the offending file. - -Note that the PC shown in the abort message is relocated. In order to -be able to match it to an address in the ELF binary dump, you will need -to know the relocation offset. If your target defines CONFIG_CMD_BDI -and if you can get to the prompt and enter commands before the abort -happens, then command "bdinfo" will give you the offset. Otherwise you -will need to try a build with DEBUG set, which will display the offset, -or use a debugger and set a breakpoint at relocate_code() to see the -offset (passed as an argument). - -* - -Since U-Boot runs on a variety of hardware, some only able to perform -unaligned accesses with a strong penalty, some unable to perform them -at all, the policy regarding unaligned accesses is to not perform any, -unless absolutely necessary because of hardware or standards. - -Also, on hardware which permits it, the core is configured to throw -data abort exceptions on unaligned accesses in order to catch these -unallowed accesses as early as possible. - -Until version 4.7, the gcc default for performing unaligned accesses -(-mno-unaligned-access) is to emulate unaligned accesses using aligned -loads and stores plus shifts and masks. Emulated unaligned accesses -will not be caught by hardware. These accesses may be costly and may -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. - -From gcc 4.7 onward starting at armv7 architectures, the default for -performing unaligned accesses is to use unaligned native loads and -stores (-munaligned-access), because the cost of unaligned accesses -has dropped on armv7 and beyond. This should not affect U-Boot's -policy of controlling unaligned accesses, however the compiler may -generate uncontrolled unaligned accesses on its own in at least one -known case: when declaring a local initialized char array, e.g. - -function foo() -{ - char buffer[] = "initial value"; -/* or */ - char buffer[] = { 'i', 'n', 'i', 't', 0 }; - ... -} - -Under -munaligned-accesses with optimizations on, this declaration -causes the compiler to generate native loads from the literal string -and native stores to the buffer, and the literal string alignment -cannot be controlled. If it is misaligned, then the core will throw -a data abort exception. - -Quite probably the same might happen for 16-bit array initializations -where the constant is aligned on a boundary which is a multiple of 2 -but not of 4: - -function foo() -{ - u16 buffer[] = { 1, 2, 3 }; - ... -} - -The long term solution to this issue is to add an option to gcc to -allow controlling the general alignment of data, including constant -initialization values. - -However this will only apply to the version of gcc which will have such -an option. For other versions, there are four workarounds: - -a) Enforce as a rule that array initializations as described above - are forbidden. This is generally not acceptable as they are valid, - and usual, C constructs. The only case where they could be rejected - is when they actually equate to a const char* declaration, i.e. the - array is initialized and never modified in the function's scope. - -b) Drop the requirement on unaligned accesses at least for ARMv7, - i.e. do not throw a data abort exception upon unaligned accesses. - But that will allow adding badly aligned code to U-Boot, only for - it to fail when re-used with a stricter target, possibly once the - bad code is already in mainline. - -c) Relax the -munaligned-access rule globally. This will prevent native - unaligned accesses of course, but that will also hide any bug caused - by a bad unaligned access, making it much harder to diagnose it. It - is actually what already happens when building ARM targets with a - pre-4.7 gcc, and it may actually already hide some bugs yet unseen - until the target gets compiled with -munaligned-access. - -d) Relax the -munaligned-access rule only for for files susceptible to - the local initialized array issue and for armv7 architectures and - beyond. This minimizes the quantity of code which can hide unwanted - misaligned accesses. - -The option retained is d). - -Considering that actual occurrences of the issue are rare (as of this -writing, 5 files out of 7840 in U-Boot, or .3%, contain an initialized -local char array which cannot actually be replaced with a const char*), -contributors should not be required to systematically try and detect -the issue in their patches. - -Detecting files susceptible to the issue can be automated through a -filter installed as a hook in .git which recognizes local char array -initializations. Automation should err on the false positive side, for -instance flagging non-local arrays as if they were local if they cannot -be told apart. - -In any case, detection shall not prevent committing the patch, but -shall pre-populate the commit message with a note to the effect that -this patch contains an initialized local char or 16-bit array and thus -should be protected from the gcc 4.7 issue. - -Upon a positive detection, either $(PLATFORM_NO_UNALIGNED) should be -added to CFLAGS for the affected file(s), or if the array is a pseudo -const char*, it should be replaced by an actual one. diff --git a/fs/ubifs/Makefile b/fs/ubifs/Makefile index bfe687406..ccffe85ee 100644 --- a/fs/ubifs/Makefile +++ b/fs/ubifs/Makefile @@ -42,9 +42,6 @@ all: $(LIB) $(AOBJS) $(LIB): $(obj).depend $(OBJS) $(call cmd_link_o_target, $(OBJS)) -# SEE README.arm-unaligned-accesses -$(obj)super.o: CFLAGS += $(PLATFORM_NO_UNALIGNED) - ######################################################################### # defines $(obj).depend target diff --git a/lib/Makefile b/lib/Makefile index bd450d2ae..7229b1f84 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -78,9 +78,6 @@ OBJS := $(addprefix $(obj),$(COBJS)) $(LIB): $(obj).depend $(OBJS) $(call cmd_link_o_target, $(OBJS)) -# SEE README.arm-unaligned-accesses -$(obj)bzlib.o: CFLAGS += $(PLATFORM_NO_UNALIGNED) - ######################################################################### # defines $(obj).depend target