[PATCH v3 0/7] Clean up arm linker scripts

[PATCH v3 0/7] Clean up arm linker scripts

[Ilias Apalodimas]

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   11 __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: 0000000000102b10     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: 000000000010bbb0     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   10 __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 v2:
- Preserve the .bss alignment since it's needed by some C runtime setup code.
  The sdram .bss region for armv8 SPL is defined by a Kconfig option, so instead
  of aligning it explicitly assert if the Kconfig symbol is not 8b aligned
- Align image_copy_end on patch #6. Richard I kept you r-b since that change was
  minimal, but some code assume the dtb will be appended which requires alignment.
  Please yell if you see something wrong
- Added comments based on Richards review on why bss_start - bss_end needs
  to be divided by 4/8 (for armv7/8 respectively)
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           |  29 ++---
 arch/arm/cpu/armv8/u-boot.lds               |  43 ++-----
 arch/arm/cpu/u-boot-spl.lds                 |   2 +-
 arch/arm/cpu/u-boot.lds                     |  72 +++--------
 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    |  23 ++--
 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, 72 insertions(+), 310 deletions(-)
 delete mode 100644 board/vscom/baltos/u-boot.lds

--
2.37.2

[PATCH v3 1/7] arm: baltos: remove custom linker script

[Ilias Apalodimas]

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

[PATCH v3 2/7] arm: clean up v7 and v8 linker scripts for bss_start/end

[Ilias Apalodimas]

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

Tested-by: Caleb Connolly <caleb.connolly@linaro.org> # Qualcomm sdm845
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 arch/arm/cpu/armv8/u-boot-spl.lds        | 18 +++++++-----------
 arch/arm/cpu/armv8/u-boot.lds            | 16 ++++------------
 arch/arm/cpu/u-boot.lds                  | 22 +++++++---------------
 arch/arm/lib/sections.c                  |  2 --
 arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 15 ++++-----------
 arch/arm/mach-zynq/u-boot.lds            | 22 +++++++---------------
 6 files changed, 29 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..692414fe46fb 100644
--- a/arch/arm/cpu/armv8/u-boot-spl.lds
+++ b/arch/arm/cpu/armv8/u-boot-spl.lds
@@ -63,18 +63,11 @@ 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));
+		. = ALIGN(8);
+		__bss_end = .;
 	} >.sdram
 
 	/DISCARD/ : { *(.rela*) }
@@ -89,3 +82,6 @@ SECTIONS
 #include "linux-kernel-image-header-vars.h"
 #endif
 }
+ASSERT(CONFIG_SPL_BSS_START_ADDR % 8 == 0, \
+       "CONFIG_SPL_BSS_START_ADDR must be 8-byte aligned");
+
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index fb6a30c922f7..9640cc7a04b8 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -149,19 +149,11 @@ SECTIONS
 
 	_end = .;
 
