[PATCH v2 0/9] spl: atf: add support for LOAD_IMAGE_V2

Newer TF-A versions provide a new image loading protocol. This is used on (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 -> u-boot. With this series it is possible that U-Boot SPL loads the bl31 directly and thus replacing bl1 and bl2 from the TF-A.
This was tested on the Kontron sl28 board using NXPs bl31 and the upstream version of the OP-TEE Trusted OS.
Changes since v1: - removed firmware entry from loadable, suggested by Michal - use kernel-doc function annotation format - new patch "board: sl28: remove u-boot from loadable DT node"
Michael Walle (9): treewide: use CONFIG_IS_ENABLED() for ARMV8_SEC_FIRMWARE_SUPPORT spl: atf: move storage for bl31_params into function spl: atf: provide a bl2_plat_get_bl31_params_default() spl: atf: remove helper structure from common header spl: atf: add support for LOAD_IMAGE_V2 armv8: layerscape: don't initialize GIC in SPL board: sl28: remove u-boot from loadable DT node board: sl28: add ATF support (bl31) board: sl28: add OP-TEE Trusted OS support (bl32)
arch/arm/cpu/armv8/cpu-dt.c | 2 +- arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 8 +- arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S | 2 + arch/arm/cpu/armv8/fsl-layerscape/ppa.c | 2 +- .../dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi | 80 ++++++++++- arch/arm/lib/bootm-fdt.c | 2 +- arch/arm/lib/psci-dt.c | 6 +- board/kontron/sl28/Kconfig | 33 +++++ board/kontron/sl28/Makefile | 6 +- board/kontron/sl28/sl28.c | 7 + board/kontron/sl28/spl_atf.c | 54 ++++++++ common/spl/Kconfig | 9 ++ common/spl/spl_atf.c | 129 ++++++++++++++++-- include/atf_common.h | 42 ++++-- include/spl.h | 78 +++++++++-- 15 files changed, 410 insertions(+), 50 deletions(-) create mode 100644 board/kontron/sl28/spl_atf.c

There is SPL_ARMV8_SEC_FIRMWARE_SUPPORT and ARMV8_SEC_FIRMWARE_SUPPORT. Thus use CONFIG_IS_ENABLED() instead of the simple #ifdef.
Signed-off-by: Michael Walle michael@walle.cc Acked-by: Michal Simek michal.simek@xilinx.com --- arch/arm/cpu/armv8/cpu-dt.c | 2 +- arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 8 ++++---- arch/arm/cpu/armv8/fsl-layerscape/ppa.c | 2 +- arch/arm/lib/bootm-fdt.c | 2 +- arch/arm/lib/psci-dt.c | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm/cpu/armv8/cpu-dt.c b/arch/arm/cpu/armv8/cpu-dt.c index 97d4473a68..61c38b17cb 100644 --- a/arch/arm/cpu/armv8/cpu-dt.c +++ b/arch/arm/cpu/armv8/cpu-dt.c @@ -9,7 +9,7 @@ #include <asm/system.h> #include <asm/armv8/sec_firmware.h>
-#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT +#if CONFIG_IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT) int psci_update_dt(void *fdt) { /* diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c index 6d3391db3b..598ee2ffa2 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c @@ -26,7 +26,7 @@ #endif #include <fsl_sec.h> #include <asm/arch-fsl-layerscape/soc.h> -#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT +#if CONFIG_IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT) #include <asm/armv8/sec_firmware.h> #endif #include <asm/arch/speed.h> @@ -81,7 +81,7 @@ void ft_fixup_cpu(void *blob) "device_type", "cpu", 4); }
-#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) && \ +#if CONFIG_IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT) && \ defined(CONFIG_SEC_FIRMWARE_ARMV8_PSCI) int node; u32 psci_ver; @@ -383,7 +383,7 @@ static void fdt_fixup_msi(void *blob) } #endif
-#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT +#if CONFIG_IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT) /* Remove JR node used by SEC firmware */ void fdt_fixup_remove_jr(void *blob) { @@ -488,7 +488,7 @@ void ft_cpu_setup(void *blob, struct bd_info *bd) else { ccsr_sec_t __iomem *sec;
-#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT +#if CONFIG_IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT) fdt_fixup_remove_jr(blob); fdt_fixup_kaslr(blob); #endif diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c index 1ddb267093..2285296ea0 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c @@ -16,7 +16,7 @@ #elif defined(CONFIG_FSL_LSCH2) #include <asm/arch/immap_lsch2.h> #endif -#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT +#if CONFIG_IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT) #include <asm/armv8/sec_firmware.h> #endif #ifdef CONFIG_CHAIN_OF_TRUST diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c index 02a49a8e10..fe46a7d7c9 100644 --- a/arch/arm/lib/bootm-fdt.c +++ b/arch/arm/lib/bootm-fdt.c @@ -63,7 +63,7 @@ int arch_fixup_fdt(void *blob) #endif
#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV8_PSCI) || \ - defined(CONFIG_SEC_FIRMWARE_ARMV8_PSCI) + CONFIG_IS_ENABLED(SEC_FIRMWARE_ARMV8_PSCI) ret = psci_update_dt(blob); if (ret) return ret; diff --git a/arch/arm/lib/psci-dt.c b/arch/arm/lib/psci-dt.c index 0ed29a43f1..903b335704 100644 --- a/arch/arm/lib/psci-dt.c +++ b/arch/arm/lib/psci-dt.c @@ -10,7 +10,7 @@ #include <linux/sizes.h> #include <linux/kernel.h> #include <asm/psci.h> -#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT +#if CONFIG_IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT) #include <asm/armv8/sec_firmware.h> #endif
@@ -64,7 +64,7 @@ int fdt_psci(void *fdt) return nodeoff;
init_psci_node: -#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT +#if CONFIG_IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT) psci_ver = sec_firmware_support_psci_version(); #elif defined(CONFIG_ARMV7_PSCI_1_0) || defined(CONFIG_ARMV8_PSCI) psci_ver = ARM_PSCI_VER_1_0; @@ -85,7 +85,7 @@ init_psci_node: return tmp; }
-#ifndef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT +#if !CONFIG_IS_ENABLED(ARMV8_SEC_FIRMWARE_SUPPORT) /* * The Secure firmware framework isn't able to support PSCI version 0.1. */

On Wed, Nov 18, 2020 at 05:45:54PM +0100, Michael Walle wrote:
There is SPL_ARMV8_SEC_FIRMWARE_SUPPORT and ARMV8_SEC_FIRMWARE_SUPPORT. Thus use CONFIG_IS_ENABLED() instead of the simple #ifdef.
Signed-off-by: Michael Walle michael@walle.cc Acked-by: Michal Simek michal.simek@xilinx.com
Applied to u-boot/next, thanks!

There is no need to have the storage available globally. This is also a preparation for LOAD_IMAGE_V2 support. That will introduce a similar generator function which also has its own storage.
Signed-off-by: Michael Walle michael@walle.cc Acked-by: Michal Simek michal.simek@xilinx.com --- common/spl/spl_atf.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c index 9bd25f6b32..df0a198d55 100644 --- a/common/spl/spl_atf.c +++ b/common/spl/spl_atf.c @@ -18,13 +18,12 @@ #include <spl.h> #include <asm/cache.h>
-static struct bl2_to_bl31_params_mem bl31_params_mem; -static struct bl31_params *bl2_to_bl31_params; - __weak struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry, uintptr_t bl33_entry, uintptr_t fdt_addr) { + static struct bl2_to_bl31_params_mem bl31_params_mem; + struct bl31_params *bl2_to_bl31_params; struct entry_point_info *bl32_ep_info; struct entry_point_info *bl33_ep_info;

On Wed, Nov 18, 2020 at 05:45:55PM +0100, Michael Walle wrote:
There is no need to have the storage available globally. This is also a preparation for LOAD_IMAGE_V2 support. That will introduce a similar generator function which also has its own storage.
Signed-off-by: Michael Walle michael@walle.cc Acked-by: Michal Simek michal.simek@xilinx.com
Applied to u-boot/next, thanks!

Move the actual implementation of the bl2_plat_get_bl31_params() to its own function. The weak function will just call the default implementation. This has the advantage that board code can still call the original implementation if it just want to modify minor things.
Signed-off-by: Michael Walle michael@walle.cc --- common/spl/spl_atf.c | 14 +++++++++++--- include/spl.h | 43 +++++++++++++++++++++++++++++++------------ 2 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c index df0a198d55..63af6a6207 100644 --- a/common/spl/spl_atf.c +++ b/common/spl/spl_atf.c @@ -18,9 +18,9 @@ #include <spl.h> #include <asm/cache.h>
-__weak struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry, - uintptr_t bl33_entry, - uintptr_t fdt_addr) +struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry, + uintptr_t bl33_entry, + uintptr_t fdt_addr) { static struct bl2_to_bl31_params_mem bl31_params_mem; struct bl31_params *bl2_to_bl31_params; @@ -77,6 +77,14 @@ __weak struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry, return bl2_to_bl31_params; }
+__weak struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry, + uintptr_t bl33_entry, + uintptr_t fdt_addr) +{ + return bl2_plat_get_bl31_params_default(bl32_entry, bl33_entry, + fdt_addr); +} + static inline void raw_write_daif(unsigned int daif) { __asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory"); diff --git a/include/spl.h b/include/spl.h index b72dfc7e3d..fd928377f0 100644 --- a/include/spl.h +++ b/include/spl.h @@ -526,25 +526,44 @@ int spl_ymodem_load_image(struct spl_image_info *spl_image, void spl_invoke_atf(struct spl_image_info *spl_image);
/** - * bl2_plat_get_bl31_params() - prepare params for bl31. - * @bl32_entry address of BL32 executable (secure) - * @bl33_entry address of BL33 executable (non secure) - * @fdt_addr address of Flat Device Tree + * bl2_plat_get_bl31_params() - return params for bl31. + * @bl32_entry: address of BL32 executable (secure) + * @bl33_entry: address of BL33 executable (non secure) + * @fdt_addr: address of Flat Device Tree * - * This function assigns a pointer to the memory that the platform has kept - * aside to pass platform specific and trusted firmware related information - * to BL31. This memory is allocated by allocating memory to - * bl2_to_bl31_params_mem structure which is a superset of all the - * structure whose information is passed to BL31 - * NOTE: This function should be called only once and should be done - * before generating params to BL31 + * This is a weak function which might be overridden by the board code. By + * default it will just call bl2_plat_get_bl31_params_default(). * - * @return bl31 params structure pointer + * If you just want to manipulate or add some parameters, you can override + * this function, call bl2_plat_get_bl31_params_default and operate on the + * returned bl31 params. + * + * Return: bl31 params structure pointer */ struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry, uintptr_t bl33_entry, uintptr_t fdt_addr);
+/** + * bl2_plat_get_bl31_params_default() - prepare params for bl31. + * @bl32_entry: address of BL32 executable (secure) + * @bl33_entry: address of BL33 executable (non secure) + * @fdt_addr: address of Flat Device Tree + * + * This is the default implementation of bl2_plat_get_bl31_params(). It assigns + * a pointer to the memory that the platform has kept aside to pass platform + * specific and trusted firmware related information to BL31. This memory is + * allocated by allocating memory to bl2_to_bl31_params_mem structure which is + * a superset of all the structure whose information is passed to BL31 + * + * NOTE: The memory is statically allocated, thus this function should be + * called only once. All subsequent calls will overwrite any changes. + * + * Return: bl31 params structure pointer + */ +struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry, + uintptr_t bl33_entry, + uintptr_t fdt_addr); /** * spl_optee_entry - entry function for optee *

On Wed, Nov 18, 2020 at 05:45:56PM +0100, Michael Walle wrote:
Move the actual implementation of the bl2_plat_get_bl31_params() to its own function. The weak function will just call the default implementation. This has the advantage that board code can still call the original implementation if it just want to modify minor things.
Signed-off-by: Michael Walle michael@walle.cc
Applied to u-boot/next, thanks!

bl2_to_bl31_params_mem is just an implementation detail of the SPL ATF support and is not needed anywhere else. Move it from the header to the actual module.
Signed-off-by: Michael Walle michael@walle.cc Acked-by: Michal Simek michal.simek@xilinx.com --- common/spl/spl_atf.c | 11 +++++++++++ include/atf_common.h | 14 -------------- 2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c index 63af6a6207..51b45d5dc6 100644 --- a/common/spl/spl_atf.c +++ b/common/spl/spl_atf.c @@ -18,6 +18,17 @@ #include <spl.h> #include <asm/cache.h>
+/* Holds all the structures we need for bl31 parameter passing */ +struct bl2_to_bl31_params_mem { + struct bl31_params bl31_params; + struct atf_image_info bl31_image_info; + struct atf_image_info bl32_image_info; + struct atf_image_info bl33_image_info; + struct entry_point_info bl33_ep_info; + struct entry_point_info bl32_ep_info; + struct entry_point_info bl31_ep_info; +}; + struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry, uintptr_t bl33_entry, uintptr_t fdt_addr) diff --git a/include/atf_common.h b/include/atf_common.h index fd5454c55b..e173a10ca9 100644 --- a/include/atf_common.h +++ b/include/atf_common.h @@ -162,20 +162,6 @@ struct bl31_params { struct atf_image_info *bl33_image_info; };
-/******************************************************************************* - * This structure represents the superset of information that is passed to - * BL31, e.g. while passing control to it from BL2, bl31_params - * and other platform specific params - ******************************************************************************/ -struct bl2_to_bl31_params_mem { - struct bl31_params bl31_params; - struct atf_image_info bl31_image_info; - struct atf_image_info bl32_image_info; - struct atf_image_info bl33_image_info; - struct entry_point_info bl33_ep_info; - struct entry_point_info bl32_ep_info; - struct entry_point_info bl31_ep_info; -};
#endif /*__ASSEMBLY__ */

