[PATCH] ARM: stm32: Initialize TAMP_SMCR BKP..PROT fields on STM32MP15xx

[Marek Vasut]

On 4/15/24 11:48 AM, Patrice CHOTARD wrote:
> 
> 
> On 4/14/24 20:39, 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.
>>
>> 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 <marex at denx.de>
>> ---
>> Cc: Igor Opaniuk <igor.opaniuk at foundries.io>
>> Cc: Patrice Chotard <patrice.chotard at foss.st.com>
>> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
>> Cc: Simon Glass <sjg at chromium.org
>> Cc: Simon Glass <sjg at chromium.org>
>> Cc: Tom Rini <trini at konsulko.com>
>> Cc: u-boot at dh-electronics.com
>> Cc: uboot-stm32 at st-md-mailman.stormreply.com
>> ---
>>   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..138a6d6b614 100644
>> --- a/arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c
>> +++ b/arch/arm/mach-stm32mp/stm32mp1/stm32mp15x.c
>> @@ -14,6 +14,7 @@
>>   #include <asm/arch/sys_proto.h>
>>   #include <dm/device.h>
>>   #include <dm/uclass.h>
>> +#include <linux/bitfield.h>
>>   
>>   /* 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_BKPWDPROT	GENMASK(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_BKPRWDPROT,
> 
> Hi Marek
> 
> there is a typo, you used twice TAMP_SMCR_BKPRWDPROT :
> 
> TAMP_SMCR_BKPRWDPROT | TAMP_SMCR_BKPRWDPROT  => TAMP_SMCR_BKPRWDPROT | TAMP_SMCR_BKPWDPROT
>                                      ^~~~~

Fixed in V2, thanks.

btw are there any other such undocumented surprises in the BootROM ?