Hi Alexandru,

On 8/26/21 11:47 PM, Alexandru Gagniuc wrote:

Hi Patrick,

I proposing a better fix fir the issues I outlined earlier, I made a classification of the currently supported boot modes.

1) BL1 -> SPL -> U-Boot
2) BL1 -> SPL -> OP-TEE

---

| 3) BL1 -> TF-A -> U-Boot | | 4) BL1 -> TF-A -> OP-TEE | | _________________________________________________________________ | || 5) BL1 -> TF-A -> FIP container || || CONFIG_TFABOOT_FIP || ||_________________________________________________________________|| | | | CONFIG_TFABOOT |

---

Here, I'm looking at FIP as a new boot mode. In order to avoid breakage, any changes to support FIP it should naturally be done only to this new path.

This proposal contains several changes, but I've squashed them into one for ease of discussion.

This better matches the boot mode classification above.

1) is supported but with many constraint for security part and low power management

it is not recommended for real product / it will be not supported by STMicroelectronics

2) is not supported by STMicroelectronics, introduced by your patchset

3 and 4) are common for U-Boot point of view: running in normal world, STM32IMAGE container

BL1 -> TF-A -> SecureMonitor -> U-Boot (STM32IMAGE or FIP container)

With Secure Monitor

= SPMIN (included in a  the STM32IMAGE container of TF-A)

or OP-TEE (loaded by TF-A BL2 in "tf-a.stm32", using the STM32IMAGE container)

The STM32IMAGE container are correct for the BL1 load with first root of trust but have limitations

for new security features (Public key infrastructure, secure software update, M4 firmware load by TF-A)

or for next products (coming soon) with limited size of embedded RAM.

So we decide to use for future development/product the default TF-A container = the FIP.

=> For the next products only we only support FIP container.

And the FIP containers is now the recommended TF-A container for STM32MP15 platform

and we could decide one day to deprecate/remove the STM32IMAGE support for STM32MP15 platform....

It is why I introduced the CONFIG_STM32MP15x_STM32IMAGE,

to keep the compatibility with the previous "trusted" boot chain

and the default container is/becomes the FIP for all STM32MP product

=> this STM32IMAGE support (under compilation flag) could be removed in future

by choice for the maintenance I prefer to mark the code

to remove under a specific compilation flags only supported for STM32MP15x

Today I have the classification:

1) BL1 -> SPL -> u-boot ("basic" boot)

--------------------------------------------------------------------- | 2) BL1 -> TF-A -> u-boot ("trusted" boot) | | | | 2.1) TF-A BL2 use STM32IMAGE container | | (CONFIG_STM32MP15x_STM32IMAGE => deprecated) | | | | 2.2) TF-A BL2 use FIP container (new default) | | | | CONFIG_TFABOOT | ---------------------------------------------------------------------

The STM32IMAGE container support is only variant for STM32MP15x trusted boot.

FIP is a evolution of the STMicroelectronics proposal

and is now the default recommended container for trusted boot.

OP-TEE presence is detected dynamically in U-Boot (it should be mandatory for next STM32MP product).

The root cause the of the issue is the introduction of

the "strange" OP-TEE after SPL boot mode (*)

1) BL1 -> SPL (secure) -> U-Boot (secure)

*) BL1 -> SPL (secure) -> OP-TEE (secure) -> kernel (normal world) for falcon mode

-> U-Boot (normal world)

it is strange for me, because SPL and U-Boot are not running at the same exception level, and OP-TEE firmware is only present/running when U-Boot proper is running.

So all my assumption in STM32MP platform are false (SPL support => U-Boot is running in secure world when TFABOOT is not present) and not only the FIP support.

This mode is not recommended: it will not supported by STMicroelectronics and not supported by next products.

But you are free to manage it.

This new boot scheme could be managed by a new defconfig.... to avoid to break the existing configuration (basic / trusted)

This config mixes boot path (2) with paths (3) and (4), and thus is contrary to our goal of making changes only to the new FIP path. Because it mixes and matches SPL assumptions with TF-A assumptions, I've had a hard time figuting it out. I suspect it would be just as confusing for others in the future.

Yes I make the assumption that SPL is for "basic" boot

SPL loads the next stage in secure world because it the general use case for SPL / U-Boot:

