stm32mp: The purpose of "!tee_find_device()"

[Alex G.]

On 10/30/20 3:28 AM, Etienne Carriere wrote:
> On Thu, 29 Oct 2020 at 15:33, Alex G. <mr.nuke.me at gmail.com> wrote:
>>
>> On 9/30/20 6:03 PM, Alex G. wrote:
>>> Hi
>>>
>>> I'm trying to wrap my head around the purpose of the following lines in
>>> ft_system_setup():
>>>
>>>       if (!CONFIG_IS_ENABLED(OPTEE) ||
>>>           !tee_find_device(NULL, NULL, NULL, NULL))
>>>           stm32_fdt_disable_optee(blob);
>>
>> Hi! Me again! Do we have a (good) reason for this, or should I submit a
>> patch to remove this problematic code?
>>
>> Alex
>>
>>> My interpretation is "if optee is not running, delete the FDT node".
>>> The problem is that tee_find_device() invokes device_probe(). This in
>>> turn does an SMC call. This call results in an abort and reboot if optee
>>> is not running in the first place.
>>>
>>> So I don't think that tee_find_device() can be used as a check for "Is
>>> optee running?". Exhibit B: Outside of mach-stm32mp, tee_find_device()
>>> is used to obtain of a _working_ TEE node, not to ask if "is optee
>>> running?".
>>>
>>>
>>> My problem is that trying to start linux with CONFIG_OPTEE=y will cause
>>> the bootm command to crash (log in appendix A):
>>>
>>>       
>>>       load mmc 0:7 $fdt_addr_r boot/stm32mp157c-dk2.dtb
>>>       load mmc 0:7 0xc8000000 boot/utee
>>>       setenv bootm_boot_mode sec
>>>       bootm 0xc8000000 - $fdt_addr_r
>>>
>>> What is the intent of calling tee_find_device() in an FDT fixup
>>> function?
> 
> The scheme is the generic U-Boot implementation do copy OP-TEE
> related nodes when found in its FDT to the FDT provided to Linux.
> (called from common/image-fdt.c)
> 
> However stm32mp1 can be used with or without OP-TEE installed. To
> get a generic stm32mp1/U-Boot image that support both configurations
> (with and w/o OP-TEE installed), U-Boot FDT and config for this plaform
> do enable OP-TEE but, at u-boot runtime, if we find OP-TEE's not present,
> we remove the FTD node so that Linux does get it and expect OP-TEE
> is present.
> 
>>> Do you have any ideas how to make it not crash (short of
>>> commenting out the problem lines) ?
> 
> The crash seems due to that there is no secure monitor by the time
> you have this sequence called. Secure monitor is the code that
> handles the SMC. If none installed, SMCs ends nowhere and
> likely badly crash the systel. If OP-TEE is not running but there
> is a secure monitor loaded, it should not crash.
> 
> It seems to me that U-Boot does set up a secure monitor for
> PSCI minimal support, so the U-Boot PSCI stack should
> nicely handle the SMC to report that there is no OP-TEE installed.
> Enabling CONFIG_ARMV7_PSCI should fix the issue I think.

Hi Etienne. I understand the reasoning behind this, and I agree that 
things shouldn't break in theory. However,I just double-checked this 
with master (7a8ac9df5d). I think we have a bug on our hands:

stm32mp15_basic_defconfig, with the following changes:

	CONFIG_TEE=y
	CONFIG_OPTEE=y
	CONFIG_OPTEE_TZDRAM_SIZE=0x01000000
	# CONFIG_SYSRESET_CMD_POWEROFF is not set

I double-checked that CONFIG_ARMV7_PSCI is indeed set. The following 
sequence will cause a bad mode abort:

	> load mmc 0:7 $loadaddr boot/uImage
	> load mmc 0:7 $fdt_addr_r boot/stm32mp157c-dk2.dtb
	> setenv bootargs console=ttySTM0 root=/dev/mmcblk0p7
	> bootm $loadaddr - $fdt_addr_r

Alex



> Regards,
> Etienne
> 
>>>
>>> Alex
>>>
>>>
>>> Appendix A: u-boot log after bootm command
>>>
>>> ## Booting kernel from Legacy Image at c8000000 ...
>>>      Image Name:
>>>      Created:      2020-09-28  20:58:56 UTC
>>>      Image Type:   ARM Trusted Execution Environment Kernel Image
>>> (uncompressed)
>>>      Data Size:    349276 Bytes = 341.1 KiB
>>>      Load Address: fdffffe4
>>>      Entry Point:  fe000000
>>>      Verifying Checksum ... OK
>>>      Loading Kernel Image
>>> ## Flattened Device Tree blob at c4000000
>>>      Booting using the fdt blob at 0xc4000000
>>>      Loading Device Tree to cffef000, end cffff5e2 ... OK
>>> <BOARD RESETS WITHOUT USER INPUT>
>>> U-Boot SPL 2020.10-rc4 (Sep 20 2020 - 23:46:47 +0000)
>>> Model: STMicroelectronics STM32MP157C-DK2 Discovery Board