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

[Heinrich Schuchardt]

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;

Regards

Heinrich
> 
> 
> Alex
> 
> commit dede284d1ce9f9d9e79a5114fe7eb814fec07679
> Author: Andreas Färber <afaerber at suse.de>
> Date:   Wed Apr 13 14:04:38 2016 +0200
> 
>     efi_loader: Handle memory overflows
> 
>     jetson-tk1 has 2 GB of RAM at 0x80000000, causing gd->ram_top to be
> zero.
>     Handle this by either avoiding ram_top or by using the same type as
>     ram_top to reverse the overflow effect.
> 
>     Cc: Alexander Graf <agraf at suse.de>
>     Signed-off-by: Andreas Färber <afaerber at suse.de>
>     Reviewed-by: Alexander Graf <agraf at suse.de>
> 
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index df995858ed..71a3d19269 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 */
> 
> 
> 
>