Hi Kever,

On 2020-04-17 08:48, Kever Yang wrote:

Hi Justin,

On 2020/4/1 上午1:25, Justin Swartz wrote:

...

If OP-TEE is configured, it makes sense to use CONFIG_OPTEE_TZDRAM_BASE and CONFIG_OPTEE_TZDRAM_SIZE to declare the boundaries of the TrustZone memory reserved for OP-TEE instead of assuming that a 32MB reservation is always in place.

In this case, the following calculations may be used to determine the boundaries of DRAM bank 0 and 1 which surround the TrustZone reservation:

[DRAM bank 0] base = CONFIG_SYS_DRAM_BASE size = CONFIG_OPTEE_TZDRAM_BASE - CONFIG_SYS_SDRAM_BASE

[DRAM bank 1] base = CONFIG_OPTEE_TZDRAM_BASE + CONFIG_OPTEE_TZDRAM_SIZE size = top of memory - base of DRAM bank 1

We do not use CONFIG_OPTEE_TZDRAM_BASE and code in lib/optee/ for rockchip platform now, and this patch update to use this macro without adapt other code will break the boards already run with it.

Considering that this block of code is guarded by "#ifdef CONFIG_SPL_OPTEE", why wouldn't it make sense to use the pair of CONFIG_OPTEE_TZDRAM_BASE and CONFIG_OPTEE_TZDRAM_SIZE pair to define the boundaries of the TrustZone reservation?

Surely not every configuration requires a 32MB reservation at a fixed offset?

I am curious to know which boards will break with these changes applied considering that the current calculation of the DRAM bank 1 size appears to be incorrect.

From an RK3229 based device, with 1024MB of RAM, here is an excerpt from the output of the bdinfo command prior to applying the patch:

=> bdinfo arch_number = 0x00000000 boot_params = 0x00000000 DRAM bank = 0x00000000 -> start = 0x60000000 -> size = 0x08400000 DRAM bank = 0x00000001 -> start = 0x6a400000 -> size = 0x95c00000 baudrate = 1500000 bps TLB addr = 0x9fff0000 relocaddr = 0x9ff7c000 reloc off = 0x3ef7c000 irq_sp = 0x9df6d040 sp start = 0x9df6d030 Early malloc usage: 6c4 / 800 fdt_blob = 0x9df6d058

And after applying the patch:

=> bdinfo arch_number = 0x00000000 boot_params = 0x00000000 DRAM bank = 0x00000000 -> start = 0x60000000 -> size = 0x08400000 DRAM bank = 0x00000001 -> start = 0x68600000 -> size = 0x37a00000 baudrate = 1500000 bps TLB addr = 0x9fff0000 relocaddr = 0x9ff81000 reloc off = 0x3ef81000 irq_sp = 0x9df71580 sp start = 0x9df71570 Early malloc usage: 6ac / 800 fdt_blob = 0x9df71598

Notice the difference in reported DRAM bank 1 sizes, and the lower DRAM bank 1 starting offset after patching.

As much as I might wish that this device has 0x95c00000 bytes (+-2396MB) to spare beyond OP-TEE, it really doesn't. :)

Kind Regards, Justin

Thanks,

• Kever

...

Signed-off-by: Justin Swartz justin.swartz@risingedge.co.za

arch/arm/mach-rockchip/sdram.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c index 530644c043..def2c23294 100644 --- a/arch/arm/mach-rockchip/sdram.c +++ b/arch/arm/mach-rockchip/sdram.c @@ -55,16 +55,14 @@ int dram_init_banksize(void)

• CONFIG_SYS_SDRAM_BASE;

gd->bd->bi_dram[1].start = tos_parameter->tee_mem.phy_addr + tos_parameter->tee_mem.size;

•    gd->bd->bi_dram[1].size = gd->bd->bi_dram[0].start
  

•                + top - gd->bd->bi_dram[1].start;
  

•    gd->bd->bi_dram[1].size = top - gd->bd->bi_dram[1].start;
  

} else { gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;

•    gd->bd->bi_dram[0].size = 0x8400000;
  

•    /* Reserve 32M for OPTEE with TA */
  

•    gd->bd->bi_dram[1].start = CONFIG_SYS_SDRAM_BASE
  

•                + gd->bd->bi_dram[0].size + 0x2000000;
  

•    gd->bd->bi_dram[1].size = gd->bd->bi_dram[0].start
  

•                + top - gd->bd->bi_dram[1].start;
  

•    gd->bd->bi_dram[0].size = CONFIG_OPTEE_TZDRAM_BASE
  

•                - CONFIG_SYS_SDRAM_BASE;
  

•    gd->bd->bi_dram[1].start = CONFIG_OPTEE_TZDRAM_BASE
  

•                + CONFIG_OPTEE_TZDRAM_SIZE;
  

•    gd->bd->bi_dram[1].size = top - gd->bd->bi_dram[1].start;
  

} #else gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;