[U-Boot] [PATCH 1/1] x86: efi_loader: Fix invalid address return from efi_alloc()

[Bin Meng]

+Heinrich,

On Wed, Aug 28, 2019 at 2:35 AM Park, Aiden <aiden.park at intel.com> wrote:
>
> This issue can be seen on 32bit operation when one of E820_RAM type
> entries is greater than 4GB memory space.
>
> The efi_alloc() finds a free memory in the conventional memory which
> is greater than 4GB. But, it does type cast to 32bit address space
> and eventually returns invalid address.
>
> Signed-off-by: Aiden Park <aiden.park at intel.com>
> ---
>  arch/x86/lib/e820.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/lib/e820.c b/arch/x86/lib/e820.c
> index d6ae2c4e9d..3e93931231 100644
> --- a/arch/x86/lib/e820.c
> +++ b/arch/x86/lib/e820.c
> @@ -41,11 +41,15 @@ void efi_add_known_memory(void)
>  {
>         struct e820_entry e820[E820MAX];
>         unsigned int i, num;
> -       u64 start, pages;
> +       u64 start, pages, ram_top;
>         int type;
>
>         num = install_e820_map(ARRAY_SIZE(e820), e820);
>
> +       ram_top = (u64)gd->ram_top & ~EFI_PAGE_MASK;
> +       if (!ram_top)

So for the logic here to work, gd->ram_top is already zero in 32-bit,
right? I was wondering how U-Boot could boot on such target?

> +               ram_top = 0x100000000ULL;
> +
>         for (i = 0; i < num; ++i) {
>                 start = e820[i].addr;
>                 pages = ALIGN(e820[i].size, EFI_PAGE_SIZE) >> EFI_PAGE_SHIFT;
> @@ -70,6 +74,22 @@ void efi_add_known_memory(void)
>                 }
>
>                 efi_add_memory_map(start, pages, type, false);
> +
> +               if (type == EFI_CONVENTIONAL_MEMORY) {
> +                       u64 end = start + (pages << EFI_PAGE_SHIFT);
> +
> +                       /* reserve the memory region greater than ram_top */
> +                       if (ram_top < start) {
> +                               efi_add_memory_map(start, pages,
> +                                                  EFI_BOOT_SERVICES_DATA,
> +                                                  true);

Heinrich, could you please review the changes here?

> +                       } else if (start < ram_top && ram_top < end) {
> +                               pages = (end - ram_top) >> EFI_PAGE_SHIFT;
> +                               efi_add_memory_map(ram_top, pages,
> +                                                  EFI_BOOT_SERVICES_DATA,
> +                                                  true);
> +                       }
> +               }
>         }
>  }
>  #endif /* CONFIG_IS_ENABLED(EFI_LOADER) */
> --

Regards,
Bin