On Wed, Nov 18, 2020 at 05:45:57PM +0100, Michael Walle wrote:
bl2_to_bl31_params_mem is just an implementation detail of the SPL ATF support and is not needed anywhere else. Move it from the header to the actual module.
Signed-off-by: Michael Walle michael@walle.cc Acked-by: Michal Simek michal.simek@xilinx.com
Applied to u-boot/next, thanks!

Newer platforms use the LOAD_IMAGE_V2 parameter passing method. Add support for it.
Signed-off-by: Michael Walle michael@walle.cc --- common/spl/Kconfig | 9 ++++ common/spl/spl_atf.c | 99 ++++++++++++++++++++++++++++++++++++++++++-- include/atf_common.h | 30 ++++++++++++++ include/spl.h | 35 ++++++++++++++++ 4 files changed, 169 insertions(+), 4 deletions(-)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig index d8086bd9e8..6d980be0b7 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -1276,6 +1276,15 @@ 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_ATF_LOAD_IMAGE_V2 + bool "Use the new LOAD_IMAGE_V2 parameter passing" + depends on SPL_ATF + help + Some platforms use the newer LOAD_IMAGE_V2 parameter passing. + + If you want to load a bl31 image from the SPL and need the new + method, say Y. + config SPL_ATF_NO_PLATFORM_PARAM bool "Pass no platform parameter" depends on SPL_ATF diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c index 51b45d5dc6..e1b68dd561 100644 --- a/common/spl/spl_atf.c +++ b/common/spl/spl_atf.c @@ -29,6 +29,19 @@ struct bl2_to_bl31_params_mem { struct entry_point_info bl31_ep_info; };
+struct bl2_to_bl31_params_mem_v2 { + struct bl_params bl_params; + struct bl_params_node bl31_params_node; + struct bl_params_node bl32_params_node; + struct bl_params_node bl33_params_node; + struct atf_image_info bl31_image_info; + struct atf_image_info bl32_image_info; + struct atf_image_info bl33_image_info; + struct entry_point_info bl33_ep_info; + struct entry_point_info bl32_ep_info; + struct entry_point_info bl31_ep_info; +}; + struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry, uintptr_t bl33_entry, uintptr_t fdt_addr) @@ -96,6 +109,79 @@ __weak struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry, fdt_addr); }
+struct bl_params *bl2_plat_get_bl31_params_v2_default(uintptr_t bl32_entry, + uintptr_t bl33_entry, + uintptr_t fdt_addr) +{ + static struct bl2_to_bl31_params_mem_v2 bl31_params_mem; + struct bl_params *bl_params; + struct bl_params_node *bl_params_node; + + /* + * Initialise the memory for all the arguments that needs to + * be passed to BL31 + */ + memset(&bl31_params_mem, 0, sizeof(bl31_params_mem)); + + /* Assign memory for TF related information */ + bl_params = &bl31_params_mem.bl_params; + SET_PARAM_HEAD(bl_params, ATF_PARAM_BL_PARAMS, ATF_VERSION_2, 0); + bl_params->head = &bl31_params_mem.bl31_params_node; + + /* Fill BL31 related information */ + bl_params_node = &bl31_params_mem.bl31_params_node; + bl_params_node->image_id = ATF_BL31_IMAGE_ID; + bl_params_node->image_info = &bl31_params_mem.bl31_image_info; + bl_params_node->ep_info = &bl31_params_mem.bl31_ep_info; + bl_params_node->next_params_info = &bl31_params_mem.bl32_params_node; + SET_PARAM_HEAD(bl_params_node->image_info, ATF_PARAM_IMAGE_BINARY, + ATF_VERSION_2, 0); + + /* Fill BL32 related information */ + bl_params_node = &bl31_params_mem.bl32_params_node; + bl_params_node->image_id = ATF_BL32_IMAGE_ID; + bl_params_node->image_info = &bl31_params_mem.bl32_image_info; + bl_params_node->ep_info = &bl31_params_mem.bl32_ep_info; + bl_params_node->next_params_info = &bl31_params_mem.bl33_params_node; + SET_PARAM_HEAD(bl_params_node->ep_info, ATF_PARAM_EP, + ATF_VERSION_2, ATF_EP_SECURE); + + /* secure payload is optional, so set pc to 0 if absent */ + bl_params_node->ep_info->args.arg3 = fdt_addr; + bl_params_node->ep_info->pc = bl32_entry ? bl32_entry : 0; + bl_params_node->ep_info->spsr = SPSR_64(MODE_EL1, MODE_SP_ELX, + DISABLE_ALL_EXECPTIONS); + SET_PARAM_HEAD(bl_params_node->image_info, ATF_PARAM_IMAGE_BINARY, + ATF_VERSION_2, 0); + + /* Fill BL33 related information */ + bl_params_node = &bl31_params_mem.bl33_params_node; + bl_params_node->image_id = ATF_BL33_IMAGE_ID; + bl_params_node->image_info = &bl31_params_mem.bl33_image_info; + bl_params_node->ep_info = &bl31_params_mem.bl33_ep_info; + bl_params_node->next_params_info = NULL; + SET_PARAM_HEAD(bl_params_node->ep_info, ATF_PARAM_EP, + ATF_VERSION_2, ATF_EP_NON_SECURE); + + /* BL33 expects to receive the primary CPU MPID (through x0) */ + bl_params_node->ep_info->args.arg0 = 0xffff & read_mpidr(); + bl_params_node->ep_info->pc = bl33_entry; + bl_params_node->ep_info->spsr = SPSR_64(MODE_EL2, MODE_SP_ELX, + DISABLE_ALL_EXECPTIONS); + SET_PARAM_HEAD(bl_params_node->image_info, ATF_PARAM_IMAGE_BINARY, + ATF_VERSION_2, 0); + + return bl_params; +} + +__weak struct bl_params *bl2_plat_get_bl31_params_v2(uintptr_t bl32_entry, + uintptr_t bl33_entry, + uintptr_t fdt_addr) +{ + return bl2_plat_get_bl31_params_v2_default(bl32_entry, bl33_entry, + fdt_addr); +} + static inline void raw_write_daif(unsigned int daif) { __asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory"); @@ -106,16 +192,21 @@ typedef void (*atf_entry_t)(struct bl31_params *params, void *plat_params); static void bl31_entry(uintptr_t bl31_entry, uintptr_t bl32_entry, uintptr_t bl33_entry, uintptr_t fdt_addr) { - struct bl31_params *bl31_params; atf_entry_t atf_entry = (atf_entry_t)bl31_entry; + void *bl31_params;
- bl31_params = bl2_plat_get_bl31_params(bl32_entry, bl33_entry, - fdt_addr); + if (CONFIG_IS_ENABLED(ATF_LOAD_IMAGE_V2)) + bl31_params = bl2_plat_get_bl31_params_v2(bl32_entry, + bl33_entry, + fdt_addr); + else + bl31_params = bl2_plat_get_bl31_params(bl32_entry, bl33_entry, + fdt_addr);
raw_write_daif(SPSR_EXCEPTION_MASK); dcache_disable();
- atf_entry((void *)bl31_params, (void *)fdt_addr); + atf_entry(bl31_params, (void *)fdt_addr); }
static int spl_fit_images_find(void *blob, int os) diff --git a/include/atf_common.h b/include/atf_common.h index e173a10ca9..d69892fac6 100644 --- a/include/atf_common.h +++ b/include/atf_common.h @@ -14,8 +14,14 @@ #define ATF_PARAM_EP 0x01 #define ATF_PARAM_IMAGE_BINARY 0x02 #define ATF_PARAM_BL31 0x03 +#define ATF_PARAM_BL_PARAMS 0x05
#define ATF_VERSION_1 0x01 +#define ATF_VERSION_2 0x02 + +#define ATF_BL31_IMAGE_ID 0x03 +#define ATF_BL32_IMAGE_ID 0x04 +#define ATF_BL33_IMAGE_ID 0x05
#define ATF_EP_SECURE 0x0 #define ATF_EP_NON_SECURE 0x1 @@ -121,6 +127,9 @@ struct atf_image_info { struct param_header h; uintptr_t image_base; /* physical address of base of image */ uint32_t image_size; /* bytes read from image file */ +#if CONFIG_IS_ENABLED(ATF_LOAD_IMAGE_V2) + uint32_t image_max_size; +#endif };
/***************************************************************************** @@ -162,6 +171,27 @@ struct bl31_params { struct atf_image_info *bl33_image_info; };
+/* BL image node in the BL image execution sequence */ +struct bl_params_node { + unsigned int image_id; + struct atf_image_info *image_info; + struct entry_point_info *ep_info; + struct bl_params_node *next_params_info; +}; + +/* + * BL image head node in the BL image execution sequence + * It is also used to pass information to next BL image. + */ +struct bl_params { + struct param_header h; + struct bl_params_node *head; +}; + +#define for_each_bl_params_node(bl_params, node) \ + for ((node) = (bl_params)->head; \ + (node); \ + (node) = (node)->next_params_info)
#endif /*__ASSEMBLY__ */
diff --git a/include/spl.h b/include/spl.h index fd928377f0..374a295fa3 100644 --- a/include/spl.h +++ b/include/spl.h @@ -564,6 +564,41 @@ struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry, struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry, uintptr_t bl33_entry, uintptr_t fdt_addr); + +/** + * bl2_plat_get_bl31_params_v2() - return params for bl31 + * @bl32_entry: address of BL32 executable (secure) + * @bl33_entry: address of BL33 executable (non secure) + * @fdt_addr: address of Flat Device Tree + * + * This function does the same as bl2_plat_get_bl31_params() except that is is + * used for the new LOAD_IMAGE_V2 option, which uses a slightly different + * method to pass the parameters. + * + * Return: bl31 params structure pointer + */ +struct bl_params *bl2_plat_get_bl31_params_v2(uintptr_t bl32_entry, + uintptr_t bl33_entry, + uintptr_t fdt_addr); + +/** + * bl2_plat_get_bl31_params_v2_default() - prepare params for bl31. + * @bl32_entry: address of BL32 executable (secure) + * @bl33_entry: address of BL33 executable (non secure) + * @fdt_addr: address of Flat Device Tree + * + * This is the default implementation of bl2_plat_get_bl31_params_v2(). It + * prepares the linked list of the bl31 params, populates the image types and + * set the entry points for bl32 and bl33 (if available). + * + * NOTE: The memory is statically allocated, thus this function should be + * called only once. All subsequent calls will overwrite any changes. + * + * Return: bl31 params structure pointer + */ +struct bl_params *bl2_plat_get_bl31_params_v2_default(uintptr_t bl32_entry, + uintptr_t bl33_entry, + uintptr_t fdt_addr); /** * spl_optee_entry - entry function for optee *

On Wed, Nov 18, 2020 at 05:45:58PM +0100, Michael Walle wrote:
Newer platforms use the LOAD_IMAGE_V2 parameter passing method. Add support for it.
Signed-off-by: Michael Walle michael@walle.cc
Applied to u-boot/next, thanks!

The BL31 expects the GIC to be uninitialized. Thus, if we are loading the BL31 by the SPL we must not initialize it. If u-boot is loaded by the SPL directly, it will initialize the GIC again (in the same lowlevel_init()).
This was tested on a custom board with SPL loading the BL31 and jumping to u-boot as BL33 as well as loading u-boot directly by the SPL. In case the ATF BL1/BL2 is used, this patch won't change anything, because no SPL is used at all.
Signed-off-by: Michael Walle michael@walle.cc --- arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S index a519f6ed67..d8803738f1 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S +++ b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S @@ -192,6 +192,7 @@ ENTRY(lowlevel_init) #endif
/* Initialize GIC Secure Bank Status */ +#if !defined(CONFIG_SPL_BUILD) #if defined(CONFIG_GICV2) || defined(CONFIG_GICV3) branch_if_slave x0, 1f bl get_gic_offset @@ -205,6 +206,7 @@ ENTRY(lowlevel_init) bl gic_init_secure_percpu #endif #endif +#endif
100: branch_if_master x0, x1, 2f

