[PATCH] lmb: prohibit allocations above ram_top even from same bank

[Patrice CHOTARD]

On 12/11/24 19:16, Sughosh Ganu wrote:
> On Wed, 11 Dec 2024 at 22:20, Patrice CHOTARD
> <patrice.chotard at foss.st.com> wrote:
>>
>>
>>
>> On 12/11/24 17:27, Sughosh Ganu wrote:
>>> On Wed, 11 Dec 2024 at 21:50, Patrice CHOTARD
>>> <patrice.chotard at foss.st.com> wrote:
>>>>
>>>>
>>>>
>>>> On 12/7/24 16:57, Tom Rini wrote:
>>>>> On Mon, 02 Dec 2024 12:36:24 +0530, Sughosh Ganu wrote:
>>>>>
>>>>>> There are platforms which set the value of ram_top based on certain
>>>>>> restrictions that the platform might have in accessing memory above
>>>>>> ram_top, even when the memory region is in the same DRAM bank. So,
>>>>>> even though the LMB allocator works as expected, when trying to
>>>>>> allocate memory above ram_top, prohibit this by marking all memory
>>>>>> above ram_top as reserved, even if the said memory region is from the
>>>>>> same bank.
>>>>>>
>>>>>> [...]
>>>>>
>>>>> Applied to u-boot/master, thanks!
>>>>>
>>>> Hello
>>>>
>>>> This patch is breaking the boot on STM32MP135F-DK.
>>>>
>>>> On this platform, we got an area above gd->ram_top,
>>>> this area, reserved for OPTEE, is tagged with LMB_NOMAP in boot_fdt_add_mem_rsv_regions().
>>>>
>>>> Since this commit 1a48b0be93d4 ("lmb: prohibit allocations above ram_top even from same bank"),
>>>> this area is no more tagged as LMB_NOMAP, because it's previously been
>>>> tagged with LMB_NOOVERWRITE in lmb_add_memory().
>>>>
>>>> By not being tagged LMB_NOMAP, the MMU configuration is impacted and leads to a panic.
>>>>
>>>> I suggest to revert this patch.
>>>
>>> I don't think that this patch should be reverted. If the said platform
>>> has a reserved memory region above ram_top, I would suggest to either
>>> a) move the ram_top on this platform so that the op-tee region gets
>>> marked as no-map in the lmb memory map, or b) do not use the lmb
>>
>> In my explanation above, i indicated that before this commit,
>> this area was marked as LMB_NOMAP in the lmb memory map by boot_fdt_add_mem_rsv_regions().
>> this is exactly what you described in the possible solution "a)".
>>
>> But now with this commit, as lmb_add_memory() is called in lmb_init() the area above ram_top is marked LMB_NOOVERWRITE.
>> Then later, boot_fdt_add_mem_rsv_regions() is executed, but the area above ram_top can't be marked as
>> LMB_NOMAP as previously because it's already marked LMB_NOOVERWRITE.
> 
> This has been done to ensure that memory above ram_top is not taken
> into consideration when it comes to U-Boot. The reason why memory

It was already the case before this commit, ram_top was designed to
indicate to U-Boot the top address of available RAM,
see include/asm-generic/global_data.h :

	/**
	 * @ram_top: top address of RAM used by U-Boot
	 */
	phys_addr_t ram_top;

> above ram_top also needs to be added is to ensure that this memory
> also gets passed on to the OS when booting with EFI. If it has to be
> considered by U-Boot, the value of ram_top needs to be adjusted
> accordingly. Is that not possible on the platform? If not, the only
> other solution is to obtain this memory region from the DT, and then
> configure the MMU.

Currently, ram_top is adjusted on STM32MP platforms, 
for example in stm32mp135f-dk.dts :

	reserved-memory {
		#address-cells = <1>;
		#size-cells = <1>;
		ranges;

		optee at dd000000 {
			reg = <0xdd000000 0x3000000>;
			no-map;
		};
	};

          0xE000 0000  ********************  
                       *                  *  
                       *       OPTEE      *  
                       *    (LMB_NOMAP)   *  
                       *                  *  
ram_top = 0xDD00 0000  ********************  
                       *                  *
                       *                  *
                       *                  *
                       *                  *
                       *                  *
                       *                  *
                       *                  *
                       *                  *
                       *                  *
                       *                  *
          0xC000 0000  ********************

On STM32MP platforms, we already obtain all memory regions from DT with 
property "no-map" and we marked them LMB_NOMAP.

Later we parse the LMB regions, all of these region marked LMB_NOMAP are 
used to configure our MMU accordingly.
So, again, we are doing things as you suggested.

This commit now forbids to mark OPTEE memory region with LMB_NOMAP as 
indicated in DT.

For information, it has impact on all STM32MP platforms (at least 6 boards).

Patrice



> 
>>
>>
>>> memory map to configure the MMU -- can the MMU configuration logic not
>>> read the DT to get which regions are to be marked as no-map?
>>>
>>> As far as the lmb module is concerned, it is being told through this
>>> commit to not consider memory above ram_top for allocations, which is
>>> not an incorrect thing imo.
>>
>> That's the case, we don't consider memory above ram_top for allocations,
>> we only marked it with LMB_NOMAP.
> 
> That was because the lmb scope was local. That meant a platform could
> add any size that it wanted, and then use that map for whatever it
> fancied. The use of lmb for "allocating" addresses for io-va addresses
> by the apple iommu is another such case.
> 
> -sughosh
> 
>>
>> Thanks
>> Patrice
>>
>>>
>>> -sughosh
>>>
>>>>
>>>> Patrice