[U-Boot] [PATCH 1/2] armv8: calculate __bss_size in u-boot-spl.lds

[Simon Glass]

Hi,

On 19 January 2017 at 04:28, Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
> Hi.
>
> (CCing Simon because he wrote the code addressed in this discussion.)
>
>
> 2017-01-17 18:27 GMT+09:00 Oded Gabbay <oded.gabbay at gmail.com>:
>> Hi Mashiro,
>> Could you please also take a look at the patch I sent to add the
>> missing __bss_size ?
>>
>> Thanks,
>> Oded
>>
>> On Mon, Dec 26, 2016 at 4:20 PM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
>>> This patch adds a missing __bss_size symbol to the default armv8
>>> u-boot-spl.lds file.
>>> Makefile.spl relies on __bss_size to be present when it creates the SPL
>>> image. It uses that symbol to create a pad file that will be used to place
>>> the dtb after the bss section.
>>> In ARMv8 default u-boot-spl.lds, __bss_size was missing and therefore, the
>>> pad file was always 0. As a result, the dtb was placed after
>>> _image_binary_end, which caused a failure when loading it inside the SPL.
>>>
>>> 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 | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds
>>> index cc427c3..e7799cc 100644
>>> --- a/arch/arm/cpu/armv8/u-boot-spl.lds
>>> +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
>>> @@ -70,6 +70,7 @@ SECTIONS
>>>                 KEEP(*(.__bss_end));
>>>         } >.sdram
>>>
>>> +       __bss_size = __bss_end - _image_binary_end;
>>>         /DISCARD/ : { *(.dynsym) }
>>>         /DISCARD/ : { *(.dynstr*) }
>>>         /DISCARD/ : { *(.dynamic*) }
>
>
>
> This patch seems wrong.
>
>
> This linker script seems to support separate two memory regions.
>
> 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 }
>
>
> _image_binary_end is the end of the first region (.sram),
> __bss_end is the end of the second region (.sdram).
>
> So, with your patch,
>
>    __bss_size = (gap between .sram and .sdram) + (size of .bss)
>
> This will append really big padding there is a gap between the two regions.
> (at least, it will break my boards.)
>
>
> If you take a look at fdtdec_setup(),
>
>     #  ifdef CONFIG_SPL_BUILD
>     /* FDT is at end of BSS unless it is in a different memory region */
>     if (IS_ENABLED(CONFIG_SPL_SEPARATE_BSS))
>              gd->fdt_blob = (ulong *)&_image_binary_end;
>     else
>              gd->fdt_blob = (ulong *)&__bss_end;
>
> I think you are supposed to enable CONFIG_SPL_SEPARATE_BSS
> if you want to use SPL_OF_CONTROL on your arm64 boards.
>
>
>
> Checking the following commit will help you.
>
>
> commit 10172962479ddd6609101fdb83bde66c0719852c
> Author: Simon Glass <sjg at chromium.org>
> Date:   Sat Oct 17 19:41:19 2015 -0600
>
>     dm: spl: Support device tree when BSS is in a different section
>
>     At present in SPL we place the device tree immediately after BSS. This
>     avoids needing to copy it out of the way before BSS can be used. However on
>     some boards BSS is not placed with the image - e.g. it can be in RAM if
>     available.
>
>     Add an option to tell U-Boot that the device tree should be placed at the
>     end of the image binary (_image_binary_end) instead of at the end of BSS.
>
>     Note: A common reason to place BSS in RAM is to support the FAT filesystem.
>     We should update the code so that it does not use so much BSS.
>
>     Signed-off-by: Simon Glass <sjg at chromium.org>
>     Signed-off-by: Michal Simek <michal.simek at xilinx.com>

Yes that's right. Thank you Masahiro.

Also, consider moving to use binman to generate the image.

Regards,
Simon