[PATCH 2/3] efi_loader: binary_run: register an initrd

[Ilias Apalodimas]

On Wed, 19 Mar 2025 at 15:02, Adriano Córdova <adrianox at gmail.com> wrote:
>
> Hi Ilias,
>
> Ok, I will change that in a v2.

Ok i'll have a look at that

> As a question: in here it is a U-Boot routine the one that is doing the memcpy of the initrd, the one implemented in the load_file2 protocol, the OS never knows about this dp explicitly, could we technically use reserved data in here then?

Technically yes, but what are you trying to achieve with the reserved memory?
The loadfile2 design from the kernel does not include a device path to
the initrd. The basically kernel tells the firmware "Can you give me
my initrd now in this buffer which  I allocated" and the firmware is
supposed to know what to copy in that buffer.

Cheers
/Ilias
>
> Il mer 19 mar 2025, 09:37 Ilias Apalodimas <ilias.apalodimas at linaro.org> ha scritto:
>>
>> On Wed, 19 Mar 2025 at 14:07, Adriano Córdova <adrianox at gmail.com> wrote:
>> >
>> > Hi Ilias,
>> >
>> > El mié, 19 mar 2025 a las 6:32, Ilias Apalodimas (<ilias.apalodimas at linaro.org>) escribió:
>> >>
>> >> Hi Adriano,
>> >>
>> >> [...]
>> >>
>> >> >   */
>> >> >  static efi_status_t efi_binary_run_dp(void *image, size_t size, void *fdt,
>> >> > +                                     void *initrd, size_t initd_sz,
>> >> >                                       struct efi_device_path *dp_dev,
>> >> >                                       struct efi_device_path *dp_img)
>> >> >  {
>> >> >         efi_status_t ret;
>> >> > +       struct efi_device_path *dp_initrd;
>> >> >
>> >> >         /* Initialize EFI drivers */
>> >> >         ret = efi_init_obj_list();
>> >> > @@ -230,6 +234,14 @@ 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_RESERVED_MEMORY_TYPE, (uintptr_t)initrd, initd_sz);
>> >>
>> >> Why do we need reserved memory? What the EFI stub does, is allocate a
>> >> new memory buffer which you later copy the initrd to.
>> >> I think Boot Services Data is enough.
>>
>> I meant to write loader data*
>>
>> >
>> >
>> > dp_initrd is freed when the efi application returns in efi_initrd_deregister. Do you suggest to change it to
>> > Boot Sevices Data and when the efi application returns just assume that it is freed?
>>
>> You will still have to free it on failures which is what
>> efi_initrd_deregister() will do once we return from efi boot manager.
>> But if you define it as reserved memory, it won't be usable by the OS.
>> Since linux will eventually copy that memory in a buffer it allocated,
>> you don't have to preserve it. So you can just define it as
>> EFI_LOADER_DATA
>>
>> >  I took the usage from:
>> > https://github.com/u-boot/u-boot/blob/8bc3542384e3a1219e5ffb62b79d16dddc1b1fb9/lib/efi_loader/efi_bootmgr.c#L514
>>
>> This is a different use-case. In this we download and run an EFI
>> application from memory and I guess the assumption is that the EFI
>> application will return so the type doesn't matter that much as it
>> will eventually be freed. This obviously is not the case if the EFI
>> application is a kernel.
>> That being said, I think we can switch this to LOADER_CODE as well.
>> Heinrich any ideas?
>>
>> Thanks
>> /Ilias
>>
>> >
>> >>
>> >>
>> >> [...]
>> >>
>> >> Thanks
>> >> /Ilias