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