[RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[Sam Edwards]

Here is a series of patches aimed at improving support for the LLVM
toolchain (clang, lld, and to a lesser extent, llvm-objcopy) when
targeting ARM. This toolchain is a cross-compiler "by default" -- a user
generally should not need to install anything more to target their board
of choice if they already have Clang installed. Additionally, Clang has
a few nice diagnostics that should help appreciably with code quality,
if Clang starts to be used regularly by a few more U-Boot developers. ;)

Most of these patches are trivial and as such they should be pretty easy
to review, but the later patches in the patchset start making some
pretty big changes to the linker scripts. There are no behavioral
changes with those (U-Boot should still function the same) but there is
always the risk of compatibility with some third-party tool or loader
being broken. Fortunately, I foresee any problems making themselves
immediately apparent upon testing.

I have tested booting on sunxi/T113, and building for Raspberry Pi
(32-bit and 64-bit). The remaining testing efforts should be focused on:
- Exynos
- EFI loader (esp. on Lenovo X13s, which Heinrich Schuchardt had
    mentioned in a previous commit as being fickle)
- Zynq
- Rockchip TPL
- AArch64 SPL

I'm submitting this as an RFC since this doesn't completely guarantee
LLVM toolchain compatibility yet; my focus here is mostly on ensuring
that I haven't caused any regressions in GNU-land. Also, I haven't
discussed most of these changes before doing them. Perhaps alternate
approaches to some of these things can be proposed - I'm all ears.

Outstanding problems are:
- LLD sometimes puts .hash outside of the image binary area, though this
  is flagged by binary_size_check
- The main makefile uses --gap-fill=0xff, which is not supported in
  llvm-objcopy
- llvm-objcopy also doesn't appear to speak S-Record; the u-boot.srec
  target has to be deleted manually
- llvm-objcopy gets upset at some of the EFI code, since the EFI linker
  scripts preserve dynamic sections that llvm-objcopy doesn't want to
  strip off

Cheers,
Sam


Sam Edwards (10):
  makefile: Fix symbol typo in binary_size_check
  arm: set alignment properly for asm funcs
  arm: exclude eabi_compat from LTO
  arm: add __aeabi_memclr in eabi_compat
  arm: add aligned-memory aliases to eabi_compat
  arm: discard .gnu.version* sections
  arm: efi_loader: discard hash, unwind information
  arm: efi_loader: move .dynamic out of .text in EFI
  arm: discard all .dyn* sections
  arm: migrate away from sections.c

 Makefile                                 |   2 +-
 arch/arm/cpu/armv8/spl_data.c            |   4 +-
 arch/arm/cpu/armv8/u-boot-spl.lds        |  26 ++----
 arch/arm/cpu/armv8/u-boot.lds            |  48 +++--------
 arch/arm/cpu/u-boot.lds                  | 103 +++++++----------------
 arch/arm/include/asm/linkage.h           |   4 +-
 arch/arm/lib/Makefile                    |   3 +-
 arch/arm/lib/eabi_compat.c               |  17 ++++
 arch/arm/lib/elf_aarch64_efi.lds         |   3 +-
 arch/arm/lib/elf_arm_efi.lds             |   3 +
 arch/arm/lib/sections.c                  |  36 --------
 arch/arm/mach-rockchip/u-boot-tpl-v8.lds |  26 ++----
 arch/arm/mach-zynq/u-boot.lds            |  73 +++-------------
 13 files changed, 96 insertions(+), 252 deletions(-)
 delete mode 100644 arch/arm/lib/sections.c

-- 
2.39.2

[RFC PATCH 01/10] makefile: Fix symbol typo in binary_size_check

[Sam Edwards]

The start-of-image marker symbol is `__image_copy_start`; by
searching for `_image_copy_start` instead, this check can
accidentally match `_image_copy_start_ofs`.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
---

 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index a5ab5e3da9..19017cbfa0 100644
--- a/Makefile
+++ b/Makefile
@@ -1262,7 +1262,7 @@ OBJCOPYFLAGS_u-boot-nodtb.bin := -O binary \
 binary_size_check: u-boot-nodtb.bin FORCE
 	@file_size=$(shell wc -c u-boot-nodtb.bin | awk '{print $$1}') ; \
 	map_size=$(shell cat u-boot.map | \
-		awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \
+		awk '/__image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \
 		| sed 's/0X//g' \
 		| bc); \
 	if [ "" != "$$map_size" ]; then \
-- 
2.39.2

[RFC PATCH 02/10] arm: set alignment properly for asm funcs

[Sam Edwards]

ARM requires a 4-byte alignment on all ARM code (though this
requirement is relaxed to 2-byte for some THUMB code) and we
should be explicit about that here.

GAS has its own fix for this[1] that forces proper alignment
on any section containing assembled instructions, but this is
not universal: Clang's and other gaslike assemblers lack this
implicit alignment. Whether or not this is considered a bug in
those assemblers, it is better to ask directly for what we want.

[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=12931

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---

 arch/arm/include/asm/linkage.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/linkage.h b/arch/arm/include/asm/linkage.h
index dbe4b4e31a..73bf25ba4e 100644
--- a/arch/arm/include/asm/linkage.h
+++ b/arch/arm/include/asm/linkage.h
@@ -1,7 +1,7 @@
 #ifndef __ASM_LINKAGE_H
 #define __ASM_LINKAGE_H
 
-#define __ALIGN .align 0
-#define __ALIGN_STR ".align 0"
+#define __ALIGN .p2align 2
+#define __ALIGN_STR ".p2align 2"
 
 #endif
-- 
2.39.2

[RFC PATCH 03/10] arm: exclude eabi_compat from LTO

[Sam Edwards]

These symbols need to survive the IR-level dead function elimination
pass, since nothing at the IR level is referencing them (calls to these
are inserted later, at codegen time).

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---

 arch/arm/lib/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index 62cf80f373..d65dc33f5b 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -88,6 +88,7 @@ obj-$(CONFIG_DEBUG_LL)	+= debug.o
 # For EABI conformant tool chains, provide eabi_compat()
 ifneq (,$(findstring -mabi=aapcs-linux,$(PLATFORM_CPPFLAGS)))
 extra-y	+= eabi_compat.o
+CFLAGS_REMOVE_eabi_compat.o := $(LTO_CFLAGS)
 endif
 
 # some files can only build in ARM or THUMB2, not THUMB1
-- 
2.39.2

[RFC PATCH 04/10] arm: add __aeabi_memclr in eabi_compat

[Sam Edwards]

This is sometimes used by LLVM's code generator.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---

 arch/arm/lib/eabi_compat.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/lib/eabi_compat.c b/arch/arm/lib/eabi_compat.c
index f7029918d4..059ca07265 100644
--- a/arch/arm/lib/eabi_compat.c
+++ b/arch/arm/lib/eabi_compat.c
@@ -35,3 +35,8 @@ void __aeabi_memset(void *dest, size_t n, int c)
 {
 	(void) memset(dest, c, n);
 }
+
+void __aeabi_memclr(void *dest, size_t n)
+{
+	(void)memset(dest, 0, n);
+}
-- 
2.39.2

[RFC PATCH 05/10] arm: add aligned-memory aliases to eabi_compat

[Sam Edwards]

These are sometimes used by LLVM's code-generator, when it can guarantee
that the memory buffer being passed is aligned on a (4- or 8-byte)
boundary. They can safely be aliases to the unaligned versions.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---

 arch/arm/lib/eabi_compat.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/lib/eabi_compat.c b/arch/arm/lib/eabi_compat.c
index 059ca07265..d00f68a3f6 100644
--- a/arch/arm/lib/eabi_compat.c
+++ b/arch/arm/lib/eabi_compat.c
@@ -31,12 +31,24 @@ void __aeabi_memcpy(void *dest, const void *src, size_t n)
 	(void) memcpy(dest, src, n);
 }
 