the switch to normal world is done AFTER U-Boot when the

secure monitor for PSCI suport is installed by U-Boot.

But in you use-case you mix the 2 boot schemes ("basic" for SPL and "trusted" for U-boot)

For my point of view you can use

- SPL (u-boot-spl.stm32) from stm32mp15_basic_defconfig:

=> simple loader / running in secure world

- U-Boot (u-boot-nodtb.bin + u-boot.dtb = included in FIT) from stm32mp15_trusted_defconfig:

=> U-Boot running in normal world  / using OP-TEE

If you want manage the feature with a single defconfig it should be difficult

to avoid to break the existing boot schemes.

I've had issues with tee_find_device() in the past when using SPL as the FSBL. As u-boot was running in secure mode and did not have a handler, it would result in a CPU exception and crash.

yes it is normal for me: OP-TEE driver support should be only activated when

U-Boot is running in normal world, in "trusted" defconfig

tee_find_device() should not be called when OP-TEE driver is not supported, in SPL or for "basic" boot.

PS: in the code the API call are protected by CONFIG_IS_ENABLED:

if (CONFIG_IS_ENABLED(OPTEE) && !tee_find_device(NULL, NULL, NULL, NULL))

I thought that was enought because

the config CONFIG_OPTEE is used to activate the OP-TEE driver support in U-Boot

So the CONFIG_SPL_OPTEE is used to activate the OP-TEE driver support in SPL

=> for me CONFIG_SPL_OPTEE should be never activated

but after check and I am confused with the CONFIG_OPTEE: it is defined 2 times with 2 meanings

and  I think U-Boot have a issue here....

./lib/optee/Kconfig:

config OPTEE     bool "Support OPTEE images"     help       U-Boot can be configured to boot OPTEE images.       Selecting this option will enable shared OPTEE library code and           enable an OPTEE specific bootm command that will perform additional           OPTEE specific checks before booting an OPTEE image created with           mkimage.

./drivers/tee/optee/Kconfig:

config OPTEE     bool "OP-TEE"     depends on ARM_SMCCC     help       This implements the OP-TEE Trusted Execution Environment (TEE)       driver. OP-TEE is a Trusted OS designed primarily to rely on the       ARM TrustZone(R) technology as the underlying hardware isolation       mechanism. This driver can request services from OP-TEE, but also       handle Remote Procedure Calls (RPC) from OP-TEE needed to       execute a service. For more information see: https://www.op-tee.org

/lib/Makefile:19:obj-$(CONFIG_OPTEE) += optee/ ./drivers/tee/Makefile:7:obj-$(CONFIG_OPTEE) += optee/

I think one existing CONFIG_OPTEE should be renamed to a better name

"CONFIG_OPTEE_IMAGE" for "BOOT of OPTEE firmware"

./lib/optee/Kconfig:

- config OPTEE

+ config OPTEE_IMAGE

and the associated becomes

- config SPL_OPTEE

+ config SPL_OPTEE_IMAGE

bool "Support boot of OP-TEE Trusted OS"     depends on ARM     help       OP-TEE is an open source Trusted OS  which is loaded by SPL.       More detail at: https://github.com/OP-TEE/optee_os

.....

- #if CONFIG_IS_ENABLED(OPTEE)

- #if CONFIG_IS_ENABLED(OPTEE_IMAGE)     case IH_OS_TEE:         debug("Jumping to U-Boot via OP-TEE\n");         spl_board_prepare_for_optee(spl_image.fdt_addr);         spl_optee_entry(NULL, NULL, spl_image.fdt_addr,                 (void *)spl_image.entry_point);         break; #endif

+ other part in the code

I think I will propose this patch to solve this issue

The second argument against this is that stm32mp1 is the only platform to call tee_find_device() with the intent of detecting the presence of OP-TEE.

I introduce this check to avoid to duplicate the trusted defconfig

for OP-TEE /SPMIN support and I think the dynamic detection is more flexible:

the same U-Boot can be used with SPMIN or OP-TEE as secure monitor

I don't see the problem here: it CONFIG_OPTEE is activated,

the OP-TEE driver is present  and the function can be called

(at least in U-Boot, but see previous remark).

but perhaps some inline function in include/tee.h could be more elegant

