[PATCH 3/3] configs: stm32mp1: Fix misleading SPL size limitations

[Patrick DELAUNAY]

Hi,

On 3/22/21 2:20 PM, Alexandru Gagniuc wrote:
> A now removed comment promises to "limit SYSRAM usage to first 128 KB".
> This would imply that only SYSRAM from 0x2ffc0000 - 0x2ffe0000 would be
> used. This is not what happens at all.
>
> First, SPL_MAX_SIZE is referenced from SPL_TEXT_BASE, which on all
> existing configs is set to 0x2ffc2500, not SYSRAM_BASE (0x2ffc0000).
> Some of it is in the first 128 KiB and some of it is in the second
> 128 KiB chunk of SYSRAM.
>
> Second, SPL_MAX_SIZE, does not restrict the BSS size. While a valiant
> attempt is made via SPL_BSS_MAX_SIZE, the value of 0x00100000 is much
> larger than SYSRAM, and doesn't account for the non-BSS sections.
>
> Because we're putting the .text and .bss in the same boat, the correct
> way to limit them together is via SPL_MAX_FOOTPRINT. With the current
> SPL_TEXT_BASE, we couldn't limit even a very basic SPL to the first
> 128 KiB, and there is no technical reason to do so. Because of this,
> simply allow the SPL to use all SYSRAM.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me at gmail.com>
> ---
>   include/configs/stm32mp1.h | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
> index 56a70cb584..440efa1a55 100644
> --- a/include/configs/stm32mp1.h
> +++ b/include/configs/stm32mp1.h
> @@ -50,12 +50,12 @@
>   /* SPL support */
>   #ifdef CONFIG_SPL
>   /* SPL use DDR */
> -#define CONFIG_SPL_BSS_MAX_SIZE		0x00100000
>   #define CONFIG_SYS_SPL_MALLOC_START	0xC0300000
>   #define CONFIG_SYS_SPL_MALLOC_SIZE	0x01D00000
>   
> -/* limit SYSRAM usage to first 128 KB */
> -#define CONFIG_SPL_MAX_SIZE		0x00020000
> +/* Restrict SPL to fit within SYSRAM */
> +#define STM32_SYSRAM_END		(STM32_SYSRAM_BASE + STM32_SYSRAM_SIZE)
> +#define CONFIG_SPL_MAX_FOOTPRINT	(STM32_SYSRAM_END - CONFIG_SPL_TEXT_BASE)
>   #define CONFIG_SPL_STACK		(STM32_SYSRAM_BASE + \
>   					 STM32_SYSRAM_SIZE)
>   #endif /* #ifdef CONFIG_SPL */


Thanks for the analysis.


We don't revisit this part this the first STM32MP port on U-Boot

and CONFIG_SPL_MAX_FOOTPRINT is the correct way to limit the SPL size.


If I remember correctly, the 128KB limitation was only a attempt to reserved

enough SYSRAM for BSS + SPL stack located at CONFIG_SPL_STACK = end of 
sysram.



Reviewed-by: Patrick Delaunay <patrick.delaunay at foss.st.com>

Thanks


Patrick