[U-Boot] [PATCH] arm: socfpga: check total size of SPL

[Simon Goldschmidt]

Am Fr., 2. Nov. 2018, 01:08 hat Marek Vasut <marex at denx.de> geschrieben:

> On 11/01/2018 08:26 PM, Simon Goldschmidt wrote:
> > On 31.10.2018 11:00, Marek Vasut wrote:
> >> On 10/31/2018 06:44 AM, Simon Goldschmidt wrote:
> >>> On Tue, Oct 30, 2018 at 11:02 PM Marek Vasut <marek.vasut at gmail.com>
> >>> wrote:
> >>>> On 10/30/2018 10:30 PM, Simon Goldschmidt wrote:
> >>>>> On 30.10.2018 22:26, Marek Vasut wrote:
> >>>>>> On 10/30/2018 10:23 PM, Simon Goldschmidt wrote:
> >>>>>>> Correctly define CONFIG_SPL_MAX_FOOTPRINT to make the default arm
> >>>>>>> linker script for SPL check the total SRAM size available for SPL
> >>>>>>> (code, data, bss, heap, global data).
> >>>>>>>
> >>>>>>> The previously existing define CONFIG_SPL_MAX_SIZE seems to only
> >>>>>>> check the binary size (which is without bss, heap and gd).
> >>>>>>>
> >>>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> >>>>>>> ---
> >>>>>>>
> >>>>>>>    include/configs/socfpga_common.h | 7 +++++++
> >>>>>>>    1 file changed, 7 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/include/configs/socfpga_common.h
> >>>>>>> b/include/configs/socfpga_common.h
> >>>>>>> index 2330143cf1..9103d0a966 100644
> >>>>>>> --- a/include/configs/socfpga_common.h
> >>>>>>> +++ b/include/configs/socfpga_common.h
> >>>>>>> @@ -242,6 +242,13 @@ unsigned int
> >>>>>>> cm_get_qspi_controller_clk_hz(void);
> >>>>>>>    #define CONFIG_SPL_TEXT_BASE        CONFIG_SYS_INIT_RAM_ADDR
> >>>>>>>    #define CONFIG_SPL_MAX_SIZE        CONFIG_SYS_INIT_RAM_SIZE
> >>>>>>>    +/* Check total size of SPL including BSS, malloc area and gd */
> >>>>>>> +#include <generated/generic-asm-offsets.h>
> >>>>>>> +#define CONFIG_SPL_MAX_FOOTPRINT    (CONFIG_SYS_INIT_SP_ADDR - \
> >>>>>>> +                     CONFIG_SYS_INIT_RAM_ADDR - \
> >>>>>>> +                     CONFIG_SYS_MALLOC_F_LEN - \
> >>>>>>> +                     GENERATED_GBL_DATA_SIZE)
> >>>>>> Are you sure this calculation is correct ? INIT_SP_ADDR is I think
> >>>>>> the
> >>>>>> SRAM offset in the address space. Shouldn't that contain
> >>>>>> INIT_SP_SIZE or
> >>>>>> something ?
> >>>>> Yes, I'm pretty sure. INIT_SP_ADDR is defined as INIT_RAM_ADDR +
> >>>>> INIT_RAM_SIZE.
> >>>>> So by subtracting INIT_RAM_ADDR again, I effectively get
> >>>>> "INIT_RAM_ADDR
> >>>>> - MALLOC_F_LEN - GBL_DATA_SIZE".
> >>>>>
> >>>>> But I did it this way to keep it working after Stefan's fix for
> >>>>> reserving the boot counter location is applied.
> >>>> Now that's confusing :-) Add a comment explaining this into a V2
> >>>> please,
> >>>> otherwise the confusion will continue ...
> >>> You're probably right. I'll send a V2 making this more clear.
> >> Thanks
> >
> > Re-checking this, I found the check is still not correct as it would
> > leave 4 bytes for the stack only. Is there an option that controls how
> > much stack should be preserved for SPL somewhere?
>
> Stack just grows down, that's all, there's no limit.
>

Right. But that sounds kind of dangerous...

And it would make sense to let heap and stack grow against each other
instead of letting heap grow up against end of ram and stack down against
bss_end or the devicetree...

To achieve that, 'gd' could be placed as low as possible, but the common
startup code doesn't seem to make that easy, especially as there is no
linker symbol for the devicetree start/end of it is not embedded.

Simon

>