Understood, thanks! v2 sent. Best, Adriano
El mié, 19 mar 2025 a las 12:30, Ilias Apalodimas (< ilias.apalodi...@linaro.org>) escribió: > 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 >