Re: [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
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
Re: [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
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. [...]
Re: [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
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 0x2ffc, 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 0xde00. 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=
Re: [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
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 0x2ffc, 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 0xde00. 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= 0xde00\
CFG_DRAM_SIZE=0x2000 "
Alex
Re: [PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
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
[PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2
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
