All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 0/1] arm64: enable SPL with ATF support
@ 2017-04-05  5:05 Kever Yang
  2017-04-05  5:05 ` [U-Boot] [PATCH v3] spl: add support to booting with ATF Kever Yang
  0 siblings, 1 reply; 7+ messages in thread
From: Kever Yang @ 2017-04-05  5:05 UTC (permalink / raw)
  To: u-boot


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 v3:
- remove no neccessary Macro definition in header file.
- rename some Macro with 'ATF_' prefix.

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

Changes in v1:
- 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 |  97 +++++++++++++++++++++++++++
 include/atf_common.h | 183 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/spl.h        |   1 +
 6 files changed, 299 insertions(+)
 create mode 100644 common/spl/spl_atf.c
 create mode 100644 include/atf_common.h

-- 
1.9.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH v3] spl: add support to booting with ATF
  2017-04-05  5:05 [U-Boot] [PATCH v3 0/1] arm64: enable SPL with ATF support Kever Yang
@ 2017-04-05  5:05 ` Kever Yang
  2017-04-05  5:35   ` Masahiro Yamada
  2017-04-05 16:40   ` Masahiro Yamada
  0 siblings, 2 replies; 7+ messages in thread
From: Kever Yang @ 2017-04-05  5:05 UTC (permalink / raw)
  To: u-boot

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 v3:
- remove no neccessary Macro definition in header file.
- rename some Macro with 'ATF_' prefix.

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

Changes in v1:
- 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 |  97 +++++++++++++++++++++++++++
 include/atf_common.h | 183 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/spl.h        |   1 +
 6 files changed, 299 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..c5a4bf1
