On Tue, Nov 29, 2022 at 06:35:40PM +0100, Heinrich Schuchardt wrote: > On 11/29/22 16:38, Ilias Apalodimas wrote: > > Hi Heinrich, > > > > On Tue, 29 Nov 2022 at 17:04, Heinrich Schuchardt > > <heinrich.schucha...@canonical.com> wrote: > > > > > > Memory allocated by U-Boot for internal usage should be > > > EFI_BOOT_SERVICES_DATA or _CODE or EFI_RUNTIME_SERVICES_DATA or _CODE. > > > > Agreed, EFI_LOADER_DATA should be for EFI apps. > > > > > > > > Reported-by: François-Frédéric Ozog <f...@ozog.com> > > > Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com> > > > > [...] > > > > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > > > index a17b426d11..0c336f98d2 100644 > > > --- a/lib/efi_loader/efi_memory.c > > > +++ b/lib/efi_loader/efi_memory.c > > > @@ -823,7 +823,7 @@ static void add_u_boot_and_runtime(void) > > > uboot_stack_size) & ~EFI_PAGE_MASK; > > > uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) - > > > uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; > > > - efi_add_memory_map_pg(uboot_start, uboot_pages, EFI_LOADER_DATA, > > > + efi_add_memory_map_pg(uboot_start, uboot_pages, > > > EFI_BOOT_SERVICES_DATA, > > > false); > > > > I am not sure if we should have this as _DATA or _CODE. None of these > > is an exact match of what we allocate here and both of these are > > backed by EFI_MEMORY_WB. So your reasoning here is prefer _DATA since > > it's not memory that holds boottime service drivers? > > We are lacking a clear separation of data and code here. We would have to > add another pointer global data and enforce that data is in separate pages > if we wanted to do so. > > The same problem exists when loading applications as some sections are data > and others are code but we put all into EFI_LOADER_CODE. > > Please, tell if you would prefer EFI_BOOT_SERVICES_CODE here.
I think I prefer _CODE, but I don't really mind tbh With or without the changes. In case you update the patch can you add a sentence along the lines of "EFI_LOADER_DATA/CODE is reserved for EFI applications" Reviewed-by: Ilias Apalodimas <ilias.apalodi...@linaro.org> > > Best regards > > Heinrich