[U-Boot] [PATCH 2/2] armv8: add asserts of sizes to u-boot-spl.lds

Oded Gabbay oded.gabbay at gmail.com
Tue Jan 17 09:04:44 CET 2017


On Tue, Jan 17, 2017 at 9:55 AM, Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
> 2016-12-26 23:20 GMT+09:00 Oded Gabbay <oded.gabbay at 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 at gmail.com>
>> Cc: Albert Aribaud <albert.u.boot at 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


More information about the U-Boot mailing list