--- /dev/null
+++ b/common/spl/spl_atf.c
@@ -0,0 +1,97 @@
+/*
+ * 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, ATF_PARAM_BL31, ATF_VERSION_1, 0);
+
+	/* Fill BL31 related information */
+	SET_PARAM_HEAD(bl2_to_bl31_params->bl31_image_info,
+		       ATF_PARAM_IMAGE_BINARY, ATF_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, ATF_PARAM_EP,
+		       ATF_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,
+		       ATF_PARAM_IMAGE_BINARY, ATF_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, ATF_PARAM_EP, ATF_VERSION_1,
+		       ATF_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,
+		       ATF_PARAM_IMAGE_BINARY, ATF_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..5185fef
--- /dev/null
+++ b/include/atf_common.h
@@ -0,0 +1,183 @@
+/*
+ * 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 ATF_PARAM_EP		0x01
+#define ATF_PARAM_IMAGE_BINARY	0x02
+#define ATF_PARAM_BL31		0x03
+
+#define ATF_VERSION_1	0x01
+
+#define ATF_EP_SECURE	0x0
+#define ATF_EP_NON_SECURE	0x1
+
+#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)
+
+#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 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)
+
+#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)
+
+#define DAIF_FIQ_BIT (1<<0)
+#define DAIF_IRQ_BIT (1<<1)
+#define DAIF_ABT_BIT (1<<2)
+#define DAIF_DBG_BIT (1<<3)
+#define DISABLE_ALL_EXECPTIONS	\
+	(DAIF_FIQ_BIT | DAIF_IRQ_BIT | DAIF_ABT_BIT | DAIF_DBG_BIT)
+
+#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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH v3] spl: add support to booting with ATF
  2017-04-05  5:05 ` [U-Boot] [PATCH v3] spl: add support to booting with ATF Kever Yang
@ 2017-04-05  5:35   ` Masahiro Yamada
  2017-04-05  8:25     ` Dan Handley
  2017-04-05 16:40   ` Masahiro Yamada
  1 sibling, 1 reply; 7+ messages in thread
From: Masahiro Yamada @ 2017-04-05  5:35 UTC (permalink / raw)
  To: u-boot

2017-04-05 14:05 GMT+09:00 Kever Yang <kever.yang@rock-chips.com>:
> 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


SCP-BL30 ?


Dan,

I see BL30 in old documents, but this was deprecated, right?


bl2/bl2_image_load.c implies that:

/*
 * Check for platforms that use obsolete image terminology
 */
#ifdef BL30_BASE
# error "BL30_BASE platform define no longer used - please use SCP_BL2_BASE"
#endif



What is the correct terminology for the SCP firmware
on the latest code base?

SCP_BL2 ?



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH v3] spl: add support to booting with ATF
  2017-04-05  5:35   ` Masahiro Yamada
@ 2017-04-05  8:25     ` Dan Handley
  2017-05-05  3:07       ` Kever Yang
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Handley @ 2017-04-05  8:25 UTC (permalink / raw)
  To: u-boot

Hi

> -----Original Message-----
> From: Masahiro Yamada [mailto:yamada.masahiro at socionext.com]
> Sent: 05 April 2017 06:36
>
> 2017-04-05 14:05 GMT+09:00 Kever Yang <kever.yang@rock-chips.com>:
> > 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
>
>
> SCP-BL30 ?
>
>
> Dan,
>
> I see BL30 in old documents, but this was deprecated, right?
>
Yes, the BL30 terminology was deprecated in favour of SCP_BL2. See:
https://github.com/ARM-software/arm-trusted-firmware/wiki/ARM-Trusted-Firmware-Image-Terminology#scp-ram-firmware-scp_bl2-previously-bl3-0

However, given this is an optional image, it might be worth removing the reference altogether.

Also, I notice this latest patch still uses bl31_params_t rather than the more recent bl_params_t.

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH v3] spl: add support to booting with ATF
  2017-04-05  5:05 ` [U-Boot] [PATCH v3] spl: add support to booting with ATF Kever Yang
  2017-04-05  5:35   ` Masahiro Yamada
@ 2017-04-05 16:40   ` Masahiro Yamada
  2017-05-05  3:33     ` Kever Yang
  1 sibling, 1 reply; 7+ messages in thread
From: Masahiro Yamada @ 2017-04-05 16:40 UTC (permalink / raw)
  To: u-boot

Hi.


2017-04-05 14:05 GMT+09:00 Kever Yang <kever.yang@rock-chips.com>:

> --- 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

This statement may not be precise.
ATF supports Aarch32 as well as Aarch64.


I thik this is just U-Boot side matter.
As far as I understood from the U-Boot directory structure
(I only see Aarch64 code in arch/arm/cpu/armv8),
U-Boot does not expect 32bit ARM code in ARMv8 generation
(such as Cortex-A32, Coretex-A35, ...).
As for U-Boot, ARM64 == ARMV8
unless I am missing something.



> +config SPL_ATF_TEXT_BASE
> +       depends on SPL_ATF_SUPPORT
> +       hex "ATF TEXT BASE addr"

hex "ATF BL31 base address"



> +       help
> +         This is the base address in memory for ATF text and entry point.

I'd like you to mention "BL31" explicitly
because there are many images in ATF.



>  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);




        switch (spl_image.os) {
        case IH_OS_U_BOOT:
                debug("Jumping to U-Boot\n");
                break;
#ifdef CONFIG_SPL_OS_BOOT
        case IH_OS_LINUX:
                debug("Jumping to Linux\n");
                spl_board_prepare_for_linux();
                jump_to_image_linux(&spl_image,
                                    (void *)CONFIG_SYS_SPL_ARGS_ADDR);
#endif
        default:
                debug("Unsupported OS image.. Jumping nevertheless..\n");
        }


Which debug message is printed
before SPL jumps to BL31?

Jumping to U-Boot or Linux or Unsupported OS?








> +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);
> +}

I guess SPL is supposed to load at least BL31 and BL33(=U-Boot full)
because BL31 does not have a facility to load images from non-volatile devices.

I can not see how to do it from this patch.

Relying on Andre's work for multi images in FIT?
http://patchwork.ozlabs.org/patch/745827/


I am not tracking how they are related, though.




> +/*******************************************************************************
> + * 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 {


Please do not add _t suffix for structure.


> +       unsigned long arg0;
> +       unsigned long arg1;
> +       unsigned long arg2;
> +       unsigned long arg3;
> +       unsigned long arg4;
> +       unsigned long arg5;
> +       unsigned long arg6;
> +       unsigned long arg7;
> +};



In ATF, every structure has a shorthand
typedef with _t suffix, like this:

typedef struct aapcs64_params {
        u_register_t arg0;
        u_register_t arg1;
        u_register_t arg2;
        u_register_t arg3;
        u_register_t arg4;
        u_register_t arg5;
        u_register_t arg6;
        u_register_t arg7;
} aapcs64_params_t;


This is bad habit anyway, so
no new typedef:s, no _t suffix in U-boot.





> +/***************************************************************************
> + * This structure provides version information and the size of the
> + * structure, attributes for the structure it represents
> + ***************************************************************************/
> +struct param_header_t {

Please rename to
struct param_header

Likewise for the others.



> +/*******************************************************************************
> + * 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;
> +};
> +

This is old interface. (LOAD_IMAGE_V1).


As Dan explained, this was superseded by struct bl_params and bl_params_node.


I'd recommend you to implement LOAD_IMAGE_V2.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH v3] spl: add support to booting with ATF
  2017-04-05  8:25     ` Dan Handley
