[PATCH v4 2/5] bootm: Add a bootm command for type IH_OS_EFI

[Heinrich Schuchardt]

On 12/29/19 11:34 AM, Heinrich Schuchardt wrote:
> On 12/24/19 5:05 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.ciocaltea at gmail.com>
>> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>   cmd/Kconfig       |  7 ++++++
>>   common/bootm_os.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 63 insertions(+)
>>
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index 1e4cf146c5..87f2335a3c 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"
>
> Shouldn't this be "Support booting UEFI FIT images"?
>
>> +    depends on CMD_BOOTEFI
>
> depends on BOOTM

depends on CMD_BOOTM

The patch series compiles without CONFIG_FIT. But shouldn't this also be
a dependency?

If we place the definition directly after CMD_BOOTM, it will be indented
so that it is evident that this is a sub-feature of CMD_BOOTM.

So how about the following?

config CMD_BOOTM
         bool "bootm"
         default y
         help
           Boot an application image from the memory.

config BOOTM_EFI
         bool "Support booting UEFI FIT images"
         depends on CMD_BOOTEFI && CMD_BOOTM && FIT
         default y
         help
           Support booting UEFI FIT images via the bootm command.

Best regards

Heinrich

>
> is missing here.
>
>> +    default y
>> +    help
>> +      Support booting EFI images via the bootm command.
>
> Should we say:
>
> Support booting UEFI FIT images via the bootm command.
>
> Best regards
>
> Heinrich
>
>> +
>>   config BOOTM_LINUX
>>       bool "Support booting Linux OS images"
>>       depends on CMD_BOOTM || CMD_BOOTZ || CMD_BOOTI
>> diff --git a/common/bootm_os.c b/common/bootm_os.c
>> index d89ddc32b0..1d58462509 100644
>> --- a/common/bootm_os.c
>> +++ b/common/bootm_os.c
>> @@ -7,10 +7,12 @@
>>   #include <common.h>
>>   #include <bootm.h>
>>   #include <cpu_func.h>
>> +#include <efi_loader.h>
>>   #include <env.h>
>>   #include <fdt_support.h>
>>   #include <linux/libfdt.h>
>>   #include <malloc.h>
>> +#include <mapmem.h>
>>   #include <vxworks.h>
>>   #include <tee/optee.h>
>>
>> @@ -498,6 +500,57 @@ 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;
>> +    efi_status_t efi_ret;
>> +    void *image_buf;
>> +
>> +    if (flag != BOOTM_STATE_OS_GO)
>> +        return 0;
>> +
>> +    /* Locate FDT, if provided */
>> +    ret = bootm_find_images(flag, argc, argv);
>> +    if (ret)
>> +        return ret;
>> +
>> +    /* Initialize EFI drivers */
>> +    efi_ret = efi_init_obj_list();
>> +    if (efi_ret != EFI_SUCCESS) {
>> +        printf("## Failed to initialize UEFI sub-system: r = %lu\n",
>> +               efi_ret & ~EFI_ERROR_MASK);
>> +        return 1;
>> +    }
>> +
>> +    /* Install device tree */
>> +    efi_ret = efi_install_fdt(images->ft_len
>> +                  ? images->ft_addr : EFI_FDT_USE_INTERNAL);
>> +    if (efi_ret != EFI_SUCCESS) {
>> +        printf("## Failed to install device tree: r = %lu\n",
>> +               efi_ret & ~EFI_ERROR_MASK);
>> +        return 1;
>> +    }
>> +
>> +    /* Run EFI image */
>> +    printf("## Transferring control to EFI (at address %08lx) ...\n",
>> +           images->ep);
>> +    bootstage_mark(BOOTSTAGE_ID_RUN_OS);
>> +
>> +    image_buf = map_sysmem(images->ep, images->os.image_len);
>> +
>> +    efi_ret = efi_run_image(image_buf, images->os.image_len);
>> +    if (efi_ret != EFI_SUCCESS) {
>> +        printf("## Failed to run EFI image: r = %lu\n",
>> +               efi_ret & ~EFI_ERROR_MASK);
>> +        return 1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +#endif
>> +
>>   static boot_os_fn *boot_os[] = {
>>       [IH_OS_U_BOOT] = do_bootm_standalone,
>>   #ifdef CONFIG_BOOTM_LINUX
>> @@ -534,6 +587,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 */
>>
>
>