bootefi issue

[Heinrich Schuchardt]

On 2/23/23 17:24, Alexandre Ghiti wrote:
> On Thu, Feb 23, 2023 at 4:57 PM Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> wrote:
>>
>> On 2/23/23 14:38, Alexandre Ghiti wrote:
>>> Hi Heinrich,
>>>
>>> I fell into an issue in u-boot, and I struggle to find the correct
>>> fix: I'm loading a kernel of 66MB at kernel_addr_r (=0x8400_0000, this
>>> is the default value) and then I'm booting it using bootefi. But the
>>> loaded kernel is overwritten by the fdt because u-boot loads the dtb
>>> at 0x87f0_0000 (https://elixir.bootlin.com/u-boot/v2023.01/source/cmd/bootefi.c#L211:
>>> I guess efi_allocate_pages does not fail because the efi allocator is
>>> not aware of the kernel that was loaded there). So I have multiple
>>> fixes and don't know which one is right:
>>>
>>> 1. bootefi could load the kernel first and then the fdt (so that the
>>> efi allocator fails at 0x87f0_0000)
>>> 2. check before efi_allocate_pages that the "normal" allocator is free
>>> there (I tried malloc_usable_size but it fails).
>>> 3. decrease kernel_addr_r...But I would disagree :)
>>>
>>> Any idea?
>>>
>>> Thanks,
>>>
>>> Alex
>>
>> Hello Alexandre,
>>
>> this address for copying the device-tree dates back to 2016-04-11 when
>> AllocatePages() didn't work properly.
>>
>> For riscv64 I see no reason why we should not use a high address.
>>
>> @Ilias
>> Does ARM have any problem with the device-tree being in high memory?
>> Otherwise we should make a change as in the diff below.
>>
>> Best regards
>>
>> Heinrich
>>
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 6618335ddf..08a0cfa764 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -210,19 +210,12 @@ static efi_status_t copy_fdt(void **fdtp)
>>            */
>>           new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f00000 +
>>                                                fdt_size, 0);
> 
> I guess we can entirely remove this ^ right?
> 
>> -       ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
>> +       ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
>>                                    EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
>>                                    &new_fdt_addr);
>>           if (ret != EFI_SUCCESS) {
>> -               /* If we can't put it there, put it somewhere */
>> -               new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
>> -               ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
>> -                                        EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
>> -                                        &new_fdt_addr);
>> -               if (ret != EFI_SUCCESS) {
>> -                       log_err("ERROR: Failed to reserve space for FDT\n");
>> -                       goto done;
>> -               }
>> +               log_err("ERROR: Failed to reserve space for FDT\n");
>> +               goto done;
>>           }
>>           new_fdt = (void *)(uintptr_t)new_fdt_addr;
>>           memcpy(new_fdt, fdt, fdt_totalsize(fdt));
>>
> 
> What would prevent the efi allocator to return an address that
> overwrites the loaded kernel?

Our AllocatePages() assigns high memory when called with 
EFI_ALLOCATE_ANY_PAGES.

Currently we have a gap in U-Boot. Memory is not allocated by the 'load' 
command. We need to merge the EFI memory allocator and lib/lmb.c.

A quick fix has been commit 06d514d77c37 ("lmb: consider EFI memory 
map") but that does not cover everything.

Best regards

Heinrich