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?
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.
>> 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).
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...
>> efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA, false);
>>
>> /* Add Runtime Services */
>> --
>> 2.6.6
Thanks,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