On Wed, Nov 18, 2020 at 05:45:59PM +0100, Michael Walle wrote:
The BL31 expects the GIC to be uninitialized. Thus, if we are loading the BL31 by the SPL we must not initialize it. If u-boot is loaded by the SPL directly, it will initialize the GIC again (in the same lowlevel_init()).
This was tested on a custom board with SPL loading the BL31 and jumping to u-boot as BL33 as well as loading u-boot directly by the SPL. In case the ATF BL1/BL2 is used, this patch won't change anything, because no SPL is used at all.
Signed-off-by: Michael Walle michael@walle.cc
Applied to u-boot/next, thanks!

It is not needed. Remove it.
Signed-off-by: Michael Walle michael@walle.cc --- arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi | 3 --- 1 file changed, 3 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi b/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi index 2375549c6e..87e14d6ae6 100644 --- a/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi +++ b/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi @@ -80,21 +80,18 @@ conf-1 { description = "fsl-ls1028a-kontron-sl28"; firmware = "uboot"; - loadables = "uboot"; fdt = "fdt-1"; };
conf-2 { description = "fsl-ls1028a-kontron-sl28-var3"; firmware = "uboot"; - loadables = "uboot"; fdt = "fdt-2"; };
conf-3 { description = "fsl-ls1028a-kontron-sl28-var4"; firmware = "uboot"; - loadables = "uboot"; fdt = "fdt-3"; }; };

