From: Simon Goldschmidt Date: Thu, 1 Nov 2018 20:26:05 +0100 Subject: [U-Boot] [PATCH] arm: socfpga: check total size of SPL In-Reply-To: <1451b497-ca80-a811-08b9-88e8b24d780d@denx.de> References: <20181030212311.28037-1-simon.k.r.goldschmidt@gmail.com> <5008750b-8b86-9461-3ed5-272aadbbf8d7@gmail.com> <1451b497-ca80-a811-08b9-88e8b24d780d@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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 >>>>>> --- >>>>>> >>>>>> 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 >>>>>> +#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? Simon