On 11/01/2018 10:13 PM, Simon Goldschmidt wrote: > On 01.11.2018 21:59, Marek Vasut wrote: >> On 11/01/2018 09:31 PM, Simon Goldschmidt wrote: >>> On Thu, Nov 1, 2018 at 8:26 PM Simon Goldschmidt >>> <simon.k.r.goldschm...@gmail.com> 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.va...@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.goldschm...@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? >>> And to make matters worse, the devicetree is also not included in the >>> size check done by 'arch/arm/cpu/u-boot-spl.lds' unless we select >>> CONFIG_OF_EMBED. >>> >>> While this would be OK for SPL, I think it wouldn't be for U-Boot. How >>> do other platforms handle this size check? >> Can git grep help here ? > > Not really. If so, I wouldn't have asked ;-) > > I can only git grep for known strings and I only see constants for > CONFIG_SPL_MAX_SIZE. > CONFIG_SPL_MAX_FOOTPRINT seems to be rarely used and no platform seems > to be taking the devicetree into account.
Ouch :( > Stack usage seems to be covered in one or two boards, but I expect this > to be quite platform specific (given he different drivers at least), so > we might need an additional runtime check, which I haven't seen so far? > In my (smaller) embedded projects, I often enable runtime stack checks > to be on the safe side... Makes sense. > Maybe printing free memory at build-time and printing stack usage at > runtime would be a solution? But I only know U-Boot from the socfpga > gen5 perspective, that's why I ask. Wouldn't the stack usage change with usage too ? I think it might, albeit slightly. I'm not sure this would be really useful then. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
