From b9f7b57d3a14e629af43bebc531763a923032239 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 7 Apr 2020 11:17:38 +0900 Subject: [PATCH 1/3] bl1: remove '.' from stacks section in linker script Only BL1 specifies '.' in the address field of the stacks section. Commit 4f59d8359f97 ("Make BL1 RO and RW base addresses configurable") added '.' on purpose but the commit message does not help to understand why. This commit gets rid of it in order to factor out the stacks section into include/common/bl_common.ld.h I compared the build result for PLAT=qemu. 'aarch64-linux-gnu-nm -n build/qemu/release/bl1/bl1.elf' will change as follows: @@ -336,8 +336,8 @@ 000000000e04e0e0 d max_log_level 000000000e04e0e4 D console_state 000000000e04e0e5 D __DATA_RAM_END__ -000000000e04e0e5 B __STACKS_START__ 000000000e04e100 b platform_normal_stacks +000000000e04e100 B __STACKS_START__ 000000000e04f100 b bl1_cpu_context 000000000e04f100 B __BSS_START__ 000000000e04f100 B __STACKS_END__ After this change, __STACKS_START__ will match to platform_normal_stacks, and I think it makes more sense. 'aarch64-linux-gnu-objdump -h build/qemu/release/bl1/bl1.elf' will change as follows: @@ -9,11 +9,11 @@ CONTENTS, ALLOC, LOAD, READONLY, DATA 2 .data 000000e5 000000000e04e000 0000000000004a60 0001e000 2**4 CONTENTS, ALLOC, LOAD, DATA - 3 stacks 0000101b 000000000e04e0e5 000000000e04e0e5 0001e0e5 2**6 + 3 stacks 00001000 000000000e04e100 0000000000004b45 0001e100 2**6 ALLOC - 4 .bss 000007e0 000000000e04f100 000000000e04f100 0001e0e5 2**5 + 4 .bss 000007e0 000000000e04f100 0000000000004b50 0001f100 2**5 ALLOC - 5 xlat_table 00006000 000000000e050000 000000000e050000 0001e0e5 2**12 + 5 xlat_table 00006000 000000000e050000 0000000000004b45 00020000 2**12 ALLOC 6 coherent_ram 00000000 000000000e056000 000000000e056000 0001f000 2**12 CONTENTS Sandrine pointed me to a useful document [1] to understand why LMAs of stacks, .bss, and xlat_table section have changed. Before this patch, they fell into this scenario: "If the section has a specific VMA address, then this is used as the LMA address as well." With this commit, the following applies: "Otherwise if a memory region can be found that is compatible with the current section, and this region contains at least one section, then the LMA is set so the difference between the VMA and LMA is the same as the difference between the VMA and LMA of the last section in the located region." Anyway, those three sections are not loaded, so the LMA changes will not be a problem. The size of bl1.bin is still the same. QEMU still boots successfully with this change. A good thing is, this fixes the error for the latest LLD. If I use the mainline LLVM, I see the following error. The alignment check will probably be included in the LLVM 11 release, so it is better to fix it now. $ PLAT=qemu CC=clang CROSS_COMPILE=aarch64-linux-gnu- [ snip ] ld.lld: error: address (0xe04e0e5) of section stacks is not a multiple of alignment (64) make: *** [Makefile:1050: build/qemu/release/bl1/bl1.elf] Error 1 [1]: https://sourceware.org/binutils/docs/ld/Output-Section-LMA.html#Output-Section-LMA Change-Id: I3d2f3cc2858be8b3ce2eab3812a76d1e0b5f3a32 Signed-off-by: Masahiro Yamada --- bl1/bl1.ld.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bl1/bl1.ld.S b/bl1/bl1.ld.S index 75355ebff..009a9b59c 100644 --- a/bl1/bl1.ld.S +++ b/bl1/bl1.ld.S @@ -103,7 +103,7 @@ SECTIONS __DATA_RAM_END__ = .; } >RAM AT>ROM - stacks . (NOLOAD) : { + stacks (NOLOAD) : { __STACKS_START__ = .; *(tzfw_normal_stacks) __STACKS_END__ = .; From a926a9f60aa94a034b0a06eed296996363245d30 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 7 Apr 2020 13:04:24 +0900 Subject: [PATCH 2/3] linker_script: move stacks section to bl_common.ld.h The stacks section is the same for all BL linker scripts. Move it to the common header file. Change-Id: Ibd253488667ab4f69702d56ff9e9929376704f6c Signed-off-by: Masahiro Yamada --- bl1/bl1.ld.S | 7 +------ bl2/bl2.ld.S | 7 +------ bl2/bl2_el3.ld.S | 7 +------ bl2u/bl2u.ld.S | 7 +------ bl31/bl31.ld.S | 7 +------ bl32/sp_min/sp_min.ld.S | 7 +------ bl32/tsp/tsp.ld.S | 7 +------ plat/mediatek/mt6795/bl31.ld.S | 7 +------ 8 files changed, 8 insertions(+), 48 deletions(-) diff --git a/bl1/bl1.ld.S b/bl1/bl1.ld.S index 009a9b59c..4ebe8a02a 100644 --- a/bl1/bl1.ld.S +++ b/bl1/bl1.ld.S @@ -103,12 +103,7 @@ SECTIONS __DATA_RAM_END__ = .; } >RAM AT>ROM - stacks (NOLOAD) : { - __STACKS_START__ = .; - *(tzfw_normal_stacks) - __STACKS_END__ = .; - } >RAM - + STACK_SECTION >RAM BSS_SECTION >RAM XLAT_TABLE_SECTION >RAM diff --git a/bl2/bl2.ld.S b/bl2/bl2.ld.S index 15df5dd03..17475f061 100644 --- a/bl2/bl2.ld.S +++ b/bl2/bl2.ld.S @@ -88,12 +88,7 @@ SECTIONS __DATA_END__ = .; } >RAM - stacks (NOLOAD) : { - __STACKS_START__ = .; - *(tzfw_normal_stacks) - __STACKS_END__ = .; - } >RAM - + STACK_SECTION >RAM BSS_SECTION >RAM XLAT_TABLE_SECTION >RAM diff --git a/bl2/bl2_el3.ld.S b/bl2/bl2_el3.ld.S index d04f226e9..ea7a23500 100644 --- a/bl2/bl2_el3.ld.S +++ b/bl2/bl2_el3.ld.S @@ -123,12 +123,7 @@ SECTIONS } >RAM __RELA_END__ = .; - stacks (NOLOAD) : { - __STACKS_START__ = .; - *(tzfw_normal_stacks) - __STACKS_END__ = .; - } >RAM - + STACK_SECTION >RAM BSS_SECTION >RAM XLAT_TABLE_SECTION >RAM diff --git a/bl2u/bl2u.ld.S b/bl2u/bl2u.ld.S index 8c0bbbdd0..3ab43825c 100644 --- a/bl2u/bl2u.ld.S +++ b/bl2u/bl2u.ld.S @@ -90,12 +90,7 @@ SECTIONS __DATA_END__ = .; } >RAM - stacks (NOLOAD) : { - __STACKS_START__ = .; - *(tzfw_normal_stacks) - __STACKS_END__ = .; - } >RAM - + STACK_SECTION >RAM BSS_SECTION >RAM XLAT_TABLE_SECTION >RAM diff --git a/bl31/bl31.ld.S b/bl31/bl31.ld.S index 1cdf7c943..94d03e3da 100644 --- a/bl31/bl31.ld.S +++ b/bl31/bl31.ld.S @@ -158,12 +158,7 @@ SECTIONS __NOBITS_START__ = .; #endif - stacks (NOLOAD) : { - __STACKS_START__ = .; - *(tzfw_normal_stacks) - __STACKS_END__ = .; - } >NOBITS - + STACK_SECTION >NOBITS BSS_SECTION >NOBITS XLAT_TABLE_SECTION >NOBITS diff --git a/bl32/sp_min/sp_min.ld.S b/bl32/sp_min/sp_min.ld.S index da005db64..8e91cec91 100644 --- a/bl32/sp_min/sp_min.ld.S +++ b/bl32/sp_min/sp_min.ld.S @@ -101,12 +101,7 @@ SECTIONS ASSERT(. <= BL32_PROGBITS_LIMIT, "BL32 progbits has exceeded its limit.") #endif - stacks (NOLOAD) : { - __STACKS_START__ = .; - *(tzfw_normal_stacks) - __STACKS_END__ = .; - } >RAM - + STACK_SECTION >RAM BSS_SECTION >RAM XLAT_TABLE_SECTION >RAM diff --git a/bl32/tsp/tsp.ld.S b/bl32/tsp/tsp.ld.S index bf77c9234..7428c0345 100644 --- a/bl32/tsp/tsp.ld.S +++ b/bl32/tsp/tsp.ld.S @@ -91,12 +91,7 @@ SECTIONS ASSERT(. <= TSP_PROGBITS_LIMIT, "TSP progbits has exceeded its limit.") #endif - stacks (NOLOAD) : { - __STACKS_START__ = .; - *(tzfw_normal_stacks) - __STACKS_END__ = .; - } >RAM - + STACK_SECTION >RAM BSS_SECTION >RAM XLAT_TABLE_SECTION >RAM diff --git a/plat/mediatek/mt6795/bl31.ld.S b/plat/mediatek/mt6795/bl31.ld.S index b061b91ce..91ca87cb6 100644 --- a/plat/mediatek/mt6795/bl31.ld.S +++ b/plat/mediatek/mt6795/bl31.ld.S @@ -74,12 +74,7 @@ SECTIONS ASSERT(. <= BL31_PROGBITS_LIMIT, "BL3-1 progbits has exceeded its limit.") #endif - stacks (NOLOAD) : { - __STACKS_START__ = .; - *(tzfw_normal_stacks) - __STACKS_END__ = .; - } >RAM - + STACK_SECTION >RAM BSS_SECTION >RAM __RW_END__ = __BSS_END__; From caa3e7e0a4aeb657873bbd2c002c0e33a614eb1d Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 22 Apr 2020 10:50:12 +0900 Subject: [PATCH 3/3] linker_script: move .data section to bl_common.ld.h Move the data section to the common header. I slightly tweaked some scripts as follows: [1] bl1.ld.S has ALIGN(16). I added DATA_ALIGN macro, which is 1 by default, but overridden by bl1.ld.S. Currently, ALIGN(16) of the .data section is redundant because commit 412865907699 ("Fix boot failures on some builds linked with ld.lld.") padded out the previous section to work around the issue of LLD version <= 10.0. This will be fixed in the future release of LLVM, so I am keeping the proper way to align LMA. [2] bl1.ld.S and bl2_el3.ld.S define __DATA_RAM_{START,END}__ instead of __DATA_{START,END}__. I put them out of the .data section. [3] SORT_BY_ALIGNMENT() is missing tsp.ld.S, sp_min.ld.S, and mediatek/mt6795/bl31.ld.S. This commit adds SORT_BY_ALIGNMENT() for all images, so the symbol order in those three will change, but I do not think it is a big deal. Change-Id: I215bb23c319f045cd88e6f4e8ee2518c67f03692 Signed-off-by: Masahiro Yamada --- bl1/bl1.ld.S | 26 +++++++++++--------------- bl2/bl2.ld.S | 12 +----------- bl2/bl2_el3.ld.S | 13 +++---------- bl2u/bl2u.ld.S | 12 +----------- bl31/bl31.ld.S | 11 +---------- bl32/sp_min/sp_min.ld.S | 6 +----- bl32/tsp/tsp.ld.S | 6 +----- include/common/bl_common.ld.h | 16 ++++++++++++++++ plat/mediatek/mt6795/bl31.ld.S | 11 +---------- 9 files changed, 36 insertions(+), 77 deletions(-) diff --git a/bl1/bl1.ld.S b/bl1/bl1.ld.S index 4ebe8a02a..bc23828e4 100644 --- a/bl1/bl1.ld.S +++ b/bl1/bl1.ld.S @@ -4,6 +4,14 @@ * SPDX-License-Identifier: BSD-3-Clause */ +/* + * The .data section gets copied from ROM to RAM at runtime. + * Its LMA should be 16-byte aligned to allow efficient copying of 16-bytes + * aligned regions in it. + * Its VMA must be page-aligned as it marks the first read/write page. + */ +#define DATA_ALIGN 16 + #include #include @@ -87,21 +95,9 @@ SECTIONS ASSERT(BL1_RW_BASE == ALIGN(PAGE_SIZE), "BL1_RW_BASE address is not aligned on a page boundary.") - /* - * The .data section gets copied from ROM to RAM at runtime. - * Its LMA should be 16-byte aligned to allow efficient copying of 16-bytes - * aligned regions in it. - * Its VMA must be page-aligned as it marks the first read/write page. - * - * It must be placed at a lower address than the stacks if the stack - * protector is enabled. Alternatively, the .data.stack_protector_canary - * section can be placed independently of the main .data section. - */ - .data . : ALIGN(16) { - __DATA_RAM_START__ = .; - *(SORT_BY_ALIGNMENT(.data*)) - __DATA_RAM_END__ = .; - } >RAM AT>ROM + DATA_SECTION >RAM AT>ROM + __DATA_RAM_START__ = __DATA_START__; + __DATA_RAM_END__ = __DATA_END__; STACK_SECTION >RAM BSS_SECTION >RAM diff --git a/bl2/bl2.ld.S b/bl2/bl2.ld.S index 17475f061..37849c312 100644 --- a/bl2/bl2.ld.S +++ b/bl2/bl2.ld.S @@ -77,17 +77,7 @@ SECTIONS */ __RW_START__ = . ; - /* - * .data must be placed at a lower address than the stacks if the stack - * protector is enabled. Alternatively, the .data.stack_protector_canary - * section can be placed independently of the main .data section. - */ - .data . : { - __DATA_START__ = .; - *(SORT_BY_ALIGNMENT(.data*)) - __DATA_END__ = .; - } >RAM - + DATA_SECTION >RAM STACK_SECTION >RAM BSS_SECTION >RAM XLAT_TABLE_SECTION >RAM diff --git a/bl2/bl2_el3.ld.S b/bl2/bl2_el3.ld.S index ea7a23500..8c45d9898 100644 --- a/bl2/bl2_el3.ld.S +++ b/bl2/bl2_el3.ld.S @@ -101,16 +101,9 @@ SECTIONS */ __RW_START__ = . ; - /* - * .data must be placed at a lower address than the stacks if the stack - * protector is enabled. Alternatively, the .data.stack_protector_canary - * section can be placed independently of the main .data section. - */ - .data . : { - __DATA_RAM_START__ = .; - *(SORT_BY_ALIGNMENT(.data*)) - __DATA_RAM_END__ = .; - } >RAM AT>ROM + DATA_SECTION >RAM AT>ROM + __DATA_RAM_START__ = __DATA_START__; + __DATA_RAM_END__ = __DATA_END__; /* * .rela.dyn needs to come after .data for the read-elf utility to parse diff --git a/bl2u/bl2u.ld.S b/bl2u/bl2u.ld.S index 3ab43825c..a7752a490 100644 --- a/bl2u/bl2u.ld.S +++ b/bl2u/bl2u.ld.S @@ -79,17 +79,7 @@ SECTIONS */ __RW_START__ = . ; - /* - * .data must be placed at a lower address than the stacks if the stack - * protector is enabled. Alternatively, the .data.stack_protector_canary - * section can be placed independently of the main .data section. - */ - .data . : { - __DATA_START__ = .; - *(SORT_BY_ALIGNMENT(.data*)) - __DATA_END__ = .; - } >RAM - + DATA_SECTION >RAM STACK_SECTION >RAM BSS_SECTION >RAM XLAT_TABLE_SECTION >RAM diff --git a/bl31/bl31.ld.S b/bl31/bl31.ld.S index 94d03e3da..11e86a3c1 100644 --- a/bl31/bl31.ld.S +++ b/bl31/bl31.ld.S @@ -114,16 +114,7 @@ SECTIONS */ __RW_START__ = . ; - /* - * .data must be placed at a lower address than the stacks if the stack - * protector is enabled. Alternatively, the .data.stack_protector_canary - * section can be placed independently of the main .data section. - */ - .data . : { - __DATA_START__ = .; - *(SORT_BY_ALIGNMENT(.data*)) - __DATA_END__ = .; - } >RAM + DATA_SECTION >RAM /* * .rela.dyn needs to come after .data for the read-elf utility to parse diff --git a/bl32/sp_min/sp_min.ld.S b/bl32/sp_min/sp_min.ld.S index 8e91cec91..9e0596f1f 100644 --- a/bl32/sp_min/sp_min.ld.S +++ b/bl32/sp_min/sp_min.ld.S @@ -91,11 +91,7 @@ SECTIONS */ __RW_START__ = . ; - .data . : { - __DATA_START__ = .; - *(.data*) - __DATA_END__ = .; - } >RAM + DATA_SECTION >RAM #ifdef BL32_PROGBITS_LIMIT ASSERT(. <= BL32_PROGBITS_LIMIT, "BL32 progbits has exceeded its limit.") diff --git a/bl32/tsp/tsp.ld.S b/bl32/tsp/tsp.ld.S index 7428c0345..bdcd2cf70 100644 --- a/bl32/tsp/tsp.ld.S +++ b/bl32/tsp/tsp.ld.S @@ -70,11 +70,7 @@ SECTIONS */ __RW_START__ = . ; - .data . : { - __DATA_START__ = .; - *(.data*) - __DATA_END__ = .; - } >RAM + DATA_SECTION >RAM /* * .rela.dyn needs to come after .data for the read-elf utility to parse diff --git a/include/common/bl_common.ld.h b/include/common/bl_common.ld.h index 8ea7d6a8c..97fed7204 100644 --- a/include/common/bl_common.ld.h +++ b/include/common/bl_common.ld.h @@ -17,6 +17,10 @@ #define BSS_ALIGN 8 #endif +#ifndef DATA_ALIGN +#define DATA_ALIGN 1 +#endif + #define CPU_OPS \ . = ALIGN(STRUCT_ALIGN); \ __CPU_OPS_START__ = .; \ @@ -85,6 +89,18 @@ GOT \ BASE_XLAT_TABLE_RO +/* + * .data must be placed at a lower address than the stacks if the stack + * protector is enabled. Alternatively, the .data.stack_protector_canary + * section can be placed independently of the main .data section. + */ +#define DATA_SECTION \ + .data . : ALIGN(DATA_ALIGN) { \ + __DATA_START__ = .; \ + *(SORT_BY_ALIGNMENT(.data*)) \ + __DATA_END__ = .; \ + } + #define STACK_SECTION \ stacks (NOLOAD) : { \ __STACKS_START__ = .; \ diff --git a/plat/mediatek/mt6795/bl31.ld.S b/plat/mediatek/mt6795/bl31.ld.S index 91ca87cb6..3d881fc43 100644 --- a/plat/mediatek/mt6795/bl31.ld.S +++ b/plat/mediatek/mt6795/bl31.ld.S @@ -59,16 +59,7 @@ SECTIONS */ __RW_START__ = . ; - /* - * .data must be placed at a lower address than the stacks if the stack - * protector is enabled. Alternatively, the .data.stack_protector_canary - * section can be placed independently of the main .data section. - */ - .data . : { - __DATA_START__ = .; - *(.data*) - __DATA_END__ = .; - } >RAM + DATA_SECTION >RAM #ifdef BL31_PROGBITS_LIMIT ASSERT(. <= BL31_PROGBITS_LIMIT, "BL3-1 progbits has exceeded its limit.")