[U-Boot] [PATCH 01/15] image: add IH_OS_ARM_TRUSTED_FIRMWARE for ARM Trusted Firmware

[U-Boot] [PATCH 01/15] image: add IH_OS_ARM_TRUSTED_FIRMWARE for ARM Trusted Firmware

[Philipp Tomsich]

To boot on ARMv8 systems with ARM Trusted Firmware, we need to
assemble an ATF-specific parameter structure and also provide the
address of the images started by ATF (e.g. BL3-3, which may be the
full U-Boot).

To allow us to identify an ARM Trusted Firmware contained in a FIT
image, this adds the necessary definitions.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 common/image.c  | 1 +
 include/image.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/common/image.c b/common/image.c
index 332b5f2..be91be2 100644
--- a/common/image.c
+++ b/common/image.c
@@ -99,6 +99,7 @@ static const table_entry_t uimage_arch[] = {
 
 static const table_entry_t uimage_os[] = {
 	{	IH_OS_INVALID,	"invalid",	"Invalid OS",		},
+	{       IH_OS_ARM_TRUSTED_FIRMWARE, "arm-trusted-firmware", "ARM Trusted Firmware"  },
 	{	IH_OS_LINUX,	"linux",	"Linux",		},
 #if defined(CONFIG_LYNXKDI) || defined(USE_HOSTCC)
 	{	IH_OS_LYNXOS,	"lynxos",	"LynxOS",		},
diff --git a/include/image.h b/include/image.h
index 339f79c..b35b65f 100644
--- a/include/image.h
+++ b/include/image.h
@@ -152,6 +152,7 @@ enum {
 	IH_OS_OSE,			/* OSE		*/
 	IH_OS_PLAN9,			/* Plan 9	*/
 	IH_OS_OPENRTOS,		/* OpenRTOS	*/
+	IH_OS_ARM_TRUSTED_FIRMWARE,     /* ARM Trusted Firmware */
 
 	IH_OS_COUNT,
 };
-- 
2.1.4

[U-Boot] [PATCH 02/15] spl: add a fdt_addr field to spl_image_info

[Philipp Tomsich]

When loading a full U-Boot with detached device-tree using the SPL FIT
backend, we should store the address of the FDT loaded as part of the
SPL image info: this allows us to fixup the FDT with additional info
we may want to propagate onward.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 include/spl.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/spl.h b/include/spl.h
index ce4cf0a..acbc47a 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -25,6 +25,9 @@ struct spl_image_info {
 	u8 os;
 	ulong load_addr;
 	ulong entry_point;
+#if CONFIG_IS_ENABLED(LOAD_FIT)
+	void *fdt_addr;
+#endif
 	u32 size;
 	u32 flags;
 	void *arg;
-- 
2.1.4

[U-Boot] [PATCH 03/15] spl: change load_addr and entry_point to uintptr_t

[Philipp Tomsich]

Mainly a stylistic change: convert the load_addr and entry_point
fields of struct spl_image_info to uintptr_t (from ulong).

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 include/spl.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/spl.h b/include/spl.h
index acbc47a..19e64e3 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -23,8 +23,8 @@
 struct spl_image_info {
 	const char *name;
 	u8 os;
-	ulong load_addr;
-	ulong entry_point;
+	uintptr_t load_addr;
+	uintptr_t entry_point;
 #if CONFIG_IS_ENABLED(LOAD_FIT)
 	void *fdt_addr;
 #endif
@@ -270,7 +270,10 @@ 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);
+/**
+ * spl_invoke_atf - boot using an ARM trusted firmware image
+ */
+void spl_invoke_atf(struct spl_image_info *spl_image);
 
 /**
  * board_return_to_bootrom - allow for boards to continue with the boot ROM
-- 
2.1.4

[U-Boot] [PATCH 04/15] spl: fit: simplify logic for FDT loading for non-OS boots

[Philipp Tomsich]

To better support bootin through an ATF or OPTEE, we need to
streamline some of the logic for when the FDT is appended to an image:
depending on the image type, we'd like to append the FDT not at all
(the case for the OS boot), to the 'firmware' image (if it is a
U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
OPTEE, or some other image-type and U-Boot is listed in the
loadabled).

To achieve this goal, we drop the os_boot flag and track the type of
image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 56 insertions(+), 30 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 49ccf1c..9f05e1e 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -215,6 +215,30 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
 	return 0;
 }
 
+static int spl_fit_append_fdt(struct spl_image_info *spl_image,
+			      struct spl_load_info *info, ulong sector,
+			      void *fit, int images, ulong base_offset)
+{
+	struct spl_image_info image_info;
+	int node, ret;
+
+	/* Figure out which device tree the board wants to use */
+	node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP, 0);
+	if (node < 0) {
+		debug("%s: cannot find FDT node\n", __func__);
+		return node;
+	}
+
+	/*
+	 * Read the device tree and place it after the image.
+	 * Align the destination address to ARCH_DMA_MINALIGN.
+	 */
+	image_info.load_addr = spl_image->load_addr + spl_image->size;
+	ret = spl_load_fit_image(info, sector, fit, base_offset, node,
+				 &image_info);
+	return ret;
+}
+
 int spl_load_simple_fit(struct spl_image_info *spl_image,
 			struct spl_load_info *info, ulong sector, void *fit)
 {
@@ -222,7 +246,6 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 	ulong size;
 	unsigned long count;
 	struct spl_image_info image_info;
-	bool boot_os = false;
 	int node = -1;
 	int images, ret;
 	int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
@@ -270,17 +293,18 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 		return -1;
 	}
 
+	/*
+	 * Find the U-Boot image using the following search order:
+	 *   - start at 'firmware' (e.g. an ARM Trusted Firmware)
+	 *   - fall back 'kernel' (e.g. a Falcon-mode OS boot
+	 *   - fall back to using the first 'loadables' entry
+	 */
+	if (node < 0)
+		node = spl_fit_get_image_node(fit, images, "firmware", 0);
 #ifdef CONFIG_SPL_OS_BOOT
-	/* Find OS image first */
-	node = spl_fit_get_image_node(fit, images, FIT_KERNEL_PROP, 0);
 	if (node < 0)
-		debug("No kernel image.\n");
-	else
-		boot_os = true;
+		node = spl_fit_get_image_node(fit, images, FIT_KERNEL_PROP, 0);
 #endif
-	/* find the U-Boot image */
-	if (node < 0)
-		node = spl_fit_get_image_node(fit, images, "firmware", 0);
 	if (node < 0) {
 		debug("could not find firmware image, trying loadables...\n");
 		node = spl_fit_get_image_node(fit, images, "loadables", 0);
@@ -302,34 +326,29 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 	if (ret)
 		return ret;
 
-#ifdef CONFIG_SPL_OS_BOOT
+	/*
+	 * For backward compatibility, we treat the first node that is
+	 * as a U-Boot image, if no OS-type has been declared.
+	 */
 	if (!fit_image_get_os(fit, node, &spl_image->os))
 		debug("Image OS is %s\n", genimg_get_os_name(spl_image->os));
-#else
-	spl_image->os = IH_OS_U_BOOT;
+#if !defined(CONFIG_SPL_OS_BOOT)
+	else
+		spl_image->os = IH_OS_U_BOOT;
 #endif
 
-	if (!boot_os) {
-		/* Figure out which device tree the board wants to use */
-		node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP, 0);
-		if (node < 0) {
-			debug("%s: cannot find FDT node\n", __func__);
-			return node;
-		}
-
-		/*
-		 * Read the device tree and place it after the image.
-		 * Align the destination address to ARCH_DMA_MINALIGN.
-		 */
-		image_info.load_addr = spl_image->load_addr + spl_image->size;
-		ret = spl_load_fit_image(info, sector, fit, base_offset, node,
-					 &image_info);
-		if (ret < 0)
-			return ret;
-	}
+	/*
+	 * Booting a next-stage U-Boot may require us to append the FDT.
+	 * We allow this to fail, as the U-Boot image might embed its FDT.
+	 */
+	if (spl_image->os == IH_OS_U_BOOT)
+		spl_fit_append_fdt(spl_image, info, sector, fit,
+				   images, base_offset);
 
 	/* Now check if there are more images for us to load */
 	for (; ; index++) {
+		uint8_t os_type = IH_OS_INVALID;
+
 		node = spl_fit_get_image_node(fit, images, "loadables", index);
 		if (node < 0)
 			break;
@@ -339,6 +358,13 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 		if (ret < 0)
 			continue;
 
+		if (!fit_image_get_os(fit, node, &os_type))
+			debug("Loadable is %s\n", genimg_get_os_name(os_type));
+
+		if (spl_image->os == IH_OS_U_BOOT)
+			spl_fit_append_fdt(spl_image, info, sector,
+					   fit, images, base_offset);
+
 		/*
 		 * If the "firmware" image did not provide an entry point,
 		 * use the first valid entry point from the loadables.
-- 
2.1.4

[U-Boot] [PATCH 05/15] spl: fit: implement fdt_record_loadable

[Philipp Tomsich]

During the loading of more complex FIT images (e.g. when the invoked
next stage needs to find additional firmware for a power-management
core... or if there are multiple images for different privilege levels
started in parallel), it is helpful to create a record of what images
are loaded where: if a FDT is loaded for one of the next stages, it
can be used to convey the status and location of loadables.

This adds a fdt_record_loadable() function that can be invoked to
record the status of each loadable below the /fit-images path.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 common/fdt_support.c  | 39 +++++++++++++++++++++++++++++++++++++++
 include/fdt_support.h | 20 ++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 916a448..c936c33 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -410,6 +410,45 @@ static int fdt_pack_reg(const void *fdt, void *buf, u64 *address, u64 *size,
 	return p - (char *)buf;
 }
 
+int fdt_record_loadable(void *blob, u32 index, const char *name,
+			uintptr_t load_addr, u32 size, uintptr_t entry_point,
+			const char *type, const char *os)
+{
+	int err, node;
+
+	err = fdt_check_header(blob);
+	if (err < 0) {
+		printf("%s: %s\n", __func__, fdt_strerror(err));
+		return err;
+	}
+
+	/* find or create "/fit-images" node */
+	node = fdt_find_or_add_subnode(blob, 0, "fit-images");
+	if (node < 0)
+			return node;
+
+	/* find or create "/fit-images/<name>" node */
+	node = fdt_find_or_add_subnode(blob, node, name);
+	if (node < 0)
+		return node;
+
+	/*
+	 * We record these as 32bit entities, possibly truncating addresses.
+	 * However, spl_fit.c is not 64bit safe either: i.e. we should not
+	 * have an issue here.
+	 */
+	fdt_setprop_u32(blob, node, "load-addr", load_addr);
+	if (entry_point != -1)
+		fdt_setprop_u32(blob, node, "entry-point", entry_point);
+	fdt_setprop_u32(blob, node, "size", size);
+	if (type)
+		fdt_setprop_string(blob, node, "type", type);
+	if (os)
+		fdt_setprop_string(blob, node, "os", os);
+
+	return node;
+}
+
 #ifdef CONFIG_NR_DRAM_BANKS
 #define MEMORY_BANKS_MAX CONFIG_NR_DRAM_BANKS
 #else
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 5ef78cc..4297bde 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -133,6 +133,26 @@ void fdt_fixup_crypto_node(void *blob, int sec_rev);
 static inline void fdt_fixup_crypto_node(void *blob, int sec_rev) {}
 #endif
 
+#if CONFIG_IS_ENABLED(LOAD_FIT)
+/**
+ * Record information about a processed loadable in /fit-images (creating
+ * /fit-images if necessary).
+ *
+ * @param blob		FDT blob to update
+ * @param index	        index of this loadable
+ * @param name          name of the loadable
+ * @param load_addr     address the loadable was loaded to
+ * @param size          number of bytes loaded
+ * @param entry_point   entry point (if specified, otherwise pass -1)
+ * @param type          type (if specified, otherwise pass NULL)
+ * @param os            os-type (if specified, otherwise pass NULL)
+ * @return 0 if ok, or -1 or -FDT_ERR_... on error
+ */
+int fdt_record_loadable(void *blob, u32 index, const char *name,
+			uintptr_t load_addr, u32 size, uintptr_t entry_point,
+			const char *type, const char *os);
+#endif
+
 #ifdef CONFIG_PCI
 #include <pci.h>
 int fdt_pci_dma_ranges(void *blob, int phb_off, struct pci_controller *hose);
-- 
2.1.4

[U-Boot] [PATCH 06/15] spl: fit: implement recording of loadables into /fit-images

[Philipp Tomsich]

If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
address and record information for all loadables into this FDT.  This
allows us to easily keep track of images for multiple privilege levels
(e.g. with ATF) or of firmware images preloaded into temporary
locations (e.g. PMU firmware that may overlap the SPL stage).

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 81 insertions(+), 14 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 9f05e1e..6dc0969 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -2,7 +2,7 @@
  * Copyright (C) 2016 Google, Inc
  * Written by Simon Glass <sjg@chromium.org>
  *
- * SPDX-License-Identifier:     GPL-2.0+
+ * SPDX-License-Identifier:	GPL-2.0+
  */
 
 #include <common.h>
@@ -16,22 +16,24 @@
 #endif
 
 /**
- * spl_fit_get_image_node(): By using the matching configuration subnode,
+ * spl_fit_get_image_name(): By using the matching configuration subnode,
  * retrieve the name of an image, specified by a property name and an index
  * into that.
  * @fit:	Pointer to the FDT blob.
  * @images:	Offset of the /images subnode.
  * @type:	Name of the property within the configuration subnode.
  * @index:	Index into the list of strings in this property.
+ * @outname:	Name of the image
  *
- * Return:	the node offset of the respective image node or a negative
- * 		error number.
+ * Return:	0 on success, or a negative error number
  */
-static int spl_fit_get_image_node(const void *fit, int images,
-				  const char *type, int index)
+static int spl_fit_get_image_name(const void *fit, int images,
+				  const char *type, int index,
+				  char **outname)
 {
 	const char *name, *str;
-	int node, conf_node;
+	__maybe_unused int node;
+	int conf_node;
 	int len, i;
 
 	conf_node = fit_find_config_node(fit);
@@ -63,7 +65,35 @@ static int spl_fit_get_image_node(const void *fit, int images,
 		}
 	}
 
+	*outname = (char *)str;
+	return 0;
+}
+
+/**
+ * spl_fit_get_image_node(): By using the matching configuration subnode,
+ * retrieve the name of an image, specified by a property name and an index
+ * into that.
+ * @fit:	Pointer to the FDT blob.
+ * @images:	Offset of the /images subnode.
+ * @type:	Name of the property within the configuration subnode.
+ * @index:	Index into the list of strings in this property.
+ *
+ * Return:	the node offset of the respective image node or a negative
+ *		error number.
+ */
+static int spl_fit_get_image_node(const void *fit, int images,
+				  const char *type, int index)
+{
+	char *str;
+	int err;
+	int node;
+
+	err = spl_fit_get_image_name(fit, images, type, index, &str);
+	if (err)
+		return err;
+
 	debug("%s: '%s'\n", type, str);
+
 	node = fdt_subnode_offset(fit, images, str);
 	if (node < 0) {
 		debug("cannot find image node '%s': %d\n", str, node);
@@ -116,15 +146,15 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
  * @info:	points to information about the device to load data from
  * @sector:	the start sector of the FIT image on the device
  * @fit:	points to the flattened device tree blob describing the FIT
- * 		image
+ *		image
  * @base_offset: the beginning of the data area containing the actual
  *		image data, relative to the beginning of the FIT
  * @node:	offset of the DT node describing the image to load (relative
- * 		to @fit)
+ *		to @fit)
  * @image_info:	will be filled with information about the loaded image
- * 		If the FIT node does not contain a "load" (address) property,
- * 		the image gets loaded to the address pointed to by the
- * 		load_addr member in this struct.
+ *		If the FIT node does not contain a "load" (address) property,
+ *		the image gets loaded to the address pointed to by the
+ *		load_addr member in this struct.
  *
  * Return:	0 on success or a negative error number.
  */
@@ -236,6 +266,35 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,
 	image_info.load_addr = spl_image->load_addr + spl_image->size;
 	ret = spl_load_fit_image(info, sector, fit, base_offset, node,
 				 &image_info);
+
+	if (ret < 0)
+		return ret;
+
+	/* Make the load-address of the FDT available for the SPL framework */
+	spl_image->fdt_addr = (void *)image_info.load_addr;
+	/* Try to make space, so we can inject details on the loadables */
+	ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
+
+	return ret;
+}
+
+static int spl_fit_record_loadable(const void *fit, int images, int index,
+				   void *blob, struct spl_image_info *image)
+{
+	char *name;
+	int node, ret;
+
+	ret = spl_fit_get_image_name(fit, images, "loadables",
+				     index, &name);
+	if (ret < 0)
+		return ret;
+
+	node = spl_fit_get_image_node(fit, images, "loadables", index);
+
+	ret = fdt_record_loadable(blob, index, name, image->load_addr,
+				  image->size, image->entry_point,
+				  fdt_getprop(fit, node, "type", NULL),
+				  fdt_getprop(fit, node, "os", NULL));
 	return ret;
 }
 