when the CONFIG issue is solved....

#if CONFIG_IS_ENABLED(OPTEE)

struct udevice *tee_find_device(struct udevice *start,

int (*match)(struct tee_version_data *vers,                          const void *data),                 const void *data,                 struct tee_version_data *vers);

#else

static inline struct udevice *tee_find_device(struct udevice *start,

int (*match)(struct tee_version_data *vers,                          const void *data),                 const void *data,                 struct tee_version_data *vers) {

return NULL; }

#endif

Your problem here is that you activate OPTEE in SPL= CONFIG_SPL_OPTEE

and this config is strange, the OP-TEE driver is not activated in SPL but SPL boot OPTEE image.

Have there been issues with not callinf this in the past, or was this more of a "seems nice to have" ?

"stm32mp15_defconfig" implies that would be the correct configuraion for STM32MP1. New contributor chooses this config, tries to run SPL

• u-boot, which is what u-boot user expects is the default. Things

likely fail miserably. A lot of u-boot users don't know what FIP is. It's an extra concept that is not strictly necessary in u-boot.

For me, SPL is not the only configuration expected by U-Boot user

(sometime you have SPL , TPL, TF-A or proprietary first stage bootloader).

And yes "stm32mp15_defconfig" it the new STMicroelectronics configuration to use for STMicroelectronics STM32MP15x boards and the recommended starting point for new customer board support. And it doesn't use SPL.

TF-A and FIP are perhaps not necessary today for STM32MP15x,

but they will be mandatory in the next products (SPL boot will be no more supported).

I try to document each defconfig in platform documentation (in doc/board/st/)

https://u-boot.readthedocs.io/en/latest/board/st/stm32mp1.html#boot-sequence...

And it is also explained in our WIKI:

https://wiki.st.com/stm32mpu/wiki/Boot_chain_overview

https://wiki.st.com/stm32mpu/wiki/TF-A_overview

but if the user don't read the SW documentation....

yes he can be confused.

So I think this name is vague, as it doesn't really describe what is going on. If we change it to "stm32mp15_tfaboot_fip_defconfig", then it very accurately describes the boot scenario, and avoids the confusion above.

for next product STM32MP1X, and for ST boards, as only the TF-A with FIP

is supported I will use the simple name:

- stm32mpXX_defconfig

with for STM32MP15x:

- stm32mp15_defconfig => boot with TF-A with FIP (default)

only for stm32mp15 I keeps the previous defconfig only for compatibility

- stm32mp15_basic_defconfig => boot with SPL

- stm32mp15_trusted_defconfig => boot with TF-A without FIP support (deprecated)

PS: our Marketing team don't like "FIP" / "tfaboot" in config name...

my first proposal was stm32mp15_spl_defconfig / stm32mp15_tfa_defconfig

instead of stm32mp15_basic_defconfig / stm32mp15_trusted_defconfig

but if you want support a other configuration / a other customization / a other board, you can manage

your stm32mp15 defconfig with your boot configuration, for example

stm32mp15_spl_optee_defconfig

We're setting CONFIG_MTDPARTS_xxx based on TFABOOT_FIP_CONTAINER now, so I don't think we need any ifdefs here. This part needs the most scrutiny, as I don't have a way to test if I've broken anything.

---

arch/arm/Kconfig | 14 +++++++ arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 7 ++-- arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi | 9 ++--- arch/arm/mach-stm32mp/Kconfig | 7 ---- .../cmd_stm32prog/cmd_stm32prog.c | 2 - .../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 | 37 ------------------- .../arm/mach-stm32mp/include/mach/stm32prog.h | 2 - board/st/common/Kconfig | 20 +++++----- board/st/common/stm32mp_mtdparts.c | 18 --------- board/st/stm32mp1/stm32mp1.c | 6 +-- ...config => stm32mp15_tfaboot_fip_defconfig} | 1 + configs/stm32mp15_trusted_defconfig | 1 - 15 files changed, 35 insertions(+), 97 deletions(-) rename configs/{stm32mp15_defconfig => stm32mp15_tfaboot_fip_defconfig} (99%)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index d692139199..4c6f7ab3de 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1905,6 +1905,20 @@ config TFABOOT Enabling this option will make a U-Boot binary that is relying on other firmware layers to provide secure functionality.

