[PATCH v4 1/5] spl: mmc: Support OP-TEE payloads in Falcon mode

[Patrick DELAUNAY]

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 at gmail.com>
> Reviewed-by: Tom Rini <trini at konsulko.com>
> Reviewed-by: Patrick Delaunay <patrick.delaunay at 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