Hi Simon, On 8/7/2026 10:40 PM, Simon Glass wrote: > Hi Jonas, > > On 2026-08-07T09:04:31, Jonas Karlman <[email protected]> wrote: >> rockchip: px30: Fix TPL_STACK overflow into BootROM reserved region >> >> The PX30/RK3326 has 16 KiB SRAM at [0xff0e0000, 0xff0e4000), with the >> initial 4 KiB at [0xff0e0000, 0xff0e1000) reserved for BootROM at boot. >> >> SRAM addressing only seem to use 14 bits, meaning that reading from >> 0xff0e4000+ wraps around and instad reads back data at 0xff0e0000+. > > Typos: 'seem' -> 'seems', 'instad' -> 'instead' and 'possible looks' > below -> 'possibly looks'.
Thanks, will update in a v2. > >> >> Using a TPL_STACK at 0xff0e4ff0 (16-bytes aligned) means TPL use BootROM >> reserved region for its global data, malloc area and runtime stack. >> >> TPL on PX30/RK3326 does not have any use for the malloc area and the >> generated GD_SIZE is typically around 304 bytes. >> >> Current memory layout possible looks something like: >> >> [0xff0e0000, 0xff0e4000) 16 KiB SRAM >> [0xff0e0000, 0xff0e1000) 4 KiB BROM area >> [0xff0e09f0, 0xff0e0ff0) 1536 bytes malloc area (unused) >> [0xff0e08c0, 0xff0e09f0) 304 bytes global data >> [...] >> >> arch/arm/mach-rockchip/px30/Kconfig | 4 ++-- >> common/spl/Kconfig.tpl | 2 +- >> tools/rkcommon.c | 2 +- >> 3 files changed, 4 insertions(+), 4 deletions(-) > >> diff --git a/common/spl/Kconfig.tpl b/common/spl/Kconfig.tpl >> @@ -129,7 +129,7 @@ config TPL_MAX_SIZE >> - default 0x2800 if ROCKCHIP_PX30 >> + default 0x2c00 if ROCKCHIP_PX30 > > Just to check, the layout shows [0xff0e3c00, 0xff0e3ed0) as pure > stack, but BSS (e.g. dram_info in sdram_px30.c) sits after > __image_copy_end and is not counted by the TPL_MAX_SIZE assert in > u-boot-tpl-v8.lds, so with an image close to 11 KiB the real stack > headroom is a bit less than 720 bytes. You are correct, current BSS is 72 bytes, will update the commit message to mention that the 720 bytes must fit both BSS and the stack in a v2. What is interesting is that global data is so large, especially when in this case nothing here is really using global data. We could save 16 bytes by fully disable CONFIG_TPL_SYS_MALLOC_F, but that was too intrusive to be part of this fix. Regards, Jonas > > The rest checks out: crt0_64.S does bic sp, x0, #0xf then > board_init_f_alloc_reserve(), so with 0xff0e4000 the GD ends up at > rounddown(0xff0e4000 - sizeof(gd), 16) and nothing writes at or above > 0xff0e4000, and px30-board-tpl.c uses no malloc, so a zero > TPL_SYS_MALLOC_F_LEN is safe. > > Reviewed-by: Simon Glass <[email protected]> > > Regards, > Simon
