Hi Alexandru,

On 9/9/21 4:55 PM, Alexandru Gagniuc wrote:
u-boot v2021.10-rc2 Introduced support for booting from FIP images
(not to be confused with FIT) for stm32mp. I am also working on a
different boot flow based on u-boot's falcon mode. The changes to
accommodate the FIP mode inadvertently broke the falcon flow. This
series intends to address that.

The core issue is that optee nodes are removed from the u-boot
devicetree. For reasons detailed in my other series
("[PATCH v2 00/11] stm32mp1: Support falcon mode with OP-TEE payloads")
the "optee" nodes are required.

It might seem like an obvious solution to "just re-add the nodes", but
I found out it didn't work like that. I couldn't use
STM32MP15x_STM32IMAGE to get me back my nodes, because that would have
required TFABOOT. What I needed was a new config that more accuratelyreflected 
the available boot flows.

STM32MP15x_STM32IMAGE is a confusing because it conflates image
generation with u-boot behavior. I'm proposing replacing it with
TFABOOT_FIP_CONTAINER because I think this new config is much easier
to understand in layman's terms. I also thinks it maps more elegantly
to what STM is trying to do: add a new boot flow.

Again, I don't add a new boot flow BUT the support of the default

container = the FIP for the existing TFA BOOT flow .


It is the SAME boot flow, previously named "trusted"


TF-A (BL2) => secure monitor => U-Boot => kernel


And we have also 2 variant here: the secure monitor can be OP-TEE or SP_MIN


The only difference is the containers used by TF-A BL2 (FSBL) to load

the next stage (secure monitor / SSBL) and the DT is provided to U-Boot by TF-A/OP-TEE.

And booth container can be used with  OP-TEE or SP_MIN.


In the first support of STM32MP15 in TF-A, we use the STM32IMAGE container (several files *.stm32)

but after some requests (PKI) and evolution and to avoid limitation (SYSRAM size)

we conclude it was a error to a use this proprietary format after TF-A BL2.


We keep this format only for ROM code, for the first boot stage loader = TF-A BL2 or SPL.


Now we are migrating the "trusted" boot (with TF-A boot) to use the FIP

container.


So the usage of STM32IMAGE container for U-Boot is STM32MP15x specific

and the FIP is the default container for TFABOOT

(for me no need to specific CONFIG to managed generic behavior),


It is strange to add a config for all board (so always activated), to solve a problem

only present in the STM32MP platform because we badly support TFA in the

first upstreamed code.


So I prefer use a "CONFIG_STM32MP15x_" config to manage it;

it is only activated for TFABOOT and for STM32MP15


But perhaps you prefer a longer name ?


=> CONFIG_STM32MP15x_TFABOOT_STM32IMAGE_CONTAINER

or

=> CONFIG_STM32MP15x_TFABOOT_WITH_STM32IMAGE


For the OP-TEE nodes, on ST boards at least, they are assumes added by OP-TEE

firmware. It is the expected behavior when OP-TEE is compiled for ST boards.


So for me it is not a issue but the expected behavior for ST boards for upstream.


This behavior allows to dynamically manage the case when OP-TEE is not present: driver is not probed,

because it can be replaced by SP-MIN in FIP or in STM32IMAGE, and avoid to force information

in U-Boot device tree configuration which can be managed by OP-TEE.


U-Boot can start any OP-TEE binary, even if memory configuration change

and I try to limit the number of U-Bot add-on in "stm32mp*-u-boot.dtsi".


I agree that for you use can you could force OP-TEE nodes, when OP-TEE is

not compiled to update device tree, by it is not a option chosen

on ST Microelectronics boards for upstream support.


Moreover for me it is not possible to have one compilation flag which defined the boot flow

and I see several other issue with the "falcon OP-TEE" mode:


SPL => OP-TEE => U-Boot


OP-TEE should provide PSCI / SCMI server to U-Boot running in non secure,

so the U-Boot configuration change:

=> CONFIG_ARCH_SUPPORT_PSCI = n

=> CONFIG_ARM_SMCCC

=> CONFIG_CPU_V7_HAS_NONSEC

=> CONFIG_SYSRESET_PSCI

=> CONFIG_SCMI_FIRMWARE / CONFIG_CLK_SCMI / CONFIG_RESET_SCMI

=> CONFIG_TEE / CONFIG_OPTEE

Today they configuration are only activate for TF-A boot
because I assume that with SPL, U-Boot is running is secure level with direct 
access
to secure ressources / wihtout OPTEE


I think you need to update  arch/arm/mach-stm32mp/Kconfig