On Wed, Nov 18, 2020 at 05:46:00PM +0100, Michael Walle wrote:
It is not needed. Remove it.
Signed-off-by: Michael Walle michael@walle.cc
Applied to u-boot/next, thanks!

On Wed, Nov 18, 2020 at 05:46:00PM +0100, Michael Walle wrote:
It is not needed. Remove it.
Signed-off-by: Michael Walle michael@walle.cc
Applied to u-boot/next, thanks!

Add support to load the bl31 part of the ARM Trusted Firmware by the SPL.
Signed-off-by: Michael Walle michael@walle.cc --- .../dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi | 41 +++++++++++++- board/kontron/sl28/Kconfig | 10 ++++ board/kontron/sl28/Makefile | 6 ++- board/kontron/sl28/spl_atf.c | 54 +++++++++++++++++++ 4 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 board/kontron/sl28/spl_atf.c
diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi b/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi index 87e14d6ae6..54ef0b4258 100644 --- a/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi +++ b/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi @@ -16,7 +16,7 @@ ethernet3 = &enetc6; };
- binman { + binman: binman { filename = "u-boot.rom"; pad-byte = <0xff>;
@@ -99,6 +99,45 @@ }; };
+#ifdef CONFIG_SL28_SPL_LOADS_ATF_BL31 +&binman { + fit { + images { + bl31 { + description = "ARM Trusted Firmware (bl31)"; + type = "firmware"; + arch = "arm"; + os = "arm-trusted-firmware"; + compression = "none"; + load = <CONFIG_SL28_BL31_ENTRY_ADDR>; + entry = <CONFIG_SL28_BL31_ENTRY_ADDR>; + + blob-ext { + filename = "bl31.bin"; + }; + }; + }; + + configurations { + conf-1 { + firmware = "bl31"; + loadables = "uboot"; + }; + + conf-2 { + firmware = "bl31"; + loadables = "uboot"; + }; + + conf-3 { + firmware = "bl31"; + loadables = "uboot"; + }; + }; + }; +}; +#endif + &i2c0 { rtc: rtc@32 { }; diff --git a/board/kontron/sl28/Kconfig b/board/kontron/sl28/Kconfig index cdec39be01..aba49fc115 100644 --- a/board/kontron/sl28/Kconfig +++ b/board/kontron/sl28/Kconfig @@ -15,4 +15,14 @@ config SYS_CONFIG_NAME config SYS_TEXT_BASE default 0x96000000
+config SL28_SPL_LOADS_ATF_BL31 + bool "SPL loads BL31 of the ARM Trusted Firmware" + select SPL_ATF + select SPL_ATF_LOAD_IMAGE_V2 + select ARMV8_SEC_FIRMWARE_SUPPORT + select SEC_FIRMWARE_ARMV8_PSCI + help + Enable this to load a BL31 image by the SPL. You have to + provde a bl31.bin in u-boot's root directory. + endif diff --git a/board/kontron/sl28/Makefile b/board/kontron/sl28/Makefile index 74d8012f0f..5d220f0744 100644 --- a/board/kontron/sl28/Makefile +++ b/board/kontron/sl28/Makefile @@ -5,4 +5,8 @@ obj-y += sl28.o cmds.o endif
obj-y += common.o ddr.o -obj-$(CONFIG_SPL_BUILD) += spl.o + +ifdef CONFIG_SPL_BUILD +obj-y += spl.o +obj-$(CONFIG_SPL_ATF) += spl_atf.o +endif diff --git a/board/kontron/sl28/spl_atf.c b/board/kontron/sl28/spl_atf.c new file mode 100644 index 0000000000..5438b5239c --- /dev/null +++ b/board/kontron/sl28/spl_atf.c @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * LS1028A TF-A calling support + * + * Copyright (c) 2020 Michael Walle michael@walle.cc + */ + +#include <common.h> +#include <asm/io.h> +#include <atf_common.h> +#include <spl.h> + +DECLARE_GLOBAL_DATA_PTR; + +struct region_info { + u64 addr; + u64 size; +}; + +struct dram_regions_info { + u64 num_dram_regions; + u64 total_dram_size; + struct region_info region[CONFIG_NR_DRAM_BANKS]; +}; + +struct bl_params *bl2_plat_get_bl31_params_v2(uintptr_t bl32_entry, + uintptr_t bl33_entry, + uintptr_t fdt_addr) +{ + static struct dram_regions_info dram_regions_info = { 0 }; + struct bl_params *bl_params; + struct bl_params_node *node; + void *dcfg_ccsr = (void *)DCFG_BASE; + int i; + + dram_regions_info.num_dram_regions = CONFIG_NR_DRAM_BANKS; + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { + dram_regions_info.region[i].addr = gd->bd->bi_dram[i].start; + dram_regions_info.region[i].size = gd->bd->bi_dram[i].size; + dram_regions_info.total_dram_size += gd->bd->bi_dram[i].size; + } + + bl_params = bl2_plat_get_bl31_params_v2_default(bl32_entry, bl33_entry, + fdt_addr); + + for_each_bl_params_node(bl_params, node) { + if (node->image_id == ATF_BL31_IMAGE_ID) { + node->ep_info->args.arg3 = (uintptr_t)&dram_regions_info; + node->ep_info->args.arg4 = in_le32(dcfg_ccsr + DCFG_PORSR1); + } + } + + return bl_params; +}