@@ -361,9 +420,11 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 		if (!fit_image_get_os(fit, node, &os_type))
 			debug("Loadable is %s\n", genimg_get_os_name(os_type));
 
-		if (spl_image->os == IH_OS_U_BOOT)
-			spl_fit_append_fdt(spl_image, info, sector,
+		if (os_type == IH_OS_U_BOOT) {
+			spl_fit_append_fdt(&image_info, info, sector,
 					   fit, images, base_offset);
+			spl_image->fdt_addr = image_info.fdt_addr;
+		}
 
 		/*
 		 * If the "firmware" image did not provide an entry point,
@@ -372,6 +433,12 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
 		if (spl_image->entry_point == FDT_ERROR &&
 		    image_info.entry_point != FDT_ERROR)
 			spl_image->entry_point = image_info.entry_point;
+
+		/* Record our loadables into the FDT */
+		if (spl_image->fdt_addr)
+			spl_fit_record_loadable(fit, images, index,
+						spl_image->fdt_addr,
+						&image_info);
 	}
 
 	/*
-- 
2.1.4

[U-Boot] [PATCH 07/15] spl: atf: introduce spl_invoke_atf and make bl31_entry private

[Philipp Tomsich]

This adds a new interface spl_invoke_atf() that takes a spl_image_info
argument and then derives the necessary parameters for the ATF entry.
Based on the additional information recorded (into /fit-images) from
the FIT loadables, we can now easily locate the next boot stage.

We now pass a pointer to a FDT as the platform-specific parameter
pointer to ATF (so we don't run into the future headache of every
board/platform defining their own proprietary tag-structure), as
FDT access is already available in ATF.

With the necessary infrastructure in place, we can now update the
support for the ARM Trusted Firmware to dispatch into the
spl_invoke_atf function only if a IH_OS_ARM_TRUSTED_FIRMWARE image is
loaded.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 common/spl/spl.c     | 11 +++----
 common/spl/spl_atf.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 82 insertions(+), 13 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 8b219ba..32198ef 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -410,6 +410,12 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	case IH_OS_U_BOOT:
 		debug("Jumping to U-Boot\n");
 		break;
+#if CONFIG_IS_ENABLED(ATF)
+	case IH_OS_ARM_TRUSTED_FIRMWARE:
+		debug("Jumping to U-Boot via ARM Trusted Firmware\n");
+		spl_invoke_atf(&spl_image);
+		break;
+#endif
 #ifdef CONFIG_SPL_OS_BOOT
 	case IH_OS_LINUX:
 		debug("Jumping to Linux\n");
@@ -425,11 +431,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	      gd->malloc_ptr / 1024);
 #endif
 
-	if (CONFIG_IS_ENABLED(ATF_SUPPORT)) {
-		debug("loaded - jumping to U-Boot via ATF BL31.\n");
-		bl31_entry();
-	}
-
 	debug("loaded - jumping to U-Boot...\n");
 #ifdef CONFIG_BOOTSTAGE_STASH
 	int ret;
diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
index 6e8f928..63557c0 100644
--- a/common/spl/spl_atf.c
+++ b/common/spl/spl_atf.c
@@ -5,6 +5,7 @@
  * reserved.
  * Copyright (C) 2016 Rockchip Electronic Co.,Ltd
  * Written by Kever Yang <kever.yang@rock-chips.com>
+ * Copyright (C) 2017 Theobroma Systems Design und Consulting GmbH
  *
  * SPDX-License-Identifier:     BSD-3-Clause
  */
@@ -30,7 +31,7 @@ static struct bl31_params *bl2_to_bl31_params;
  *
  * @return bl31 params structure pointer
  */
-struct bl31_params *bl2_plat_get_bl31_params(void)
+static struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl33_entry)
 {
 	struct entry_point_info *bl33_ep_info;
 
@@ -66,7 +67,7 @@ struct bl31_params *bl2_plat_get_bl31_params(void)
 
 	/* 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->pc = bl33_entry;
 	bl33_ep_info->spsr = SPSR_64(MODE_EL2, MODE_SP_ELX,
 				     DISABLE_ALL_EXECPTIONS);
 
@@ -77,21 +78,88 @@ struct bl31_params *bl2_plat_get_bl31_params(void)
 	return bl2_to_bl31_params;
 }
 
-void raw_write_daif(unsigned int daif)
+static inline void raw_write_daif(unsigned int daif)
 {
 	__asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory");
 }
 
-void bl31_entry(void)
+typedef void (*atf_entry_t)(struct bl31_params *params, void *plat_params);
+
+static void bl31_entry(uintptr_t bl31_entry, uintptr_t bl33_entry,
+		       uintptr_t fdt_addr)
 {
 	struct bl31_params *bl31_params;
-	void (*entry)(struct bl31_params *params, void *plat_params) = NULL;
+	atf_entry_t  atf_entry = (atf_entry_t)bl31_entry;
 
-	bl31_params = bl2_plat_get_bl31_params();
-	entry = (void *)CONFIG_SPL_ATF_TEXT_BASE;
+	bl31_params = bl2_plat_get_bl31_params(bl33_entry);
 
 	raw_write_daif(SPSR_EXCEPTION_MASK);
 	dcache_disable();
 
-	entry(bl31_params, NULL);
+	atf_entry((void *)bl31_params, (void *)fdt_addr);
+}
+
+static int spl_fit_images_find_uboot(void *blob)
+{
+	int parent, node, ndepth;
+	const void *data;
+
+	if (!blob)
+		return -FDT_ERR_BADMAGIC;
+
+	parent = fdt_path_offset(blob, "/fit-images");
+	if (parent < 0)
+		return -FDT_ERR_NOTFOUND;
+
+	for (node = fdt_next_node(blob, parent, &ndepth);
+	     (node >= 0) && (ndepth > 0);
+	     node = fdt_next_node(blob, node, &ndepth)) {
+		if (ndepth != 1)
+			continue;
+
+		data = fdt_getprop(blob, node, FIT_OS_PROP, NULL);
+		if (!data)
+			continue;
+
+		if (genimg_get_os_id(data) == IH_OS_U_BOOT)
+			return node;
+	};
+
+	return -FDT_ERR_NOTFOUND;
+}
+
+uintptr_t spl_fit_images_get_entry(void *blob, int node)
+{
+	ulong  val;
+
+	val = fdt_getprop_u32(blob, node, "entry-point");
+	if (val == FDT_ERROR)
+		val = fdt_getprop_u32(blob, node, "load-addr");
+
+	debug("%s: entry point 0x%lx\n", __func__, val);
+	return val;
+}
+
+void spl_invoke_atf(struct spl_image_info *spl_image)
+{
+	uintptr_t  bl33_entry = CONFIG_SYS_TEXT_BASE;
+	void *blob = spl_image->fdt_addr;
+	int node;
+
+	/*
+	 * Find the U-Boot binary (in /fit-images) load addreess or
+	 * entry point (if different) and pass it as the BL3-3 entry
+	 * point.
+	 * This will need to be extended to support Falcon mode.
+	 */
+
+	node = spl_fit_images_find_uboot(blob);
+	if (node >= 0)
+		bl33_entry = spl_fit_images_get_entry(blob, node);
+
+	/*
+	 * We don't provide a BL3-2 entry yet, but this will be possible
+	 * using similar logic.
+	 */
+	bl31_entry(spl_image->entry_point, bl33_entry, (uintptr_t)blob);
 }
-- 
2.1.4

[U-Boot] [PATCH 08/15] spl: rename config item SPL_ATF_SUPPORT to SPL_ATF

[Philipp Tomsich]

Having CONFIG_SPL_ATF seems more natural.
Rename it, while it it is easy and there's few boards that use it
(only RK3399 and RK3368 boards).

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 common/spl/Kconfig  | 2 +-
 common/spl/Makefile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index b05ec21..b1e0875 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -692,7 +692,7 @@ 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
+config SPL_ATF
 	bool "Support ARM Trusted Firmware"
 	depends on ARM64
 	help
diff --git a/common/spl/Makefile b/common/spl/Makefile
index e229947..9bf8a2d 100644
--- a/common/spl/Makefile
+++ b/common/spl/Makefile
@@ -22,7 +22,7 @@ endif
 obj-$(CONFIG_$(SPL_TPL_)UBI) += spl_ubi.o
 obj-$(CONFIG_$(SPL_TPL_)NET_SUPPORT) += spl_net.o
 obj-$(CONFIG_$(SPL_TPL_)MMC_SUPPORT) += spl_mmc.o
-obj-$(CONFIG_$(SPL_TPL_)ATF_SUPPORT) += spl_atf.o
+obj-$(CONFIG_$(SPL_TPL_)ATF) += spl_atf.o
 obj-$(CONFIG_$(SPL_TPL_)USB_SUPPORT) += spl_usb.o
 obj-$(CONFIG_$(SPL_TPL_)FAT_SUPPORT) += spl_fat.o
 obj-$(CONFIG_$(SPL_TPL_)EXT_SUPPORT) += spl_ext.o
-- 
2.1.4

[U-Boot] [PATCH 09/15] spl: atf: drop the SPL_ATF_TEXT_BASE configuration item

[Philipp Tomsich]

