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 <ag...@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?

>       saddr = argv[1];
> +     sfdt = argv[2];
>  
>       addr = simple_strtoul(saddr, NULL, 16);
> +     fdt_addr = simple_strtoul(sfdt, NULL, 16);
>  
>       printf("## Starting EFI application at 0x%08lx ...\n", addr);
> -     r = do_bootefi_exec((void *)addr);
> +     r = do_bootefi_exec((void *)addr, (void*)fdt_addr);
>       printf("## Application terminated, r = %d\n", r);
>  
>       if (r != 0)
> @@ -238,16 +233,13 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int 
> argc, char * const argv[])
>  
>  #ifdef CONFIG_SYS_LONGHELP
>  static char bootefi_help_text[] =
> -     "<image address>\n"
> -     "  - boot EFI payload stored at address <image address>\n"
> -     "\n"
> -     "Since most EFI payloads want to have a device tree provided, please\n"
> -     "make sure you load a device tree using the fdt addr command before\n"
> -     "executing bootefi.\n";
> +     "<image address> <fdt address>\n"
> +     "  - boot EFI payload stored at address <image address> with a device\n"
> +     "    tree located at <fdt address>\n";

Nit: This could get a better explanation than "with a ...". :) U-Boot
exposes it as some table/hook/callback/etc.?

>  #endif
>  
>  U_BOOT_CMD(
> -     bootefi, 2, 0, do_bootefi,
> +     bootefi, 3, 0, do_bootefi,
>       "Boots an EFI payload from memory\n",
>       bootefi_help_text
>  );
> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
> index ad9045e..67eb8f2 100644
> --- a/include/config_distro_bootcmd.h
> +++ b/include/config_distro_bootcmd.h
> @@ -103,12 +103,15 @@
>       "boot_efi_binary="                                                \
>               "load ${devtype} ${devnum}:${distro_bootpart} "           \
>                       "${kernel_addr_r} efi/boot/"BOOTEFI_NAME"; "      \
> -             "bootefi ${kernel_addr_r}\0"                              \
> +             "if fdt addr ${fdt_addr_r}; then "                        \
> +               "bootefi ${kernel_addr_r} ${fdt_addr_r};"               \
> +                "else"                                                    \

Spaces vs. tabs?

> +               "bootefi ${kernel_addr_r} ${fdtcontroladdr};"           \
> +                "fi\0"                                                    \
>       \
>       "load_efi_dtb="                                                   \
>               "load ${devtype} ${devnum}:${distro_bootpart} "           \
> -                     "${fdt_addr_r} ${prefix}${fdtfile}; "             \
> -             "fdt addr ${fdt_addr_r}\0"                                \
> +                     "${fdt_addr_r} ${prefix}${fdtfile}\0"             \
>       \
>       "efi_dtb_prefixes=/ /dtb/ /dtb/current/\0"                        \
>       "scan_dev_for_efi="                                               \

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to