[PATCH 0/3] stm32mp: Attempt to resolve unintended breakage with v2021.10-rc2

[Patrick DELAUNAY]

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