Hi Jonas,
On 2/12/24 12:08, Jonas Karlman wrote:
Hi Quentin,
On 2024-02-12 11:37, Quentin Schulz wrote:
Hi Jonas,
On 2/12/24 01:59, Jonas Karlman wrote:
With the stack and text base used by U-Boot SPL and proper on RK3399
there is a high likelihood of overlapping when U-Boot proper + FDT nears
1 MiB in size.
Currently the following memory layout is used:
- 0x00000000 (@0 MiB): U-Boot SPL text base
- 0x00040000 (@256 KiB): TF-A
- 0x00200000 (@2 MiB): U-Boot proper text base
- 0x00300000 (@3 MiB): U-Boot proper init stack
- 0x00400000 (@4 MiB): U-Boot SPL init stack
- 0x00400000 (@4 MiB): U-Boot SPL bss
- 0x02000000 (@64 MiB): U-Boot SPL reloc stack
- 0x08400000 (@132 MiB): optional OPTEE
SPL load TF-A, U-Boot proper and optional OPTEE after SPL stack has
relocated, meaning that there is room for close to 2 MiB (@2-4 MiB).
However, the initial stack used by U-Boot proper is limiting the size
of U-Boot proper + FDT to below 1 MiB (@2-3 MiB).
Instead change to use the following memory layout:
- 0x00000000 (@0 MiB): U-Boot SPL text base
- 0x00040000 (@256 KiB): TF-A
- 0x00200000 (@2 MiB): U-Boot proper text base
- 0x00400000 (@4 MiB): U-Boot SPL init stack
- 0x00800000 (@8 MiB): U-Boot proper init stack (changed)
- 0x02000000 (@32 MiB): U-Boot SPL bss (changed)
- 0x04000000 (@64 MiB): U-Boot SPL reloc stack
- 0x08400000 (@132 MiB): optional OPTEE
This should leave room for loading and running a U-Boot proper close
to 6 MiB (@2-8 MiB) in size. When U-Boot proper has relocated is should
be possible to use @2-132 MiB and @164 MiB+ for scripts, kernel etc.
With the moved SPL reloc stack it is also possible to use a much larger
malloc area in SPL, e.g. using default SPL_STACK_R_MALLOC_SIMPLE_LEN.
Signed-off-by: Jonas Karlman <jo...@kwiboo.se>
Do we not need to update memory addresses in the environment as well?
The addresses in environment is to my knowledge only used in U-Boot
proper after relocation. And at that time we should be free to use @2+
MiB up to somewhere close to gd ram_top/relocaddr. The optional Rockchip
OPTEE blobb may occupy @132-164 MiB, but I do not think loading OPTEE
blobs works as-is, so that may not be a big issue.
Upstream OP-TEE OS seems to support RK3399, c.f.
https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/plat-rockchip/platform_rk3399.c
I cannot say whether it works, I have no knowledge of any of our
customers using it (but that predates my joining Theobroma, so maybe
long time customers do use it?). However, we do use an upstream-based
OP-TEE OS for one of our PX30-based boards, so I would say Rockchip's
OP-TEE OS is not the only option so we shouldn't just disregard this
location in RAM... especially since ramdisk_addr_r starts at 96MiB. But
since we're not changing that location.... people who would have this
issue, would have encountered the issue by now already (provided they
load a ramdisk bigger than 36MiB). So let's ignore that one actually :)
So to my knowledge we do not need to update any of the memory addresses
in the environment as well.
scriptaddr=0x00500000
This is at 5MiB if I'm not mistaken?
Basically everything in include/configs/rk3399_common.h in
ENV_MEM_LAYOUT_SETTINGS needs to be updated to be located after 164MiB?
Did I misunderstand something? I find it weird the current values seems
to be located after U-Boot SPL bss and that e.g.
kernel_addr_r=0x02080000
is at 32.5MiB, and with U-Boot SPL reloc stack currently at 64MiB... a
31.5+MiB kernel Image would mess up the data there? (what do we need it
for BTW once in U-boot proper after relocation?).
After SPL has jumped to TF-A and U-boot proper any memory used by SPL
can be overwritten/used for something else.
My understanding is as follow:
- SPL always need access to bss
- SPL need access to init stack until it has relocated to reloc stack
- SPL will load FIT images using the reloc stack, at this time only
SPL code, bss and reloc stack matters
- U-boot proper will use its init stack until it has relocated to end
of memory. Until U-Boot proper has relocated it will use init stack
- We could use same memory position for init stack for SPL and proper,
e.g. SPL_SHARES_INIT_SP_ADDR=y without any issue
- After U-Boot proper has relocated we should be free to use @2+ MiB
up to where U-Boot proper was relocated.
Not sure how Falcon boot fit in all of this and whether it makes use of
the environment variable (likely in the SPL binary itself).
Was this from experience and/or is there some literature in U-Boot git
repo already? Couldn't find much in the 5min I looked for it :)
Thanks for the taking the time to explain this, much appreciated.
Side question, what about making those values the default for RK3399
instead of having it in each defconfig? This could grow quickly out of
hands to move this to the Kconfig symbol itself, so not sure it's that
interesting to the project, but raising this anyway.
Agree, was trying to define these as defaults in soc Kconfig, but that
rearranged some values in other defconfigs so I avoided doing that in
this series. Also touching each defconfig helped me ping board
maintainers of the change.
Hopefully a follow-up series can move these, and other options, into
soc Kconfig.
I was more suggesting adding:
"""
diff --git a/Kconfig b/Kconfig
index 00ed1ecc173..b816e536674 100644
--- a/Kconfig
+++ b/Kconfig
@@ -249,6 +249,7 @@ config HAS_CUSTOM_SYS_INIT_SP_ADDR
config CUSTOM_SYS_INIT_SP_ADDR
hex "Static location for the initial stack pointer"
depends on HAS_CUSTOM_SYS_INIT_SP_ADDR
+ default 0x800000 if ROCKCHIP_RK3399
default TEXT_BASE if TFABOOT
config SYS_MALLOC_F
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 6405374bcc1..08b71713b79 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -121,6 +121,7 @@ config SPL_BSS_START_ADDR
default 0x81f80000 if ARCH_SUNXI && MACH_SUNIV
default 0x4ff80000 if ARCH_SUNXI && !(MACH_SUN9I || MACH_SUNIV)
default 0x2ff80000 if ARCH_SUNXI && MACH_SUN9I
+ default 0x02000000 if ROCKCHIP_RK3399
default 0x1000 if ARCH_ZYNQMP
choice
"""
So that boards could still override it in defconfigs if they really want
to. I don't have knowledge of any other way to provide an overridable
default?
Anyway, a topic for another series, nothing requiring to block this
series, so:
Reviewed-by: Quentin Schulz <quentin.sch...@theobroma-systems.com>
Thanks,
Quentin
