<div dir="ltr"><div dir="ltr">Hello,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 1 Sept 2021 at 17:10, Alex G. <<a href="mailto:mr.nuke.me@gmail.com">mr.nuke.me@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Patrick,<br>
<br>
On 8/31/21 12:24 PM, Patrick DELAUNAY wrote:<br>
> Hi,<br>
> <br>
> On 8/26/21 11:42 PM, Alexandru Gagniuc wrote:<br>
>> OP-TEE does not take a devicetree for its own use. However, it does<br>
>> pass the devicetree to the normal world OS. In most cases that will<br>
>> be some other devicetree-bearing platform, such as linux.<br>
>><br>
>> As in other cases where there's an OPTEE payload (e.g. BOOTM_OPTEE),<br>
>> it is required to copy the optee nodes to he target's FDT. Do this as<br>
>> part of spl_board_prepare_for_optee().<br>
>><br>
>> Signed-off-by: Alexandru Gagniuc <<a href="mailto:mr.nuke.me@gmail.com" target="_blank">mr.nuke.me@gmail.com</a>><br>
>> ---<br>
>>   arch/arm/mach-stm32mp/spl.c | 2 ++<br>
>>   1 file changed, 2 insertions(+)<br>
>><br>
>> diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c<br>
>> index d9fdc5926c..94fbb45cf9 100644<br>
>> --- a/arch/arm/mach-stm32mp/spl.c<br>
>> +++ b/arch/arm/mach-stm32mp/spl.c<br>
>> @@ -19,6 +19,7 @@<br>
>>   #include <asm/arch/sys_proto.h><br>
>>   #include <mach/tzc.h><br>
>>   #include <linux/libfdt.h><br>
>> +#include <tee/optee.h><br>
>>   u32 spl_boot_device(void)<br>
>>   {<br>
>> @@ -182,6 +183,7 @@ void stm32_init_tzc_for_optee(void)<br>
>>   void spl_board_prepare_for_optee(void *fdt)<br>
>>   {<br>
>>       stm32_fdt_setup_mac_addr(fdt);<br>
>> +    optee_copy_fdt_nodes(fdt);<br>
>>       stm32_init_tzc_for_optee();<br>
>>   }<br>
> <br>
> <br>
> NAK the OP-TEE nodes are ADDED by the OP-TEE firmware in the unsecure <br>
> device tree (when CFG_DT is activated)<br>
> <br>
> before to jump to the kernel (that remove the need to have DT <br>
> allignement with U-Boot SPL device tree)<br>
> <br>
> => SPL should not copy the OP-TEE nodes in next stage DT<br>
> <br>
> reference in optee_os = core/arch/arm/kernel/boot.c: add_optee_dt_node()<br>
> <br>
> add_optee_dt_node() <= update_external_dt() <= paged_init_primary()<br>
> <br>
> It is the expected configuration for OP-TEE on STM32MP15 platform.<br>
<br>
I agree that if a component modifies the platform, it should be <br>
responsible for updating the devicetree. Two issues though<br>
<br>
<br>
1. Consistency<br>
<br>
The STM32IMAGE boot path expects u-boot to add the optee nodes, while <br>
the FIP path expects OP-TEE to add the nodes. Which one do we stick to?<br></blockquote><div><br></div><div>U-boot already handles this, no? optee_copy_fdt_nodes() already skips addition of optee nodes when it's already in.</div><div>So if OP-TEE has already filled the DT, u-boot won't add anything, which looks good (to me).</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
2. Memory access<br>
<br>
I don't understand is how OP-TEE would get past the TZC.<br>
If we look at optee_os => plat-0stm32mp1/plat_tzc400:<br>
     "Early boot stage is in charge of configuring memory regions"<br>
The following memory has been set up by SPL (or TF-A for that matter):<br>
<br>
     Nonsec: c0000000->ddffffff<br>
     Sec:    de000000->dfdfffff<br>
     SHMEM:  dfe00000->dfffffff<br>
<br>
The external DTB will be in the nonsec region, which OP-TEE allegedly <br>
can't access. So how would this get patched?<br></blockquote><div><br></div><div>OP-TEE secure world can access non-secure memory, provided it maps it as non-secure memory. </div><div><br></div><div>regards,</div><div>etienne</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Alex<br>
</blockquote></div></div>