[PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin

Wed Jun 14 19:22:04 CEST 2023

On 2023-06-14 09:46, Peter Robinson wrote:
> On Tue, Jun 13, 2023 at 7:48 PM Jonas Karlman <jonas at kwiboo.se> wrote:
>>
>> Hi Peter,
>>
>> On 2023-06-13 09:58, Peter Robinson wrote:
>>> On Mon, Jun 12, 2023 at 11:20 PM Jonas Karlman <jonas at kwiboo.se> wrote:
>>>>
>>>> Hi Peter,
>>>>
>>>> On 2023-06-11 22:22, Peter Robinson wrote:
>>>>> Hi Jonas,
>>>>>
>>>>> This regresses the Rockpro64 build for me when building with gcc 12/13
>>>>> with the following error, if I remove CONFIG_LTO it builds, but
>>>>> overlaps.
>>>>>
>>>>> /usr/bin/aarch64-linux-gnu-ld:
>>>>> /usr/lib/gcc/aarch64-linux-gnu/13/libgcc.a(lse-init.o): in function
>>>>> `init_have_lse_atomics':
>>>>> /builddir/build/BUILD/gcc-13.1.1-20230519/aarch64-linux-gnu/aarch64-linux-gnu/libgcc/../../../gcc-13.1.1-20230519/libgcc/config/aarch64/lse-init.c:46:
>>>>> undefined reference to `__getauxval'
>>>>> collect2: fatal error: ld terminated with signal 11 [Segmentation
>>>>> fault], core dumped
>>>>> compilation terminated.
>>>>
>>>> Interesting, my test build using ubuntu-22.04 and gcc-12 did not seem
>>>> to fail, see [1]. Compiling with -mno-outline-atomics flag may work, not
>>>> sure that is a good workaround.
>>>
>>> I would prefer a fix.
>>
>> Not sure what such fix could be, cross compiling in CI and under ubuntu
>> 22.04 does not trigger this linking issue, not sure what is different in
>> your build environment to not allow build with CONFIG_LTO=y.
>>
>> Searching for similar linking issue suggest it could be related to
>> outline-atomics, do you use any special C/LDFLAGS or similar?
>>
>>>
>>>> An alternative could be to move the payload to @ 512KB instead of @ 384KB.
>>>
>>> What impact will moving this around have on upgrades? I am not sure we
>>> should be randomly moving stuff without knowing the impact TBH. Will
>>> this break existing users vs what you feel is appropriate for your
>>> usecase?
>>
>> I did a re-calculation and using @ 512 KB offset may not be enough
>> for a worst-case scenario.
>>
>> mkimage will enforce a 184 KB limit for the TPL on RK3399. With SPL
>> loaded at 0x0 and TF-A loaded to 0x40000, we have a 256 KB size limit of
>> SPL. BootRom that loads both TPL and SPL only read first 2 KB of every
>> 4 KB page of SPI flash, so idbloader.img (TPL+SPL) could grow up to
>> ~880 KB in a worst-case scenario. (184+256) x 2 = 880
>>
>> Using a payload offset of @ 896 KB (0xE0000) is probably the safest
>> default option if we need to revert the CONFIG_LTO=y change.
>>
>> As long as u-boot-rockchip-spi.bin is used to update SPI flash changing
>> offset should not have an impact. However, there are guides and scripts
>> that write idbloader.img and u-boot.itb separately, those could break.
> 
> Well I believe your patch only enabled that option by default so most
> users I suspect write them separately up until now, does the new
> offsets affect existing env variables at all?

It does not look like it will overwrite env offset.

  CONFIG_ENV_SIZE=0x8000
  CONFIG_ENV_OFFSET=0x3F8000

There will be room for a 3168 KB u-boot.itb payload until it would
overlap env in SPI flash with a 0xE0000 offset for u-boot.itb.

Also look like many RK3399 defconfig have a SPL_MAX_SIZE=0x2e000,
something that could be increased to SPL_MAX_SIZE=0x40000 with an
updated offset for u-boot.itb.

I will prepare a small fix series that address your LTO issue and use
proper offsets and max size options.

> 
>> Such guides and scripts can already lead to users overwriting part of
>> SPL when the produced idbloader.img is over 384 KB in size.
>> With CONFIG_ROCKCHIP_SPI_IMAGE=y the size of idbloader.img is instead
>> enforced to not cause an overlap of the u-boot.itb at build time.
> 
> Ultimately I would prefer to not actively break existing users if
> they're not aware of changes, and that is hard to communicate. People
> with broken devices tend to scream a lot.

Such change will not actively break existing users, SPL look for a FIT
header and will fall back to next media in u-boot,spl-boot-order.

Will include an update to SPI flash section of rockchip.rst to mention
how to properly flash to correct offset using u-boot-rockchip-spi.bin
in the fix series.

Regards,
Jonas

> 
>> Regards,
>> Jonas
>>
>>>
>>>> configs/rockpro64-rk3399_defconfig:
>>>>   CONFIG_SYS_SPI_U_BOOT_OFFS=0x80000
>>>>
>>>> arch/arm/dts/rk3399-rockpro64-u-boot.dtsi:
>>>>   u-boot,spl-payload-offset = <0x80000>;
>>>>
>>>>
>>>> [1] https://github.com/Kwiboo/u-boot-build/actions/runs/5248398191/jobs/9479827969
>>>>
>>>> Regards,
>>>> Jonas
>>>>
>>>>>
>>>>> Peter
>>>>>
>>>>> On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas at kwiboo.se> wrote:
>>>>>>
>>>>>> Enable CONFIG_ROCKCHIP_SPI_IMAGE to build u-boot-rockchip-spi.bin.
>>>>>> Define CONFIG_SYS_SPI_U_BOOT_OFFS to write u-boot.itb at the expected
>>>>>> offset. Enable CONFIG_LTO to reduce size of SPL so that the mkimage
>>>>>> output fit before the 0x60000 offset in u-boot-rockchip-spi.bin.
>>>>>>
>>>>>>   => sf probe
>>>>>>   SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB
>>>>>>   => load mmc 1:1 10000000 u-boot-rockchip-spi.bin
>>>>>>   1442304 bytes read in 27 ms (50.9 MiB/s)
>>>>>>   => sf update $fileaddr 0 $filesize
>>>>>>   device 0 offset 0x0, size 0x160200
>>>>>>   1421824 bytes written, 20480 bytes skipped in 9.501s, speed 155432 B/s
>>>>>>
>>>>>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>>>>>> Reviewed-by: Kever Yang <kever.yang at rock-chips.com>
>>>>>> ---
>>>>>> v2:
>>>>>> - Collect r-b tag
>>>>>>
>>>>>>  configs/rockpro64-rk3399_defconfig | 3 +++
>>>>>>  1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
>>>>>> index 0ca2cecade25..f41c03067903 100644
>>>>>> --- a/configs/rockpro64-rk3399_defconfig
>>>>>> +++ b/configs/rockpro64-rk3399_defconfig
>>>>>> @@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x3F8000
>>>>>>  CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
>>>>>>  CONFIG_DM_RESET=y
>>>>>>  CONFIG_ROCKCHIP_RK3399=y
>>>>>> +CONFIG_ROCKCHIP_SPI_IMAGE=y
>>>>>>  CONFIG_TARGET_ROCKPRO64_RK3399=y
>>>>>>  CONFIG_SPL_STACK=0x400000
>>>>>>  CONFIG_DEBUG_UART_BASE=0xFF1A0000
>>>>>> @@ -20,6 +21,7 @@ CONFIG_SPL_SPI=y
>>>>>>  CONFIG_SYS_LOAD_ADDR=0x800800
>>>>>>  CONFIG_PCI=y
>>>>>>  CONFIG_DEBUG_UART=y
>>>>>> +CONFIG_LTO=y
>>>>>>  CONFIG_SPL_FIT_SIGNATURE=y
>>>>>>  CONFIG_BOOTSTAGE=y
>>>>>>  CONFIG_BOOTSTAGE_REPORT=y
>>>>>> @@ -37,6 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
>>>>>>  CONFIG_SPL_STACK_R=y
>>>>>>  CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
>>>>>>  CONFIG_SPL_SPI_LOAD=y
>>>>>> +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000
>>>>>>  CONFIG_TPL=y
>>>>>>  CONFIG_CMD_BOOTZ=y
>>>>>>  CONFIG_CMD_GPT=y
>>>>>> --
>>>>>> 2.40.1
>>>>>>
>>>>
>>