[RFC PATCH 0/6] 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]. This has led to confusion over the years, ending up with mixed section definitions. Some sections being 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.
I am not sure if this used to be a compiler bug, but 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.
The resulting binary (tested in QEMU v7/v8) had no size differences apart from the emited sections and object types of those variables. I've also added prints throughout the U-Boot init sequence. The offsets and delta for 'end - start' section sizes is unchanged.
For example on QEMU v8: $~ ./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%
$~ readelf -sW u-boot | grep bss_s 12: 00000000001029b8 0 SECTION LOCAL DEFAULT 12 .bss_start 8088: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8376: 00000000001029b8 0 OBJECT GLOBAL DEFAULT 12 __bss_start $~ readelf -sW u-boot.new | grep bss_s 8085: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8373: 00000000001029b8 0 NOTYPE GLOBAL DEFAULT 12 __bss_start
For QEMU v7 the differences are a bit bigger but only affect the variables placed in the .bss section because that was defined as an OVERLAY in the existing linker script. For example: $~ nm u-boot | grep tftp_prev_block 000ca0dc ? tftp_prev_block $~ nm u-boot.new | grep tftp_prev_block 000e0a5c b tftp_prev_block -----> The symbol is now placed into .bss
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")
Ilias Apalodimas (6): 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
arch/arm/cpu/armv8/u-boot-spl.lds | 14 +-- arch/arm/cpu/armv8/u-boot.lds | 45 ++------ arch/arm/cpu/u-boot.lds | 74 +++---------- arch/arm/lib/sections.c | 10 -- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 22 +--- arch/arm/mach-zynq/u-boot.lds | 72 +++--------- board/qualcomm/dragonboard820c/u-boot.lds | 41 ++----- board/vscom/baltos/u-boot.lds | 128 ---------------------- include/asm-generic/sections.h | 3 + lib/efi_loader/efi_runtime.c | 1 + 10 files changed, 58 insertions(+), 352 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 --- 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
On Wed, Feb 28, 2024 at 12:48:04PM +0200, Ilias Apalodimas wrote:
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
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.
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.
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] https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_mono/ld.html#SEC13
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- arch/arm/cpu/armv8/u-boot-spl.lds | 14 +++----------- arch/arm/cpu/armv8/u-boot.lds | 15 +++------------ arch/arm/cpu/u-boot.lds | 22 ++++------------------ arch/arm/lib/sections.c | 2 -- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 14 +++----------- arch/arm/mach-zynq/u-boot.lds | 21 +++------------------ board/qualcomm/dragonboard820c/u-boot.lds | 15 +++------------ 7 files changed, 19 insertions(+), 84 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..90d329b1ebe0 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -206,27 +206,13 @@ SECTIONS *(.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_start = .; *(.bss*) - . = ALIGN(4); - __bss_limit = .; - } - - .bss_end __bss_limit (OVERLAY) : { - KEEP(*(.__bss_end)); + __bss_end = .; }
- .dynsym _image_binary_end : { *(.dynsym) } + .dynsym : { *(.dynsym) } .dynbss : { *(.dynbss) } .dynstr : { *(.dynstr*) } .dynamic : { *(.dynamic*) } 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..8d3259821719 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -102,26 +102,11 @@ 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) - */ - - .bss_start __rel_dyn_start (OVERLAY) : { - KEEP(*(.__bss_start)); - __bss_base = .; - } - - .bss __bss_base (OVERLAY) : { + .bss ALIGN(4): { + __bss_start = .; *(.bss*) - . = ALIGN(8); - __bss_limit = .; - } - - .bss_end __bss_limit (OVERLAY) : { - KEEP(*(.__bss_end)); + __bss_end = .; } - /* * Zynq needs to discard these sections because the user * is expected to pass this image on to tools for boot.bin diff --git a/board/qualcomm/dragonboard820c/u-boot.lds b/board/qualcomm/dragonboard820c/u-boot.lds index 5251b59fbe76..d3cc5278b610 100644 --- a/board/qualcomm/dragonboard820c/u-boot.lds +++ b/board/qualcomm/dragonboard820c/u-boot.lds @@ -87,19 +87,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) } -- 2.37.2
On 28/02/2024 10:48, Ilias Apalodimas wrote:
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.
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.
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] https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_mono/ld.html#SEC13
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Tested-by: Caleb Connolly caleb.connolly@linaro.org # Qualcomm sdm845
This patch and a few others in this series conflict with my patch [1] which drops the dragonboard820c linker script. My patch doesn't have any external dependencies, so would it be easier for you to take it along with this series? Or how best should we avoid the merge conflicts here?
[1]: https://lore.kernel.org/u-boot/20240226-b4-qcom-common-target-v5-19-10c8e078...
Thanks and regards,
arch/arm/cpu/armv8/u-boot-spl.lds | 14 +++----------- arch/arm/cpu/armv8/u-boot.lds | 15 +++------------ arch/arm/cpu/u-boot.lds | 22 ++++------------------ arch/arm/lib/sections.c | 2 -- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 14 +++----------- arch/arm/mach-zynq/u-boot.lds | 21 +++------------------ board/qualcomm/dragonboard820c/u-boot.lds | 15 +++------------ 7 files changed, 19 insertions(+), 84 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 = .;
. = 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 = .;
. = 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..90d329b1ebe0 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -206,27 +206,13 @@ SECTIONS *(.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_start = .;
. = ALIGN(4);__bss_limit = .;- }
- .bss_end __bss_limit (OVERLAY) : {
KEEP(*(.__bss_end));
__bss_end = .;
- .dynsym _image_binary_end : { *(.dynsym) }
- .dynsym : { *(.dynsym) } .dynbss : { *(.dynbss) } .dynstr : { *(.dynstr*) } .dynamic : { *(.dynamic*) }
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 = .;
. = 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..8d3259821719 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -102,26 +102,11 @@ 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)
- */
- .bss_start __rel_dyn_start (OVERLAY) : {
KEEP(*(.__bss_start));__bss_base = .;- }
- .bss __bss_base (OVERLAY) : {
- .bss ALIGN(4): {
__bss_start = .;
. = ALIGN(8);__bss_limit = .;- }
- .bss_end __bss_limit (OVERLAY) : {
KEEP(*(.__bss_end));
__bss_end = .;
- /*
- Zynq needs to discard these sections because the user
- is expected to pass this image on to tools for boot.bin
diff --git a/board/qualcomm/dragonboard820c/u-boot.lds b/board/qualcomm/dragonboard820c/u-boot.lds index 5251b59fbe76..d3cc5278b610 100644 --- a/board/qualcomm/dragonboard820c/u-boot.lds +++ b/board/qualcomm/dragonboard820c/u-boot.lds @@ -87,19 +87,10 @@ SECTIONS
_end = .;
- . = ALIGN(8);
- .bss_start : {
KEEP(*(.__bss_start));- }
- .bss : {
- .bss ALIGN(8): {
__bss_start = .;
. = ALIGN(8);- }
- .bss_end : {
KEEP(*(.__bss_end));
__bss_end = .;}
/DISCARD/ : { *(.dynsym) }
-- 2.37.2
Thanks for testing Caleb,
On Thu, 29 Feb 2024 at 15:49, Caleb Connolly caleb.connolly@linaro.org wrote:
On 28/02/2024 10:48, Ilias Apalodimas wrote:
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.
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.
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] https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_mono/ld.html#SEC13
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Tested-by: Caleb Connolly caleb.connolly@linaro.org # Qualcomm sdm845
This patch and a few others in this series conflict with my patch [1] which drops the dragonboard820c linker script. My patch doesn't have any external dependencies, so would it be easier for you to take it along with this series? Or how best should we avoid the merge conflicts here?
Yes, I can carry the snapdragon lds removal. Rebasing on top is of it is pretty easy
/Ilias
Thanks and regards,
arch/arm/cpu/armv8/u-boot-spl.lds | 14 +++----------- arch/arm/cpu/armv8/u-boot.lds | 15 +++------------ arch/arm/cpu/u-boot.lds | 22 ++++------------------ arch/arm/lib/sections.c | 2 -- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 14 +++----------- arch/arm/mach-zynq/u-boot.lds | 21 +++------------------ board/qualcomm/dragonboard820c/u-boot.lds | 15 +++------------ 7 files changed, 19 insertions(+), 84 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..90d329b1ebe0 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -206,27 +206,13 @@ SECTIONS *(.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_start = .; *(.bss*)
. = ALIGN(4);__bss_limit = .;}.bss_end __bss_limit (OVERLAY) : {KEEP(*(.__bss_end));
__bss_end = .; }
.dynsym _image_binary_end : { *(.dynsym) }
.dynsym : { *(.dynsym) } .dynbss : { *(.dynbss) } .dynstr : { *(.dynstr*) } .dynamic : { *(.dynamic*) }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..8d3259821719 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -102,26 +102,11 @@ 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)
- */
.bss_start __rel_dyn_start (OVERLAY) : {KEEP(*(.__bss_start));__bss_base = .;}.bss __bss_base (OVERLAY) : {
.bss ALIGN(4): {__bss_start = .; *(.bss*)
. = ALIGN(8);__bss_limit = .;}.bss_end __bss_limit (OVERLAY) : {KEEP(*(.__bss_end));
__bss_end = .; }
/* * Zynq needs to discard these sections because the user * is expected to pass this image on to tools for boot.bindiff --git a/board/qualcomm/dragonboard820c/u-boot.lds b/board/qualcomm/dragonboard820c/u-boot.lds index 5251b59fbe76..d3cc5278b610 100644 --- a/board/qualcomm/dragonboard820c/u-boot.lds +++ b/board/qualcomm/dragonboard820c/u-boot.lds @@ -87,19 +87,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) }-- 2.37.2
-- // Caleb (they/them)
__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. 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.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- 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 90d329b1ebe0..70e78ce46672 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 8d3259821719..66a9e37f9198 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
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.
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 [0].
[0] https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_mono/ld.html#SEC13
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- 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 +++----------- board/qualcomm/dragonboard820c/u-boot.lds | 15 +++------------ 5 files changed, 12 insertions(+), 49 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 70e78ce46672..7c6e7891d360 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 66a9e37f9198..71dea4a1f60a 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 : diff --git a/board/qualcomm/dragonboard820c/u-boot.lds b/board/qualcomm/dragonboard820c/u-boot.lds index d3cc5278b610..2969f139bfd2 100644 --- a/board/qualcomm/dragonboard820c/u-boot.lds +++ b/board/qualcomm/dragonboard820c/u-boot.lds @@ -69,20 +69,11 @@ 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 = .; -- 2.37.2
__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. 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.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- 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 7c6e7891d360..df55bb716e35 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 71dea4a1f60a..fcd0f42a7106 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
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.
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.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- arch/arm/cpu/armv8/u-boot.lds | 10 ++-------- arch/arm/cpu/u-boot.lds | 10 ++-------- arch/arm/lib/sections.c | 2 -- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 8 ++------ arch/arm/mach-zynq/u-boot.lds | 9 ++------- board/qualcomm/dragonboard820c/u-boot.lds | 11 ++--------- 6 files changed, 10 insertions(+), 40 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds index e737de761a9d..340acf5c043b 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -23,7 +23,7 @@ SECTIONS . = ALIGN(8); .text : { - *(.__image_copy_start) + __image_copy_start = .; CPUDIR/start.o (.text*) }
@@ -120,13 +120,7 @@ SECTIONS *(.rel*.efi_runtime) *(.rel*.efi_runtime.*) __efi_runtime_rel_stop = .; - } - - . = ALIGN(8); - - .image_copy_end : - { - *(.__image_copy_end) + __image_copy_end = .; }
.rela.dyn ALIGN(8) : { diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index df55bb716e35..7c4f9c2d4dd3 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -37,7 +37,7 @@ SECTIONS . = ALIGN(4); .text : { - *(.__image_copy_start) + __image_copy_start = .; *(.vectors) CPUDIR/start.o (.text*) } @@ -151,13 +151,7 @@ SECTIONS *(.rel*.efi_runtime) *(.rel*.efi_runtime.*) __efi_runtime_rel_stop = .; - } - - . = ALIGN(4); - - .image_copy_end : - { - *(.__image_copy_end) + __image_copy_end = .; }
.rel.dyn ALIGN(4) : { 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-rockchip/u-boot-tpl-v8.lds b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds index b7887194026e..4b480ebb0c4d 100644 --- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds +++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds @@ -24,7 +24,7 @@ SECTIONS
.text : { . = ALIGN(8); - *(.__image_copy_start) + __image_copy_start = .; CPUDIR/start.o (.text*) *(.text*) } @@ -42,11 +42,7 @@ SECTIONS __u_boot_list : { . = ALIGN(8); KEEP(*(SORT(__u_boot_list*))); - } - - .image_copy_end : { - . = ALIGN(8); - *(.__image_copy_end) + __image_copy_end = .; }
.end : { diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds index fcd0f42a7106..553bcf182272 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -16,7 +16,7 @@ SECTIONS . = ALIGN(4); .text : { - *(.__image_copy_start) + __image_copy_start = .; *(.vectors) CPUDIR/start.o (.text*) } @@ -57,12 +57,7 @@ SECTIONS *(.rel*.efi_runtime) *(.rel*.efi_runtime.*) __efi_runtime_rel_stop = .; - } - - . = ALIGN(8); - .image_copy_end : - { - *(.__image_copy_end) + __image_copy_end = .; }
.rel.dyn ALIGN(8) : { diff --git a/board/qualcomm/dragonboard820c/u-boot.lds b/board/qualcomm/dragonboard820c/u-boot.lds index 2969f139bfd2..f8d3c030ef32 100644 --- a/board/qualcomm/dragonboard820c/u-boot.lds +++ b/board/qualcomm/dragonboard820c/u-boot.lds @@ -17,7 +17,7 @@ SECTIONS . = ALIGN(8); .text : { - *(.__image_copy_start) + __image_copy_start = .; board/qualcomm/dragonboard820c/head.o (.text*) CPUDIR/start.o (.text*) } @@ -60,16 +60,9 @@ SECTIONS *(.rel*.efi_runtime) *(.rel*.efi_runtime.*) __efi_runtime_rel_stop = .; + __image_copy_end = .; }
- . = ALIGN(8); - - .image_copy_end : - { - *(.__image_copy_end) - } - - .rela.dyn ALIGN(8) : { __rel_dyn_start = .; *(.rela*) -- 2.37.2
On Wed, 28 Feb 2024 at 10:58, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
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]. This has led to confusion over the years, ending up with mixed section definitions. Some sections being 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.
I am not sure if this used to be a compiler bug, but 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
Should we be setting/upping the minimum version of the linker version as part of this?
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.
The resulting binary (tested in QEMU v7/v8) had no size differences apart from the emited sections and object types of those variables. I've also added prints throughout the U-Boot init sequence. The offsets and delta for 'end - start' section sizes is unchanged.
For example on QEMU v8: $~ ./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%
$~ readelf -sW u-boot | grep bss_s 12: 00000000001029b8 0 SECTION LOCAL DEFAULT 12 .bss_start 8088: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8376: 00000000001029b8 0 OBJECT GLOBAL DEFAULT 12 __bss_start $~ readelf -sW u-boot.new | grep bss_s 8085: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8373: 00000000001029b8 0 NOTYPE GLOBAL DEFAULT 12 __bss_start
For QEMU v7 the differences are a bit bigger but only affect the variables placed in the .bss section because that was defined as an OVERLAY in the existing linker script. For example: $~ nm u-boot | grep tftp_prev_block 000ca0dc ? tftp_prev_block $~ nm u-boot.new | grep tftp_prev_block 000e0a5c b tftp_prev_block -----> The symbol is now placed into .bss
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")
Ilias Apalodimas (6): 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
arch/arm/cpu/armv8/u-boot-spl.lds | 14 +-- arch/arm/cpu/armv8/u-boot.lds | 45 ++------ arch/arm/cpu/u-boot.lds | 74 +++---------- arch/arm/lib/sections.c | 10 -- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 22 +--- arch/arm/mach-zynq/u-boot.lds | 72 +++--------- board/qualcomm/dragonboard820c/u-boot.lds | 41 ++----- board/vscom/baltos/u-boot.lds | 128 ---------------------- include/asm-generic/sections.h | 3 + lib/efi_loader/efi_runtime.c | 1 + 10 files changed, 58 insertions(+), 352 deletions(-) delete mode 100644 board/vscom/baltos/u-boot.lds
-- 2.37.2
On Wed, 28 Feb 2024 at 13:11, Peter Robinson pbrobinson@gmail.com wrote:
On Wed, 28 Feb 2024 at 10:58, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
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]. This has led to confusion over the years, ending up with mixed section definitions. Some sections being 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.
I am not sure if this used to be a compiler bug, but 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
Should we be setting/upping the minimum version of the linker version as part of this?
The patch that added those as C-defined variables, was in 2013. So any linker that's not ancient history should emit those correctly.
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.
The resulting binary (tested in QEMU v7/v8) had no size differences apart from the emited sections and object types of those variables. I've also added prints throughout the U-Boot init sequence. The offsets and delta for 'end - start' section sizes is unchanged.
For example on QEMU v8: $~ ./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%
$~ readelf -sW u-boot | grep bss_s 12: 00000000001029b8 0 SECTION LOCAL DEFAULT 12 .bss_start 8088: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8376: 00000000001029b8 0 OBJECT GLOBAL DEFAULT 12 __bss_start $~ readelf -sW u-boot.new | grep bss_s 8085: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8373: 00000000001029b8 0 NOTYPE GLOBAL DEFAULT 12 __bss_start
For QEMU v7 the differences are a bit bigger but only affect the variables placed in the .bss section because that was defined as an OVERLAY in the existing linker script. For example: $~ nm u-boot | grep tftp_prev_block 000ca0dc ? tftp_prev_block $~ nm u-boot.new | grep tftp_prev_block 000e0a5c b tftp_prev_block -----> The symbol is now placed into .bss
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")
Ilias Apalodimas (6): 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
arch/arm/cpu/armv8/u-boot-spl.lds | 14 +-- arch/arm/cpu/armv8/u-boot.lds | 45 ++------ arch/arm/cpu/u-boot.lds | 74 +++---------- arch/arm/lib/sections.c | 10 -- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 22 +--- arch/arm/mach-zynq/u-boot.lds | 72 +++--------- board/qualcomm/dragonboard820c/u-boot.lds | 41 ++----- board/vscom/baltos/u-boot.lds | 128 ---------------------- include/asm-generic/sections.h | 3 + lib/efi_loader/efi_runtime.c | 1 + 10 files changed, 58 insertions(+), 352 deletions(-) delete mode 100644 board/vscom/baltos/u-boot.lds
-- 2.37.2
On 2/28/24 04:15, Ilias Apalodimas wrote:
On Wed, 28 Feb 2024 at 13:11, Peter Robinson pbrobinson@gmail.com wrote:
On Wed, 28 Feb 2024 at 10:58, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
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]. This has led to confusion over the years, ending up with mixed section definitions. Some sections being 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.
I am not sure if this used to be a compiler bug, but 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
Should we be setting/upping the minimum version of the linker version as part of this?
The patch that added those as C-defined variables, was in 2013. So any linker that's not ancient history should emit those correctly.
GNU ld fixed the problem on 2016-02-23, with this entry in the bfd changelog:
* elflink.c (bfd_elf_record_link_assignment): Check for shared library, instead of PIC, and don't check PDE when making linker assigned symbol dynamic.
It looks like the change was first included in binutils 2.27, released 2016-08-03. I don't know where the minimum linker version requirement is memorialized but there's a good chance that U-Boot already requires a version more recent than that. (Someone who knows where that is will have to check.)
In either case, I strongly agree: sections.c is an unnecessary workaround with any reasonably recent linker version, it's fickle/incompatible with non-GNU linkers such as LLD, and the marker symbols belong in the linker script. This patchset is a big step in the right direction!
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.
The resulting binary (tested in QEMU v7/v8) had no size differences apart from the emited sections and object types of those variables. I've also added prints throughout the U-Boot init sequence. The offsets and delta for 'end - start' section sizes is unchanged.
For example on QEMU v8: $~ ./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%
$~ readelf -sW u-boot | grep bss_s 12: 00000000001029b8 0 SECTION LOCAL DEFAULT 12 .bss_start 8088: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8376: 00000000001029b8 0 OBJECT GLOBAL DEFAULT 12 __bss_start $~ readelf -sW u-boot.new | grep bss_s 8085: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8373: 00000000001029b8 0 NOTYPE GLOBAL DEFAULT 12 __bss_start
For QEMU v7 the differences are a bit bigger but only affect the variables placed in the .bss section because that was defined as an OVERLAY in the existing linker script. For example: $~ nm u-boot | grep tftp_prev_block 000ca0dc ? tftp_prev_block $~ nm u-boot.new | grep tftp_prev_block 000e0a5c b tftp_prev_block -----> The symbol is now placed into .bss
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")
Ilias Apalodimas (6): 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
arch/arm/cpu/armv8/u-boot-spl.lds | 14 +-- arch/arm/cpu/armv8/u-boot.lds | 45 ++------ arch/arm/cpu/u-boot.lds | 74 +++---------- arch/arm/lib/sections.c | 10 -- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 22 +--- arch/arm/mach-zynq/u-boot.lds | 72 +++--------- board/qualcomm/dragonboard820c/u-boot.lds | 41 ++----- board/vscom/baltos/u-boot.lds | 128 ---------------------- include/asm-generic/sections.h | 3 + lib/efi_loader/efi_runtime.c | 1 + 10 files changed, 58 insertions(+), 352 deletions(-) delete mode 100644 board/vscom/baltos/u-boot.lds
-- 2.37.2
On Thu, Feb 29, 2024 at 02:07:39PM -0700, Sam Edwards wrote:
On 2/28/24 04:15, Ilias Apalodimas wrote:
On Wed, 28 Feb 2024 at 13:11, Peter Robinson pbrobinson@gmail.com wrote:
On Wed, 28 Feb 2024 at 10:58, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
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]. This has led to confusion over the years, ending up with mixed section definitions. Some sections being 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.
I am not sure if this used to be a compiler bug, but 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
Should we be setting/upping the minimum version of the linker version as part of this?
The patch that added those as C-defined variables, was in 2013. So any linker that's not ancient history should emit those correctly.
GNU ld fixed the problem on 2016-02-23, with this entry in the bfd changelog:
- elflink.c (bfd_elf_record_link_assignment): Check for shared
library, instead of PIC, and don't check PDE when making linker assigned symbol dynamic.
It looks like the change was first included in binutils 2.27, released 2016-08-03. I don't know where the minimum linker version requirement is memorialized but there's a good chance that U-Boot already requires a version more recent than that. (Someone who knows where that is will have to check.)
In either case, I strongly agree: sections.c is an unnecessary workaround with any reasonably recent linker version, it's fickle/incompatible with non-GNU linkers such as LLD, and the marker symbols belong in the linker script. This patchset is a big step in the right direction!
We have the ld-version check logic same as the kernel but do not today enforce any minimum version. On ARM we only enforce gcc 6.0.0 being a minimum version, and that's likely too old too, in reality. So adding a check would be nice, but not required I think, given the age of the fix. Thanks for digging in to this!
Thanks Sam
On Thu, 29 Feb 2024 at 23:07, Sam Edwards cfsworks@gmail.com wrote:
On 2/28/24 04:15, Ilias Apalodimas wrote:
On Wed, 28 Feb 2024 at 13:11, Peter Robinson pbrobinson@gmail.com wrote:
On Wed, 28 Feb 2024 at 10:58, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
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]. This has led to confusion over the years, ending up with mixed section definitions. Some sections being 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.
I am not sure if this used to be a compiler bug, but 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
Should we be setting/upping the minimum version of the linker version as part of this?
The patch that added those as C-defined variables, was in 2013. So any linker that's not ancient history should emit those correctly.
GNU ld fixed the problem on 2016-02-23, with this entry in the bfd changelog:
* elflink.c (bfd_elf_record_link_assignment): Check for shared library, instead of PIC, and don't check PDE when making linker assigned symbol dynamic.It looks like the change was first included in binutils 2.27, released 2016-08-03. I don't know where the minimum linker version requirement is memorialized but there's a good chance that U-Boot already requires a version more recent than that. (Someone who knows where that is will have to check.)
Ok, this is useful. Thanks for digging it up. I'll update the commit messages on the next version.
In either case, I strongly agree: sections.c is an unnecessary workaround with any reasonably recent linker version, it's fickle/incompatible with non-GNU linkers such as LLD, and the marker symbols belong in the linker script. This patchset is a big step in the right direction!
Yes and the only reason I haven't removed secure_start/end etc, is that I don't have a platform to test. So I'd rather land this first and then clean up the remaining 5 entries of sections.c (and the file itself)
Cheers /Ilias
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.
The resulting binary (tested in QEMU v7/v8) had no size differences apart from the emited sections and object types of those variables. I've also added prints throughout the U-Boot init sequence. The offsets and delta for 'end - start' section sizes is unchanged.
For example on QEMU v8: $~ ./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%
$~ readelf -sW u-boot | grep bss_s 12: 00000000001029b8 0 SECTION LOCAL DEFAULT 12 .bss_start 8088: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8376: 00000000001029b8 0 OBJECT GLOBAL DEFAULT 12 __bss_start $~ readelf -sW u-boot.new | grep bss_s 8085: 0000000000000018 0 NOTYPE GLOBAL DEFAULT 1 _bss_start_ofs 8373: 00000000001029b8 0 NOTYPE GLOBAL DEFAULT 12 __bss_start
For QEMU v7 the differences are a bit bigger but only affect the variables placed in the .bss section because that was defined as an OVERLAY in the existing linker script. For example: $~ nm u-boot | grep tftp_prev_block 000ca0dc ? tftp_prev_block $~ nm u-boot.new | grep tftp_prev_block 000e0a5c b tftp_prev_block -----> The symbol is now placed into .bss
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")
Ilias Apalodimas (6): 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
arch/arm/cpu/armv8/u-boot-spl.lds | 14 +-- arch/arm/cpu/armv8/u-boot.lds | 45 ++------ arch/arm/cpu/u-boot.lds | 74 +++---------- arch/arm/lib/sections.c | 10 -- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 22 +--- arch/arm/mach-zynq/u-boot.lds | 72 +++--------- board/qualcomm/dragonboard820c/u-boot.lds | 41 ++----- board/vscom/baltos/u-boot.lds | 128 ---------------------- include/asm-generic/sections.h | 3 + lib/efi_loader/efi_runtime.c | 1 + 10 files changed, 58 insertions(+), 352 deletions(-) delete mode 100644 board/vscom/baltos/u-boot.lds
-- 2.37.2
participants (5)
-
Caleb Connolly -
Ilias Apalodimas -
Peter Robinson -
Sam Edwards -
Tom Rini