The SPL_ATF_TEXT_BASE configuration item has become obsolete.
Remove it from Kconfig.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 common/spl/Kconfig | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index b1e0875..1193852 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -700,12 +700,6 @@ 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_TEXT_BASE
-	depends on SPL_ATF_SUPPORT
-	hex "ATF BL31 base address"
-	help
-	  This is the base address in memory for ATF BL31 text and entry point.
-
 config TPL
 	bool
 	depends on SUPPORT_TPL
-- 
2.1.4

[U-Boot] [PATCH 10/15] rockchip: dts: rk3399-puma: add /config/arm-trusted-firmware, reset-gpio property

[Philipp Tomsich]

With the ATF capable of accessing the FDT passed to the next stage,
we can specify configuration items for the ATF in the /config path.

This adds the arm-trusted-firmware,reset-gpio that conveys the number
of the GPIO used to reset the board (used, when a reboot is requested
from ATF via PSCI).

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 arch/arm/dts/rk3399-puma.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/dts/rk3399-puma.dtsi b/arch/arm/dts/rk3399-puma.dtsi
index d33c710..70d551b 100644
--- a/arch/arm/dts/rk3399-puma.dtsi
+++ b/arch/arm/dts/rk3399-puma.dtsi
@@ -16,6 +16,7 @@
 		u-boot,mmc-env-offset = <0x4000>;      /* @  16KB */
 		u-boot,efi-partition-entries-offset = <0x200000>; /* 2MB */
 		u-boot,boot-led = "module_led";
+		arm-trusted-firmware,reset-gpio = <38>;
 	};
 
 	chosen {
-- 
2.1.4

[U-Boot] [PATCH 11/15] rockchip: defconfig: firefly-rk3399: sync up with SPL changes for ATF

[Philipp Tomsich]

This tracks the SPL changes for ATF for the Firefly:
 * renames ATF_SUPPORT to ATF
 * drops CONFIG_SPL_ATF_TEXT_BASE

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 configs/firefly-rk3399_defconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/configs/firefly-rk3399_defconfig b/configs/firefly-rk3399_defconfig
index 94b9209..a7a9839 100644
--- a/configs/firefly-rk3399_defconfig
+++ b/configs/firefly-rk3399_defconfig
@@ -13,8 +13,7 @@ CONFIG_SPL_LOAD_FIT=y
 CONFIG_SPL_STACK_R=y
 CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x4000
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x200
-CONFIG_SPL_ATF_SUPPORT=y
-CONFIG_SPL_ATF_TEXT_BASE=0x00010000
+CONFIG_SPL_ATF=y
 CONFIG_CMD_BOOTZ=y
 # CONFIG_CMD_IMLS is not set
 CONFIG_CMD_GPT=y
-- 
2.1.4

[U-Boot] [PATCH 12/15] rockchip: board: puma-rk3399: update .its file to use new features

[Philipp Tomsich]

This commit updates the .its file for the RK3399-Q7 to use the new
features and demonstrates how to use those:
 * it marks the ATF as the 'firmware'
 * it tracks the OS-type for U-Boot and ATF
 * it loads the PMU (M0) firmware to DRAM and records the location
   to /fit-images (where our ATF reads it from)

With the handoff of the next-stage FDT to ATF in place, we can now use
this to pass information about the load addresses and names of each
loadables to ATF: now we can load the M0 firmware into DRAM and avoid
overwriting parts of the SPL stage.  This is achieved by changing our
.its-file to use an available area of DRAM as the load-address.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 board/theobroma-systems/puma_rk3399/fit_spl_atf.its | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/board/theobroma-systems/puma_rk3399/fit_spl_atf.its b/board/theobroma-systems/puma_rk3399/fit_spl_atf.its
index 520f846..cb7d92f 100644
--- a/board/theobroma-systems/puma_rk3399/fit_spl_atf.its
+++ b/board/theobroma-systems/puma_rk3399/fit_spl_atf.its
@@ -17,6 +17,7 @@
 			description = "U-Boot (64-bit)";
 			data = /incbin/("../../../u-boot-nodtb.bin");
 			type = "standalone";
+			os = "U-Boot";
 			arch = "arm64";
 			compression = "none";
 			load = <0x00200000>;
@@ -26,16 +27,17 @@
 			data = /incbin/("../../../bl31-rk3399.bin");
 			type = "firmware";
 			arch = "arm64";
+			os = "arm-trusted-firmware";
 			compression = "none";
-			load = <0x00001000>;
-			entry = <0x00001000>;
+			load = <0x1000>;
+			entry = <0x1000>;
 		};
 		pmu {
 		        description = "Cortex-M0 firmware";
 			data = /incbin/("../../../rk3399m0.bin");
 			type = "pmu-firmware";
 			compression = "none";
-			load = <0xff8c0000>;
+			load = <0x180000>;
                 };
 		fdt {
 			description = "RK3399-Q7 (Puma) flat device-tree";
@@ -49,8 +51,8 @@
 		default = "conf";
 		conf {
 			description = "Theobroma Systems RK3399-Q7 (Puma) SoM";
-			firmware = "uboot";
-			loadables = "atf";
+			firmware = "atf";
+			loadables = "uboot", "pmu";
 			fdt = "fdt";
 		};
 	};
-- 
2.1.4

[U-Boot] [PATCH 13/15] rockchip: board: lion-rk3368: update .its file

[Philipp Tomsich]

For the RK3368-uQ7, we can now update the .its file to mark the
Trusted Firmware as out 'firmware' bootable and annotate both ATF and
U-Boot with an OS-type.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 board/theobroma-systems/lion_rk3368/fit_spl_atf.its | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/theobroma-systems/lion_rk3368/fit_spl_atf.its b/board/theobroma-systems/lion_rk3368/fit_spl_atf.its
index 405750f..5cdad3b 100644
--- a/board/theobroma-systems/lion_rk3368/fit_spl_atf.its
+++ b/board/theobroma-systems/lion_rk3368/fit_spl_atf.its
@@ -17,6 +17,7 @@
 			description = "U-Boot (64-bit)";
 			data = /incbin/("../../../u-boot-nodtb.bin");
 			type = "standalone";
+			os = "U-Boot";
 			arch = "arm64";
 			compression = "none";
 			load = <0x00200000>;
@@ -25,6 +26,7 @@
 			description = "ARM Trusted Firmware";
 			data = /incbin/("../../../bl31-rk3368.bin");
 			type = "firmware";
+			os = "arm-trusted-firmware";
 			arch = "arm64";
 			compression = "none";
 			load = <0x00010000>;
@@ -43,8 +45,8 @@
 		default = "conf";
 		conf {
 			description = "Theobroma Systems RK3368-uQ7 (Puma) SoM";
-			firmware = "uboot";
-			loadables = "atf";
+			firmware = "atf";
+			loadables = "uboot";
 			fdt = "fdt";
 		};
 	};
-- 
2.1.4

[U-Boot] [PATCH 14/15] rockchip: defconfig: puma-rk3399: sync up with SPL changes for ATF

[Philipp Tomsich]

This defconfig update makes use of the new features:
 * CONFIG_ROCKCHIP_SPL_RESERVE_IRAM is now set to 0, as there is no
   overlap between the M0 firmware and the ATF (we load this to DRAM
   and relocate it to its final location within the ATF)
 * tracks the ATF_SUPPORT -> ATF renaming

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 configs/puma-rk3399_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/puma-rk3399_defconfig b/configs/puma-rk3399_defconfig
index 62a8d7c..2d08c4e 100644
--- a/configs/puma-rk3399_defconfig
+++ b/configs/puma-rk3399_defconfig
@@ -5,6 +5,7 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_SYS_MALLOC_F_LEN=0x4000
 CONFIG_ROCKCHIP_RK3399=y
+CONFIG_ROCKCHIP_SPL_RESERVE_IRAM=0
 CONFIG_TARGET_PUMA_RK3399=y
 CONFIG_SPL_SPI_FLASH_SUPPORT=y
 CONFIG_SPL_SPI_SUPPORT=y
@@ -23,6 +24,7 @@ CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x4000
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x200
 CONFIG_SPL_I2C_SUPPORT=y
 CONFIG_SPL_POWER_SUPPORT=y
+CONFIG_SPL_ATF=y
 CONFIG_CMD_BOOTZ=y
 # CONFIG_CMD_IMLS is not set
 CONFIG_CMD_GPT=y
-- 
2.1.4

[U-Boot] [PATCH 15/15] rockchip: defconfig: lion-rk3368: sync up with SPL changes for ATF

[Philipp Tomsich]

This tracks the SPL changes for ATF for the RK3368-uQ7:
 * renames ATF_SUPPORT to ATF
 * drops CONFIG_SPL_ATF_TEXT_BASE (now dynamically retrieved from
   the .itb file)

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 configs/lion-rk3368_defconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/configs/lion-rk3368_defconfig b/configs/lion-rk3368_defconfig
index c7ee7b3..289cb72 100644
--- a/configs/lion-rk3368_defconfig
+++ b/configs/lion-rk3368_defconfig
@@ -33,8 +33,7 @@ CONFIG_SPL_BOOTROM_SUPPORT=y
 CONFIG_TPL_SYS_MALLOC_SIMPLE=y
 CONFIG_SPL_STACK_R=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x200
-CONFIG_SPL_ATF_SUPPORT=y
-CONFIG_SPL_ATF_TEXT_BASE=0x10000
+CONFIG_SPL_ATF=y
 CONFIG_TPL=y
 CONFIG_TPL_BOOTROM_SUPPORT=y
 CONFIG_TPL_DRIVERS_MISC_SUPPORT=y
-- 
2.1.4

[U-Boot] [PATCH 04/15] spl: fit: simplify logic for FDT loading for non-OS boots

[York Sun]

On 09/13/2017 12:30 PM, Philipp Tomsich wrote:
> To better support bootin through an ATF or OPTEE, we need to
> streamline some of the logic for when the FDT is appended to an image:
> depending on the image type, we'd like to append the FDT not at all
> (the case for the OS boot), to the 'firmware' image (if it is a
> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
> OPTEE, or some other image-type and U-Boot is listed in the
> loadabled).
> 
> To achieve this goal, we drop the os_boot flag and track the type of
> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> 
>   common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>   1 file changed, 56 insertions(+), 30 deletions(-)

This change seems OK. Make sure you test booting OS. Let me know if you 
need help.

York

[U-Boot] [PATCH 04/15] spl: fit: simplify logic for FDT loading for non-OS boots

[Dr. Philipp Tomsich]

York,

> On 13 Sep 2017, at 23:16, York Sun <york.sun@nxp.com> wrote:
> 
> On 09/13/2017 12:30 PM, Philipp Tomsich wrote:
>> To better support bootin through an ATF or OPTEE, we need to
>> streamline some of the logic for when the FDT is appended to an image:
>> depending on the image type, we'd like to append the FDT not at all
>> (the case for the OS boot), to the 'firmware' image (if it is a
>> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
>> OPTEE, or some other image-type and U-Boot is listed in the
>> loadabled).
>> 
>> To achieve this goal, we drop the os_boot flag and track the type of
>> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
>> 
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>> 
>>  common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>>  1 file changed, 56 insertions(+), 30 deletions(-)
> 
> This change seems OK. Make sure you test booting OS. Let me know if you 
> need help.

I’d appreciate if you could test this on your end as well. I currently only
have test setups that need to go through ATF before entering an OS (as
the PSCI implementation lives in ATF).

Regards,
Philipp.

[U-Boot] [PATCH 04/15] spl: fit: simplify logic for FDT loading for non-OS boots

[York Sun]

On 09/13/2017 02:21 PM, Dr. Philipp Tomsich wrote:
> York,
> 
>> On 13 Sep 2017, at 23:16, York Sun <york.sun@nxp.com> wrote:
>>
>> On 09/13/2017 12:30 PM, Philipp Tomsich wrote:
>>> To better support bootin through an ATF or OPTEE, we need to
>>> streamline some of the logic for when the FDT is appended to an image:
>>> depending on the image type, we'd like to append the FDT not at all
>>> (the case for the OS boot), to the 'firmware' image (if it is a
>>> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
>>> OPTEE, or some other image-type and U-Boot is listed in the
>>> loadabled).
>>>
>>> To achieve this goal, we drop the os_boot flag and track the type of
>>> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
>>>
>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>> ---
>>>
>>>   common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>>>   1 file changed, 56 insertions(+), 30 deletions(-)
>>
>> This change seems OK. Make sure you test booting OS. Let me know if you
>> need help.
> 
> I’d appreciate if you could test this on your end as well. I currently only
> have test setups that need to go through ATF before entering an OS (as
> the PSCI implementation lives in ATF).

Philipp,

I can test _this_ patch on my board. Looks like it doesn't depend on 
other patches.

York

[U-Boot] [PATCH 04/15] spl: fit: simplify logic for FDT loading for non-OS boots

[Dr. Philipp Tomsich]

York,