-	. = ALIGN(8);
-
-	.bss_start : {
-		KEEP(*(.__bss_start));
-	}
-
-	.bss : {
+	.bss ALIGN(8): {
+		__bss_start = .;
 		*(.bss*)
-		 . = ALIGN(8);
-	}
-
-	.bss_end : {
-		KEEP(*(.__bss_end));
+		. = ALIGN(8);
+		__bss_end = .;
 	}
 
 	/DISCARD/ : { *(.dynsym) }
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 7724c9332c3b..0dfe5f633b16 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -207,23 +207,15 @@ 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));
+		. = ALIGN(4);
+		__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..712c485d4d0b 100644
--- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
+++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
@@ -56,18 +56,11 @@ 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));
+		. = ALIGN(8);
+		__bss_end = .;
 	}
 
 	/DISCARD/ : { *(.dynsym) }
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index 3b7c9d515f8b..3c5008b57392 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -103,23 +103,15 @@ 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));
+		. = ALIGN(8);
+		__bss_end = .;
 	}
 
 	/*
-- 
2.37.2

[PATCH v3 3/7] arm: fix __efi_runtime_rel_start/end definitions

[Ilias Apalodimas]

__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>
Reviewed-by: Sam Edwards <CFSworks@gmail.com>
Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
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 9640cc7a04b8..8561e1b3142e 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 0dfe5f633b16..f19f2812ee91 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 3c5008b57392..bb0e0ceb32ec 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

[PATCH v3 4/7] arm: clean up v7 and v8 linker scripts for __rel_dyn_start/end

[Ilias Apalodimas]

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>
Reviewed-by: Sam Edwards <CFSworks@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical
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 +++-----------
 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 8561e1b3142e..5ba54dcedf24 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 f19f2812ee91..0682d34207fa 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 bb0e0ceb32ec..3b1f0d349356 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

[PATCH v3 5/7] arm: fix __efi_runtime_start/end definitions

[Ilias Apalodimas]

__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>
Reviewed-by: Sam Edwards <CFSworks@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical
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 0682d34207fa..6813d8aeb838 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 3b1f0d349356..9eac7de0dcbd 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

[PATCH v3 6/7] arm: move image_copy_start/end to linker symbols

[Ilias Apalodimas]

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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 arch/arm/cpu/armv8/u-boot-spl.lds           | 8 +++-----
 arch/arm/cpu/armv8/u-boot.lds               | 8 ++------
 arch/arm/cpu/u-boot-spl.lds                 | 2 +-
 arch/arm/cpu/u-boot.lds                     | 8 ++------
 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    | 8 +++-----
 arch/arm/mach-zynq/u-boot-spl.lds           | 2 +-
 arch/arm/mach-zynq/u-boot.lds               | 7 ++-----
 9 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds
index 692414fe46fb..2e61f759ee6b 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,8 @@ SECTIONS
 		KEEP(*(SORT(__u_boot_list*)));
 	} >.sram

-	.image_copy_end : {
-		. = ALIGN(8);
-		*(.__image_copy_end)
-	} >.sram
+	. = ALIGN(8);
+	__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 5ba54dcedf24..147a6e8028d5 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*)
 	}

@@ -123,11 +123,7 @@ SECTIONS
 	}

 	. = 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 6813d8aeb838..798858e3ed6e 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*)
 	}
@@ -154,11 +154,7 @@ SECTIONS
 	}

 	. = 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 712c485d4d0b..ad32654085b3 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,8 @@ SECTIONS
 		KEEP(*(SORT(__u_boot_list*)));
 	}

-	.image_copy_end : {
-		. = ALIGN(8);
-		*(.__image_copy_end)
-	}
+	. = ALIGN(8);
+	__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 9eac7de0dcbd..f6c99a8ce218 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*)
 	}
@@ -60,10 +60,7 @@ SECTIONS
 	}

 	. = ALIGN(8);
-	.image_copy_end :
-	{
-		*(.__image_copy_end)
-	}
+	__image_copy_end = .;

 	.rel.dyn ALIGN(8) : {
 		__rel_dyn_start = .;
--
2.37.2

[PATCH v3 7/7] arm: remove redundant section alignments

[Ilias Apalodimas]

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 when they are not needed.

While at add comments for the alignment of __bss_start/end since our
C runtime setup assembly assumes that __bss_start - __bss_end will be
a multiple of 4/8 for armv7 and armv8 respectively.

It's worth noting that the alignment is preserved on .rel.dyn for
mach-zynq which was explicitly aligning that section on an 8b
boundary instead of 4b one.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 arch/arm/cpu/armv8/u-boot.lds | 9 ++++++---
 arch/arm/cpu/u-boot.lds       | 8 ++++++--
 arch/arm/mach-zynq/u-boot.lds | 4 ++--
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index 147a6e8028d5..857f44412e07 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.*)
@@ -125,7 +125,7 @@ SECTIONS
 	. = ALIGN(8);
 	__image_copy_end = .;
 
-	.rela.dyn ALIGN(8) : {
+	.rela.dyn : {
 		__rel_dyn_start = .;
 		*(.rela*)
 		__rel_dyn_end = .;
@@ -133,7 +133,10 @@ SECTIONS
 
 	_end = .;
 
-	.bss ALIGN(8): {
+	/*
+	 * arch/arm/lib/crt0_64.S assumes __bss_start - __bss_end % 8 == 0
+	 */
+	.bss ALIGN(8) : {
 		__bss_start = .;
 		*(.bss*)
 		. = ALIGN(8);
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 798858e3ed6e..707b19795f08 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.*)
@@ -156,6 +156,10 @@ SECTIONS
 	. = ALIGN(4);
 	__image_copy_end = .;
 