something like:


 config STM32MP15x
     bool "Support STMicroelectronics STM32MP15x Soc"
-    select ARCH_SUPPORT_PSCI if !TFABOOT
-    select ARM_SMCCC if TFABOOT
+    select ARCH_SUPPORT_PSCI if !TFABOOT && !SPL_OPTEE_IMAGE
+    select ARM_SMCCC if TFABOOT || SPL_OPTEE_IMAGE
     select CPU_V7A
-    select CPU_V7_HAS_NONSEC if !TFABOOT
+    select CPU_V7_HAS_NONSEC if !TFABOOT && !SPL_OPTEE_IMAGE
     select CPU_V7_HAS_VIRT
     select OF_BOARD_SETUP
     select PINCTRL_STM32
@@ -47,8 +47,8 @@ config STM32MP15x
     select STM32_SERIAL
     select SYS_ARCH_TIMER
     imply CMD_NVEDIT_INFO
-    imply SYSRESET_PSCI if TFABOOT
-    imply SYSRESET_SYSCON if !TFABOOT
+    imply SYSRESET_PSCI if TFABOOT || SPL_OPTEE_IMAGE
+    imply SYSRESET_SYSCON if !TFABOOT && !SPL_OPTEE_IMAGE
     help
         support of STMicroelectronics SOC STM32MP15x family
         STM32MP157, STM32MP153 or STM32MP151
@@ -153,7 +153,7 @@ config NR_DRAM_BANKS

 config DDR_CACHEABLE_SIZE
     hex "Size of the DDR marked cacheable in pre-reloc stage"
-    default 0x10000000 if TFABOOT
+    default 0x10000000 if TFABOOT || SPL_OPTEE_IMAGE
     default 0x40000000
     help
         Define the size of the DDR marked as cacheable in U-Boot


or move all these ARCH option from Kconfig in each defconfig....


So just introduce CONFIG_TFABOOT_FIP_CONTAINER don't solve all the issues....


I think you need to manage CONFIG_SPL_OPTEE_IMAGE

to handle specific case when OPTEE is running after SPL.


I try to experiment the OPTEE load by SPL but I don't see how

the OP-TEE pager can be managed by SPL in the current code.


It must loaded in SYRAM at 0x2ffc0000, with a risk to overwrite the SPL

code loaded by rom code at 0x2ffc2500.


or how to manage several binary, see OP-TEE header v2 support in OP-TEE,

Several file it is already supported in TF-A BL2 with FIP:

tee-header_v2.bin
tee-pager_v2.bin
tee-pageable_v2.bin


Alexandru Gagniuc (3):
   stm32mp: Rename FIP config to stm32mp15_tfaboot_fip_defconig
   arm: Kconfig: Introduce a TFABOOT_FIP_CONTAINER option
   stm32mp1: Replace STM32MP15x_STM32IMAGE with TFABOOT_FIP_CONTAINER

  arch/arm/Kconfig                              | 15 ++++++++++++
  arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi      |  9 ++++----
  arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi      |  9 ++++----
  arch/arm/mach-stm32mp/Kconfig                 |  7 ------
  .../cmd_stm32prog/cmd_stm32prog.c             |  5 ++--
  .../mach-stm32mp/cmd_stm32prog/stm32prog.c    |  4 ----
  .../mach-stm32mp/cmd_stm32prog/stm32prog.h    |  2 --
  arch/arm/mach-stm32mp/config.mk               |  2 +-
  arch/arm/mach-stm32mp/fdt.c                   |  4 +---
  .../arm/mach-stm32mp/include/mach/stm32prog.h |  2 --
  board/st/common/Kconfig                       | 23 ++++++++++---------
  board/st/common/stm32mp_mtdparts.c            | 20 +---------------
  board/st/stm32mp1/MAINTAINERS                 |  2 +-
  board/st/stm32mp1/stm32mp1.c                  |  6 ++---
  ...config => stm32mp15_tfaboot_fip_defconfig} |  1 +
  configs/stm32mp15_trusted_defconfig           |  1 -
  doc/board/st/stm32mp1.rst                     | 16 ++++++-------
  17 files changed, 54 insertions(+), 74 deletions(-)
  rename configs/{stm32mp15_defconfig => stm32mp15_tfaboot_fip_defconfig} (99%)


PS: I push a patch to solve the GPT partition name force to fip in basic boot

http://patchwork.ozlabs.org/project/uboot/list/?series=262257&state=*


NB: I will share my working branch with my proposal of SPL & OPT-TEE

       support in few days


Regards

Patrick

Reply via email to