@ 2017-05-05  3:07       ` Kever Yang
  0 siblings, 0 replies; 7+ messages in thread
From: Kever Yang @ 2017-05-05  3:07 UTC (permalink / raw)
  To: u-boot

Hi


On 04/05/2017 04:25 PM, Dan Handley wrote:
> Hi
>
>> -----Original Message-----
>> From: Masahiro Yamada [mailto:yamada.masahiro at socionext.com]
>> Sent: 05 April 2017 06:36
>>
>> 2017-04-05 14:05 GMT+09:00 Kever Yang <kever.yang@rock-chips.com>:
>>> 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
>>
>> SCP-BL30 ?
>>
>>
>> Dan,
>>
>> I see BL30 in old documents, but this was deprecated, right?
>>
> Yes, the BL30 terminology was deprecated in favour of SCP_BL2. See:
> https://github.com/ARM-software/arm-trusted-firmware/wiki/ARM-Trusted-Firmware-Image-Terminology#scp-ram-firmware-scp_bl2-previously-bl3-0

I get this concept from old document, I will remove it.
>
> However, given this is an optional image, it might be worth removing the reference altogether.
>
> Also, I notice this latest patch still uses bl31_params_t rather than the more recent bl_params_t.

I understand you prefer using IMAGE_V2 than IMAGE_V1, but I would like 
to support
IMAGE_V1 first, because for Rockchip rk3399, there is already binary 
release if bl31 which
is in IMAGE_V1 format like 
https://github.com/rockchip-linux/rkbin/tree/master/rk33.

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [U-Boot] [PATCH v3] spl: add support to booting with ATF
  2017-04-05 16:40   ` Masahiro Yamada
