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

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Michael Walle-2
Newer TF-A versions provide a new image loading protocol. This is used on
(newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 ->
u-boot. With this series it is possible that U-Boot SPL loads the bl31
directly and thus replacing bl1 and bl2 from the TF-A.

This was tested on the Kontron sl28 board using NXPs bl31 and the upstream
version of the OP-TEE Trusted OS.

Changes since v1:
 - removed firmware entry from loadable, suggested by Michal
 - use kernel-doc function annotation format
 - new patch "board: sl28: remove u-boot from loadable DT node"

Michael Walle (9):
  treewide: use CONFIG_IS_ENABLED() for ARMV8_SEC_FIRMWARE_SUPPORT
  spl: atf: move storage for bl31_params into function
  spl: atf: provide a bl2_plat_get_bl31_params_default()
  spl: atf: remove helper structure from common header
  spl: atf: add support for LOAD_IMAGE_V2
  armv8: layerscape: don't initialize GIC in SPL
  board: sl28: remove u-boot from loadable DT node
  board: sl28: add ATF support (bl31)
  board: sl28: add OP-TEE Trusted OS support (bl32)

 arch/arm/cpu/armv8/cpu-dt.c                   |   2 +-
 arch/arm/cpu/armv8/fsl-layerscape/fdt.c       |   8 +-
 arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S  |   2 +
 arch/arm/cpu/armv8/fsl-layerscape/ppa.c       |   2 +-
 .../dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi  |  80 ++++++++++-
 arch/arm/lib/bootm-fdt.c                      |   2 +-
 arch/arm/lib/psci-dt.c                        |   6 +-
 board/kontron/sl28/Kconfig                    |  33 +++++
 board/kontron/sl28/Makefile                   |   6 +-
 board/kontron/sl28/sl28.c                     |   7 +
 board/kontron/sl28/spl_atf.c                  |  54 ++++++++
 common/spl/Kconfig                            |   9 ++
 common/spl/spl_atf.c                          | 129 ++++++++++++++++--
 include/atf_common.h                          |  42 ++++--
 include/spl.h                                 |  78 +++++++++--
 15 files changed, 410 insertions(+), 50 deletions(-)
 create mode 100644 board/kontron/sl28/spl_atf.c

--
2.20.1

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 1/9] treewide: use CONFIG_IS_ENABLED() for ARMV8_SEC_FIRMWARE_SUPPORT

Michael Walle-2
There is SPL_ARMV8_SEC_FIRMWARE_SUPPORT and ARMV8_SEC_FIRMWARE_SUPPORT.
Thus use CONFIG_IS_ENABLED() instead of the simple #ifdef.

Signed-off-by: Michael Walle <[hidden email]>
Acked-by: Michal Simek <[hidden email]>
---
 arch/arm/cpu/armv8/cpu-dt.c             | 2 +-
 arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 8 ++++----
 arch/arm/cpu/armv8/fsl-layerscape/ppa.c | 2 +-
 arch/arm/lib/bootm-fdt.c                | 2 +-
 arch/arm/lib/psci-dt.c                  | 6 +++---
 5 files changed, 10 insertions(+), 10 deletions(-)

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

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 2/9] spl: atf: move storage for bl31_params into function

Michael Walle-2
In reply to this post by Michael Walle-2
There is no need to have the storage available globally. This is also a
preparation for LOAD_IMAGE_V2 support. That will introduce a similar
generator function which also has its own storage.

Signed-off-by: Michael Walle <[hidden email]>
Acked-by: Michal Simek <[hidden email]>
---
 common/spl/spl_atf.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
index 9bd25f6b32..df0a198d55 100644
--- a/common/spl/spl_atf.c
+++ b/common/spl/spl_atf.c
@@ -18,13 +18,12 @@
 #include <spl.h>
 #include <asm/cache.h>
 
-static struct bl2_to_bl31_params_mem bl31_params_mem;
-static struct bl31_params *bl2_to_bl31_params;
-
 __weak struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry,
     uintptr_t bl33_entry,
     uintptr_t fdt_addr)
 {
+ static struct bl2_to_bl31_params_mem bl31_params_mem;
+ struct bl31_params *bl2_to_bl31_params;
  struct entry_point_info *bl32_ep_info;
  struct entry_point_info *bl33_ep_info;
 
--
2.20.1

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 3/9] spl: atf: provide a bl2_plat_get_bl31_params_default()

Michael Walle-2
In reply to this post by Michael Walle-2
Move the actual implementation of the bl2_plat_get_bl31_params() to its
own function. The weak function will just call the default
implementation. This has the advantage that board code can still call
the original implementation if it just want to modify minor things.

