Hi Jonas, Il giorno mar 22 set 2026 alle ore 15:28 Jonas Karlman ha scritto: > > Hi Massimo, > > On 9/22/2026 1:35 PM, Massimo Pegorer wrote: > > Hi Jonas, > > > > Il giorno dom 20 set 2026 alle ore 18:19 Jonas Karlman > > <[email protected]> ha scritto: > >> > >> 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 seems to use 14 bits, meaning that reading from > >> 0xff0e4000+ wraps around and instead reads back data at 0xff0e0000+. > >> > >> 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, the > >> generated GD_SIZE is typically around 304 bytes and BSS around 72 bytes. > >> > >> Current memory layout possibly looks something like: > >> > >> [0xff0e0000, 0xff0e4000) 16 KiB SRAM > >> [0xff0e0000, 0xff0e1000) 4 KiB BROM area > >> [..., 0xff0e08c0) stack area > >> [0xff0e08c0] stack pointer > >> [0xff0e08c0, 0xff0e09f0) 304 bytes global data (zeroed) > >> [0xff0e09f0, 0xff0e0ff0) 1536 bytes malloc area (untouched) > >> [0xff0e1000, 0xff0e4000) 12 KiB TPL area > >> [0xff0e1000, 0xff0e3800) 10 KiB code > >> [0xff0e3800, 0xff0e3848) 72 bytes BSS > >> > >> TPL currently does not touch the malloc area, instead the reserved > >> TPL_SYS_MALLOC_F_LEN bytes help push the zeroed global data struct and > >> the TPL stack addresses deeper into the BootROM reserved area. > >> > >> Using current offsets for global data and stack does not seem to cause > >> any known issue for BootROM after TPL returns to BootROM to load SPL > >> into DRAM, however any change to TPL_STACK, TPL_SYS_MALLOC_F_LEN or code > >> could theoretically change this and cause issues for BootROM. > > > > Here with "any change to [...] code" I suppose you mean any change of global > > data and/or BSS sizes, correct? > > Not only, the change to TPL_STACK, TPL_SYS_MALLOC_F_LEN or 'struct > global_data' would affect where the TPL stack ends up. However, and any > change to the code may change how the stack is used and thus possible > write to more/less/other data inside the BootROM area. > > > As code size itself is checked against > > TPL_MAX_SIZE at build time. Regarding build time checks, I have a couple > > of doubts about [1]: > > - shouldn't we check for less than or equal against TPL_MAX_SIZE, instead of > > strictly less than? > > - shouldn't be more correct to check the TPL "footprint" size against the 10 > > (or 11 or 12) KiB limit, instead only the code size? > > Correct, this was also briefly discussed/mentioned in the v2 thread. However, > this patch only tries to address the issue with TPL stack being placed inside > BootROM reserved area.
Indeed. I was just wondering if in a future patch we should avoid TPL_MAX_SIZE configuration, and instead have just TPL_MAX_FOOTPRINT configured to 0x3000 adding GD_SIZE and TPL_SYS_MALLOC_F_LEN in the linker check. > > > > > With the new layout the check should be: > > code size + BSS + GD_SIZE + TPL_SYS_MALLOC_F_LEN + <min_stack> <= 12 KiB > > > > [1] > > https://github.com/u-boot/u-boot/blob/main/arch/arm/mach-rockchip/u-boot-tpl-v8.lds > > > >> > >> Update TPL_STACK to use the top of SRAM address and reduce MALLOC_F_LEN > >> to 0x200 to avoid overflowing global data, malloc area and stack into > >> the BootROM reserved region. > > > > The new layout is certainly safer, but I wonder if we are losing the > > opportunity for TPL to have approximately 2 extra KiB as they are not used > > by > > BootROM once TPL returns to it. I mean, it would be useful to know where > > initial values of TPL_STACK and TPL_SYS_MALLOC_F_LEN came from, and the > > rationales behind them. If Rockchip were to assure that preserving only the > > first 2 KiB of SRAM is enough for SPL loading, keeping these 2 extra KiB on > > the radar could be useful for future TPL development and/or maintenance. > > Please look at v1/v2, they safely allowed for up to 11 KiB TPL. However, > that change should be handled in a separate patch/series. I had looked at them, and I agree on a separate patch/series. But here I mean the 2 KiB that (maybe?) can be used in the 4 KiB BROM SRAM area, in addition to memory in the 12 KiB TPL SRAM area. > > Some of vendor ddr blobs continue to use the BootROM stack, and others > setup a dedicated stack at end of SRAM. Mainline U-Boot use a dedicated > stack, and it is placed at end of SRAM for all other Rockchip SoCs. I totally agree on following a common approach over more SoCs when possible. Not a relevant contribution, anyway that is my: Tested-by: Massimo Pegorer <[email protected]> # PX30 custom board Regards, Massimo > > This patch tries to align PX30/RK3326 to use TPL stack at end of SRAM, > ahead of a future pending TPL consolidation/refactor series, see [2] or > deep in my work-in-progress branch at [3]. > > [2] > https://git.u-boot-project.org/u-boot/contributors/kwiboo/u-boot/-/commits/tpl > [3] > https://git.u-boot-project.org/u-boot/contributors/kwiboo/u-boot/-/commits/rk3562 > > Regards, > Jonas > > > > > Regards, > > Massimo > > > >> > >> With these changes the memory layout looks something like: > >> > >> [0xff0e0000, 0xff0e4000) 16 KiB SRAM > >> [0xff0e0000, 0xff0e1000) 4 KiB BROM area > >> [0xff0e1000, 0xff0e4000) 12 KiB TPL area > >> [0xff0e1000, 0xff0e3800) 10 KiB code > >> [0xff0e3800, 0xff0e3848) 72 bytes BSS > >> [0xff0e3848, 0xff0e3cd0) 1160 bytes stack area > >> [0xff0e3cd0] stack pointer > >> [0xff0e3cd0, 0xff0e3e00) 304 bytes global data > >> [0xff0e3e00, 0xff0e4000) 512 bytes malloc area (unused) > >> > >> Signed-off-by: Jonas Karlman <[email protected]> > >> Reviewed-by: Simon Glass <[email protected]> > >> --- > >> The overflow/wrapping was runtime validated on a RK3326 ODROID-GO Super > >> by printing out content of [0xff0e0000, 0xff0e5000] and comparing the > >> initial 4 KiB with the 16-20 KiB and the data matched 1:1. > >> > >> Changes in v3: > >> - Drop TPL_MAX_SIZE change > >> - Keep a small 512 byte malloc area > >> - Update commit message to reflect changes > >> > >> Changes in v2: > >> - Mention BSS in commit message > >> - Update spelling in commit message > >> - Collect r-b tag > >> --- > >> arch/arm/mach-rockchip/px30/Kconfig | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/arm/mach-rockchip/px30/Kconfig > >> b/arch/arm/mach-rockchip/px30/Kconfig > >> index adba1b49a52b..0088d436d36f 100644 > >> --- a/arch/arm/mach-rockchip/px30/Kconfig > >> +++ b/arch/arm/mach-rockchip/px30/Kconfig > >> @@ -81,10 +81,10 @@ config TPL_TEXT_BASE > >> default 0xff0e1000 > >> > >> config TPL_STACK > >> - default 0xff0e4fff > >> + default 0xff0e4000 > >> > >> config TPL_SYS_MALLOC_F_LEN > >> - default 0x600 > >> + default 0x200 > >> > >> config DEBUG_UART_CHANNEL > >> int "Mux channel to use for debug UART2/UART3" > >> -- > >> 2.55.0 > >> >
