* [PATCH 1/6] firmware: rockchip: rename from rk3xxx-op-tee.bin to rk3xxx-bl32.bin
2024-11-06 18:57 [PATCH 0/6] ARM: Rockchip: fix OP-TEE as BL32 installation Ahmad Fatoum
@ 2024-11-06 18:57 ` Ahmad Fatoum
2024-11-06 22:13 ` [PATCH] fixup! " Ahmad Fatoum
2024-11-06 18:57 ` [PATCH 2/6] ARM: lds: move stack top section to front of rodata Ahmad Fatoum
` (5 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Ahmad Fatoum @ 2024-11-06 18:57 UTC (permalink / raw)
To: barebox; +Cc: Robin van der Gracht, Ahmad Fatoum
Judging by the addresses used for locating OP-TEE, the OP-TEE support in
barebox could never have worked. This is also Sascha's recollection.
Follow-up patches will fix this, but lets set the stage by cleaning
up first and the future OP-TEE support for Rockchip:
- Have a single single option for OP-TEE support instead of one per SoC
- Rename the binaries to end in -bl32.bin, like they do on i.MX
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Documentation/boards/rockchip.rst | 2 +-
arch/arm/mach-rockchip/Kconfig | 14 ++++----------
arch/arm/mach-rockchip/atf.c | 8 ++++----
firmware/Makefile | 11 ++++++++---
4 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/Documentation/boards/rockchip.rst b/Documentation/boards/rockchip.rst
index aa2febc8ebe8..2d2e0572dd8f 100644
--- a/Documentation/boards/rockchip.rst
+++ b/Documentation/boards/rockchip.rst
@@ -76,7 +76,7 @@ The build process needs three binary files which have to be copied from the
.. code-block:: sh
cp $RKBIN/bin/rk35/rk3568_bl31_v1.34.elf firmware/rk3568-bl31.bin
- cp $RKBIN/bin/rk35/rk3568_bl32_v2.08.bin firmware/rk3568-op-tee.bin
+ cp $RKBIN/bin/rk35/rk3568_bl32_v2.08.bin firmware/rk3568-bl32.bin
cp $RKBIN/bin/rk35/rk3568_ddr_1560MHz_v1.13.bin arch/arm/boards/rockchip-rk3568-evb/sdram-init.bin
With these barebox can be compiled as:
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index ddaab7c28414..4069ad4ca138 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -135,18 +135,12 @@ config ARCH_ROCKCHIP_ATF
useful for debugging early startup, but for all other cases,
say y here.
-config ARCH_RK3399_OPTEE
- bool "Build rk3399 OP-TEE binary into barebox"
+config ARCH_ROCKCHIP_OPTEE
+ bool "Build rockchip OP-TEE binary into barebox"
depends on ARCH_ROCKCHIP_ATF && ARCH_RK3399
+ select PBL_OPTEE
help
- With this option enabled the RK3399 OP-TEE binary is compiled
- into barebox and started along with the BL31 trusted firmware.
-
-config ARCH_RK3568_OPTEE
- bool "Build rk3568 OP-TEE binary into barebox"
- depends on ARCH_ROCKCHIP_ATF && ARCH_RK3568
- help
- With this option enabled the RK3568 OP-TEE binary is compiled
+ With this option enabled the OP-TEE binary is compiled
into barebox and started along with the BL31 trusted firmware.
endmenu
diff --git a/arch/arm/mach-rockchip/atf.c b/arch/arm/mach-rockchip/atf.c
index eaba209ff32d..1e1861191c99 100644
--- a/arch/arm/mach-rockchip/atf.c
+++ b/arch/arm/mach-rockchip/atf.c
@@ -49,7 +49,7 @@ static unsigned long load_elf64_image_phdr(const void *elf)
\
bl31 = load_elf64_image_phdr(bl31_elf); \
\
- if (IS_ENABLED(CONFIG_ARCH_##SOC##_OPTEE)) { \
+ if (IS_ENABLED(CONFIG_ARCH_ROCKCHIP_OPTEE)) { \
optee_load_address = SOC##_OPTEE_LOAD_ADDRESS; \
\
get_builtin_firmware(tee_bin, &optee, &optee_size); \
@@ -68,12 +68,12 @@ static unsigned long load_elf64_image_phdr(const void *elf)
void rk3399_atf_load_bl31(void *fdt)
{
- rockchip_atf_load_bl31(RK3399, rk3399_bl31_bin, rk3399_op_tee_bin, fdt);
+ rockchip_atf_load_bl31(RK3399, rk3399_bl31_bin, rk3399_bl32_bin, fdt);
}
void rk3568_atf_load_bl31(void *fdt)
{
- rockchip_atf_load_bl31(RK3568, rk3568_bl31_bin, rk3568_op_tee_bin, fdt);
+ rockchip_atf_load_bl31(RK3568, rk3568_bl31_bin, rk3568_bl32_bin, fdt);
}
void __noreturn rk3568_barebox_entry(void *fdt)
@@ -107,7 +107,7 @@ void __noreturn rk3568_barebox_entry(void *fdt)
void rk3588_atf_load_bl31(void *fdt)
{
- rockchip_atf_load_bl31(RK3588, rk3588_bl31_bin, rk3588_op_tee_bin, fdt);
+ rockchip_atf_load_bl31(RK3588, rk3588_bl31_bin, rk3588_bl32_bin, fdt);
}
void __noreturn rk3588_barebox_entry(void *fdt)
diff --git a/firmware/Makefile b/firmware/Makefile
index fd63de1c559d..2f47e04bee41 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -24,11 +24,16 @@ fw-external-$(CONFIG_FIRMWARE_IMX93_OPTEE) += imx93-bl32.bin \
mx93a0-ahab-container.img \
mx93a1-ahab-container.img
pbl-firmware-$(CONFIG_ARCH_RK3568) += rk3568-bl31.bin
-pbl-firmware-$(CONFIG_ARCH_RK3568_OPTEE) += rk3568-op-tee.bin
pbl-firmware-$(CONFIG_ARCH_RK3588) += rk3588-bl31.bin
-pbl-firmware-$(CONFIG_ARCH_RK3588_OPTEE) += rk3588-op-tee.bin
pbl-firmware-$(CONFIG_ARCH_RK3399) += rk3399-bl31.bin
-pbl-firmware-$(CONFIG_ARCH_RK3399_OPTEE) += rk3399-op-tee.bin
+ifeq ($(CONFIG_ARCH_ROCKCHIP_OPTEE),y)
+# We install BL31 & BL32 while already running in DRAM,
+# so fw-external is not needed
+pbl-firmware-$(CONFIG_ARCH_RK3568) += rk3568-bl32.bin
+pbl-firmware-$(CONFIG_ARCH_RK3588) += rk3588-bl32.bin
+pbl-firmware-$(CONFIG_ARCH_RK3399) += rk3399-bl32.bin
+endif
+
firmware-$(CONFIG_DRIVER_NET_FSL_FMAN) += fsl_fman_ucode_ls1046_r1.0_106_4_18.bin
fw-external-$(CONFIG_FIRMWARE_LS1028A_ATF) += ls1028a-bl31.bin
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 2/6] ARM: lds: move stack top section to front of rodata
2024-11-06 18:57 [PATCH 0/6] ARM: Rockchip: fix OP-TEE as BL32 installation Ahmad Fatoum
2024-11-06 18:57 ` [PATCH 1/6] firmware: rockchip: rename from rk3xxx-op-tee.bin to rk3xxx-bl32.bin Ahmad Fatoum
@ 2024-11-06 18:57 ` Ahmad Fatoum
2024-11-06 18:57 ` [PATCH 3/6] ARM: Rockchip: fix clang warning about passing 32-bit register operand Ahmad Fatoum
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2024-11-06 18:57 UTC (permalink / raw)
To: barebox; +Cc: Robin van der Gracht, Ahmad Fatoum
The stack top is referenced from the entry point before relocation.
Depending on the firmware, .rodata can get quite big and this position-
independent relocation may lead to link errors:
(.text_head_prologue_common+0x4): relocation truncated to fit: R_AARCH64_ADR_PREL_LO21 against
symbol `__pbl_board_stack_top'
One way to workaround this is using fw-external, but if barebox is
running second stage anyway it is a hack having to use fw-external when
there is no chainloading of barebox proper involved anyway.
Let's avoid this linker error, by moving the stack top in front of
.rodata instead of behind it.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/lib/pbl.lds.S | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/lib/pbl.lds.S b/arch/arm/lib/pbl.lds.S
index 4f58a4eba0c5..f097ac1fdd46 100644
--- a/arch/arm/lib/pbl.lds.S
+++ b/arch/arm/lib/pbl.lds.S
@@ -61,9 +61,6 @@ SECTIONS
BAREBOX_BARE_INIT_SIZE
BAREBOX_PBL_SIZE
- . = ALIGN(4);
- .rodata : { *(.rodata*) }
-
. = ALIGN(ASM_SZPTR);
__pbl_board_stack_top = .;
.rodata.pbl_board_stack_top : {
@@ -73,6 +70,9 @@ SECTIONS
}
ASSERT(. - __pbl_board_stack_top <= 2 * ASM_SZPTR, "Only One PBL per Image allowed")
+ . = ALIGN(4);
+ .rodata : { *(.rodata*) }
+
.barebox_imd : { BAREBOX_IMD }
. = ALIGN(PBL_SEGMENT_ALIGN);
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 3/6] ARM: Rockchip: fix clang warning about passing 32-bit register operand
2024-11-06 18:57 [PATCH 0/6] ARM: Rockchip: fix OP-TEE as BL32 installation Ahmad Fatoum
2024-11-06 18:57 ` [PATCH 1/6] firmware: rockchip: rename from rk3xxx-op-tee.bin to rk3xxx-bl32.bin Ahmad Fatoum
2024-11-06 18:57 ` [PATCH 2/6] ARM: lds: move stack top section to front of rodata Ahmad Fatoum
@ 2024-11-06 18:57 ` Ahmad Fatoum
2024-11-07 8:42 ` Marco Felsch
2024-11-06 18:57 ` [PATCH 4/6] ARM: Rockchip: save pointer to scratch memory in global variable Ahmad Fatoum
` (3 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Ahmad Fatoum @ 2024-11-06 18:57 UTC (permalink / raw)
To: barebox; +Cc: Robin van der Gracht, Ahmad Fatoum
clangd warns that "Value size does not match register size specified by
the constraint and modifier".
Promote the subtraction result to unsigned long to fix this.
The 16 is likely unnecessary, but I am hesitant to remove it without testing.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/mach-rockchip/atf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/atf.c b/arch/arm/mach-rockchip/atf.c
index 1e1861191c99..4c16ec3bc66a 100644
--- a/arch/arm/mach-rockchip/atf.c
+++ b/arch/arm/mach-rockchip/atf.c
@@ -59,7 +59,7 @@ static unsigned long load_elf64_image_phdr(const void *elf)
\
/* Setup an initial stack for EL2 */ \
asm volatile("msr sp_el2, %0" : : \
- "r" (SOC##_BAREBOX_LOAD_ADDRESS - 16) : \
+ "r" ((ulong)SOC##_BAREBOX_LOAD_ADDRESS - 16) : \
"cc"); \
\
bl31_entry(bl31, optee_load_address, \
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/6] ARM: Rockchip: fix clang warning about passing 32-bit register operand
2024-11-06 18:57 ` [PATCH 3/6] ARM: Rockchip: fix clang warning about passing 32-bit register operand Ahmad Fatoum
@ 2024-11-07 8:42 ` Marco Felsch
2024-11-07 8:48 ` Ahmad Fatoum
0 siblings, 1 reply; 11+ messages in thread
From: Marco Felsch @ 2024-11-07 8:42 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox, Robin van der Gracht
Hi Ahmad,
On 24-11-06, Ahmad Fatoum wrote:
> clangd warns that "Value size does not match register size specified by
> the constraint and modifier".
>
> Promote the subtraction result to unsigned long to fix this.
>
> The 16 is likely unnecessary, but I am hesitant to remove it without testing.
Is it worth to add a comment like: /* TODO: check if 16 is necessary */ ?
Regards,
Marco
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> arch/arm/mach-rockchip/atf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-rockchip/atf.c b/arch/arm/mach-rockchip/atf.c
> index 1e1861191c99..4c16ec3bc66a 100644
> --- a/arch/arm/mach-rockchip/atf.c
> +++ b/arch/arm/mach-rockchip/atf.c
> @@ -59,7 +59,7 @@ static unsigned long load_elf64_image_phdr(const void *elf)
> \
> /* Setup an initial stack for EL2 */ \
> asm volatile("msr sp_el2, %0" : : \
> - "r" (SOC##_BAREBOX_LOAD_ADDRESS - 16) : \
> + "r" ((ulong)SOC##_BAREBOX_LOAD_ADDRESS - 16) : \
> "cc"); \
> \
> bl31_entry(bl31, optee_load_address, \
> --
> 2.39.5
>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/6] ARM: Rockchip: fix clang warning about passing 32-bit register operand
2024-11-07 8:42 ` Marco Felsch
@ 2024-11-07 8:48 ` Ahmad Fatoum
0 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2024-11-07 8:48 UTC (permalink / raw)
To: Marco Felsch; +Cc: barebox, Robin van der Gracht
Hello Marco,
On 07.11.24 09:42, Marco Felsch wrote:
> Hi Ahmad,
>
> On 24-11-06, Ahmad Fatoum wrote:
>> clangd warns that "Value size does not match register size specified by
>> the constraint and modifier".
>>
>> Promote the subtraction result to unsigned long to fix this.
>>
>> The 16 is likely unnecessary, but I am hesitant to remove it without testing.
>
> Is it worth to add a comment like: /* TODO: check if 16 is necessary */ ?
I am waiting for Sascha's opinion, we have similar subtractions in i.MX code too.
Cheers,
Ahmad
>
> Regards,
> Marco
>
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> arch/arm/mach-rockchip/atf.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-rockchip/atf.c b/arch/arm/mach-rockchip/atf.c
>> index 1e1861191c99..4c16ec3bc66a 100644
>> --- a/arch/arm/mach-rockchip/atf.c
>> +++ b/arch/arm/mach-rockchip/atf.c
>> @@ -59,7 +59,7 @@ static unsigned long load_elf64_image_phdr(const void *elf)
>> \
>> /* Setup an initial stack for EL2 */ \
>> asm volatile("msr sp_el2, %0" : : \
>> - "r" (SOC##_BAREBOX_LOAD_ADDRESS - 16) : \
>> + "r" ((ulong)SOC##_BAREBOX_LOAD_ADDRESS - 16) : \
>> "cc"); \
>> \
>> bl31_entry(bl31, optee_load_address, \
>> --
>> 2.39.5
>>
>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/6] ARM: Rockchip: save pointer to scratch memory in global variable
2024-11-06 18:57 [PATCH 0/6] ARM: Rockchip: fix OP-TEE as BL32 installation Ahmad Fatoum
` (2 preceding siblings ...)
2024-11-06 18:57 ` [PATCH 3/6] ARM: Rockchip: fix clang warning about passing 32-bit register operand Ahmad Fatoum
@ 2024-11-06 18:57 ` Ahmad Fatoum
2024-11-06 18:57 ` [PATCH 5/6] ARM: Rockchip: implement tee.bin v1 header parsing Ahmad Fatoum
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2024-11-06 18:57 UTC (permalink / raw)
To: barebox; +Cc: Robin van der Gracht, Ahmad Fatoum
This makes it easier to reference the scratch area in PBL without having
to compute the end of memory every time.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/mach-rockchip/atf.c | 22 ++++++++++++++--------
include/mach/rockchip/bootrom.h | 13 +++++++------
2 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-rockchip/atf.c b/arch/arm/mach-rockchip/atf.c
index 4c16ec3bc66a..2649b80af9e2 100644
--- a/arch/arm/mach-rockchip/atf.c
+++ b/arch/arm/mach-rockchip/atf.c
@@ -12,6 +12,8 @@
#include <mach/rockchip/rk3568-regs.h>
#include <mach/rockchip/rk3588-regs.h>
+struct rockchip_scratch_space *rk_scratch;
+
static unsigned long load_elf64_image_phdr(const void *elf)
{
const Elf64_Ehdr *ehdr; /* Elf header structure pointer */
@@ -78,14 +80,16 @@ void rk3568_atf_load_bl31(void *fdt)
void __noreturn rk3568_barebox_entry(void *fdt)
{
- unsigned long membase, memsize;
+ unsigned long membase, endmem;
membase = RK3568_DRAM_BOTTOM;
- memsize = rk3568_ram0_size() - RK3568_DRAM_BOTTOM;
+ endmem = rk3568_ram0_size();
+
+ rk_scratch = (void *)arm_mem_scratch(endmem);
if (current_el() == 3) {
rk3568_lowlevel_init();
- rockchip_store_bootrom_iram(membase, memsize, IOMEM(RK3568_IRAM_BASE));
+ rockchip_store_bootrom_iram(IOMEM(RK3568_IRAM_BASE));
/*
* The downstream TF-A doesn't cope with our device tree when
@@ -102,7 +106,7 @@ void __noreturn rk3568_barebox_entry(void *fdt)
/* not reached when CONFIG_ARCH_ROCKCHIP_ATF */
}
- barebox_arm_entry(membase, memsize, fdt);
+ barebox_arm_entry(membase, endmem - membase, fdt);
}
void rk3588_atf_load_bl31(void *fdt)
@@ -112,14 +116,16 @@ void rk3588_atf_load_bl31(void *fdt)
void __noreturn rk3588_barebox_entry(void *fdt)
{
- unsigned long membase, memsize;
+ unsigned long membase, endmem;
membase = RK3588_DRAM_BOTTOM;
- memsize = rk3588_ram0_size() - RK3588_DRAM_BOTTOM;
+ endmem = rk3588_ram0_size();
+
+ rk_scratch = (void *)arm_mem_scratch(endmem);
if (current_el() == 3) {
rk3588_lowlevel_init();
- rockchip_store_bootrom_iram(membase, memsize, IOMEM(RK3588_IRAM_BASE));
+ rockchip_store_bootrom_iram(IOMEM(RK3588_IRAM_BASE));
/*
* The downstream TF-A doesn't cope with our device tree when
@@ -136,5 +142,5 @@ void __noreturn rk3588_barebox_entry(void *fdt)
/* not reached when CONFIG_ARCH_ROCKCHIP_ATF */
}
- barebox_arm_entry(membase, memsize, fdt);
+ barebox_arm_entry(membase, endmem - membase, fdt);
}
diff --git a/include/mach/rockchip/bootrom.h b/include/mach/rockchip/bootrom.h
index 5b999fc6068a..06450829b99a 100644
--- a/include/mach/rockchip/bootrom.h
+++ b/include/mach/rockchip/bootrom.h
@@ -5,23 +5,24 @@
#include <linux/compiler.h>
#include <linux/string.h>
+#include <pbl.h>
#include <asm/barebox-arm.h>
struct rockchip_scratch_space {
u32 irom[16];
};
-static inline void rockchip_store_bootrom_iram(ulong membase,
- ulong memsize,
- const void *iram)
+extern struct rockchip_scratch_space *rk_scratch;
+
+static inline void rockchip_store_bootrom_iram(const void *iram)
{
- void *dst = (void *)arm_mem_scratch(membase + memsize);
- memcpy(dst, iram, sizeof(struct rockchip_scratch_space));
+ if (rk_scratch)
+ memcpy(rk_scratch, iram, sizeof(struct rockchip_scratch_space));
}
static inline const struct rockchip_scratch_space *rockchip_scratch_space(void)
{
- return arm_mem_scratch_get();
+ return IN_PBL ? rk_scratch : arm_mem_scratch_get();
}
void rockchip_parse_bootrom_iram(const void *iram);
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 5/6] ARM: Rockchip: implement tee.bin v1 header parsing
2024-11-06 18:57 [PATCH 0/6] ARM: Rockchip: fix OP-TEE as BL32 installation Ahmad Fatoum
` (3 preceding siblings ...)
2024-11-06 18:57 ` [PATCH 4/6] ARM: Rockchip: save pointer to scratch memory in global variable Ahmad Fatoum
@ 2024-11-06 18:57 ` Ahmad Fatoum
2024-11-06 18:57 ` [PATCH 6/6] ARM: Rockchip: use vendor blob OPTEE load addresses Ahmad Fatoum
2024-11-08 10:20 ` [PATCH 0/6] ARM: Rockchip: fix OP-TEE as BL32 installation Sascha Hauer
6 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2024-11-06 18:57 UTC (permalink / raw)
To: barebox; +Cc: Robin van der Gracht, Ahmad Fatoum
Building OP-TEE produces a number of binaries among them tee.elf,
tee.bin and tee-raw.bin.
So far, we assumed in Rockchip code that the binary provided by the user
to be built into barebox is tee-raw.bin.
That binary is installed as-is to address 0x200000, which is not
generally applicable:
- At least in upstream OP-TEE, the upstream RK3399 support uses
0x30000000 as base address
- Even if OP-TEE didn't hang, 0x200000 is likely not at the end
of RAM, but that's where barebox assumes OP-TEE is by default,
and thus barebox would carve out unneeded reserved memory.
The tee.bin on the other hand has an OP-TEE header in front of it, which
our i.MX OP-TEE support already uses to identify where to load the binary
too and how much memory to reserve.
Let's do the same for Rockchip and check if have an OP-TEE header and
use that to inform our placement of OP-TEE and reservation of memory in
barebox itself and in the reserved memory entries that will be fixed up
for Linux.
As OP-TEE support is added anew, we also make sure to always have
a OP-TEE header for barebox proper to consult and we fake one if
there isn't any.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/mach-rockchip/atf.c | 74 +++++++++++++++++++++++++++++--
arch/arm/mach-rockchip/rockchip.c | 16 +++++++
include/mach/rockchip/atf.h | 15 ++++++-
include/mach/rockchip/bootrom.h | 17 +++++++
4 files changed, 117 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-rockchip/atf.c b/arch/arm/mach-rockchip/atf.c
index 2649b80af9e2..86a28c51016b 100644
--- a/arch/arm/mach-rockchip/atf.c
+++ b/arch/arm/mach-rockchip/atf.c
@@ -4,8 +4,10 @@
#include <asm/system.h>
#include <mach/rockchip/atf.h>
#include <elf.h>
+#include <tee/optee.h>
#include <asm/atf_common.h>
#include <asm/barebox-arm.h>
+#include <asm-generic/memory_layout.h>
#include <mach/rockchip/dmc.h>
#include <mach/rockchip/rockchip.h>
#include <mach/rockchip/bootrom.h>
@@ -14,6 +16,17 @@
struct rockchip_scratch_space *rk_scratch;
+static void rk_scratch_save_optee_hdr(const struct optee_header *hdr)
+{
+ if (!rk_scratch) {
+ pr_err("No scratch area initialized, skip saving optee-hdr");
+ return;
+ }
+
+ pr_debug("Saving optee-hdr to scratch area 0x%p\n", &rk_scratch->optee_hdr);
+ rk_scratch->optee_hdr = *hdr;
+}
+
static unsigned long load_elf64_image_phdr(const void *elf)
{
const Elf64_Ehdr *ehdr; /* Elf header structure pointer */
@@ -41,6 +54,59 @@ static unsigned long load_elf64_image_phdr(const void *elf)
return ehdr->e_entry;
}
+static uintptr_t rk_load_optee(uintptr_t bl32, const void *bl32_image,
+ size_t bl32_size)
+{
+ const struct optee_header *hdr = bl32_image;
+ struct optee_header dummy_hdr;
+
+ /* We already have ELF support for BL31, but adding it for BL32,
+ * would require us to identify a range that fits all ELF
+ * sections and fake a dummy OP-TEE header that describes it.
+ * This is doable, but let's postpone that until there is an
+ * actual user interested in this.
+ */
+ BUG_ON(memcmp(bl32_image, ELFMAG, 4) == 0);
+
+ if (optee_verify_header(hdr) == 0) {
+ bl32_size -= sizeof(*hdr);
+ bl32_image += sizeof(*hdr);
+
+ bl32 = (u64)hdr->init_load_addr_hi << 32;
+ bl32 |= hdr->init_load_addr_lo;
+
+ pr_debug("optee: adjusting address to 0x%lx\n", bl32);
+ } else if (bl32 != ROCKCHIP_OPTEE_HEADER_REQUIRED) {
+ dummy_hdr.magic = OPTEE_MAGIC;
+ dummy_hdr.version = OPTEE_VERSION_V1;
+ dummy_hdr.arch = OPTEE_ARCH_ARM64;
+ dummy_hdr.flags = 0;
+ dummy_hdr.init_size = bl32_size;
+ dummy_hdr.init_load_addr_hi = upper_32_bits(bl32);
+ dummy_hdr.init_load_addr_lo = lower_32_bits(bl32);
+ dummy_hdr.init_mem_usage = 0;
+ dummy_hdr.paged_size = 0;
+
+ hdr = &dummy_hdr;
+
+ pr_debug("optee: assuming load address is 0x%lx\n", bl32);
+
+ } else {
+ /* If we have neither a header, nor a defined load address
+ * there is really nothing we can do here.
+ */
+ pr_err("optee: skipping. No header and no hardcoded load address\n");
+ return 0;
+ }
+
+ rk_scratch_save_optee_hdr(hdr);
+
+
+ memcpy((void *)bl32, bl32_image, bl32_size);
+
+ return bl32;
+}
+
#define rockchip_atf_load_bl31(SOC, atf_bin, tee_bin, fdt) do { \
const void *bl31_elf, *optee; \
unsigned long bl31; \
@@ -52,11 +118,9 @@ static unsigned long load_elf64_image_phdr(const void *elf)
bl31 = load_elf64_image_phdr(bl31_elf); \
\
if (IS_ENABLED(CONFIG_ARCH_ROCKCHIP_OPTEE)) { \
- optee_load_address = SOC##_OPTEE_LOAD_ADDRESS; \
- \
get_builtin_firmware(tee_bin, &optee, &optee_size); \
- \
- memcpy((void *)optee_load_address, optee, optee_size); \
+ optee_load_address = rk_load_optee(SOC##_OPTEE_LOAD_ADDRESS, \
+ optee, optee_size); \
} \
\
/* Setup an initial stack for EL2 */ \
@@ -106,6 +170,7 @@ void __noreturn rk3568_barebox_entry(void *fdt)
/* not reached when CONFIG_ARCH_ROCKCHIP_ATF */
}
+ optee_set_membase(rk_scratch_get_optee_hdr());
barebox_arm_entry(membase, endmem - membase, fdt);
}
@@ -142,5 +207,6 @@ void __noreturn rk3588_barebox_entry(void *fdt)
/* not reached when CONFIG_ARCH_ROCKCHIP_ATF */
}
+ optee_set_membase(rk_scratch_get_optee_hdr());
barebox_arm_entry(membase, endmem - membase, fdt);
}
diff --git a/arch/arm/mach-rockchip/rockchip.c b/arch/arm/mach-rockchip/rockchip.c
index 1c962ad8c89f..3d2ef791d20f 100644
--- a/arch/arm/mach-rockchip/rockchip.c
+++ b/arch/arm/mach-rockchip/rockchip.c
@@ -1,7 +1,10 @@
// SPDX-License-Identifier: GPL-2.0-only
#include <common.h>
#include <init.h>
+#include <asm/optee.h>
+#include <asm-generic/memory_layout.h>
#include <mach/rockchip/rockchip.h>
+#include <mach/rockchip/bootrom.h>
static int __rockchip_soc;
@@ -26,6 +29,19 @@ int rockchip_soc(void)
static int rockchip_init(void)
{
+ const struct optee_header *hdr = rk_scratch_get_optee_hdr();
+
+ if (IS_ENABLED(CONFIG_PBL_OPTEE) && optee_verify_header(hdr) == 0) {
+ static struct of_optee_fixup_data optee_fixup_data = {
+ .shm_size = OPTEE_SHM_SIZE,
+ .method = "smc",
+ };
+
+ optee_set_membase(hdr);
+ of_optee_fixup(of_get_root_node(), &optee_fixup_data);
+ of_register_fixup(of_optee_fixup, &optee_fixup_data);
+ }
+
switch (rockchip_soc()) {
case 3188:
return rk3188_init();
diff --git a/include/mach/rockchip/atf.h b/include/mach/rockchip/atf.h
index 89129abc01b9..1b07128f2e9b 100644
--- a/include/mach/rockchip/atf.h
+++ b/include/mach/rockchip/atf.h
@@ -8,11 +8,24 @@
#define RK3568_DRAM_BOTTOM 0xa00000
#define RK3588_DRAM_BOTTOM 0xa00000
-/* OP-TEE expects to be loaded here */
+/*
+ * The tee.bin image has an OP-TEE specific header that describes the
+ * initial load address and size. Unfortunately, the vendor blobs are in the
+ * tee-raw.bin format, which omits the header. We thus hardcode here the
+ * fallback addresses that should be used when barebox encounters
+ * tee-raw.bin instead of tee.bin.
+ */
#define RK3399_OPTEE_LOAD_ADDRESS 0x200000
#define RK3568_OPTEE_LOAD_ADDRESS 0x200000
#define RK3588_OPTEE_LOAD_ADDRESS 0x200000
+/*
+ * Hopefully for future platforms, the vendor binaries would use the image
+ * with an OP-TEE header and we can just set the load address for new SoCs
+ * to below macro to enforce that only tee.bin is used.
+ */
+#define ROCKCHIP_OPTEE_HEADER_REQUIRED 0
+
/*
* board lowlevel code should relocate barebox here. This is where
* OP-TEE jumps to after initialization.
diff --git a/include/mach/rockchip/bootrom.h b/include/mach/rockchip/bootrom.h
index 06450829b99a..6776ac5ef981 100644
--- a/include/mach/rockchip/bootrom.h
+++ b/include/mach/rockchip/bootrom.h
@@ -7,9 +7,12 @@
#include <linux/string.h>
#include <pbl.h>
#include <asm/barebox-arm.h>
+#include <tee/optee.h>
+#include <pbl.h>
struct rockchip_scratch_space {
u32 irom[16];
+ struct optee_header optee_hdr;
};
extern struct rockchip_scratch_space *rk_scratch;
@@ -29,5 +32,19 @@ void rockchip_parse_bootrom_iram(const void *iram);
int rockchip_bootsource_get_active_slot(void);
+static inline const struct optee_header *rk_scratch_get_optee_hdr(void)
+{
+ struct rockchip_scratch_space *scratch;
+
+ if (IN_PBL)
+ scratch = rk_scratch;
+ else
+ scratch = (void *)arm_mem_scratch_get();
+
+ if (!scratch)
+ return ERR_PTR(-EINVAL);
+
+ return &scratch->optee_hdr;
+}
#endif
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 6/6] ARM: Rockchip: use vendor blob OPTEE load addresses
2024-11-06 18:57 [PATCH 0/6] ARM: Rockchip: fix OP-TEE as BL32 installation Ahmad Fatoum
` (4 preceding siblings ...)
2024-11-06 18:57 ` [PATCH 5/6] ARM: Rockchip: implement tee.bin v1 header parsing Ahmad Fatoum
@ 2024-11-06 18:57 ` Ahmad Fatoum
2024-11-08 10:20 ` [PATCH 0/6] ARM: Rockchip: fix OP-TEE as BL32 installation Sascha Hauer
6 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2024-11-06 18:57 UTC (permalink / raw)
To: barebox; +Cc: Robin van der Gracht, Ahmad Fatoum
It's unknown where the 0x200000 came from, but it's most likely wrong.
- For the RK3399, the upstream default is 0x30000000 and
the vendor load address in rkbin/RKTRUST/RK3399TRUST.ini is
0x08400000 for the blob.
- For the RK3568, there's no upstream support and for both RK35{68,88}
the vendor load address in rkbin/RKTRUST/RK356xTRUST.ini is
0x08400000 for the blob.
The hardcoded OP-TEE load address in barebox is only relevant, when the
OP-TEE header is missing. Users compiling OP-TEE themselves can use the
tee.bin and ignore the hardcoded value altogether.
Users using the blob don't have the header, so it's a good trade off to
use the vendor load addresses as default addresses.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Documentation/boards/rockchip.rst | 9 ++++++---
include/mach/rockchip/atf.h | 8 +++++---
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/Documentation/boards/rockchip.rst b/Documentation/boards/rockchip.rst
index 2d2e0572dd8f..b94c7a78e115 100644
--- a/Documentation/boards/rockchip.rst
+++ b/Documentation/boards/rockchip.rst
@@ -86,10 +86,13 @@ With these barebox can be compiled as:
make ARCH=arm rockchip_v8_defconfig
make ARCH=arm
-**NOTE** I found the bl32 firmware non working for me as of 7d631e0d5b2d373b54d4533580d08fb9bd2eaad4 in the rkbin repository.
+.. note:: When compiling OP-TEE yourself, use the tee.bin image as it has
+ a header telling barebox where to load the image to.
+ Without the header, barebox will fallback to the hardcoded addresses expected
+ by the vendor blobs in the rkbin repository.
-**NOTE** The RK3566 and RK3568 seem to share the bl31 and bl32 firmware files,
-whereas the memory initialization blob is different.
+.. note:: The RK3566 and RK3568 seem to share the bl31 and bl32 firmware files,
+ whereas the memory initialization blob is different.
Creating a bootable SD card
---------------------------
diff --git a/include/mach/rockchip/atf.h b/include/mach/rockchip/atf.h
index 1b07128f2e9b..db00dc71195f 100644
--- a/include/mach/rockchip/atf.h
+++ b/include/mach/rockchip/atf.h
@@ -14,10 +14,12 @@
* tee-raw.bin format, which omits the header. We thus hardcode here the
* fallback addresses that should be used when barebox encounters
* tee-raw.bin instead of tee.bin.
+ *
+ * The values are taken from rkbin/RKTRUST/RK3*.ini: [BL32_OPTION] ADDR
*/
-#define RK3399_OPTEE_LOAD_ADDRESS 0x200000
-#define RK3568_OPTEE_LOAD_ADDRESS 0x200000
-#define RK3588_OPTEE_LOAD_ADDRESS 0x200000
+#define RK3399_OPTEE_LOAD_ADDRESS 0x8400000
+#define RK3568_OPTEE_LOAD_ADDRESS 0x8400000
+#define RK3588_OPTEE_LOAD_ADDRESS 0x8400000
/*
* Hopefully for future platforms, the vendor binaries would use the image
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 0/6] ARM: Rockchip: fix OP-TEE as BL32 installation
2024-11-06 18:57 [PATCH 0/6] ARM: Rockchip: fix OP-TEE as BL32 installation Ahmad Fatoum
` (5 preceding siblings ...)
2024-11-06 18:57 ` [PATCH 6/6] ARM: Rockchip: use vendor blob OPTEE load addresses Ahmad Fatoum
@ 2024-11-08 10:20 ` Sascha Hauer
6 siblings, 0 replies; 11+ messages in thread
From: Sascha Hauer @ 2024-11-08 10:20 UTC (permalink / raw)
To: barebox, Ahmad Fatoum; +Cc: Robin van der Gracht
On Wed, 06 Nov 2024 19:57:18 +0100, Ahmad Fatoum wrote:
> This series resulted while getting OP-TEE to work on RK3399.
> There is no upstream OP-TEE support for the RK3568 or RK3588,
> but the expectation that loading the blob is analogous to the
> RK3399, which is fixed by this series.
>
> Ahmad Fatoum (6):
> firmware: rockchip: rename from rk3xxx-op-tee.bin to rk3xxx-bl32.bin
> ARM: lds: move stack top section to front of rodata
> ARM: Rockchip: fix clang warning about passing 32-bit register operand
> ARM: Rockchip: save pointer to scratch memory in global variable
> ARM: Rockchip: implement tee.bin v1 header parsing
> ARM: Rockchip: use vendor blob OPTEE load addresses
>
> [...]
Applied, thanks!
[1/6] firmware: rockchip: rename from rk3xxx-op-tee.bin to rk3xxx-bl32.bin
https://git.pengutronix.de/cgit/barebox/commit/?id=c8d797dad31a (link may not be stable)
[2/6] ARM: lds: move stack top section to front of rodata
https://git.pengutronix.de/cgit/barebox/commit/?id=3def2f136aea (link may not be stable)
[3/6] ARM: Rockchip: fix clang warning about passing 32-bit register operand
https://git.pengutronix.de/cgit/barebox/commit/?id=9cd71fc171b6 (link may not be stable)
[4/6] ARM: Rockchip: save pointer to scratch memory in global variable
https://git.pengutronix.de/cgit/barebox/commit/?id=73a2b3c8d293 (link may not be stable)
[5/6] ARM: Rockchip: implement tee.bin v1 header parsing
https://git.pengutronix.de/cgit/barebox/commit/?id=e1ecb0bdeeed (link may not be stable)
[6/6] ARM: Rockchip: use vendor blob OPTEE load addresses
https://git.pengutronix.de/cgit/barebox/commit/?id=d857e9ea91fd (link may not be stable)
Best regards,
--
Sascha Hauer <s.hauer@pengutronix.de>
^ permalink raw reply [flat|nested] 11+ messages in thread