Signed-off-by: Michael Walle <[hidden email]>
---
 common/spl/spl_atf.c | 14 +++++++++++---
 include/spl.h        | 43 +++++++++++++++++++++++++++++++------------
 2 files changed, 42 insertions(+), 15 deletions(-)

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

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 4/9] spl: atf: remove helper structure from common header

Michael Walle-2
In reply to this post by Michael Walle-2
bl2_to_bl31_params_mem is just an implementation detail of the SPL ATF
support and is not needed anywhere else. Move it from the header to the
actual module.

Signed-off-by: Michael Walle <[hidden email]>
Acked-by: Michal Simek <[hidden email]>
---
 common/spl/spl_atf.c | 11 +++++++++++
 include/atf_common.h | 14 --------------
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
index 63af6a6207..51b45d5dc6 100644
--- a/common/spl/spl_atf.c
+++ b/common/spl/spl_atf.c
@@ -18,6 +18,17 @@
 #include <spl.h>
 #include <asm/cache.h>
 
+/* Holds all the structures we need for bl31 parameter passing */
+struct bl2_to_bl31_params_mem {
+ struct bl31_params bl31_params;
+ struct atf_image_info bl31_image_info;
+ struct atf_image_info bl32_image_info;
+ struct atf_image_info bl33_image_info;
+ struct entry_point_info bl33_ep_info;
+ struct entry_point_info bl32_ep_info;
+ struct entry_point_info bl31_ep_info;
+};
+
 struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry,
      uintptr_t bl33_entry,
      uintptr_t fdt_addr)
diff --git a/include/atf_common.h b/include/atf_common.h
index fd5454c55b..e173a10ca9 100644
--- a/include/atf_common.h
+++ b/include/atf_common.h
@@ -162,20 +162,6 @@ struct bl31_params {
  struct atf_image_info *bl33_image_info;
 };
 
-/*******************************************************************************
- * This structure represents the superset of information that is passed to
- * BL31, e.g. while passing control to it from BL2, bl31_params
- * and other platform specific params
- ******************************************************************************/
-struct bl2_to_bl31_params_mem {
- struct bl31_params bl31_params;
- struct atf_image_info bl31_image_info;
- struct atf_image_info bl32_image_info;
- struct atf_image_info bl33_image_info;
- struct entry_point_info bl33_ep_info;
- struct entry_point_info bl32_ep_info;
- struct entry_point_info bl31_ep_info;
-};
 
 #endif /*__ASSEMBLY__ */
 
--
2.20.1

Reply | Threaded
Open this post in threaded view
|

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

Michael Walle-2
In reply to this post by Michael Walle-2
Newer platforms use the LOAD_IMAGE_V2 parameter passing method. Add
support for it.

Signed-off-by: Michael Walle <[hidden email]>
---
 common/spl/Kconfig   |  9 ++++
 common/spl/spl_atf.c | 99 ++++++++++++++++++++++++++++++++++++++++++--
 include/atf_common.h | 30 ++++++++++++++
 include/spl.h        | 35 ++++++++++++++++
 4 files changed, 169 insertions(+), 4 deletions(-)

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

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 6/9] armv8: layerscape: don't initialize GIC in SPL

Michael Walle-2
In reply to this post by Michael Walle-2
The BL31 expects the GIC to be uninitialized. Thus, if we are loading
the BL31 by the SPL we must not initialize it. If u-boot is loaded by
the SPL directly, it will initialize the GIC again (in the same
lowlevel_init()).

This was tested on a custom board with SPL loading the BL31 and jumping
to u-boot as BL33 as well as loading u-boot directly by the SPL. In case
the ATF BL1/BL2 is used, this patch won't change anything, because no
SPL is used at all.

Signed-off-by: Michael Walle <[hidden email]>
---
 arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S
index a519f6ed67..d8803738f1 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S
+++ b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S
@@ -192,6 +192,7 @@ ENTRY(lowlevel_init)
 #endif
 
  /* Initialize GIC Secure Bank Status */
+#if !defined(CONFIG_SPL_BUILD)
 #if defined(CONFIG_GICV2) || defined(CONFIG_GICV3)
  branch_if_slave x0, 1f
  bl get_gic_offset
@@ -205,6 +206,7 @@ ENTRY(lowlevel_init)
  bl gic_init_secure_percpu
 #endif
 #endif
+#endif
 
 100:
  branch_if_master x0, x1, 2f
--
2.20.1

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 7/9] board: sl28: remove u-boot from loadable DT node

Michael Walle-2
In reply to this post by Michael Walle-2
It is not needed. Remove it.

Signed-off-by: Michael Walle <[hidden email]>
---
 arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi b/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi
