Re: [PATCH] rockchip: px30: Fix TPL_STACK overflow into BootROM reserved region

Hi Simon,

On 8/8/2026 12:00 AM, Simon Glass wrote:
> Hi Jonas,
> 
> On Fri, 7 Aug 2026 at 15:41, Jonas Karlman <[email protected]> wrote:
>>
>> 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.
> 
> Well we have DRAM in there now. Actually now that Ilas' series is in,
> we could take a look at trimming and aligning things - some things are
> long which could be int, which matters on arm64.

The px30/rk3326 boards do use CONFIG_NR_DRAM_BANKS=1 or 2, so dram
should at least be limited somewhat.

I think there are some DM related fields that could be excluded to save
little bit more space. With below diff the GD_SIZE is down to 240 bytes,
16 for TPL_SYS_MALLOC_F and remaining 48 due to the DM related fields.
Not sure this is safe, but the odroid-go2_defconfig could be built.

I do not think the global data size is really a big issue for this
board, but rk3036 only have 4 KiB that need to fit SPL (2716b),
BSS (0b), global data (160b) and stack. SPL image is down to 2716 bytes
after upcoming TPL refactoring at [1] (tpl branch).

diff --git a/configs/odroid-go2_defconfig b/configs/odroid-go2_defconfig
index dfdea66cd70c..c5fd6514010c 100644
--- a/configs/odroid-go2_defconfig
+++ b/configs/odroid-go2_defconfig
@@ -118,3 +118,4 @@ CONFIG_USB_FUNCTION_ROCKUSB=y
 CONFIG_TPL_TINY_MEMSET=y
 CONFIG_LZO=y
 CONFIG_ERRNO_STR=y
+# CONFIG_TPL_SYS_MALLOC_F is not set
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index fd116b0cebf2..886c12900ddb 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -174,10 +174,12 @@ struct global_data {
 	 * @arch: architecture-specific data
 	 */
 	struct arch_global_data arch;
+#if CONFIG_IS_ENABLED(DM)
 	/**
 	 * @dmtag_list: List of DM tags
 	 */
 	struct list_head dmtag_list;
+#endif
 	/**
 	 * @timebase_h: high 32 bits of timer
 	 */
@@ -228,7 +230,7 @@ struct global_data {
 	 */
 	long precon_buf_idx;
 #endif
-#ifdef CONFIG_DM
+#if CONFIG_IS_ENABLED(DM)
 	/**
 	 * @dm_root: root instance for Driver Model
 	 */

[1] https://git.u-boot-project.org/u-boot/contributors/kwiboo/u-boot/-/commit/8c8b98a29aa35880b93bbacce8daf68edb537eca

Regards,
Jonas

> 
> I also did some work on struct board_f which is another way to reduce
> space, if we can find more things which are only needed in early board
> start-up. I suppose we could even expand that to include the
> board_init_r() calls.
> 
> Regards,
> Simon