Hi Quentin,
On 9/24/26 12:03 PM, Quentin Schulz wrote:
> Hi Massimo,
>
> On 9/23/26 7:52 PM, Massimo Pegorer wrote:
> > 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.
> >
>
> I had never heard of that symbol before and went checking. It has never
> been used (there isn't even a Kconfig symbol for it!) so we would need a
> bit of work to make this work (probably not a crazy amount though :) ).
>
> I'm not sure we still need arch/arm/mach-rockchip/u-boot-tpl-v8.lds
> though. It's only used for evb-px5_defconfig, all PX30, all RK3328, all
> RK3399 and RV1126.
> I think it was necessary when added back in 2019 because
> arch/arm/cpu/armv8/u-boot-spl.lds didn't allow yet to remove the SDRAM
> part in it (c.f. 3cbec0ad090f ("rockchip: add u-boot-tpl-v8.lds") which
> claims "we don't have both SRAM and SDRAM in TPL" but we have SRAM
> access in TPL, don't we?), but that has changed since 2024. They are a
> bit different though and I have no experience with linker scripts so no
> clue if it makes sense to merge them (if at all possible). I'm guessing
> the additional asserts would make sense for everybody anyway.
Me too, almost no experience with linker scripts. Anyway, I had a quick look
at arch/arm/cpu/armv8/u-boot-spl.lds for TPL building purpose, and IMO it's
broken by design because of SPL specific configuration options dependencies:
SPL_SEPARATE_BSS and SPL_RECOVER_DATA_SECTION. I will check more in depth.
BTW, the .sram MEMORY segment definition in the script would do the game: here
the size that linker would check against TPL_MAX_SIZE includes BSS (consider
the case SPL_SEPARATE_BSS not defined, of course), while u-boot-tpl-v8.lds
checks size without BSS against TPL_MAX_SIZE.
I suppose that the "we don't have both SRAM and SDRAM in TPL" reason to fork
this TPL Rockchip specific script has been overcome by a3403e6fbf80 ("armv8:
Support not having separate BSS") which added SEPARATE_BSS usage (even if not
in the suitable way).
>
> >>
> >>>
> >>> 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.
> >
>
> I.. would rather not. While I don't expect Rockchip to change the
> BootROM without changing the SoC official name, it's not impossible. I
> would err on the side of what Rockchip is doing in their fork and trust
> that they get this early init stuff right, hopefully *they* know what's
> happening in the BootROM for their own SoCs :)
Understood, I agree.
Regards,
Massimo
>
> >>
> >> 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:
>
> Review is relevant contribution, so thanks for doing it!
>
> Cheers,
> Quentin
