[PATCH v2 1/2] efi_loader: bootbin: do not load an initrd if none is provided

[Heinrich Schuchardt]

On 4/29/25 08:24, Ilias Apalodimas wrote:
> On Mon, 28 Apr 2025 at 17:53, Adriano Cordova <adrianox at gmail.com> wrote:
>>
>> Do not try to create an initrd device path nor try to register
>> an initrd with the EFI_LOAD_FILE2_PROTOCOL if none is provided.
>>
>> Handle initrd installation in efi_binary_run_dp with
>> efi_install_initrd, imitating what is done for the fdt.
>>
>> Fixes: 36835a9105c ("efi_loader: binary_run: register an initrd")
>> Reported-by: Weizhao Ouyang <o451686892 at gmail.com>
>> Signed-off-by: Adriano Cordova <adriano.cordova at canonical.com>
>> ---
>>   boot/bootm.c                 |  3 +++
>>   include/efi_loader.h         |  2 ++
>>   lib/efi_loader/efi_bootbin.c |  7 +------
>>   lib/efi_loader/efi_helper.c  | 29 +++++++++++++++++++++++++++++
>>   4 files changed, 35 insertions(+), 6 deletions(-)
>>
>> diff --git a/boot/bootm.c b/boot/bootm.c
>> index f5cbb10f0d1..9693e0d336d 100644
>> --- a/boot/bootm.c
>> +++ b/boot/bootm.c
>> @@ -1008,6 +1008,9 @@ int bootm_run_states(struct bootm_info *bmi, int states)
>>                          ret = 0;
>>          }
>>
>> +       /* Clear the initrd from a previous boot */
>> +       images->initrd_start = 0;

The bootm change should be in a different patch than the EFI change.

Bootm passes through multiple stages, see parameter states.

I don't believe that you want to zero images->initrd_start after the 
BOOTM_STATE_RAMDISK state.

Specically you don't want to zero initrd_start in the BOOTM_STATE_OS_GO 
state otherwise you will always boot without initrd.

Isn't initialization meant to happen for states == BOOTM_STATE_START in 
bootm_start()?

In test/py/tests/test_efi_fit.py we currently only test the case without 
initrd. We need a test with initrd too.

@Simon, @Tom,

We lack a documentation of the the different BOOTM_STATEs.
The documentation of the bootm command is incomplete, it does not 
contain the subcommands.

Best regards

Heinrich

>> +
>>          /* Relocate the ramdisk */
>>   #ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
>>          if (!ret && (states & BOOTM_STATE_RAMDISK)) {
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 144b749278a..84e8cfe320e 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -597,6 +597,8 @@ efi_status_t efi_env_set_load_options(efi_handle_t handle, const char *env_var,
>>   void *efi_get_configuration_table(const efi_guid_t *guid);
>>   /* Install device tree */
>>   efi_status_t efi_install_fdt(void *fdt);
>> +/* Install initrd */
>> +efi_status_t efi_install_initrd(void *initrd, size_t initd_sz);
>>   /* Execute loaded UEFI image */
>>   efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options);
>>   /* Run loaded UEFI image with given fdt */
>> diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c
>> index d0f7da309ce..6a189c31ffa 100644
>> --- a/lib/efi_loader/efi_bootbin.c
>> +++ b/lib/efi_loader/efi_bootbin.c
>> @@ -220,7 +220,6 @@ static efi_status_t efi_binary_run_dp(void *image, size_t size, void *fdt,
>>                                        struct efi_device_path *dp_img)
>>   {
>>          efi_status_t ret;
>> -       struct efi_device_path *dp_initrd;
>>
>>          /* Initialize EFI drivers */
>>          ret = efi_init_obj_list();
>> @@ -234,11 +233,7 @@ static efi_status_t efi_binary_run_dp(void *image, size_t size, void *fdt,
>>          if (ret != EFI_SUCCESS)
>>                  return ret;
>>
>> -       dp_initrd = efi_dp_from_mem(EFI_LOADER_DATA, (uintptr_t)initrd, initd_sz);
>> -       if (!dp_initrd)
>> -               return EFI_OUT_OF_RESOURCES;
>> -
>> -       ret = efi_initrd_register(dp_initrd);
>> +       ret = efi_install_initrd(initrd, initd_sz);
>>          if (ret != EFI_SUCCESS)
>>                  return ret;
>>
>> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
>> index 3936139ca41..19fb5d03fec 100644
>> --- a/lib/efi_loader/efi_helper.c
>> +++ b/lib/efi_loader/efi_helper.c
>> @@ -622,6 +622,35 @@ efi_status_t efi_install_fdt(void *fdt)
>>          return EFI_SUCCESS;
>>   }
>>
>> +/**
>> + * efi_install_initrd() - install initrd
>> + *
>> + * Install the initrd located at @initrd using the EFI_LOAD_FILE2
>> + * protocol.
>> + *
>> + * @initrd:    address of initrd or NULL if none is provided
>> + * @initrd_sz: size of initrd
>> + * Return:     status code
>> + */
>> +efi_status_t efi_install_initrd(void *initrd, size_t initd_sz)
>> +{
>> +       efi_status_t ret;
>> +       struct efi_device_path *dp_initrd;
>> +
>> +       if (!initrd)
>> +               return EFI_SUCCESS;
>> +
>> +       dp_initrd = efi_dp_from_mem(EFI_LOADER_DATA, (uintptr_t)initrd, initd_sz);
>> +       if (!dp_initrd)
>> +               return EFI_OUT_OF_RESOURCES;
>> +
>> +       ret = efi_initrd_register(dp_initrd);
>> +       if (ret != EFI_SUCCESS)
>> +               efi_free_pool(dp_initrd);
>> +
>> +       return ret;
>> +}
>> +
>>   /**
>>    * do_bootefi_exec() - execute EFI binary
>>    *
>> --
>> 2.48.1
>>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>