+config TFABOOT_FIP_CONTAINER

• bool "Support for booting from TF-A inside a FIP container"
• depends on TFABOOT
• default n

default y ?

• help

•  TF-A has its own container format, named FIP (not to be confused with
  

•  FIT). When u-boot is started from a FIP, it sometimes needs to make
  

•  different assumptions than it would with a non-FIP boot. Although
  

•  those could be resolved with dynamic devicetree patching, TF-A is
  

•  either can't patch devicetrees, or is unwilling to do so.
  

•  Enabling this option will tell u-boot platform code that it is okay
  

•  to assume U-Boot will be started from a FIP container, even if such
  

•  assumptions would break things in a more normal setting.
  

As FIP container is the default container for TF-A.... no need to have configuration for it.

I prefer to have a compilation flag for the ST specific container

= STM32IMAGE (it will be marked deprecated soon)

config TI_SECURE_DEVICE bool "HS Device Type Support" depends on ARCH_KEYSTONE || ARCH_OMAP2PLUS || ARCH_K3 diff --git a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi index 0101962ea5..54424e398f 100644 --- a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi +++ b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi @@ -15,18 +15,18 @@ config { u-boot,boot-led = "heartbeat"; u-boot,error-led = "error";

• u-boot,mmc-env-partition = "fip";
  

• u-boot,mmc-env-partition = "fip1";
  

why it is "fip1" for dk1 and "fip"  for ed1, in eMMC the GPT parttion is named fip

fip1 is for NAND

st,adc_usb_pd = <&adc1 18>, <&adc1 19>;
st,fastboot-gpios = <&gpioa 13 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
st,stm32prog-gpios = <&gpioa 14 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;

};

-#ifdef CONFIG_STM32MP15x_STM32IMAGE +#ifdef CONFIG_TFABOOT_FIP_CONTAINER config { u-boot,mmc-env-partition = "ssbl"; }; +#endif

I prefer the initial code => the default devicetree if for FIP support on ST board because it is the ST Microelectronics recommandation

And I assure the backward compatibility with compilation flag.

=> it is more easy to compare the STM32MP15 device tree with next products.

NB: a other solution is to remove the "basic" / SPL support .... as ST Microelectronics don't support it.

but it is not acceptable as it is already used by customers

• /* only needed for boot with TF-A, witout FIP support */ firmware { optee { compatible = "linaro,optee-tz";

@@ -43,7 +43,6 @@ u-boot,dm-spl; }; }; -#endif

