Hi Quentin, Il giorno lun 28 set 2026 alle ore 11:10 Quentin Schulz <[email protected]> ha scritto: > > Hi Massimo, > > On 9/28/26 8:42 AM, Massimo Pegorer wrote: > > 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. > > > > We can always migrate those to CONFIG_IS_ENABLED(SEPARATE_BSS), > CONFIG_IS_ENABLED(RECOVER_DATA_SECTION), CONFIG_VAL(BSS_START_ADDR), > CONFIG_VAL(BSS_MAX_SIZE), etc., if needed.
Thanks for the suggestion. I will run some tests and then try to send a patch to fix the usage of u-boot-spl.lds for generic XPL cases, as a first step to eventually drop u-boot-tpl-v8.lds later on, if possibile. > > > 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. > > > > We would have the same behavior of size checking if we had > TPL_SEPARATE_BSS enabled I believe. But if we had TPL_SEPARATE_BSS > enabled, then we would have an .sdram section too, which is the reason > for having a separate linker script in the first place. > > I'm also not sure about IMAGE_TEXT_BASE (set by CONFIG_TEXT_BASE) only > used in the armv8 linker script and the first > > . = 0x00000000; > > in Rockchip's TPL linker script. > > But maybe we should really have BSS part of the size check. > > I'm out of my depth here though, no clue what would be proper. > > Cheers, > Quentin
