stm32mp: The purpose of "!tee_find_device()"

[Etienne Carriere]

On Tue, 3 Nov 2020 at 16:53, Alex G. <mr.nuke.me at gmail.com> wrote:
>
> 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

My apologies, I did'nt notice you use the _basic_defconfig.
With this defconfig, U-Boot cannot invoke OP-TEE services since OP-TEE
is booted only once U-Boot execution is completed (once bootm
command jumps to OP-TEE boot entry). So in this config U-Boot cannot
find the OP-TEE node.

Maybe ft_system_setup() (mach-stm32mp/fdt.c) should bypass OP-TEE
auto probing when CONFIG_BOOTM_OPTEE is enabled.

That would probably require to change stm32mp15_trusted_defconfig
to explicitly disable CONFIG_BOOTM_OPTEE, since this config
does not allow booting OP-TEE from U-Boot.

Patrick, your opinion?

FYI Alex, stm32mp15_basic_defconfig is not well suited to boot OP-TEE.
Consider using stm32mp15_trusted_defconfig instead. The "Trusted"
means U-Boot is booted after secure world. SPL could still be used
to load/boot OP-TEE but stm32mp15_trusted_defconfig does not
provide such support in current U-Boot. TF-A is an alternative to SPL
in this case.

br,
etienne




>         # 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