index 2375549c6e..87e14d6ae6 100644
--- a/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi
@@ -80,21 +80,18 @@
  conf-1 {
  description = "fsl-ls1028a-kontron-sl28";
  firmware = "uboot";
- loadables = "uboot";
  fdt = "fdt-1";
  };
 
  conf-2 {
  description = "fsl-ls1028a-kontron-sl28-var3";
  firmware = "uboot";
- loadables = "uboot";
  fdt = "fdt-2";
  };
 
  conf-3 {
  description = "fsl-ls1028a-kontron-sl28-var4";
  firmware = "uboot";
- loadables = "uboot";
  fdt = "fdt-3";
  };
  };
--
2.20.1

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 8/9] board: sl28: add ATF support (bl31)

Michael Walle-2
In reply to this post by Michael Walle-2
Add support to load the bl31 part of the ARM Trusted Firmware by the
SPL.

Signed-off-by: Michael Walle <[hidden email]>
---
 .../dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi  | 41 +++++++++++++-
 board/kontron/sl28/Kconfig                    | 10 ++++
 board/kontron/sl28/Makefile                   |  6 ++-
 board/kontron/sl28/spl_atf.c                  | 54 +++++++++++++++++++
 4 files changed, 109 insertions(+), 2 deletions(-)
 create mode 100644 board/kontron/sl28/spl_atf.c

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

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 9/9] board: sl28: add OP-TEE Trusted OS support (bl32)

Michael Walle-2
In reply to this post by Michael Walle-2
Add support to load the OP-TEE Trusted OS by the SPL.

Signed-off-by: Michael Walle <[hidden email]>
---
 .../dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi  | 36 +++++++++++++++++++
 board/kontron/sl28/Kconfig                    | 23 ++++++++++++
 board/kontron/sl28/sl28.c                     |  7 ++++
 3 files changed, 66 insertions(+)

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

Reply | Threaded
Open this post in threaded view
|

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

Michael Walle-2
In reply to this post by Michael Walle-2
Am 2020-11-20 11:14, schrieb Michal Simek:

