diff mbox series

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

Message ID 081eca5bc4d705e36fbe563d4035eeb200777ff0.1585230866.git.michal.simek@xilinx.com
State New
Headers show
Series [v1] arch: Add explicit linker script for u-boot-elf | expand

Commit Message

Michal Simek March 26, 2020, 1:54 p.m. UTC 
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 v1:
- Compare to RFC
- MIPS has different elf entry point that's why I replaced simple cat with
  sed to setup proper entry point

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.

---
 Makefile            | 11 +++++++++--
 arch/u-boot-elf.lds |  9 +++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 arch/u-boot-elf.lds

Comments

Daniel Schwierzeck March 26, 2020, 2:39 p.m. UTC | #1 
Hi Michal,

Am 26.03.20 um 14:54 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 v1:
> - Compare to RFC
> - MIPS has different elf entry point that's why I replaced simple cat with
>   sed to setup proper entry point
> 
> 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.

maybe include/asm-generic/ ?

> 
> ---
>  Makefile            | 11 +++++++++--
>  arch/u-boot-elf.lds |  9 +++++++++
>  2 files changed, 18 insertions(+), 2 deletions(-)
>  create mode 100644 arch/u-boot-elf.lds
> 
> diff --git a/Makefile b/Makefile
> index be1513227361..7aebfc3a2de1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1654,12 +1654,19 @@ ifndef PLATFORM_ELFENTRY
>  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=$(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)
>  
> +quiet_sed_elfentry = SED_ELF $@
> +cmd_sed_elfentry =                                                      \
> +	sed "s at PLATFORM_ELFENTRY@$(PLATFORM_ELFENTRY)@g" $< > $@
> +
> +u-boot-elf.lds: arch/u-boot-elf.lds prepare FORCE
> +	$(call if_changed,sed_elfentry)

you could also run the script through the preprocessor like it's done
for the other .lds files. This way you could use Kconfig options for
stuff like PLATFORM_ELFENTRY.

> +
>  # 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/u-boot-elf.lds b/arch/u-boot-elf.lds
> new file mode 100644
> index 000000000000..d3eb6f3fc1db
> --- /dev/null
> +++ b/arch/u-boot-elf.lds
> @@ -0,0 +1,9 @@
> +ENTRY(PLATFORM_ELFENTRY)
> +SECTIONS
> +{
> +	. = PLATFORM_ELFENTRY;
> +
> +	.data : {
> +		*(.data*)
> +	}
> +}
>
Michal Simek March 26, 2020, 3:20 p.m. UTC | #2 
Hi,

On 26. 03. 20 15:39, Daniel Schwierzeck wrote:
> Hi Michal,
> 
> Am 26.03.20 um 14:54 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 v1:
>> - Compare to RFC
>> - MIPS has different elf entry point that's why I replaced simple cat with
>>   sed to setup proper entry point
>>
>> 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.
> 
> maybe include/asm-generic/ ?

I will wait also for others to comment.

> 
>>
>> ---
>>  Makefile            | 11 +++++++++--
>>  arch/u-boot-elf.lds |  9 +++++++++
>>  2 files changed, 18 insertions(+), 2 deletions(-)
>>  create mode 100644 arch/u-boot-elf.lds
>>
>> diff --git a/Makefile b/Makefile
>> index be1513227361..7aebfc3a2de1 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1654,12 +1654,19 @@ ifndef PLATFORM_ELFENTRY
>>  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=$(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)
>>  
>> +quiet_sed_elfentry = SED_ELF $@
>> +cmd_sed_elfentry =                                                      \
>> +	sed "s at PLATFORM_ELFENTRY@$(PLATFORM_ELFENTRY)@g" $< > $@
>> +
>> +u-boot-elf.lds: arch/u-boot-elf.lds prepare FORCE
>> +	$(call if_changed,sed_elfentry)
> 
> you could also run the script through the preprocessor like it's done
> for the other .lds files. This way you could use Kconfig options for
> stuff like PLATFORM_ELFENTRY.

Thanks for suggestion. I have tried it and also works and looks better.
Will wait for location and then will send v2.

M
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index be1513227361..7aebfc3a2de1 100644
--- a/Makefile
+++ b/Makefile
@@ -1654,12 +1654,19 @@  ifndef PLATFORM_ELFENTRY
 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=$(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)
 
+quiet_sed_elfentry = SED_ELF $@
+cmd_sed_elfentry =                                                      \
+	sed "s at PLATFORM_ELFENTRY@$(PLATFORM_ELFENTRY)@g" $< > $@
+
+u-boot-elf.lds: arch/u-boot-elf.lds prepare FORCE
+	$(call if_changed,sed_elfentry)
+
 # 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/u-boot-elf.lds b/arch/u-boot-elf.lds
new file mode 100644
index 000000000000..d3eb6f3fc1db
--- /dev/null
+++ b/arch/u-boot-elf.lds
@@ -0,0 +1,9 @@ 
+ENTRY(PLATFORM_ELFENTRY)
+SECTIONS
+{
+	. = PLATFORM_ELFENTRY;
+
+	.data : {
+		*(.data*)
+	}
+}