Re: [U-Boot] [PATCH 1/2] efi_loader: Pass fdt address directly to bootefi cmd

On 14.04.16 00:26, Andreas Färber wrote:

Am 13.04.2016 um 23:22 schrieb Alexander Graf:

...

The bootefi cmd today fetches its device tree pointer from either the location appointed by "fdt addr" with a fallback to the U-Boot control fdt.

This integration is unusual for U-Boot and diverges from the way we usually handle parameters to boot commands. So let's pass the fdt directly into the bootefi command instead and move the control fdt logic into the distro boot script.

Signed-off-by: Alexander Graf agraf@suse.de

cmd/bootefi.c | 34 +++++++++++++--------------------- include/config_distro_bootcmd.h | 9 ++++++--- 2 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c index b213ef1..ab39b95 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -142,12 +142,11 @@ static void *copy_fdt(void *fdt)

• Load an EFI payload into a newly allocated piece of memory, register all
• EFI objects it would want to access and jump to it.

*/ -static unsigned long do_bootefi_exec(void *efi) +static unsigned long do_bootefi_exec(void *efi, void *fdt) { ulong (*entry)(void *image_handle, struct efi_system_table *st); ulong fdt_pages, fdt_size, fdt_start, fdt_end; bootm_headers_t img = { 0 };

• void *fdt = working_fdt;

  /*

  • gd lives in a fixed register which may get clobbered while we execute

@@ -155,13 +154,7 @@ static unsigned long do_bootefi_exec(void *efi) */ efi_save_gd();

• /* Update system table to point to our currently loaded FDT */
• /* Fall back to included fdt if none was manually loaded */
• if (!fdt && gd->fdt_blob)

• fdt = (void *)gd->fdt_blob;
  

• if (fdt) {

• if (fdt && !fdt_check_header(fdt)) { /* Prepare fdt for payload */ fdt = copy_fdt(fdt);

@@ -185,7 +178,7 @@ static unsigned long do_bootefi_exec(void *efi) efi_add_memory_map(fdt_start, fdt_pages, EFI_BOOT_SERVICES_DATA, true); } else {

• printf("WARNING: No device tree loaded, expect boot to fail\n");
  

• printf("WARNING: Invalid device tree, expect boot to fail\n");
  

  systab.nr_tables = 0; }

@@ -216,18 +209,20 @@ static unsigned long do_bootefi_exec(void *efi) /* Interpreter command to boot an arbitrary EFI image from memory */ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) {

• char *saddr;
• unsigned long addr;

• char *saddr, *sfdt;
• unsigned long addr, fdt_addr; int r = 0;

• if (argc < 2)

• if (argc < 3) return 1;

Hm, I had specifically requested to make it an optional argument... do_bootefi_exec() seems to still handle !fdt, but here we seem to choke on absence of the third argument, not matching bootm/bootz/booti? GRUB does not need a DT to display its shell/menu, so we don't need to force it IMO. Think of people calling it from the prompt or from a script.

Or am I misunderstanding U-Boot argument handling?

I thought that Stephen's argument was that booting Linux without a working device tree is moot anyway. I tend to agree ;).

If you really want to boot without a device tree, you can always just pass in "-" as the fdt argument. But by requiring a device tree we hopefully get less confused people using the uefi support.

Alex