[PATCH 0/7 v2] Clean up arm linker scripts
The arm linker scripts had a mix of symbols and C defined variables in an effort to emit relative references instead of absolute ones e.g [0]. A linker bug prevented us from doing so [1] -- fixed since 2016. This has led to confusion over the years, ending up with mixed section definitions. Some sections are defined with overlays and different definitions between v7 and v8 architectures. For example __efi_runtime_rel_start/end is defined as a linker symbol for armv8 and a C variable in armv7.
Linker scripts nowadays can emit relative references, as long as the symbol definition is contained within the section definition. So let's switch most of the C defined variables and clean up the arm sections.c file. There's still a few symbols remaining -- __secure_start/end, __secure_stack_start/end and __end which can be cleaned up in a followup series.
For both QEMU v7/v8 bloat-o-meter shows now size difference $~ ./scripts/bloat-o-meter u-boot u-boot.new add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0) Function old new delta Total: Before=798861, After=798861, chg +0.00%
The symbols seem largely unchanged apart from a difference in .bss as well as the emited sections and object types of the affected variables.
On the output below the first value is from -next and the second comes from -next + this patchset. The .bss_start/end sections have disappeared from the newer binaries.
# For example on QEMU v8: efi_runtime_start 7945: 0000000000000178 0 OBJECT GLOBAL DEFAULT 2 __efi_runtime_start 7942: 0000000000000178 0 NOTYPE GLOBAL DEFAULT 2 __efi_runtime_start efi_runtime_stop 9050: 0000000000000d38 0 OBJECT GLOBAL DEFAULT 2 __efi_runtime_stop 9047: 0000000000000d38 0 NOTYPE GLOBAL DEFAULT 2 __efi_runtime_stop __efi_runtime_rel_start 7172: 00000000000dc2f0 0 OBJECT GLOBAL DEFAULT 10 __efi_runtime_rel_start 7169: 00000000000dc2f0 0 NOTYPE GLOBAL DEFAULT 10 __efi_runtime_rel_start __efi_runtime_rel_stop 7954: 00000000000dc4a0 0 OBJECT GLOBAL DEFAULT 10 __efi_runtime_rel_stop 7951: 00000000000dc4a0 0 NOTYPE GLOBAL DEFAULT 10 __efi_runtime_rel_stop __rel_dyn_start 7030: 00000000000dc4a0 0 OBJECT GLOBAL DEFAULT 11 __rel_dyn_start 7027: 00000000000dc4a0 0 NOTYPE GLOBAL DEFAULT 11 __rel_dyn_start __rel_dyn_end 8959: 0000000000102b10 0 OBJECT GLOBAL DEFAULT 12 __rel_dyn_end 8956: 0000000000102b10 0 NOTYPE GLOBAL DEFAULT 11 __rel_dyn_end image_copy_start 9051: 0000000000000000 0 OBJECT GLOBAL DEFAULT 1 __image_copy_start 9048: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 1 __image_copy_start image_copy_end 7467: 00000000000dc4a0 0 OBJECT GLOBAL DEFAULT 11 __image_copy_end 7464: 00000000000dc4a0 0 NOTYPE GLOBAL DEFAULT 10 __image_copy_end bss_start 12: 0000000000102b10 0 SECTION LOCAL DEFAULT 12 .bss_start 8087: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8375: 0000000000102b10 0 OBJECT GLOBAL DEFAULT 12 __bss_start 8084: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8372: 0000000000102b80 0 NOTYPE GLOBAL DEFAULT 12 __bss_start bss_end 14: 000000000010bc30 0 SECTION LOCAL DEFAULT 14 .bss_end 7683: 000000000010bc30 0 OBJECT GLOBAL DEFAULT 14 __bss_end 8479: 0000000000000020 0 NOTYPE GLOBAL DEFAULT 1 _bss_end_ofs 7680: 000000000010bc30 0 NOTYPE GLOBAL DEFAULT 12 __bss_end 8476: 0000000000000020 0 NOTYPE GLOBAL DEFAULT 1 _bss_end_ofs
# For QEMU v7: efi_runtime_start 10703: 000003bc 0 OBJECT GLOBAL DEFAULT 2 __efi_runtime_start 10699: 000003c0 0 NOTYPE GLOBAL DEFAULT 2 __efi_runtime_start efi_runtime_stop 11796: 000012ec 0 OBJECT GLOBAL DEFAULT 2 __efi_runtime_stop 11792: 000012ec 0 NOTYPE GLOBAL DEFAULT 2 __efi_runtime_stop __efi_runtime_rel_start 9937: 000c40dc 0 OBJECT GLOBAL DEFAULT 8 __efi_runtime_rel_start 9935: 000c40dc 0 NOTYPE GLOBAL DEFAULT 9 __efi_runtime_rel_start __efi_runtime_rel_stop 10712: 000c41dc 0 OBJECT GLOBAL DEFAULT 10 __efi_runtime_rel_stop 10708: 000c41dc 0 NOTYPE GLOBAL DEFAULT 9 __efi_runtime_rel_stop __rel_dyn_start 9791: 000c41dc 0 OBJECT GLOBAL DEFAULT 10 __rel_dyn_start 9789: 000c41dc 0 NOTYPE GLOBAL DEFAULT 10 __rel_dyn_start __rel_dyn_end 11708: 000da5f4 0 OBJECT GLOBAL DEFAULT 10 __rel_dyn_end 11704: 000da5f4 0 NOTYPE GLOBAL DEFAULT 10 __rel_dyn_end image_copy_start 448: 0000177c 0 NOTYPE LOCAL DEFAULT 3 _image_copy_start_ofs 11797: 00000000 0 OBJECT GLOBAL DEFAULT 1 __image_copy_start 445: 0000177c 0 NOTYPE LOCAL DEFAULT 3 _image_copy_start_ofs 11793: 00000000 0 NOTYPE GLOBAL DEFAULT 1 __image_copy_start image_copy_end 450: 00001780 0 NOTYPE LOCAL DEFAULT 3 _image_copy_end_ofs 10225: 000c41dc 0 OBJECT GLOBAL DEFAULT 10 __image_copy_end 447: 00001780 0 NOTYPE LOCAL DEFAULT 3 _image_copy_end_ofs 10222: 000c41dc 0 NOTYPE GLOBAL DEFAULT 9 __image_copy_end bss_start 11: 000c41dc 0 SECTION LOCAL DEFAULT 11 .bss_start 11124: 000c41dc 0 OBJECT GLOBAL DEFAULT 11 __bss_start 11120: 000c41dc 0 NOTYPE GLOBAL DEFAULT 11 __bss_start bss_end 13: 000cbbf8 0 SECTION LOCAL DEFAULT 13 .bss_end 10442: 000cbbf8 0 OBJECT GLOBAL DEFAULT 13 __bss_end 10439: 000cbbf8 0 NOTYPE GLOBAL DEFAULT 11 __bss_end
It's worth noting that since the efi regions are affected by the change, booting with EFI is preferable while testing. Booting the kernel only should be enough since the efi stub and the kernel proper do request boottime and runtime services respectively. Something along the lines of
virtio scan && load virtio 0 $kernel_addr_r Image && bootefi $kernel_addr_r
will work for QEMU aarch64.
Tested platforms: - QEMU aarch64 - Xilinx kv260 kria starter kit & zynq - QEMU armv7 - STM32MP157C-DK2
[0] commit 3ebd1cbc49f0 ("arm: make __bss_start and __bss_end__ compiler-generated") [1] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared object")
Changes since v1: - bring back overlays for armv7 rel.dyn and bss sections and add a comment explaining why we overlay those - Remove redundant alignment from sections (new patch #7) - Added r-b tags from Sam Changes since RFC: - Rebase on top of -next and get rid of the dragonboard linker script changes. Caleb removed that file completely - Rewrite some commit messages to include the binutils bug details (thanks Sam)
Ilias Apalodimas (7): arm: baltos: remove custom linker script arm: clean up v7 and v8 linker scripts for bss_start/end arm: fix __efi_runtime_rel_start/end definitions arm: clean up v7 and v8 linker scripts for __rel_dyn_start/end arm: fix __efi_runtime_start/end definitions arm: move image_copy_start/end to linker symbols arm: remove redundant section alignments
arch/arm/cpu/armv8/u-boot-spl.lds | 21 +--- arch/arm/cpu/armv8/u-boot.lds | 38 +----- arch/arm/cpu/u-boot-spl.lds | 2 +- arch/arm/cpu/u-boot.lds | 66 +++------- arch/arm/lib/sections.c | 10 -- arch/arm/mach-aspeed/ast2600/u-boot-spl.lds | 2 +- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 21 +--- arch/arm/mach-zynq/u-boot-spl.lds | 2 +- arch/arm/mach-zynq/u-boot.lds | 67 +++------- board/vscom/baltos/u-boot.lds | 128 -------------------- include/asm-generic/sections.h | 3 + lib/efi_loader/efi_runtime.c | 1 + 12 files changed, 52 insertions(+), 309 deletions(-) delete mode 100644 board/vscom/baltos/u-boot.lds
-- 2.37.2
commit 3d74a0977f514 ("ti: am335x: Remove unused linker script") removed the linker script for the TI variant. This linker script doesn't seem to do anything special and on top of that, has no definitions for the EFI runtime sections.
So let's get rid of it and use the generic linker script which defines those correctly
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Tom Rini trini@konsulko.com --- board/vscom/baltos/u-boot.lds | 128 ---------------------------------- 1 file changed, 128 deletions(-) delete mode 100644 board/vscom/baltos/u-boot.lds
diff --git a/board/vscom/baltos/u-boot.lds b/board/vscom/baltos/u-boot.lds deleted file mode 100644 index cb2ee6769753..000000000000 --- a/board/vscom/baltos/u-boot.lds +++ /dev/null @@ -1,128 +0,0 @@ -/* - * Copyright (c) 2004-2008 Texas Instruments - * - * (C) Copyright 2002 - * Gary Jennejohn, DENX Software Engineering, garyj@denx.de - * - * See file CREDITS for list of people who contributed to this - * project. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; either version 2 of - * the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, - * MA 02111-1307 USA - */ - -OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") -OUTPUT_ARCH(arm) -ENTRY(_start) -SECTIONS -{ - . = 0x00000000; - - . = ALIGN(4); - .text : - { - *(.__image_copy_start) - *(.vectors) - CPUDIR/start.o (.text*) - board/vscom/baltos/built-in.o (.text*) - *(.text*) - } - - . = ALIGN(4); - .rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) } - - . = ALIGN(4); - .data : { - *(.data*) - } - - . = ALIGN(4); - - . = .; - - . = ALIGN(4); - __u_boot_list : { - KEEP(*(SORT(__u_boot_list*))); - } - - . = ALIGN(4); - - .image_copy_end : - { - *(.__image_copy_end) - } - - .rel_dyn_start : - { - *(.__rel_dyn_start) - } - - .rel.dyn : { - *(.rel*) - } - - .rel_dyn_end : - { - *(.__rel_dyn_end) - } - - .hash : { *(.hash*) } - - .end : - { - *(.__end) - } - - _image_binary_end = .; - - /* - * Deprecated: this MMU section is used by pxa at present but - * should not be used by new boards/CPUs. - */ - . = ALIGN(4096); - .mmutable : { - *(.mmutable) - } - -/* - * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c - * __bss_base and __bss_limit are for linker only (overlay ordering) - */ - - .bss_start __rel_dyn_start (OVERLAY) : { - KEEP(*(.__bss_start)); - __bss_base = .; - } - - .bss __bss_base (OVERLAY) : { - *(.bss*) - . = ALIGN(4); - __bss_limit = .; - } - - .bss_end __bss_limit (OVERLAY) : { - KEEP(*(.__bss_end)); - } - - .dynsym _image_binary_end : { *(.dynsym) } - .dynbss : { *(.dynbss) } - .dynstr : { *(.dynstr*) } - .dynamic : { *(.dynamic*) } - .gnu.hash : { *(.gnu.hash) } - .plt : { *(.plt*) } - .interp : { *(.interp*) } - .gnu : { *(.gnu*) } - .ARM.exidx : { *(.ARM.exidx*) } -} -- 2.37.2
commit 3ebd1cbc49f0 ("arm: make __bss_start and __bss_end__ compiler-generated") and commit f84a7b8f54db ("ARM: Fix __bss_start and __bss_end in linker scripts") were moving the bss_start/end on c generated variables that were injected in their own sections. The reason was that we needed relative relocations for position independent code and linker bugs back then prevented us from doing so [0].
However, the linker documentation pages states that symbols that are defined within a section definition will create a relocatable type with the value being a fixed offset from the base of a section [1]. So let's start cleaning this up starting with the bss_start and bss_end variables. Convert them into symbols within the .bss section definition.
[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared object") [1] https://sourceware.org/binutils/docs/ld/Expression-Section.html
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Tested-by: Caleb Connolly caleb.connolly@linaro.org # Qualcomm sdm845 --- arch/arm/cpu/armv8/u-boot-spl.lds | 14 +++----------- arch/arm/cpu/armv8/u-boot.lds | 15 +++------------ arch/arm/cpu/u-boot.lds | 21 ++++++--------------- arch/arm/lib/sections.c | 2 -- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 14 +++----------- arch/arm/mach-zynq/u-boot.lds | 21 ++++++--------------- 6 files changed, 21 insertions(+), 66 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds index 7cb9d731246d..16fddb46e9cb 100644 --- a/arch/arm/cpu/armv8/u-boot-spl.lds +++ b/arch/arm/cpu/armv8/u-boot-spl.lds @@ -63,18 +63,10 @@ SECTIONS
_image_binary_end = .;
- .bss_start (NOLOAD) : { - . = ALIGN(8); - KEEP(*(.__bss_start)); - } >.sdram - - .bss (NOLOAD) : { + .bss : { + __bss_start = .; *(.bss*) - . = ALIGN(8); - } >.sdram - - .bss_end (NOLOAD) : { - KEEP(*(.__bss_end)); + __bss_end = .; } >.sdram
/DISCARD/ : { *(.rela*) } diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds index fb6a30c922f7..c4ee10ebc3ff 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -149,19 +149,10 @@ SECTIONS
_end = .;
- . = ALIGN(8); - - .bss_start : { - KEEP(*(.__bss_start)); - } - - .bss : { + .bss ALIGN(8): { + __bss_start = .; *(.bss*) - . = ALIGN(8); - } - - .bss_end : { - KEEP(*(.__bss_end)); + __bss_end = .; }
/DISCARD/ : { *(.dynsym) } diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 7724c9332c3b..01accb109fad 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -207,23 +207,14 @@ SECTIONS }
/* - * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c - * __bss_base and __bss_limit are for linker only (overlay ordering) + * These sections occupy the same memory, but their lifetimes do + * not overlap: U-Boot initializes .bss only after applying dynamic + * relocations and therefore after it doesn't need .rel.dyn any more. */ - - .bss_start __rel_dyn_start (OVERLAY) : { - KEEP(*(.__bss_start)); - __bss_base = .; - } - - .bss __bss_base (OVERLAY) : { + .bss ADDR(.rel.dyn) (OVERLAY): { + __bss_start = .; *(.bss*) - . = ALIGN(4); - __bss_limit = .; - } - - .bss_end __bss_limit (OVERLAY) : { - KEEP(*(.__bss_end)); + __bss_end = .; }
.dynsym _image_binary_end : { *(.dynsym) } diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c index 857879711c6a..8e8bd5797e16 100644 --- a/arch/arm/lib/sections.c +++ b/arch/arm/lib/sections.c @@ -19,8 +19,6 @@ * aliasing warnings. */
-char __bss_start[0] __section(".__bss_start"); -char __bss_end[0] __section(".__bss_end"); char __image_copy_start[0] __section(".__image_copy_start"); char __image_copy_end[0] __section(".__image_copy_end"); char __rel_dyn_start[0] __section(".__rel_dyn_start"); diff --git a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds index 74618eba591b..b7887194026e 100644 --- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds +++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds @@ -56,18 +56,10 @@ SECTIONS
_image_binary_end = .;
- .bss_start (NOLOAD) : { - . = ALIGN(8); - KEEP(*(.__bss_start)); - } - - .bss (NOLOAD) : { + .bss ALIGN(8) : { + __bss_start = .; *(.bss*) - . = ALIGN(8); - } - - .bss_end (NOLOAD) : { - KEEP(*(.__bss_end)); + __bss_end = .; }
/DISCARD/ : { *(.dynsym) } diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds index 3b7c9d515f8b..25c81b21018b 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -103,23 +103,14 @@ SECTIONS _image_binary_end = .;
/* - * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c - * __bss_base and __bss_limit are for linker only (overlay ordering) + * These sections occupy the same memory, but their lifetimes do + * not overlap: U-Boot initializes .bss only after applying dynamic + * relocations and therefore after it doesn't need .rel.dyn any more. */ - - .bss_start __rel_dyn_start (OVERLAY) : { - KEEP(*(.__bss_start)); - __bss_base = .; - } - - .bss __bss_base (OVERLAY) : { + .bss ADDR(.rel.dyn) (OVERLAY): { + __bss_start = .; *(.bss*) - . = ALIGN(8); - __bss_limit = .; - } - - .bss_end __bss_limit (OVERLAY) : { - KEEP(*(.__bss_end)); + __bss_end = .; }
/* -- 2.37.2
On 3/12/24 04:08, Ilias Apalodimas wrote:
- .bss_start (NOLOAD) : {
. = ALIGN(8);
This alignment got lost.
KEEP(*(.__bss_start));- } >.sdram
- .bss (NOLOAD) : {
- .bss : {
If it is required, the best replacement would be here on the output section definition.
- . = ALIGN(8);
- .bss_start : {
KEEP(*(.__bss_start));- }
- .bss : {
- .bss ALIGN(8): {
Like you did here.
r~
Hi Richard,
Thanks for taking a look
On Tue, 12 Mar 2024 at 17:51, Richard Henderson richard.henderson@linaro.org wrote:
On 3/12/24 04:08, Ilias Apalodimas wrote:
.bss_start (NOLOAD) : {. = ALIGN(8);This alignment got lost.
KEEP(*(.__bss_start));} >.sdram.bss (NOLOAD) : {
.bss : {If it is required, the best replacement would be here on the output section definition.
As you mentioned in patch #7 we should preserve some alignments or fix the underlying assembly. But in any case, I'll preserve the .bss alignment here since it will be easy to bisect/read the commit logs & patches in the future
. = ALIGN(8);.bss_start : {KEEP(*(.__bss_start));}.bss : {
.bss ALIGN(8): {Like you did here.
Thanks! /Ilias
r~
__efi_runtime_rel_start/end are defined as c variables for arm7 only in order to force the compiler emit relative references. However, defining those within a section definition will do the same thing since [0]. On top of that the v8 linker scripts define it as a symbol.
So let's remove the special sections from the linker scripts, the variable definitions from sections.c and define them as a symbols within the correct section.
[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared object")
Suggested-by: Sam Edwards CFSworks@gmail.com Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Sam Edwards CFSworks@gmail.com Tested-by: Sam Edwards CFSworks@gmail.com # Binary output identical --- arch/arm/cpu/armv8/u-boot.lds | 4 +--- arch/arm/cpu/u-boot.lds | 16 +++------------- arch/arm/lib/sections.c | 2 -- arch/arm/mach-zynq/u-boot.lds | 16 +++------------- include/asm-generic/sections.h | 2 ++ lib/efi_loader/efi_runtime.c | 1 + 6 files changed, 10 insertions(+), 31 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds index c4ee10ebc3ff..eccb116d3cfa 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -115,9 +115,7 @@ SECTIONS KEEP(*(SORT(__u_boot_list*))); }
- . = ALIGN(8); - - .efi_runtime_rel : { + .efi_runtime_rel ALIGN(8) : { __efi_runtime_rel_start = .; *(.rel*.efi_runtime) *(.rel*.efi_runtime.*) diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 01accb109fad..37f4cd27d79a 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -152,21 +152,11 @@ SECTIONS KEEP(*(SORT(__u_boot_list*))); }
- . = ALIGN(4); - - .efi_runtime_rel_start : - { - *(.__efi_runtime_rel_start) - } - - .efi_runtime_rel : { + .efi_runtime_rel ALIGN(4) : { + __efi_runtime_rel_start = .; *(.rel*.efi_runtime) *(.rel*.efi_runtime.*) - } - - .efi_runtime_rel_stop : - { - *(.__efi_runtime_rel_stop) + __efi_runtime_rel_stop = .; }
. = ALIGN(4); diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c index 8e8bd5797e16..ddfde52163fc 100644 --- a/arch/arm/lib/sections.c +++ b/arch/arm/lib/sections.c @@ -29,6 +29,4 @@ char __secure_stack_start[0] __section(".__secure_stack_start"); char __secure_stack_end[0] __section(".__secure_stack_end"); char __efi_runtime_start[0] __section(".__efi_runtime_start"); char __efi_runtime_stop[0] __section(".__efi_runtime_stop"); -char __efi_runtime_rel_start[0] __section(".__efi_runtime_rel_start"); -char __efi_runtime_rel_stop[0] __section(".__efi_runtime_rel_stop"); char _end[0] __section(".__end"); diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds index 25c81b21018b..11b6e7278a9c 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -58,21 +58,11 @@ SECTIONS KEEP(*(SORT(__u_boot_list*))); }
- . = ALIGN(4); - - .efi_runtime_rel_start : - { - *(.__efi_runtime_rel_start) - } - - .efi_runtime_rel : { + .efi_runtime_rel ALIGN(4) : { + __efi_runtime_rel_start = .; *(.rel*.efi_runtime) *(.rel*.efi_runtime.*) - } - - .efi_runtime_rel_stop : - { - *(.__efi_runtime_rel_stop) + __efi_runtime_rel_stop = .; }
. = ALIGN(8); diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index 1e1657a01673..60949200dd93 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -34,6 +34,8 @@ extern char __priv_data_start[], __priv_data_end[]; /* Start and end of .ctors section - used for constructor calls. */ extern char __ctors_start[], __ctors_end[];
+extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; + /* function descriptor handling (if any). Override * in asm/sections.h */ #ifndef dereference_function_descriptor diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 18da6892e796..9185f1894c47 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -15,6 +15,7 @@ #include <rtc.h> #include <asm/global_data.h> #include <u-boot/crc.h> +#include <asm/sections.h>
/* For manual relocation support */ DECLARE_GLOBAL_DATA_PTR; -- 2.37.2
On 3/12/24 04:08, Ilias Apalodimas wrote:
__efi_runtime_rel_start/end are defined as c variables for arm7 only in order to force the compiler emit relative references. However, defining those within a section definition will do the same thing since [0]. On top of that the v8 linker scripts define it as a symbol.
So let's remove the special sections from the linker scripts, the variable definitions from sections.c and define them as a symbols within the correct section.
[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared object")
Suggested-by: Sam EdwardsCFSworks@gmail.com Signed-off-by: Ilias Apalodimasilias.apalodimas@linaro.org Reviewed-by: Sam EdwardsCFSworks@gmail.com Tested-by: Sam EdwardsCFSworks@gmail.com # Binary output identical
arch/arm/cpu/armv8/u-boot.lds | 4 +--- arch/arm/cpu/u-boot.lds | 16 +++------------- arch/arm/lib/sections.c | 2 -- arch/arm/mach-zynq/u-boot.lds | 16 +++------------- include/asm-generic/sections.h | 2 ++ lib/efi_loader/efi_runtime.c | 1 + 6 files changed, 10 insertions(+), 31 deletions(-)
Reviewed-by: Richard Henderson richard.henderson@linaro.org
r~
commit 47bd65ef057f ("arm: make __rel_dyn_{start, end} compiler-generated") were moving the __rel_dyn_start/end on c generated variables that were injected in their own sections. The reason was that we needed relative relocations for position independent code and linker bugs back then prevented us from doing so [0].
However, the linker documentation pages states that symbols that are defined within a section definition will create a relocatable type with the value being a fixed offset from the base of a section [1].
[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared object") [1] https://sourceware.org/binutils/docs/ld/Expression-Section.html
Suggested-by: Sam Edwards CFSworks@gmail.com Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Sam Edwards CFSworks@gmail.com Tested-by: Sam Edwards CFSworks@gmail.com # Binary output identical --- arch/arm/cpu/armv8/u-boot.lds | 16 +++------------- arch/arm/cpu/u-boot.lds | 14 +++----------- arch/arm/lib/sections.c | 2 -- arch/arm/mach-zynq/u-boot.lds | 14 +++----------- 4 files changed, 9 insertions(+), 37 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds index eccb116d3cfa..e737de761a9d 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -129,20 +129,10 @@ SECTIONS *(.__image_copy_end) }
- . = ALIGN(8); - - .rel_dyn_start : - { - *(.__rel_dyn_start) - } - - .rela.dyn : { + .rela.dyn ALIGN(8) : { + __rel_dyn_start = .; *(.rela*) - } - - .rel_dyn_end : - { - *(.__rel_dyn_end) + __rel_dyn_end = .; }
_end = .; diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 37f4cd27d79a..b1065ec8063d 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -166,18 +166,10 @@ SECTIONS *(.__image_copy_end) }
- .rel_dyn_start : - { - *(.__rel_dyn_start) - } - - .rel.dyn : { + .rel.dyn ALIGN(4) : { + __rel_dyn_start = .; *(.rel*) - } - - .rel_dyn_end : - { - *(.__rel_dyn_end) + __rel_dyn_end = .; }
.end : diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c index ddfde52163fc..1ee3dd3667ba 100644 --- a/arch/arm/lib/sections.c +++ b/arch/arm/lib/sections.c @@ -21,8 +21,6 @@
char __image_copy_start[0] __section(".__image_copy_start"); char __image_copy_end[0] __section(".__image_copy_end"); -char __rel_dyn_start[0] __section(".__rel_dyn_start"); -char __rel_dyn_end[0] __section(".__rel_dyn_end"); char __secure_start[0] __section(".__secure_start"); char __secure_end[0] __section(".__secure_end"); char __secure_stack_start[0] __section(".__secure_stack_start"); diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds index 11b6e7278a9c..16245fd4ac2f 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -71,18 +71,10 @@ SECTIONS *(.__image_copy_end) }
- .rel_dyn_start : - { - *(.__rel_dyn_start) - } - - .rel.dyn : { + .rel.dyn ALIGN(8) : { + __rel_dyn_start = .; *(.rel*) - } - - .rel_dyn_end : - { - *(.__rel_dyn_end) + __rel_dyn_end = .; }
.end : -- 2.37.2
On 3/12/24 04:08, Ilias Apalodimas wrote:
commit 47bd65ef057f ("arm: make __rel_dyn_{start, end} compiler-generated") were moving the __rel_dyn_start/end on c generated variables that were injected in their own sections. The reason was that we needed relative relocations for position independent code and linker bugs back then prevented us from doing so [0].
However, the linker documentation pages states that symbols that are defined within a section definition will create a relocatable type with the value being a fixed offset from the base of a section [1].
[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared object") [1]https://sourceware.org/binutils/docs/ld/Expression-Section.html
Suggested-by: Sam EdwardsCFSworks@gmail.com Signed-off-by: Ilias Apalodimasilias.apalodimas@linaro.org Reviewed-by: Sam EdwardsCFSworks@gmail.com Tested-by: Sam EdwardsCFSworks@gmail.com # Binary output identical
arch/arm/cpu/armv8/u-boot.lds | 16 +++------------- arch/arm/cpu/u-boot.lds | 14 +++----------- arch/arm/lib/sections.c | 2 -- arch/arm/mach-zynq/u-boot.lds | 14 +++----------- 4 files changed, 9 insertions(+), 37 deletions(-)
Reviewed-by: Richard Henderson richard.henderson@linaro.org
r~
__efi_runtime_start/end are defined as c variables for arm7 only in order to force the compiler emit relative references. However, defining those within a section definition will do the same thing since [0]. On top of that the v8 linker scripts define it as a symbol.
So let's remove the special sections from the linker scripts, the variable definitions from sections.c and define them as a symbols within the correct section.
[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared object")
Suggested-by: Sam Edwards CFSworks@gmail.com Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Sam Edwards CFSworks@gmail.com Tested-by: Sam Edwards CFSworks@gmail.com # Binary output identical --- arch/arm/cpu/u-boot.lds | 12 +++--------- arch/arm/lib/sections.c | 2 -- arch/arm/mach-zynq/u-boot.lds | 12 +++--------- include/asm-generic/sections.h | 1 + 4 files changed, 7 insertions(+), 20 deletions(-)
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index b1065ec8063d..df979986a5ef 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -43,18 +43,12 @@ SECTIONS }
/* This needs to come before *(.text*) */ - .__efi_runtime_start : { - *(.__efi_runtime_start) - } - - .efi_runtime : { + .efi_runtime ALIGN(4) : { + __efi_runtime_start = .; *(.text.efi_runtime*) *(.rodata.efi_runtime*) *(.data.efi_runtime*) - } - - .__efi_runtime_stop : { - *(.__efi_runtime_stop) + __efi_runtime_stop = .; }
.text_rest : diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c index 1ee3dd3667ba..a4d4202e99f5 100644 --- a/arch/arm/lib/sections.c +++ b/arch/arm/lib/sections.c @@ -25,6 +25,4 @@ char __secure_start[0] __section(".__secure_start"); char __secure_end[0] __section(".__secure_end"); char __secure_stack_start[0] __section(".__secure_stack_start"); char __secure_stack_end[0] __section(".__secure_stack_end"); -char __efi_runtime_start[0] __section(".__efi_runtime_start"); -char __efi_runtime_stop[0] __section(".__efi_runtime_stop"); char _end[0] __section(".__end"); diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds index 16245fd4ac2f..fc0ba0ed9a84 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -22,18 +22,12 @@ SECTIONS }
/* This needs to come before *(.text*) */ - .__efi_runtime_start : { - *(.__efi_runtime_start) - } - - .efi_runtime : { + .efi_runtime ALIGN(4) : { + __efi_runtime_start = .; *(.text.efi_runtime*) *(.rodata.efi_runtime*) *(.data.efi_runtime*) - } - - .__efi_runtime_stop : { - *(.__efi_runtime_stop) + __efi_runtime_stop = .; }
.text_rest : diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index 60949200dd93..b6bca53db10d 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[]; extern char __ctors_start[], __ctors_end[];
extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; +extern char __efi_runtime_start[], __efi_runtime_stop[];
/* function descriptor handling (if any). Override * in asm/sections.h */ -- 2.37.2
On 3/12/24 04:08, Ilias Apalodimas wrote:
__efi_runtime_start/end are defined as c variables for arm7 only in order to force the compiler emit relative references. However, defining those within a section definition will do the same thing since [0]. On top of that the v8 linker scripts define it as a symbol.
So let's remove the special sections from the linker scripts, the variable definitions from sections.c and define them as a symbols within the correct section.
[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared object")
Suggested-by: Sam EdwardsCFSworks@gmail.com Signed-off-by: Ilias Apalodimasilias.apalodimas@linaro.org Reviewed-by: Sam EdwardsCFSworks@gmail.com Tested-by: Sam EdwardsCFSworks@gmail.com # Binary output identical
arch/arm/cpu/u-boot.lds | 12 +++--------- arch/arm/lib/sections.c | 2 -- arch/arm/mach-zynq/u-boot.lds | 12 +++--------- include/asm-generic/sections.h | 1 + 4 files changed, 7 insertions(+), 20 deletions(-)
Reviewed-by: Richard Henderson richard.henderson@linaro.org
r~
image_copy_start/end are defined as c variables in order to force the compiler emit relative references. However, defining those within a section definition will do the same thing since [0].
So let's remove the special sections from the linker scripts, the variable definitions from sections.c and define them as a symbols within a section.
[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared object")
Suggested-by: Sam Edwards CFSworks@gmail.com Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Tested-by: Sam Edwards CFSworks@gmail.com # Binary output identical --- arch/arm/cpu/armv8/u-boot-spl.lds | 7 ++----- arch/arm/cpu/armv8/u-boot.lds | 9 ++------- arch/arm/cpu/u-boot-spl.lds | 2 +- arch/arm/cpu/u-boot.lds | 9 ++------- arch/arm/lib/sections.c | 2 -- arch/arm/mach-aspeed/ast2600/u-boot-spl.lds | 2 +- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 7 ++----- arch/arm/mach-zynq/u-boot-spl.lds | 2 +- arch/arm/mach-zynq/u-boot.lds | 8 ++------ 9 files changed, 13 insertions(+), 35 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds index 16fddb46e9cb..6417cbd08e2c 100644 --- a/arch/arm/cpu/armv8/u-boot-spl.lds +++ b/arch/arm/cpu/armv8/u-boot-spl.lds @@ -21,9 +21,9 @@ OUTPUT_ARCH(aarch64) ENTRY(_start) SECTIONS { + __image_copy_start = ADDR(.text); .text : { . = ALIGN(8); - __image_copy_start = .; CPUDIR/start.o (.text*) *(.text*) } >.sram @@ -51,10 +51,7 @@ SECTIONS KEEP(*(SORT(__u_boot_list*))); } >.sram
- .image_copy_end : { - . = ALIGN(8); - *(.__image_copy_end) - } >.sram + __image_copy_end = .;
.end : { . = ALIGN(8); diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds index e737de761a9d..33f4624b561d 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -21,9 +21,9 @@ SECTIONS . = 0x00000000;
. = ALIGN(8); + __image_copy_start = ADDR(.text); .text : { - *(.__image_copy_start) CPUDIR/start.o (.text*) }
@@ -122,12 +122,7 @@ SECTIONS __efi_runtime_rel_stop = .; }
- . = ALIGN(8); - - .image_copy_end : - { - *(.__image_copy_end) - } + __image_copy_end = .;
.rela.dyn ALIGN(8) : { __rel_dyn_start = .; diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds index fb2189d50dea..9ed62395a9c5 100644 --- a/arch/arm/cpu/u-boot-spl.lds +++ b/arch/arm/cpu/u-boot-spl.lds @@ -14,9 +14,9 @@ SECTIONS . = 0x00000000;
. = ALIGN(4); + __image_copy_start = ADDR(.text); .text : { - __image_copy_start = .; *(.vectors) CPUDIR/start.o (.text*) *(.text*) diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index df979986a5ef..b6b19a4174fe 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -35,9 +35,9 @@ SECTIONS . = 0x00000000;
. = ALIGN(4); + __image_copy_start = ADDR(.text); .text : { - *(.__image_copy_start) *(.vectors) CPUDIR/start.o (.text*) } @@ -153,12 +153,7 @@ SECTIONS __efi_runtime_rel_stop = .; }
- . = ALIGN(4); - - .image_copy_end : - { - *(.__image_copy_end) - } + __image_copy_end = .;
.rel.dyn ALIGN(4) : { __rel_dyn_start = .; diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c index a4d4202e99f5..db5463b2bbbc 100644 --- a/arch/arm/lib/sections.c +++ b/arch/arm/lib/sections.c @@ -19,8 +19,6 @@ * aliasing warnings. */
-char __image_copy_start[0] __section(".__image_copy_start"); -char __image_copy_end[0] __section(".__image_copy_end"); char __secure_start[0] __section(".__secure_start"); char __secure_end[0] __section(".__secure_end"); char __secure_stack_start[0] __section(".__secure_stack_start"); diff --git a/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds b/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds index 37f0ccd92201..ada6570d9712 100644 --- a/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds +++ b/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds @@ -22,9 +22,9 @@ SECTIONS . = 0x00000000;
. = ALIGN(4); + __image_copy_start = ADDR(.text); .text : { - __image_copy_start = .; *(.vectors) CPUDIR/start.o (.text*) *(.text*) diff --git a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds index b7887194026e..8ff35b5c17cf 100644 --- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds +++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds @@ -22,9 +22,9 @@ SECTIONS { . = 0x00000000;
+ __image_copy_start = ADDR(.text); .text : { . = ALIGN(8); - *(.__image_copy_start) CPUDIR/start.o (.text*) *(.text*) } @@ -44,10 +44,7 @@ SECTIONS KEEP(*(SORT(__u_boot_list*))); }
- .image_copy_end : { - . = ALIGN(8); - *(.__image_copy_end) - } + __image_copy_end = .;
.end : { . = ALIGN(8); diff --git a/arch/arm/mach-zynq/u-boot-spl.lds b/arch/arm/mach-zynq/u-boot-spl.lds index 8c18d3f91f4b..d96a57702886 100644 --- a/arch/arm/mach-zynq/u-boot-spl.lds +++ b/arch/arm/mach-zynq/u-boot-spl.lds @@ -18,9 +18,9 @@ ENTRY(_start) SECTIONS { . = ALIGN(4); + __image_copy_start = ADDR(.text); .text : { - __image_copy_start = .; *(.vectors) CPUDIR/start.o (.text*) *(.text*) diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds index fc0ba0ed9a84..f739d1cfce85 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -14,9 +14,9 @@ SECTIONS . = 0x00000000;
. = ALIGN(4); + __image_copy_start = ADDR(.text); .text : { - *(.__image_copy_start) *(.vectors) CPUDIR/start.o (.text*) } @@ -59,11 +59,7 @@ SECTIONS __efi_runtime_rel_stop = .; }
- . = ALIGN(8); - .image_copy_end : - { - *(.__image_copy_end) - } + __image_copy_end = .;
.rel.dyn ALIGN(8) : { __rel_dyn_start = .; -- 2.37.2
On 3/12/24 04:08, Ilias Apalodimas wrote:
image_copy_start/end are defined as c variables in order to force the compiler emit relative references. However, defining those within a section definition will do the same thing since [0].
So let's remove the special sections from the linker scripts, the variable definitions from sections.c and define them as a symbols within a section.
[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared object")
Suggested-by: Sam EdwardsCFSworks@gmail.com Signed-off-by: Ilias Apalodimasilias.apalodimas@linaro.org Tested-by: Sam EdwardsCFSworks@gmail.com # Binary output identical
arch/arm/cpu/armv8/u-boot-spl.lds | 7 ++----- arch/arm/cpu/armv8/u-boot.lds | 9 ++------- arch/arm/cpu/u-boot-spl.lds | 2 +- arch/arm/cpu/u-boot.lds | 9 ++------- arch/arm/lib/sections.c | 2 -- arch/arm/mach-aspeed/ast2600/u-boot-spl.lds | 2 +- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 7 ++----- arch/arm/mach-zynq/u-boot-spl.lds | 2 +- arch/arm/mach-zynq/u-boot.lds | 8 ++------ 9 files changed, 13 insertions(+), 35 deletions(-)
Reviewed-by: Richard Henderson richard.henderson@linaro.org
r~
Previous patches cleaning up linker symbols, also merged any explicit . = ALIGN(x); into section definitions -- e.g '.bss ALIGN(x) :' instead of '. - ALIGN(x); bss : {...}'
However, if the output address is not specified then one will be chosen for the section. This address will be adjusted to fit the alignment requirement of the output section following the strictest alignment of any input section contained within the output section. So let's get rid of the redundant ALIGN directives.
It's worth noting that the only platform the alignment is preserved on .rel.dyn is mach-zynq, which was explicitly aligning that section on an 8b boundary instead of the automatically chosen 4b alignment.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- arch/arm/cpu/armv8/u-boot.lds | 6 +++--- arch/arm/cpu/u-boot.lds | 6 +++--- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 2 +- arch/arm/mach-zynq/u-boot.lds | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds index 33f4624b561d..ccdd1966cfbc 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -115,7 +115,7 @@ SECTIONS KEEP(*(SORT(__u_boot_list*))); }
- .efi_runtime_rel ALIGN(8) : { + .efi_runtime_rel : { __efi_runtime_rel_start = .; *(.rel*.efi_runtime) *(.rel*.efi_runtime.*) @@ -124,7 +124,7 @@ SECTIONS
__image_copy_end = .;
- .rela.dyn ALIGN(8) : { + .rela.dyn : { __rel_dyn_start = .; *(.rela*) __rel_dyn_end = .; @@ -132,7 +132,7 @@ SECTIONS
_end = .;
- .bss ALIGN(8): { + .bss : { __bss_start = .; *(.bss*) __bss_end = .; diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index b6b19a4174fe..a9fcbbf22e96 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -43,7 +43,7 @@ SECTIONS }
/* This needs to come before *(.text*) */ - .efi_runtime ALIGN(4) : { + .efi_runtime : { __efi_runtime_start = .; *(.text.efi_runtime*) *(.rodata.efi_runtime*) @@ -146,7 +146,7 @@ SECTIONS KEEP(*(SORT(__u_boot_list*))); }
- .efi_runtime_rel ALIGN(4) : { + .efi_runtime_rel : { __efi_runtime_rel_start = .; *(.rel*.efi_runtime) *(.rel*.efi_runtime.*) @@ -155,7 +155,7 @@ SECTIONS
__image_copy_end = .;
- .rel.dyn ALIGN(4) : { + .rel.dyn : { __rel_dyn_start = .; *(.rel*) __rel_dyn_end = .; diff --git a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds index 8ff35b5c17cf..e67416c09c1c 100644 --- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds +++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds @@ -53,7 +53,7 @@ SECTIONS
_image_binary_end = .;
- .bss ALIGN(8) : { + .bss : { __bss_start = .; *(.bss*) __bss_end = .; diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds index f739d1cfce85..111410215072 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -22,7 +22,7 @@ SECTIONS }
/* This needs to come before *(.text*) */ - .efi_runtime ALIGN(4) : { + .efi_runtime : { __efi_runtime_start = .; *(.text.efi_runtime*) *(.rodata.efi_runtime*) @@ -52,7 +52,7 @@ SECTIONS KEEP(*(SORT(__u_boot_list*))); }
- .efi_runtime_rel ALIGN(4) : { + .efi_runtime_rel : { __efi_runtime_rel_start = .; *(.rel*.efi_runtime) *(.rel*.efi_runtime.*) -- 2.37.2
On 3/12/24 04:08, Ilias Apalodimas wrote:
index 33f4624b561d..ccdd1966cfbc 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -132,7 +132,7 @@ SECTIONS
_end = .;
- .bss ALIGN(8): {
- .bss : { __bss_start = .; *(.bss*) __bss_end = .;
The code in arch/arm/lib/crt0_64.S assumes __bss_end - __bss_start is a multiple of 8. But that could probably be replaced by a proper call to memset fairly easily.
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index b6b19a4174fe..a9fcbbf22e96 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -155,7 +155,7 @@ SECTIONS
__image_copy_end = .;
- .rel.dyn ALIGN(4) : {
- .rel.dyn : { __rel_dyn_start = .; *(.rel*) __rel_dyn_end = .;
Because of the overlay, this affects .bss too.
The code in arch/arm/lib/crt0.S may or may not be configured to use memset. When it isn't, it requires __bss_end - __bss_start to be a multiple of 4. Why does this not always use memset?
r~
Hi Richard,
Pasting some of the discussions we had over IRC for completeness.
On Tue, 12 Mar 2024 at 19:00, Richard Henderson richard.henderson@linaro.org wrote:
On 3/12/24 04:08, Ilias Apalodimas wrote:
index 33f4624b561d..ccdd1966cfbc 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -132,7 +132,7 @@ SECTIONS
_end = .;
.bss ALIGN(8): {
.bss : { __bss_start = .; *(.bss*) __bss_end = .;The code in arch/arm/lib/crt0_64.S assumes __bss_end - __bss_start is a multiple of 8. But that could probably be replaced by a proper call to memset fairly easily.
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index b6b19a4174fe..a9fcbbf22e96 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -155,7 +155,7 @@ SECTIONS
__image_copy_end = .;
.rel.dyn ALIGN(4) : {
.rel.dyn : { __rel_dyn_start = .; *(.rel*) __rel_dyn_end = .;Because of the overlay, this affects .bss too.
The code in arch/arm/lib/crt0.S may or may not be configured to use memset. When it isn't, it requires __bss_end - __bss_start to be a multiple of 4. Why does this not always use memset?
I think it's due to size restrictions in SPL. Since I can't test all affected platforms, I prefer leaving it as is at least for now. I'll explicitly align sections to 4/8 bytes in v3. Once we get this merged we can try and clean those up as you suggested on a followup patchset
Thanks /Ilias
r~
participants (2)
-
Ilias Apalodimas -
Richard Henderson