From 665e71b8ea28162ec7737c1411bca3ea89e5957e Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 9 Mar 2020 17:39:48 +0900 Subject: Factor xlat_table sections in linker scripts out into a header file TF-A has so many linker scripts, at least one linker script for each BL image, and some platforms have their own ones. They duplicate quite similar code (and comments). When we add some changes to linker scripts, we end up with touching so many files. This is not nice in the maintainability perspective. When you look at Linux kernel, the common code is macrofied in include/asm-generic/vmlinux.lds.h, which is included from each arch linker script, arch/*/kernel/vmlinux.lds.S TF-A can follow this approach. Let's factor out the common code into include/common/bl_common.ld.h As a start point, this commit factors out the xlat_table section. Change-Id: Ifa369e9b48e8e12702535d721cc2a16d12397895 Signed-off-by: Masahiro Yamada --- include/common/bl_common.ld.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 include/common/bl_common.ld.h (limited to 'include/common/bl_common.ld.h') diff --git a/include/common/bl_common.ld.h b/include/common/bl_common.ld.h new file mode 100644 index 000000000..32c54b4d2 --- /dev/null +++ b/include/common/bl_common.ld.h @@ -0,0 +1,21 @@ +/* + * Copyright (c) 2020, ARM Limited and Contributors. All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#ifndef BL_COMMON_LD_H +#define BL_COMMON_LD_H + +/* + * The xlat_table section is for full, aligned page tables (4K). + * Removing them from .bss avoids forcing 4K alignment on + * the .bss section. The tables are initialized to zero by the translation + * tables library. + */ +#define XLAT_TABLE_SECTION \ + xlat_table (NOLOAD) : { \ + *(xlat_table) \ + } + +#endif /* BL_COMMON_LD_H */ -- cgit v1.2.3 From 9fb288a03ed2ced7706defbbf78f008e921e17e2 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 26 Mar 2020 10:51:39 +0900 Subject: linker_script: move more common code to bl_common.ld.h These are mostly used to collect data from special structure, and repeated in many linker scripts. To differentiate the alignment size between aarch32/aarch64, I added a new macro STRUCT_ALIGN. While I moved the PMF_SVC_DESCS, I dropped #if ENABLE_PMF conditional. As you can see in include/lib/pmf/pmf_helpers.h, PMF_REGISTER_SERVICE* are no-op when ENABLE_PMF=0. So, pmf_svc_descs and pmf_timestamp_array data are not populated. Change-Id: I3f4ab7fa18f76339f1789103407ba76bda7e56d0 Signed-off-by: Masahiro Yamada --- include/common/bl_common.ld.h | 112 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) (limited to 'include/common/bl_common.ld.h') diff --git a/include/common/bl_common.ld.h b/include/common/bl_common.ld.h index 32c54b4d2..d9e2e015f 100644 --- a/include/common/bl_common.ld.h +++ b/include/common/bl_common.ld.h @@ -7,6 +7,118 @@ #ifndef BL_COMMON_LD_H #define BL_COMMON_LD_H +#include + +#ifdef __aarch64__ +#define STRUCT_ALIGN 8 +#else +#define STRUCT_ALIGN 4 +#endif + +#define CPU_OPS \ + . = ALIGN(STRUCT_ALIGN); \ + __CPU_OPS_START__ = .; \ + KEEP(*(cpu_ops)) \ + __CPU_OPS_END__ = .; + +#define PARSER_LIB_DESCS \ + . = ALIGN(STRUCT_ALIGN); \ + __PARSER_LIB_DESCS_START__ = .; \ + KEEP(*(.img_parser_lib_descs)) \ + __PARSER_LIB_DESCS_END__ = .; + +#define RT_SVC_DESCS \ + . = ALIGN(STRUCT_ALIGN); \ + __RT_SVC_DESCS_START__ = .; \ + KEEP(*(rt_svc_descs)) \ + __RT_SVC_DESCS_END__ = .; + +#define PMF_SVC_DESCS \ + . = ALIGN(STRUCT_ALIGN); \ + __PMF_SVC_DESCS_START__ = .; \ + KEEP(*(pmf_svc_descs)) \ + __PMF_SVC_DESCS_END__ = .; + +#define FCONF_POPULATOR \ + . = ALIGN(STRUCT_ALIGN); \ + __FCONF_POPULATOR_START__ = .; \ + KEEP(*(.fconf_populator)) \ + __FCONF_POPULATOR_END__ = .; + +/* + * Keep the .got section in the RO section as it is patched prior to enabling + * the MMU and having the .got in RO is better for security. GOT is a table of + * addresses so ensure pointer size alignment. + */ +#define GOT \ + . = ALIGN(STRUCT_ALIGN); \ + __GOT_START__ = .; \ + *(.got) \ + __GOT_END__ = .; + +#define STACK_SECTION \ + stacks (NOLOAD) : { \ + __STACKS_START__ = .; \ + *(tzfw_normal_stacks) \ + __STACKS_END__ = .; \ + } + +/* + * If BL doesn't use any bakery lock then __PERCPU_BAKERY_LOCK_SIZE__ + * will be zero. For this reason, the only two valid values for + * __PERCPU_BAKERY_LOCK_SIZE__ are 0 or the platform defined value + * PLAT_PERCPU_BAKERY_LOCK_SIZE. + */ +#ifdef PLAT_PERCPU_BAKERY_LOCK_SIZE +#define BAKERY_LOCK_SIZE_CHECK \ + ASSERT((__PERCPU_BAKERY_LOCK_SIZE__ == 0) || \ + (__PERCPU_BAKERY_LOCK_SIZE__ == PLAT_PERCPU_BAKERY_LOCK_SIZE), \ + "PLAT_PERCPU_BAKERY_LOCK_SIZE does not match bakery lock requirements"); +#else +#define BAKERY_LOCK_SIZE_CHECK +#endif + +/* + * Bakery locks are stored in normal .bss memory + * + * Each lock's data is spread across multiple cache lines, one per CPU, + * but multiple locks can share the same cache line. + * The compiler will allocate enough memory for one CPU's bakery locks, + * the remaining cache lines are allocated by the linker script + */ +#if !USE_COHERENT_MEM +#define BAKERY_LOCK_NORMAL \ + . = ALIGN(CACHE_WRITEBACK_GRANULE); \ + __BAKERY_LOCK_START__ = .; \ + __PERCPU_BAKERY_LOCK_START__ = .; \ + *(bakery_lock) \ + . = ALIGN(CACHE_WRITEBACK_GRANULE); \ + __PERCPU_BAKERY_LOCK_END__ = .; \ + __PERCPU_BAKERY_LOCK_SIZE__ = ABSOLUTE(__PERCPU_BAKERY_LOCK_END__ - __PERCPU_BAKERY_LOCK_START__); \ + . = . + (__PERCPU_BAKERY_LOCK_SIZE__ * (PLATFORM_CORE_COUNT - 1)); \ + __BAKERY_LOCK_END__ = .; \ + BAKERY_LOCK_SIZE_CHECK +#else +#define BAKERY_LOCK_NORMAL +#endif + +/* + * Time-stamps are stored in normal .bss memory + * + * The compiler will allocate enough memory for one CPU's time-stamps, + * the remaining memory for other CPUs is allocated by the + * linker script + */ +#define PMF_TIMESTAMP \ + . = ALIGN(CACHE_WRITEBACK_GRANULE); \ + __PMF_TIMESTAMP_START__ = .; \ + KEEP(*(pmf_timestamp_array)) \ + . = ALIGN(CACHE_WRITEBACK_GRANULE); \ + __PMF_PERCPU_TIMESTAMP_END__ = .; \ + __PERCPU_TIMESTAMP_SIZE__ = ABSOLUTE(. - __PMF_TIMESTAMP_START__); \ + . = . + (__PERCPU_TIMESTAMP_SIZE__ * (PLATFORM_CORE_COUNT - 1)); \ + __PMF_TIMESTAMP_END__ = .; + /* * The xlat_table section is for full, aligned page tables (4K). * Removing them from .bss avoids forcing 4K alignment on -- cgit v1.2.3 From 0a0a7a9ac82cb79af91f098cedc69cc67bca3978 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 26 Mar 2020 10:57:12 +0900 Subject: linker_script: replace common read-only data with RODATA_COMMON The common section data are repeated in many linker scripts (often twice in each script to support SEPARATE_CODE_AND_RODATA). When you add a new read-only data section, you end up with touching lots of places. After this commit, you will only need to touch bl_common.ld.h when you add a new section to RODATA_COMMON. Replace a series of RO section with RODATA_COMMON, which contains 6 sections, some of which did not exist before. This is not a big deal because unneeded data should not be compiled in the first place. I believe this should be controlled by BL*_SOURCES in Makefiles, not by linker scripts. When I was working on this commit, the BL1 image size increased due to the fconf_populator. Commit c452ba159c14 ("fconf: exclude fconf_dyn_cfg_getter.c from BL1_SOURCES") fixed this issue. I investigated BL1, BL2, BL2U, BL31 for plat=fvp, and BL2-AT-EL3, BL31, BL31 for plat=uniphier. I did not see any more unexpected code addition. Change-Id: I5d14d60dbe3c821765bce3ae538968ef266f1460 Signed-off-by: Masahiro Yamada --- include/common/bl_common.ld.h | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'include/common/bl_common.ld.h') diff --git a/include/common/bl_common.ld.h b/include/common/bl_common.ld.h index d9e2e015f..5c5fe5b15 100644 --- a/include/common/bl_common.ld.h +++ b/include/common/bl_common.ld.h @@ -56,6 +56,14 @@ *(.got) \ __GOT_END__ = .; +#define RODATA_COMMON \ + RT_SVC_DESCS \ + FCONF_POPULATOR \ + PMF_SVC_DESCS \ + PARSER_LIB_DESCS \ + CPU_OPS \ + GOT + #define STACK_SECTION \ stacks (NOLOAD) : { \ __STACKS_START__ = .; \ -- cgit v1.2.3 From a7739bc7b16bf3e43f370864f8a800cf8943b391 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 26 Mar 2020 13:16:33 +0900 Subject: linker_script: move bss section to bl_common.ld.h Move the bss section to the common header. This adds BAKERY_LOCK_NORMAL and PMF_TIMESTAMP, which previously existed only in BL31. This is not a big deal because unused data should not be compiled in the first place. I believe this should be controlled by BL*_SOURCES in Makefiles, not by linker scripts. I investigated BL1, BL2, BL2U, BL31 for plat=fvp, and BL2-AT-EL3, BL31, BL31 for plat=uniphier. I did not see any more unexpected code addition. The bss section has bigger alignment. I added BSS_ALIGN for this. Currently, SORT_BY_ALIGNMENT() is missing in sp_min.ld.S, and with this change, the BSS symbols in SP_MIN will be sorted by the alignment. This is not a big deal (or, even better in terms of the image size). Change-Id: I680ee61f84067a559bac0757f9d03e73119beb33 Signed-off-by: Masahiro Yamada --- include/common/bl_common.ld.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'include/common/bl_common.ld.h') diff --git a/include/common/bl_common.ld.h b/include/common/bl_common.ld.h index 5c5fe5b15..3fc8e970d 100644 --- a/include/common/bl_common.ld.h +++ b/include/common/bl_common.ld.h @@ -11,8 +11,10 @@ #ifdef __aarch64__ #define STRUCT_ALIGN 8 +#define BSS_ALIGN 16 #else #define STRUCT_ALIGN 4 +#define BSS_ALIGN 8 #endif #define CPU_OPS \ @@ -127,6 +129,22 @@ . = . + (__PERCPU_TIMESTAMP_SIZE__ * (PLATFORM_CORE_COUNT - 1)); \ __PMF_TIMESTAMP_END__ = .; + +/* + * The .bss section gets initialised to 0 at runtime. + * Its base address has bigger alignment for better performance of the + * zero-initialization code. + */ +#define BSS_SECTION \ + .bss (NOLOAD) : ALIGN(BSS_ALIGN) { \ + __BSS_START__ = .; \ + *(SORT_BY_ALIGNMENT(.bss*)) \ + *(COMMON) \ + BAKERY_LOCK_NORMAL \ + PMF_TIMESTAMP \ + __BSS_END__ = .; \ + } + /* * The xlat_table section is for full, aligned page tables (4K). * Removing them from .bss avoids forcing 4K alignment on -- cgit v1.2.3 From 268131c24fedb804589a2226d4adb3ef97cd8874 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 26 Mar 2020 13:18:48 +0900 Subject: xlat_tables_v2: fix assembler warning of PLAT_RO_XLAT_TABLES If PLAT_RO_XLAT_TABLES is defined, the base xlat table goes to the .rodata section instead of .bss section. This causes a warning like: /tmp/ccswitLr.s: Assembler messages: /tmp/ccswitLr.s:297: Warning: setting incorrect section attributes for .rodata It is practically no problem, but I want to keep the build log clean. Put the base table into the "base_xlat_table" section to suppress the assembler warnings. The linker script determines its final destination; rodata section if PLAT_RO_XLAT_TABLES=1, or bss section otherwise. So, the result is the same. Change-Id: Ic85d1d2dddd9b5339289fc2378cbcb21dd7db02e Signed-off-by: Masahiro Yamada --- include/common/bl_common.ld.h | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'include/common/bl_common.ld.h') diff --git a/include/common/bl_common.ld.h b/include/common/bl_common.ld.h index 3fc8e970d..8ea7d6a8c 100644 --- a/include/common/bl_common.ld.h +++ b/include/common/bl_common.ld.h @@ -58,13 +58,32 @@ *(.got) \ __GOT_END__ = .; +/* + * The base xlat table + * + * It is put into the rodata section if PLAT_RO_XLAT_TABLES=1, + * or into the bss section otherwise. + */ +#define BASE_XLAT_TABLE \ + . = ALIGN(16); \ + *(base_xlat_table) + +#if PLAT_RO_XLAT_TABLES +#define BASE_XLAT_TABLE_RO BASE_XLAT_TABLE +#define BASE_XLAT_TABLE_BSS +#else +#define BASE_XLAT_TABLE_RO +#define BASE_XLAT_TABLE_BSS BASE_XLAT_TABLE +#endif + #define RODATA_COMMON \ RT_SVC_DESCS \ FCONF_POPULATOR \ PMF_SVC_DESCS \ PARSER_LIB_DESCS \ CPU_OPS \ - GOT + GOT \ + BASE_XLAT_TABLE_RO #define STACK_SECTION \ stacks (NOLOAD) : { \ @@ -142,6 +161,7 @@ *(COMMON) \ BAKERY_LOCK_NORMAL \ PMF_TIMESTAMP \ + BASE_XLAT_TABLE_BSS \ __BSS_END__ = .; \ } -- cgit v1.2.3 From caa3e7e0a4aeb657873bbd2c002c0e33a614eb1d Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 22 Apr 2020 10:50:12 +0900 Subject: 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 --- include/common/bl_common.ld.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'include/common/bl_common.ld.h') 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__ = .; \ -- cgit v1.2.3 From 34dd1e96fdae59d56d19a8d1270a03860af9f015 Mon Sep 17 00:00:00 2001 From: Alexei Fedorov Date: Sat, 30 May 2020 17:33:26 +0100 Subject: TF-A: Fix BL31 linker script error The patch fixes BL31 linker script error "Init code ends past the end of the stacks" for platforms with number of CPUs less than 4, which is caused by __STACKS_END__ address being lower than __INIT_CODE_END__. The modified BL31 linker script detects such cases and increases the total amount of stack memory, setting __STACKS_END__ = __INIT_CODE_END__, and CPUs' stacks are calculated by BL31 'plat_get_my_stack' function accordingly. For platforms with more than 4 CPUs and __INIT_CODE_END__ < __STACKS_END__ stack memory does not increase and allocated CPUs' stacks match the existing implementation. The patch removes exclusion of PSCI initialization functions from the reclaimed .init section in 'arm_reclaim_init.ld.S' script, which increases the size of reclaimed memory region. Change-Id: I927773e00dd84e1ffe72f9ee534f4f2fc7b6153c Signed-off-by: Alexei Fedorov --- include/common/bl_common.ld.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include/common/bl_common.ld.h') diff --git a/include/common/bl_common.ld.h b/include/common/bl_common.ld.h index 97fed7204..208e3d681 100644 --- a/include/common/bl_common.ld.h +++ b/include/common/bl_common.ld.h @@ -101,12 +101,14 @@ __DATA_END__ = .; \ } +#if !(defined(IMAGE_BL31) && RECLAIM_INIT_CODE) #define STACK_SECTION \ stacks (NOLOAD) : { \ __STACKS_START__ = .; \ *(tzfw_normal_stacks) \ __STACKS_END__ = .; \ } +#endif /* * If BL doesn't use any bakery lock then __PERCPU_BAKERY_LOCK_SIZE__ -- cgit v1.2.3 From e8ad6168b0153e09f1a54ee887555db7833019df Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 22 Apr 2020 11:27:55 +0900 Subject: linker_script: move .rela.dyn section to bl_common.ld.h The .rela.dyn section is the same for BL2-AT-EL3, BL31, TSP. Move it to the common header file. I slightly changed the definition so that we can do "RELA_SECTION >RAM". It still produced equivalent elf images. Please note I got rid of '.' from the VMA field. Otherwise, if the end of previous .data section is not 8-byte aligned, it fails to link. aarch64-linux-gnu-ld.bfd: warning: changing start of section .rela.dyn by 4 bytes aarch64-linux-gnu-ld.bfd: warning: changing start of section .rela.dyn by 4 bytes aarch64-linux-gnu-ld.bfd: warning: changing start of section .rela.dyn by 4 bytes make: *** [Makefile:1071: build/qemu/release/bl31/bl31.elf] Error 1 Change-Id: Iba7422d99c0374d4d9e97e6fd47bae129dba5cc9 Signed-off-by: Masahiro Yamada --- include/common/bl_common.ld.h | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'include/common/bl_common.ld.h') diff --git a/include/common/bl_common.ld.h b/include/common/bl_common.ld.h index 208e3d681..ab3391aa2 100644 --- a/include/common/bl_common.ld.h +++ b/include/common/bl_common.ld.h @@ -101,6 +101,17 @@ __DATA_END__ = .; \ } +/* + * .rela.dyn needs to come after .data for the read-elf utility to parse + * this section correctly. + */ +#define RELA_SECTION \ + .rela.dyn : ALIGN(STRUCT_ALIGN) { \ + __RELA_START__ = .; \ + *(.rela*) \ + __RELA_END__ = .; \ + } + #if !(defined(IMAGE_BL31) && RECLAIM_INIT_CODE) #define STACK_SECTION \ stacks (NOLOAD) : { \ -- cgit v1.2.3