[U-Boot] [PATCH] efi_loader: don't allocate unusable RAM

[Heinrich Schuchardt]

On 08/02/2018 07:00 PM, Stephen Warren wrote:
> On 08/01/2018 11:57 PM, Heinrich Schuchardt wrote:
>> On 08/01/2018 06:17 PM, Stephen Warren wrote:
>>> On 08/01/2018 12:07 AM, Heinrich Schuchardt wrote:
>>>> On 07/31/2018 09:44 PM, Stephen Warren wrote:
>>>>> From: Stephen Warren <swarren at nvidia.com>
>>>>>
>>>>> Some boards define a maximum usable RAM top that's more restrictive
>>>>> than
>>>>> the ranges defined by U-Boot's memory bank definitions[1]. In this
>>>>> case,
>>>>> the unusable RAM isn't mapped in the page tables, and so the EFI code
>>>>> must
>>>>> not attempt to allocate RAM from outside the usable regions. Fix
>>>>> efi_find_free_memory() to detect when max_addr is unconstrained or
>>>>> out of
>>>>> range, and substitue a valid value.
>>>>>
>>>>
>>>> In this case the board has to call efi_add_memory_map() to mark the
>>>> reserved region (cf. board/raspberrypi/rpi/rpi.c) or have to mark the
>>>> memory region as reserved in the device tree (see
>>>> efi_carve_out_dt_rsv()).
>>>>
>>>> Please, check if the tegra boards with which you had problems do so.
>>>
>>> I don't think this makes sense; memory should be managed the same way
>>> within U-Boot no matter which part of U-Boot is consuming that memory.
>>> Memory regions are currently represented by the content of the memory
>>> bank definitions and gd->ram_top. Having different parts of the code
>>> define legal RAM usage in different ways is horribly inconsistent.
>>
>> Memory banks and gd->top together cannot reflect unusable memory regions
>> in the middle of the RAM area.
> 
> Sure this is possible. For example, a common set of memory banks for
> Tegra is:
> 
> bank 0:
> start:  0x080000000
> length: 0x070000000
> limit:  0x0f0000000
> bank 1:
> start:  0x100000000
> length: 0x080000000
> limit:  0x180000000
> 
> gd->ram_top = 0x0f0000000
> That is set so that U-Boot itself (not just the UEFI code) doesn't use
> RAM above 4GB due to HW peripherals that can't access such RAM. The RAM
> above 4GB is left in the DRAM banks so that the kernel can use it for
> non-DMA purposes

Hello Stephen,

this explanation makes the problem much clearer. Memory usable by the
operating system but not by the firmware cannot be expressed in the
device tree.

If bank 1 cannot be used by U-Boot but can be used by the operating
system we should mark it as EFI_BOOT_SERVICES_DATA by calling
efi_add_memory_map() in arch/arm/mach-tegra/board2.c. This will indicate
to the operating system that it may use this memory after exiting boot
services and the U-Boot EFI subsystem will not use this region. This
call could be done in dram_init_banksize().

Could you give this a try, please.

Best regards

Heinrich


> 
> That represents a hole at 0xf0000000..0x100000000.
> 
>> To be consistent reflect all information in the device tree and
>> calculate gd->ram_top from the device tree information.
> 
> Nothing in U-Boot that I'm aware of gets RAM information from DT. It all
> comes from the banks in gd. That's set up by some early low level code
> in arch/arm/mach-tegra/board2.c, which at least upstream hard-codes the
> size of the RAM gap below 4G rather than getting information from either
> DT or the earlier FW loads. So, I'm not sure what DT has to do with this.
> 
>>> At this point, I think the best thing is to revert aa909462d018
>>> "efi_loader: efi_allocate_pages is too restrictive" since it causes a
>>> regression on Jetson TX1, and we can work out the correct way to fix all
>>> this once the system is working again.
>>>
>>
>> The situation before the patch was really buggy. It is sufficient if you
>> get the Jetson device tree right.
> 
> What bugs specifically? Perhaps there's a better way to fix them.
> 
> Anyway, there was agreement in other messages in the thread to revert
> the problematic patch to avoid breaking the code while we work on a more
> complete solution. That's about all I can do right now since I'm heading
> off on vacation for a couple of weeks and would rather we have a working
> U-Boot running in the test farm during that time so that any other bugs
> that are introduced get caught rather than hidden. I'll send the revert.
>