+	/*
+	 * if CONFIG_USE_ARCH_MEMSET is not selected __bss_end - __bss_start
+	 * needs to be a multiple of 4 and we overlay .bss with .rel.dyn
+	 */
 	.rel.dyn ALIGN(4) : {
 		__rel_dyn_start = .;
 		*(.rel*)
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index f6c99a8ce218..3e0c96c50556 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

Re: [PATCH v3 2/7] arm: clean up v7 and v8 linker scripts for bss_start/end

[Richard Henderson]

On 3/13/24 06:23, Ilias Apalodimas wrote:
> +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
> @@ -63,18 +63,11 @@ 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));
> +		. = ALIGN(8);
> +		__bss_end = .;

Still missing the alignment on .bss, previously in .bss_start.

With that fixed,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v3 7/7] arm: remove redundant section alignments

[Richard Henderson]

On 3/13/24 06:23, Ilias Apalodimas wrote:
> 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 when they are not needed.
> 
> While at add comments for the alignment of __bss_start/end since our
> C runtime setup assembly assumes that __bss_start - __bss_end will be
> a multiple of 4/8 for armv7 and armv8 respectively.
> 
> It's worth noting that the alignment is preserved on .rel.dyn for
> mach-zynq which was explicitly aligning that section on an 8b
> boundary instead of 4b one.
> 
> Signed-off-by: Ilias Apalodimas<ilias.apalodimas@linaro.org>
> ---
>   arch/arm/cpu/armv8/u-boot.lds | 9 ++++++---
>   arch/arm/cpu/u-boot.lds       | 8 ++++++--
>   arch/arm/mach-zynq/u-boot.lds | 4 ++--
>   3 files changed, 14 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v3 2/7] arm: clean up v7 and v8 linker scripts for bss_start/end

[Ilias Apalodimas]

Hi Richard,

On Wed, 13 Mar 2024 at 22:19, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 3/13/24 06:23, Ilias Apalodimas wrote:
> > +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
> > @@ -63,18 +63,11 @@ 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));
> > +             . = ALIGN(8);
> > +             __bss_end = .;
>
> Still missing the alignment on .bss, previously in .bss_start.

Since this is emitted in .sdram memory I can't define it as
.bss ALIGN(8) : {....} since the calculated memory will be outside the
sdram-defined region

I could define it as
.bss : {
        . = ALIGN(8);
        __bss_start = .;
        ......
}

But instead, I added an assert at the bottom which will break the
linking if the __bss_start is not 8byte aligned.

Looking at the output for xilinx_zynqmp_kria_defconfig (which is a v8
board & SPL) looks identical and correct since
CONFIG_SPL_BSS_START_ADDR=0x1000 for that board.

$~ readelf -sW spl/u-boot-spl | grep bss_start
  1550: 0000000000001000     0 NOTYPE  GLOBAL DEFAULT    6 __bss_start

Isn't the assert enough? Or do you think adding the . = ALIGN(8) in
the section definition is better?

Thanks
/Ilias


>
> With that fixed,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> r~