+void __aeabi_memcpy4(void *dest, const void *src, size_t n) __alias(__aeabi_memcpy);
+
+void __aeabi_memcpy8(void *dest, const void *src, size_t n) __alias(__aeabi_memcpy);
+
 void __aeabi_memset(void *dest, size_t n, int c)
 {
 	(void) memset(dest, c, n);
 }
 
+void __aeabi_memset4(void *dest, size_t n, int c) __alias(__aeabi_memset);
+
+void __aeabi_memset8(void *dest, size_t n, int c) __alias(__aeabi_memset);
+
 void __aeabi_memclr(void *dest, size_t n)
 {
 	(void)memset(dest, 0, n);
 }
+
+void __aeabi_memclr4(void *dest, size_t n) __alias(__aeabi_memclr);
+
+void __aeabi_memclr8(void *dest, size_t n) __alias(__aeabi_memclr);
-- 
2.39.2

[RFC PATCH 06/10] arm: discard .gnu.version* sections

[Sam Edwards]

These are often a consequence of --pie, but they aren't actually
used in the runtime relocation code. It is better to discard them
than to aggregate them, because they tend to be of different types,
and this upsets some linkers (e.g. LLD).

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---

 arch/arm/cpu/u-boot.lds | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index fc4f63d834..8cdf08a730 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -229,6 +229,12 @@ SECTIONS
 		KEEP(*(.__bss_end));
 	}
 
+	/*
+	 * LLD's --pie may synthesize these sections, even if they are empty;
+	 * discard them, for we do not need version symbols
+	 */
+	/DISCARD/ : { *(.gnu.version*) }
+
 	.dynsym _image_binary_end : { *(.dynsym) }
 	.dynbss : { *(.dynbss) }
 	.dynstr : { *(.dynstr*) }
-- 
2.39.2

[RFC PATCH 07/10] arm: efi_loader: discard hash, unwind information

[Sam Edwards]

LLD tends to put these at the very beginning of the file, only
for the .text 0x0 directive to end up going backward and
overlapping them, creating an error.

Since they don't appear to be used at runtime, just discard them.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---

 arch/arm/lib/elf_arm_efi.lds | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/lib/elf_arm_efi.lds b/arch/arm/lib/elf_arm_efi.lds
index 767ebda635..0b6cc5bcb6 100644
--- a/arch/arm/lib/elf_arm_efi.lds
+++ b/arch/arm/lib/elf_arm_efi.lds
@@ -62,5 +62,8 @@ SECTIONS
 		*(.dynstr)
 		*(.note.gnu.build-id)
 		*(.comment)
+		*(.hash)
+		*(.gnu.hash)
+		*(.ARM.exidx)
 	}
 }
-- 
2.39.2

[RFC PATCH 08/10] arm: efi_loader: move .dynamic out of .text in EFI

[Sam Edwards]

This is not proper: A .text section is SHT_PROGBITS,
while the .dynamic section is SHT_DYNAMIC. Attempting to
combine them like this creates a section type mismatch.

It does seem that GNU ld does not complain, but LLVM's lld
considers this an error.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---

 arch/arm/lib/elf_aarch64_efi.lds | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/lib/elf_aarch64_efi.lds b/arch/arm/lib/elf_aarch64_efi.lds
index 5dd9809169..986f13936d 100644
--- a/arch/arm/lib/elf_aarch64_efi.lds
+++ b/arch/arm/lib/elf_aarch64_efi.lds
@@ -24,10 +24,9 @@ SECTIONS
 		*(.gnu.linkonce.t.*)
 		*(.srodata)
 		*(.rodata*)
-		. = ALIGN(16);
-		*(.dynamic);
 		. = ALIGN(512);
 	}
+	.dynamic : { *(.dynamic) }
 	.rela.dyn : { *(.rela.dyn) }
 	.rela.plt : { *(.rela.plt) }
 	.rela.got : { *(.rela.got) }
-- 
2.39.2

[RFC PATCH 09/10] arm: discard all .dyn* sections

[Sam Edwards]

This makes the output ELF friendlier to llvm-objcopy, which
otherwise complains about discarding .dynsym when it's
(potentially) referenced by .rel.dyn.

Since u-boot doesn't actually make use of a dynamic linker,
and these sections are only here because we're building --pie,
it's best to discard what we don't need.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---

 arch/arm/cpu/u-boot.lds | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 8cdf08a730..bd4650bd86 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -235,10 +235,13 @@ SECTIONS
 	 */
 	/DISCARD/ : { *(.gnu.version*) }
 
-	.dynsym _image_binary_end : { *(.dynsym) }
-	.dynbss : { *(.dynbss) }
-	.dynstr : { *(.dynstr*) }
-	.dynamic : { *(.dynamic*) }
+	/*
+	 * Not needed at runtime: relocations are done directly on .rel.dyn,
+	 * and we're self-relocating so we don't need compatibility with an
+	 * external dynamic linker.
+	 */
+	/DISCARD/ : { *(.dyn*) }
+
 	.plt : { *(.plt*) }
 	.interp : { *(.interp*) }
 	.gnu.hash : { *(.gnu.hash) }
-- 
2.39.2

[RFC PATCH 10/10] arm: migrate away from sections.c

[Sam Edwards]

This patch effectively reverts 3ebd1cbc49f0005092d69cf0d9a6e64d7a1c300b.

The approach taken in that commit was to have the section-marking
symbols generated into empty sections by the compiler, for the linker
script to include at the correct location. The rationale was that at
the time, the linker considered linker-assigned symbols to be dynamic
when they were in PIC (PIEs or shared libraries), which meant they were
represented at runtime by a R_ARM_ABS32 relocation (by symbol name)
rather than by M_ARM_RELATIVE.

That commit landed in March 2013, but GNU ld later changed its behavior
on 2016-02-23 to default linker-assigned symbols to dynamic only in
shared libraries (not PIE), so this approach is unnecessary.

I am removing it, because:
1) It required keeping sections.c in sync with multiple linker scripts.
2) It added complexity to the linker scripts, making them less readable.
3) It added unnecessary sections to the output, which can't be merged
   because the sections are sometimes of different types.
4) The linker may insert sections not explicitly named in the script
   somewhere between explicit sections; having the marker symbols
   outside of the sections they were marking meant the markers could
   end up with an unintended section inserted within that region.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>

---

 arch/arm/cpu/armv8/spl_data.c            |  4 +-
 arch/arm/cpu/armv8/u-boot-spl.lds        | 26 +++----
 arch/arm/cpu/armv8/u-boot.lds            | 48 ++++---------
 arch/arm/cpu/u-boot.lds                  | 88 +++++-------------------
 arch/arm/lib/Makefile                    |  2 -
 arch/arm/lib/sections.c                  | 36 ----------
 arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 26 ++-----
 arch/arm/mach-zynq/u-boot.lds            | 73 ++++----------------
 8 files changed, 59 insertions(+), 244 deletions(-)
 delete mode 100644 arch/arm/lib/sections.c

diff --git a/arch/arm/cpu/armv8/spl_data.c b/arch/arm/cpu/armv8/spl_data.c
index 8f1231c86e..017b117183 100644
--- a/arch/arm/cpu/armv8/spl_data.c
+++ b/arch/arm/cpu/armv8/spl_data.c
@@ -6,8 +6,8 @@
 #include <common.h>
 #include <spl.h>
 
-char __data_save_start[0] __section(".__data_save_start");
-char __data_save_end[0] __section(".__data_save_end");
+extern char __data_save_start[0];
+extern char __data_save_end[0];
 
 u32 cold_reboot_flag = 1;
 
diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds
index 7cb9d73124..137113a823 100644
--- a/arch/arm/cpu/armv8/u-boot-spl.lds
+++ b/arch/arm/cpu/armv8/u-boot-spl.lds
@@ -40,9 +40,9 @@ SECTIONS
 
 #ifdef CONFIG_SPL_RECOVER_DATA_SECTION
 	.data_save : {
-		*(.__data_save_start)
+		__data_save_start = .;
 		. = SIZEOF(.data);
-		*(.__data_save_end)
+		__data_save_end = .;
 	} >.sram
 #endif
 
@@ -51,30 +51,20 @@ SECTIONS
 		KEEP(*(SORT(__u_boot_list*)));
 	} >.sram
 
-	.image_copy_end : {
-		. = ALIGN(8);
-		*(.__image_copy_end)
-	} >.sram
+	. = ALIGN(8);
 
-	.end : {
-		. = ALIGN(8);
-		*(.__end)
-	} >.sram
+	__image_copy_end = .;
+	_end = .;
 
 	_image_binary_end = .;
 
-	.bss_start (NOLOAD) : {
-		. = ALIGN(8);
-		KEEP(*(.__bss_start));
-	} >.sdram
+	. = ALIGN(8);
 
 	.bss (NOLOAD) : {
+		__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 fb6a30c922..c14c3365a8 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*)
 	}
 
@@ -42,13 +42,9 @@ SECTIONS
 	}
 
 #ifdef CONFIG_ARMV8_PSCI
-	.__secure_start :
 #ifndef CONFIG_ARMV8_SECURE_BASE
-		ALIGN(CONSTANT(COMMONPAGESIZE))
+	. = ALIGN(CONSTANT(COMMONPAGESIZE));
 #endif
-	{
-		KEEP(*(.__secure_start))
-	}
 
 #ifndef CONFIG_ARMV8_SECURE_BASE
 #define __ARMV8_SECURE_BASE
@@ -57,8 +53,8 @@ SECTIONS
 #define __ARMV8_SECURE_BASE	CONFIG_ARMV8_SECURE_BASE
 #endif
 	.secure_text __ARMV8_SECURE_BASE :
-		AT(ADDR(.__secure_start) + SIZEOF(.__secure_start))
 	{
+		__secure_start = .;
 		*(._secure.text)
 		. = ALIGN(8);
 		__secure_svc_tbl_start = .;
@@ -79,23 +75,20 @@ SECTIONS
 		AT(LOADADDR(.secure_data) + SIZEOF(.secure_data))
 #endif
 	{
-		KEEP(*(.__secure_stack_start))
+		__secure_stack_start = .;
 
 		. = . + CONFIG_ARMV8_PSCI_NR_CPUS * ARM_PSCI_STACK_SIZE;
 
 		. = ALIGN(CONSTANT(COMMONPAGESIZE));
 
-		KEEP(*(.__secure_stack_end))
+		__secure_stack_end = .;
 	}
 
 #ifndef __ARMV8_PSCI_STACK_IN_RAM
 	. = LOADADDR(.secure_stack);
 #endif
 
-	.__secure_end : AT(ADDR(.__secure_end)) {
-		KEEP(*(.__secure_end))
-		LONG(0x1d1071c);	/* Must output something to reset LMA */
-	}
+	__secure_end = .;
 #endif
 
 	. = ALIGN(8);
@@ -126,42 +119,25 @@ SECTIONS
 
 	. = ALIGN(8);
 
-	.image_copy_end :
-	{
-		*(.__image_copy_end)
-	}
+	__image_copy_end = .;
 
 	. = ALIGN(8);
 
-	.rel_dyn_start :
-	{
-		*(.__rel_dyn_start)
-	}
-
 	.rela.dyn : {
+		__rel_dyn_start = .;
 		*(.rela*)
-	}
-
-	.rel_dyn_end :
-	{
-		*(.__rel_dyn_end)
+		__rel_dyn_end = .;
 	}
 
 	_end = .;
 
 	. = ALIGN(8);
 
-	.bss_start : {
-		KEEP(*(.__bss_start));
-	}
-
 	.bss : {
+		__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 bd4650bd86..f0e58f4d72 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -40,24 +40,18 @@ SECTIONS
 	. = ALIGN(4);
 	.text :
 	{
-		*(.__image_copy_start)
+		__image_copy_start = .;
 		*(.vectors)
 		CPUDIR/start.o (.text*)
 	}
 
 	/* This needs to come before *(.text*) */
-	.__efi_runtime_start : {
-		*(.__efi_runtime_start)
-	}
-
 	.efi_runtime : {
+		__efi_runtime_start = .;
 		*(.text.efi_runtime*)
 		*(.rodata.efi_runtime*)
 		*(.data.efi_runtime*)
-	}
-
-	.__efi_runtime_stop : {
-		*(.__efi_runtime_stop)
+		__efi_runtime_stop = .;
 	}
 
 	.text_rest :
@@ -68,13 +62,9 @@ SECTIONS
 #ifdef CONFIG_ARMV7_NONSEC
 
 	/* Align the secure section only if we're going to use it in situ */
-	.__secure_start
 #ifndef CONFIG_ARMV7_SECURE_BASE
-		ALIGN(CONSTANT(COMMONPAGESIZE))
+	. = ALIGN(CONSTANT(COMMONPAGESIZE));
 #endif
-	: {
-		KEEP(*(.__secure_start))
-	}
 
 #ifndef CONFIG_ARMV7_SECURE_BASE
 #define __ARMV7_SECURE_BASE
@@ -84,8 +74,8 @@ SECTIONS
 #endif
 
 	.secure_text __ARMV7_SECURE_BASE :
-		AT(ADDR(.__secure_start) + SIZEOF(.__secure_start))
 	{
+		__secure_start = .;
 		*(._secure.text)
 	}
 
@@ -103,7 +93,7 @@ SECTIONS
 		AT(LOADADDR(.secure_data) + SIZEOF(.secure_data))
 #endif
 	{
-		KEEP(*(.__secure_stack_start))
+		__secure_stack_start = .;
 
 		/* Skip addreses for stack */
 		. = . + CONFIG_ARMV7_PSCI_NR_CPUS * ARM_PSCI_STACK_SIZE;
@@ -111,7 +101,7 @@ SECTIONS
 		/* Align end of stack section to page boundary */
 		. = ALIGN(CONSTANT(COMMONPAGESIZE));
 
-		KEEP(*(.__secure_stack_end))
+		__secure_stack_end = .;
 
 #ifdef CONFIG_ARMV7_SECURE_MAX_SIZE
 		/*
@@ -133,7 +123,7 @@ SECTIONS
 #endif
 
 	.__secure_end : AT(ADDR(.__secure_end)) {
-		*(.__secure_end)
+		__secure_end = .;
 		LONG(0x1d1071c);	/* Must output something to reset LMA */
 	}
 #endif
@@ -146,58 +136,29 @@ SECTIONS
 		*(.data*)
 	}
 
-	. = ALIGN(4);
-
-	. = .;
-
 	. = ALIGN(4);
 	__u_boot_list : {
 		KEEP(*(SORT(__u_boot_list*)));
 	}
 
 	. = ALIGN(4);
-
-	.efi_runtime_rel_start :
-	{
-		*(.__efi_runtime_rel_start)
-	}
-
 	.efi_runtime_rel : {
+		__efi_runtime_rel_start = .;
 		*(.rel*.efi_runtime)
 		*(.rel*.efi_runtime.*)
-	}
-
-	.efi_runtime_rel_stop :
-	{
-		*(.__efi_runtime_rel_stop)
+		__efi_runtime_rel_stop = .;
 	}
 
 	. = ALIGN(4);
-
-	.image_copy_end :
-	{
-		*(.__image_copy_end)
-	}
-
-	.rel_dyn_start :
-	{
-		*(.__rel_dyn_start)
-	}
+	__image_copy_end = .;
 
 	.rel.dyn : {
+		__rel_dyn_start = .;
 		*(.rel*)
+		__rel_dyn_end = .;
 	}
 
-	.rel_dyn_end :
-	{
-		*(.__rel_dyn_end)
-	}
-
-	.end :
-	{
-		*(.__end)
-	}
-
+	_end = .;
 	_image_binary_end = .;
 
 	/*
@@ -209,24 +170,11 @@ 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 : {
+		__bss_start = .;
 		*(.bss*)
-		 . = ALIGN(4);
-		 __bss_limit = .;
-	}
-
-	.bss_end __bss_limit (OVERLAY) : {
-		KEEP(*(.__bss_end));
+		. = ALIGN(4);
+		__bss_end = .;
 	}
 
 	/*
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index d65dc33f5b..c77e2c1f4b 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -57,8 +57,6 @@ endif
 
 # obj-$(CONFIG_SAVE_PREV_BL_INITRAMFS_START_ADDR) += save_prev_bl_data.o
 obj-y	+= bdinfo.o
-obj-y	+= sections.o
-CFLAGS_REMOVE_sections.o := $(LTO_CFLAGS)
 
 obj-y	+= stack.o
 ifdef CONFIG_CPU_V7M
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
deleted file mode 100644
index 857879711c..0000000000
--- a/arch/arm/lib/sections.c
+++ /dev/null
@@ -1,36 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Copyright 2013 Albert ARIBAUD <albert.u.boot@aribaud.net>
- */
-#include <linux/compiler.h>
-
-/**
- * These two symbols are declared in a C file so that the linker
- * uses R_ARM_RELATIVE relocation, rather than the R_ARM_ABS32 one
- * it would use if the symbols were defined in the linker file.
- * Using only R_ARM_RELATIVE relocation ensures that references to
- * the symbols are correct after as well as before relocation.
- *
- * We need a 0-byte-size type for these symbols, and the compiler
- * does not allow defining objects of C type 'void'. Using an empty
- * struct is allowed by the compiler, but causes gcc versions 4.4 and
- * below to complain about aliasing. Therefore we use the next best
- * thing: zero-sized arrays, which are both 0-byte-size and exempt from
- * 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");
-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");
-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-rockchip/u-boot-tpl-v8.lds b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
index 74618eba59..0cd6730533 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*)
 	}
@@ -44,30 +44,16 @@ SECTIONS
 		KEEP(*(SORT(__u_boot_list*)));
 	}
 
-	.image_copy_end : {
-		. = ALIGN(8);
-		*(.__image_copy_end)
-	}
-
-	.end : {
-		. = ALIGN(8);
-		*(.__end)
-	}
+	__image_copy_end = .;
+	_end = .;
 
 	_image_binary_end = .;
 
-	.bss_start (NOLOAD) : {
-		. = ALIGN(8);
-		KEEP(*(.__bss_start));
-	}
-
 	.bss (NOLOAD) : {
+		__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 3b7c9d515f..41d0b9471c 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -16,24 +16,18 @@ SECTIONS
 	. = ALIGN(4);
 	.text :
 	{
-		*(.__image_copy_start)
+		__image_copy_start = .;
 		*(.vectors)
 		CPUDIR/start.o (.text*)
 	}
 
 	/* This needs to come before *(.text*) */
-	.__efi_runtime_start : {
-		*(.__efi_runtime_start)
-	}
-
 	.efi_runtime : {
+		__efi_runtime_start = .;
 		*(.text.efi_runtime*)
 		*(.rodata.efi_runtime*)
 		*(.data.efi_runtime*)
-	}
-
-	.__efi_runtime_stop : {
-		*(.__efi_runtime_stop)
+		__efi_runtime_stop = .;
 	}
 
 	.text_rest :
@@ -49,77 +43,36 @@ SECTIONS
 		*(.data*)
 	}
 
