On 04/13/2016 01:26 PM, Andreas Färber wrote:
Am 13.04.2016 um 07:50 schrieb Alexander Graf:
Am 13.04.2016 um 05:24 schrieb Andreas Färber <afaer...@suse.de>:
jetson-tk1 has 2 GB of RAM at 0x80000000, causing gd->ram_top to be zero.
Handle this by replacing it with 0x100000000 in that case.
Nice catch!
Cc: Alexander Graf <ag...@suse.de>
Signed-off-by: Andreas Färber <afaer...@suse.de>
---
lib/efi_loader/efi_memory.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index 138736f..7b87108 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -225,7 +225,7 @@ efi_status_t efi_allocate_pages(int type, int memory_type,
switch (type) {
case 0:
/* Any page */
- addr = efi_find_free_memory(len, gd->ram_top);
+ addr = efi_find_free_memory(len, gd->ram_top == 0 ? 0x100000000ull :
gd->ram_top);
If we do use a constant, we should probably be using
UINT64_C(0x100000000) rather than ull for compatibility.
Couldn't we just use gd->ram_top - 1? Then we underflow to 0xffffffff and
everything should just work.
Hm, that does work in my testing. Is it guaranteed it will handle that
as unsigned long rather than uint64_t? And did you mean to always use it
that way or just in the zero case? I.e., might we be wasting one byte in
the non-zero case or is it guaranteed that the top of RAM is always
reserved?
Top of RAM is where U-Boot lives, so it's always reserved anyways :).
I was wondering whether we might run into the same overflow problem on
aarch64, in which case my hunk would be wrong, but your -1 should work.
I'm not aware of an aarch64 implementation (or even spec) that would
allow for 64bit physical address space, so we're safe on aarch64. But I
like the -1 better, since it just completely negates any overflow.
if (!addr) {
r = EFI_NOT_FOUND;
break;
@@ -343,7 +343,7 @@ int efi_memory_init(void)
/* Add U-Boot */
uboot_start = (gd->start_addr_sp - uboot_stack_size) & ~EFI_PAGE_MASK;
- uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
+ uboot_pages = ((gd->ram_top == 0 ? 0x100000000ull : gd->ram_top) - uboot_start)
>> EFI_PAGE_SHIFT;
Are you sure this hunk is necessary? We should already underflow to the correct
value here.
Positive that _something_ here is necessary for sanity, it was using a
huge number of pages (uboot_start is uint64_t).
Ok, so the fix would be to change uboot_start to unsigned long to have
the same type as gd->ram_top.
If we use an odd number like -1, then we probably should add
EFI_PAGE_MASK as done for the RAM for the non-zero case, shouldn't we?
That might overflow on aarch64 though...
Since we shift afterwards we don't need to mask anything. Bits that we
would mask away are already gone after the shift ;).
Does the patch below work?
Alex
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index df99585..71a3d19 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -220,7 +220,7 @@ efi_status_t efi_allocate_pages(int type, int
memory_type,
switch (type) {
case 0:
/* Any page */
- addr = efi_find_free_memory(len, gd->ram_top);
+ addr = efi_find_free_memory(len, gd->start_addr_sp);
if (!addr) {
r = EFI_NOT_FOUND;
break;
@@ -320,9 +320,9 @@ efi_status_t efi_get_memory_map(unsigned long
*memory_map_size,
int efi_memory_init(void)
{
- uint64_t runtime_start, runtime_end, runtime_pages;
- uint64_t uboot_start, uboot_pages;
- uint64_t uboot_stack_size = 16 * 1024 * 1024;
+ unsigned long runtime_start, runtime_end, runtime_pages;
+ unsigned long uboot_start, uboot_pages;
+ unsigned long uboot_stack_size = 16 * 1024 * 1024;
int i;
/* Add RAM */
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
