[PATCH 1/1] efi_loader: don't use EFI_LOADER_DATA internally

[Heinrich Schuchardt]

On 11/29/22 16:38, Ilias Apalodimas wrote:
> Hi Heinrich,
> 
> On Tue, 29 Nov 2022 at 17:04, Heinrich Schuchardt
> <heinrich.schuchardt at 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 <ff at ozog.com>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at 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.

Best regards

Heinrich