> On 13 Sep 2017, at 23:24, York Sun <york.sun@nxp.com> wrote:
> 
> On 09/13/2017 02:21 PM, Dr. Philipp Tomsich wrote:
>> York,
>> 
>>> On 13 Sep 2017, at 23:16, York Sun <york.sun@nxp.com> wrote:
>>> 
>>> On 09/13/2017 12:30 PM, Philipp Tomsich wrote:
>>>> To better support bootin through an ATF or OPTEE, we need to
>>>> streamline some of the logic for when the FDT is appended to an image:
>>>> depending on the image type, we'd like to append the FDT not at all
>>>> (the case for the OS boot), to the 'firmware' image (if it is a
>>>> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
>>>> OPTEE, or some other image-type and U-Boot is listed in the
>>>> loadabled).
>>>> 
>>>> To achieve this goal, we drop the os_boot flag and track the type of
>>>> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
>>>> 
>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>> ---
>>>> 
>>>>  common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>>>>  1 file changed, 56 insertions(+), 30 deletions(-)
>>> 
>>> This change seems OK. Make sure you test booting OS. Let me know if you
>>> need help.
>> 
>> I’d appreciate if you could test this on your end as well. I currently only
>> have test setups that need to go through ATF before entering an OS (as
>> the PSCI implementation lives in ATF).
> 
> Philipp,
> 
> I can test _this_ patch on my board. Looks like it doesn't depend on 
> other patches.

The other patches shouldn’t interfere (and have been exercised quite
a bit on our end): I am only worried about possibly breaking some of
your changes for your scenario.

Philipp.

[U-Boot] [PATCH 04/15] spl: fit: simplify logic for FDT loading for non-OS boots

[York Sun]

On 09/13/2017 03:11 PM, Dr. Philipp Tomsich wrote:
> York,
> 
>> On 13 Sep 2017, at 23:24, York Sun <york.sun@nxp.com> wrote:
>>
>> On 09/13/2017 02:21 PM, Dr. Philipp Tomsich wrote:
>>> York,
>>>
>>>> On 13 Sep 2017, at 23:16, York Sun <york.sun@nxp.com> wrote:
>>>>
>>>> On 09/13/2017 12:30 PM, Philipp Tomsich wrote:
>>>>> To better support bootin through an ATF or OPTEE, we need to
>>>>> streamline some of the logic for when the FDT is appended to an image:
>>>>> depending on the image type, we'd like to append the FDT not at all
>>>>> (the case for the OS boot), to the 'firmware' image (if it is a
>>>>> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
>>>>> OPTEE, or some other image-type and U-Boot is listed in the
>>>>> loadabled).
>>>>>
>>>>> To achieve this goal, we drop the os_boot flag and track the type of
>>>>> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
>>>>>
>>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>>> ---
>>>>>
>>>>>   common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>>>>>   1 file changed, 56 insertions(+), 30 deletions(-)
>>>>
>>>> This change seems OK. Make sure you test booting OS. Let me know if you
>>>> need help.
>>>
>>> I’d appreciate if you could test this on your end as well. I currently only
>>> have test setups that need to go through ATF before entering an OS (as
>>> the PSCI implementation lives in ATF).
>>
>> Philipp,
>>
>> I can test _this_ patch on my board. Looks like it doesn't depend on
>> other patches.
> 
> The other patches shouldn’t interfere (and have been exercised quite
> a bit on our end): I am only worried about possibly breaking some of
> your changes for your scenario.
> 

Acked-by: York Sun <york.sun@nxp.com>

[U-Boot] [PATCH 01/15] image: add IH_OS_ARM_TRUSTED_FIRMWARE for ARM Trusted Firmware

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> To boot on ARMv8 systems with ARM Trusted Firmware, we need to
> assemble an ATF-specific parameter structure and also provide the
> address of the images started by ATF (e.g. BL3-3, which may be the
> full U-Boot).
>
> To allow us to identify an ARM Trusted Firmware contained in a FIT
> image, this adds the necessary definitions.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  common/image.c  | 1 +
>  include/image.h | 1 +
>  2 files changed, 2 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

You might consider IH_ATF instead of IH_OS_ARM_TRUSTED_FIRMWARE, but
it's up to you.

[U-Boot] [PATCH 02/15] spl: add a fdt_addr field to spl_image_info

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> When loading a full U-Boot with detached device-tree using the SPL FIT
> backend, we should store the address of the FDT loaded as part of the
> SPL image info: this allows us to fixup the FDT with additional info
> we may want to propagate onward.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  include/spl.h | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Could perhaps drop the #ifdef since it is only small?

[U-Boot] [PATCH 03/15] spl: change load_addr and entry_point to uintptr_t

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> Mainly a stylistic change: convert the load_addr and entry_point
> fields of struct spl_image_info to uintptr_t (from ulong).
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  include/spl.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

I am a bit nervous though since we use ulong everywhere in U-Boot for addresses.

[U-Boot] [PATCH 05/15] spl: fit: implement fdt_record_loadable

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> During the loading of more complex FIT images (e.g. when the invoked
> next stage needs to find additional firmware for a power-management
> core... or if there are multiple images for different privilege levels
> started in parallel), it is helpful to create a record of what images
> are loaded where: if a FDT is loaded for one of the next stages, it
> can be used to convey the status and location of loadables.
>
> This adds a fdt_record_loadable() function that can be invoked to
> record the status of each loadable below the /fit-images path.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  common/fdt_support.c  | 39 +++++++++++++++++++++++++++++++++++++++
>  include/fdt_support.h | 20 ++++++++++++++++++++
>  2 files changed, 59 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 06/15] spl: fit: implement recording of loadables into /fit-images

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
> address and record information for all loadables into this FDT.  This
> allows us to easily keep track of images for multiple privilege levels
> (e.g. with ATF) or of firmware images preloaded into temporary
> locations (e.g. PMU firmware that may overlap the SPL stage).
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 81 insertions(+), 14 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

I wonder if this should be a new CONFIG option to reduce code size for
things that don't need it?

[U-Boot] [PATCH 07/15] spl: atf: introduce spl_invoke_atf and make bl31_entry private

[Simon Glass]

Hi Philipp,

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> This adds a new interface spl_invoke_atf() that takes a spl_image_info
> argument and then derives the necessary parameters for the ATF entry.
> Based on the additional information recorded (into /fit-images) from
> the FIT loadables, we can now easily locate the next boot stage.
>
> We now pass a pointer to a FDT as the platform-specific parameter
> pointer to ATF (so we don't run into the future headache of every
> board/platform defining their own proprietary tag-structure), as
> FDT access is already available in ATF.
>
> With the necessary infrastructure in place, we can now update the
> support for the ARM Trusted Firmware to dispatch into the
> spl_invoke_atf function only if a IH_OS_ARM_TRUSTED_FIRMWARE image is
> loaded.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  common/spl/spl.c     | 11 +++----
>  common/spl/spl_atf.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 82 insertions(+), 13 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Please see question below

[..]

> index 6e8f928..63557c0 100644
> --- a/common/spl/spl_atf.c
> +++ b/common/spl/spl_atf.c
> @@ -5,6 +5,7 @@
>   * reserved.
>   * Copyright (C) 2016 Rockchip Electronic Co.,Ltd
>   * Written by Kever Yang <kever.yang@rock-chips.com>
> + * Copyright (C) 2017 Theobroma Systems Design und Consulting GmbH
>   *
>   * SPDX-License-Identifier:     BSD-3-Clause
>   */
> @@ -30,7 +31,7 @@ static struct bl31_params *bl2_to_bl31_params;
>   *
>   * @return bl31 params structure pointer
>   */
> -struct bl31_params *bl2_plat_get_bl31_params(void)
> +static struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl33_entry)
>  {
>         struct entry_point_info *bl33_ep_info;
>
> @@ -66,7 +67,7 @@ struct bl31_params *bl2_plat_get_bl31_params(void)
>
>         /* 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->pc = bl33_entry;
>         bl33_ep_info->spsr = SPSR_64(MODE_EL2, MODE_SP_ELX,
>                                      DISABLE_ALL_EXECPTIONS);
>
> @@ -77,21 +78,88 @@ struct bl31_params *bl2_plat_get_bl31_params(void)
>         return bl2_to_bl31_params;
>  }
>
> -void raw_write_daif(unsigned int daif)
> +static inline void raw_write_daif(unsigned int daif)
>  {
>         __asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory");
>  }
>
> -void bl31_entry(void)
> +typedef void (*atf_entry_t)(struct bl31_params *params, void *plat_params);
> +
> +static void bl31_entry(uintptr_t bl31_entry, uintptr_t bl33_entry,
> +                      uintptr_t fdt_addr)
>  {
>         struct bl31_params *bl31_params;
> -       void (*entry)(struct bl31_params *params, void *plat_params) = NULL;
> +       atf_entry_t  atf_entry = (atf_entry_t)bl31_entry;
>
> -       bl31_params = bl2_plat_get_bl31_params();
> -       entry = (void *)CONFIG_SPL_ATF_TEXT_BASE;
> +       bl31_params = bl2_plat_get_bl31_params(bl33_entry);
>
>         raw_write_daif(SPSR_EXCEPTION_MASK);
>         dcache_disable();
>
> -       entry(bl31_params, NULL);
> +       atf_entry((void *)bl31_params, (void *)fdt_addr);
> +}
> +
> +static int spl_fit_images_find_uboot(void *blob)
> +{
> +       int parent, node, ndepth;
> +       const void *data;
> +
> +       if (!blob)
> +               return -FDT_ERR_BADMAGIC;
> +
> +       parent = fdt_path_offset(blob, "/fit-images");
> +       if (parent < 0)
> +               return -FDT_ERR_NOTFOUND;
> +
> +       for (node = fdt_next_node(blob, parent, &ndepth);
> +            (node >= 0) && (ndepth > 0);
> +            node = fdt_next_node(blob, node, &ndepth)) {
> +               if (ndepth != 1)
> +                       continue;
> +
> +               data = fdt_getprop(blob, node, FIT_OS_PROP, NULL);
> +               if (!data)
> +                       continue;
> +
> +               if (genimg_get_os_id(data) == IH_OS_U_BOOT)
> +                       return node;

How come this is in 'data' instead of the 'type' property?

Regards,
Simon

[U-Boot] [PATCH 08/15] spl: rename config item SPL_ATF_SUPPORT to SPL_ATF

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> Having CONFIG_SPL_ATF seems more natural.
> Rename it, while it it is easy and there's few boards that use it
> (only RK3399 and RK3368 boards).
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  common/spl/Kconfig  | 2 +-
>  common/spl/Makefile | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 09/15] spl: atf: drop the SPL_ATF_TEXT_BASE configuration item

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> The SPL_ATF_TEXT_BASE configuration item has become obsolete.
> Remove it from Kconfig.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  common/spl/Kconfig | 6 ------
>  1 file changed, 6 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

But should also drop from defconfigs that use it.

[U-Boot] [PATCH 10/15] rockchip: dts: rk3399-puma: add /config/arm-trusted-firmware, reset-gpio property

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> With the ATF capable of accessing the FDT passed to the next stage,
> we can specify configuration items for the ATF in the /config path.
>
> This adds the arm-trusted-firmware,reset-gpio that conveys the number
> of the GPIO used to reset the board (used, when a reboot is requested
> from ATF via PSCI).
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  arch/arm/dts/rk3399-puma.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/dts/rk3399-puma.dtsi b/arch/arm/dts/rk3399-puma.dtsi
> index d33c710..70d551b 100644
> --- a/arch/arm/dts/rk3399-puma.dtsi
> +++ b/arch/arm/dts/rk3399-puma.dtsi
> @@ -16,6 +16,7 @@
>                 u-boot,mmc-env-offset = <0x4000>;      /* @  16KB */
>                 u-boot,efi-partition-entries-offset = <0x200000>; /* 2MB */
>                 u-boot,boot-led = "module_led";
> +               arm-trusted-firmware,reset-gpio = <38>;
>         };
>
>         chosen {
> --
> 2.1.4
>

How about putting this in an atf {} subnode?

That reminds me that these things should be documented in
doc/device-tree-bindings/config.txt

[U-Boot] [PATCH 11/15] rockchip: defconfig: firefly-rk3399: sync up with SPL changes for ATF

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> This tracks the SPL changes for ATF for the Firefly:
>  * renames ATF_SUPPORT to ATF
>  * drops CONFIG_SPL_ATF_TEXT_BASE
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  configs/firefly-rk3399_defconfig | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 12/15] rockchip: board: puma-rk3399: update .its file to use new features

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> This commit updates the .its file for the RK3399-Q7 to use the new
> features and demonstrates how to use those:
>  * it marks the ATF as the 'firmware'
>  * it tracks the OS-type for U-Boot and ATF
>  * it loads the PMU (M0) firmware to DRAM and records the location
>    to /fit-images (where our ATF reads it from)
>
> With the handoff of the next-stage FDT to ATF in place, we can now use
> this to pass information about the load addresses and names of each
> loadables to ATF: now we can load the M0 firmware into DRAM and avoid
> overwriting parts of the SPL stage.  This is achieved by changing our
> .its-file to use an available area of DRAM as the load-address.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  board/theobroma-systems/puma_rk3399/fit_spl_atf.its | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 14/15] rockchip: defconfig: puma-rk3399: sync up with SPL changes for ATF

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> This defconfig update makes use of the new features:
>  * CONFIG_ROCKCHIP_SPL_RESERVE_IRAM is now set to 0, as there is no
>    overlap between the M0 firmware and the ATF (we load this to DRAM
>    and relocate it to its final location within the ATF)
>  * tracks the ATF_SUPPORT -> ATF renaming
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  configs/puma-rk3399_defconfig | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 13/15] rockchip: board: lion-rk3368: update .its file

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> For the RK3368-uQ7, we can now update the .its file to mark the
> Trusted Firmware as out 'firmware' bootable and annotate both ATF and
> U-Boot with an OS-type.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  board/theobroma-systems/lion_rk3368/fit_spl_atf.its | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 15/15] rockchip: defconfig: lion-rk3368: sync up with SPL changes for ATF

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> This tracks the SPL changes for ATF for the RK3368-uQ7:
>  * renames ATF_SUPPORT to ATF
>  * drops CONFIG_SPL_ATF_TEXT_BASE (now dynamically retrieved from
>    the .itb file)
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  configs/lion-rk3368_defconfig | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 04/15] spl: fit: simplify logic for FDT loading for non-OS boots

