On Tue, Jan 17, 2017 at 9:55 AM, Masahiro Yamada yamada.masahiro@socionext.com wrote:
2016-12-26 23:20 GMT+09:00 Oded Gabbay oded.gabbay@gmail.com:
This patch copies from arm u-boot-spl.lds some asserts that check that the size of the SPL image and BSS doesn't violate their max size.
Signed-off-by: Oded Gabbay oded.gabbay@gmail.com Cc: Albert Aribaud albert.u.boot@aribaud.net
arch/arm/cpu/armv8/u-boot-spl.lds | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds index e7799cc..0876a94 100644 --- a/arch/arm/cpu/armv8/u-boot-spl.lds +++ b/arch/arm/cpu/armv8/u-boot-spl.lds @@ -78,3 +78,22 @@ SECTIONS /DISCARD/ : { *(.interp*) } /DISCARD/ : { *(.gnu*) } }
+#if defined(CONFIG_SPL_MAX_SIZE) +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
"SPL image too big");+#endif
+#if defined(CONFIG_SPL_BSS_MAX_SIZE) && defined(CONFIG_SPL_MAX_FOOTPRINT) +ASSERT(0, "CONFIG_SPL_BSS_MAX_SIZE and CONFIG_SPL_MAX_FOOTPRINT must not be defined together"); +#endif
+#if defined(CONFIG_SPL_BSS_MAX_SIZE) +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
"SPL image BSS too big");+#endif
+#if defined(CONFIG_SPL_MAX_FOOTPRINT) +ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
"SPL image plus BSS too big");+#endif
2.7.4
This patch is wrong in multiple ways.
See the top lines of this file.
MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE, LENGTH = CONFIG_SPL_MAX_SIZE } MEMORY { .sdram : ORIGIN = CONFIG_SPL_BSS_START_ADDR, LENGTH = CONFIG_SPL_BSS_MAX_SIZE }
This means CONFIG_SPL_BSS_MAX_SIZE must always be defined.
Your code
+#if defined(CONFIG_SPL_BSS_MAX_SIZE) && defined(CONFIG_SPL_MAX_FOOTPRINT) +ASSERT(0, "CONFIG_SPL_BSS_MAX_SIZE and CONFIG_SPL_MAX_FOOTPRINT must not be defined together"); +#endif
... means CONFIG_SPL_BSS_MAX_FOOTPRINT can never be defined.
Correct. According to the README:
"CONFIG_SPL_MAX_FOOTPRINT and CONFIG_SPL_BSS_MAX_SIZE must not be both defined at the same time."
So this assert checks this. If this is wrong, then we should fix the README instead.
As a result,
+#if defined(CONFIG_SPL_BSS_MAX_SIZE) +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
"SPL image BSS too big");+#endif
the ifdef is always true, so meaningless.
Correct, we can remove this #ifdef
+ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
"SPL image BSS too big");is already size-checked by the following:
MEMORY { .sdram : ORIGIN = CONFIG_SPL_BSS_START_ADDR, LENGTH = CONFIG_SPL_BSS_MAX_SIZE }
so, meaningless.
+#if defined(CONFIG_SPL_MAX_FOOTPRINT) +ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
"SPL image plus BSS too big");+#endif
This code is never enabled.
Correct, assuming we leave the above "#if defined(CONFIG_SPL_BSS_MAX_SIZE) && defined(CONFIG_SPL_MAX_FOOTPRINT)" in place.
Thanks, Oded
-- Best Regards Masahiro Yamada