Hi Cristian, On Sun, 24 Nov 2019 at 23:22, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > On 11/24/19 9:11 PM, Cristian Ciocaltea wrote: > > Add support for booting EFI binaries contained in FIT images. > > A typical usage scenario is chain-loading GRUB2 in a verified > > boot environment. > > > > Signed-off-by: Cristian Ciocaltea <cristian.ciocal...@gmail.com> > > --- > > cmd/Kconfig | 9 ++++++++- > > cmd/bootefi.c | 2 +- > > common/bootm_os.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > > include/bootm.h | 2 ++ > > 4 files changed, 55 insertions(+), 2 deletions(-) > > > > diff --git a/cmd/Kconfig b/cmd/Kconfig > > index cf982ff65e..1bec840f5a 100644 > > --- a/cmd/Kconfig > > +++ b/cmd/Kconfig > > @@ -263,6 +263,13 @@ config CMD_BOOTI > > help > > Boot an AArch64 Linux Kernel image from memory. > > > > +config BOOTM_EFI > > + bool "Support booting EFI OS images" > > + depends on CMD_BOOTEFI > > + default y > > + help > > + Support booting EFI images via the bootm command. > > + > > config BOOTM_LINUX > > bool "Support booting Linux OS images" > > depends on CMD_BOOTM || CMD_BOOTZ || CMD_BOOTI > > @@ -316,7 +323,7 @@ config CMD_BOOTEFI > > depends on EFI_LOADER > > default y > > help > > - Boot an EFI image from memory. > > + Boot an EFI binary from memory. > > > > config CMD_BOOTEFI_HELLO_COMPILE > > bool "Compile a standard EFI hello world binary for testing" > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > > index f613cce7e2..f25d639dfe 100644 > > --- a/cmd/bootefi.c > > +++ b/cmd/bootefi.c > > @@ -553,7 +553,7 @@ static int do_efi_selftest(void) > > * @argv: command line arguments > > * Return: status code > > */ > > -static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const > > argv[]) > > +int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > > { > > efi_status_t ret; > > > > diff --git a/common/bootm_os.c b/common/bootm_os.c > > index 6fb7d658da..706151913a 100644 > > --- a/common/bootm_os.c > > +++ b/common/bootm_os.c > > @@ -462,6 +462,47 @@ static int do_bootm_tee(int flag, int argc, char * > > const argv[], > > } > > #endif > > > > +#ifdef CONFIG_BOOTM_EFI > > +static int do_bootm_efi(int flag, int argc, char * const argv[], > > + bootm_headers_t *images) > > +{ > > + int ret; > > + int local_argc = 2; > > + char *local_args[3]; > > + char str_efi_addr[16]; > > + char str_fdt_addr[16]; > > + > > + if (flag != BOOTM_STATE_OS_GO) > > + return 0; > > + > > + /* Locate FDT etc */ > > + ret = bootm_find_images(flag, argc, argv); > > + if (ret) > > + return ret; > > + > > + printf("## Transferring control to EFI (at address %08lx) ...\n", > > + images->ep); > > + bootstage_mark(BOOTSTAGE_ID_RUN_OS); > > + > > + local_args[0] = argv[0]; > > + > > + /* Write efi addr into string */ > > + sprintf(str_efi_addr, "%lx", images->ep); > > I think we should avoid transferring arguments as strings. It is > preferable to separate do_bootefi() into an argument parser and an > execution part. > > Let's CC the developers who have contributed to common/bootm_os.c.
Yes I think it would be good to create a function to hold most of the code from do_bootefi(), call the function with appropriate parameters and changing do_bootefi() to call that new function. > > Best regards > > Heinrich > > > + /* and provide it via the arguments */ > > + local_args[1] = str_efi_addr; > > + > > + if (images->ft_len) { > > + /* Write fdt addr into string */ > > + sprintf(str_fdt_addr, "%lx", (unsigned long)images->ft_addr); > > + /* and provide it via the arguments */ > > + local_args[2] = str_fdt_addr; > > + local_argc = 3; > > + } > > + > > + return do_bootefi(NULL, 0, local_argc, local_args); > > +} > > +#endif > > + > > static boot_os_fn *boot_os[] = { > > [IH_OS_U_BOOT] = do_bootm_standalone, > > #ifdef CONFIG_BOOTM_LINUX > > @@ -498,6 +539,9 @@ static boot_os_fn *boot_os[] = { > > #ifdef CONFIG_BOOTM_OPTEE > > [IH_OS_TEE] = do_bootm_tee, > > #endif > > +#ifdef CONFIG_BOOTM_EFI > > + [IH_OS_EFI] = do_bootm_efi, > > +#endif > > }; > > > > /* Allow for arch specific config before we boot */ > > diff --git a/include/bootm.h b/include/bootm.h > > index edeeacb0df..a0da86dc32 100644 > > --- a/include/bootm.h > > +++ b/include/bootm.h > > @@ -37,7 +37,9 @@ typedef int boot_os_fn(int flag, int argc, char * const > > argv[], > > extern boot_os_fn do_bootm_linux; > > extern boot_os_fn do_bootm_vxworks; > > > > +int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); Please add a function comment. > > int do_bootelf(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); > > + > > void lynxkdi_boot(image_header_t *hdr); > > > > boot_os_fn *bootm_os_get_boot_func(int os); > > > Regards, Simon