| Message ID | 20210531174314.1395666-2-mr.nuke.me@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Patrick Delaunay |
| Headers | show |
| Series | stm32mp: Enable OP-TEE and TZC support in SPL | expand |
Hi, On 5/31/21 7:43 PM, Alexandru Gagniuc wrote: > In general, Falcon mode means we're booting a linux kernel directly. > With FIT images, however, an OP-TEE secure kernel can be booted before > linux. Thus, if the next stage is an IH_OS_TEE, this isn't necessarily > a problem. > > Of course, a general solution would involve mmc_load_image_raw_os() > only loading the binary, and leaving the decision of suitability to > someone else. However, a rework of the boot flow is beyond the scope > of this patch. Accept IH_OS_TEE as a valid OS value. > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> > Reviewed-by: Tom Rini <trini@konsulko.com> > Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com> > --- > common/spl/spl_mmc.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c > index add2785b4e3..bab558d055f 100644 > --- a/common/spl/spl_mmc.c > +++ b/common/spl/spl_mmc.c > @@ -230,8 +230,10 @@ static int mmc_load_image_raw_os(struct spl_image_info *spl_image, > if (ret) > return ret; > > - if (spl_image->os != IH_OS_LINUX) { > - puts("Expected Linux image is not found. Trying to start U-boot\n"); > + if (spl_image->os != IH_OS_LINUX && spl_image->os != IH_OS_TEE) { > + puts("Expected OS image is not found. Instead found "); > + puts(genimg_get_os_name(spl_image->os)); > + puts(". Trying to start U-boot\n"); > return -ENOENT; > } > When I merge this patch on master branch, I get the error: arm: + imx6dl_mamoj +===================== WARNING ====================== +This board does not use CONFIG_DM_USB (Driver Model +for USB). Please update the board to use +CONFIG_DM_USB before the v2019.07 release. Failure to +update by the deadline may result in board removal. +See doc/driver-model/migration.rst for more info. +==================================================== +spl/u-boot-spl.bin exceeds file size limit: + limit: 0xefa0 bytes + actual: 0xf41d bytes + excess: 0x47d bytes +make[1]: *** [Makefile:1997: spl/u-boot-spl.bin] Error 1 +make[1]: *** Deleting file 'spl/u-boot-spl.bin' +make: *** [Makefile:177: sub-make] Error 2 The overhead: $> tools/buildman/buildman -b TEST -c 2 imx6dl_mamoj $> tools/buildman/buildman -b TEST -c 2 imx6dl_mamoj -seS Summary of 2 commits for 1 boards (1 thread, 12 jobs per thread) 01: Merge branch '2021-07-14-build-and-host-updates' arm: w+ imx6dl_mamoj +===================== WARNING ====================== +This board does not use CONFIG_DM_USB (Driver Model +for USB). Please update the board to use +CONFIG_DM_USB before the v2019.07 release. Failure to +update by the deadline may result in board removal. +See doc/driver-model/migration.rst for more info. +==================================================== 02: spl: mmc: Support OP-TEE payloads in Falcon mode arm: + imx6dl_mamoj +spl/u-boot-spl.bin exceeds file size limit: + limit: 0xefa0 bytes + actual: 0xf41d bytes + excess: 0x47d bytes +make[1]: *** [Makefile:1997: spl/u-boot-spl.bin] Error 1 +make[1]: *** Deleting file 'spl/u-boot-spl.bin' +make: *** [Makefile:177: sub-make] Error 2 arm: (for 1/1 boards) spl/u-boot-spl:all +2242.0 spl/u-boot-spl:rodata +2162.0 spl/u-boot-spl:text +80.0 With details : diff 01_gc11f5abce8_Merge-branch-\'2021-0/imx6dl_mamoj/spl-u-boot-spl.sizes 02_g0620649bfa_spl--mmc--Support-OP/imx6dl_mamoj/spl-u-boot-spl.sizes 157a158,159 > 00000012 T get_table_entry > 00000012 T get_table_entry_name 171a174 > 00000014 T genimg_get_os_name 531a535 > 000000a8 r uimage_os 563d566 < 00000134 T spl_mmc_load 566a570 > 0000014c T spl_mmc_load This issue need to be solved before I accept and merge the serie. See also ./common/spl/spl_fit.c:514: ............ This saves some * space by omitting the large table of OS types. */ But I think again about the title of this patch : - spl: mmc: Support OP-TEE payloads in Falcon mode If I unterstood after the serie the sequence for MMC is: ROM code => SPL => TEE (as raw OS) => U-Boot but it is not really the Falcon mode and OP-TEE + falcon mode is not really supported... And the patch title is disturbing. For me the correct sequence is in Falcon mode is : ROM code => SPL => TEE (secure world) => kernel (normal world) With the TEE and the kernel loaded by the SPL...... and without falcon mode : (A) ROM code => SPL => TEE (secure world) => U-Boot or (B) ROM code => SPL (TZ) => U-Boot (TZ) execute bootm => TEE (secure world) => kernel what it your expected sequence in spl_load_simple_fit in this serie / in your defconfig ? Today with the normal world address can be: 1/ = spl_image->entry_point (bootm_os.c in U-Boot proprer) 2/ = CONFIG_SYS_TEXT_BASE (hardcoded for SPL in spl_optee.S) for 2/ When U-Boot is not used after SPL = in falcon mode, the LR register shoud be set to kernel entry point. Patrick
On 7/15/21 1:27 PM, Patrick DELAUNAY wrote: > Hi, [snip] > When I merge this patch on master branch, I get the error: > > arm: + imx6dl_mamoj > +spl/u-boot-spl.bin exceeds file size limit: > + limit: 0xefa0 bytes > + actual: 0xf41d bytes > + excess: 0x47d bytes > +make[1]: *** [Makefile:1997: spl/u-boot-spl.bin] Error 1 > +make[1]: *** Deleting file 'spl/u-boot-spl.bin' > +make: *** [Makefile:177: sub-make] Error 2 > This issue need to be solved before I accept and merge the serie. Okay, I'll have to drop the call to genimg_get_os_name(). > But I think again about the title of this patch : > > - spl: mmc: Support OP-TEE payloads in Falcon mode > > > If I unterstood after the serie the sequence for MMC is: > > ROM code => SPL => TEE (as raw OS) => U-Boot > > > but it is not really the Falcon mode and > OP-TEE + falcon mode is not really supported... > > > And the patch title is disturbing. > > > For me the correct sequence is in Falcon mode is : > > ROM code => SPL => TEE (secure world) => kernel (normal world) > This is exactly the use case that this patch intends to support. > > With the TEE and the kernel loaded by the SPL...... > > > and without falcon mode : > > (A) ROM code => SPL => TEE (secure world) => U-Boot > > or > > (B) ROM code => SPL (TZ) => U-Boot (TZ) execute bootm => TEE (secure > world) => kernel > > > what it your expected sequence in spl_load_simple_fit in this serie / in > your defconfig ? > > Today with the normal world address can be: > > 1/ = spl_image->entry_point (bootm_os.c in U-Boot proprer) > > 2/ = CONFIG_SYS_TEXT_BASE (hardcoded for SPL in spl_optee.S) > > > for 2/ When U-Boot is not used after SPL = in falcon mode, > > the LR register shoud be set to kernel entry point. How does SPL know where OP-TEE should jump to? One could parse the FIT image, and try to figure out which of the loadables is the kernel. But what if there's a linux and u-boot, with different "/configurations" nodes? Figuring out where OP-TEE wants to start the normal world is a hard problem that the u=boot infrastructure is not prepared for. The solution I'm using is to build OP-TEE with CFG_NS_ENTRY_ADDR=[linux entry addr] Then LR is irrelevant. Alex
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index add2785b4e3..bab558d055f 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -230,8 +230,10 @@ static int mmc_load_image_raw_os(struct spl_image_info *spl_image, if (ret) return ret; - if (spl_image->os != IH_OS_LINUX) { - puts("Expected Linux image is not found. Trying to start U-boot\n"); + if (spl_image->os != IH_OS_LINUX && spl_image->os != IH_OS_TEE) { + puts("Expected OS image is not found. Instead found "); + puts(genimg_get_os_name(spl_image->os)); + puts(". Trying to start U-boot\n"); return -ENOENT; }