Re: [PATCH v3 2/7] arm: clean up v7 and v8 linker scripts for bss_start/end

[Richard Henderson]

On 3/13/24 11:43, Ilias Apalodimas wrote:
> Hi Richard,
> 
> On Wed, 13 Mar 2024 at 22:19, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 3/13/24 06:23, Ilias Apalodimas wrote:
>>> +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
>>> @@ -63,18 +63,11 @@ 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));
>>> +             . = ALIGN(8);
>>> +             __bss_end = .;
>>
>> Still missing the alignment on .bss, previously in .bss_start.
> 
> Since this is emitted in .sdram memory I can't define it as
> .bss ALIGN(8) : {....} since the calculated memory will be outside the
> sdram-defined region
> 
> I could define it as
> .bss : {
>          . = ALIGN(8);
>          __bss_start = .;
>          ......
> }
> 
> But instead, I added an assert at the bottom which will break the
> linking if the __bss_start is not 8byte aligned.

I think it would be clearer to assert on __bss_start address rather than 
CONFIG_SPL_BSS_START_ADDR, if that's possible.  If not, then the constant will have to do.


r~

Re: [PATCH v3 2/7] arm: clean up v7 and v8 linker scripts for bss_start/end

[Sam Edwards]

On 3/13/24 10:23, 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 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
> 
> Tested-by: Caleb Connolly <caleb.connolly@linaro.org> # Qualcomm sdm845
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   arch/arm/cpu/armv8/u-boot-spl.lds        | 18 +++++++-----------
>   arch/arm/cpu/armv8/u-boot.lds            | 16 ++++------------
>   arch/arm/cpu/u-boot.lds                  | 22 +++++++---------------
>   arch/arm/lib/sections.c                  |  2 --
>   arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 15 ++++-----------
>   arch/arm/mach-zynq/u-boot.lds            | 22 +++++++---------------
>   6 files changed, 29 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..692414fe46fb 100644
> --- a/arch/arm/cpu/armv8/u-boot-spl.lds
> +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
> @@ -63,18 +63,11 @@ 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));
> +		. = ALIGN(8);
> +		__bss_end = .;
>   	} >.sdram
>   
>   	/DISCARD/ : { *(.rela*) }
> @@ -89,3 +82,6 @@ SECTIONS
>   #include "linux-kernel-image-header-vars.h"
>   #endif
>   }
> +ASSERT(CONFIG_SPL_BSS_START_ADDR % 8 == 0, \
> +       "CONFIG_SPL_BSS_START_ADDR must be 8-byte aligned");
> +

Git complains about this added blank line at the end of the file. (My 
personal preference would be a blank line before the ASSERT, if the 
ASSERT is truly necessary.)

But beyond that:
Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical

Still really excited for this to land! I'm going to have to blow the 
dust off of my Clang/LLD support series here soon. :)

Best,
Sam

> diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
> index fb6a30c922f7..9640cc7a04b8 100644
> --- a/arch/arm/cpu/armv8/u-boot.lds
> +++ b/arch/arm/cpu/armv8/u-boot.lds
> @@ -149,19 +149,11 @@ SECTIONS
>   
>   	_end = .;
>   
> -	. = ALIGN(8);
> -
> -	.bss_start : {
> -		KEEP(*(.__bss_start));
> -	}
> -
> -	.bss : {
> +	.bss ALIGN(8): {
> +		__bss_start = .;
>   		*(.bss*)
> -		 . = ALIGN(8);
> -	}
> -
> -	.bss_end : {
> -		KEEP(*(.__bss_end));
> +		. = ALIGN(8);
> +		__bss_end = .;
>   	}
>   
>   	/DISCARD/ : { *(.dynsym) }
> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
> index 7724c9332c3b..0dfe5f633b16 100644
> --- a/arch/arm/cpu/u-boot.lds
> +++ b/arch/arm/cpu/u-boot.lds
> @@ -207,23 +207,15 @@ 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));
> +		. = ALIGN(4);
> +		__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..712c485d4d0b 100644
> --- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
> +++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
> @@ -56,18 +56,11 @@ 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));
> +		. = ALIGN(8);
> +		__bss_end = .;
>   	}
>   
>   	/DISCARD/ : { *(.dynsym) }
> diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
> index 3b7c9d515f8b..3c5008b57392 100644
> --- a/arch/arm/mach-zynq/u-boot.lds
> +++ b/arch/arm/mach-zynq/u-boot.lds
> @@ -103,23 +103,15 @@ 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));
> +		. = ALIGN(8);
> +		__bss_end = .;
>   	}
>   
>   	/*

Re: [PATCH v3 2/7] arm: clean up v7 and v8 linker scripts for bss_start/end

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

On Wed, Mar 13, 2024 at 04:57:57PM -0600, Sam Edwards wrote:

[snip]
> Still really excited for this to land! I'm going to have to blow the dust
> off of my Clang/LLD support series here soon. :)

Please tell me that includes updates to the Clang support in general
copied over from the Linux kernel :)

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 2/7] arm: clean up v7 and v8 linker scripts for bss_start/end

[Ilias Apalodimas]

Hi Richard,


On Thu, 14 Mar 2024 at 00:43, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 3/13/24 11:43, Ilias Apalodimas wrote:
> > Hi Richard,
> >
> > On Wed, 13 Mar 2024 at 22:19, Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> On 3/13/24 06:23, Ilias Apalodimas wrote:
> >>> +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
> >>> @@ -63,18 +63,11 @@ 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));
> >>> +             . = ALIGN(8);
> >>> +             __bss_end = .;
> >>
> >> Still missing the alignment on .bss, previously in .bss_start.
> >
> > Since this is emitted in .sdram memory I can't define it as
> > .bss ALIGN(8) : {....} since the calculated memory will be outside the
> > sdram-defined region
> >
> > I could define it as
> > .bss : {
> >          . = ALIGN(8);
> >          __bss_start = .;
> >          ......
> > }
> >
> > But instead, I added an assert at the bottom which will break the
> > linking if the __bss_start is not 8byte aligned.
>
> I think it would be clearer to assert on __bss_start address rather than
> CONFIG_SPL_BSS_START_ADDR, if that's possible.  If not, then the constant will have to do.


Fair enough and it is possible. I can convert that assertion to
ASSERT(ADDR(.bss) % 8 == 0, ".bss must be 8-byte aligned");

Keep in mind that .bss, due to the linker aligning the section to the
strictest input section requirement will end up aligning that to 8b
regardless. IOW compiling with CONFIG_SPL_BSS_START_ADDR=0x1001, won't
break the linking since .bss will end up at 0x1008.

Testing the assertion with a section definition which looks like this
will trigger the assert.
        .bss 0x1001 : {
                __bss_start = .;
                *(.bss*)
                . = ALIGN(8);
                __bss_end = .;
        } >.sdram

I don't have a really strong opinion here. I think the assertion above
is ok. What we could also do is

        .bss  : {
                . = ALIGN(8);
                __bss_start = .;
                *(.bss*)
                . = ALIGN(8);
                __bss_end = .;
        } >.sdram

which would also fix the __bss_start alignment regardless of the .bss
output address.
For example with the linker entry below __bss_start will end up at 0x1008
        .bss  0x1001 : {
                . = ALIGN(8);
                __bss_start = .;
                *(.bss*)
                . = ALIGN(8);
                __bss_end = .;
        } >.sdram

As you already mentioned, the bss_end - bss_start % 8 == 0 can be
fixed by using memset. So since I plan to continue working on this to
get RO, RW^x etc mappings I think I'll just go for the assertion for
now -- but test the .bss address instead of the config option.

Anyone objects?

Thanks
/Ilias

>
>
> r~