Re: [PATCH v2] ARM: stm32: Initialize TAMP_SMCR BKP..PROT fields on STM32MP15xx
On 4/18/24 8:24 PM, Patrick DELAUNAY wrote:
Hi,
Hi,
[...]
@@ -136,6 +140,18 @@ static void security_init(void)
*/
writel(0x0, TAMP_CR1);
+ /*
+ * TAMP: Configure non-zero secure protection settings. This is
+ * checked by BootROM function 35ac on OTP-CLOSED device during
+ * CPU core 1 release from endless loop. If secure protection
+ * fields are zero, the core 1 is not released from endless
+ * loop on second SGI0.
+ */
+ clrsetbits_le32(TAMP_SMCR,
+ TAMP_SMCR_BKPRWDPROT | TAMP_SMCR_BKPWDPROT,
+ FIELD_PREP(TAMP_SMCR_BKPRWDPROT, 0x10) |
+ FIELD_PREP(TAMP_SMCR_BKPWDPROT, 0x10));
+
/* GPIOZ: deactivate the security */
writel(BIT(0), RCC_MP_AHB5ENSETR);
writel(0x0, GPIOZ_SECCFGR);
The recommended mapping (the mapping done in OP-TEE for OpenSTLinux) is
described in Wiki page
- 10 backup register secure
- 4 backup register secure write / non secure read
- 17 backup register Non-secure
It is done in
https://github.com/STMicroelectronics/optee_os/blob/3.19.0-stm32mp/core/arch/arm/plat-stm32mp1/main.c
with
static TEE_Result stm32_configure_tamp(void)
{
TEE_Result res __maybe_unused = TEE_SUCCESS;
struct stm32_bkpregs_conf bkpregs_conf = {
.nb_zone1_regs = 10, /* 10 registers in zone 1 */
.nb_zone2_regs = 5 /* 5 registers in zone 2 */
/* Zone3 all remaining */
};
/* Enable BKP Register protection */
if (stm32_tamp_set_secure_bkpregs(_conf))
panic();
But when you are booting with SPL U-boot, all the boot chain and the
Linux kernel
is running in secure world
So you have no reason to manage any limit for the access to backup
register,
you can allocate all the backup registers (the 32 one) to secure world
See "Figure 552. Backup registers secure protections" in reference mnauel
Protection zone 1 => x = 31 with BKPRWDPROT = 31
Protection zone 2 & 3 => empty
+ clrsetbits_le32(TAMP_SMCR,
+ TAMP_SMCR_BKPRWDPROT | TAMP_SMCR_BKPWDPROT,
+ FIELD_PREP(TAMP_SMCR_BKPRWDPROT, 0x20) |
+ FIELD_PREP(TAMP_SMCR_BKPWDPROT, 0x20));
Sorry for the delay, I need also to check on my side
But anyway your proposal is functional,
So with or without the previous remark
Thank you for the detailed explanation and for checking this.
V3 is coming now.
Re: [PATCH v2] ARM: stm32: Initialize TAMP_SMCR BKP..PROT fields on STM32MP15xx
Hi,
On 4/15/24 14:55, Marek Vasut wrote:
In case of an OTP-CLOSED STM32MP15xx system, the CPU core 1 cannot be
released from endless loop in BootROM only by populating TAMP BKPxR 4
and 5 with magic and branch address and sending SGI0 interrupt from
core 0 to core 1 twice. TAMP_SMCR BKP..PROT fields must be initialized
as well to release the core 1 from endless loop during the second SGI0
handling on core 1. Initialize TAMP_SMCR to protect the first 16 backup
registers, the ones which contain the core 1 magic, branch address and
boot information.
This requirement seems to be undocumented, therefore it was necessary
to trace and analyze the STM32MP15xx BootROM using OpenOCD and objdump.
Ultimately, it turns out that a certain BootROM function reads out the
TAMP_SMCR register and tests whether the BKP..PROT fields are non-zero.
If they are zero, the BootROM code again waits for SGI0 using WFI, else
the execution moves forward until it reaches handoff to the TAMP BKPxR 5
branch address.
These backup registers are documented in
https://wiki.st.com/stm32mpu/wiki/STM32MP15_backup_registers
This "security" configuration is done in STMicoelectronics delivery
(OpenSTLinux) in OP-TEE.
This fixes CPU core 1 release using U-Boot PSCI implementation on an
OTP-CLOSED system, i.e. system with fuse 0 bit 6 set.
A ROM code security check is done only for closed device to avoid malicious
code execution: "unsecure" code on CPU2 during wake-up by changing
BRANCH_ADDRESS
=> the STM32MP15 ROM check that only the secure world can update
the TAMP_BKP5R = BRANCH_ADDRESS
before to start the CPU2 and jump to this address.
Sorry to inconvenient, we will improve this part on next release
= OpenSTLinux V5.1
Reviewed-by: Patrick Delaunay
Thanks
Patrick
Signed-off-by: Marek Vasut
---
Cc: Igor Opaniuk
Cc: Patrice Chotard
Cc: Patrick Delaunay
Cc: Simon Glass
Cc: Tom Rini
Cc: u-b...@dh-electronics.com
Cc: uboot-st...@st-md-mailman.stormreply.com
---
V2: Fix up the BKPRWD/BKPWD mask typo
---
arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c | 16
1 file changed, 16 insertions(+)
diff --git a/arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c
b/arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c
index dd99150fbc2..a2496361e01 100644
--- a/arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c
+++ b/arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
/* RCC register */
#define RCC_TZCR (STM32_RCC_BASE + 0x00)
@@ -41,6 +42,9 @@
#define TZC_REGION_ID_ACCESS0 (STM32_TZC_BASE + 0x114)
#define TAMP_CR1 (STM32_TAMP_BASE + 0x00)
+#define TAMP_SMCR (STM32_TAMP_BASE + 0x20)
+#define TAMP_SMCR_BKPRWDPROT GENMASK(7, 0)
+#define TAMP_SMCR_BKPWDPROTGENMASK(23, 16)
#define PWR_CR1 (STM32_PWR_BASE + 0x00)
#define PWR_MCUCR (STM32_PWR_BASE + 0x14)
@@ -136,6 +140,18 @@ static void security_init(void)
*/
writel(0x0, TAMP_CR1);
+ /*
+* TAMP: Configure non-zero secure protection settings. This is
+* checked by BootROM function 35ac on OTP-CLOSED device during
+* CPU core 1 release from endless loop. If secure protection
+* fields are zero, the core 1 is not released from endless
+* loop on second SGI0.
+*/
+ clrsetbits_le32(TAMP_SMCR,
+ TAMP_SMCR_BKPRWDPROT | TAMP_SMCR_BKPWDPROT,
+ FIELD_PREP(TAMP_SMCR_BKPRWDPROT, 0x10) |
+ FIELD_PREP(TAMP_SMCR_BKPWDPROT, 0x10));
+
/* GPIOZ: deactivate the security */
writel(BIT(0), RCC_MP_AHB5ENSETR);
writel(0x0, GPIOZ_SECCFGR);
The recommended mapping (the mapping done in OP-TEE for OpenSTLinux) is
described in Wiki page
- 10 backup register secure
- 4 backup register secure write / non secure read
- 17 backup register Non-secure
It is done in
https://github.com/STMicroelectronics/optee_os/blob/3.19.0-stm32mp/core/arch/arm/plat-stm32mp1/main.c
with
static TEE_Result stm32_configure_tamp(void)
{
TEE_Result res __maybe_unused = TEE_SUCCESS;
struct stm32_bkpregs_conf bkpregs_conf = {
.nb_zone1_regs = 10, /* 10 registers in zone 1 */
.nb_zone2_regs = 5 /* 5 registers in zone 2 */
/* Zone3 all remaining */
};
/* Enable BKP Register protection */
if (stm32_tamp_set_secure_bkpregs(_conf))
panic();
But when you are booting with SPL U-boot, all the boot chain and the
Linux kernel
is running in secure world
So you have no reason to manage any limit for the access to backup
register,
you can allocate all the backup registers (the 32 one) to secure world
See "Figure 552. Backup registers secure protections" in reference mnauel
Protection zone 1 => x = 31 with BKPRWDPROT = 31
Protection zone 2 & 3 => empty
+ clrsetbits_le32(TAMP_SMCR,
+ TAMP_SMCR_BKPRWDPROT | TAMP_SMCR_BKPWDPROT,
+
[PATCH v2] ARM: stm32: Initialize TAMP_SMCR BKP..PROT fields on STM32MP15xx
In case of an OTP-CLOSED STM32MP15xx system, the CPU core 1 cannot be released from endless loop in BootROM only by populating TAMP BKPxR 4 and 5 with magic and branch address and sending SGI0 interrupt from core 0 to core 1 twice. TAMP_SMCR BKP..PROT fields must be initialized as well to release the core 1 from endless loop during the second SGI0 handling on core 1. Initialize TAMP_SMCR to protect the first 16 backup registers, the ones which contain the core 1 magic, branch address and boot information. This requirement seems to be undocumented, therefore it was necessary to trace and analyze the STM32MP15xx BootROM using OpenOCD and objdump. Ultimately, it turns out that a certain BootROM function reads out the TAMP_SMCR register and tests whether the BKP..PROT fields are non-zero. If they are zero, the BootROM code again waits for SGI0 using WFI, else the execution moves forward until it reaches handoff to the TAMP BKPxR 5 branch address. This fixes CPU core 1 release using U-Boot PSCI implementation on an OTP-CLOSED system, i.e. system with fuse 0 bit 6 set. Signed-off-by: Marek Vasut --- Cc: Igor Opaniuk Cc: Patrice Chotard Cc: Patrick Delaunay Cc: Simon Glass Cc: Tom Rini Cc: u-b...@dh-electronics.com Cc: uboot-st...@st-md-mailman.stormreply.com --- V2: Fix up the BKPRWD/BKPWD mask typo --- arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c | 16 1 file changed, 16 insertions(+) diff --git a/arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c b/arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c index dd99150fbc2..a2496361e01 100644 --- a/arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c +++ b/arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c @@ -14,6 +14,7 @@ #include #include #include +#include /* RCC register */ #define RCC_TZCR (STM32_RCC_BASE + 0x00) @@ -41,6 +42,9 @@ #define TZC_REGION_ID_ACCESS0 (STM32_TZC_BASE + 0x114) #define TAMP_CR1 (STM32_TAMP_BASE + 0x00) +#define TAMP_SMCR (STM32_TAMP_BASE + 0x20) +#define TAMP_SMCR_BKPRWDPROT GENMASK(7, 0) +#define TAMP_SMCR_BKPWDPROTGENMASK(23, 16) #define PWR_CR1(STM32_PWR_BASE + 0x00) #define PWR_MCUCR (STM32_PWR_BASE + 0x14) @@ -136,6 +140,18 @@ static void security_init(void) */ writel(0x0, TAMP_CR1); + /* +* TAMP: Configure non-zero secure protection settings. This is +* checked by BootROM function 35ac on OTP-CLOSED device during +* CPU core 1 release from endless loop. If secure protection +* fields are zero, the core 1 is not released from endless +* loop on second SGI0. +*/ + clrsetbits_le32(TAMP_SMCR, + TAMP_SMCR_BKPRWDPROT | TAMP_SMCR_BKPWDPROT, + FIELD_PREP(TAMP_SMCR_BKPRWDPROT, 0x10) | + FIELD_PREP(TAMP_SMCR_BKPWDPROT, 0x10)); + /* GPIOZ: deactivate the security */ writel(BIT(0), RCC_MP_AHB5ENSETR); writel(0x0, GPIOZ_SECCFGR); -- 2.43.0
