[PATCH] Revert "efi_memory: do not add U-Boot memory to the memory map"

[Heinrich Schuchardt]

On 11/13/24 12:28, Ilias Apalodimas wrote:
> Hi Sughosh
>
> On Tue, 12 Nov 2024 at 20:46, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>>
>> On Tue, 12 Nov 2024 at 18:48, Simon Glass <sjg at chromium.org> wrote:
>>>
>>> A bisect of Ubuntu 2022.04 boot-failure on qemu-x86_64 resulted in this
>>> patch. I am not sure how to investigate it.
>>>
>>> The boot hangs at some point during booting of the install image, before
>>> the Ubuntu logo appears.

I tried to replicate the problem with an Ubuntu 24.04.1 cloud image
using a 6.8 kernel.

I see efi_exit_boot_services being reached but not
efi_set_virtual_address_map. The kernel seems to be stuck in an endless
loop in a virtual address range (0xffffffff????????).

The memory map looks wrong:

=> efidebug memmap
Type             Start            End              Attributes
================ ================ ================ ==========
CONVENTIONAL     0000000000000000-00000000000a0000 WB
RESERVED         00000000000a0000-00000000000f0000 WB
ACPI RECLAIM MEM 00000000000f0000-00000000000f1000 WB
RESERVED         00000000000f1000-0000000000100000 WB
CONVENTIONAL     0000000000100000-000000003dca4000 WB
BOOT DATA        000000003dca4000-000000003dca6000 WB
RUNTIME DATA     000000003dca6000-000000003dca7000 WB|RT
BOOT DATA        000000003dca7000-000000003dca8000 WB
RUNTIME DATA     000000003dca8000-000000003dcca000 WB|RT
BOOT DATA        000000003dcca000-000000003ecda000 WB
RESERVED         000000003ecda000-000000003ece0000 WB
ACPI RECLAIM MEM 000000003ece0000-000000003ed02000 WB
RUNTIME DATA     000000003ed02000-000000003ed03000 WB|RT
RESERVED         000000003ed03000-000000003ef1a000 WB
RUNTIME CODE     000000003ef1a000-000000003ef1c000 WB|RT
RESERVED         000000003ef1c000-0000000040000000 WB
RESERVED         00000000e0000000-00000000f0000000 WB

The relocated U-Boot code should not be marked as EfiReservedMemoryType
but as EfiServicesCode according to the UEFI specification.

The ACPI table is marked as EfiACPIReclaimMemory but I don't think
memory in the BIOS memory range should be reclaimable. In the journal on
my laptop I see

BIOS-provided physical RAM map
BIOS-e820: [mem 0x00000000000e0000-0x00000000000fffff] reserved

The e820 table is generated from the EFI memory map in setup_e820() of
the Linux efi stub.

Best regards

Heinrich

>>>
>>> I will sent a series with a script showing how it is run.
>>>
>>> This reverts commit a68c9ac5d8afc51c619c5d3e865fcf147bea90cb.
>>>
>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>> ---
>>
>> This seems to be an issue specific to the x86 architecture. The
>> installation works on arm64 and riscv (confirmed by Heinrich)
>> architectures. I checked the output of the EFI memory map with the
>> above commit against the master branch, and the only difference is
>> that with this revert, the EFI memory map has the u-boot memory region
>> marked as EFI_BOOT_SERVICES_CODE, whereas without this commit, that
>> region is marked as reserved.
>
> Hmm why? LMB is adding this with EFI_BOOT_SERVICES_CODE  in
> lmb_map_update_notify(). Who's switching it to reserved?
> This is a pretty big change since the memory switches from reusable to
> reserved for the OS.
>
>> I need to look into this in more detail,
>> but it would seem like the x86 kernel (or some efi stub code ?) is
>> expecting some region of memory marked as EFI_BOOT_SERVICES_CODE,
>> which is not the case for other architectures.
>
> Not EFI_BOOT_SERVICES_CODE explicitly, probably just a reusable type of memory.
>
> Cheers
> /Ilias
>
>
>
>>
>> -sughosh
>>
>>>
>>>   lib/efi_loader/efi_memory.c | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>> index d2f5d563f2a..c7400ec9854 100644
>>> --- a/lib/efi_loader/efi_memory.c
>>> +++ b/lib/efi_loader/efi_memory.c
>>> @@ -809,6 +809,16 @@ static void add_u_boot_and_runtime(void)
>>>   {
>>>          unsigned long runtime_start, runtime_end, runtime_pages;
>>>          unsigned long runtime_mask = EFI_PAGE_MASK;
>>> +       unsigned long uboot_start, uboot_pages;
>>> +       unsigned long uboot_stack_size = CONFIG_STACK_SIZE;
>>> +
>>> +       /* Add U-Boot */
>>> +       uboot_start = ((uintptr_t)map_sysmem(gd->start_addr_sp, 0) -
>>> +                      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_BOOT_SERVICES_CODE,
>>> +                             false);
>>>
>>>   #if defined(__aarch64__)
>>>          /*
>>> --
>>> 2.34.1
>>>