[U-Boot] [PATCH 1/1] efi_loader: efi_allocate_pages is too restrictive

[Alexander Graf]

On 03/09/2018 05:35 PM, Heinrich Schuchardt wrote:
> On 03/09/2018 05:19 PM, Alexander Graf wrote:
>> On 03/09/2018 04:58 PM, Heinrich Schuchardt wrote:
>>> On 03/09/2018 01:48 PM, Alexander Graf wrote:
>>>> On 03/03/2018 03:48 PM, Heinrich Schuchardt wrote:
>>>>> When running on the sandbox the stack is not necessarily at a higher
>>>>> memory
>>>>> address than the highest free memory.
>>>>>
>>>>> There is no reason why the checking of the highest memory address
>>>>> should be
>>>>> more restrictive for EFI_ALLOCATE_ANY_PAGES than for
>>>>> EFI_ALLOCATE_MAX_ADDRESS.
>>>>>
>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>>> ---
>>>>>     lib/efi_loader/efi_memory.c | 2 +-
>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>>>> index cc271e0709d..0efbb973231 100644
>>>>> --- a/lib/efi_loader/efi_memory.c
>>>>> +++ b/lib/efi_loader/efi_memory.c
>>>>> @@ -294,7 +294,7 @@ efi_status_t efi_allocate_pages(int type, int
>>>>> memory_type,
>>>>>         switch (type) {
>>>>>         case EFI_ALLOCATE_ANY_PAGES:
>>>>>             /* Any page */
>>>>> -        addr = efi_find_free_memory(len, gd->start_addr_sp);
>>>>> +        addr = efi_find_free_memory(len, (uint64_t)-1);
>>>> This will break on systems that do not map high address space into the
>>>> linear map (IIRC nvidia systems had that issue).
>>>>
>>> The memory map is also passed on to the operating system when booting.
>>> If a memory reservation is missing for any board it has to be fixed in
>>> the board or driver files, cf.
>>>
>>> sunxi: video: mark framebuffer as EFI reserved memory
>>> https://lists.denx.de/pipermail/u-boot/2018-March/321820.htm
>>>
>>> For type = EFI_ALLOCATE_MAX_ADDRESS we don't care about
>>> gd->start_addr_sp. So if the memory map is incomplete the current code
>>> may fail. Keeping things as they are is not a viable option.
>>>
>>> Could you, please, identify for which Nvidia system a problem was
>>> reported? Then we can add a call to efi_add_memory_map() for the board.
>> Git blame points to this commit. I guess -1 should do the same thing
>> then, true.
>>
>> Andreas, would you see any reason -1 will not work?
> Are we talking about this line:
>
> arch/arm/mach-tegra/board2.c:317:
> gd->pci_ram_top = gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size;

pci_ram_top != ram_top, no? :)

I'd rather assume it's this one:

/*
  * Most hardware on 64-bit Tegra is still restricted to DMA to the lower
  * 32-bits of the physical address space. Cap the maximum usable RAM area
  * at 4 GiB to avoid DMA buffers from being allocated beyond the 32-bit
  * boundary that most devices can address. Also, don't let U-Boot use any
  * carve-out, as mentioned above.
  *
  * This function is called before dram_init_banksize(), so we can't simply
  * return gd->bd->bi_dram[1].start + gd->bd->bi_dram[1].size.
  */
ulong board_get_usable_ram_top(ulong total_size)
{
         return CONFIG_SYS_SDRAM_BASE + usable_ram_size_below_4g();
}

But the real problem is that ram_top of 0 is perfectly valid for 32bit 
systems.

Also ram_top may be used by platforms (like tegra again) to ensure that 
we don't use addresses >32bit for DMA. I guess the real solution to that 
would be to enable bounce buffers for tegra though. But we should make 
sure we don't regress tegra support, so we need to enable bounce buffers 
first for tegra and then allow the allocation to walk the full address 
space.


Alex