[PATCH 07/10] Support separate DTB files with the UEFI app

[Ilias Apalodimas]

Hi Matthew

On Sat, 23 Nov 2024 at 21:57, Matthew Garrett <mjg59 at srcf.ucam.org> wrote:
>
> From: Matthew Garrett <mgarrett at aurora.tech>
>
> The UEFI app is an actual executable with things like section headers,
> so just gluing the DTB onto the end of it won't work. Add an additional
> section to contain this and allocate some space, and then during build
> copy the DTB into that section.
>
> Signed-off-by: Matthew Garrett <mgarrett at aurora.tech>
> ---
>
>  Makefile                        | 7 ++++++-
>  arch/x86/config.mk              | 2 +-
>  arch/x86/lib/elf_x86_64_efi.lds | 4 ++++
>  include/asm-generic/sections.h  | 1 +
>  lib/efi/Makefile                | 2 +-
>  lib/efi/efi_dtb.S               | 6 ++++++
>  lib/fdtdec.c                    | 3 +++
>  7 files changed, 22 insertions(+), 3 deletions(-)
>  create mode 100644 lib/efi/efi_dtb.S
>
> diff --git a/Makefile b/Makefile
> index 2eaae427961..18abaa1ac52 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1067,6 +1067,10 @@ quiet_cmd_objcopy = OBJCOPY $@
>  cmd_objcopy = $(OBJCOPY) --gap-fill=0xff $(OBJCOPYFLAGS) \
>         $(OBJCOPYFLAGS_$(@F)) $< $@
>
> +# Inject the DTB into u-boot
> +quiet_cmd_embeddtb = OBJCOPY $@
> +cmd_embeddtb = $(OBJCOPY) --update-section .embedded_dtb=dts/dt.dtb --set-section-flags .embedded_dtb=contents,alloc,load,data $<
> +
>  # Provide a version which does not do this, for use by EFI
>  quiet_cmd_zobjcopy = OBJCOPY $@
>  cmd_zobjcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $(OBJCOPYFLAGS_$(@F)) $< $@
> @@ -1673,7 +1677,8 @@ u-boot-x86-reset16.bin: u-boot FORCE
>  endif # CONFIG_X86
>
>  OBJCOPYFLAGS_u-boot-app.efi := $(OBJCOPYFLAGS_EFI)
> -u-boot-app.efi: u-boot FORCE
> +u-boot-app.efi: u-boot dts/dt.dtb FORCE
> +       $(call if_changed,embeddtb)
>         $(call if_changed,zobjcopy)
>
>  u-boot.bin.o: u-boot.bin FORCE
> diff --git a/arch/x86/config.mk b/arch/x86/config.mk
> index 6d4839dfb38..ac1f1922b12 100644
> --- a/arch/x86/config.mk
> +++ b/arch/x86/config.mk
> @@ -45,7 +45,7 @@ LDFLAGS_EFI_PAYLOAD := -Bsymbolic -Bsymbolic-functions -shared --no-undefined \
>                        -s -zexecstack
>
>  OBJCOPYFLAGS_EFI := -j .text -j .sdata -j .data -j .dynamic -j .dynsym \
> -       -j .rel -j .rela -j .reloc --strip-all
> +       -j .rel -j .rela -j .reloc -j .embedded_dtb --strip-all
>
>  # Compiler flags to be added when building UEFI applications
>  CFLAGS_EFI := -fpic -fshort-wchar
> diff --git a/arch/x86/lib/elf_x86_64_efi.lds b/arch/x86/lib/elf_x86_64_efi.lds
> index ada024c05c3..cb656ac46ea 100644
> --- a/arch/x86/lib/elf_x86_64_efi.lds
> +++ b/arch/x86/lib/elf_x86_64_efi.lds
> @@ -79,5 +79,9 @@ SECTIONS
>                 *(.note.GNU-stack)
>         }
>
> +       .embedded_dtb : {
> +               *(.embedded_dtb)
> +       }
> +
>         .comment 0 : { *(.comment) }
>  }
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index b6bca53db10..4113ea2a866 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -70,6 +70,7 @@ extern char __image_copy_start[], __image_copy_end[];
>  extern char __bss_end[];
>  extern char __rel_dyn_start[], __rel_dyn_end[];
>  extern char _image_binary_end[];
> +extern char _dtb[];
>
>  /*
>   * This is the U-Boot entry point - prior to relocation it should be same
> diff --git a/lib/efi/Makefile b/lib/efi/Makefile
> index 63845287336..9f51671c65d 100644
> --- a/lib/efi/Makefile
> +++ b/lib/efi/Makefile
> @@ -2,7 +2,7 @@
>  #
>  # (C) Copyright 2015 Google, Inc
>
> -obj-$(CONFIG_EFI_APP) += efi_app.o efi.o efi_app_init.o efi_vars.o
> +obj-$(CONFIG_EFI_APP) += efi_app.o efi.o efi_app_init.o efi_vars.o efi_dtb.o
>  obj-$(CONFIG_EFI_STUB) += efi_info.o
>
>  CFLAGS_REMOVE_efi_stub.o := -mregparm=3 \
> diff --git a/lib/efi/efi_dtb.S b/lib/efi/efi_dtb.S
> new file mode 100644
> index 00000000000..75e0c4a5765
> --- /dev/null
> +++ b/lib/efi/efi_dtb.S
> @@ -0,0 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +#ifdef CONFIG_OF_SEPARATE
> +.section .embedded_dtb, "a"
> +.globl __dtb
> +__dtb: .fill 1024*1024
> +#endif
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index b0655988029..63853f816f4 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -1236,6 +1236,9 @@ static void *fdt_find_separate(void)
>                 fdt_blob = (ulong *)_image_binary_end;
>         else
>                 fdt_blob = (ulong *)__bss_end;
> +#elif defined CONFIG_EFI_APP

Don't you need CONFIG_OF_SEPARATE as well here?
I haven't checked the EFI app, is this the only available option?

> +       /* FDT is in a separate section */
> +       fdt_blob = (ulong *)__dtb;
>  #else
>         /* FDT is at end of image */
>         fdt_blob = (ulong *)_end;
> --
> 2.47.0
>

Other than that this looks reasonable to me.

Thanks
/Ilias