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

[Marek Vasut]

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(&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.