sunxi/arm64 build broken (was: [PATCH v4 12/27] x86: Makefile: Drop explicit targets built by binman)

[André Przywara]

On 19/07/2020 20:56, Simon Glass wrote:

Hi,

> On x86 various files that need to be created by binman. It does not make
> sense to enumerate these in the Makefile. They are described in the
> configuration (devicetree) for each board and we can simply run binman
> (always) to generate them.
> 
> Update the Makefile to have a separate, final step which runs binman,
> once all input dependencies are present.
> 
> This avoid sprinkling the Makefile with arch-specific code.

it seems that this patch (committed as 42b18df80fdb) breaks sunxi(!).
When building current master, the Trusted Firmware binary does not make
it into the u-boot-sunxi-with-spl.bin file. U-Boot still runs, but Linux
won't get very far without BL31. To me it looks like we combine
u-boot.img with the SPL, instead of u-boot.itb. Only the latter contains
bl31.bin.

I bisected down into the hunks of this patch here, and by reverting just
hunk #3 (the one introducing .binman_stamp) it works again.
My knowledge of the U-Boot build system is somewhat limited, so I don't
immediately see the problem or a fix.

A simple way to verify a build is complete would be:
$ strings u-boot-sunxi-with-spl.bin | grep BL31
If it reports some lines, TF-A is included and it would work correctly.
If it comes back empty, TF-A is missing and we won't boot.

I would be grateful for any insights!

Cheers,
Andre

> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> (no changes since v3)
> 
> Changes in v3:
> - Drop rockchip changes which should not be in this patch
> 
>  Makefile | 63 +++++++++++++-------------------------------------------
>  1 file changed, 14 insertions(+), 49 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 0147d9d49b..b4b5b10813 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -921,16 +921,6 @@ INPUTS-$(CONFIG_REMAKE_ELF) += u-boot.elf
>  INPUTS-$(CONFIG_EFI_APP) += u-boot-app.efi
>  INPUTS-$(CONFIG_EFI_STUB) += u-boot-payload.efi
>  
> -ifneq ($(CONFIG_HAS_ROM),)
> -ifneq ($(BUILD_ROM)$(CONFIG_BUILD_ROM),)
> -INPUTS-$(CONFIG_X86_RESET_VECTOR) += u-boot.rom
> -endif
> -endif
> -
> -ifeq ($(CONFIG_SYS_COREBOOT)$(CONFIG_SPL),yy)
> -INPUTS-$(CONFIG_BINMAN) += u-boot-x86-with-spl.bin
> -endif
> -
>  # Build a combined spl + u-boot image for sunxi
>  ifeq ($(CONFIG_ARCH_SUNXI)$(CONFIG_SPL),yy)
>  INPUTS-y += u-boot-sunxi-with-spl.bin
> @@ -961,6 +951,10 @@ ifeq ($(CONFIG_ARCH_ROCKCHIP)$(CONFIG_SPL),yy)
>  INPUTS-y += u-boot-rockchip.bin
>  endif
>  
> +INPUTS-$(CONFIG_X86) += u-boot-x86-start16.bin u-boot-x86-reset16.bin \
> +	$(if $(CONFIG_SPL_X86_16BIT_INIT),spl/u-boot-spl.bin) \
> +	$(if $(CONFIG_TPL_X86_16BIT_INIT),tpl/u-boot-tpl.bin)
> +
>  LDFLAGS_u-boot += $(LDFLAGS_FINAL)
>  
>  # Avoid 'Not enough room for program headers' error on binutils 2.28 onwards.
> @@ -1018,7 +1012,14 @@ cmd_cfgcheck = $(srctree)/scripts/check-config.sh $2 \
>  PHONY += inputs
>  inputs: $(INPUTS-y)
>  
> -all: inputs
> +all: .binman_stamp inputs
> +ifeq ($(CONFIG_BINMAN),y)
> +	$(call if_changed,binman)
> +endif
> +
> +# Timestamp file to make sure that binman always runs
> +.binman_stamp: FORCE
> +	@touch $@
>  
>  ifeq ($(CONFIG_DEPRECATED),y)
>  	$(warning "You have deprecated configuration options enabled in your .config! Please check your configuration.")
> @@ -1311,7 +1312,7 @@ quiet_cmd_binman = BINMAN  $@
>  cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>                  --toolpath $(objtree)/tools \
>  		$(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \
> -		build -u -d u-boot.dtb -O . -m \
> +		build -u -d u-boot.dtb -O . -m --allow-missing \
>  		-I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
>  		$(BINMAN_$(@F))
>  
> @@ -1588,27 +1589,11 @@ u-boot-br.bin: u-boot FORCE
>  endif
>  endif
>  
> -# x86 uses a large ROM. We fill it with 0xff, put the 16-bit stuff (including
> -# reset vector) at the top, Intel ME descriptor at the bottom, and U-Boot in
> -# the middle. This is handled by binman based on an image description in the
> -# board's device tree.
> -ifneq ($(CONFIG_HAS_ROM),)
> -rom: u-boot.rom FORCE
> -
> -refcode.bin: $(srctree)/board/$(BOARDDIR)/refcode.bin FORCE
> -	$(call if_changed,copy)
> -
>  quiet_cmd_ldr = LD      $@
>  cmd_ldr = $(LD) $(LDFLAGS_$(@F)) \
>  	       $(filter-out FORCE,$^) -o $@
>  
> -rom-deps := u-boot.bin
>  ifdef CONFIG_X86
> -rom-deps += u-boot-x86-start16.bin u-boot-x86-reset16.bin \
> -		$(if $(CONFIG_SPL_X86_16BIT_INIT),spl/u-boot-spl.bin) \
> -		$(if $(CONFIG_TPL_X86_16BIT_INIT),tpl/u-boot-tpl.bin) \
> -		$(if $(CONFIG_HAVE_REFCODE),refcode.bin)
> -
>  OBJCOPYFLAGS_u-boot-x86-start16.bin := -O binary -j .start16
>  u-boot-x86-start16.bin: u-boot FORCE
>  	$(call if_changed,objcopy)
> @@ -1617,24 +1602,7 @@ OBJCOPYFLAGS_u-boot-x86-reset16.bin := -O binary -j .resetvec
>  u-boot-x86-reset16.bin: u-boot FORCE
>  	$(call if_changed,objcopy)
>  
> -else # !CONFIG_X86
> -
> -ifdef CONFIG_SPL
> -rom-deps += spl/u-boot-spl.bin
> -
> -# We can rely on CONFIG_SPL_FRAMEWORK being set for boards that use binman
> -rom-deps += u-boot.img
> -endif
> -
> -ifdef CONFIG_TPL
> -rom-deps += tpl/u-boot-tpl.bin
> -endif
> -
> -endif
> -
> -u-boot.rom: $(rom-deps) FORCE
> -	$(call if_changed,binman)
> -endif
> +endif # CONFIG_X86
>  
>  ifneq ($(CONFIG_ARCH_SUNXI),)
>  ifeq ($(CONFIG_ARM64),)
> @@ -1646,9 +1614,6 @@ u-boot-sunxi-with-spl.bin: spl/sunxi-spl.bin u-boot.itb FORCE
>  endif
>  endif
>  
> -u-boot-x86-with-spl.bin: spl/u-boot-spl.bin u-boot.bin FORCE
> -	$(call if_changed,binman)
> -
>  ifneq ($(CONFIG_ARCH_TEGRA),)
>  # Makes u-boot-dtb-tegra.bin u-boot-tegra.bin u-boot-nodtb-tegra.bin
>  %-dtb-tegra.bin %-tegra.bin %-nodtb-tegra.bin: spl/%-spl %.bin FORCE
>