led { red { diff --git a/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi b/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi index 32777384c6..184774df58 100644 --- a/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi +++ b/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi @@ -15,17 +15,17 @@ config { u-boot,boot-led = "heartbeat"; u-boot,error-led = "error";

• u-boot,mmc-env-partition = "fip";
  

• u-boot,mmc-env-partition = "ssbl";
  

  st,fastboot-gpios = <&gpioa 13 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>; st,stm32prog-gpios = <&gpioa 14 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>; };

-#ifdef CONFIG_STM32MP15x_STM32IMAGE +#ifdef CONFIG_TFABOOT_FIP_CONTAINER

config {

• u-boot,mmc-env-partition = "ssbl";
  

• u-boot,mmc-env-partition = "fip";
  

  };

+#endif

• /* only needed for boot with TF-A, witout FIP support */ firmware { optee { compatible = "linaro,optee-tz";

@@ -39,7 +39,6 @@ no-map; }; }; -#endif

led { red { diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-stm32mp/Kconfig index 5d7eca649a..4c1eeef165 100644 --- a/arch/arm/mach-stm32mp/Kconfig +++ b/arch/arm/mach-stm32mp/Kconfig @@ -56,13 +56,6 @@ config STM32MP15x dual core A7 for STM32MP157/3, monocore for STM32MP151 target all the STMicroelectronics board with SOC STM32MP1 family

-config STM32MP15x_STM32IMAGE

• bool "Support STM32 image for generated U-Boot image"
• depends on STM32MP15x && TFABOOT
• help

• Support of STM32 image generation for SOC STM32MP15x
  

• for TF-A boot when FIP container is not used
  

• choice prompt "STM32MP15x board select" optional

diff --git a/arch/arm/mach-stm32mp/cmd_stm32prog/cmd_stm32prog.c b/arch/arm/mach-stm32mp/cmd_stm32prog/cmd_stm32prog.c index 41452b5a29..be53a52977 100644 --- a/arch/arm/mach-stm32mp/cmd_stm32prog/cmd_stm32prog.c +++ b/arch/arm/mach-stm32mp/cmd_stm32prog/cmd_stm32prog.c @@ -185,7 +185,6 @@ U_BOOT_CMD(stm32prog, 5, 0, do_stm32prog, " <size> = size of flashlayout (optional for image with STM32 header)\n" );

-#ifdef CONFIG_STM32MP15x_STM32IMAGE bool stm32prog_get_tee_partitions(void) { if (stm32prog_data) @@ -193,7 +192,6 @@ bool stm32prog_get_tee_partitions(void)

return false; } -#endif

I prefer to have the code under compilation flag to prepare code removal

and STM32IMAGE will be no more supported

and also easy customer board creation (with FIP support) to detect part of obsolete code.

bool stm32prog_get_fsbl_nor(void) { diff --git a/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.c b/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.c index 3b6ca4e773..26fe8b654a 100644 --- a/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.c +++ b/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.c @@ -824,9 +824,7 @@ static int treat_partition_list(struct stm32prog_data *data) INIT_LIST_HEAD(&data->dev[j].part_list); }

-#ifdef CONFIG_STM32MP15x_STM32IMAGE

#ifndef CONFIG_TFABOOT_FIP_CONTAINER ?

data->tee_detected = false; -#endif data->fsbl_nor_detected = false; for (i = 0; i < data->part_nb; i++) { part = &data->part_array[i]; @@ -880,12 +878,10 @@ static int treat_partition_list(struct stm32prog_data *data) /* fallthrough */ case STM32PROG_NAND: case STM32PROG_SPI_NAND: -#ifdef CONFIG_STM32MP15x_STM32IMAGE

#ifndef CONFIG_TFABOOT_FIP_CONTAINER ?

	if (!data->tee_detected &&
	    !strncmp(part->name, "tee", 3))
		data->tee_detected = true;
	break;

-#endif default: break; } diff --git a/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.h b/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.h index 240c5c44bc..9d58cf0e2d 100644 --- a/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.h +++ b/arch/arm/mach-stm32mp/cmd_stm32prog/stm32prog.h @@ -122,9 +122,7 @@ struct stm32prog_data { struct stm32prog_dev_t dev[STM32PROG_MAX_DEV]; /* array of device */ int part_nb; /* nb of partition */ struct stm32prog_part_t *part_array; /* array of partition */ -#ifdef CONFIG_STM32MP15x_STM32IMAGE bool tee_detected; -#endif bool fsbl_nor_detected;

/* command internal information */ diff --git a/arch/arm/mach-stm32mp/config.mk b/arch/arm/mach-stm32mp/config.mk index f7f5b77c41..c30bf482f7 100644 --- a/arch/arm/mach-stm32mp/config.mk +++ b/arch/arm/mach-stm32mp/config.mk @@ -4,7 +4,7 @@ #

ifndef CONFIG_SPL -INPUTS-$(CONFIG_STM32MP15x_STM32IMAGE) += u-boot.stm32 +INPUTS-y += u-boot.stm32

normally "u-boot.stm32" is not generated when FIP is expected to avoid user confusion !

it is no more the case with your proposal

else ifdef CONFIG_SPL_BUILD INPUTS-y += u-boot-spl.stm32 diff --git a/arch/arm/mach-stm32mp/fdt.c b/arch/arm/mach-stm32mp/fdt.c index a19e954cf7..8c729e1f32 100644 --- a/arch/arm/mach-stm32mp/fdt.c +++ b/arch/arm/mach-stm32mp/fdt.c @@ -224,30 +224,6 @@ static void stm32_fdt_disable(void *fdt, int offset, u32 addr, string, addr, name); }

-static void stm32_fdt_disable_optee(void *blob) -{

• int off, node;
• /* Delete "optee" firmware node */
• off = fdt_node_offset_by_compatible(blob, -1, "linaro,optee-tz");
• if (off >= 0 && fdtdec_get_is_enabled(blob, off))

• fdt_del_node(blob, off);
  

• /* Delete "optee@..." reserved-memory node */
• off = fdt_path_offset(blob, "/reserved-memory/");
• if (off < 0)

• return;
  

• for (node = fdt_first_subnode(blob, off);

•     node >= 0;
  

•     node = fdt_next_subnode(blob, node)) {
  

• if (strncmp(fdt_get_name(blob, node, NULL), "optee@", 6))
  

• 	continue;
  

• if (fdt_del_node(blob, node))
  

• 	printf("Failed to remove optee reserved-memory node\n");
  

• }

-}

• /*
  • This function is called right before the kernel is booted. "blob" is the
  • device tree that will be passed to the kernel.

@@ -332,18 +308,5 @@ int ft_system_setup(void *blob, struct bd_info *bd) "st,package", pkg, false); }

• /*

• * TEMP: remove OP-TEE nodes in kernel device tree
  

• *       copied from U-Boot device tree by optee_copy_fdt_nodes
  

• *       when OP-TEE is not detected (probe failed)
  

• * these OP-TEE nodes are present in <board>-u-boot.dtsi
  

• * under CONFIG_STM32MP15x_STM32IMAGE only for compatibility
  

• * when FIP is not used by TF-A
  

• */
  

• if (CONFIG_IS_ENABLED(STM32MP15x_STM32IMAGE) &&

•    CONFIG_IS_ENABLED(OPTEE) &&
  

•    !tee_find_device(NULL, NULL, NULL, NULL))
  

• stm32_fdt_disable_optee(blob);
  

without this code you break the TF-A boot with SPMIN (the existing upstreamed support)

return ret; } diff --git a/arch/arm/mach-stm32mp/include/mach/stm32prog.h b/arch/arm/mach-stm32mp/include/mach/stm32prog.h index 99be4e1d65..c080b9cc42 100644 --- a/arch/arm/mach-stm32mp/include/mach/stm32prog.h +++ b/arch/arm/mach-stm32mp/include/mach/stm32prog.h @@ -11,8 +11,6 @@ int stm32prog_read_medium_virt(struct dfu_entity *dfu, u64 offset, void *buf, long *len); int stm32prog_get_medium_size_virt(struct dfu_entity *dfu, u64 *size);

-#ifdef CONFIG_STM32MP15x_STM32IMAGE bool stm32prog_get_tee_partitions(void); -#endif

bool stm32prog_get_fsbl_nor(void); diff --git a/board/st/common/Kconfig b/board/st/common/Kconfig index 2f57118bb2..0bb8e808aa 100644 --- a/board/st/common/Kconfig +++ b/board/st/common/Kconfig @@ -8,9 +8,8 @@ config CMD_STBOARD

config MTDPARTS_NAND0_BOOT string "mtd boot partitions for nand0"

• default "2m(fsbl),2m(ssbl1),2m(ssbl2)" if STM32MP15x_STM32IMAGE || \

• 				  !TFABOOT
  

• default "2m(fsbl),4m(fip1),4m(fip2)"

• default "2m(fsbl),2m(ssbl1),2m(ssbl2)"
• default "2m(fsbl),4m(fip1),4m(fip2)" if TFABOOT_FIP_CONTAINER depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP help This define the partitions of nand0 used to build mtparts dynamically

@@ -23,7 +22,7 @@ config MTDPARTS_NAND0_BOOT config MTDPARTS_NAND0_TEE string "mtd tee partitions for nand0" default "512k(teeh),512k(teed),512k(teex)"

• depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP && STM32MP15x_STM32IMAGE

• depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP

NAK =>  the tee partitions are not present when FIP is used by TF-A / this define should be not defined

help This define the tee partitions added in mtparts dynamically when tee is supported with boot from nand0. @@ -32,9 +31,8 @@ config MTDPARTS_NAND0_TEE

config MTDPARTS_NOR0_BOOT string "mtd boot partitions for nor0"

• default "256k(fsbl1),256k(fsbl2),2m(ssbl),512k(u-boot-env)" if STM32MP15x_STM32IMAGE || \

• 						       !TFABOOT
  

• default "256k(fsbl1),256k(fsbl2),4m(fip),512k(u-boot-env)"

• default "256k(fsbl1),256k(fsbl2),2m(ssbl),512k(u-boot-env)"
• default "256k(fsbl1),256k(fsbl2),4m(fip),512k(u-boot-env)" if TFABOOT_FIP_CONTAINER depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP help This define the partitions of nand0 used to build mtparts dynamically

@@ -46,15 +44,15 @@ config MTDPARTS_NOR0_BOOT config MTDPARTS_NOR0_TEE string "mtd tee partitions for nor0" default "256k(teeh),512k(teed),256k(teex)"

• depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP && STM32MP15x_STM32IMAGE

• depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP

NAK => same

help This define the tee partitions added in mtparts dynamically when tee is supported with boot from nor0.

config MTDPARTS_SPINAND0_BOOT string "mtd boot partitions for spi-nand0"

• default "2m(fsbl),2m(ssbl1),2m(ssbl2)" if STM32MP15x_STM32IMAGE || !TFABOOT
• default "2m(fsbl),4m(fip1),4m(fip2)"

• default "2m(fsbl),2m(ssbl1),2m(ssbl2)"
• default "2m(fsbl),4m(fip1),4m(fip2)" if TFABOOT_FIP_CONTAINER depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP help This define the partitions of nand0 used to build mtparts dynamically

@@ -66,7 +64,7 @@ config MTDPARTS_SPINAND0_BOOT config MTDPARTS_SPINAND0_TEE string "mtd tee partitions for spi-nand0" default "512k(teeh),512k(teed),512k(teex)"

• depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP && STM32MP15x_STM32IMAGE

• depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP

NAK => same

help This define the tee partitions added in mtparts dynamically when tee is supported with boot from spi-nand0, diff --git a/board/st/common/stm32mp_mtdparts.c b/board/st/common/stm32mp_mtdparts.c index 8b636d62fa..dea5d506a1 100644 --- a/board/st/common/stm32mp_mtdparts.c +++ b/board/st/common/stm32mp_mtdparts.c @@ -11,9 +11,7 @@ #include <log.h> #include <mtd.h> #include <mtd_node.h> -#ifdef CONFIG_STM32MP15x_STM32IMAGE #include <tee.h> -#endif #include <asm/arch/stm32prog.h> #include <asm/arch/sys_proto.h> #include <asm/global_data.h> @@ -33,9 +31,7 @@ static void board_set_mtdparts(const char *dev, char *mtdids, char *mtdparts, const char *boot, -#ifdef CONFIG_STM32MP15x_STM32IMAGE const char *tee, -#endif const char *user) { /* mtdids: "<dev>=<dev>, ...." */ @@ -59,12 +55,10 @@ static void board_set_mtdparts(const char *dev, strncat(mtdparts, ",", MTDPARTS_LEN); }

-#ifdef CONFIG_STM32MP15x_STM32IMAGE if (tee) { strncat(mtdparts, tee, MTDPARTS_LEN); strncat(mtdparts, ",", MTDPARTS_LEN); } -#endif

strncat(mtdparts, user, MTDPARTS_LEN); } @@ -77,9 +71,7 @@ void board_mtdparts_default(const char **mtdids, const char **mtdparts) static char ids[MTDIDS_LEN + 1]; static bool mtd_initialized; bool nor, nand, spinand, serial; -#ifdef CONFIG_STM32MP15x_STM32IMAGE bool tee = false; -#endif