@ 2017-05-05  3:33     ` Kever Yang
  0 siblings, 0 replies; 7+ messages in thread
From: Kever Yang @ 2017-05-05  3:33 UTC (permalink / raw)
  To: u-boot

Hi


On 04/06/2017 12:40 AM, Masahiro Yamada wrote:
> Hi.
>
>
> 2017-04-05 14:05 GMT+09:00 Kever Yang <kever.yang@rock-chips.com>:
>
>> --- 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
> This statement may not be precise.
> ATF supports Aarch32 as well as Aarch64.
>
>
> I thik this is just U-Boot side matter.
> As far as I understood from the U-Boot directory structure
> (I only see Aarch64 code in arch/arm/cpu/armv8),
> U-Boot does not expect 32bit ARM code in ARMv8 generation
> (such as Cortex-A32, Coretex-A35, ...).
> As for U-Boot, ARM64 == ARMV8
> unless I am missing something.

I would like to leave it as is now, people can remove this if there is 
aarch32 want to
support ATF, at least I didn't see any of them now.
>
>
>> +config SPL_ATF_TEXT_BASE
>> +       depends on SPL_ATF_SUPPORT
>> +       hex "ATF TEXT BASE addr"
> hex "ATF BL31 base address"

Will fix, thanks.
>
>
>
>> +       help
>> +         This is the base address in memory for ATF text and entry point.
> I'd like you to mention "BL31" explicitly
> because there are many images in ATF.

Will fix, thanks.
>
>
>
>>   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);
>
>
>
>          switch (spl_image.os) {
>          case IH_OS_U_BOOT:
>                  debug("Jumping to U-Boot\n");
>                  break;
> #ifdef CONFIG_SPL_OS_BOOT
>          case IH_OS_LINUX:
>                  debug("Jumping to Linux\n");
>                  spl_board_prepare_for_linux();
>                  jump_to_image_linux(&spl_image,
>                                      (void *)CONFIG_SYS_SPL_ARGS_ADDR);
> #endif
>          default:
>                  debug("Unsupported OS image.. Jumping nevertheless..\n");
>          }
>
>
> Which debug message is printed
> before SPL jumps to BL31?
>
> Jumping to U-Boot or Linux or Unsupported OS?

It's Jumping to U-Boot now, how about add one message before jump to 
BL31 like this:
debug("loaded - jumping to U-Boot via ATF BL31.\n");
>
>
>
>
>
>
>
>> +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);
>> +}
> I guess SPL is supposed to load at least BL31 and BL33(=U-Boot full)
> because BL31 does not have a facility to load images from non-volatile devices.
>
> I can not see how to do it from this patch.
>
> Relying on Andre's work for multi images in FIT?
> http://patchwork.ozlabs.org/patch/745827/

Yes, I use Andre's patch to load multi image in FIT, this can handle 
flexible number of
ATF target.

>
>
> I am not tracking how they are related, though.
>
>
>
>
>> +/*******************************************************************************
>> + * 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 {
>
> Please do not add _t suffix for structure.
>
>
>> +       unsigned long arg0;
>> +       unsigned long arg1;
>> +       unsigned long arg2;
>> +       unsigned long arg3;
>> +       unsigned long arg4;
>> +       unsigned long arg5;
>> +       unsigned long arg6;
>> +       unsigned long arg7;
>> +};
>
>
> In ATF, every structure has a shorthand
> typedef with _t suffix, like this:
>
> typedef struct aapcs64_params {
>          u_register_t arg0;
>          u_register_t arg1;
>          u_register_t arg2;
>          u_register_t arg3;
>          u_register_t arg4;
>          u_register_t arg5;
>          u_register_t arg6;
>          u_register_t arg7;
> } aapcs64_params_t;
>
>
> This is bad habit anyway, so
> no new typedef:s, no _t suffix in U-boot.
>
>
>
>
>
>> +/***************************************************************************
>> + * This structure provides version information and the size of the
>> + * structure, attributes for the structure it represents
>> + ***************************************************************************/
>> +struct param_header_t {
> Please rename to
> struct param_header
>
> Likewise for the others.

OK, will do it.
>
>
>
>> +/*******************************************************************************
>> + * 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;
>> +};
>> +
> This is old interface. (LOAD_IMAGE_V1).
>
>
> As Dan explained, this was superseded by struct bl_params and bl_params_node.
>
>
> I'd recommend you to implement LOAD_IMAGE_V2.

Yes, there is new IMAGE_V2 available now, but I would like to support V1 
first,
because there already have bl31 binary release from Rockchip which is V1 
format.

Thanks,
- Kever
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-05-05  3:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-05  5:05 [U-Boot] [PATCH v3 0/1] arm64: enable SPL with ATF support Kever Yang
2017-04-05  5:05 ` [U-Boot] [PATCH v3] spl: add support to booting with ATF Kever Yang
2017-04-05  5:35   ` Masahiro Yamada
2017-04-05  8:25     ` Dan Handley
2017-05-05  3:07       ` Kever Yang
2017-04-05 16:40   ` Masahiro Yamada
2017-05-05  3:33     ` Kever Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.