On 8/29/19 7:04 AM, Bin Meng wrote:
Hi Aiden,

On Thu, Aug 29, 2019 at 12:02 PM Park, Aiden <aiden.p...@intel.com> wrote:

Hi Bin,

-----Original Message-----
From: Bin Meng [mailto:bmeng...@gmail.com]
Sent: Wednesday, August 28, 2019 8:37 PM
To: Park, Aiden <aiden.p...@intel.com>; Heinrich Schuchardt
<xypron.g...@gmx.de>
Cc: Simon Glass <s...@chromium.org>; u-boot@lists.denx.de
Subject: Re: [PATCH 1/1] x86: efi_loader: Fix invalid address return from
efi_alloc()

+Heinrich,

On Wed, Aug 28, 2019 at 2:35 AM Park, Aiden <aiden.p...@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.p...@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?

The efi_add_known_memory() in lib/efi_loader/efi_memory.c covers this case.

+               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

I have replicated this issue with qemu-x86_defconfig as below.

diff --git a/arch/x86/cpu/qemu/e820.c b/arch/x86/cpu/qemu/e820.c
index e682486547..7e5ae38c07 100644
--- a/arch/x86/cpu/qemu/e820.c
+++ b/arch/x86/cpu/qemu/e820.c
@@ -42,5 +42,9 @@ unsigned int install_e820_map(unsigned int max_entries,
         entries[5].size = CONFIG_PCIE_ECAM_SIZE;
         entries[5].type = E820_RESERVED;

-       return 6;
+       entries[6].addr = 0x100000000ULL;
+       entries[6].size = 0x100000000ULL;
+       entries[6].type = E820_RAM;
+
+       return 7;
  }
diff --git a/configs/qemu-x86_defconfig b/configs/qemu-x86_defconfig
index e71b8a0ee1..2998d18bdd 100644
--- a/configs/qemu-x86_defconfig
+++ b/configs/qemu-x86_defconfig
@@ -41,3 +41,4 @@ CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
  CONFIG_FRAMEBUFFER_VESA_MODE_USER=y
  CONFIG_FRAMEBUFFER_VESA_MODE=0x144
  CONFIG_CONSOLE_SCROLL_LINES=5
+CONFIG_CMD_BOOTEFI_HELLO=y

$ qemu-system-i386 -nographic -bios u-boot.rom -m 8192
=> bootefi hello

OK, thanks for the test case. However I believe this never broke QEMU x86.

As in arch/x86/cpu/qemu/dram.c::dram_init():

gd->ram_size will be always set to 3GiB when "-m 4G" or more memory is
specified for QEMU target, hence gd->ram_top is always set to 3GiB.

Why would we have such an artificial limit? An LPAE kernel should work
fine with 8GB.

Is this a U-Boot or a QEMU issue?

Best regards

Heinrich


So it never happens in QEMU.

I think you encountered an issue on real hardware. Shouldn't we fix
gd->ram_top instead?

Regards,
Bn


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to