[U-Boot] [PATCH v2 0/1] arm64: enable SPL with ATF support

[U-Boot] [PATCH v2 0/1] arm64: enable SPL with ATF support

[Kever Yang]

This patch needs work with some patch for SPL support multi
binary in FIT which is from Andre.

The entry point of bl31 and bl33 is still using hard code because we
still can not get them from the FIT image information.

This patch tested on rk3399 platform.


Changes in v2:
- Kconfig comment update with Simon's suggestion
- including file ordering,
- update function comment format,
- use 'if' instead of '#ifdef' for bl31_entry
- add ATF Kconfig option depend on ARM64
Series-changes: 1
- license update
- split out as separate patch

Kever Yang (1):
  spl: add support to booting with ATF

 common/spl/Kconfig   |  14 +++
 common/spl/Makefile  |   1 +
 common/spl/spl.c     |   3 +
 common/spl/spl_atf.c |  96 ++++++++++++++++++
 include/atf_common.h | 276 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/spl.h        |   1 +
 6 files changed, 391 insertions(+)
 create mode 100644 common/spl/spl_atf.c
 create mode 100644 include/atf_common.h

-- 
1.9.1

[U-Boot] [PATCH v2] spl: add support to booting with ATF

[Kever Yang]

ATF(ARM Trusted Firmware) is used by ARM arch64 SoCs, find more infomation
about ATF at: https://github.com/ARM-software/arm-trusted-firmware

SPL is considered as BL2 in ATF terminology, it needs to load other parts
of ATF binary like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to
prepare the parameter for BL31 which including entry and image information
for all other images. Then the SPL handle PC to BL31 with the parameter,
the BL31 will do the rest of work and at last get into U-Boot(BL33).

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v2:
- Kconfig comment update with Simon's suggestion
- including file ordering,
- update function comment format,
- use 'if' instead of '#ifdef' for bl31_entry
- add ATF Kconfig option depend on ARM64
Series-changes: 1
- license update
- split out as separate patch

 common/spl/Kconfig   |  14 +++
 common/spl/Makefile  |   1 +
 common/spl/spl.c     |   3 +
 common/spl/spl_atf.c |  96 ++++++++++++++++++
 include/atf_common.h | 276 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/spl.h        |   1 +
 6 files changed, 391 insertions(+)
 create mode 100644 common/spl/spl_atf.c
 create mode 100644 include/atf_common.h

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 60ae60c..1b7fb8d 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -670,6 +670,20 @@ config SPL_YMODEM_SUPPORT
 	  means of transmitting U-Boot over a serial line for using in SPL,
 	  with a checksum to ensure correctness.
 
+config SPL_ATF_SUPPORT
+	bool "Support ARM Trusted Firmware"
+	depends on SPL && ARM64
+	help
+	  ATF(ARM Trusted Firmware) is a component for ARM arch64 which which
+	  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_TEXT_BASE
+	depends on SPL_ATF_SUPPORT
+	hex "ATF TEXT BASE addr"
+	help
+	  This is the base address in memory for ATF text and entry point.
+
 config TPL_ENV_SUPPORT
 	bool "Support an environment"
 	depends on TPL
diff --git a/common/spl/Makefile b/common/spl/Makefile
index 1933cbd..b3b34d6 100644
--- a/common/spl/Makefile
+++ b/common/spl/Makefile
@@ -20,6 +20,7 @@ endif
 obj-$(CONFIG_SPL_UBI) += spl_ubi.o
 obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o
 obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o
+obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o
 obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o
 obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o
 obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.o
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 26bc9ef..7041e1b 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -374,6 +374,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	      gd->malloc_ptr / 1024);
 #endif
 
+	if (IS_ENABLED(CONFIG_SPL_ATF_SUPPORT))
+		bl31_entry();
+
 	debug("loaded - jumping to U-Boot...\n");
 	spl_board_prepare_for_boot();
 	jump_to_image_no_args(&spl_image);
diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
new file mode 100644
index 0000000..38753d7
--- /dev/null
+++ b/common/spl/spl_atf.c
@@ -0,0 +1,96 @@
+/*
+ * Reference to the ARM TF Project,
+ * plat/arm/common/arm_bl2_setup.c
+ * Portions copyright (c) 2013-2016, ARM Limited and Contributors. All rights
+ * reserved.
+ * Copyright (C) 2016 Rockchip Electronic Co.,Ltd
+ * Written by Kever Yang <kever.yang@rock-chips.com>
+ *
+ * SPDX-License-Identifier:     BSD-3-Clause
+ */
+
+#include <common.h>
+#include <atf_common.h>
+#include <errno.h>
+#include <spl.h>
+
+static struct bl2_to_bl31_params_mem_t bl31_params_mem;
+static struct bl31_params_t *bl2_to_bl31_params;
+
+/**
+ * bl2_plat_get_bl31_params() - prepare params for bl31.
+ *
+ * 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_t 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
+ *
+ * @return bl31 params structure pointer
+ */
+struct bl31_params_t *bl2_plat_get_bl31_params(void)
+{
+	struct entry_point_info_t *bl33_ep_info;
+
+	/*
+	 * Initialise the memory for all the arguments that needs to
+	 * be passed to BL31
+	 */
+	memset(&bl31_params_mem, 0, sizeof(struct bl2_to_bl31_params_mem_t));
+
+	/* Assign memory for TF related information */
+	bl2_to_bl31_params = &bl31_params_mem.bl31_params;
+	SET_PARAM_HEAD(bl2_to_bl31_params, PARAM_BL31, VERSION_1, 0);
+
+	/* Fill BL31 related information */
+	SET_PARAM_HEAD(bl2_to_bl31_params->bl31_image_info, PARAM_IMAGE_BINARY,
+		       VERSION_1, 0);
+
+	/* Fill BL32 related information if it exists */
+#ifdef BL32_BASE
+	bl2_to_bl31_params->bl32_ep_info = &bl31_params_mem.bl32_ep_info;
+	SET_PARAM_HEAD(bl2_to_bl31_params->bl32_ep_info, PARAM_EP,
+		       VERSION_1, 0);
+	bl2_to_bl31_params->bl32_image_info = &bl31_params_mem.bl32_image_info;
+	SET_PARAM_HEAD(bl2_to_bl31_params->bl32_image_info, PARAM_IMAGE_BINARY,
+		       VERSION_1, 0);
+#endif /* BL32_BASE */
+
+	/* Fill BL33 related information */
+	bl2_to_bl31_params->bl33_ep_info = &bl31_params_mem.bl33_ep_info;
+	bl33_ep_info = &bl31_params_mem.bl33_ep_info;
+	SET_PARAM_HEAD(bl33_ep_info, PARAM_EP, VERSION_1, EP_NON_SECURE);
+
+	/* BL33 expects to receive the primary CPU MPID (through x0) */
+	bl33_ep_info->args.arg0 = 0xffff & read_mpidr();
+	bl33_ep_info->pc = CONFIG_SYS_TEXT_BASE;
+	bl33_ep_info->spsr = SPSR_64(MODE_EL2, MODE_SP_ELX,
+				     DISABLE_ALL_EXECPTIONS);
+
+	bl2_to_bl31_params->bl33_image_info = &bl31_params_mem.bl33_image_info;
+	SET_PARAM_HEAD(bl2_to_bl31_params->bl33_image_info, PARAM_IMAGE_BINARY,
+		       VERSION_1, 0);
+
+	return bl2_to_bl31_params;
+}
+
+void raw_write_daif(uint32_t daif)
+{
+	__asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory");
+}
+
+void bl31_entry(void)
+{
+	struct bl31_params_t *bl31_params;
+	void (*entry)(struct bl31_params_t *params, void *plat_params) = NULL;
+
+	bl31_params = bl2_plat_get_bl31_params();
+	entry = (void *)CONFIG_SPL_ATF_TEXT_BASE;
+
+	raw_write_daif(SPSR_EXCEPTION_MASK);
+	dcache_disable();
+
+	entry(bl31_params, NULL);
+}
diff --git a/include/atf_common.h b/include/atf_common.h
new file mode 100644
index 0000000..90c7fce
--- /dev/null
+++ b/include/atf_common.h
@@ -0,0 +1,276 @@
+/*
+ * This is from the ARM TF Project,
+ * Repository: https://github.com/ARM-software/arm-trusted-firmware.git
+ * File: include/common/bl_common.h
+ * Portions copyright (c) 2013-2016, ARM Limited and Contributors. All rights
+ * reserved.
+ * Copyright (C) 2016-2017 Rockchip Electronic Co.,Ltd
+ *
+ * SPDX-License-Identifier:     BSD-3-Clause
+ */
+
+#ifndef __BL_COMMON_H__
+#define __BL_COMMON_H__
+
+#define SECURE		0x0
+#define NON_SECURE	0x1
+#define sec_state_is_valid(s) (((s) == SECURE) || ((s) == NON_SECURE))
+
+#define UP	1
+#define DOWN	0
+
+/*******************************************************************************
+ * Constants to identify the location of a memory region in a given memory
+ * layout.
+******************************************************************************/
+#define TOP	0x1
+#define BOTTOM	!TOP
+
+/*******************************************************************************
+ * Constants that allow assembler code to access members of and the
+ * 'entry_point_info' structure at their correct offsets.
+ ******************************************************************************/
+#define ENTRY_POINT_INFO_PC_OFFSET	0x08
+#define ENTRY_POINT_INFO_ARGS_OFFSET	0x18
+
+/* The following are used to set/get image attributes. */
+#define PARAM_EP_SECURITY_MASK		(0x1)
+
+#define GET_SECURITY_STATE(x) (x & PARAM_EP_SECURITY_MASK)
+#define SET_SECURITY_STATE(x, security) \
+			((x) = ((x) & ~PARAM_EP_SECURITY_MASK) | (security))
+
+
+/*
+ * The following are used for image state attributes.
+ * Image can only be in one of the following state.
+ */
+#define IMAGE_STATE_RESET			0
+#define IMAGE_STATE_COPIED			1
+#define IMAGE_STATE_COPYING			2
+#define IMAGE_STATE_AUTHENTICATED		3
+#define IMAGE_STATE_EXECUTED			4
+#define IMAGE_STATE_INTERRUPTED			5
+
+#define EP_SECURE	0x0
+#define EP_NON_SECURE	0x1
+
+#define EP_EE_MASK	0x2
+#define EP_EE_LITTLE	0x0
+#define EP_EE_BIG	0x2
+#define EP_GET_EE(x) (x & EP_EE_MASK)
+#define EP_SET_EE(x, ee) ((x) = ((x) & ~EP_EE_MASK) | (ee))
+
+#define EP_ST_MASK	0x4
+#define EP_ST_DISABLE	0x0
+#define EP_ST_ENABLE	0x4
+#define EP_GET_ST(x) (x & EP_ST_MASK)
+#define EP_SET_ST(x, ee) ((x) = ((x) & ~EP_ST_MASK) | (ee))
+
+#define EP_EXE_MASK	0x8
+#define NON_EXECUTABLE	0x0
+#define EXECUTABLE	0x8
+#define EP_GET_EXE(x) (x & EP_EXE_MASK)
+#define EP_SET_EXE(x, ee) ((x) = ((x) & ~EP_EXE_MASK) | (ee))
+
+#define PARAM_EP		0x01
+#define PARAM_IMAGE_BINARY	0x02
+#define PARAM_BL31		0x03
+
+#define VERSION_1	0x01
+
+#define INVALID_IMAGE_ID		(0xFFFFFFFF)
+
+#define SET_PARAM_HEAD(_p, _type, _ver, _attr) do { \
+	(_p)->h.type = (uint8_t)(_type); \
+	(_p)->h.version = (uint8_t)(_ver); \
+	(_p)->h.size = (uint16_t)sizeof(*_p); \
+	(_p)->h.attr = (uint32_t)(_attr) ; \
+	} while (0)
+
+/* Following is used for populating structure members statically. */
+#define SET_STATIC_PARAM_HEAD(_p, _type, _ver, _p_type, _attr)	\
+	._p.h.type = (uint8_t)(_type), \
+	._p.h.version = (uint8_t)(_ver), \
+	._p.h.size = (uint16_t)sizeof(_p_type), \
+	._p.h.attr = (uint32_t)(_attr)
+
+#define MODE_RW_SHIFT	0x4
+#define MODE_RW_MASK	0x1
+#define MODE_RW_64	0x0
+#define MODE_RW_32	0x1
+
+#define MODE_EL_SHIFT	0x2
+#define MODE_EL_MASK	0x3
+#define MODE_EL3	0x3
+#define MODE_EL2	0x2
+#define MODE_EL1	0x1
+#define MODE_EL0	0x0
+
+#define MODE_SP_SHIFT	0x0
+#define MODE_SP_MASK	0x1
+#define MODE_SP_EL0	0x0
+#define MODE_SP_ELX	0x1
+
+#define SPSR_DAIF_SHIFT	6
+#define SPSR_DAIF_MASK	0x0f
+
+#define DAIF_FIQ_BIT (1<<0)
+#define DAIF_IRQ_BIT (1<<0)
+#define DAIF_ABT_BIT (1<<0)
+#define DAIF_DBG_BIT (1<<0)
+#define DISABLE_ALL_EXECPTIONS	\
+	(DAIF_FIQ_BIT | DAIF_IRQ_BIT | DAIF_ABT_BIT | DAIF_DBG_BIT)
+
+#define SPSR_64(el, sp, daif)		\
+	(MODE_RW_64 << MODE_RW_SHIFT |	\
+	 ((el) & MODE_EL_MASK) << MODE_EL_SHIFT |	\
+	 ((sp) & MODE_SP_MASK) << MODE_SP_SHIFT |	\
+	 ((daif) & SPSR_DAIF_MASK) << SPSR_DAIF_SHIFT)
+
+/*******************************************************************************
+ * Constants to indicate type of exception to the common exception handler.
+ ******************************************************************************/
+#define SYNC_EXCEPTION_SP_EL0		0x0
+#define IRQ_SP_EL0			0x1
+#define FIQ_SP_EL0			0x2
+#define SERROR_SP_EL0			0x3
+#define SYNC_EXCEPTION_SP_ELX		0x4
+#define IRQ_SP_ELX			0x5
+#define FIQ_SP_ELX			0x6
+#define SERROR_SP_ELX			0x7
+#define SYNC_EXCEPTION_AARCH64		0x8
+#define IRQ_AARCH64			0x9
+#define FIQ_AARCH64			0xa
+#define SERROR_AARCH64			0xb
+#define SYNC_EXCEPTION_AARCH32		0xc
+#define IRQ_AARCH32			0xd
+#define FIQ_AARCH32			0xe
+#define SERROR_AARCH32			0xf
+
+#define SPSR_USE_L           0
+#define SPSR_USE_H           1
+#define SPSR_L_H_MASK        1
+#define SPSR_M_SHIFT         4
+#define SPSR_ERET_32         (1 << SPSR_M_SHIFT)
+#define SPSR_ERET_64         (0 << SPSR_M_SHIFT)
+#define SPSR_FIQ             (1 << 6)
+#define SPSR_IRQ             (1 << 7)
+#define SPSR_SERROR          (1 << 8)
+#define SPSR_DEBUG           (1 << 9)
+#define SPSR_EXCEPTION_MASK  (SPSR_FIQ | SPSR_IRQ | SPSR_SERROR | SPSR_DEBUG)
+
+#ifndef __ASSEMBLY__
+
+/*******************************************************************************
+ * Structure used for telling the next BL how much of a particular type of
+ * memory is available for its use and how much is already used.
+ ******************************************************************************/
+struct aapcs64_params_t {
+	unsigned long arg0;
+	unsigned long arg1;
+	unsigned long arg2;
+	unsigned long arg3;
+	unsigned long arg4;
+	unsigned long arg5;
+	unsigned long arg6;
+	unsigned long arg7;
+};
+
+/***************************************************************************
+ * This structure provides version information and the size of the
+ * structure, attributes for the structure it represents
+ ***************************************************************************/
+struct param_header_t {
+	uint8_t type;		/* type of the structure */
+	uint8_t version;    /* version of this structure */
+	uint16_t size;      /* size of this structure in bytes */
+	uint32_t attr;      /* attributes: unused bits SBZ */
+};
+
+/*****************************************************************************
+ * This structure represents the superset of information needed while
+ * switching exception levels. The only two mechanisms to do so are
+ * ERET & SMC. Security state is indicated using bit zero of header
+ * attribute
+ * NOTE: BL1 expects entrypoint followed by spsr@an offset from the start
+ * of this structure defined by the macro `ENTRY_POINT_INFO_PC_OFFSET` while
+ * processing SMC to jump to BL31.
+ *****************************************************************************/
+struct entry_point_info_t {
+	struct param_header_t h;
+	uintptr_t pc;
+	uint32_t spsr;
+	struct aapcs64_params_t args;
+};
+
+/*****************************************************************************
+ * Image info binary provides information from the image loader that
+ * can be used by the firmware to manage available trusted RAM.
+ * More advanced firmware image formats can provide additional
+ * information that enables optimization or greater flexibility in the
+ * common firmware code
+ *****************************************************************************/
+struct atf_image_info_t {
+	struct param_header_t h;
+	uintptr_t image_base;   /* physical address of base of image */
+	uint32_t image_size;    /* bytes read from image file */
+};
+
+/*****************************************************************************
+ * The image descriptor struct definition.
+ *****************************************************************************/
+struct image_desc_t {
+	/* Contains unique image id for the image. */
+	unsigned int image_id;
+	/*
+	 * This member contains Image state information.
+	 * Refer IMAGE_STATE_XXX defined above.
+	 */
+	unsigned int state;
+	uint32_t copied_size;	/* image size copied in blocks */
+	struct atf_image_info_t atf_image_info;
+	struct entry_point_info_t ep_info;
+};
+
+/*******************************************************************************
+ * This structure represents the superset of information that can be passed to
+ * BL31 e.g. while passing control to it from BL2. The BL32 parameters will be
+ * populated only if BL2 detects its presence. A pointer to a structure of this
+ * type should be passed in X0 to BL31's cold boot entrypoint.
+ *
+ * Use of this structure and the X0 parameter is not mandatory: the BL31
+ * platform code can use other mechanisms to provide the necessary information
+ * about BL32 and BL33 to the common and SPD code.
+ *
+ * BL31 image information is mandatory if this structure is used. If either of
+ * the optional BL32 and BL33 image information is not provided, this is
+ * indicated by the respective image_info pointers being zero.
+ ******************************************************************************/
+struct bl31_params_t {
+	struct param_header_t h;
+	struct atf_image_info_t *bl31_image_info;
+	struct entry_point_info_t *bl32_ep_info;
+	struct atf_image_info_t *bl32_image_info;
+	struct entry_point_info_t *bl33_ep_info;
+	struct atf_image_info_t *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_t {
+	struct bl31_params_t bl31_params;
+	struct atf_image_info_t bl31_image_info;
+	struct atf_image_info_t bl32_image_info;
+	struct atf_image_info_t bl33_image_info;
+	struct entry_point_info_t bl33_ep_info;
+	struct entry_point_info_t bl32_ep_info;
+	struct entry_point_info_t bl31_ep_info;
+};
+
+#endif /*__ASSEMBLY__*/
+
+#endif /* __BL_COMMON_H__ */
diff --git a/include/spl.h b/include/spl.h
index a89ac00..8883657 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -260,4 +260,5 @@ int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface, char *devstr);
 int spl_mmc_load_image(struct spl_image_info *spl_image,
 		       struct spl_boot_device *bootdev);
 
+void bl31_entry(void);
 #endif
-- 
1.9.1

[U-Boot] [PATCH v2] spl: add support to booting with ATF

[Andre Przywara]

Hi Kever,

I was wondering if we really need to copy in all those ATF definitions.
I think this is really an *interface* between the loader (SPL or BL2)
and the runtime services (BL31), so it's supposed to be stable and we
wouldn't need to pull in all those headers.

So given that, can't we simply model the data structure, which at the
end of the day is just a bunch of simple data types:

