[PATCH] rockchip: sdram: fix DRAM bank declaration around OP-TEE

[Justin Swartz]

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 at 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;