* [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
@ 2021-09-09 14:55 Alexandru Gagniuc
2021-09-09 14:55 ` [PATCH 1/3] stm32mp: Rename FIP config to stm32mp15_tfaboot_fip_defconig Alexandru Gagniuc
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Alexandru Gagniuc @ 2021-09-09 14:55 UTC (permalink / raw)
To: u-boot, patrick.delaunay
Cc: patrice.chotard, uboot-stm32, Alexandru Gagniuc, marex, etienne.carriere
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.
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%)
--
2.31.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] stm32mp: Rename FIP config to stm32mp15_tfaboot_fip_defconig
2021-09-09 14:55 [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2 Alexandru Gagniuc
@ 2021-09-09 14:55 ` Alexandru Gagniuc
2021-09-09 14:55 ` [PATCH 2/3] arm: Kconfig: Introduce a TFABOOT_FIP_CONTAINER option Alexandru Gagniuc
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Alexandru Gagniuc @ 2021-09-09 14:55 UTC (permalink / raw)
To: u-boot, patrick.delaunay
Cc: patrice.chotard, uboot-stm32, Alexandru Gagniuc, marex, etienne.carriere
STM32MP has several possible boot flows with either SPL or TF-A. The
word from STM is that they only want to support TF-A with FIP images,
and this should be default. We don't disagree. However, this argument
is orthogonal to naming our defconfigs clearly.
I'm concerned that users might be confused by the current naming. When
given the choice between "basic", "trusted", or "<empty>", someone
used with how u-boot works will think that the "<empty>" config is the
customary "SPL + u-boot". However, such confusion is far less likely
when the choices are "basic", "trusted", and "tfaboot_fip".
To this effect, avoid having a naked config name and rename it to
"stm32mp15_tfaboot_fip_defconig".
Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
board/st/stm32mp1/MAINTAINERS | 2 +-
...defconfig => stm32mp15_tfaboot_fip_defconfig} | 0
doc/board/st/stm32mp1.rst | 16 ++++++++--------
3 files changed, 9 insertions(+), 9 deletions(-)
rename configs/{stm32mp15_defconfig => stm32mp15_tfaboot_fip_defconfig} (100%)
diff --git a/board/st/stm32mp1/MAINTAINERS b/board/st/stm32mp1/MAINTAINERS
index 0e6d80fb45..e2da11b46d 100644
--- a/board/st/stm32mp1/MAINTAINERS
+++ b/board/st/stm32mp1/MAINTAINERS
@@ -5,7 +5,7 @@ T: git https://source.denx.de/u-boot/custodians/u-boot-stm.git
S: Maintained
F: arch/arm/dts/stm32mp15*
F: board/st/stm32mp1/
-F: configs/stm32mp15_defconfig
F: configs/stm32mp15_basic_defconfig
F: configs/stm32mp15_trusted_defconfig
+F: configs/stm32mp15_tfaboot_fip_defconfig
F: include/configs/stm32mp1.h
diff --git a/configs/stm32mp15_defconfig b/configs/stm32mp15_tfaboot_fip_defconfig
similarity index 100%
rename from configs/stm32mp15_defconfig
rename to configs/stm32mp15_tfaboot_fip_defconfig
diff --git a/doc/board/st/stm32mp1.rst b/doc/board/st/stm32mp1.rst
index 42bb94148d..89981023be 100644
--- a/doc/board/st/stm32mp1.rst
+++ b/doc/board/st/stm32mp1.rst
@@ -76,7 +76,7 @@ The **Trusted** boot chain with TF-A_
`````````````````````````````````````
defconfig_file :
- + **stm32mp15_defconfig** (for TF-A_ with FIP support)
+ + **stm32mp15_tfaboot_fip_defconfig** (for TF-A_ with FIP support)
+ **stm32mp15_trusted_defconfig** (for TF-A_ without FIP support)
+-------------+--------------------------+------------+-------+
@@ -184,7 +184,7 @@ Build Procedure
with <defconfig_file>:
- - For **trusted** boot mode : **stm32mp15_defconfig** or
+ - For **trusted** boot mode : **stm32mp15_tfaboot_fip_defconfig** or
stm32mp15_trusted_defconfig
- For basic boot mode: stm32mp15_basic_defconfig
@@ -197,7 +197,7 @@ Build Procedure
a) trusted boot with FIP on ev1::
# export KBUILD_OUTPUT=stm32mp15
- # make stm32mp15_defconfig
+ # make stm32mp15_tfaboot_fip_defconfig
# make DEVICE_TREE=stm32mp157c-ev1 all
b) trusted boot without FIP on dk2::
@@ -235,7 +235,7 @@ Build Procedure
So in the output directory (selected by KBUILD_OUTPUT),
you can found the needed U-Boot files:
- - stm32mp15_defconfig = **u-boot-nodtb.bin** and **u-boot.dtb**
+ - stm32mp15_tfaboot_fip_defconfig = **u-boot-nodtb.bin** and **u-boot.dtb**
- stm32mp15_trusted_defconfig = u-boot.stm32
@@ -248,11 +248,11 @@ Build Procedure
7. TF-A_ compilation
- This step is required only for **Trusted** boot (stm32mp15_defconfig and
- stm32mp15_trusted_defconfig); see OP-TEE_ and TF-A_ documentation for build
- commands.
+ This step is required only for **Trusted** boot
+ (stm32mp15_tfaboot_fip_defconfig and stm32mp15_trusted_defconfig); see
+ OP-TEE_ and TF-A_ documentation for build commands.
- - For TF-A_ with FIP support: **stm32mp15_defconfig**
+ - For TF-A_ with FIP support: **stm32mp15_tfaboot_fip_defconfig**
- with OP-TEE_ support, compile the OP-TEE to generate the binary included
in FIP
--
2.31.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] arm: Kconfig: Introduce a TFABOOT_FIP_CONTAINER option
2021-09-09 14:55 [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2 Alexandru Gagniuc
2021-09-09 14:55 ` [PATCH 1/3] stm32mp: Rename FIP config to stm32mp15_tfaboot_fip_defconig Alexandru Gagniuc
@ 2021-09-09 14:55 ` Alexandru Gagniuc
2021-09-09 14:55 ` [PATCH 3/3] stm32mp1: Replace STM32MP15x_STM32IMAGE with TFABOOT_FIP_CONTAINER Alexandru Gagniuc
2021-09-14 12:26 ` [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2 Patrick DELAUNAY
3 siblings, 0 replies; 9+ messages in thread
From: Alexandru Gagniuc @ 2021-09-09 14:55 UTC (permalink / raw)
To: u-boot, patrick.delaunay
Cc: patrice.chotard, uboot-stm32, Alexandru Gagniuc, marex, etienne.carriere
This option is intended to tell u-boot platform code that this u-boot
build is expected to be used in a FIP container, as part of a TF-A
boot flow.
It is introduced because STM32MP1 platform code needs special
considerations on a FIP boot, such as a different partition layout,
and decisions about who should patch the FDT optee nodes.
This Kconfig can be justified as a natural extension of TFABOOT.
Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
arch/arm/Kconfig | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2d59562665..0bfdc2adc4 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1907,6 +1907,21 @@ 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
+ help
+ TF-A has its own container format, named FIP (not to be confused with
+ FIT). The assumptions u-boot makes about the platform in a non-FIP
+ boot are not always true with FIP.
+ These differences could in theory be resolved with dynamic devicetree
+ patching. However TF-A either can't patch devicetrees, or is
+ unwilling to do so. Even then, passing such devicetree to u-boot
+ might require custom mechanisms.
+ 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.
+
config TI_SECURE_DEVICE
bool "HS Device Type Support"
depends on ARCH_KEYSTONE || ARCH_OMAP2PLUS || ARCH_K3
--
2.31.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] stm32mp1: Replace STM32MP15x_STM32IMAGE with TFABOOT_FIP_CONTAINER
2021-09-09 14:55 [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2 Alexandru Gagniuc
2021-09-09 14:55 ` [PATCH 1/3] stm32mp: Rename FIP config to stm32mp15_tfaboot_fip_defconig Alexandru Gagniuc
2021-09-09 14:55 ` [PATCH 2/3] arm: Kconfig: Introduce a TFABOOT_FIP_CONTAINER option Alexandru Gagniuc
@ 2021-09-09 14:55 ` Alexandru Gagniuc
2021-09-14 12:26 ` [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2 Patrick DELAUNAY
3 siblings, 0 replies; 9+ messages in thread
From: Alexandru Gagniuc @ 2021-09-09 14:55 UTC (permalink / raw)
To: u-boot, patrick.delaunay
Cc: patrice.chotard, uboot-stm32, Alexandru Gagniuc, marex, etienne.carriere
Recently, mach-stm32mp gained the ability to boot from a TF-A FIP
container, bringing the following boot flows:
"basic" SPL -> u-boot
"falcon" SPL -> OP-TEE -> u-boot or kernel
"trusted" TF-A -> OP-TEE -> u-boot
"fip" TF-A -> FIP container
However, the implementation of the new "fip" flow brought changes
which break the "falcon" flow.
1) Removal of #ifdefs
One issue with the STM32MP15x_STM32IMAGE config is the introduction of
an inordinate amount of #ifdefs. This is contrary to u-boot's coding
practices, which prefer the linker and IS_ENABLED(). We can achieve
the same results by setting the CONFIG_MTDPARTS_* Kconfig strings
appropriately for the "fip" flow. The #ifdefs superfluous.
One justification for the #ifdefs is that they make it easier to
remove the soon to be obsoleted STM32IMAGE support. This argument is
unconvincing. There is no technical cost to removing code which is
not #ifdef'd versus code that is.
2) optee nodes in u-boot devicetree
The removal of the "optee" nodes from the u-boot devicetree is
problematic for the "falcon" flow. Only remove them for "fip".
3) Makefile logic for .stm32 images
Because we've removed CONFIG_STM32MP15x_STM32IMAGE, we can't use it in
mach-stm32mp/config.mk to control the creation of u-boot.stm32 images.
Instead of complicating the makefile logic, we revert to always
creating u-boot.stm32. Creation of this image is inconsequential to
how u-boot behaves at runtime.
Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
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/stm32mp1.c | 6 ++---
configs/stm32mp15_tfaboot_fip_defconfig | 1 +
configs/stm32mp15_trusted_defconfig | 1 -
14 files changed, 30 insertions(+), 65 deletions(-)
diff --git a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
index 0101962ea5..b1e1efdc9c 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 = "ssbl";
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";
+ u-boot,mmc-env-partition = "fip";
};
+#endif
- /* 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..ce83d4aa72 100644
--- a/arch/arm/mach-stm32mp/cmd_stm32prog/cmd_stm32prog.c
+++ b/arch/arm/mach-stm32mp/cmd_stm32prog/cmd_stm32prog.c
@@ -185,15 +185,16 @@ 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 (CONFIG_IS_ENABLED(TFABOOT_FIP_CONTAINER))
+ return false;
+
if (stm32prog_data)
return stm32prog_data->tee_detected;
return false;
}
-#endif
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
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
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
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..abd5e01d9c 100644
--- a/arch/arm/mach-stm32mp/fdt.c
+++ b/arch/arm/mach-stm32mp/fdt.c
@@ -337,10 +337,8 @@ int ft_system_setup(void *blob, struct bd_info *bd)
* 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) &&
+ if (!CONFIG_IS_ENABLED(TFABOOT_FIP_CONTAINER) &&
CONFIG_IS_ENABLED(OPTEE) &&
!tee_find_device(NULL, NULL, NULL, NULL))
stm32_fdt_disable_optee(blob);
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..6477930d16 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,8 @@ 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
+ default "" if TFABOOT_FIP_CONTAINER
+ depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP
help
This define the tee partitions added in mtparts dynamically
when tee is supported with boot from nand0.
@@ -32,9 +32,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 +45,16 @@ 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
+ default "" if TFABOOT_FIP_CONTAINER
+ depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP
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 +66,8 @@ 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
+ default "" if TFABOOT_FIP_CONTAINER
+ depends on SYS_MTDPARTS_RUNTIME && ARCH_STM32MP
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..347f62851d 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) {
+ if (tee && strlen(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_tfaboot_fip_defconfig b/configs/stm32mp15_tfaboot_fip_defconfig
index e725b916b9..f7ef511e9c 100644
--- a/configs/stm32mp15_tfaboot_fip_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
--
2.31.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
2021-09-09 14:55 [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2 Alexandru Gagniuc
` (2 preceding siblings ...)
2021-09-09 14:55 ` [PATCH 3/3] stm32mp1: Replace STM32MP15x_STM32IMAGE with TFABOOT_FIP_CONTAINER Alexandru Gagniuc
@ 2021-09-14 12:26 ` Patrick DELAUNAY
2021-10-07 17:13 ` Alex G.
3 siblings, 1 reply; 9+ messages in thread
From: Patrick DELAUNAY @ 2021-09-14 12:26 UTC (permalink / raw)
To: Alexandru Gagniuc, u-boot
Cc: patrice.chotard, uboot-stm32, marex, etienne.carriere
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
2021-09-14 12:26 ` [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2 Patrick DELAUNAY
@ 2021-10-07 17:13 ` Alex G.
2021-10-08 9:18 ` Patrick DELAUNAY
0 siblings, 1 reply; 9+ messages in thread
From: Alex G. @ 2021-10-07 17:13 UTC (permalink / raw)
To: Patrick DELAUNAY, u-boot
Cc: patrice.chotard, uboot-stm32, marex, etienne.carriere
Hi Patrick,
On 9/14/21 7:26 AM, Patrick DELAUNAY wrote:
> Hi Alexandru,
> 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
This is a terrible idea. We're trying to answer a few questions:
* Did the FSBL provide a PSCI secure monitor
* Is u-boot running in normal or secure world
But instead of clearly defining those answers, we're trying to infer
them from other config options. This is confusing to start with, but
it's also wrong.
For example, SPL_OPTEE_IMAGE means "we support OPTEE images in SPL". It
doesn't mean that we loaded an OP-TEE kernel at all. SPL with
SPL_OPTEE_IMAGE may as well boot a legacy uboot image -- nothing
prevents it. So to infer from this that u-boot runs in the normal world
is wrong.
Without loss of generality, any CONFIG that conflates u-boot options
with SPL options is likely to cause some changes down the line.
> 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.
Sure, but I'd have to adjust this at runtime, not in Kconfig for the
reasons above.
> 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.
This consideration is not unique to SPL. I don't have that problem
because SPL loads OP-TEE to DRAM at 0xde000000. If OP-TEE wants to load
parts of itself to SYSRAM, that happens after SPL passed control, so the
conflict is not relevant.
> 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
I don't know how to use use the OP-TEE pager with SPL, so I elected not to:
EXTRA_OEMAKE = "PLATFORM=${OPTEE_PLATFORM} \
CFG_WITH_PAGER=n \
CFG_NS_ENTRY_ADDR=${KERNEL_UIMAGE_LOADADDRESS} \
CROSS_COMPILE=${HOST_PREFIX} \
CFG_TEE_CORE_DEBUG=y \
CFG_TEE_CORE_LOG_LEVEL=2 \
${TZDRAM_FLAGS} \
"
TZDRAM_FLAGS = "CFG_TZDRAM_START= 0xde000000\
CFG_DRAM_SIZE=0x20000000 "
Alex
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
2021-10-07 17:13 ` Alex G.
@ 2021-10-08 9:18 ` Patrick DELAUNAY
2021-10-08 11:28 ` Marek Vasut
0 siblings, 1 reply; 9+ messages in thread
From: Patrick DELAUNAY @ 2021-10-08 9:18 UTC (permalink / raw)
To: Alex G., u-boot; +Cc: patrice.chotard, uboot-stm32, marex, etienne.carriere
Hi Alex,
On 10/7/21 7:13 PM, Alex G. wrote:
> Hi Patrick,
>
> On 9/14/21 7:26 AM, Patrick DELAUNAY wrote:
>> Hi Alexandru,
>
>> 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
>
> This is a terrible idea. We're trying to answer a few questions:
> * Did the FSBL provide a PSCI secure monitor
> * Is u-boot running in normal or secure world
>
> But instead of clearly defining those answers, we're trying to infer
> them from other config options. This is confusing to start with, but
> it's also wrong.
>
> For example, SPL_OPTEE_IMAGE means "we support OPTEE images in SPL".
> It doesn't mean that we loaded an OP-TEE kernel at all. SPL with
> SPL_OPTEE_IMAGE may as well boot a legacy uboot image -- nothing
> prevents it. So to infer from this that u-boot runs in the normal
> world is wrong.
>
> Without loss of generality, any CONFIG that conflates u-boot options
> with SPL options is likely to cause some changes down the line.
>
I have a issue today with the generic part of ARM support:
1/ the PSCI is mandatory for Linux kernel
2/ the PSCI is provided by
a) U-Boot when it is executed in secure world =>
CONFIG_ARCH_SUPPORT_PSCI / CONFIG_ARMV7_PSCI / CONFIG_ARMV7_NONSEC
b) OP-TEE when U-Boot is running in normal world
I think today we can't handle it without CONFIG in U-Boot (for SMCC and
PSCI support for example)
because the dynamic detection of execution level is not done in the
generic armv7 code
for example:
/* Subcommand: GO */
static void boot_jump_linux(bootm_headers_t *images, int flag)
{
....
if (!fake) {
#ifdef CONFIG_ARMV7_NONSEC
if (armv7_boot_nonsec()) {
armv7_init_nonsec();
secure_ram_addr(_do_nonsec_entry)(kernel_entry,
0, machid, r2);
} else
#endif
kernel_entry(0, machid, r2);
}
#endif
I don't want change this generic part.
it is why I prefer select the U-Boot configuration at compilation time
that dynamic detection of execution level ('get_cpsr()' is never used
for armv7).
PS: it is partially done for armv8 with "is_hyp()"
it is why assume that if SPL can load OP-TEE, the OP-TEE must load OPTEE
to simplify the defconfig.
But I agree, I will remove this implicit rules in arch stm32mp
and let each defconfig handle the dependencies to avoid assumptions
Then you could use the same SPL for the 2 FITs / the 2 U-Boot
configuration by 2 defconfigs:
1) U-Boot running in secure world without OPTEE support => need to
compile and install PSCI / kernel is start in non secure world
2) U-Boot running in normal world with OPTEE support/ PSCI client
support / SCMI support / SMC / kernel is started at same execution level
>
>> 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.
>
> Sure, but I'd have to adjust this at runtime, not in Kconfig for the
> reasons above.
>
>> 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.
>
> This consideration is not unique to SPL. I don't have that problem
> because SPL loads OP-TEE to DRAM at 0xde000000. If OP-TEE wants to
> load parts of itself to SYSRAM, that happens after SPL passed control,
> so the conflict is not relevant.
>
>> 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
>
> I don't know how to use use the OP-TEE pager with SPL, so I elected
> not to:
>
> EXTRA_OEMAKE = "PLATFORM=${OPTEE_PLATFORM} \
> CFG_WITH_PAGER=n \
> CFG_NS_ENTRY_ADDR=${KERNEL_UIMAGE_LOADADDRESS} \
> CROSS_COMPILE=${HOST_PREFIX} \
> CFG_TEE_CORE_DEBUG=y \
> CFG_TEE_CORE_LOG_LEVEL=2 \
> ${TZDRAM_FLAGS} \
> "
>
> TZDRAM_FLAGS = "CFG_TZDRAM_START= 0xde000000\
> CFG_DRAM_SIZE=0x20000000 "
>
With pager (and v2 header) the pager code need to be loaded at the final
location by FSBL in SYSRAM....
the relocation is not handle by OP-TEE, this load is handled already by
TF-A BL2.
For STM32MP15, only the SYSRAM can be really secured, as the external
DDR can't be scrambled, it is why PAGER is recommended.
But to avoid the issue I also deactivate the pager for my test of SPL load.
Thanks
> Alex
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
2021-10-08 9:18 ` Patrick DELAUNAY
@ 2021-10-08 11:28 ` Marek Vasut
2021-10-08 13:20 ` Patrick DELAUNAY
0 siblings, 1 reply; 9+ messages in thread
From: Marek Vasut @ 2021-10-08 11:28 UTC (permalink / raw)
To: Patrick DELAUNAY, Alex G., u-boot
Cc: patrice.chotard, uboot-stm32, etienne.carriere
On 10/8/21 11:18 AM, Patrick DELAUNAY wrote:
> Hi Alex,
Hi,
[...]
>> Without loss of generality, any CONFIG that conflates u-boot options
>> with SPL options is likely to cause some changes down the line.
>>
> I have a issue today with the generic part of ARM support:
>
> 1/ the PSCI is mandatory for Linux kernel
PSCI is mandatory only on ARMv8 , NOT on ARMv7.
And the "mandatory" part is forced onto everyone not by Linux kernel,
but by the architecture specification.
[...]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
2021-10-08 11:28 ` Marek Vasut
@ 2021-10-08 13:20 ` Patrick DELAUNAY
0 siblings, 0 replies; 9+ messages in thread
From: Patrick DELAUNAY @ 2021-10-08 13:20 UTC (permalink / raw)
To: Marek Vasut, Alex G., u-boot
Cc: patrice.chotard, uboot-stm32, etienne.carriere
On 10/8/21 1:28 PM, Marek Vasut wrote:
> On 10/8/21 11:18 AM, Patrick DELAUNAY wrote:
>> Hi Alex,
>
> Hi,
>
> [...]
>
>>> Without loss of generality, any CONFIG that conflates u-boot options
>>> with SPL options is likely to cause some changes down the line.
>>>
>> I have a issue today with the generic part of ARM support:
>>
>> 1/ the PSCI is mandatory for Linux kernel
>
> PSCI is mandatory only on ARMv8 , NOT on ARMv7.
> And the "mandatory" part is forced onto everyone not by Linux kernel,
> but by the architecture specification.
>
> [...]
Sorry for the short-cut... it is not really manadatory.
Today PSCI is required by the upstreamed Linux kernel on STM32MP15x
Family to wake-up the secondary core and some other features (system
reset / power-off).
STMicroelectronics decided to use the Power State Coordination Interface
(PSCI) on all the STM32MP SoC (armv7 or armv8) even if PSCI it is only
required in AArch64 for Embedded Base Boot Requirements (EBBR) Specification
https://arm-software.github.io/ebbr/index.html#document-chapter3-secureworld
Patrick
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-10-08 13:20 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-09 14:55 [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2 Alexandru Gagniuc
2021-09-09 14:55 ` [PATCH 1/3] stm32mp: Rename FIP config to stm32mp15_tfaboot_fip_defconig Alexandru Gagniuc
2021-09-09 14:55 ` [PATCH 2/3] arm: Kconfig: Introduce a TFABOOT_FIP_CONTAINER option Alexandru Gagniuc
2021-09-09 14:55 ` [PATCH 3/3] stm32mp1: Replace STM32MP15x_STM32IMAGE with TFABOOT_FIP_CONTAINER Alexandru Gagniuc
2021-09-14 12:26 ` [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2 Patrick DELAUNAY
2021-10-07 17:13 ` Alex G.
2021-10-08 9:18 ` Patrick DELAUNAY
2021-10-08 11:28 ` Marek Vasut
2021-10-08 13:20 ` Patrick DELAUNAY
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.