struct param_header_t {
	uint8_t type;		/* type of the structure */
	uint8_t version;    /* version of this structure */
	uint16_t size;      /* size of this structure in bytes */
	uint32_t attr;      /* attributes: unused bits SBZ */

struct entry_point_info_t {
	struct param_header_t h;
	uintptr_t pc;
	uint32_t spsr;
	unsigned long args[8]; /* struct aapcs64_params_t args; */

struct atf_image_info_t {
	struct param_header_t h;
	uintptr_t image_base;   /* physical address of base of image */
	uint32_t image_size;    /* bytes read from image file */


struct bl31_params_t {
	struct param_header_t h;
	struct atf_image_info_t *bl31_image_info;
	struct entry_point_info_t *bl32_ep_info;
	.....

Whenever this gets changed we are in trouble anyway (because we break
compatibility).

So what are the opinions on this? Pull in the ATF source or define our
own and mark it as an ATF copy?

Cheers,
Andre.

On 23/03/17 03:00, Kever Yang wrote:
> ATF(ARM Trusted Firmware) is used by ARM arch64 SoCs, find more infomation
> about ATF at: https://github.com/ARM-software/arm-trusted-firmware
> 
> SPL is considered as BL2 in ATF terminology, it needs to load other parts
> of ATF binary like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to
> prepare the parameter for BL31 which including entry and image information
> for all other images. Then the SPL handle PC to BL31 with the parameter,
> the BL31 will do the rest of work and at last get into U-Boot(BL33).
> 
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
> 
> Changes in v2:
> - Kconfig comment update with Simon's suggestion
> - including file ordering,
> - update function comment format,
> - use 'if' instead of '#ifdef' for bl31_entry
> - add ATF Kconfig option depend on ARM64
> Series-changes: 1
> - license update
> - split out as separate patch
> 
>  common/spl/Kconfig   |  14 +++
>  common/spl/Makefile  |   1 +
>  common/spl/spl.c     |   3 +
>  common/spl/spl_atf.c |  96 ++++++++++++++++++
>  include/atf_common.h | 276 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/spl.h        |   1 +
>  6 files changed, 391 insertions(+)
>  create mode 100644 common/spl/spl_atf.c
>  create mode 100644 include/atf_common.h
> 
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index 60ae60c..1b7fb8d 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -670,6 +670,20 @@ config SPL_YMODEM_SUPPORT
>  	  means of transmitting U-Boot over a serial line for using in SPL,
>  	  with a checksum to ensure correctness.
>  
> +config SPL_ATF_SUPPORT
> +	bool "Support ARM Trusted Firmware"
> +	depends on SPL && ARM64
> +	help
> +	  ATF(ARM Trusted Firmware) is a component for ARM arch64 which which
> +	  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_TEXT_BASE
> +	depends on SPL_ATF_SUPPORT
> +	hex "ATF TEXT BASE addr"
> +	help
> +	  This is the base address in memory for ATF text and entry point.
> +
>  config TPL_ENV_SUPPORT
>  	bool "Support an environment"
>  	depends on TPL
> diff --git a/common/spl/Makefile b/common/spl/Makefile
> index 1933cbd..b3b34d6 100644
> --- a/common/spl/Makefile
> +++ b/common/spl/Makefile
> @@ -20,6 +20,7 @@ endif
>  obj-$(CONFIG_SPL_UBI) += spl_ubi.o
>  obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o
>  obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o
> +obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o
>  obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o
>  obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o
>  obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.o
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 26bc9ef..7041e1b 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -374,6 +374,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>  	      gd->malloc_ptr / 1024);
>  #endif
>  
> +	if (IS_ENABLED(CONFIG_SPL_ATF_SUPPORT))
> +		bl31_entry();
> +
>  	debug("loaded - jumping to U-Boot...\n");
>  	spl_board_prepare_for_boot();
>  	jump_to_image_no_args(&spl_image);
> diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
> new file mode 100644
> index 0000000..38753d7
> --- /dev/null
> +++ b/common/spl/spl_atf.c
> @@ -0,0 +1,96 @@
> +/*
> + * Reference to the ARM TF Project,
> + * plat/arm/common/arm_bl2_setup.c
> + * Portions copyright (c) 2013-2016, ARM Limited and Contributors. All rights
> + * reserved.
> + * Copyright (C) 2016 Rockchip Electronic Co.,Ltd
> + * Written by Kever Yang <kever.yang@rock-chips.com>
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +
> +#include <common.h>
> +#include <atf_common.h>
> +#include <errno.h>
> +#include <spl.h>
> +
> +static struct bl2_to_bl31_params_mem_t bl31_params_mem;
> +static struct bl31_params_t *bl2_to_bl31_params;
> +
> +/**
> + * bl2_plat_get_bl31_params() - prepare params for bl31.
> + *
> + * 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_t 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
> + *
> + * @return bl31 params structure pointer
> + */
> +struct bl31_params_t *bl2_plat_get_bl31_params(void)
> +{
> +	struct entry_point_info_t *bl33_ep_info;
> +
> +	/*
> +	 * Initialise the memory for all the arguments that needs to
> +	 * be passed to BL31
> +	 */
> +	memset(&bl31_params_mem, 0, sizeof(struct bl2_to_bl31_params_mem_t));
> +
> +	/* Assign memory for TF related information */
> +	bl2_to_bl31_params = &bl31_params_mem.bl31_params;
> +	SET_PARAM_HEAD(bl2_to_bl31_params, PARAM_BL31, VERSION_1, 0);
> +
> +	/* Fill BL31 related information */
> +	SET_PARAM_HEAD(bl2_to_bl31_params->bl31_image_info, PARAM_IMAGE_BINARY,
> +		       VERSION_1, 0);
> +
> +	/* Fill BL32 related information if it exists */
> +#ifdef BL32_BASE
> +	bl2_to_bl31_params->bl32_ep_info = &bl31_params_mem.bl32_ep_info;
> +	SET_PARAM_HEAD(bl2_to_bl31_params->bl32_ep_info, PARAM_EP,
> +		       VERSION_1, 0);
> +	bl2_to_bl31_params->bl32_image_info = &bl31_params_mem.bl32_image_info;
> +	SET_PARAM_HEAD(bl2_to_bl31_params->bl32_image_info, PARAM_IMAGE_BINARY,
> +		       VERSION_1, 0);
> +#endif /* BL32_BASE */
> +
> +	/* Fill BL33 related information */
> +	bl2_to_bl31_params->bl33_ep_info = &bl31_params_mem.bl33_ep_info;
> +	bl33_ep_info = &bl31_params_mem.bl33_ep_info;
> +	SET_PARAM_HEAD(bl33_ep_info, PARAM_EP, VERSION_1, EP_NON_SECURE);
> +
> +	/* BL33 expects to receive the primary CPU MPID (through x0) */
> +	bl33_ep_info->args.arg0 = 0xffff & read_mpidr();
> +	bl33_ep_info->pc = CONFIG_SYS_TEXT_BASE;
> +	bl33_ep_info->spsr = SPSR_64(MODE_EL2, MODE_SP_ELX,
> +				     DISABLE_ALL_EXECPTIONS);
> +
> +	bl2_to_bl31_params->bl33_image_info = &bl31_params_mem.bl33_image_info;
> +	SET_PARAM_HEAD(bl2_to_bl31_params->bl33_image_info, PARAM_IMAGE_BINARY,
> +		       VERSION_1, 0);
> +
> +	return bl2_to_bl31_params;
> +}
> +
> +void raw_write_daif(uint32_t daif)
> +{
> +	__asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory");
> +}
> +
> +void bl31_entry(void)
> +{
> +	struct bl31_params_t *bl31_params;
> +	void (*entry)(struct bl31_params_t *params, void *plat_params) = NULL;
> +
> +	bl31_params = bl2_plat_get_bl31_params();
> +	entry = (void *)CONFIG_SPL_ATF_TEXT_BASE;
> +
> +	raw_write_daif(SPSR_EXCEPTION_MASK);
> +	dcache_disable();
> +
> +	entry(bl31_params, NULL);
> +}
> diff --git a/include/atf_common.h b/include/atf_common.h
> new file mode 100644
> index 0000000..90c7fce
> --- /dev/null
> +++ b/include/atf_common.h
> @@ -0,0 +1,276 @@
> +/*
> + * This is from the ARM TF Project,
> + * Repository: https://github.com/ARM-software/arm-trusted-firmware.git
> + * File: include/common/bl_common.h
> + * Portions copyright (c) 2013-2016, ARM Limited and Contributors. All rights
> + * reserved.
> + * Copyright (C) 2016-2017 Rockchip Electronic Co.,Ltd
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +
> +#ifndef __BL_COMMON_H__
> +#define __BL_COMMON_H__
> +
> +#define SECURE		0x0
> +#define NON_SECURE	0x1
> +#define sec_state_is_valid(s) (((s) == SECURE) || ((s) == NON_SECURE))
> +
> +#define UP	1
> +#define DOWN	0
> +
> +/*******************************************************************************
> + * Constants to identify the location of a memory region in a given memory
> + * layout.
> +******************************************************************************/
> +#define TOP	0x1
> +#define BOTTOM	!TOP
> +
> +/*******************************************************************************
> + * Constants that allow assembler code to access members of and the
> + * 'entry_point_info' structure at their correct offsets.
> + ******************************************************************************/
> +#define ENTRY_POINT_INFO_PC_OFFSET	0x08
> +#define ENTRY_POINT_INFO_ARGS_OFFSET	0x18
> +
> +/* The following are used to set/get image attributes. */
> +#define PARAM_EP_SECURITY_MASK		(0x1)
> +
> +#define GET_SECURITY_STATE(x) (x & PARAM_EP_SECURITY_MASK)
> +#define SET_SECURITY_STATE(x, security) \
> +			((x) = ((x) & ~PARAM_EP_SECURITY_MASK) | (security))
> +
> +
> +/*
> + * The following are used for image state attributes.
> + * Image can only be in one of the following state.
> + */
> +#define IMAGE_STATE_RESET			0
> +#define IMAGE_STATE_COPIED			1
> +#define IMAGE_STATE_COPYING			2
> +#define IMAGE_STATE_AUTHENTICATED		3
> +#define IMAGE_STATE_EXECUTED			4
> +#define IMAGE_STATE_INTERRUPTED			5
> +
> +#define EP_SECURE	0x0
> +#define EP_NON_SECURE	0x1
> +
> +#define EP_EE_MASK	0x2
> +#define EP_EE_LITTLE	0x0
> +#define EP_EE_BIG	0x2
> +#define EP_GET_EE(x) (x & EP_EE_MASK)
> +#define EP_SET_EE(x, ee) ((x) = ((x) & ~EP_EE_MASK) | (ee))
> +
> +#define EP_ST_MASK	0x4
> +#define EP_ST_DISABLE	0x0
> +#define EP_ST_ENABLE	0x4
> +#define EP_GET_ST(x) (x & EP_ST_MASK)
> +#define EP_SET_ST(x, ee) ((x) = ((x) & ~EP_ST_MASK) | (ee))
> +
> +#define EP_EXE_MASK	0x8
> +#define NON_EXECUTABLE	0x0
> +#define EXECUTABLE	0x8
> +#define EP_GET_EXE(x) (x & EP_EXE_MASK)
> +#define EP_SET_EXE(x, ee) ((x) = ((x) & ~EP_EXE_MASK) | (ee))
> +
> +#define PARAM_EP		0x01
> +#define PARAM_IMAGE_BINARY	0x02
> +#define PARAM_BL31		0x03
> +
> +#define VERSION_1	0x01
> +
> +#define INVALID_IMAGE_ID		(0xFFFFFFFF)
> +
> +#define SET_PARAM_HEAD(_p, _type, _ver, _attr) do { \
> +	(_p)->h.type = (uint8_t)(_type); \
> +	(_p)->h.version = (uint8_t)(_ver); \
> +	(_p)->h.size = (uint16_t)sizeof(*_p); \
> +	(_p)->h.attr = (uint32_t)(_attr) ; \
> +	} while (0)
> +
> +/* Following is used for populating structure members statically. */
> +#define SET_STATIC_PARAM_HEAD(_p, _type, _ver, _p_type, _attr)	\
> +	._p.h.type = (uint8_t)(_type), \
> +	._p.h.version = (uint8_t)(_ver), \
> +	._p.h.size = (uint16_t)sizeof(_p_type), \
> +	._p.h.attr = (uint32_t)(_attr)
> +
> +#define MODE_RW_SHIFT	0x4
> +#define MODE_RW_MASK	0x1
> +#define MODE_RW_64	0x0
> +#define MODE_RW_32	0x1
> +
> +#define MODE_EL_SHIFT	0x2
> +#define MODE_EL_MASK	0x3
> +#define MODE_EL3	0x3
> +#define MODE_EL2	0x2
> +#define MODE_EL1	0x1
> +#define MODE_EL0	0x0
> +
> +#define MODE_SP_SHIFT	0x0
> +#define MODE_SP_MASK	0x1
> +#define MODE_SP_EL0	0x0
> +#define MODE_SP_ELX	0x1
> +
> +#define SPSR_DAIF_SHIFT	6
> +#define SPSR_DAIF_MASK	0x0f
> +
> +#define DAIF_FIQ_BIT (1<<0)
> +#define DAIF_IRQ_BIT (1<<0)
> +#define DAIF_ABT_BIT (1<<0)
> +#define DAIF_DBG_BIT (1<<0)
> +#define DISABLE_ALL_EXECPTIONS	\
> +	(DAIF_FIQ_BIT | DAIF_IRQ_BIT | DAIF_ABT_BIT | DAIF_DBG_BIT)
> +
> +#define SPSR_64(el, sp, daif)		\
> +	(MODE_RW_64 << MODE_RW_SHIFT |	\
> +	 ((el) & MODE_EL_MASK) << MODE_EL_SHIFT |	\
> +	 ((sp) & MODE_SP_MASK) << MODE_SP_SHIFT |	\
> +	 ((daif) & SPSR_DAIF_MASK) << SPSR_DAIF_SHIFT)
> +
> +/*******************************************************************************
> + * Constants to indicate type of exception to the common exception handler.
> + ******************************************************************************/
> +#define SYNC_EXCEPTION_SP_EL0		0x0
> +#define IRQ_SP_EL0			0x1
> +#define FIQ_SP_EL0			0x2
> +#define SERROR_SP_EL0			0x3
> +#define SYNC_EXCEPTION_SP_ELX		0x4
> +#define IRQ_SP_ELX			0x5
> +#define FIQ_SP_ELX			0x6
> +#define SERROR_SP_ELX			0x7
> +#define SYNC_EXCEPTION_AARCH64		0x8
> +#define IRQ_AARCH64			0x9
> +#define FIQ_AARCH64			0xa
> +#define SERROR_AARCH64			0xb
> +#define SYNC_EXCEPTION_AARCH32		0xc
> +#define IRQ_AARCH32			0xd
> +#define FIQ_AARCH32			0xe
> +#define SERROR_AARCH32			0xf
> +
> +#define SPSR_USE_L           0
> +#define SPSR_USE_H           1
> +#define SPSR_L_H_MASK        1
> +#define SPSR_M_SHIFT         4
> +#define SPSR_ERET_32         (1 << SPSR_M_SHIFT)
> +#define SPSR_ERET_64         (0 << SPSR_M_SHIFT)
> +#define SPSR_FIQ             (1 << 6)
> +#define SPSR_IRQ             (1 << 7)
> +#define SPSR_SERROR          (1 << 8)
> +#define SPSR_DEBUG           (1 << 9)
> +#define SPSR_EXCEPTION_MASK  (SPSR_FIQ | SPSR_IRQ | SPSR_SERROR | SPSR_DEBUG)
> +
> +#ifndef __ASSEMBLY__
> +
> +/*******************************************************************************
> + * Structure used for telling the next BL how much of a particular type of
> + * memory is available for its use and how much is already used.
> + ******************************************************************************/
> +struct aapcs64_params_t {
> +	unsigned long arg0;
> +	unsigned long arg1;
> +	unsigned long arg2;
> +	unsigned long arg3;
> +	unsigned long arg4;
> +	unsigned long arg5;
> +	unsigned long arg6;
> +	unsigned long arg7;
> +};
> +
> +/***************************************************************************
> + * This structure provides version information and the size of the
> + * structure, attributes for the structure it represents
> + ***************************************************************************/
> +struct param_header_t {
> +	uint8_t type;		/* type of the structure */
> +	uint8_t version;    /* version of this structure */
> +	uint16_t size;      /* size of this structure in bytes */
> +	uint32_t attr;      /* attributes: unused bits SBZ */
> +};
> +
> +/*****************************************************************************
> + * This structure represents the superset of information needed while
> + * switching exception levels. The only two mechanisms to do so are
> + * ERET & SMC. Security state is indicated using bit zero of header
> + * attribute
> + * NOTE: BL1 expects entrypoint followed by spsr at an offset from the start
> + * of this structure defined by the macro `ENTRY_POINT_INFO_PC_OFFSET` while
> + * processing SMC to jump to BL31.
> + *****************************************************************************/
> +struct entry_point_info_t {
> +	struct param_header_t h;
> +	uintptr_t pc;
> +	uint32_t spsr;
> +	struct aapcs64_params_t args;
> +};
> +
> +/*****************************************************************************
> + * Image info binary provides information from the image loader that
> + * can be used by the firmware to manage available trusted RAM.
> + * More advanced firmware image formats can provide additional
> + * information that enables optimization or greater flexibility in the
> + * common firmware code
> + *****************************************************************************/
> +struct atf_image_info_t {
> +	struct param_header_t h;
> +	uintptr_t image_base;   /* physical address of base of image */
> +	uint32_t image_size;    /* bytes read from image file */
> +};
> +
> +/*****************************************************************************
> + * The image descriptor struct definition.
> + *****************************************************************************/
> +struct image_desc_t {
> +	/* Contains unique image id for the image. */
> +	unsigned int image_id;
> +	/*
> +	 * This member contains Image state information.
> +	 * Refer IMAGE_STATE_XXX defined above.
> +	 */
> +	unsigned int state;
> +	uint32_t copied_size;	/* image size copied in blocks */
> +	struct atf_image_info_t atf_image_info;
> +	struct entry_point_info_t ep_info;
> +};
> +
> +/*******************************************************************************
> + * This structure represents the superset of information that can be passed to
> + * BL31 e.g. while passing control to it from BL2. The BL32 parameters will be
> + * populated only if BL2 detects its presence. A pointer to a structure of this
> + * type should be passed in X0 to BL31's cold boot entrypoint.
> + *
> + * Use of this structure and the X0 parameter is not mandatory: the BL31
> + * platform code can use other mechanisms to provide the necessary information
> + * about BL32 and BL33 to the common and SPD code.
> + *
> + * BL31 image information is mandatory if this structure is used. If either of
> + * the optional BL32 and BL33 image information is not provided, this is
> + * indicated by the respective image_info pointers being zero.
> + ******************************************************************************/
> +struct bl31_params_t {
> +	struct param_header_t h;
> +	struct atf_image_info_t *bl31_image_info;
> +	struct entry_point_info_t *bl32_ep_info;
> +	struct atf_image_info_t *bl32_image_info;
> +	struct entry_point_info_t *bl33_ep_info;
> +	struct atf_image_info_t *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_t {
> +	struct bl31_params_t bl31_params;
> +	struct atf_image_info_t bl31_image_info;
> +	struct atf_image_info_t bl32_image_info;
> +	struct atf_image_info_t bl33_image_info;
> +	struct entry_point_info_t bl33_ep_info;
> +	struct entry_point_info_t bl32_ep_info;
> +	struct entry_point_info_t bl31_ep_info;
> +};
> +
> +#endif /*__ASSEMBLY__*/
> +
> +#endif /* __BL_COMMON_H__ */
> diff --git a/include/spl.h b/include/spl.h
> index a89ac00..8883657 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -260,4 +260,5 @@ int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface, char *devstr);
>  int spl_mmc_load_image(struct spl_image_info *spl_image,
>  		       struct spl_boot_device *bootdev);
>  
> +void bl31_entry(void);
>  #endif
> 

[U-Boot] [PATCH v2] spl: add support to booting with ATF

[Kever Yang]

Hi Andre,


On 03/23/2017 05:22 PM, Andre Przywara wrote:
> Hi Kever,
>
> I was wondering if we really need to copy in all those ATF definitions.
> I think this is really an *interface* between the loader (SPL or BL2)
> and the runtime services (BL31), so it's supposed to be stable and we
> wouldn't need to pull in all those headers.

Yes, you are right, this is an interface between the loader and bl31,
and it's from ATF, just like android image header if from Android project,
so my idea copy the header definition.
>
> So given that, can't we simply model the data structure, which at the
> end of the day is just a bunch of simple data types:

I'm sorry, I don't get your point here, what's the benefit for us to 
re-define
the header data types?
>
> struct param_header_t {
> 	uint8_t type;		/* type of the structure */
> 	uint8_t version;    /* version of this structure */
> 	uint16_t size;      /* size of this structure in bytes */
> 	uint32_t attr;      /* attributes: unused bits SBZ */
>
> struct entry_point_info_t {
> 	struct param_header_t h;
> 	uintptr_t pc;
> 	uint32_t spsr;
> 	unsigned long args[8]; /* struct aapcs64_params_t args; */
>
> struct atf_image_info_t {
> 	struct param_header_t h;
> 	uintptr_t image_base;   /* physical address of base of image */
> 	uint32_t image_size;    /* bytes read from image file */
>
>
> struct bl31_params_t {
> 	struct param_header_t h;
> 	struct atf_image_info_t *bl31_image_info;
> 	struct entry_point_info_t *bl32_ep_info;
> 	.....
>
> Whenever this gets changed we are in trouble anyway (because we break
> compatibility).

Yes, we hope it won't change, but it depends on ATF not U-Boot,
no matter what kind if data type we using in U-Boot, we can not stop this,
it's the ATF break the compatibility, not U-Boot SPL.
>
> So what are the opinions on this? Pull in the ATF source or define our
> own and mark it as an ATF copy?

ATF is the one using all these header structure, if they change there 
API and break,
yes, we need/have to sync again.

Dan in CC is ATF maintainer.

Hi Dan, are you able to provide some information for the header structure,
for example, is it suppose to change/update in the future?

Thanks,
- Kever
>
> Cheers,
> Andre.
>
> On 23/03/17 03:00, Kever Yang wrote:
>> ATF(ARM Trusted Firmware) is used by ARM arch64 SoCs, find more infomation
>> about ATF at: https://github.com/ARM-software/arm-trusted-firmware
>>
>> SPL is considered as BL2 in ATF terminology, it needs to load other parts
>> of ATF binary like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to
>> prepare the parameter for BL31 which including entry and image information
>> for all other images. Then the SPL handle PC to BL31 with the parameter,
>> the BL31 will do the rest of work and at last get into U-Boot(BL33).
>>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>
>> Changes in v2:
>> - Kconfig comment update with Simon's suggestion
>> - including file ordering,
>> - update function comment format,
>> - use 'if' instead of '#ifdef' for bl31_entry
>> - add ATF Kconfig option depend on ARM64
>> Series-changes: 1
>> - license update
>> - split out as separate patch
>>
>>   common/spl/Kconfig   |  14 +++
>>   common/spl/Makefile  |   1 +
>>   common/spl/spl.c     |   3 +
>>   common/spl/spl_atf.c |  96 ++++++++++++++++++
>>   include/atf_common.h | 276 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/spl.h        |   1 +
>>   6 files changed, 391 insertions(+)
>>   create mode 100644 common/spl/spl_atf.c
>>   create mode 100644 include/atf_common.h
>>
>> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
>> index 60ae60c..1b7fb8d 100644
>> --- a/common/spl/Kconfig
>> +++ b/common/spl/Kconfig
>> @@ -670,6 +670,20 @@ config SPL_YMODEM_SUPPORT
>>   	  means of transmitting U-Boot over a serial line for using in SPL,
>>   	  with a checksum to ensure correctness.
>>   
>> +config SPL_ATF_SUPPORT
>> +	bool "Support ARM Trusted Firmware"
>> +	depends on SPL && ARM64
>> +	help
>> +	  ATF(ARM Trusted Firmware) is a component for ARM arch64 which which
>> +	  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_TEXT_BASE
>> +	depends on SPL_ATF_SUPPORT
>> +	hex "ATF TEXT BASE addr"
>> +	help
>> +	  This is the base address in memory for ATF text and entry point.
>> +
>>   config TPL_ENV_SUPPORT
>>   	bool "Support an environment"
>>   	depends on TPL
>> diff --git a/common/spl/Makefile b/common/spl/Makefile
>> index 1933cbd..b3b34d6 100644
>> --- a/common/spl/Makefile
>> +++ b/common/spl/Makefile
>> @@ -20,6 +20,7 @@ endif
>>   obj-$(CONFIG_SPL_UBI) += spl_ubi.o
>>   obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o
>>   obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o
>> +obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o
>>   obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o
>>   obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o
>>   obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.o
>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> index 26bc9ef..7041e1b 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -374,6 +374,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>>   	      gd->malloc_ptr / 1024);
>>   #endif
>>   
>> +	if (IS_ENABLED(CONFIG_SPL_ATF_SUPPORT))
>> +		bl31_entry();
>> +
>>   	debug("loaded - jumping to U-Boot...\n");
>>   	spl_board_prepare_for_boot();
>>   	jump_to_image_no_args(&spl_image);
>> diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
>> new file mode 100644
>> index 0000000..38753d7
>> --- /dev/null
>> +++ b/common/spl/spl_atf.c
>> @@ -0,0 +1,96 @@
>> +/*
>> + * Reference to the ARM TF Project,
>> + * plat/arm/common/arm_bl2_setup.c
>> + * Portions copyright (c) 2013-2016, ARM Limited and Contributors. All rights
>> + * reserved.
>> + * Copyright (C) 2016 Rockchip Electronic Co.,Ltd
>> + * Written by Kever Yang <kever.yang@rock-chips.com>
>> + *
>> + * SPDX-License-Identifier:     BSD-3-Clause
>> + */
>> +
>> +#include <common.h>
>> +#include <atf_common.h>
>> +#include <errno.h>
>> +#include <spl.h>
>> +
>> +static struct bl2_to_bl31_params_mem_t bl31_params_mem;
>> +static struct bl31_params_t *bl2_to_bl31_params;
>> +
>> +/**
>> + * bl2_plat_get_bl31_params() - prepare params for bl31.
>> + *
>> + * 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_t 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
>> + *
>> + * @return bl31 params structure pointer
>> + */
>> +struct bl31_params_t *bl2_plat_get_bl31_params(void)
>> +{
>> +	struct entry_point_info_t *bl33_ep_info;
>> +
>> +	/*
>> +	 * Initialise the memory for all the arguments that needs to
>> +	 * be passed to BL31
>> +	 */
>> +	memset(&bl31_params_mem, 0, sizeof(struct bl2_to_bl31_params_mem_t));
>> +
>> +	/* Assign memory for TF related information */
>> +	bl2_to_bl31_params = &bl31_params_mem.bl31_params;
>> +	SET_PARAM_HEAD(bl2_to_bl31_params, PARAM_BL31, VERSION_1, 0);
>> +
>> +	/* Fill BL31 related information */
>> +	SET_PARAM_HEAD(bl2_to_bl31_params->bl31_image_info, PARAM_IMAGE_BINARY,
>> +		       VERSION_1, 0);
>> +
>> +	/* Fill BL32 related information if it exists */
>> +#ifdef BL32_BASE
>> +	bl2_to_bl31_params->bl32_ep_info = &bl31_params_mem.bl32_ep_info;
>> +	SET_PARAM_HEAD(bl2_to_bl31_params->bl32_ep_info, PARAM_EP,
>> +		       VERSION_1, 0);
>> +	bl2_to_bl31_params->bl32_image_info = &bl31_params_mem.bl32_image_info;
>> +	SET_PARAM_HEAD(bl2_to_bl31_params->bl32_image_info, PARAM_IMAGE_BINARY,
>> +		       VERSION_1, 0);
>> +#endif /* BL32_BASE */
>> +
>> +	/* Fill BL33 related information */
>> +	bl2_to_bl31_params->bl33_ep_info = &bl31_params_mem.bl33_ep_info;
>> +	bl33_ep_info = &bl31_params_mem.bl33_ep_info;
>> +	SET_PARAM_HEAD(bl33_ep_info, PARAM_EP, VERSION_1, EP_NON_SECURE);
>> +
>> +	/* BL33 expects to receive the primary CPU MPID (through x0) */
>> +	bl33_ep_info->args.arg0 = 0xffff & read_mpidr();
>> +	bl33_ep_info->pc = CONFIG_SYS_TEXT_BASE;
>> +	bl33_ep_info->spsr = SPSR_64(MODE_EL2, MODE_SP_ELX,
>> +				     DISABLE_ALL_EXECPTIONS);
>> +
>> +	bl2_to_bl31_params->bl33_image_info = &bl31_params_mem.bl33_image_info;
>> +	SET_PARAM_HEAD(bl2_to_bl31_params->bl33_image_info, PARAM_IMAGE_BINARY,
>> +		       VERSION_1, 0);
>> +
>> +	return bl2_to_bl31_params;
>> +}
>> +
>> +void raw_write_daif(uint32_t daif)
>> +{
>> +	__asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory");
>> +}
>> +
>> +void bl31_entry(void)
>> +{
>> +	struct bl31_params_t *bl31_params;
>> +	void (*entry)(struct bl31_params_t *params, void *plat_params) = NULL;
>> +
>> +	bl31_params = bl2_plat_get_bl31_params();
>> +	entry = (void *)CONFIG_SPL_ATF_TEXT_BASE;
>> +
>> +	raw_write_daif(SPSR_EXCEPTION_MASK);
>> +	dcache_disable();
>> +
>> +	entry(bl31_params, NULL);
>> +}
>> diff --git a/include/atf_common.h b/include/atf_common.h
>> new file mode 100644
>> index 0000000..90c7fce
>> --- /dev/null
>> +++ b/include/atf_common.h
>> @@ -0,0 +1,276 @@
>> +/*
>> + * This is from the ARM TF Project,
>> + * Repository: https://github.com/ARM-software/arm-trusted-firmware.git
>> + * File: include/common/bl_common.h
>> + * Portions copyright (c) 2013-2016, ARM Limited and Contributors. All rights
>> + * reserved.
>> + * Copyright (C) 2016-2017 Rockchip Electronic Co.,Ltd
>> + *
>> + * SPDX-License-Identifier:     BSD-3-Clause
>> + */
>> +
>> +#ifndef __BL_COMMON_H__
>> +#define __BL_COMMON_H__
>> +
>> +#define SECURE		0x0
>> +#define NON_SECURE	0x1
>> +#define sec_state_is_valid(s) (((s) == SECURE) || ((s) == NON_SECURE))
>> +
>> +#define UP	1
>> +#define DOWN	0
>> +
>> +/*******************************************************************************
>> + * Constants to identify the location of a memory region in a given memory
>> + * layout.
>> +******************************************************************************/
>> +#define TOP	0x1
>> +#define BOTTOM	!TOP
>> +
>> +/*******************************************************************************
>> + * Constants that allow assembler code to access members of and the
>> + * 'entry_point_info' structure at their correct offsets.
>> + ******************************************************************************/
>> +#define ENTRY_POINT_INFO_PC_OFFSET	0x08
>> +#define ENTRY_POINT_INFO_ARGS_OFFSET	0x18
>> +
>> +/* The following are used to set/get image attributes. */
>> +#define PARAM_EP_SECURITY_MASK		(0x1)
>> +
>> +#define GET_SECURITY_STATE(x) (x & PARAM_EP_SECURITY_MASK)
>> +#define SET_SECURITY_STATE(x, security) \
>> +			((x) = ((x) & ~PARAM_EP_SECURITY_MASK) | (security))
>> +
>> +
>> +/*
>> + * The following are used for image state attributes.
>> + * Image can only be in one of the following state.
>> + */
>> +#define IMAGE_STATE_RESET			0
>> +#define IMAGE_STATE_COPIED			1
>> +#define IMAGE_STATE_COPYING			2
>> +#define IMAGE_STATE_AUTHENTICATED		3
>> +#define IMAGE_STATE_EXECUTED			4
>> +#define IMAGE_STATE_INTERRUPTED			5
>> +
>> +#define EP_SECURE	0x0
>> +#define EP_NON_SECURE	0x1
>> +
>> +#define EP_EE_MASK	0x2
>> +#define EP_EE_LITTLE	0x0
>> +#define EP_EE_BIG	0x2
>> +#define EP_GET_EE(x) (x & EP_EE_MASK)
>> +#define EP_SET_EE(x, ee) ((x) = ((x) & ~EP_EE_MASK) | (ee))
>> +
>> +#define EP_ST_MASK	0x4
>> +#define EP_ST_DISABLE	0x0
>> +#define EP_ST_ENABLE	0x4
>> +#define EP_GET_ST(x) (x & EP_ST_MASK)
>> +#define EP_SET_ST(x, ee) ((x) = ((x) & ~EP_ST_MASK) | (ee))
>> +
>> +#define EP_EXE_MASK	0x8
>> +#define NON_EXECUTABLE	0x0
>> +#define EXECUTABLE	0x8
>> +#define EP_GET_EXE(x) (x & EP_EXE_MASK)
>> +#define EP_SET_EXE(x, ee) ((x) = ((x) & ~EP_EXE_MASK) | (ee))
>> +
>> +#define PARAM_EP		0x01
>> +#define PARAM_IMAGE_BINARY	0x02
>> +#define PARAM_BL31		0x03
>> +
>> +#define VERSION_1	0x01
>> +
>> +#define INVALID_IMAGE_ID		(0xFFFFFFFF)
>> +
>> +#define SET_PARAM_HEAD(_p, _type, _ver, _attr) do { \
>> +	(_p)->h.type = (uint8_t)(_type); \
>> +	(_p)->h.version = (uint8_t)(_ver); \
>> +	(_p)->h.size = (uint16_t)sizeof(*_p); \
>> +	(_p)->h.attr = (uint32_t)(_attr) ; \
>> +	} while (0)
>> +
>> +/* Following is used for populating structure members statically. */
>> +#define SET_STATIC_PARAM_HEAD(_p, _type, _ver, _p_type, _attr)	\
>> +	._p.h.type = (uint8_t)(_type), \
>> +	._p.h.version = (uint8_t)(_ver), \
>> +	._p.h.size = (uint16_t)sizeof(_p_type), \
>> +	._p.h.attr = (uint32_t)(_attr)
>> +
>> +#define MODE_RW_SHIFT	0x4
>> +#define MODE_RW_MASK	0x1
>> +#define MODE_RW_64	0x0
>> +#define MODE_RW_32	0x1
>> +
>> +#define MODE_EL_SHIFT	0x2
>> +#define MODE_EL_MASK	0x3
>> +#define MODE_EL3	0x3
>> +#define MODE_EL2	0x2
>> +#define MODE_EL1	0x1
>> +#define MODE_EL0	0x0
>> +
>> +#define MODE_SP_SHIFT	0x0
>> +#define MODE_SP_MASK	0x1
>> +#define MODE_SP_EL0	0x0
>> +#define MODE_SP_ELX	0x1
>> +
>> +#define SPSR_DAIF_SHIFT	6
>> +#define SPSR_DAIF_MASK	0x0f
>> +
>> +#define DAIF_FIQ_BIT (1<<0)
>> +#define DAIF_IRQ_BIT (1<<0)
>> +#define DAIF_ABT_BIT (1<<0)
>> +#define DAIF_DBG_BIT (1<<0)
>> +#define DISABLE_ALL_EXECPTIONS	\
>> +	(DAIF_FIQ_BIT | DAIF_IRQ_BIT | DAIF_ABT_BIT | DAIF_DBG_BIT)
>> +
>> +#define SPSR_64(el, sp, daif)		\
>> +	(MODE_RW_64 << MODE_RW_SHIFT |	\
>> +	 ((el) & MODE_EL_MASK) << MODE_EL_SHIFT |	\
>> +	 ((sp) & MODE_SP_MASK) << MODE_SP_SHIFT |	\
>> +	 ((daif) & SPSR_DAIF_MASK) << SPSR_DAIF_SHIFT)
>> +
>> +/*******************************************************************************
>> + * Constants to indicate type of exception to the common exception handler.
>> + ******************************************************************************/
>> +#define SYNC_EXCEPTION_SP_EL0		0x0
>> +#define IRQ_SP_EL0			0x1
>> +#define FIQ_SP_EL0			0x2
>> +#define SERROR_SP_EL0			0x3
>> +#define SYNC_EXCEPTION_SP_ELX		0x4
>> +#define IRQ_SP_ELX			0x5
>> +#define FIQ_SP_ELX			0x6
>> +#define SERROR_SP_ELX			0x7
>> +#define SYNC_EXCEPTION_AARCH64		0x8
>> +#define IRQ_AARCH64			0x9
>> +#define FIQ_AARCH64			0xa
>> +#define SERROR_AARCH64			0xb
>> +#define SYNC_EXCEPTION_AARCH32		0xc
>> +#define IRQ_AARCH32			0xd
>> +#define FIQ_AARCH32			0xe
>> +#define SERROR_AARCH32			0xf
>> +
>> +#define SPSR_USE_L           0
>> +#define SPSR_USE_H           1
>> +#define SPSR_L_H_MASK        1
>> +#define SPSR_M_SHIFT         4
>> +#define SPSR_ERET_32         (1 << SPSR_M_SHIFT)
>> +#define SPSR_ERET_64         (0 << SPSR_M_SHIFT)
>> +#define SPSR_FIQ             (1 << 6)
>> +#define SPSR_IRQ             (1 << 7)
>> +#define SPSR_SERROR          (1 << 8)
>> +#define SPSR_DEBUG           (1 << 9)
>> +#define SPSR_EXCEPTION_MASK  (SPSR_FIQ | SPSR_IRQ | SPSR_SERROR | SPSR_DEBUG)
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +/*******************************************************************************
>> + * Structure used for telling the next BL how much of a particular type of
>> + * memory is available for its use and how much is already used.
>> + ******************************************************************************/
>> +struct aapcs64_params_t {
>> +	unsigned long arg0;
>> +	unsigned long arg1;
>> +	unsigned long arg2;
>> +	unsigned long arg3;
>> +	unsigned long arg4;
>> +	unsigned long arg5;
>> +	unsigned long arg6;
>> +	unsigned long arg7;
>> +};
>> +
>> +/***************************************************************************
>> + * This structure provides version information and the size of the
>> + * structure, attributes for the structure it represents
>> + ***************************************************************************/
>> +struct param_header_t {
>> +	uint8_t type;		/* type of the structure */
>> +	uint8_t version;    /* version of this structure */
>> +	uint16_t size;      /* size of this structure in bytes */
>> +	uint32_t attr;      /* attributes: unused bits SBZ */
>> +};
>> +
>> +/*****************************************************************************
>> + * This structure represents the superset of information needed while
>> + * switching exception levels. The only two mechanisms to do so are
>> + * ERET & SMC. Security state is indicated using bit zero of header
>> + * attribute
>> + * NOTE: BL1 expects entrypoint followed by spsr at an offset from the start
>> + * of this structure defined by the macro `ENTRY_POINT_INFO_PC_OFFSET` while
>> + * processing SMC to jump to BL31.
>> + *****************************************************************************/
>> +struct entry_point_info_t {
>> +	struct param_header_t h;
>> +	uintptr_t pc;
>> +	uint32_t spsr;
>> +	struct aapcs64_params_t args;
>> +};
>> +
>> +/*****************************************************************************
>> + * Image info binary provides information from the image loader that
>> + * can be used by the firmware to manage available trusted RAM.
>> + * More advanced firmware image formats can provide additional
>> + * information that enables optimization or greater flexibility in the
>> + * common firmware code
>> + *****************************************************************************/
>> +struct atf_image_info_t {
>> +	struct param_header_t h;
>> +	uintptr_t image_base;   /* physical address of base of image */
>> +	uint32_t image_size;    /* bytes read from image file */
>> +};
>> +
>> +/*****************************************************************************
>> + * The image descriptor struct definition.
>> + *****************************************************************************/
>> +struct image_desc_t {
>> +	/* Contains unique image id for the image. */
>> +	unsigned int image_id;
>> +	/*
>> +	 * This member contains Image state information.
>> +	 * Refer IMAGE_STATE_XXX defined above.
>> +	 */
>> +	unsigned int state;
>> +	uint32_t copied_size;	/* image size copied in blocks */
>> +	struct atf_image_info_t atf_image_info;
>> +	struct entry_point_info_t ep_info;
>> +};
>> +
>> +/*******************************************************************************
>> + * This structure represents the superset of information that can be passed to
>> + * BL31 e.g. while passing control to it from BL2. The BL32 parameters will be
>> + * populated only if BL2 detects its presence. A pointer to a structure of this
>> + * type should be passed in X0 to BL31's cold boot entrypoint.
>> + *
>> + * Use of this structure and the X0 parameter is not mandatory: the BL31
>> + * platform code can use other mechanisms to provide the necessary information
>> + * about BL32 and BL33 to the common and SPD code.
>> + *
>> + * BL31 image information is mandatory if this structure is used. If either of
>> + * the optional BL32 and BL33 image information is not provided, this is
>> + * indicated by the respective image_info pointers being zero.
>> + ******************************************************************************/
>> +struct bl31_params_t {
>> +	struct param_header_t h;
>> +	struct atf_image_info_t *bl31_image_info;
>> +	struct entry_point_info_t *bl32_ep_info;
>> +	struct atf_image_info_t *bl32_image_info;
>> +	struct entry_point_info_t *bl33_ep_info;
>> +	struct atf_image_info_t *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_t {
>> +	struct bl31_params_t bl31_params;
>> +	struct atf_image_info_t bl31_image_info;
>> +	struct atf_image_info_t bl32_image_info;
>> +	struct atf_image_info_t bl33_image_info;
>> +	struct entry_point_info_t bl33_ep_info;
>> +	struct entry_point_info_t bl32_ep_info;
>> +	struct entry_point_info_t bl31_ep_info;
>> +};
>> +
>> +#endif /*__ASSEMBLY__*/
>> +
>> +#endif /* __BL_COMMON_H__ */
>> diff --git a/include/spl.h b/include/spl.h
>> index a89ac00..8883657 100644
>> --- a/include/spl.h
>> +++ b/include/spl.h
>> @@ -260,4 +260,5 @@ int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface, char *devstr);
>>   int spl_mmc_load_image(struct spl_image_info *spl_image,
>>   		       struct spl_boot_device *bootdev);
>>   
>> +void bl31_entry(void);
>>   #endif
>>
>
>

[U-Boot] [PATCH v2] spl: add support to booting with ATF

[Dan Handley]

Hi Kever

> -----Original Message-----
> From: Kever Yang [mailto:kever.yang at rock-chips.com]
> Sent: 28 March 2017 08:23
>
> Hi Andre,
>
>
> On 03/23/2017 05:22 PM, Andre Przywara wrote:
> > Hi Kever,
> >
> > I was wondering if we really need to copy in all those ATF definitions.
> > I think this is really an *interface* between the loader (SPL or BL2)
> > and the runtime services (BL31), so it's supposed to be stable and we
> > wouldn't need to pull in all those headers.
>
> Yes, you are right, this is an interface between the loader and bl31,
> and it's from ATF, just like android image header if from Android
> project, so my idea copy the header definition.
> >
> > So given that, can't we simply model the data structure, which at the
> > end of the day is just a bunch of simple data types:
>
> I'm sorry, I don't get your point here, what's the benefit for us to re-
> define the header data types?
> >
I guess Andre's point was to copy just the essential data structures, not all the other baggage in that header file.
...
> >
> > Whenever this gets changed we are in trouble anyway (because we break
> > compatibility).
>
> Yes, we hope it won't change, but it depends on ATF not U-Boot, no
> matter what kind if data type we using in U-Boot, we can not stop this,
> it's the ATF break the compatibility, not U-Boot SPL.
> >
> > So what are the opinions on this? Pull in the ATF source or define our
> > own and mark it as an ATF copy?
>
> ATF is the one using all these header structure, if they change there
> API and break, yes, we need/have to sync again.
>
> Dan in CC is ATF maintainer.
>
> Hi Dan, are you able to provide some information for the header
> structure, for example, is it suppose to change/update in the future?
>
The ARM TF BL31 interface is defined here:
https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/firmware-design.md#required-cpu-state-when-calling-bl31_entrypoint-during-cold-boot

In summary, the arguments passed to BL31 in X0 and X1 are platform specific and not used by generic BL31 code. Conventionally, X0 has been used by TF BL2 to pass a bl31_params structure, which is used by ARM Limited platforms and possibly others but not necessarily all. This structure has now been superseded by a new structure called bl_params, which caters for AArch32 platforms and should be more futureproof. Both bl31_params and bl_params are prefixed with metadata in a param_header, which includes a version number. BL31 platform code could potentially handle both bl31_params and bl_params by checking the version number but ARM platforms expect one or the other depending on the value of a build flag (LOAD_IMAGE_V2). It's hard to say what other platforms do but I guess a lot of them use bl31_params.

I suggest that U-Boot SPL mandates that platforms that wish to use TF BL31 with U-Boot SPL must use the bl_params structure. It may also need to mandate that this contains BL33 information specific to U-Boot.

Alternatively, U-Boot SPL could define its own structure(s) according to its needs, and mandate that BL31 platforms it wishes to support conform to this.

> Thanks,
> - Kever
> >

Regards

Dan.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[U-Boot] [PATCH v2] spl: add support to booting with ATF

[Andre Przywara]

Hi,

On 28/03/17 15:16, Dan Handley wrote:
> Hi Kever
> 
>> -----Original Message-----
>> From: Kever Yang [mailto:kever.yang at rock-chips.com]
>> Sent: 28 March 2017 08:23
>> 
>> Hi Andre,
>> 
>> 
>> On 03/23/2017 05:22 PM, Andre Przywara wrote:
>> > Hi Kever,
>> >
>> > I was wondering if we really need to copy in all those ATF definitions.
>> > I think this is really an *interface* between the loader (SPL or BL2)
>> > and the runtime services (BL31), so it's supposed to be stable and we
>> > wouldn't need to pull in all those headers.
>> 
>> Yes, you are right, this is an interface between the loader and bl31,
>> and it's from ATF, just like android image header if from Android
>> project, so my idea copy the header definition.
>> >
>> > So given that, can't we simply model the data structure, which at the
>> > end of the day is just a bunch of simple data types:
>> 
>> I'm sorry, I don't get your point here, what's the benefit for us to re-
>> define the header data types?
>> >
> I guess Andre's point was to copy just the essential data structures,
> not all the other baggage in that header file.

Yes, that what my intention.
I just wanted to avoid pasting this pretty big and somewhat foreign code
into U-Boot, when all we need is four simple data structures.

>> >
>> > Whenever this gets changed we are in trouble anyway (because we break
>> > compatibility).
>> 
>> Yes, we hope it won't change, but it depends on ATF not U-Boot, no
>> matter what kind if data type we using in U-Boot, we can not stop this,
>> it's the ATF break the compatibility, not U-Boot SPL.

But as mentioned below, ATF can handle multiple versions, so it can keep
compatibility.

>> > So what are the opinions on this? Pull in the ATF source or define our
>> > own and mark it as an ATF copy?
>> 
>> ATF is the one using all these header structure, if they change there
>> API and break, yes, we need/have to sync again.
>> 
>> Dan in CC is ATF maintainer.
>> 
>> Hi Dan, are you able to provide some information for the header
>> structure, for example, is it suppose to change/update in the future?
>> 
> The ARM TF BL31 interface is defined here:
> https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/firmware-design.md#required-cpu-state-when-calling-bl31_entrypoint-during-cold-boot
> 
> In summary, the arguments passed to BL31 in X0 and X1 are platform
> specific and not used by generic BL31 code. Conventionally, X0 has been
> used by TF BL2 to pass a bl31_params structure, which is used by ARM
> Limited platforms and possibly others but not necessarily all. This
> structure has now been superseded by a new structure called bl_params,
> which caters for AArch32 platforms and should be more futureproof. Both
> bl31_params and bl_params are prefixed with metadata in a param_header,
> which includes a version number. BL31 platform code could potentially
> handle both bl31_params and bl_params by checking the version number but
> ARM platforms expect one or the other depending on the value of a build
> flag (LOAD_IMAGE_V2). It's hard to say what other platforms do but I
> guess a lot of them use bl31_params.

Yes, so with the versioning scheme I don't see many problems. A really
versatile BL31 could handle both, so would be compatible.
At the end of the day I think this is a theoretical issue, as you
usually package ATF, SPL and U-Boot together and can make sure that they
match. It's not like people insert their own bl31 into an existing
image. And with everything being open source you can always rebuild anyway.

> I suggest that U-Boot SPL mandates that platforms that wish to use TF
> BL31 with U-Boot SPL must use the bl_params structure. It may also need
> to mandate that this contains BL33 information specific to U-Boot.

That sounds like a good idea.

> Alternatively, U-Boot SPL could define its own structure(s) according to
> its needs, and mandate that BL31 platforms it wishes to support conform
> to this.

Philipp had the idea of using a small device tree blob to convey
information between the SPL (or any other loader, really) and BL31,
similar to what the FIT does. In fact it could even be the FIT image. So
the bl_params structure would just contain a pointer to that DT blob in
memory, and bl31 then parses this to get as many information as it needs
from the loader, without affecting the actual SPL->BL31 interface. Yes,
that is another level of indirection, but I think worth to explore - at
least for the future. This would for instance allow the loader to pass
the address of the device DT to bl31, which could then make use of it -
either for getting information or for inserting nodes (PSCI or other
firmware services) - or for both.

Cheers,
Andre.

[U-Boot] [PATCH v2] spl: add support to booting with ATF

[Simon Glass]

Hi,

On 28 March 2017 at 08:37, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi,
>
> On 28/03/17 15:16, Dan Handley wrote:
>> Hi Kever
>>
>>> -----Original Message-----
>>> From: Kever Yang [mailto:kever.yang at rock-chips.com]
>>> Sent: 28 March 2017 08:23
>>>
>>> Hi Andre,
>>>
>>>
>>> On 03/23/2017 05:22 PM, Andre Przywara wrote:
>>> > Hi Kever,
>>> >
>>> > I was wondering if we really need to copy in all those ATF definitions.
>>> > I think this is really an *interface* between the loader (SPL or BL2)
>>> > and the runtime services (BL31), so it's supposed to be stable and we
>>> > wouldn't need to pull in all those headers.
>>>
>>> Yes, you are right, this is an interface between the loader and bl31,
>>> and it's from ATF, just like android image header if from Android
>>> project, so my idea copy the header definition.
>>> >
>>> > So given that, can't we simply model the data structure, which at the
>>> > end of the day is just a bunch of simple data types:
>>>
>>> I'm sorry, I don't get your point here, what's the benefit for us to re-
>>> define the header data types?
>>> >
>> I guess Andre's point was to copy just the essential data structures,
>> not all the other baggage in that header file.
>
> Yes, that what my intention.
> I just wanted to avoid pasting this pretty big and somewhat foreign code
> into U-Boot, when all we need is four simple data structures.
>
>>> >
>>> > Whenever this gets changed we are in trouble anyway (because we break
>>> > compatibility).
>>>
>>> Yes, we hope it won't change, but it depends on ATF not U-Boot, no
>>> matter what kind if data type we using in U-Boot, we can not stop this,
>>> it's the ATF break the compatibility, not U-Boot SPL.
>
> But as mentioned below, ATF can handle multiple versions, so it can keep
> compatibility.
>
>>> > So what are the opinions on this? Pull in the ATF source or define our
>>> > own and mark it as an ATF copy?
>>>
>>> ATF is the one using all these header structure, if they change there
>>> API and break, yes, we need/have to sync again.
>>>
>>> Dan in CC is ATF maintainer.
>>>
>>> Hi Dan, are you able to provide some information for the header
>>> structure, for example, is it suppose to change/update in the future?
>>>
>> The ARM TF BL31 interface is defined here:
>> https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/firmware-design.md#required-cpu-state-when-calling-bl31_entrypoint-during-cold-boot
>>
>> In summary, the arguments passed to BL31 in X0 and X1 are platform
>> specific and not used by generic BL31 code. Conventionally, X0 has been
>> used by TF BL2 to pass a bl31_params structure, which is used by ARM
>> Limited platforms and possibly others but not necessarily all. This
>> structure has now been superseded by a new structure called bl_params,
>> which caters for AArch32 platforms and should be more futureproof. Both
>> bl31_params and bl_params are prefixed with metadata in a param_header,
>> which includes a version number. BL31 platform code could potentially
>> handle both bl31_params and bl_params by checking the version number but
>> ARM platforms expect one or the other depending on the value of a build
>> flag (LOAD_IMAGE_V2). It's hard to say what other platforms do but I
>> guess a lot of them use bl31_params.
>
> Yes, so with the versioning scheme I don't see many problems. A really
> versatile BL31 could handle both, so would be compatible.
> At the end of the day I think this is a theoretical issue, as you
> usually package ATF, SPL and U-Boot together and can make sure that they
> match. It's not like people insert their own bl31 into an existing
> image. And with everything being open source you can always rebuild anyway.
>
>> I suggest that U-Boot SPL mandates that platforms that wish to use TF
>> BL31 with U-Boot SPL must use the bl_params structure. It may also need
>> to mandate that this contains BL33 information specific to U-Boot.
>
> That sounds like a good idea.
>
>> Alternatively, U-Boot SPL could define its own structure(s) according to
>> its needs, and mandate that BL31 platforms it wishes to support conform
>> to this.
>
> Philipp had the idea of using a small device tree blob to convey
> information between the SPL (or any other loader, really) and BL31,
> similar to what the FIT does. In fact it could even be the FIT image. So
> the bl_params structure would just contain a pointer to that DT blob in
> memory, and bl31 then parses this to get as many information as it needs
> from the loader, without affecting the actual SPL->BL31 interface. Yes,
> that is another level of indirection, but I think worth to explore - at
> least for the future. This would for instance allow the loader to pass
> the address of the device DT to bl31, which could then make use of it -
> either for getting information or for inserting nodes (PSCI or other
> firmware services) - or for both.

Perhaps this could be future work? For now, I suggest we take this
patch once the header file has been cut down.

Regards,
Simon

[U-Boot] [PATCH v2] spl: add support to booting with ATF

[Kever Yang]

Hi,


On 04/01/2017 12:22 PM, Simon Glass wrote:
> Hi,
>
> On 28 March 2017 at 08:37, Andre Przywara <andre.przywara@arm.com> wrote:
>> Hi,
>>
>> On 28/03/17 15:16, Dan Handley wrote:
>>> Hi Kever
>>>
>>>> -----Original Message-----
>>>> From: Kever Yang [mailto:kever.yang at rock-chips.com]
>>>> Sent: 28 March 2017 08:23
>>>>
>>>> Hi Andre,
>>>>
>>>>
>>>> On 03/23/2017 05:22 PM, Andre Przywara wrote:
>>>>> Hi Kever,
>>>>>
>>>>> I was wondering if we really need to copy in all those ATF definitions.
>>>>> I think this is really an *interface* between the loader (SPL or BL2)
>>>>> and the runtime services (BL31), so it's supposed to be stable and we
>>>>> wouldn't need to pull in all those headers.
>>>> Yes, you are right, this is an interface between the loader and bl31,
>>>> and it's from ATF, just like android image header if from Android
>>>> project, so my idea copy the header definition.
>>>>> So given that, can't we simply model the data structure, which at the
>>>>> end of the day is just a bunch of simple data types:
>>>> I'm sorry, I don't get your point here, what's the benefit for us to re-
>>>> define the header data types?
>>> I guess Andre's point was to copy just the essential data structures,
>>> not all the other baggage in that header file.
>> Yes, that what my intention.
>> I just wanted to avoid pasting this pretty big and somewhat foreign code
>> into U-Boot, when all we need is four simple data structures.

Thanks for the detail explain, yes the data structures are the most 
important,
and other definition could be removed if not necessary.
>>
>>>>> Whenever this gets changed we are in trouble anyway (because we break
>>>>> compatibility).
>>>> Yes, we hope it won't change, but it depends on ATF not U-Boot, no
>>>> matter what kind if data type we using in U-Boot, we can not stop this,
>>>> it's the ATF break the compatibility, not U-Boot SPL.
>> But as mentioned below, ATF can handle multiple versions, so it can keep
>> compatibility.
>>
>>>>> So what are the opinions on this? Pull in the ATF source or define our
>>>>> own and mark it as an ATF copy?
>>>> ATF is the one using all these header structure, if they change there
>>>> API and break, yes, we need/have to sync again.
>>>>
>>>> Dan in CC is ATF maintainer.
>>>>
>>>> Hi Dan, are you able to provide some information for the header
>>>> structure, for example, is it suppose to change/update in the future?
>>>>
>>> The ARM TF BL31 interface is defined here:
>>> https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/firmware-design.md#required-cpu-state-when-calling-bl31_entrypoint-during-cold-boot
>>>
>>> In summary, the arguments passed to BL31 in X0 and X1 are platform
>>> specific and not used by generic BL31 code. Conventionally, X0 has been
>>> used by TF BL2 to pass a bl31_params structure, which is used by ARM
>>> Limited platforms and possibly others but not necessarily all. This
>>> structure has now been superseded by a new structure called bl_params,
>>> which caters for AArch32 platforms and should be more futureproof. Both
>>> bl31_params and bl_params are prefixed with metadata in a param_header,
>>> which includes a version number. BL31 platform code could potentially
>>> handle both bl31_params and bl_params by checking the version number but
>>> ARM platforms expect one or the other depending on the value of a build
>>> flag (LOAD_IMAGE_V2). It's hard to say what other platforms do but I
>>> guess a lot of them use bl31_params.
>> Yes, so with the versioning scheme I don't see many problems. A really
>> versatile BL31 could handle both, so would be compatible.
>> At the end of the day I think this is a theoretical issue, as you
>> usually package ATF, SPL and U-Boot together and can make sure that they
>> match. It's not like people insert their own bl31 into an existing
>> image. And with everything being open source you can always rebuild anyway.
>>
>>> I suggest that U-Boot SPL mandates that platforms that wish to use TF
>>> BL31 with U-Boot SPL must use the bl_params structure. It may also need
>>> to mandate that this contains BL33 information specific to U-Boot.
>> That sounds like a good idea.
>>
>>> Alternatively, U-Boot SPL could define its own structure(s) according to
>>> its needs, and mandate that BL31 platforms it wishes to support conform
>>> to this.
>> Philipp had the idea of using a small device tree blob to convey
>> information between the SPL (or any other loader, really) and BL31,
>> similar to what the FIT does. In fact it could even be the FIT image. So
>> the bl_params structure would just contain a pointer to that DT blob in
>> memory, and bl31 then parses this to get as many information as it needs
>> from the loader, without affecting the actual SPL->BL31 interface. Yes,
>> that is another level of indirection, but I think worth to explore - at
>> least for the future. This would for instance allow the loader to pass
>> the address of the device DT to bl31, which could then make use of it -
>> either for getting information or for inserting nodes (PSCI or other
>> firmware services) - or for both.
> Perhaps this could be future work? For now, I suggest we take this
> patch once the header file has been cut down.

I agree, I will send one new patch first which remove some MACRO not
used.

Thanks,
- Kever
>
> Regards,
> Simon
>