On Wed, Nov 18, 2020 at 05:46:01PM +0100, Michael Walle wrote:
Add support to load the bl31 part of the ARM Trusted Firmware by the SPL.
Signed-off-by: Michael Walle michael@walle.cc
Applied to u-boot/next, thanks!

Add support to load the OP-TEE Trusted OS by the SPL.
Signed-off-by: Michael Walle michael@walle.cc --- .../dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi | 36 +++++++++++++++++++ board/kontron/sl28/Kconfig | 23 ++++++++++++ board/kontron/sl28/sl28.c | 7 ++++ 3 files changed, 66 insertions(+)
diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi b/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi index 54ef0b4258..65d5684973 100644 --- a/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi +++ b/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi @@ -138,6 +138,42 @@ }; #endif
+#ifdef CONFIG_SL28_SPL_LOADS_OPTEE_BL32 +&binman { + fit { + images { + bl32 { + description = "OP-TEE Trusted OS (bl32)"; + type = "firmware"; + arch = "arm"; + os = "tee"; + compression = "none"; + load = <CONFIG_SL28_BL32_ENTRY_ADDR>; + entry = <CONFIG_SL28_BL32_ENTRY_ADDR>; + + blob-ext { + filename = "tee.bin"; + }; + }; + }; + + configurations { + conf-1 { + loadables = "uboot", "bl32"; + }; + + conf-2 { + loadables = "uboot", "bl32"; + }; + + conf-3 { + loadables = "uboot", "bl32"; + }; + }; + }; +}; +#endif + &i2c0 { rtc: rtc@32 { }; diff --git a/board/kontron/sl28/Kconfig b/board/kontron/sl28/Kconfig index aba49fc115..4078ef186b 100644 --- a/board/kontron/sl28/Kconfig +++ b/board/kontron/sl28/Kconfig @@ -25,4 +25,27 @@ config SL28_SPL_LOADS_ATF_BL31 Enable this to load a BL31 image by the SPL. You have to provde a bl31.bin in u-boot's root directory.
+if SL28_SPL_LOADS_ATF_BL31 + +config SL28_BL31_ENTRY_ADDR + hex "Entry point of the BL31 image" + default 0xfbe00000 + +endif + +config SL28_SPL_LOADS_OPTEE_BL32 + bool "SPL loads OP-TEE Trusted OS as BL32" + depends on SL28_SPL_LOADS_ATF_BL31 + help + Enable this to load a BL32 image by the SPL. You have to + provde a tee.bin in u-boot's root directory. + +if SL28_SPL_LOADS_OPTEE_BL32 + +config SL28_BL32_ENTRY_ADDR + hex "Entry point of the BL32 image" + default 0xfc000000 + +endif + endif diff --git a/board/kontron/sl28/sl28.c b/board/kontron/sl28/sl28.c index b18127c4d1..34f17b486b 100644 --- a/board/kontron/sl28/sl28.c +++ b/board/kontron/sl28/sl28.c @@ -50,6 +50,7 @@ int ft_board_setup(void *blob, struct bd_info *bd) u64 base[CONFIG_NR_DRAM_BANKS]; u64 size[CONFIG_NR_DRAM_BANKS]; int nbanks = CONFIG_NR_DRAM_BANKS; + int node; int i;
ft_cpu_setup(blob, bd); @@ -64,5 +65,11 @@ int ft_board_setup(void *blob, struct bd_info *bd)
fdt_fixup_icid(blob);
+ if (CONFIG_IS_ENABLED(SL28_SPL_LOADS_OPTEE_BL32)) { + node = fdt_node_offset_by_compatible(blob, -1, "linaro,optee-tz"); + if (node) + fdt_set_node_status(blob, node, FDT_STATUS_OKAY, 0); + } + return 0; }

On Wed, Nov 18, 2020 at 05:46:02PM +0100, Michael Walle wrote:
Add support to load the OP-TEE Trusted OS by the SPL.
Signed-off-by: Michael Walle michael@walle.cc
Applied to u-boot/next, thanks!

