bootefi issue

[Alexandre Ghiti]

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?