Re: [PATCH v3 4/5] stm32mp1: spl: Configure TrustZone controller for OP-TEE

["Alex G." <mr.nuke.me@gmail.com>]

On 5/28/21 5:22 AM, Patrick DELAUNAY wrote:
> Hi,
> 
> On 4/15/21 6:48 PM, Alexandru Gagniuc wrote:

[snip]

>> +    fdt_start = ofnode_get_addr_size(node, "reg", size);
> 
> warning here because size is 'fdt_size_t *' not 'u32*'
> 
> arch/arm/mach-stm32mp/spl.c:122:48: warning: passing argument 3 of 
> ‘ofnode_get_addr_size’ from inc
> 

Fixed in v4

>> +
>> +    tzc_configure(tzc, optee_config);
>> +    tzc_dump_config(tzc);
> Dump is always require, even for nomal boot, or only for debug cases ?

Because tzc_dump_config() uses log_info(), only prints when the 
appropriate log level is enabled. This is by design, so that we don't 
need extra logic here to invoke dump().

>> +
>> +    dcache_disable();
> 
> 
> You disable cache why,
> 
> it is not supported by OP-TEE ?
>
> => if it is a generic issue it should be in spl.c for case IH_OS_TEE or in
> 
>        spl_optee_entry() and not in board specific weak function
> 
>        as it done in bl31_entry for TF-A


I don't know if this can be safely generalized. stm32mp is the only 
platform to enable TZC in SPL, which brings special constraints:

We're running in secure mode, and we've touched memory that will be 
reserved to the normal world, for example the linux devicetree. Once we 
enable TZC, we can't touch that memory anymore. This could happen later 
as the CPU is evicting cache lines.

To make sure we don't hit a TZC violation as cache lines are evicted, we 
both flush the dcache, and disable the dcache. Doing this another way is 
not tested, and I can't guarantee that other variations will work reliably.

Alex