Hi,
On 18. 11. 20 17:45, Michael Walle wrote:
Newer TF-A versions provide a new image loading protocol. This is used on (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 -> u-boot. With this series it is possible that U-Boot SPL loads the bl31 directly and thus replacing bl1 and bl2 from the TF-A.
This was tested on the Kontron sl28 board using NXPs bl31 and the upstream version of the OP-TEE Trusted OS.
I still have some questions about this.
As I see from TFA previous image format has been removed in 2018 by
commit ed51b51f7a9163a7fc48289c5ed97a3fe4fe504f Author: Roberto Vargas roberto.vargas@arm.com AuthorDate: Mon Sep 24 17:20:48 2018 +0100 Commit: Antonio Nino Diaz antonio.ninodiaz@arm.com CommitDate: Fri Sep 28 15:31:52 2018 +0100
Remove build option LOAD_IMAGE_V2
The code of LOAD_IMAGE_V2=0 has been removed.
Change-Id: Iea03e5bebb90c66889bdb23f85c07d0c9717fffe Co-authored-by: Antonio Nino Diaz antonio.ninodiaz@arm.com Signed-off-by: Antonio Nino Diaz antonio.ninodiaz@arm.com
On Xilinx ZynqMP I use SPL->bl31 loading but not using that TFA structure because xilinx is using own format.
But I am curious if V2 was removed in 2018 who is really using previous one and also if current implemenation is origin or also not full v2.
And these patches are not breaking boot on zynqmp that's why not big deal for me.
Thanks, Michal

Am 2020-11-20 11:14, schrieb Michal Simek:
Hi,
On 18. 11. 20 17:45, Michael Walle wrote:
Newer TF-A versions provide a new image loading protocol. This is used on (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 -> u-boot. With this series it is possible that U-Boot SPL loads the bl31 directly and thus replacing bl1 and bl2 from the TF-A.
This was tested on the Kontron sl28 board using NXPs bl31 and the upstream version of the OP-TEE Trusted OS.
I still have some questions about this.
As I see from TFA previous image format has been removed in 2018 by
commit ed51b51f7a9163a7fc48289c5ed97a3fe4fe504f Author: Roberto Vargas roberto.vargas@arm.com AuthorDate: Mon Sep 24 17:20:48 2018 +0100 Commit: Antonio Nino Diaz antonio.ninodiaz@arm.com CommitDate: Fri Sep 28 15:31:52 2018 +0100
Remove build option LOAD_IMAGE_V2 The code of LOAD_IMAGE_V2=0 has been removed. Change-Id: Iea03e5bebb90c66889bdb23f85c07d0c9717fffe Co-authored-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com> Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>
DOH! Lol, I'm using just one non-upstream part for the whole board and of course it is doing something miserable. I wasn't aware of this.
On Xilinx ZynqMP I use SPL->bl31 loading but not using that TFA structure because xilinx is using own format.
But I am curious if V2 was removed in 2018 who is really using previous one and also if current implemenation is origin or also not full v2.
NXP, https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/...
The last non-nxp commit there was from around March 2018..
And these patches are not breaking boot on zynqmp that's why not big deal for me.
I was looking at porting TFA to upstream for this board but there is such a huge gap. Therefore, it seemed to be easier to just use the vendor version for now.
-michael

On 20. 11. 20 11:48, Michael Walle wrote:
Am 2020-11-20 11:14, schrieb Michal Simek:
Hi,
On 18. 11. 20 17:45, Michael Walle wrote:
Newer TF-A versions provide a new image loading protocol. This is used on (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 -> u-boot. With this series it is possible that U-Boot SPL loads the bl31 directly and thus replacing bl1 and bl2 from the TF-A.
This was tested on the Kontron sl28 board using NXPs bl31 and the upstream version of the OP-TEE Trusted OS.
I still have some questions about this.
As I see from TFA previous image format has been removed in 2018 by
commit ed51b51f7a9163a7fc48289c5ed97a3fe4fe504f Author: Roberto Vargas roberto.vargas@arm.com AuthorDate: Mon Sep 24 17:20:48 2018 +0100 Commit: Antonio Nino Diaz antonio.ninodiaz@arm.com CommitDate: Fri Sep 28 15:31:52 2018 +0100
Remove build option LOAD_IMAGE_V2
The code of LOAD_IMAGE_V2=0 has been removed.
Change-Id: Iea03e5bebb90c66889bdb23f85c07d0c9717fffe Co-authored-by: Antonio Nino Diaz antonio.ninodiaz@arm.com Signed-off-by: Antonio Nino Diaz antonio.ninodiaz@arm.com
DOH! Lol, I'm using just one non-upstream part for the whole board and of course it is doing something miserable. I wasn't aware of this.
On Xilinx ZynqMP I use SPL->bl31 loading but not using that TFA structure because xilinx is using own format.
But I am curious if V2 was removed in 2018 who is really using previous one and also if current implemenation is origin or also not full v2.
NXP, https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/...
The last non-nxp commit there was from around March 2018..
And these patches are not breaking boot on zynqmp that's why not big deal for me.
I was looking at porting TFA to upstream for this board but there is such a huge gap. Therefore, it seemed to be easier to just use the vendor version for now.
Please get this reviewed by people who are using current blX code. As I said this series is not breaking our flow on xilinx zynqmp soc but maybe your code is more or less duplication of what's there now. Or maybe if there is any NXP private way it should be handled differently as I do for zynqmp.
Thanks, Michal

Am 2020-11-20 12:15, schrieb Michal Simek:
On 20. 11. 20 11:48, Michael Walle wrote:
Am 2020-11-20 11:14, schrieb Michal Simek:
Hi,
On 18. 11. 20 17:45, Michael Walle wrote:
Newer TF-A versions provide a new image loading protocol. This is used on (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 -> u-boot. With this series it is possible that U-Boot SPL loads the bl31 directly and thus replacing bl1 and bl2 from the TF-A.
This was tested on the Kontron sl28 board using NXPs bl31 and the upstream version of the OP-TEE Trusted OS.
I still have some questions about this.
As I see from TFA previous image format has been removed in 2018 by
commit ed51b51f7a9163a7fc48289c5ed97a3fe4fe504f Author: Roberto Vargas roberto.vargas@arm.com AuthorDate: Mon Sep 24 17:20:48 2018 +0100 Commit: Antonio Nino Diaz antonio.ninodiaz@arm.com CommitDate: Fri Sep 28 15:31:52 2018 +0100
Remove build option LOAD_IMAGE_V2
The code of LOAD_IMAGE_V2=0 has been removed.
Change-Id: Iea03e5bebb90c66889bdb23f85c07d0c9717fffe Co-authored-by: Antonio Nino Diaz antonio.ninodiaz@arm.com Signed-off-by: Antonio Nino Diaz antonio.ninodiaz@arm.com
DOH! Lol, I'm using just one non-upstream part for the whole board and of course it is doing something miserable. I wasn't aware of this.
On Xilinx ZynqMP I use SPL->bl31 loading but not using that TFA structure because xilinx is using own format.
But I am curious if V2 was removed in 2018 who is really using previous one and also if current implemenation is origin or also not full v2.
NXP, https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/...
The last non-nxp commit there was from around March 2018..
And these patches are not breaking boot on zynqmp that's why not big deal for me.
I was looking at porting TFA to upstream for this board but there is such a huge gap. Therefore, it seemed to be easier to just use the vendor version for now.
Please get this reviewed by people who are using current blX code.
What do you mean by blX code? Nobody is using this flow for now, i.e. spl -> bl31 -> u-boot For the LOAD_IMAGE_V2 case.
NXP is using bl1 -> bl2 -> bl3 -> u-boot.
As I said this series is not breaking our flow on xilinx zynqmp soc but maybe your code is more or less duplication of what's there now. Or maybe if there is any NXP private way it should be handled differently as I do for zynqmp.
The code is split into the generic v2 handling and layerscape LS1028A specific stuff, hence the _default() function which is used in the board files and then arch specific parameters are applied. Also I don't know wether unifying the bl2_plat_get_bl31_params_default() and bl2_plat_get_bl31_params_v2_default() is worth it. If that is what you mean by "more or less a duplication".
-michael

On 20. 11. 20 12:27, Michael Walle wrote:
Am 2020-11-20 12:15, schrieb Michal Simek:
On 20. 11. 20 11:48, Michael Walle wrote:
Am 2020-11-20 11:14, schrieb Michal Simek:
Hi,
On 18. 11. 20 17:45, Michael Walle wrote:
Newer TF-A versions provide a new image loading protocol. This is used on (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 -> u-boot. With this series it is possible that U-Boot SPL loads the bl31 directly and thus replacing bl1 and bl2 from the TF-A.
This was tested on the Kontron sl28 board using NXPs bl31 and the upstream version of the OP-TEE Trusted OS.
I still have some questions about this.
As I see from TFA previous image format has been removed in 2018 by
commit ed51b51f7a9163a7fc48289c5ed97a3fe4fe504f Author: Roberto Vargas roberto.vargas@arm.com AuthorDate: Mon Sep 24 17:20:48 2018 +0100 Commit: Antonio Nino Diaz antonio.ninodiaz@arm.com CommitDate: Fri Sep 28 15:31:52 2018 +0100
Remove build option LOAD_IMAGE_V2
The code of LOAD_IMAGE_V2=0 has been removed.
Change-Id: Iea03e5bebb90c66889bdb23f85c07d0c9717fffe Co-authored-by: Antonio Nino Diaz antonio.ninodiaz@arm.com Signed-off-by: Antonio Nino Diaz antonio.ninodiaz@arm.com
DOH! Lol, I'm using just one non-upstream part for the whole board and of course it is doing something miserable. I wasn't aware of this.
On Xilinx ZynqMP I use SPL->bl31 loading but not using that TFA structure because xilinx is using own format.
But I am curious if V2 was removed in 2018 who is really using previous one and also if current implemenation is origin or also not full v2.
NXP, https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/...
The last non-nxp commit there was from around March 2018..
And these patches are not breaking boot on zynqmp that's why not big deal for me.
I was looking at porting TFA to upstream for this board but there is such a huge gap. Therefore, it seemed to be easier to just use the vendor version for now.
Please get this reviewed by people who are using current blX code.
What do you mean by blX code? Nobody is using this flow for now, i.e. spl -> bl31 -> u-boot
As I said I am not quite sure about it. I see that rockchip guys are using ATF and they also call that code in ATF. They should know which version the use. Definitely check with them to ack your patches.
Thanks, Michal

Am 2020-11-20 14:16, schrieb Michal Simek:
On 20. 11. 20 12:27, Michael Walle wrote:
Am 2020-11-20 12:15, schrieb Michal Simek:
On 20. 11. 20 11:48, Michael Walle wrote:
Am 2020-11-20 11:14, schrieb Michal Simek:
Hi,
On 18. 11. 20 17:45, Michael Walle wrote:
Newer TF-A versions provide a new image loading protocol. This is used on (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 -> u-boot. With this series it is possible that U-Boot SPL loads the bl31 directly and thus replacing bl1 and bl2 from the TF-A.
This was tested on the Kontron sl28 board using NXPs bl31 and the upstream version of the OP-TEE Trusted OS.
I still have some questions about this.
As I see from TFA previous image format has been removed in 2018 by
commit ed51b51f7a9163a7fc48289c5ed97a3fe4fe504f Author: Roberto Vargas roberto.vargas@arm.com AuthorDate: Mon Sep 24 17:20:48 2018 +0100 Commit: Antonio Nino Diaz antonio.ninodiaz@arm.com CommitDate: Fri Sep 28 15:31:52 2018 +0100
Remove build option LOAD_IMAGE_V2
The code of LOAD_IMAGE_V2=0 has been removed.
Change-Id: Iea03e5bebb90c66889bdb23f85c07d0c9717fffe Co-authored-by: Antonio Nino Diaz antonio.ninodiaz@arm.com Signed-off-by: Antonio Nino Diaz antonio.ninodiaz@arm.com
DOH! Lol, I'm using just one non-upstream part for the whole board and of course it is doing something miserable. I wasn't aware of this.
On Xilinx ZynqMP I use SPL->bl31 loading but not using that TFA structure because xilinx is using own format.
But I am curious if V2 was removed in 2018 who is really using previous one and also if current implemenation is origin or also not full v2.
NXP, https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/...
The last non-nxp commit there was from around March 2018..
And these patches are not breaking boot on zynqmp that's why not big deal for me.
I was looking at porting TFA to upstream for this board but there is such a huge gap. Therefore, it seemed to be easier to just use the vendor version for now.
Please get this reviewed by people who are using current blX code.
What do you mean by blX code? Nobody is using this flow for now, i.e. spl -> bl31 -> u-boot
As I said I am not quite sure about it. I see that rockchip guys are using ATF and they also call that code in ATF. They should know which version the use.
NXP imx8 also uses u-boot -> bl31 as far as I know.
Definitely check with them to ack your patches.
I've put Kever (who did the initial patches for rockchip) on CC for this. But again they are using the old method. So lets assume they will ignore this series (for whatever reason), won't it be accepted then? I mean this series doesn't touch the old behavior, just adding a new one.
-michael

On 20. 11. 20 14:25, Michael Walle wrote:
Am 2020-11-20 14:16, schrieb Michal Simek:
On 20. 11. 20 12:27, Michael Walle wrote:
Am 2020-11-20 12:15, schrieb Michal Simek:
On 20. 11. 20 11:48, Michael Walle wrote:
Am 2020-11-20 11:14, schrieb Michal Simek:
Hi,
On 18. 11. 20 17:45, Michael Walle wrote: > Newer TF-A versions provide a new image loading protocol. This is > used on > (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 -> > u-boot. With this series it is possible that U-Boot SPL loads the > bl31 > directly and thus replacing bl1 and bl2 from the TF-A. > > This was tested on the Kontron sl28 board using NXPs bl31 and the > upstream > version of the OP-TEE Trusted OS.
I still have some questions about this.
As I see from TFA previous image format has been removed in 2018 by
commit ed51b51f7a9163a7fc48289c5ed97a3fe4fe504f Author: Roberto Vargas roberto.vargas@arm.com AuthorDate: Mon Sep 24 17:20:48 2018 +0100 Commit: Antonio Nino Diaz antonio.ninodiaz@arm.com CommitDate: Fri Sep 28 15:31:52 2018 +0100
Remove build option LOAD_IMAGE_V2
The code of LOAD_IMAGE_V2=0 has been removed.
Change-Id: Iea03e5bebb90c66889bdb23f85c07d0c9717fffe Co-authored-by: Antonio Nino Diaz antonio.ninodiaz@arm.com Signed-off-by: Antonio Nino Diaz antonio.ninodiaz@arm.com
DOH! Lol, I'm using just one non-upstream part for the whole board and of course it is doing something miserable. I wasn't aware of this.
On Xilinx ZynqMP I use SPL->bl31 loading but not using that TFA structure because xilinx is using own format.
But I am curious if V2 was removed in 2018 who is really using previous one and also if current implemenation is origin or also not full v2.
NXP, https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/...
The last non-nxp commit there was from around March 2018..
And these patches are not breaking boot on zynqmp that's why not big deal for me.
I was looking at porting TFA to upstream for this board but there is such a huge gap. Therefore, it seemed to be easier to just use the vendor version for now.
Please get this reviewed by people who are using current blX code.
What do you mean by blX code? Nobody is using this flow for now, i.e. spl -> bl31 -> u-boot
As I said I am not quite sure about it. I see that rockchip guys are using ATF and they also call that code in ATF. They should know which version the use.
NXP imx8 also uses u-boot -> bl31 as far as I know.
Definitely check with them to ack your patches.
I've put Kever (who did the initial patches for rockchip) on CC for this. But again they are using the old method. So lets assume they will ignore this series (for whatever reason), won't it be accepted then? I mean this series doesn't touch the old behavior, just adding a new one.
up to Priyanka or Tom. I have no problem with your patches because we are not using this method and your patches doesn't break our SPL boot flow.
Thanks, Michal

Am 2020-11-20 14:35, schrieb Michal Simek:
On 20. 11. 20 14:25, Michael Walle wrote:
Am 2020-11-20 14:16, schrieb Michal Simek:
On 20. 11. 20 12:27, Michael Walle wrote:
Am 2020-11-20 12:15, schrieb Michal Simek:
[..]
Please get this reviewed by people who are using current blX code.
What do you mean by blX code? Nobody is using this flow for now, i.e. spl -> bl31 -> u-boot
As I said I am not quite sure about it. I see that rockchip guys are using ATF and they also call that code in ATF. They should know which version the use.
NXP imx8 also uses u-boot -> bl31 as far as I know.
Definitely check with them to ack your patches.
I've put Kever (who did the initial patches for rockchip) on CC for this. But again they are using the old method. So lets assume they will ignore this series (for whatever reason), won't it be accepted then? I mean this series doesn't touch the old behavior, just adding a new one.
up to Priyanka or Tom. I have no problem with your patches because we are not using this method and your patches doesn't break our SPL boot flow.
I haven't said this: I really appreciated your review and I was surprised how fast it was. Thanks and maybe Kever will also have a look. We'll see ;)
-michael

On Fri, Nov 20, 2020 at 02:35:33PM +0100, Michal Simek wrote:
On 20. 11. 20 14:25, Michael Walle wrote:
Am 2020-11-20 14:16, schrieb Michal Simek:
On 20. 11. 20 12:27, Michael Walle wrote:
Am 2020-11-20 12:15, schrieb Michal Simek:
On 20. 11. 20 11:48, Michael Walle wrote:
Am 2020-11-20 11:14, schrieb Michal Simek: > Hi, > > On 18. 11. 20 17:45, Michael Walle wrote: >> Newer TF-A versions provide a new image loading protocol. This is >> used on >> (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 -> >> u-boot. With this series it is possible that U-Boot SPL loads the >> bl31 >> directly and thus replacing bl1 and bl2 from the TF-A. >> >> This was tested on the Kontron sl28 board using NXPs bl31 and the >> upstream >> version of the OP-TEE Trusted OS. > > I still have some questions about this. > > As I see from TFA previous image format has been removed in 2018 by > > commit ed51b51f7a9163a7fc48289c5ed97a3fe4fe504f > Author: Roberto Vargas roberto.vargas@arm.com > AuthorDate: Mon Sep 24 17:20:48 2018 +0100 > Commit: Antonio Nino Diaz antonio.ninodiaz@arm.com > CommitDate: Fri Sep 28 15:31:52 2018 +0100 > > Remove build option LOAD_IMAGE_V2 > > The code of LOAD_IMAGE_V2=0 has been removed. > > Change-Id: Iea03e5bebb90c66889bdb23f85c07d0c9717fffe > Co-authored-by: Antonio Nino Diaz antonio.ninodiaz@arm.com > Signed-off-by: Antonio Nino Diaz antonio.ninodiaz@arm.com > >
DOH! Lol, I'm using just one non-upstream part for the whole board and of course it is doing something miserable. I wasn't aware of this.
> On Xilinx ZynqMP I use SPL->bl31 loading but not using that TFA > structure because xilinx is using own format. > > But I am curious if V2 was removed in 2018 who is really using > previous > one and also if current implemenation is origin or also not full v2.
NXP, https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/...
The last non-nxp commit there was from around March 2018..
> And these patches are not breaking boot on zynqmp that's why not big > deal for me.
I was looking at porting TFA to upstream for this board but there is such a huge gap. Therefore, it seemed to be easier to just use the vendor version for now.
Please get this reviewed by people who are using current blX code.
What do you mean by blX code? Nobody is using this flow for now, i.e. spl -> bl31 -> u-boot
As I said I am not quite sure about it. I see that rockchip guys are using ATF and they also call that code in ATF. They should know which version the use.
NXP imx8 also uses u-boot -> bl31 as far as I know.
Definitely check with them to ack your patches.
I've put Kever (who did the initial patches for rockchip) on CC for this. But again they are using the old method. So lets assume they will ignore this series (for whatever reason), won't it be accepted then? I mean this series doesn't touch the old behavior, just adding a new one.
up to Priyanka or Tom. I have no problem with your patches because we are not using this method and your patches doesn't break our SPL boot flow.
FWIW, I've currently assigned this to myself in patchwork with the notion that baring further changes, I'll pick this up and put it in -next, when it's around that time.

Am 2020-11-20 11:48, schrieb Michael Walle:
Am 2020-11-20 11:14, schrieb Michal Simek:
Hi,
On 18. 11. 20 17:45, Michael Walle wrote:
Newer TF-A versions provide a new image loading protocol. This is used on (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 -> u-boot. With this series it is possible that U-Boot SPL loads the bl31 directly and thus replacing bl1 and bl2 from the TF-A.
This was tested on the Kontron sl28 board using NXPs bl31 and the upstream version of the OP-TEE Trusted OS.
I still have some questions about this.
As I see from TFA previous image format has been removed in 2018 by
commit ed51b51f7a9163a7fc48289c5ed97a3fe4fe504f Author: Roberto Vargas roberto.vargas@arm.com AuthorDate: Mon Sep 24 17:20:48 2018 +0100 Commit: Antonio Nino Diaz antonio.ninodiaz@arm.com CommitDate: Fri Sep 28 15:31:52 2018 +0100
Remove build option LOAD_IMAGE_V2 The code of LOAD_IMAGE_V2=0 has been removed. Change-Id: Iea03e5bebb90c66889bdb23f85c07d0c9717fffe Co-authored-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com> Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>
DOH! Lol, I'm using just one non-upstream part for the whole board and of course it is doing something miserable. I wasn't aware of this.
Ah, this commit wasn't about removing LOAD_IMAGE_V2 but making it the only supported one. Thus it basically removes the old "LOAD_IMAGE_V2=0". That is the former boot protocol which is the only one u-boot supports until this patch series.
It seems that zynqmp doesn't check for the v2 header version. This is the common function: https://elixir.bootlin.com/arm-trusted-firmware/v2.4/source/common/desc_imag...
-michael
participants (3)
-
Michael Walle
-
Michal Simek
-
Tom Rini