[U-Boot] [U-Boot, v3, 07/11] spl: add support to booting with OP-TEE

[Kever Yang]

On 01/03/2018 01:20 AM, Philipp Tomsich wrote:
>
>
> On Tue, 19 Dec 2017, Kever Yang wrote:
>
>> OP-TEE is an open source trusted OS, in armv7, its loading and
>> running are like this:
>> loading:
>> - SPL load both OP-TEE and U-Boot
>> running:
>> - SPL run into OP-TEE in secure mode;
>> - OP-TEE run into U-Boot in non-secure mode;
>>
>> More detail:
>> https://github.com/OP-TEE/optee_os
>> and search for 'boot arguments' for detail entry parameter in:
>> core/arch/arm/kernel/generic_entry_a32.S
>>
>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>
> Reviewed-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
>
> See below for requested changed.
>
>> ---
>>
>> Changes in v3: None
>> Changes in v2:
>> - Using new image type for op-tee
>>
>> common/spl/Kconfig     |  7 +++++++
>> common/spl/Makefile    |  1 +
>> common/spl/spl.c       |  8 ++++++++
>> common/spl/spl_optee.S | 13 +++++++++++++
>> include/spl.h          |  9 +++++++++
>> 5 files changed, 38 insertions(+)
>> create mode 100644 common/spl/spl_optee.S
>>
>> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
>> index aef0034..c9a1ebb 100644
>> --- a/common/spl/Kconfig
>> +++ b/common/spl/Kconfig
>> @@ -725,6 +725,13 @@ config SPL_ATF
>>       is loaded by SPL(which is considered as BL2 in ATF terminology).
>>       More detail at: 
>> https://github.com/ARM-software/arm-trusted-firmware
>>
>> +config SPL_OPTEE
>> +    bool "Support OP-TEE Trusted OS"
>> +    depends on ARM
>> +    help
>> +      OP-TEE is an open source Trusted OS  which is loaded by SPL.
>> +      More detail at: https://github.com/OP-TEE/optee_os
>> +
>> config TPL
>>     bool
>>     depends on SUPPORT_TPL
>> diff --git a/common/spl/Makefile b/common/spl/Makefile
>> index 9bf8a2d..9918a2e 100644
>> --- a/common/spl/Makefile
>> +++ b/common/spl/Makefile
>> @@ -23,6 +23,7 @@ obj-$(CONFIG_$(SPL_TPL_)UBI) += spl_ubi.o
>> obj-$(CONFIG_$(SPL_TPL_)NET_SUPPORT) += spl_net.o
>> obj-$(CONFIG_$(SPL_TPL_)MMC_SUPPORT) += spl_mmc.o
>> obj-$(CONFIG_$(SPL_TPL_)ATF) += spl_atf.o
>> +obj-$(CONFIG_$(SPL_TPL_)OPTEE) += spl_optee.o
>> obj-$(CONFIG_$(SPL_TPL_)USB_SUPPORT) += spl_usb.o
>> obj-$(CONFIG_$(SPL_TPL_)FAT_SUPPORT) += spl_fat.o
>> obj-$(CONFIG_$(SPL_TPL_)EXT_SUPPORT) += spl_ext.o
>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> index 76c1963..2c8942b 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -436,6 +436,12 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>>         spl_invoke_atf(&spl_image);
>>         break;
>> #endif
>> +#if CONFIG_IS_ENABLED(OPTEE)
>> +    case IH_OS_OP_TEE:
>> +        debug("Jumping to U-Boot via OP-TEE\n");
>> +        spl_optee_entry(0, 0, 0, (void *)spl_image.entry_point);
>
> Why are you passing '0' for "arg2, device tree address"? This should
> be the device-tree pointer from the spl_image...

I will add dt pointer for entry next version.
>
> Also: 0 is not the same as NULL ... the arguments are all defined to
> have void* type, so 0 is not the correct type.
>
>> +        break;
>> +#endif
>> #ifdef CONFIG_SPL_OS_BOOT
>>     case IH_OS_LINUX:
>>         debug("Jumping to Linux\n");
>> @@ -450,6 +456,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>>     debug("SPL malloc() used %#lx bytes (%ld KB)\n", gd->malloc_ptr,
>>           gd->malloc_ptr / 1024);
>> #endif
>> +
>> +    debug("loaded - jumping to U-Boot...\n");
>> #ifdef CONFIG_BOOTSTAGE_STASH
>>     int ret;
>>
>> diff --git a/common/spl/spl_optee.S b/common/spl/spl_optee.S
>> new file mode 100644
>> index 0000000..4f7f8ba
>> --- /dev/null
>> +++ b/common/spl/spl_optee.S
>> @@ -0,0 +1,13 @@
>> +/*
>> + * Copyright (C) 2017 Rockchip Electronic Co.,Ltd
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <linux/linkage.h>
>> +#include <asm/assembler.h>
>> +
>> +ENTRY(spl_optee_entry)
>> +    ldr lr, =CONFIG_SYS_TEXT_BASE
>> +    mov pc, r3
>> +ENDPROC(spl_optee_entry)
>> diff --git a/include/spl.h b/include/spl.h
>> index c14448b..551ce43 100644
>> --- a/include/spl.h
>> +++ b/include/spl.h
>> @@ -288,6 +288,15 @@ int spl_mmc_load_image(struct spl_image_info 
>> *spl_image,
>> void spl_invoke_atf(struct spl_image_info *spl_image);
>>
>> /**
>> + * spl_optee_entry - entry function for optee
>> + * entry arg0, pagestore
>> + * entry arg1, (ARMv7 standard bootarg #1)
>> + * entry arg2, device tree address, (ARMv7 standard bootarg #2)
>> + * entry arg3, non-secure entry address (ARMv7 bootarg #0)
>> + */
>> +void spl_optee_entry(void *arg0, void *arg1, void *arg2, void *arg3);
>
> This doesn't look like a documentation comment for a U-Boot function.
> Also: the purpose of arg0 and arg1 are not clear from this comment.

This is a copy from OP-TEE project, we only use the entry address now.
https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/kernel/generic_entry_a32.S#L208

Thanks,
- Kever
>
>> +
>> +/**
>>  * board_return_to_bootrom - allow for boards to continue with the 
>> boot ROM
>>  *
>>  * If a board (e.g. the Rockchip RK3368 boards) provide some
>>
>