[PATCH 10/10] stm32mp1: spl: Copy optee nodes to target FDT for OP-TEE payloads

[Etienne Carriere]

Hello,

On Wed, 1 Sept 2021 at 17:10, Alex G. <mr.nuke.me at gmail.com> wrote:

> Hi Patrick,
>
> On 8/31/21 12:24 PM, Patrick DELAUNAY wrote:
> > Hi,
> >
> > On 8/26/21 11:42 PM, Alexandru Gagniuc wrote:
> >> OP-TEE does not take a devicetree for its own use. However, it does
> >> pass the devicetree to the normal world OS. In most cases that will
> >> be some other devicetree-bearing platform, such as linux.
> >>
> >> As in other cases where there's an OPTEE payload (e.g. BOOTM_OPTEE),
> >> it is required to copy the optee nodes to he target's FDT. Do this as
> >> part of spl_board_prepare_for_optee().
> >>
> >> Signed-off-by: Alexandru Gagniuc <mr.nuke.me at gmail.com>
> >> ---
> >>   arch/arm/mach-stm32mp/spl.c | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
> >> index d9fdc5926c..94fbb45cf9 100644
> >> --- a/arch/arm/mach-stm32mp/spl.c
> >> +++ b/arch/arm/mach-stm32mp/spl.c
> >> @@ -19,6 +19,7 @@
> >>   #include <asm/arch/sys_proto.h>
> >>   #include <mach/tzc.h>
> >>   #include <linux/libfdt.h>
> >> +#include <tee/optee.h>
> >>   u32 spl_boot_device(void)
> >>   {
> >> @@ -182,6 +183,7 @@ void stm32_init_tzc_for_optee(void)
> >>   void spl_board_prepare_for_optee(void *fdt)
> >>   {
> >>       stm32_fdt_setup_mac_addr(fdt);
> >> +    optee_copy_fdt_nodes(fdt);
> >>       stm32_init_tzc_for_optee();
> >>   }
> >
> >
> > NAK the OP-TEE nodes are ADDED by the OP-TEE firmware in the unsecure
> > device tree (when CFG_DT is activated)
> >
> > before to jump to the kernel (that remove the need to have DT
> > allignement with U-Boot SPL device tree)
> >
> > => SPL should not copy the OP-TEE nodes in next stage DT
> >
> > reference in optee_os = core/arch/arm/kernel/boot.c: add_optee_dt_node()
> >
> > add_optee_dt_node() <= update_external_dt() <= paged_init_primary()
> >
> > It is the expected configuration for OP-TEE on STM32MP15 platform.
>
> I agree that if a component modifies the platform, it should be
> responsible for updating the devicetree. Two issues though
>
>
> 1. Consistency
>
> The STM32IMAGE boot path expects u-boot to add the optee nodes, while
> the FIP path expects OP-TEE to add the nodes. Which one do we stick to?
>

U-boot already handles this, no? optee_copy_fdt_nodes() already skips
addition of optee nodes when it's already in.
So if OP-TEE has already filled the DT, u-boot won't add anything, which
looks good (to me).



>
>
> 2. Memory access
>
> I don't understand is how OP-TEE would get past the TZC.
> If we look at optee_os => plat-0stm32mp1/plat_tzc400:
>      "Early boot stage is in charge of configuring memory regions"
> The following memory has been set up by SPL (or TF-A for that matter):
>
>      Nonsec: c0000000->ddffffff
>      Sec:    de000000->dfdfffff
>      SHMEM:  dfe00000->dfffffff
>
> The external DTB will be in the nonsec region, which OP-TEE allegedly
> can't access. So how would this get patched?
>

OP-TEE secure world can access non-secure memory, provided it maps it as
non-secure memory.

regards,
etienne


> Alex
>