-	. = ALIGN(4);
-
-	. = .;
-
 	. = ALIGN(4);
 	__u_boot_list : {
 		KEEP(*(SORT(__u_boot_list*)));
 	}
 
 	. = ALIGN(4);
-
-	.efi_runtime_rel_start :
-	{
-		*(.__efi_runtime_rel_start)
-	}
-
 	.efi_runtime_rel : {
+		__efi_runtime_rel_start = .;
 		*(.rel*.efi_runtime)
 		*(.rel*.efi_runtime.*)
-	}
-
-	.efi_runtime_rel_stop :
-	{
-		*(.__efi_runtime_rel_stop)
+		__efi_runtime_rel_stop = .;
 	}
 
 	. = ALIGN(8);
-	.image_copy_end :
-	{
-		*(.__image_copy_end)
-	}
-
-	.rel_dyn_start :
-	{
-		*(.__rel_dyn_start)
-	}
+	__image_copy_end = .;
 
 	.rel.dyn : {
+		__rel_dyn_start = .;
 		*(.rel*)
+		__rel_dyn_end = .;
 	}
 
-	.rel_dyn_end :
-	{
-		*(.__rel_dyn_end)
-	}
-
-	.end :
-	{
-		*(.__end)
-	}
-
+	_end = .;
 	_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 : {
+		__bss_start = .;
 		*(.bss*)
-		 . = ALIGN(8);
-		 __bss_limit = .;
-	}
-
-	.bss_end __bss_limit (OVERLAY) : {
-		KEEP(*(.__bss_end));
+		. = ALIGN(8);
+		__bss_end = .;
 	}
 
 	/*
-- 
2.39.2

Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[Heinrich Schuchardt]

Am 20. Mai 2023 22:55:37 MESZ schrieb Sam Edwards <cfsworks@gmail.com>:
>
>Here is a series of patches aimed at improving support for the LLVM
>toolchain (clang, lld, and to a lesser extent, llvm-objcopy) when
>targeting ARM. This toolchain is a cross-compiler "by default" -- a user
>generally should not need to install anything more to target their board
>of choice if they already have Clang installed. Additionally, Clang has
>a few nice diagnostics that should help appreciably with code quality,
>if Clang starts to be used regularly by a few more U-Boot developers. ;)
>
>Most of these patches are trivial and as such they should be pretty easy
>to review, but the later patches in the patchset start making some
>pretty big changes to the linker scripts. There are no behavioral
>changes with those (U-Boot should still function the same) but there is
>always the risk of compatibility with some third-party tool or loader
>being broken. Fortunately, I foresee any problems making themselves
>immediately apparent upon testing.
>
>I have tested booting on sunxi/T113, and building for Raspberry Pi
>(32-bit and 64-bit). The remaining testing efforts should be focused on:
>- Exynos
>- EFI loader (esp. on Lenovo X13s, which Heinrich Schuchardt had
>    mentioned in a previous commit as being fickle)
>- Zynq
>- Rockchip TPL
>- AArch64 SPL
>
>I'm submitting this as an RFC since this doesn't completely guarantee
>LLVM toolchain compatibility yet; my focus here is mostly on ensuring
>that I haven't caused any regressions in GNU-land. Also, I haven't
>discussed most of these changes before doing them. Perhaps alternate
>approaches to some of these things can be proposed - I'm all ears.
>
>Outstanding problems are:
>- LLD sometimes puts .hash outside of the image binary area, though this
>  is flagged by binary_size_check
>- The main makefile uses --gap-fill=0xff, which is not supported in
>  llvm-objcopy
>- llvm-objcopy also doesn't appear to speak S-Record; the u-boot.srec
>  target has to be deleted manually
>- llvm-objcopy gets upset at some of the EFI code, since the EFI linker
>  scripts preserve dynamic sections that llvm-objcopy doesn't want to
>  strip off
>
>Cheers,
>Sam


Hello Sam,

thanks for looking into llvm compatibility.

I guess the documentation and the CI testing would also have to be adjusted.

What about non-ARM architectures?

Could you, please, describe how built with lld so that reviewers can test it.

I find reviewing hard when receiving only 3 out of 10 patches.

Best regards

Heinrich

>
>
>Sam Edwards (10):
>  makefile: Fix symbol typo in binary_size_check
>  arm: set alignment properly for asm funcs
>  arm: exclude eabi_compat from LTO
>  arm: add __aeabi_memclr in eabi_compat
>  arm: add aligned-memory aliases to eabi_compat
>  arm: discard .gnu.version* sections
>  arm: efi_loader: discard hash, unwind information
>  arm: efi_loader: move .dynamic out of .text in EFI
>  arm: discard all .dyn* sections
>  arm: migrate away from sections.c
>
> Makefile                                 |   2 +-
> arch/arm/cpu/armv8/spl_data.c            |   4 +-
> arch/arm/cpu/armv8/u-boot-spl.lds        |  26 ++----
> arch/arm/cpu/armv8/u-boot.lds            |  48 +++--------
> arch/arm/cpu/u-boot.lds                  | 103 +++++++----------------
> arch/arm/include/asm/linkage.h           |   4 +-
> arch/arm/lib/Makefile                    |   3 +-
> arch/arm/lib/eabi_compat.c               |  17 ++++
> arch/arm/lib/elf_aarch64_efi.lds         |   3 +-
> arch/arm/lib/elf_arm_efi.lds             |   3 +
> arch/arm/lib/sections.c                  |  36 --------
> arch/arm/mach-rockchip/u-boot-tpl-v8.lds |  26 ++----
> arch/arm/mach-zynq/u-boot.lds            |  73 +++-------------
> 13 files changed, 96 insertions(+), 252 deletions(-)
> delete mode 100644 arch/arm/lib/sections.c
>

Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[Sam Edwards]

On 5/20/23 22:26, Heinrich Schuchardt wrote:
> Hello Sam,

Hi Heinrich! Good to hear from you.

> I guess the documentation and the CI testing would also have to be adjusted.

Ah, yeah, those are going to be big things for me to look at when this 
series starts to mature out of the RFC phase. CI is definitely important 
so that the hard-won compatibility doesn't just decay away. :)

> What about non-ARM architectures?

If there's a groundswell of demand for building U-Boot on LLVM, I'd be 
willing to collaborate with others on getting the other architectures up 
to parity with GNU. But since the linker scripts, relocation thunks, 
sections, and whatnot are all arch-specific, I'm only focusing on ARM 
for now (which is both the arch I need and one of the more common ones).

Is there a particular arch you'd like to see next? It seems everything 
U-Boot supports is supported by LLVM, except for Microblaze, NIOS2, and SH.

> Could you, please, describe how built with lld so that reviewers can test it.

I've been building with:

nice make CC='clang --target=armv7l-none-eabi' \
   LTO_FINAL_LDFLAGS=-fuse-ld=lld LD=ld.lld OBJCOPY=llvm-objcopy

...though mostly at this stage I'm just hoping for folks to confirm that 
this patchset causes no regressions in their existing GNU environments. 
(Feedback from LLVM-land would be appreciated nonetheless, though!!!)

> I find reviewing hard when receiving only 3 out of 10 patches.

Oof sorry about that. I haven't actually joined the U-Boot mailing list 
yet so the other patches are probably caught in the moderation queue. I 
guess wait for moderation approval and see if the other 7 arrive then -- 
I can resend the whole series to you specifically if not.

Cheers,
Sam

Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[Ilias Apalodimas]

Hi Sam,

On Sun, 21 May 2023 at 07:59, Sam Edwards <cfsworks@gmail.com> wrote:
>
> On 5/20/23 22:26, Heinrich Schuchardt wrote:
> > Hello Sam,
>
> Hi Heinrich! Good to hear from you.
>
> > I guess the documentation and the CI testing would also have to be adjusted.
>
> Ah, yeah, those are going to be big things for me to look at when this
> series starts to mature out of the RFC phase. CI is definitely important
> so that the hard-won compatibility doesn't just decay away. :)
>
> > What about non-ARM architectures?
>
> If there's a groundswell of demand for building U-Boot on LLVM, I'd be
> willing to collaborate with others on getting the other architectures up
> to parity with GNU. But since the linker scripts, relocation thunks,
> sections, and whatnot are all arch-specific, I'm only focusing on ARM
> for now (which is both the arch I need and one of the more common ones).

I can help clean up the arm architecture even further.  I was toying
with the idea of having page-aligned sections and eventually map
u-boot with proper permissions per section.  Right now (at least for
the majority of arm platforms) we are doing RWX for all the memory,
apart from devices that are mapped as RW. I do have an awfully hacky
PoC around, but the linker script cleanup is more than welcome.

>
> Is there a particular arch you'd like to see next? It seems everything
> U-Boot supports is supported by LLVM, except for Microblaze, NIOS2, and SH.
>
> > Could you, please, describe how built with lld so that reviewers can test it.
>
> I've been building with:
>
> nice make CC='clang --target=armv7l-none-eabi' \
>    LTO_FINAL_LDFLAGS=-fuse-ld=lld LD=ld.lld OBJCOPY=llvm-objcopy
>
> ...though mostly at this stage I'm just hoping for folks to confirm that
> this patchset causes no regressions in their existing GNU environments.
> (Feedback from LLVM-land would be appreciated nonetheless, though!!!)
>
> > I find reviewing hard when receiving only 3 out of 10 patches.
>
> Oof sorry about that. I haven't actually joined the U-Boot mailing list
> yet so the other patches are probably caught in the moderation queue. I
> guess wait for moderation approval and see if the other 7 arrive then --
> I can resend the whole series to you specifically if not.

It's probably not a mailing list issue.  I only got the efi related
patches on my mailbox.  The recipients were generated with
get_maintainers.pl?  Heinirch and I only received the efi* portions as
we maintain that subsystem

Thanks!
/Ilias
>
> Cheers,
> Sam

Re: [RFC PATCH 07/10] arm: efi_loader: discard hash, unwind information

[Ilias Apalodimas]

Hi Sam

On Sat, 20 May 2023 at 23:56, Sam Edwards <cfsworks@gmail.com> wrote:
>
> LLD tends to put these at the very beginning of the file, only
> for the .text 0x0 directive to end up going backward and
> overlapping them, creating an error.
>
> Since they don't appear to be used at runtime, just discard them.
>
> Signed-off-by: Sam Edwards <CFSworks@gmail.com>
> ---
>
>  arch/arm/lib/elf_arm_efi.lds | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/lib/elf_arm_efi.lds b/arch/arm/lib/elf_arm_efi.lds
> index 767ebda635..0b6cc5bcb6 100644
> --- a/arch/arm/lib/elf_arm_efi.lds
> +++ b/arch/arm/lib/elf_arm_efi.lds
> @@ -62,5 +62,8 @@ SECTIONS
>                 *(.dynstr)
>                 *(.note.gnu.build-id)
>                 *(.comment)
> +               *(.hash)
> +               *(.gnu.hash)

The reason we end up with both hash and gnu.hash is because the hash
style is set to 'both'.  Should we perhaps use (and strip) only one of
them?

Thanks
/Ilias

> +               *(.ARM.exidx)
>         }
>  }
> --
> 2.39.2
>

Re: [RFC PATCH 08/10] arm: efi_loader: move .dynamic out of .text in EFI

[Ilias Apalodimas]

Hi Sam,

On Sat, 20 May 2023 at 23:56, Sam Edwards <cfsworks@gmail.com> wrote:
>
> This is not proper: A .text section is SHT_PROGBITS,
> while the .dynamic section is SHT_DYNAMIC. Attempting to
> combine them like this creates a section type mismatch.
>
> It does seem that GNU ld does not complain, but LLVM's lld
> considers this an error.
>
> Signed-off-by: Sam Edwards <CFSworks@gmail.com>
> Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>
>  arch/arm/lib/elf_aarch64_efi.lds | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm/lib/elf_aarch64_efi.lds b/arch/arm/lib/elf_aarch64_efi.lds
> index 5dd9809169..986f13936d 100644
> --- a/arch/arm/lib/elf_aarch64_efi.lds
> +++ b/arch/arm/lib/elf_aarch64_efi.lds
> @@ -24,10 +24,9 @@ SECTIONS
>                 *(.gnu.linkonce.t.*)
>                 *(.srodata)
>                 *(.rodata*)
> -               . = ALIGN(16);
> -               *(.dynamic);
>                 . = ALIGN(512);
>         }
> +       .dynamic : { *(.dynamic) }
This looks correct.  However, this changed recently on commit
d7ddeb66a6ce ("efi_loader: fix building aarch64 EFI binaries").

Heinrich any chance you can repeat your tests?

Thanks
/Ilias
>         .rela.dyn : { *(.rela.dyn) }
>         .rela.plt : { *(.rela.plt) }
>         .rela.got : { *(.rela.got) }
> --
> 2.39.2
>

Re: [RFC PATCH 08/10] arm: efi_loader: move .dynamic out of .text in EFI

[Heinrich Schuchardt]

On 5/20/23 22:55, Sam Edwards wrote:
> This is not proper: A .text section is SHT_PROGBITS,
> while the .dynamic section is SHT_DYNAMIC. Attempting to
> combine them like this creates a section type mismatch.
>
> It does seem that GNU ld does not complain, but LLVM's lld
> considers this an error.
>
> Signed-off-by: Sam Edwards <CFSworks@gmail.com>
> Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>
>   arch/arm/lib/elf_aarch64_efi.lds | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm/lib/elf_aarch64_efi.lds b/arch/arm/lib/elf_aarch64_efi.lds
> index 5dd9809169..986f13936d 100644
> --- a/arch/arm/lib/elf_aarch64_efi.lds
> +++ b/arch/arm/lib/elf_aarch64_efi.lds
> @@ -24,10 +24,9 @@ SECTIONS
>   		*(.gnu.linkonce.t.*)
>   		*(.srodata)
>   		*(.rodata*)
> -		. = ALIGN(16);

.dynamic should be aligned. Structure Elf64_Dyn requires at least 8 byte
alignment.

> -		*(.dynamic);
>   		. = ALIGN(512);

The symbol _etext below should be 512 aligned as in
arch/arm/lib/crt0_aarch64_efi.S we have set FileAlignment = 0x200.

Best regards

Heinrich

>   	}
> +	.dynamic : { *(.dynamic) }
>   	.rela.dyn : { *(.rela.dyn) }
>   	.rela.plt : { *(.rela.plt) }
>   	.rela.got : { *(.rela.got) }

Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[Michal Simek]

Hi

On 5/21/23 06:59, Sam Edwards wrote:
> On 5/20/23 22:26, Heinrich Schuchardt wrote:
>> Hello Sam,
> 
> Hi Heinrich! Good to hear from you.
> 
>> I guess the documentation and the CI testing would also have to be adjusted.
> 
> Ah, yeah, those are going to be big things for me to look at when this series 
> starts to mature out of the RFC phase. CI is definitely important so that the 
> hard-won compatibility doesn't just decay away. :)
> 
>> What about non-ARM architectures?
> 
> If there's a groundswell of demand for building U-Boot on LLVM, I'd be willing 
> to collaborate with others on getting the other architectures up to parity with 
> GNU. But since the linker scripts, relocation thunks, sections, and whatnot are 
> all arch-specific, I'm only focusing on ARM for now (which is both the arch I 
> need and one of the more common ones).
> 
> Is there a particular arch you'd like to see next? It seems everything U-Boot 
> supports is supported by LLVM, except for Microblaze, NIOS2, and SH.
> 
>> Could you, please, describe how built with lld so that reviewers can test it.
> 
> I've been building with:
> 
> nice make CC='clang --target=armv7l-none-eabi' \
>    LTO_FINAL_LDFLAGS=-fuse-ld=lld LD=ld.lld OBJCOPY=llvm-objcopy
> 
> ...though mostly at this stage I'm just hoping for folks to confirm that this 
> patchset causes no regressions in their existing GNU environments. (Feedback 
> from LLVM-land would be appreciated nonetheless, though!!!)

Dockerfile in repo as I see is using 3 toolchain categories.
1. llvm deb repo
2. kernel.org
3. others - xtensa/arc

For CI loop you should pretty much provide a way how to get toolchain.
That's why would be good to figure it out and then I am happy to take a look at 
changed you have done for Zynq.
Definitely nice to see this happening and I expect more warnings will be visible 
  and they should be fixed.

Thanks,
Michal

Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[Tom Rini]

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

On Mon, May 22, 2023 at 12:39:04PM +0200, Michal Simek wrote:
> Hi
> 
> On 5/21/23 06:59, Sam Edwards wrote:
> > On 5/20/23 22:26, Heinrich Schuchardt wrote:
> > > Hello Sam,
> > 
> > Hi Heinrich! Good to hear from you.
> > 
> > > I guess the documentation and the CI testing would also have to be adjusted.
> > 
> > Ah, yeah, those are going to be big things for me to look at when this
> > series starts to mature out of the RFC phase. CI is definitely important
> > so that the hard-won compatibility doesn't just decay away. :)
> > 
> > > What about non-ARM architectures?
> > 
> > If there's a groundswell of demand for building U-Boot on LLVM, I'd be
> > willing to collaborate with others on getting the other architectures up
> > to parity with GNU. But since the linker scripts, relocation thunks,
> > sections, and whatnot are all arch-specific, I'm only focusing on ARM
> > for now (which is both the arch I need and one of the more common ones).
> > 
> > Is there a particular arch you'd like to see next? It seems everything
> > U-Boot supports is supported by LLVM, except for Microblaze, NIOS2, and
> > SH.
> > 
> > > Could you, please, describe how built with lld so that reviewers can test it.
> > 
> > I've been building with:
> > 
> > nice make CC='clang --target=armv7l-none-eabi' \
> >    LTO_FINAL_LDFLAGS=-fuse-ld=lld LD=ld.lld OBJCOPY=llvm-objcopy
> > 
> > ...though mostly at this stage I'm just hoping for folks to confirm that
> > this patchset causes no regressions in their existing GNU environments.
> > (Feedback from LLVM-land would be appreciated nonetheless, though!!!)
> 
> Dockerfile in repo as I see is using 3 toolchain categories.
> 1. llvm deb repo
> 2. kernel.org
> 3. others - xtensa/arc
> 
> For CI loop you should pretty much provide a way how to get toolchain.
> That's why would be good to figure it out and then I am happy to take a look
> at changed you have done for Zynq.
> Definitely nice to see this happening and I expect more warnings will be
> visible  and they should be fixed.

So, we can trivially add lld to the Dockerfile, it's just listing lld-16
in the install list.  I think objcopy is a bit of a stretch at this
point and it's not clear from the above if you're also making use of the
assembler.  We might also want to look at backporting
scripts/Makefile.clang from the current kernel build system and then
adapting the "guess the --target argument" logic based on CONFIG_$ARCH
rather than ARCH= (which we don't use).  That would also solve the LTO
problem as that's a result of us missing some flags that the kernel has
as LLVM+LTO (logically) requires LLVM LD not GNU LD.

At that point, and once the EFI guid_t warning is resolved to everyones
satisfaction we can put qemu_arm* + clang in the CI loop, to catch new
warnings there.  I've already got clang + Pi in my CI loop, but that
doesn't fail on warnings.

-- 
Tom

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

Re: [RFC PATCH 08/10] arm: efi_loader: move .dynamic out of .text in EFI

[Sam Edwards]

On 5/22/23 02:10, Heinrich Schuchardt wrote:

Hi Heinrich,

> .dynamic should be aligned. Structure Elf64_Dyn requires at least 8 byte
> alignment.

As best as I can tell, linkers (certainly lld[1], apparently also GNU ld 
judging by its default linker scripts) themselves set the proper word 
alignment on the .dynamic section that they synthesize for their 
internal input object. That alignment requirement bubbles up to the 
output section, so explicit alignment here should not be necessary.

> The symbol _etext below should be 512 aligned as in
> arch/arm/lib/crt0_aarch64_efi.S we have set FileAlignment = 0x200.

Ah, that definitely needs to be fixed since the .rela.* sections might 
not have the 512 alignment. I've updated my local branch, though this 
also needs to be addressed in the current master branch.

Cheers,
Sam

[1]: 
https://github.com/llvm/llvm-project/blob/acce2a315945e386a7be6f014ebe90c2a28f38d9/lld/ELF/SyntheticSections.cpp#L1246

Re: [RFC PATCH 07/10] arm: efi_loader: discard hash, unwind information

[Sam Edwards]

Hi Ilias,

On 5/22/23 01:00, Ilias Apalodimas wrote:
> The reason we end up with both hash and gnu.hash is because the hash
> style is set to 'both'.  Should we perhaps use (and strip) only one of
> them?

If we do keep one, it should probably be .hash -- see commit b02bfc4dfc.

I admit I'm completely mystified as to why we need the hash tables at 
all. The ELF spec says those are just for the dynamic linker, but 
neither the EFI code nor the self-relocating thunk require it, and I 
don't know of any target where the u-boot ELF itself is the shipped 
binary. For all I know, there never was a need to include .hash and 
Albert's commit fixed whatever problem he was facing only accidentally. 
Do you have any insights?

LLD's --hash-style option doesn't appear to have a "none" option or I'd 
just be making use of that here.

Cheers,
Sam

Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[Sam Edwards]

Hi Ilias,

On 5/22/23 00:52, Ilias Apalodimas wrote:
> I can help clean up the arm architecture even further.  I was toying
> with the idea of having page-aligned sections and eventually map
> u-boot with proper permissions per section.  Right now (at least for
> the majority of arm platforms) we are doing RWX for all the memory,
> apart from devices that are mapped as RW. I do have an awfully hacky
> PoC around, but the linker script cleanup is more than welcome.

Glad to hear it (and excited by the idea of proper W^X)! The linker 
script cleanup (i.e. deleting those pesky `sections.c` files and going 
back to linker-assigned symbols) can really happen whenever; it won't 
cause a problem on any version of GNU ld from <7 years ago. Perhaps a 
series of patches (one per arch) doing that should be landed first?

> It's probably not a mailing list issue.  I only got the efi related
> patches on my mailbox.  The recipients were generated with
> get_maintainers.pl?  Heinirch and I only received the efi* portions as
> we maintain that subsystem

Well, it's true that you and Heinrich weren't Cc: on every email in the 
series. I just went with patman's default behavior.

But every patch was sent To: the u-boot list, and I do see the whole 
series showing up on the archive. Did you not even receive the other 
patches in the series via the list?

Cheers,
Sam

Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[Sam Edwards]

Hi Tom,

On 5/22/23 09:30, Tom Rini wrote:
> I think objcopy is a bit of a stretch at this
> point and it's not clear from the above if you're also making use of the
> assembler.

I agree, since getting llvm-objcopy to play nice with this currently 
requires that I make a handful of small hack edits to the makefiles. It 
does offer a great advantage over GNU's objcopy in that it doesn't balk 
at ELFs from a foreign arch, but I'm only supporting it 
"opportunistically" right now.

I do make use of LLVM's assembler, but LLVM bundles its assembler inside 
the Clang binary (`clang -cc1as` and/or just pass .S files to Clang) 
rather than installing a separate program. This is not to be confused 
with `llvm-as` which is for bitcode/IR manipulation only. But in 
general, if you're using Clang, you're also using the LLVM assembler.

> We might also want to look at backporting
> scripts/Makefile.clang from the current kernel build system and then
> adapting the "guess the --target argument" logic based on CONFIG_$ARCH
> rather than ARCH= (which we don't use).  That would also solve the LTO
> problem as that's a result of us missing some flags that the kernel has
> as LLVM+LTO (logically) requires LLVM LD not GNU LD.

Having something like Linux's `LLVM=1` to enable LLVM would be ideal, I 
think. I probably won't be doing that backporting in this patch series 
since my goal for now is just to fill in some of the pitfalls for people 
like me who are too stubborn to install a GNU toolchain.

> At that point, and once the EFI guid_t warning is resolved to everyones
> satisfaction we can put qemu_arm* + clang in the CI loop, to catch new
> warnings there.  I've already got clang + Pi in my CI loop, but that
> doesn't fail on warnings.

Do you mean, on the current master branch, and only Clang (no LLD)? I'm 
in favor, but since a few of the patches in this series (#3-5) are to 
support some of the libcalls that LLVM's codegen likes to emit, I'd be 
surprised if that worked on all targets. Feel free to pull whatever 
necessary patches of mine here into your own series though. :)

Cheers,
Sam

Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[Heinrich Schuchardt]

Am 22. Mai 2023 21:37:26 MESZ schrieb Sam Edwards <cfsworks@gmail.com>:
>Hi Ilias,
>
>On 5/22/23 00:52, Ilias Apalodimas wrote:
>> I can help clean up the arm architecture even further.  I was toying
>> with the idea of having page-aligned sections and eventually map
>> u-boot with proper permissions per section.  Right now (at least for
>> the majority of arm platforms) we are doing RWX for all the memory,
>> apart from devices that are mapped as RW. I do have an awfully hacky
>> PoC around, but the linker script cleanup is more than welcome.
>
>Glad to hear it (and excited by the idea of proper W^X)! The linker script cleanup (i.e. deleting those pesky `sections.c` files and going back to linker-assigned symbols) can really happen whenever; it won't cause a problem on any version of GNU ld from <7 years ago. Perhaps a series of patches (one per arch) doing that should be landed first?
>
>> It's probably not a mailing list issue.  I only got the efi related
>> patches on my mailbox.  The recipients were generated with
>> get_maintainers.pl?  Heinirch and I only received the efi* portions as
>> we maintain that subsystem
>
>Well, it's true that you and Heinrich weren't Cc: on every email in the series. I just went with patman's default behavior.
>
>But every patch was sent To: the u-boot list, and I do see the whole series showing up on the archive. Did you not even receive the other patches in the series via the list?

The series can be retrieved from patchwork. I personally dislike patman for this eclectic behavior. Git send-email is doing the expected.

Best regards

Heinrich

>
>Cheers,
>Sam

Re: [RFC PATCH 07/10] arm: efi_loader: discard hash, unwind information

[Ilias Apalodimas]

Hi Sam,

On Mon, 22 May 2023 at 22:25, Sam Edwards <cfsworks@gmail.com> wrote:
>
> Hi Ilias,
>
> On 5/22/23 01:00, Ilias Apalodimas wrote:
> > The reason we end up with both hash and gnu.hash is because the hash
> > style is set to 'both'.  Should we perhaps use (and strip) only one of
> > them?
>
> If we do keep one, it should probably be .hash -- see commit b02bfc4dfc.
>
> I admit I'm completely mystified as to why we need the hash tables at
> all. The ELF spec says those are just for the dynamic linker, but
> neither the EFI code nor the self-relocating thunk require it, and I
> don't know of any target where the u-boot ELF itself is the shipped
> binary.

Me neither

> For all I know, there never was a need to include .hash and
> Albert's commit fixed whatever problem he was facing only accidentally.
> Do you have any insights?

Unfortunately not.  I just started looking up the linker scripts myself.

>
> LLD's --hash-style option doesn't appear to have a "none" option or I'd
> just be making use of that here.

Indeed.  I am fine with the patch regardless, switching the makefile
to only produce one of them is a nit anyway, since we'll eventually
get rid of them

Thanks
/Ilias
>
> Cheers,
> Sam

Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[Tom Rini]

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

On Mon, May 22, 2023 at 01:59:33PM -0600, Sam Edwards wrote:
> Hi Tom,
> 
> On 5/22/23 09:30, Tom Rini wrote:
> > I think objcopy is a bit of a stretch at this
> > point and it's not clear from the above if you're also making use of the
> > assembler.
> 
> I agree, since getting llvm-objcopy to play nice with this currently
> requires that I make a handful of small hack edits to the makefiles. It does
> offer a great advantage over GNU's objcopy in that it doesn't balk at ELFs
> from a foreign arch, but I'm only supporting it "opportunistically" right
> now.
> 
> I do make use of LLVM's assembler, but LLVM bundles its assembler inside the
> Clang binary (`clang -cc1as` and/or just pass .S files to Clang) rather than
> installing a separate program. This is not to be confused with `llvm-as`
> which is for bitcode/IR manipulation only. But in general, if you're using
> Clang, you're also using the LLVM assembler.

Ah, ok.

> > We might also want to look at backporting
> > scripts/Makefile.clang from the current kernel build system and then
> > adapting the "guess the --target argument" logic based on CONFIG_$ARCH
> > rather than ARCH= (which we don't use).  That would also solve the LTO
> > problem as that's a result of us missing some flags that the kernel has
> > as LLVM+LTO (logically) requires LLVM LD not GNU LD.
> 
> Having something like Linux's `LLVM=1` to enable LLVM would be ideal, I
> think. I probably won't be doing that backporting in this patch series since
> my goal for now is just to fill in some of the pitfalls for people like me
> who are too stubborn to install a GNU toolchain.

It honestly shouldn't be too much work to just backport that file (and
move/remove some of the logic we have scattered elsewhere instead).

> > At that point, and once the EFI guid_t warning is resolved to everyones
> > satisfaction we can put qemu_arm* + clang in the CI loop, to catch new
> > warnings there.  I've already got clang + Pi in my CI loop, but that
> > doesn't fail on warnings.
> 
> Do you mean, on the current master branch, and only Clang (no LLD)? I'm in
> favor, but since a few of the patches in this series (#3-5) are to support
> some of the libcalls that LLVM's codegen likes to emit, I'd be surprised if
> that worked on all targets. Feel free to pull whatever necessary patches of
> mine here into your own series though. :)

The reason it's not in CI right now is that there's compiler warnings,
due to the EFI guid_t alignment thing that's being discussed in another
thread.  I am running (and so booting and running pytest) in my CI loop
clang-built U-Boot for Pi 3 (32bit and 64bit) and a few other targets
too.  But there's also certainly room to improve and I think some of the
issues you've addressed here are why other targets for example are too
large to boot.

-- 
Tom

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

Re: [RFC PATCH 07/10] arm: efi_loader: discard hash, unwind information

[Heinrich Schuchardt]

Am 22. Mai 2023 21:25:50 MESZ schrieb Sam Edwards <cfsworks@gmail.com>:
>Hi Ilias,
>
>On 5/22/23 01:00, Ilias Apalodimas wrote:
>> The reason we end up with both hash and gnu.hash is because the hash
>> style is set to 'both'.  Should we perhaps use (and strip) only one of
>> them?
>
>If we do keep one, it should probably be .hash -- see commit b02bfc4dfc.
>
>I admit I'm completely mystified as to why we need the hash tables at all. The ELF spec says those are just for the dynamic linker, but neither the EFI code nor the self-relocating thunk require it, and I don't know of any target where the u-boot ELF itself is the shipped binary. For all I know, there never was a need to include .hash and Albert's commit fixed whatever problem he was facing only accidentally. Do you have any insights?
>

Ubuntu's and Debian's u-boot-qemu package ships uboot.elf for multiple architectures. Cf. https://packages.debian.org/bullseye/all/u-boot-qemu/filelist

You can pass uboot.elf as -kernel parameter to QEMU.

Best regards

Heinrich 


>LLD's --hash-style option doesn't appear to have a "none" option or I'd just be making use of that here.
>
>Cheers,
>Sam

Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain

[Ilias Apalodimas]

On Mon, May 22, 2023 at 01:37:26PM -0600, Sam Edwards wrote:
> Hi Ilias,
>
> On 5/22/23 00:52, Ilias Apalodimas wrote:
> > I can help clean up the arm architecture even further.  I was toying
> > with the idea of having page-aligned sections and eventually map
> > u-boot with proper permissions per section.  Right now (at least for
> > the majority of arm platforms) we are doing RWX for all the memory,
> > apart from devices that are mapped as RW. I do have an awfully hacky
> > PoC around, but the linker script cleanup is more than welcome.
>
> Glad to hear it (and excited by the idea of proper W^X)!

Yes that's my end goal here.  Looking around the code we have in U-Boot
regarding the mmu configuration, it's a lot easier (and cleaner) to fix
the linker script, add symbols for ro_start, rw_start etc and layout the
binary in a way we can easily map it, instead of leaving it as is and try
to fix the mapping in c code.

> The linker script
> cleanup (i.e. deleting those pesky `sections.c` files and going back to
> linker-assigned symbols) can really happen whenever; it won't cause a
> problem on any version of GNU ld from <7 years ago. Perhaps a series of
> patches (one per arch) doing that should be landed first?

Yes probably, because I specifically remember digging through the history
of why the sections were defined like that in the first place.

>
> > It's probably not a mailing list issue.  I only got the efi related
> > patches on my mailbox.  The recipients were generated with
> > get_maintainers.pl?  Heinirch and I only received the efi* portions as
> > we maintain that subsystem
>
> Well, it's true that you and Heinrich weren't Cc: on every email in the
> series. I just went with patman's default behavior.
>
> But every patch was sent To: the u-boot list, and I do see the whole series
> showing up on the archive. Did you not even receive the other patches in the
> series via the list?
>
> Cheers,
> Sam

Cheers
/Ilias

Re: [RFC PATCH 10/10] arm: migrate away from sections.c

[Ilias Apalodimas]

On Sat, May 20, 2023 at 02:55:47PM -0600, Sam Edwards wrote:
> This patch effectively reverts 3ebd1cbc49f0005092d69cf0d9a6e64d7a1c300b.

Also 47bd65ef057fb71b02b32741d5cfcaf03e2f0918 ?

>
> The approach taken in that commit was to have the section-marking
> symbols generated into empty sections by the compiler, for the linker
> script to include at the correct location. The rationale was that at
> the time, the linker considered linker-assigned symbols to be dynamic
> when they were in PIC (PIEs or shared libraries), which meant they were
> represented at runtime by a R_ARM_ABS32 relocation (by symbol name)
> rather than by M_ARM_RELATIVE.
>
> That commit landed in March 2013, but GNU ld later changed its behavior
> on 2016-02-23 to default linker-assigned symbols to dynamic only in
> shared libraries (not PIE), so this approach is unnecessary.
>
> I am removing it, because:
> 1) It required keeping sections.c in sync with multiple linker scripts.
> 2) It added complexity to the linker scripts, making them less readable.
> 3) It added unnecessary sections to the output, which can't be merged
>    because the sections are sometimes of different types.
> 4) The linker may insert sections not explicitly named in the script
>    somewhere between explicit sections; having the marker symbols
>    outside of the sections they were marking meant the markers could
>    end up with an unintended section inserted within that region.
>
> Signed-off-by: Sam Edwards <CFSworks@gmail.com>
> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
>

 Thanks
 /Ilias