On Wed, 19 Mar 2025 at 15:02, Adriano Córdova <adria...@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.apalodi...@linaro.org> ha 
> scritto:
>>
>> On Wed, 19 Mar 2025 at 14:07, Adriano Córdova <adria...@gmail.com> wrote:
>> >
>> > Hi Ilias,
>> >
>> > El mié, 19 mar 2025 a las 6:32, Ilias Apalodimas 
>> > (<ilias.apalodi...@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

Reply via email to