Hi Daniel,

Tested on Netgear DGND3700v2 (bmips/bcm6362) with no regressions.

Tested-By: Álvaro Fernández Rojas <nolt...@gmail.com>

> El 2 abr 2020, a las 11:28, Daniel Schwierzeck <daniel.schwierz...@gmail.com> 
> escribió:
> 
> +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.si...@xilinx.com>
>> ---
>> 
>> Changes in v2:
>> - Use preprocessor and Kconfig entry for passing different PLATFORM_ENTRY
>>  Suggested-by: Daniel Schwierzeck <daniel.schwierz...@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.schwierz...@gmail.com 
> <mailto:daniel.schwierz...@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

Reply via email to