On 30.10.18 13:37, Simon Goldschmidt wrote:
On Tue, Oct 30, 2018 at 12:28 PM Stefan Roese sr@denx.de wrote:
On 30.10.18 12:17, Simon Goldschmidt wrote:
<snip>
> diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h > index 2330143cf1..bd8f5c8c41 100644 > --- a/include/configs/socfpga_common.h > +++ b/include/configs/socfpga_common.h > @@ -31,8 +31,21 @@ > #define CONFIG_SYS_INIT_RAM_ADDR 0xFFE00000 > #define CONFIG_SYS_INIT_RAM_SIZE 0x40000 /* 256KB */ > #endif > + > +/* > + * Some boards (e.g. socfpga_sr1500) use 8 bytes at the end of the internal > + * SRAM as bootcounter storage. Make sure to not put the stack directly > + * at this address to not overwrite the bootcounter by checking, if the > + * bootcounter address is located in the internal SRAM. > + */ > +#if ((CONFIG_SYS_BOOTCOUNT_ADDR > CONFIG_SYS_INIT_RAM_ADDR) && \ > + (CONFIG_SYS_BOOTCOUNT_ADDR < (CONFIG_SYS_INIT_RAM_ADDR + \ > + CONFIG_SYS_INIT_RAM_SIZE))) > +#define CONFIG_SYS_INIT_SP_ADDR CONFIG_SYS_BOOTCOUNT_ADDR > +#else > #define CONFIG_SYS_INIT_SP_ADDR \ > (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_RAM_SIZE) > +#endifCan we have this check on CONFIG_INIT_RAM_SIZE instead of the initial stack pointer?
That would ensure the SPL size checks stay intact.
I'm not really sure what you mean with this. Could you please explain in more detail?
Sorry for being unclear. What I meant was: currently CONFIG_SYS_INIT_RAM_SIZEis 0x10000 (the full 64 kByte). So if CONFIG_SYS_BOOTCOUNT_ADDR is 0xfffffff8, I think we should define CONFIG_SYS_INIT_RAM_SIZE to 0xfff8. That way, not only the CONFIG_SYS_INIT_SP_ADDR define is correct but CONFIG_SPL_MAX_SIZE is checked to not overlap this address, too.
Would that make sense to you?
Yes, I thought that you meant it this way. I'm not sure if we should go this way. As we would change CONFIG_SYS_INIT_RAM_SIZE to something that does not represent the physical size of the on-chip SRAM. This could be very confusing and misleading, if this define is used elsewhere.
Thanks, Stefan