> Hi,
>
> On 18. 11. 20 17:45, Michael Walle wrote:
>> Newer TF-A versions provide a new image loading protocol. This is used
>> on
>> (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 ->
>> u-boot. With this series it is possible that U-Boot SPL loads the bl31
>> directly and thus replacing bl1 and bl2 from the TF-A.
>>
>> This was tested on the Kontron sl28 board using NXPs bl31 and the
>> upstream
>> version of the OP-TEE Trusted OS.
>
> I still have some questions about this.
>
> As I see from TFA previous image format has been removed in 2018 by
>
> commit ed51b51f7a9163a7fc48289c5ed97a3fe4fe504f
> Author:     Roberto Vargas <[hidden email]>
> AuthorDate: Mon Sep 24 17:20:48 2018 +0100
> Commit:     Antonio Nino Diaz <[hidden email]>
> CommitDate: Fri Sep 28 15:31:52 2018 +0100
>
>     Remove build option LOAD_IMAGE_V2
>
>     The code of LOAD_IMAGE_V2=0 has been removed.
>
>     Change-Id: Iea03e5bebb90c66889bdb23f85c07d0c9717fffe
>     Co-authored-by: Antonio Nino Diaz <[hidden email]>
>     Signed-off-by: Antonio Nino Diaz <[hidden email]>
>
>

DOH! Lol, I'm using just one non-upstream part for the whole board
and of course it is doing something miserable. I wasn't aware of this.

> On Xilinx ZynqMP I use SPL->bl31 loading but not using that TFA
> structure because xilinx is using own format.
>
> But I am curious if V2 was removed in 2018 who is really using previous
> one and also if current implemenation is origin or also not full v2.

NXP,
https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/nxp/common/common.mk#n55

The last non-nxp commit there was from around March 2018..

> And these patches are not breaking boot on zynqmp that's why not big
> deal for me.

I was looking at porting TFA to upstream for this board but there is
such a huge gap. Therefore, it seemed to be easier to just use the
vendor version for now.

-michael
Reply | Threaded
Open this post in threaded view
|

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

Michael Walle-2
Am 2020-11-20 12:15, schrieb Michal Simek:

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

What do you mean by blX code? Nobody is using this flow for now, i.e.
   spl -> bl31 -> u-boot
For the LOAD_IMAGE_V2 case.

NXP is using bl1 -> bl2 -> bl3 -> u-boot.

> As I said this series is not breaking our flow on xilinx zynqmp soc but
> maybe your code is more or less duplication of what's there now.
> Or maybe if there is any NXP private way it should be handled
> differently as I do for zynqmp.

The code is split into the generic v2 handling and layerscape LS1028A
specific stuff, hence the _default() function which is used in the
board files and then arch specific parameters are applied.
Also I don't know wether unifying the
   bl2_plat_get_bl31_params_default() and
   bl2_plat_get_bl31_params_v2_default()
is worth it. If that is what you mean by "more or less a duplication".

-michael
Reply | Threaded
Open this post in threaded view
|

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

Michael Walle-2
In reply to this post by Michael Walle-2
Am 2020-11-20 11:48, schrieb Michael Walle:

> Am 2020-11-20 11:14, schrieb Michal Simek:
>> Hi,
>>
>> On 18. 11. 20 17:45, Michael Walle wrote:
>>> Newer TF-A versions provide a new image loading protocol. This is
>>> used on
>>> (newer?) NXP's SoCs. Normally, the bootflow is bl1 -> bl2 -> bl31 ->
>>> u-boot. With this series it is possible that U-Boot SPL loads the
>>> bl31
>>> directly and thus replacing bl1 and bl2 from the TF-A.
>>>
>>> This was tested on the Kontron sl28 board using NXPs bl31 and the
>>> upstream
>>> version of the OP-TEE Trusted OS.
>>
>> I still have some questions about this.
>>
>> As I see from TFA previous image format has been removed in 2018 by
>>
>> commit ed51b51f7a9163a7fc48289c5ed97a3fe4fe504f
>> Author:     Roberto Vargas <[hidden email]>
>> AuthorDate: Mon Sep 24 17:20:48 2018 +0100
>> Commit:     Antonio Nino Diaz <[hidden email]>
>> CommitDate: Fri Sep 28 15:31:52 2018 +0100
>>
>>     Remove build option LOAD_IMAGE_V2
>>
>>     The code of LOAD_IMAGE_V2=0 has been removed.
>>
>>     Change-Id: Iea03e5bebb90c66889bdb23f85c07d0c9717fffe
>>     Co-authored-by: Antonio Nino Diaz <[hidden email]>
>>     Signed-off-by: Antonio Nino Diaz <[hidden email]>
>>
>>
>
> DOH! Lol, I'm using just one non-upstream part for the whole board
> and of course it is doing something miserable. I wasn't aware of this.

Ah, this commit wasn't about removing LOAD_IMAGE_V2 but making it the
only
supported one. Thus it basically removes the old "LOAD_IMAGE_V2=0". That
is
the former boot protocol which is the only one u-boot supports until
this
patch series.

It seems that zynqmp doesn't check for the v2 header version. This is
the common function:
https://elixir.bootlin.com/arm-trusted-firmware/v2.4/source/common/desc_image_load.c#L341

-michael
Reply | Threaded
Open this post in threaded view
|

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

Michael Walle-2
In reply to this post by Michael Walle-2
Am 2020-11-20 14:16, schrieb Michal Simek:

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

NXP imx8 also uses u-boot -> bl31 as far as I know.

> Definitely check with them to ack your patches.

I've put Kever (who did the initial patches for rockchip) on CC for
this.
But again they are using the old method. So lets assume they will ignore
this series (for whatever reason), won't it be accepted then? I mean
this series doesn't touch the old behavior, just adding a new one.

-michael
Reply | Threaded
Open this post in threaded view
|

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

Michael Walle-2
Am 2020-11-20 14:35, schrieb Michal Simek:
> On 20. 11. 20 14:25, Michael Walle wrote:
>> Am 2020-11-20 14:16, schrieb Michal Simek:
>>> On 20. 11. 20 12:27, Michael Walle wrote:
>>>> Am 2020-11-20 12:15, schrieb Michal Simek:
[..]

>>>>> Please get this reviewed by people who are using current blX code.
>>>>
>>>> What do you mean by blX code? Nobody is using this flow for now,
>>>> i.e.
>>>>   spl -> bl31 -> u-boot
>>>
>>> As I said I am not quite sure about it. I see that rockchip guys are
>>> using ATF and they also call that code in ATF. They should know which
>>> version the use.
>>
>> NXP imx8 also uses u-boot -> bl31 as far as I know.
>>
>>> Definitely check with them to ack your patches.
>>
>> I've put Kever (who did the initial patches for rockchip) on CC for
>> this.
>> But again they are using the old method. So lets assume they will
>> ignore
>> this series (for whatever reason), won't it be accepted then? I mean
>> this series doesn't touch the old behavior, just adding a new one.
>
> up to Priyanka or Tom.
> I have no problem with your patches because we are not using this
> method
> and your patches doesn't break our SPL boot flow.

I haven't said this: I really appreciated your review and I was
surprised
how fast it was. Thanks and maybe Kever will also have a look. We'll see
;)

-michael
Reply | Threaded
Open this post in threaded view
|

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

Tom Rini-4
In reply to this post by Michael Walle-2
On Fri, Nov 20, 2020 at 02:35:33PM +0100, Michal Simek wrote:

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

--
Tom

signature.asc (673 bytes) Download Attachment