[Simon Glass]

On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> To better support bootin through an ATF or OPTEE, we need to
> streamline some of the logic for when the FDT is appended to an image:
> depending on the image type, we'd like to append the FDT not at all
> (the case for the OS boot), to the 'firmware' image (if it is a
> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
> OPTEE, or some other image-type and U-Boot is listed in the
> loadabled).
>
> To achieve this goal, we drop the os_boot flag and track the type of
> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 56 insertions(+), 30 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 00/15] spl: atf: update booting images via ATF to use info from FIT images

[Michal Simek]

Hi Philipp,

On 13.9.2017 21:29, Philipp Tomsich wrote:
> 
> A number of things about how we boot the RK3368 and RK3399 through ATF
> are less than ideal today, especially when considering future
> platforms that will follow a similar boot concept:
> - the auto-detection of images from the FIT images was limited (i.e.
>   the start address of the BL33 image could not automatically retrieved)
> - no implementation for the platform-specific parameters exists (and
>   there is a danger that we'll end up with highly different, proprietary
>   platform parameters for different SOCs and boards, even though the
>   ATF code base already has FDT support)
> 
> This series tries to put us into a better position to support various
> boot scenarios (e.g. loading an OPTEE from the FIT image; and: booting
> a Linux kernel via ATF) in the future... and it establishes the FDT as
> a mechanism to pass boot-info to later stages.
> 
> For a practical example, refer to how we use this on the RK3399-Q7:
> * the ATF can read the full U-Boot's FDT to determine how to best issue
>   a cold-reset for the board
> * we inject information on where we loaded the M0 firmware into the
>   same FDT that is now visible to the ATF, so the ATF can relocate it
>   to its final destination---and we no longer need to overwrite parts
>   of the SPL binary during bootup
> 
> Note that there are still some limitations (e.g. the support for
> passing OPTEE as a BL3-2, is not in this version ... and there isn't
> support for booting Linux directly via ATF yet, either), but these can
> now be plugged cleanly into this infrastructure.


can you please send also sent your bootlogs?
I would like to see your flow also with pmu-firmware loading.

Thanks,
Michal

[U-Boot] [PATCH 07/15] spl: atf: introduce spl_invoke_atf and make bl31_entry private

[Dr. Philipp Tomsich]

> On 17 Sep 2017, at 19:53, Simon Glass <sjg@chromium.org> wrote:
> 
> Hi Philipp,
> 
> On 13 September 2017 at 13:29, Philipp Tomsich
> <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>> wrote:
>> This adds a new interface spl_invoke_atf() that takes a spl_image_info
>> argument and then derives the necessary parameters for the ATF entry.
>> Based on the additional information recorded (into /fit-images) from
>> the FIT loadables, we can now easily locate the next boot stage.
>> 
>> We now pass a pointer to a FDT as the platform-specific parameter
>> pointer to ATF (so we don't run into the future headache of every
>> board/platform defining their own proprietary tag-structure), as
>> FDT access is already available in ATF.
>> 
>> With the necessary infrastructure in place, we can now update the
>> support for the ARM Trusted Firmware to dispatch into the
>> spl_invoke_atf function only if a IH_OS_ARM_TRUSTED_FIRMWARE image is
>> loaded.
>> 
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>> 
>> common/spl/spl.c     | 11 +++----
>> common/spl/spl_atf.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++-----
>> 2 files changed, 82 insertions(+), 13 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org>>
> 
> Please see question below
> 
> [..]
> 
>> index 6e8f928..63557c0 100644
>> --- a/common/spl/spl_atf.c
>> +++ b/common/spl/spl_atf.c
>> @@ -5,6 +5,7 @@
>>  * reserved.
>>  * Copyright (C) 2016 Rockchip Electronic Co.,Ltd
>>  * Written by Kever Yang <kever.yang at rock-chips.com <mailto:kever.yang@rock-chips.com>>
>> + * Copyright (C) 2017 Theobroma Systems Design und Consulting GmbH
>>  *
>>  * SPDX-License-Identifier:     BSD-3-Clause
>>  */
>> @@ -30,7 +31,7 @@ static struct bl31_params *bl2_to_bl31_params;
>>  *
>>  * @return bl31 params structure pointer
>>  */
>> -struct bl31_params *bl2_plat_get_bl31_params(void)
>> +static struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl33_entry)
>> {
>>        struct entry_point_info *bl33_ep_info;
>> 
>> @@ -66,7 +67,7 @@ struct bl31_params *bl2_plat_get_bl31_params(void)
>> 
>>        /* 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->pc = bl33_entry;
>>        bl33_ep_info->spsr = SPSR_64(MODE_EL2, MODE_SP_ELX,
>>                                     DISABLE_ALL_EXECPTIONS);
>> 
>> @@ -77,21 +78,88 @@ struct bl31_params *bl2_plat_get_bl31_params(void)
>>        return bl2_to_bl31_params;
>> }
>> 
>> -void raw_write_daif(unsigned int daif)
>> +static inline void raw_write_daif(unsigned int daif)
>> {
>>        __asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory");
>> }
>> 
>> -void bl31_entry(void)
>> +typedef void (*atf_entry_t)(struct bl31_params *params, void *plat_params);
>> +
>> +static void bl31_entry(uintptr_t bl31_entry, uintptr_t bl33_entry,
>> +                      uintptr_t fdt_addr)
>> {
>>        struct bl31_params *bl31_params;
>> -       void (*entry)(struct bl31_params *params, void *plat_params) = NULL;
>> +       atf_entry_t  atf_entry = (atf_entry_t)bl31_entry;
>> 
>> -       bl31_params = bl2_plat_get_bl31_params();
>> -       entry = (void *)CONFIG_SPL_ATF_TEXT_BASE;
>> +       bl31_params = bl2_plat_get_bl31_params(bl33_entry);
>> 
>>        raw_write_daif(SPSR_EXCEPTION_MASK);
>>        dcache_disable();
>> 
>> -       entry(bl31_params, NULL);
>> +       atf_entry((void *)bl31_params, (void *)fdt_addr);
>> +}
>> +
>> +static int spl_fit_images_find_uboot(void *blob)
>> +{
>> +       int parent, node, ndepth;
>> +       const void *data;
>> +
>> +       if (!blob)
>> +               return -FDT_ERR_BADMAGIC;
>> +
>> +       parent = fdt_path_offset(blob, "/fit-images");
>> +       if (parent < 0)
>> +               return -FDT_ERR_NOTFOUND;
>> +
>> +       for (node = fdt_next_node(blob, parent, &ndepth);
>> +            (node >= 0) && (ndepth > 0);
>> +            node = fdt_next_node(blob, node, &ndepth)) {
>> +               if (ndepth != 1)
>> +                       continue;
>> +
>> +               data = fdt_getprop(blob, node, FIT_OS_PROP, NULL);
>> +               if (!data)
>> +                       continue;
>> +
>> +               if (genimg_get_os_id(data) == IH_OS_U_BOOT)
>> +                       return node;
> 
> How come this is in 'data' instead of the 'type' property?

In fact it’s in FIT_OS_PROP (which expands to “os”).
Naming this variable ‘data' may have not been the best choice, but ‘prop’ didn’t
seem much better…

Regards,
Philipp.

[U-Boot] [PATCH 10/15] rockchip: dts: rk3399-puma: add /config/arm-trusted-firmware, reset-gpio property

[Dr. Philipp Tomsich]

> On 17 Sep 2017, at 19:54, Simon Glass <sjg@chromium.org> wrote:
> 
> On 13 September 2017 at 13:29, Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com> wrote:
>> With the ATF capable of accessing the FDT passed to the next stage,
>> we can specify configuration items for the ATF in the /config path.
>> 
>> This adds the arm-trusted-firmware,reset-gpio that conveys the number
>> of the GPIO used to reset the board (used, when a reboot is requested
>> from ATF via PSCI).
>> 
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>> 
>> arch/arm/dts/rk3399-puma.dtsi | 1 +
>> 1 file changed, 1 insertion(+)
>> 
>> diff --git a/arch/arm/dts/rk3399-puma.dtsi b/arch/arm/dts/rk3399-puma.dtsi
>> index d33c710..70d551b 100644
>> --- a/arch/arm/dts/rk3399-puma.dtsi
>> +++ b/arch/arm/dts/rk3399-puma.dtsi
>> @@ -16,6 +16,7 @@
>>                u-boot,mmc-env-offset = <0x4000>;      /* @  16KB */
>>                u-boot,efi-partition-entries-offset = <0x200000>; /* 2MB */
>>                u-boot,boot-led = "module_led";
>> +               arm-trusted-firmware,reset-gpio = <38>;
>>        };
>> 
>>        chosen {
>> --
>> 2.1.4
>> 
> 
> How about putting this in an atf {} subnode?
> 
> That reminds me that these things should be documented in
> doc/device-tree-bindings/config.txt


This is in fact a good idea (as this allows to prefix the the reset-gpio with a 
board-specific prefix):
	config {
		arm-trusted-firmware {
			rk3399,reset-gpio = ...
		}
	}

However, as we already have an ATF version floating around customers that
uses this, I will merge it as-is and submit a follow-up series that updates it to
the new model (together with proper documentation), so this is documented
in the git history.

Regards,
Philipp.

[U-Boot] [U-Boot, 01/15] image: add IH_OS_ARM_TRUSTED_FIRMWARE for ARM Trusted Firmware

[Philipp Tomsich]

> To boot on ARMv8 systems with ARM Trusted Firmware, we need to
> assemble an ATF-specific parameter structure and also provide the
> address of the images started by ATF (e.g. BL3-3, which may be the
> full U-Boot).
> 
> To allow us to identify an ARM Trusted Firmware contained in a FIT
> image, this adds the necessary definitions.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/image.c  | 1 +
>  include/image.h | 1 +
>  2 files changed, 2 insertions(+)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 02/15] spl: add a fdt_addr field to spl_image_info

[Philipp Tomsich]

> When loading a full U-Boot with detached device-tree using the SPL FIT
> backend, we should store the address of the FDT loaded as part of the
> SPL image info: this allows us to fixup the FDT with additional info
> we may want to propagate onward.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  include/spl.h | 3 +++
>  1 file changed, 3 insertions(+)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 03/15] spl: change load_addr and entry_point to uintptr_t

[Philipp Tomsich]

> Mainly a stylistic change: convert the load_addr and entry_point
> fields of struct spl_image_info to uintptr_t (from ulong).
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  include/spl.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 04/15] spl: fit: simplify logic for FDT loading for non-OS boots

[Philipp Tomsich]

> To better support bootin through an ATF or OPTEE, we need to
> streamline some of the logic for when the FDT is appended to an image:
> depending on the image type, we'd like to append the FDT not at all
> (the case for the OS boot), to the 'firmware' image (if it is a
> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
> OPTEE, or some other image-type and U-Boot is listed in the
> loadabled).
> 
> To achieve this goal, we drop the os_boot flag and track the type of
> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Acked-by: York Sun <york.sun@nxp.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 56 insertions(+), 30 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 05/15] spl: fit: implement fdt_record_loadable

[Philipp Tomsich]

> During the loading of more complex FIT images (e.g. when the invoked
> next stage needs to find additional firmware for a power-management
> core... or if there are multiple images for different privilege levels
> started in parallel), it is helpful to create a record of what images
> are loaded where: if a FDT is loaded for one of the next stages, it
> can be used to convey the status and location of loadables.
> 
> This adds a fdt_record_loadable() function that can be invoked to
> record the status of each loadable below the /fit-images path.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/fdt_support.c  | 39 +++++++++++++++++++++++++++++++++++++++
>  include/fdt_support.h | 20 ++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 06/15] spl: fit: implement recording of loadables into /fit-images

[Philipp Tomsich]

> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
> address and record information for all loadables into this FDT.  This
> allows us to easily keep track of images for multiple privilege levels
> (e.g. with ATF) or of firmware images preloaded into temporary
> locations (e.g. PMU firmware that may overlap the SPL stage).
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 81 insertions(+), 14 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 07/15] spl: atf: introduce spl_invoke_atf and make bl31_entry private

[Philipp Tomsich]

> This adds a new interface spl_invoke_atf() that takes a spl_image_info
> argument and then derives the necessary parameters for the ATF entry.
> Based on the additional information recorded (into /fit-images) from
> the FIT loadables, we can now easily locate the next boot stage.
> 
> We now pass a pointer to a FDT as the platform-specific parameter
> pointer to ATF (so we don't run into the future headache of every
> board/platform defining their own proprietary tag-structure), as
> FDT access is already available in ATF.
> 
> With the necessary infrastructure in place, we can now update the
> support for the ARM Trusted Firmware to dispatch into the
> spl_invoke_atf function only if a IH_OS_ARM_TRUSTED_FIRMWARE image is
> loaded.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/spl/spl.c     | 11 +++----
>  common/spl/spl_atf.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 82 insertions(+), 13 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 08/15] spl: rename config item SPL_ATF_SUPPORT to SPL_ATF

