[PATCH v2] arch: Add explicit linker script for u-boot-elf

[Daniel Schwierzeck]

+cc Álvaro

Am 30.03.20 um 13:58 schrieb Michal Simek:
> Commit f4dc714aaa2d ("arm64: Turn u-boot.bin back into an ELF file after
> relocate-rela")
> introduce REMAKE_ELF option to recreate u-boot.elf from u-boot ->
> u-boot.bin + DT -> u-boot.elf.
> 
> The best is to ilustrate it from make V=1 output
>   cat u-boot-nodtb.bin dts/dt.dtb > u-boot-dtb.bin
>   cp u-boot-dtb.bin u-boot.bin
> aarch64-linux-gnu-objcopy -I binary -B aarch64 -O elf64-littleaarch64  u-boot.bin u-boot-elf.o
>   aarch64-linux-gnu-ld.bfd u-boot-elf.o -o u-boot.elf --defsym="_start"=0x8000000 -Ttext=0x8000000
> 
> Last command has no explicit linker script passed that's why toolchain
> internal linker script is used.
> In Binutils 2.32 case it contains SIZEOF_HEADERS symbol which has changed
> behavior by commit
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=64029e93683a266c38d19789e780f3748bd6a188
> which result in situation that program headers has changed from
> (xilinx_zynqmp_mini_defconfig)
> 
> Program Headers:
>   Type           Offset             VirtAddr           PhysAddr
>                  FileSiz            MemSiz              Flags  Align
>   LOAD           0x0000000000010000 0x00000000fffc0000 0x00000000fffc0000
>                  0x0000000000018918 0x0000000000018918  RW     0x10000
> 
> to
> 
> Program Headers:
>   Type           Offset             VirtAddr           PhysAddr
>                  FileSiz            MemSiz              Flags  Align
>   LOAD           0x0000000000000000 0x00000000fffb0000 0x00000000fffb0000
>                  0x0000000000028918 0x0000000000028918  RW     0x10000
> 
> Xilinx tools like XSDB or Bootgen are using program headers for loading ELF
> to the right location and by above binutils change ELF is loaded to
> incorrect location.
> 
> The patch is explicitly use u-boot-elf.lds (just cat now) for u-boot.elf
> recreation which is called when REMAKE_ELF is setup.
> By purpose u-boot-elf.lds doesn't contain OUTPUT_FORMAT/OUTPUT_ARCH to be
> able to use by all archs.
> 
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
> 
> Changes in v2:
> - Use preprocessor and Kconfig entry for passing different PLATFORM_ENTRY
>   Suggested-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
> - I kept u-boot.elf.lds in origin location because most of lds are in arch/
>   folder
> 
> Changes in v1:
> - Compare to RFC
> - MIPS has different elf entry point that's why I replaced simple cat with
>   sed to setup proper entry point

Álvaro, could you test for regressions on BMIPS platform? Thanks.

Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>

> 
> Sadanand, Mahesh: Please correct my description if I am wrong based on our
> internal discussion about this issue.
> 
> Tom: I have been told that it is GCC issue but I found that commit in
> binutils.
> I am not doing these makefile stuff that's why I expect this should be
> changed a little bit. Also arch/u-boot-elf.lds is likely incorrect location
> but didn't find any better.
> 
> ---
>  Kconfig             |  5 +++++
>  Makefile            | 11 +++++------
>  arch/mips/config.mk |  1 -
>  arch/u-boot-elf.lds |  9 +++++++++
>  4 files changed, 19 insertions(+), 7 deletions(-)
>  create mode 100644 arch/u-boot-elf.lds
> 
> diff --git a/Kconfig b/Kconfig
> index 66148ce47790..dc23c35000f2 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -298,6 +298,11 @@ config ERR_PTR_OFFSET
>  	  pointer values - up to 'MAX_ERRNO' bytes below this value must be
>  	  unused/invalid addresses.
>  
> +config PLATFORM_ELFENTRY
> +	string
> +	default "__start" if MIPS
> +	default "_start"
> +
>  endmenu		# General setup
>  
>  menu "Boot images"
> diff --git a/Makefile b/Makefile
> index be1513227361..c1dc185e5c0e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1648,18 +1648,17 @@ OBJCOPYFLAGS_u-boot-img-spl-at-end.bin := -I binary -O binary \
>  u-boot-img-spl-at-end.bin: u-boot.img spl/u-boot-spl.bin FORCE
>  	$(call if_changed,pad_cat)
>  
> -# Create a new ELF from a raw binary file.
> -ifndef PLATFORM_ELFENTRY
> -  PLATFORM_ELFENTRY = "_start"
> -endif
>  quiet_cmd_u-boot-elf ?= LD      $@
>  	cmd_u-boot-elf ?= $(LD) u-boot-elf.o -o $@ \
> -	--defsym=$(PLATFORM_ELFENTRY)=$(CONFIG_SYS_TEXT_BASE) \
> +	-T u-boot-elf.lds --defsym=$(CONFIG_PLATFORM_ELFENTRY)=$(CONFIG_SYS_TEXT_BASE) \
>  	-Ttext=$(CONFIG_SYS_TEXT_BASE)
> -u-boot.elf: u-boot.bin
> +u-boot.elf: u-boot.bin u-boot-elf.lds
>  	$(Q)$(OBJCOPY) -I binary $(PLATFORM_ELFFLAGS) $< u-boot-elf.o
>  	$(call if_changed,u-boot-elf)
>  
> +u-boot-elf.lds: arch/u-boot-elf.lds prepare FORCE
> +	$(call if_changed_dep,cpp_lds)
> +
>  # MediaTek's ARM-based u-boot needs a header to contains its load address
>  # which is parsed by the BootROM.
>  # If the SPL build is enabled, the header will be added to the spl binary,
> diff --git a/arch/mips/config.mk b/arch/mips/config.mk
> index 9d3a84539a7d..527fd6a2fd92 100644
> --- a/arch/mips/config.mk
> +++ b/arch/mips/config.mk
> @@ -36,7 +36,6 @@ CONFIG_STANDALONE_LOAD_ADDR	?= 0xffffffff80200000
>  endif
>  
>  PLATFORM_CPPFLAGS += -D__MIPS__
> -PLATFORM_ELFENTRY = "__start"
>  PLATFORM_ELFFLAGS += -B mips $(OBJCOPYFLAGS)
>  
>  #
> diff --git a/arch/u-boot-elf.lds b/arch/u-boot-elf.lds
> new file mode 100644
> index 000000000000..1666027e3635
> --- /dev/null
> +++ b/arch/u-boot-elf.lds
> @@ -0,0 +1,9 @@
> +ENTRY(CONFIG_PLATFORM_ELFENTRY)
> +SECTIONS
> +{
> +	. = CONFIG_PLATFORM_ELFENTRY;
> +
> +	.data : {
> +		*(.data*)
> +	}
> +}
> 

-- 
- Daniel