if (mtd_initialized) { *mtdids = ids; @@ -97,9 +89,7 @@ void board_mtdparts_default(const char **mtdids, const char **mtdparts) case BOOT_SERIAL_USB: serial = true; if (CONFIG_IS_ENABLED(CMD_STM32PROG)) { -#ifdef CONFIG_STM32MP15x_STM32IMAGE tee = stm32prog_get_tee_partitions(); -#endif nor = stm32prog_get_fsbl_nor(); } nand = true; @@ -118,11 +108,9 @@ void board_mtdparts_default(const char **mtdids, const char **mtdparts) break; }

-#ifdef CONFIG_STM32MP15x_STM32IMAGE if (!serial && CONFIG_IS_ENABLED(OPTEE) && tee_find_device(NULL, NULL, NULL, NULL)) tee = true; -#endif

memset(parts, 0, sizeof(parts)); memset(ids, 0, sizeof(ids)); @@ -139,9 +127,7 @@ void board_mtdparts_default(const char **mtdids, const char **mtdparts) if (!IS_ERR_OR_NULL(mtd)) { board_set_mtdparts("nand0", ids, parts, CONFIG_MTDPARTS_NAND0_BOOT, -#ifdef CONFIG_STM32MP15x_STM32IMAGE !nor && tee ? CONFIG_MTDPARTS_NAND0_TEE : NULL, -#endif "-(UBI)"); put_mtd_device(mtd); } @@ -152,9 +138,7 @@ void board_mtdparts_default(const char **mtdids, const char **mtdparts) if (!IS_ERR_OR_NULL(mtd)) { board_set_mtdparts("spi-nand0", ids, parts, CONFIG_MTDPARTS_SPINAND0_BOOT, -#ifdef CONFIG_STM32MP15x_STM32IMAGE !nor && tee ? CONFIG_MTDPARTS_SPINAND0_TEE : NULL, -#endif "-(UBI)"); put_mtd_device(mtd); } @@ -164,9 +148,7 @@ void board_mtdparts_default(const char **mtdids, const char **mtdparts) if (!uclass_get_device(UCLASS_SPI_FLASH, 0, &dev)) { board_set_mtdparts("nor0", ids, parts, CONFIG_MTDPARTS_NOR0_BOOT, -#ifdef CONFIG_STM32MP15x_STM32IMAGE tee ? CONFIG_MTDPARTS_NOR0_TEE : NULL, -#endif "-(nor_user)"); } } diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c index 032f08d795..583f923d29 100644 --- a/board/st/stm32mp1/stm32mp1.c +++ b/board/st/stm32mp1/stm32mp1.c @@ -106,10 +106,10 @@ int checkboard(void) int fdt_compat_len;

if (IS_ENABLED(CONFIG_TFABOOT)) {

• if (IS_ENABLED(CONFIG_STM32MP15x_STM32IMAGE))
  

• 	mode = "trusted - stm32image";
  

• else
  

• if (IS_ENABLED(CONFIG_TFABOOT_FIP_CONTAINER))
  mode = "trusted";
  

• else
  

• 	mode = "trusted - stm32image";
  

  } else { mode = "basic"; }

diff --git a/configs/stm32mp15_defconfig b/configs/stm32mp15_tfaboot_fip_defconfig similarity index 99% rename from configs/stm32mp15_defconfig rename to configs/stm32mp15_tfaboot_fip_defconfig index e725b916b9..f7ef511e9c 100644 --- a/configs/stm32mp15_defconfig +++ b/configs/stm32mp15_tfaboot_fip_defconfig @@ -1,6 +1,7 @@ CONFIG_ARM=y CONFIG_ARCH_STM32MP=y CONFIG_TFABOOT=y +CONFIG_TFABOOT_FIP_CONTAINER=y CONFIG_SYS_MALLOC_F_LEN=0x3000 CONFIG_SYS_MEMTEST_START=0xc0000000 CONFIG_SYS_MEMTEST_END=0xc4000000 diff --git a/configs/stm32mp15_trusted_defconfig b/configs/stm32mp15_trusted_defconfig index 2e2f0c76ca..1671cb24f5 100644 --- a/configs/stm32mp15_trusted_defconfig +++ b/configs/stm32mp15_trusted_defconfig @@ -7,7 +7,6 @@ CONFIG_SYS_MEMTEST_END=0xc4000000 CONFIG_ENV_OFFSET=0x280000 CONFIG_ENV_SECT_SIZE=0x40000 CONFIG_DEFAULT_DEVICE_TREE="stm32mp157c-ev1" -CONFIG_STM32MP15x_STM32IMAGE=y CONFIG_TARGET_ST_STM32MP15x=y CONFIG_CMD_STM32KEY=y CONFIG_CMD_STM32PROG=y

For me you just inverse the logic for the config

CONFIG_STM32MP15x_STM32IMAGE => !CONFIG_TFABOOT_FIP_CONTAINER

I don't see the gain here, and this proposition will make more difficult for me the introduction of the next product (in few months) because it is based only on TF-A/FIP.

Sorry for the long answer, and sorry if my last update breaks your OP-TEE load feature in SPL.

Patrick