[Philipp Tomsich]

> Having CONFIG_SPL_ATF seems more natural.
> Rename it, while it it is easy and there's few boards that use it
> (only RK3399 and RK3368 boards).
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/spl/Kconfig  | 2 +-
>  common/spl/Makefile | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 10/15] rockchip: dts: rk3399-puma: add /config/arm-trusted-firmware, reset-gpio property

[Philipp Tomsich]

> With the ATF capable of accessing the FDT passed to the next stage,
> we can specify configuration items for the ATF in the /config path.
> 
> This adds the arm-trusted-firmware,reset-gpio that conveys the number
> of the GPIO used to reset the board (used, when a reboot is requested
> from ATF via PSCI).
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> 
>  arch/arm/dts/rk3399-puma.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 09/15] spl: atf: drop the SPL_ATF_TEXT_BASE configuration item

[Philipp Tomsich]

> The SPL_ATF_TEXT_BASE configuration item has become obsolete.
> Remove it from Kconfig.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/spl/Kconfig | 6 ------
>  1 file changed, 6 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 11/15] rockchip: defconfig: firefly-rk3399: sync up with SPL changes for ATF

[Philipp Tomsich]

> This tracks the SPL changes for ATF for the Firefly:
>  * renames ATF_SUPPORT to ATF
>  * drops CONFIG_SPL_ATF_TEXT_BASE
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  configs/firefly-rk3399_defconfig | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 12/15] rockchip: board: puma-rk3399: update .its file to use new features

[Philipp Tomsich]

> This commit updates the .its file for the RK3399-Q7 to use the new
> features and demonstrates how to use those:
>  * it marks the ATF as the 'firmware'
>  * it tracks the OS-type for U-Boot and ATF
>  * it loads the PMU (M0) firmware to DRAM and records the location
>    to /fit-images (where our ATF reads it from)
> 
> With the handoff of the next-stage FDT to ATF in place, we can now use
> this to pass information about the load addresses and names of each
> loadables to ATF: now we can load the M0 firmware into DRAM and avoid
> overwriting parts of the SPL stage.  This is achieved by changing our
> .its-file to use an available area of DRAM as the load-address.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  board/theobroma-systems/puma_rk3399/fit_spl_atf.its | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 13/15] rockchip: board: lion-rk3368: update .its file

[Philipp Tomsich]

> For the RK3368-uQ7, we can now update the .its file to mark the
> Trusted Firmware as out 'firmware' bootable and annotate both ATF and
> U-Boot with an OS-type.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  board/theobroma-systems/lion_rk3368/fit_spl_atf.its | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 14/15] rockchip: defconfig: puma-rk3399: sync up with SPL changes for ATF

[Philipp Tomsich]

> This defconfig update makes use of the new features:
>  * CONFIG_ROCKCHIP_SPL_RESERVE_IRAM is now set to 0, as there is no
>    overlap between the M0 firmware and the ATF (we load this to DRAM
>    and relocate it to its final location within the ATF)
>  * tracks the ATF_SUPPORT -> ATF renaming
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  configs/puma-rk3399_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 15/15] rockchip: defconfig: lion-rk3368: sync up with SPL changes for ATF

[Philipp Tomsich]

> This tracks the SPL changes for ATF for the RK3368-uQ7:
>  * renames ATF_SUPPORT to ATF
>  * drops CONFIG_SPL_ATF_TEXT_BASE (now dynamically retrieved from
>    the .itb file)
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  configs/lion-rk3368_defconfig | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

[U-Boot] [U-Boot, 10/15] rockchip: dts: rk3399-puma: add /config/arm-trusted-firmware, reset-gpio property

[Dr. Philipp Tomsich]

> On 23 Nov 2017, at 15:51, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote:
> 
>> With the ATF capable of accessing the FDT passed to the next stage,
>> we can specify configuration items for the ATF in the /config path.
>> 
>> This adds the arm-trusted-firmware,reset-gpio that conveys the number
>> of the GPIO used to reset the board (used, when a reboot is requested
>> from ATF via PSCI).
>> 
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>> 
>> arch/arm/dts/rk3399-puma.dtsi | 1 +
>> 1 file changed, 1 insertion(+)
>> 
> 
> Applied to u-boot-rockchip, thanks!

Given that this one causes a spurious DTC warning (due to the property
ending in “-gpio”), I am dropping it from u-boot-rockchip.

Once we have updated our ATF to the final naming scheme, I’ll submit a
separate patch.

[U-Boot] [U-Boot, 07/15] spl: atf: introduce spl_invoke_atf and make bl31_entry private

[Kever Yang]

Hi Philipp,

     This patch use fdt_addr as plat_params, break the compatible with 
upstream

ATF, and get error:

"ERROR:   not expected type found 6410029648624618960"

The ATF do have a requirement for plat_params structure, and fdt_addr 
does not match this:

/* common header for all plat parameter type */
struct bl31_plat_param {
 >-------uint64_t type;
 >-------void *next;
};


Thanks,
- Kever

On 11/23/2017 10:51 PM, Philipp Tomsich wrote:
>> This adds a new interface spl_invoke_atf() that takes a spl_image_info
>> argument and then derives the necessary parameters for the ATF entry.
>> Based on the additional information recorded (into /fit-images) from
>> the FIT loadables, we can now easily locate the next boot stage.
>>
>> We now pass a pointer to a FDT as the platform-specific parameter
>> pointer to ATF (so we don't run into the future headache of every
>> board/platform defining their own proprietary tag-structure), as
>> FDT access is already available in ATF.
>>
>> With the necessary infrastructure in place, we can now update the
>> support for the ARM Trusted Firmware to dispatch into the
>> spl_invoke_atf function only if a IH_OS_ARM_TRUSTED_FIRMWARE image is
>> loaded.
>>
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>   common/spl/spl.c     | 11 +++----
>>   common/spl/spl_atf.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++-----
>>   2 files changed, 82 insertions(+), 13 deletions(-)
>>
> Applied to u-boot-rockchip, thanks!
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 06/15] spl: fit: implement recording of loadables into /fit-images

[Michal Simek]

Hi Philipp,


2017-09-13 21:29 GMT+02:00 Philipp Tomsich <
philipp.tomsich@theobroma-systems.com>:

> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
> address and record information for all loadables into this FDT.  This
> allows us to easily keep track of images for multiple privilege levels
> (e.g. with ATF) or of firmware images preloaded into temporary
> locations (e.g. PMU firmware that may overlap the SPL stage).
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++
> ++++++++++++++--------
>  1 file changed, 81 insertions(+), 14 deletions(-)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 9f05e1e..6dc0969 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -2,7 +2,7 @@
>   * Copyright (C) 2016 Google, Inc
>   * Written by Simon Glass <sjg@chromium.org>
>   *
> - * SPDX-License-Identifier:     GPL-2.0+
> + * SPDX-License-Identifier:    GPL-2.0+
>   */
>
>  #include <common.h>
> @@ -16,22 +16,24 @@
>  #endif
>
>  /**
> - * spl_fit_get_image_node(): By using the matching configuration subnode,
> + * spl_fit_get_image_name(): By using the matching configuration subnode,
>   * retrieve the name of an image, specified by a property name and an
> index
>   * into that.
>   * @fit:       Pointer to the FDT blob.
>   * @images:    Offset of the /images subnode.
>   * @type:      Name of the property within the configuration subnode.
>   * @index:     Index into the list of strings in this property.
> + * @outname:   Name of the image
>   *
> - * Return:     the node offset of the respective image node or a negative
> - *             error number.
> + * Return:     0 on success, or a negative error number
>   */
> -static int spl_fit_get_image_node(const void *fit, int images,
> -                                 const char *type, int index)
> +static int spl_fit_get_image_name(const void *fit, int images,
> +                                 const char *type, int index,
> +                                 char **outname)
>  {
>         const char *name, *str;
> -       int node, conf_node;
> +       __maybe_unused int node;
> +       int conf_node;
>         int len, i;
>
>         conf_node = fit_find_config_node(fit);
> @@ -63,7 +65,35 @@ static int spl_fit_get_image_node(const void *fit, int
> images,
>                 }
>         }
>
> +       *outname = (char *)str;
> +       return 0;
> +}
> +
> +/**
> + * spl_fit_get_image_node(): By using the matching configuration subnode,
> + * retrieve the name of an image, specified by a property name and an
> index
> + * into that.
> + * @fit:       Pointer to the FDT blob.
> + * @images:    Offset of the /images subnode.
> + * @type:      Name of the property within the configuration subnode.
> + * @index:     Index into the list of strings in this property.
> + *
> + * Return:     the node offset of the respective image node or a negative
> + *             error number.
> + */
> +static int spl_fit_get_image_node(const void *fit, int images,
> +                                 const char *type, int index)
> +{
> +       char *str;
> +       int err;
> +       int node;
> +
> +       err = spl_fit_get_image_name(fit, images, type, index, &str);
> +       if (err)
> +               return err;
> +
>         debug("%s: '%s'\n", type, str);
> +
>         node = fdt_subnode_offset(fit, images, str);
>         if (node < 0) {
>                 debug("cannot find image node '%s': %d\n", str, node);
> @@ -116,15 +146,15 @@ static int get_aligned_image_size(struct
> spl_load_info *info, int data_size,
>   * @info:      points to information about the device to load data from
>   * @sector:    the start sector of the FIT image on the device
>   * @fit:       points to the flattened device tree blob describing the FIT
> - *             image
> + *             image
>   * @base_offset: the beginning of the data area containing the actual
>   *             image data, relative to the beginning of the FIT
>   * @node:      offset of the DT node describing the image to load
> (relative
> - *             to @fit)
> + *             to @fit)
>   * @image_info:        will be filled with information about the loaded
> image
> - *             If the FIT node does not contain a "load" (address)
> property,
> - *             the image gets loaded to the address pointed to by the
> - *             load_addr member in this struct.
> + *             If the FIT node does not contain a "load" (address)
> property,
> + *             the image gets loaded to the address pointed to by the
> + *             load_addr member in this struct.
>   *
>   * Return:     0 on success or a negative error number.
>   */
> @@ -236,6 +266,35 @@ static int spl_fit_append_fdt(struct spl_image_info
> *spl_image,
>         image_info.load_addr = spl_image->load_addr + spl_image->size;
>         ret = spl_load_fit_image(info, sector, fit, base_offset, node,
>                                  &image_info);
> +
> +       if (ret < 0)
> +               return ret;
> +
> +       /* Make the load-address of the FDT available for the SPL
> framework */
> +       spl_image->fdt_addr = (void *)image_info.load_addr;
> +       /* Try to make space, so we can inject details on the loadables */
> +       ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
> +
> +       return ret;
> +}
> +
> +static int spl_fit_record_loadable(const void *fit, int images, int index,
> +                                  void *blob, struct spl_image_info
> *image)
> +{
> +       char *name;
> +       int node, ret;
> +
> +       ret = spl_fit_get_image_name(fit, images, "loadables",
> +                                    index, &name);
> +       if (ret < 0)
> +               return ret;
> +
> +       node = spl_fit_get_image_node(fit, images, "loadables", index);
> +
> +       ret = fdt_record_loadable(blob, index, name, image->load_addr,
> +                                 image->size, image->entry_point,
> +                                 fdt_getprop(fit, node, "type", NULL),
> +                                 fdt_getprop(fit, node, "os", NULL));
>


Calling this fdt_record_loadable is causing compilation issue when
CONFIG_ARCH_FIXUP_FDT_MEMORY is disabled.

common/spl/built-in.o: In function `spl_fit_record_loadable':
/mnt/disk/u-boot/common/spl/spl_fit.c:308: undefined reference to
`fdt_record_loadable'
/mnt/disk/u-boot/common/spl/spl_fit.c:308:(.text.spl_load_simple_fit+0x2ec):
relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol
`fdt_record_loadable'
make[1]: *** [spl/u-boot-spl] Error 1

You can see this issue with xilinx_zynqmp_zcu102_rev1_0_defconfig when you
disable that option.
Just a note adding dependency to Kconfig is not a proper solution because
for this combination I need fit image support.

Can you please propose solution?

Thanks,
Michal

[U-Boot] [PATCH 06/15] spl: fit: implement recording of loadables into /fit-images

[Dr. Philipp Tomsich]

Michal,

> On 18 Jan 2018, at 13:56, Michal Simek <monstr@monstr.eu> wrote:
> 
> Hi Philipp,
> 
> 
> 2017-09-13 21:29 GMT+02:00 Philipp Tomsich <philipp.tomsich@theobroma-systems.com>:
> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
> address and record information for all loadables into this FDT.  This
> allows us to easily keep track of images for multiple privilege levels
> (e.g. with ATF) or of firmware images preloaded into temporary
> locations (e.g. PMU firmware that may overlap the SPL stage).
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> 
>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 81 insertions(+), 14 deletions(-)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 9f05e1e..6dc0969 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -2,7 +2,7 @@
>   * Copyright (C) 2016 Google, Inc
>   * Written by Simon Glass <sjg@chromium.org>
>   *
> - * SPDX-License-Identifier:     GPL-2.0+
> + * SPDX-License-Identifier:    GPL-2.0+
>   */
> 
>  #include <common.h>
> @@ -16,22 +16,24 @@
>  #endif
> 
>  /**
> - * spl_fit_get_image_node(): By using the matching configuration subnode,
> + * spl_fit_get_image_name(): By using the matching configuration subnode,
>   * retrieve the name of an image, specified by a property name and an index
>   * into that.
>   * @fit:       Pointer to the FDT blob.
>   * @images:    Offset of the /images subnode.
>   * @type:      Name of the property within the configuration subnode.
>   * @index:     Index into the list of strings in this property.
> + * @outname:   Name of the image
>   *
> - * Return:     the node offset of the respective image node or a negative
> - *             error number.
> + * Return:     0 on success, or a negative error number
>   */
> -static int spl_fit_get_image_node(const void *fit, int images,
> -                                 const char *type, int index)
> +static int spl_fit_get_image_name(const void *fit, int images,
> +                                 const char *type, int index,
> +                                 char **outname)
>  {
>         const char *name, *str;
> -       int node, conf_node;
> +       __maybe_unused int node;
> +       int conf_node;
>         int len, i;
> 
>         conf_node = fit_find_config_node(fit);
> @@ -63,7 +65,35 @@ static int spl_fit_get_image_node(const void *fit, int images,
>                 }
>         }
> 
> +       *outname = (char *)str;
> +       return 0;
> +}
> +
> +/**
> + * spl_fit_get_image_node(): By using the matching configuration subnode,
> + * retrieve the name of an image, specified by a property name and an index
> + * into that.
> + * @fit:       Pointer to the FDT blob.
> + * @images:    Offset of the /images subnode.
> + * @type:      Name of the property within the configuration subnode.
> + * @index:     Index into the list of strings in this property.
> + *
> + * Return:     the node offset of the respective image node or a negative
> + *             error number.
> + */
> +static int spl_fit_get_image_node(const void *fit, int images,
> +                                 const char *type, int index)
> +{
> +       char *str;
> +       int err;
> +       int node;
> +
> +       err = spl_fit_get_image_name(fit, images, type, index, &str);
> +       if (err)
> +               return err;
> +
>         debug("%s: '%s'\n", type, str);
> +
>         node = fdt_subnode_offset(fit, images, str);
>         if (node < 0) {
>                 debug("cannot find image node '%s': %d\n", str, node);
> @@ -116,15 +146,15 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>   * @info:      points to information about the device to load data from
>   * @sector:    the start sector of the FIT image on the device
>   * @fit:       points to the flattened device tree blob describing the FIT
> - *             image
> + *             image
>   * @base_offset: the beginning of the data area containing the actual
>   *             image data, relative to the beginning of the FIT
>   * @node:      offset of the DT node describing the image to load (relative
> - *             to @fit)
> + *             to @fit)
>   * @image_info:        will be filled with information about the loaded image
> - *             If the FIT node does not contain a "load" (address) property,
> - *             the image gets loaded to the address pointed to by the
> - *             load_addr member in this struct.
> + *             If the FIT node does not contain a "load" (address) property,
> + *             the image gets loaded to the address pointed to by the
> + *             load_addr member in this struct.
>   *
>   * Return:     0 on success or a negative error number.
>   */
> @@ -236,6 +266,35 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,
>         image_info.load_addr = spl_image->load_addr + spl_image->size;
>         ret = spl_load_fit_image(info, sector, fit, base_offset, node,
>                                  &image_info);
> +
> +       if (ret < 0)
> +               return ret;
> +
> +       /* Make the load-address of the FDT available for the SPL framework */
> +       spl_image->fdt_addr = (void *)image_info.load_addr;
> +       /* Try to make space, so we can inject details on the loadables */
> +       ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
> +
> +       return ret;
> +}
> +
> +static int spl_fit_record_loadable(const void *fit, int images, int index,
> +                                  void *blob, struct spl_image_info *image)
> +{
> +       char *name;
> +       int node, ret;
> +
> +       ret = spl_fit_get_image_name(fit, images, "loadables",
> +                                    index, &name);
> +       if (ret < 0)
> +               return ret;
> +
> +       node = spl_fit_get_image_node(fit, images, "loadables", index);
> +
> +       ret = fdt_record_loadable(blob, index, name, image->load_addr,
> +                                 image->size, image->entry_point,
> +                                 fdt_getprop(fit, node, "type", NULL),
> +                                 fdt_getprop(fit, node, "os", NULL));
> 
> 
> Calling this fdt_record_loadable is causing compilation issue when CONFIG_ARCH_FIXUP_FDT_MEMORY is disabled.
> 
> common/spl/built-in.o: In function `spl_fit_record_loadable':
> /mnt/disk/u-boot/common/spl/spl_fit.c:308: undefined reference to `fdt_record_loadable'
> /mnt/disk/u-boot/common/spl/spl_fit.c:308:(.text.spl_load_simple_fit+0x2ec): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `fdt_record_loadable'
> make[1]: *** [spl/u-boot-spl] Error 1
> 
> You can see this issue with xilinx_zynqmp_zcu102_rev1_0_defconfig when you disable that option.
> Just a note adding dependency to Kconfig is not a proper solution because for this combination I need fit image support.
> 
> Can you please propose solution?

Thanks for the heads-up.

Sloppy editing on my part. The fdt_record_loadable function needs to be moved out of
the #ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY guard in common/fdt_support.c.
I’ll send a patch.


> Thanks,
> Michal
> 

[U-Boot] [PATCH 06/15] spl: fit: implement recording of loadables into /fit-images

[Michal Simek]

On 18.1.2018 14:17, Dr. Philipp Tomsich wrote:
> Michal,
> 
>> On 18 Jan 2018, at 13:56, Michal Simek <monstr@monstr.eu> wrote:
>>
>> Hi Philipp,
>>
>>
>> 2017-09-13 21:29 GMT+02:00 Philipp Tomsich <philipp.tomsich@theobroma-systems.com>:
>> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
>> address and record information for all loadables into this FDT.  This
>> allows us to easily keep track of images for multiple privilege levels
>> (e.g. with ATF) or of firmware images preloaded into temporary
>> locations (e.g. PMU firmware that may overlap the SPL stage).
>>
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>>
>>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 81 insertions(+), 14 deletions(-)
>>
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index 9f05e1e..6dc0969 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -2,7 +2,7 @@
>>   * Copyright (C) 2016 Google, Inc
>>   * Written by Simon Glass <sjg@chromium.org>
>>   *
>> - * SPDX-License-Identifier:     GPL-2.0+
>> + * SPDX-License-Identifier:    GPL-2.0+
>>   */
>>
>>  #include <common.h>
>> @@ -16,22 +16,24 @@
>>  #endif
>>
>>  /**
>> - * spl_fit_get_image_node(): By using the matching configuration subnode,
>> + * spl_fit_get_image_name(): By using the matching configuration subnode,
>>   * retrieve the name of an image, specified by a property name and an index
>>   * into that.
>>   * @fit:       Pointer to the FDT blob.
>>   * @images:    Offset of the /images subnode.
>>   * @type:      Name of the property within the configuration subnode.
>>   * @index:     Index into the list of strings in this property.
>> + * @outname:   Name of the image
>>   *
>> - * Return:     the node offset of the respective image node or a negative
>> - *             error number.
>> + * Return:     0 on success, or a negative error number
>>   */
>> -static int spl_fit_get_image_node(const void *fit, int images,
>> -                                 const char *type, int index)
>> +static int spl_fit_get_image_name(const void *fit, int images,
>> +                                 const char *type, int index,
>> +                                 char **outname)
>>  {
>>         const char *name, *str;
>> -       int node, conf_node;
>> +       __maybe_unused int node;
>> +       int conf_node;
>>         int len, i;
>>
>>         conf_node = fit_find_config_node(fit);
>> @@ -63,7 +65,35 @@ static int spl_fit_get_image_node(const void *fit, int images,
>>                 }
>>         }
>>
>> +       *outname = (char *)str;
>> +       return 0;
>> +}
>> +
>> +/**
>> + * spl_fit_get_image_node(): By using the matching configuration subnode,
>> + * retrieve the name of an image, specified by a property name and an index
>> + * into that.
>> + * @fit:       Pointer to the FDT blob.
>> + * @images:    Offset of the /images subnode.
>> + * @type:      Name of the property within the configuration subnode.
>> + * @index:     Index into the list of strings in this property.
>> + *
>> + * Return:     the node offset of the respective image node or a negative
>> + *             error number.
>> + */
>> +static int spl_fit_get_image_node(const void *fit, int images,
>> +                                 const char *type, int index)
>> +{
>> +       char *str;
>> +       int err;
>> +       int node;
>> +
>> +       err = spl_fit_get_image_name(fit, images, type, index, &str);
>> +       if (err)
>> +               return err;
>> +
>>         debug("%s: '%s'\n", type, str);
>> +
>>         node = fdt_subnode_offset(fit, images, str);
>>         if (node < 0) {
>>                 debug("cannot find image node '%s': %d\n", str, node);
>> @@ -116,15 +146,15 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>>   * @info:      points to information about the device to load data from
>>   * @sector:    the start sector of the FIT image on the device
>>   * @fit:       points to the flattened device tree blob describing the FIT
>> - *             image
>> + *             image
>>   * @base_offset: the beginning of the data area containing the actual
>>   *             image data, relative to the beginning of the FIT
>>   * @node:      offset of the DT node describing the image to load (relative
>> - *             to @fit)
>> + *             to @fit)
>>   * @image_info:        will be filled with information about the loaded image
>> - *             If the FIT node does not contain a "load" (address) property,
>> - *             the image gets loaded to the address pointed to by the
>> - *             load_addr member in this struct.
>> + *             If the FIT node does not contain a "load" (address) property,
>> + *             the image gets loaded to the address pointed to by the
>> + *             load_addr member in this struct.
>>   *
>>   * Return:     0 on success or a negative error number.
>>   */
>> @@ -236,6 +266,35 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,
>>         image_info.load_addr = spl_image->load_addr + spl_image->size;
>>         ret = spl_load_fit_image(info, sector, fit, base_offset, node,
>>                                  &image_info);
>> +
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       /* Make the load-address of the FDT available for the SPL framework */
>> +       spl_image->fdt_addr = (void *)image_info.load_addr;
>> +       /* Try to make space, so we can inject details on the loadables */
>> +       ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
>> +
>> +       return ret;
>> +}
>> +
>> +static int spl_fit_record_loadable(const void *fit, int images, int index,
>> +                                  void *blob, struct spl_image_info *image)
>> +{
>> +       char *name;
>> +       int node, ret;
>> +
>> +       ret = spl_fit_get_image_name(fit, images, "loadables",
>> +                                    index, &name);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       node = spl_fit_get_image_node(fit, images, "loadables", index);
>> +
>> +       ret = fdt_record_loadable(blob, index, name, image->load_addr,
>> +                                 image->size, image->entry_point,
>> +                                 fdt_getprop(fit, node, "type", NULL),
>> +                                 fdt_getprop(fit, node, "os", NULL));
>>
>>
>> Calling this fdt_record_loadable is causing compilation issue when CONFIG_ARCH_FIXUP_FDT_MEMORY is disabled.
>>
>> common/spl/built-in.o: In function `spl_fit_record_loadable':
>> /mnt/disk/u-boot/common/spl/spl_fit.c:308: undefined reference to `fdt_record_loadable'
>> /mnt/disk/u-boot/common/spl/spl_fit.c:308:(.text.spl_load_simple_fit+0x2ec): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `fdt_record_loadable'
>> make[1]: *** [spl/u-boot-spl] Error 1
>>
>> You can see this issue with xilinx_zynqmp_zcu102_rev1_0_defconfig when you disable that option.
>> Just a note adding dependency to Kconfig is not a proper solution because for this combination I need fit image support.
>>
>> Can you please propose solution?
> 
> Thanks for the heads-up.
> 
> Sloppy editing on my part. The fdt_record_loadable function needs to be moved out of
> the #ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY guard in common/fdt_support.c.
> I’ll send a patch.

Will wait for a patch.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180118/206ddfa5/attachment.sig>

[U-Boot] [PATCH 06/15] spl: fit: implement recording of loadables into /fit-images

[Michal Simek]

On 18.1.2018 14:17, Dr. Philipp Tomsich wrote:
> Michal,
> 
>> On 18 Jan 2018, at 13:56, Michal Simek <monstr@monstr.eu> wrote:
>>
>> Hi Philipp,
>>
>>
>> 2017-09-13 21:29 GMT+02:00 Philipp Tomsich <philipp.tomsich@theobroma-systems.com>:
>> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
>> address and record information for all loadables into this FDT.  This
>> allows us to easily keep track of images for multiple privilege levels
>> (e.g. with ATF) or of firmware images preloaded into temporary
>> locations (e.g. PMU firmware that may overlap the SPL stage).
>>
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>>
>>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 81 insertions(+), 14 deletions(-)
>>
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index 9f05e1e..6dc0969 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -2,7 +2,7 @@
>>   * Copyright (C) 2016 Google, Inc
>>   * Written by Simon Glass <sjg@chromium.org>
>>   *
>> - * SPDX-License-Identifier:     GPL-2.0+
>> + * SPDX-License-Identifier:    GPL-2.0+
>>   */
>>
>>  #include <common.h>
>> @@ -16,22 +16,24 @@
>>  #endif
>>
>>  /**
>> - * spl_fit_get_image_node(): By using the matching configuration subnode,
>> + * spl_fit_get_image_name(): By using the matching configuration subnode,
>>   * retrieve the name of an image, specified by a property name and an index
>>   * into that.
>>   * @fit:       Pointer to the FDT blob.
>>   * @images:    Offset of the /images subnode.
>>   * @type:      Name of the property within the configuration subnode.
>>   * @index:     Index into the list of strings in this property.
>> + * @outname:   Name of the image
>>   *
>> - * Return:     the node offset of the respective image node or a negative
>> - *             error number.
>> + * Return:     0 on success, or a negative error number
>>   */
>> -static int spl_fit_get_image_node(const void *fit, int images,
>> -                                 const char *type, int index)
>> +static int spl_fit_get_image_name(const void *fit, int images,
>> +                                 const char *type, int index,
>> +                                 char **outname)
>>  {
>>         const char *name, *str;
>> -       int node, conf_node;
>> +       __maybe_unused int node;
>> +       int conf_node;
>>         int len, i;
>>
>>         conf_node = fit_find_config_node(fit);
>> @@ -63,7 +65,35 @@ static int spl_fit_get_image_node(const void *fit, int images,
>>                 }
>>         }
>>
>> +       *outname = (char *)str;
>> +       return 0;
>> +}
>> +
>> +/**
>> + * spl_fit_get_image_node(): By using the matching configuration subnode,
>> + * retrieve the name of an image, specified by a property name and an index
>> + * into that.
>> + * @fit:       Pointer to the FDT blob.
>> + * @images:    Offset of the /images subnode.
>> + * @type:      Name of the property within the configuration subnode.
>> + * @index:     Index into the list of strings in this property.
>> + *
>> + * Return:     the node offset of the respective image node or a negative
>> + *             error number.
>> + */
>> +static int spl_fit_get_image_node(const void *fit, int images,
>> +                                 const char *type, int index)
>> +{
>> +       char *str;
>> +       int err;
>> +       int node;
>> +
>> +       err = spl_fit_get_image_name(fit, images, type, index, &str);
>> +       if (err)
>> +               return err;
>> +
>>         debug("%s: '%s'\n", type, str);
>> +
>>         node = fdt_subnode_offset(fit, images, str);
>>         if (node < 0) {
>>                 debug("cannot find image node '%s': %d\n", str, node);
>> @@ -116,15 +146,15 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>>   * @info:      points to information about the device to load data from
>>   * @sector:    the start sector of the FIT image on the device
>>   * @fit:       points to the flattened device tree blob describing the FIT
>> - *             image
>> + *             image
>>   * @base_offset: the beginning of the data area containing the actual
>>   *             image data, relative to the beginning of the FIT
>>   * @node:      offset of the DT node describing the image to load (relative
>> - *             to @fit)
>> + *             to @fit)
>>   * @image_info:        will be filled with information about the loaded image
>> - *             If the FIT node does not contain a "load" (address) property,
>> - *             the image gets loaded to the address pointed to by the
>> - *             load_addr member in this struct.
>> + *             If the FIT node does not contain a "load" (address) property,
>> + *             the image gets loaded to the address pointed to by the
>> + *             load_addr member in this struct.
>>   *
>>   * Return:     0 on success or a negative error number.
>>   */
>> @@ -236,6 +266,35 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,
>>         image_info.load_addr = spl_image->load_addr + spl_image->size;
>>         ret = spl_load_fit_image(info, sector, fit, base_offset, node,
>>                                  &image_info);
>> +
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       /* Make the load-address of the FDT available for the SPL framework */
>> +       spl_image->fdt_addr = (void *)image_info.load_addr;
>> +       /* Try to make space, so we can inject details on the loadables */
>> +       ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
>> +
>> +       return ret;
>> +}
>> +
>> +static int spl_fit_record_loadable(const void *fit, int images, int index,
>> +                                  void *blob, struct spl_image_info *image)
>> +{
>> +       char *name;
>> +       int node, ret;
>> +
>> +       ret = spl_fit_get_image_name(fit, images, "loadables",
>> +                                    index, &name);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       node = spl_fit_get_image_node(fit, images, "loadables", index);
>> +
>> +       ret = fdt_record_loadable(blob, index, name, image->load_addr,
>> +                                 image->size, image->entry_point,
>> +                                 fdt_getprop(fit, node, "type", NULL),
>> +                                 fdt_getprop(fit, node, "os", NULL));
>>
>>
>> Calling this fdt_record_loadable is causing compilation issue when CONFIG_ARCH_FIXUP_FDT_MEMORY is disabled.
>>
>> common/spl/built-in.o: In function `spl_fit_record_loadable':
>> /mnt/disk/u-boot/common/spl/spl_fit.c:308: undefined reference to `fdt_record_loadable'
>> /mnt/disk/u-boot/common/spl/spl_fit.c:308:(.text.spl_load_simple_fit+0x2ec): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `fdt_record_loadable'
>> make[1]: *** [spl/u-boot-spl] Error 1
>>
>> You can see this issue with xilinx_zynqmp_zcu102_rev1_0_defconfig when you disable that option.
>> Just a note adding dependency to Kconfig is not a proper solution because for this combination I need fit image support.
>>
>> Can you please propose solution?
> 
> Thanks for the heads-up.
> 
> Sloppy editing on my part. The fdt_record_loadable function needs to be moved out of
> the #ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY guard in common/fdt_support.c.
> I’ll send a patch.

Have you sent this patch?

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180201/45a5102f/attachment.sig>

[U-Boot] [PATCH 06/15] spl: fit: implement recording of loadables into /fit-images

[Dr. Philipp Tomsich]

> On 1 Feb 2018, at 08:51, Michal Simek <monstr@monstr.eu> wrote:
> 
> On 18.1.2018 14:17, Dr. Philipp Tomsich wrote:
>> Michal,
>> 
>>> On 18 Jan 2018, at 13:56, Michal Simek <monstr@monstr.eu> wrote:
>>> 
>>> Hi Philipp,
>>> 
>>> 
>>> 2017-09-13 21:29 GMT+02:00 Philipp Tomsich <philipp.tomsich@theobroma-systems.com>:
>>> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
>>> address and record information for all loadables into this FDT.  This
>>> allows us to easily keep track of images for multiple privilege levels
>>> (e.g. with ATF) or of firmware images preloaded into temporary
>>> locations (e.g. PMU firmware that may overlap the SPL stage).
>>> 
>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>> ---
>>> 
>>> common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>>> 1 file changed, 81 insertions(+), 14 deletions(-)
>>> 
>>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>>> index 9f05e1e..6dc0969 100644
>>> --- a/common/spl/spl_fit.c
>>> +++ b/common/spl/spl_fit.c
>>> @@ -2,7 +2,7 @@
>>>  * Copyright (C) 2016 Google, Inc
>>>  * Written by Simon Glass <sjg@chromium.org>
>>>  *
>>> - * SPDX-License-Identifier:     GPL-2.0+
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>>  */
>>> 
>>> #include <common.h>
>>> @@ -16,22 +16,24 @@
>>> #endif
>>> 
>>> /**
>>> - * spl_fit_get_image_node(): By using the matching configuration subnode,
>>> + * spl_fit_get_image_name(): By using the matching configuration subnode,
>>>  * retrieve the name of an image, specified by a property name and an index
>>>  * into that.
>>>  * @fit:       Pointer to the FDT blob.
>>>  * @images:    Offset of the /images subnode.
>>>  * @type:      Name of the property within the configuration subnode.
>>>  * @index:     Index into the list of strings in this property.
>>> + * @outname:   Name of the image
>>>  *
>>> - * Return:     the node offset of the respective image node or a negative
>>> - *             error number.
>>> + * Return:     0 on success, or a negative error number
>>>  */
>>> -static int spl_fit_get_image_node(const void *fit, int images,
>>> -                                 const char *type, int index)
>>> +static int spl_fit_get_image_name(const void *fit, int images,
>>> +                                 const char *type, int index,
>>> +                                 char **outname)
>>> {
>>>        const char *name, *str;
>>> -       int node, conf_node;
>>> +       __maybe_unused int node;
>>> +       int conf_node;
>>>        int len, i;
>>> 
>>>        conf_node = fit_find_config_node(fit);
>>> @@ -63,7 +65,35 @@ static int spl_fit_get_image_node(const void *fit, int images,
>>>                }
>>>        }
>>> 
>>> +       *outname = (char *)str;
>>> +       return 0;
>>> +}
>>> +
>>> +/**
>>> + * spl_fit_get_image_node(): By using the matching configuration subnode,
>>> + * retrieve the name of an image, specified by a property name and an index
>>> + * into that.
>>> + * @fit:       Pointer to the FDT blob.
>>> + * @images:    Offset of the /images subnode.
>>> + * @type:      Name of the property within the configuration subnode.
>>> + * @index:     Index into the list of strings in this property.
>>> + *
>>> + * Return:     the node offset of the respective image node or a negative
>>> + *             error number.
>>> + */
>>> +static int spl_fit_get_image_node(const void *fit, int images,
>>> +                                 const char *type, int index)
>>> +{
>>> +       char *str;
>>> +       int err;
>>> +       int node;
>>> +
>>> +       err = spl_fit_get_image_name(fit, images, type, index, &str);
>>> +       if (err)
>>> +               return err;
>>> +
>>>        debug("%s: '%s'\n", type, str);
>>> +
>>>        node = fdt_subnode_offset(fit, images, str);
>>>        if (node < 0) {
>>>                debug("cannot find image node '%s': %d\n", str, node);
>>> @@ -116,15 +146,15 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>>>  * @info:      points to information about the device to load data from
>>>  * @sector:    the start sector of the FIT image on the device
>>>  * @fit:       points to the flattened device tree blob describing the FIT
>>> - *             image
>>> + *             image
>>>  * @base_offset: the beginning of the data area containing the actual
>>>  *             image data, relative to the beginning of the FIT
>>>  * @node:      offset of the DT node describing the image to load (relative
>>> - *             to @fit)
>>> + *             to @fit)
>>>  * @image_info:        will be filled with information about the loaded image
>>> - *             If the FIT node does not contain a "load" (address) property,
>>> - *             the image gets loaded to the address pointed to by the
>>> - *             load_addr member in this struct.
>>> + *             If the FIT node does not contain a "load" (address) property,
>>> + *             the image gets loaded to the address pointed to by the
>>> + *             load_addr member in this struct.
>>>  *
>>>  * Return:     0 on success or a negative error number.
>>>  */
>>> @@ -236,6 +266,35 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,
>>>        image_info.load_addr = spl_image->load_addr + spl_image->size;
>>>        ret = spl_load_fit_image(info, sector, fit, base_offset, node,
>>>                                 &image_info);
>>> +
>>> +       if (ret < 0)
>>> +               return ret;
>>> +
>>> +       /* Make the load-address of the FDT available for the SPL framework */
>>> +       spl_image->fdt_addr = (void *)image_info.load_addr;
>>> +       /* Try to make space, so we can inject details on the loadables */
>>> +       ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
>>> +
>>> +       return ret;
>>> +}
>>> +
>>> +static int spl_fit_record_loadable(const void *fit, int images, int index,
>>> +                                  void *blob, struct spl_image_info *image)
>>> +{
>>> +       char *name;
>>> +       int node, ret;
>>> +
>>> +       ret = spl_fit_get_image_name(fit, images, "loadables",
>>> +                                    index, &name);
>>> +       if (ret < 0)
>>> +               return ret;
>>> +
>>> +       node = spl_fit_get_image_node(fit, images, "loadables", index);
>>> +
>>> +       ret = fdt_record_loadable(blob, index, name, image->load_addr,
>>> +                                 image->size, image->entry_point,
>>> +                                 fdt_getprop(fit, node, "type", NULL),
>>> +                                 fdt_getprop(fit, node, "os", NULL));
>>> 
>>> 
>>> Calling this fdt_record_loadable is causing compilation issue when CONFIG_ARCH_FIXUP_FDT_MEMORY is disabled.
>>> 
>>> common/spl/built-in.o: In function `spl_fit_record_loadable':
>>> /mnt/disk/u-boot/common/spl/spl_fit.c:308: undefined reference to `fdt_record_loadable'
>>> /mnt/disk/u-boot/common/spl/spl_fit.c:308:(.text.spl_load_simple_fit+0x2ec): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `fdt_record_loadable'
>>> make[1]: *** [spl/u-boot-spl] Error 1
>>> 
>>> You can see this issue with xilinx_zynqmp_zcu102_rev1_0_defconfig when you disable that option.
>>> Just a note adding dependency to Kconfig is not a proper solution because for this combination I need fit image support.
>>> 
>>> Can you please propose solution?
>> 
>> Thanks for the heads-up.
>> 
>> Sloppy editing on my part. The fdt_record_loadable function needs to be moved out of
>> the #ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY guard in common/fdt_support.c.
>> I’ll send a patch.
> 
> Have you sent this patch?

This had fallen off my radar.
The patch is on the list now.

Regards,
Philipp.