On 8/29/19 5:36 AM, Bin Meng wrote:
+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?
If in 32bit mode RAM addresses up to 2^32-1 are available, gd->ram_top
is 0 due to overflow.
+ ram_top = 0x100000000ULL;
In this special case we correct the value to 4GB.
+
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?
These lines are verbatim copies of what we have in
lib/efi_loader/efi_memory.c. Function wise this is ok.
But this creates code duplication. Most of what is in this loop and in
the loop in lib/efi_loader/efi_memory.c (starting at /* Remove partial
pages */) could be put into a separate common function.
Aiden, do you want to give a try?
Best regards
Heinrich
+ } 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
