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

[Adriano Córdova]

Understood, thanks! v2 sent.

Best,
Adriano

El mié, 19 mar 2025 a las 12:30, Ilias Apalodimas (<
ilias.apalodimas at linaro.org>) escribió:

> 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
>