[PATCH v4 0/2] Add support for loading images above 4GB

[PATCH v4 0/2] Add support for loading images above 4GB

[Michal Simek]

Hi,

We have several use cases where customers want to partition memory by
putting NS images above 4GB. On Xilinx arm 64bit SOC 0-2GB can be used for
others CPU in the systems (like R5) or for secure sw.
Currently there is limitation in SPL to record load/entry addresses in
64bit format because they are recorded in 32bit only.
This series add support for it.
Patches have been tested on Xilinx ZynqMP zcu102 board in SD bootmode with
images generated by binman. Because u-boot is using
CONFIG_POSITION_INDEPENDENT it can be put to others 4k aligned addresses
and there is no real need to build it to certain offset.

Thanks,
Michal

Changes in v4:
- In case of missing entry point initialize image_info->entry_point to
  FDT_ERROR. This has been seen on RISCV instances. FDT_ERROR handling
  should be changed separately because entry_point is uintptr_t and there
  is no way how to pass error conditions that's why FDT_ERROR is used
  instead.
- RISCV32_SPL reports issue that two cells format (64bit value) is
  unsupported address size. But that's not accurate because two cell format
  is valid on any 32bit platform. What it is not supported is address above
  4GB. That's why code is fixed by reading value to 64bit type first and then
  check if upper 32bits are zero or not. On all 32bit platforms upper bits
  should be 0 and if not, message is shown.

Changes in v3:
- Change example to have 64bit addresses for u-boot
- Add reviewed-by from Simon

Changes in v2:
- Also fix opensbi
- Add record to doc/uImage.FIT/howto.txt - reported by Simon

Michal Simek (2):
  spl: Use standard FIT entries
  spl: fdt: Record load/entry fit-images entries in 64bit format

 common/fdt_support.c     |  9 +----
 common/image-fit.c       | 11 +++---
 common/spl/spl_atf.c     |  7 ++--
 common/spl/spl_fit.c     |  8 +++-
 common/spl/spl_opensbi.c |  8 ++--
 doc/uImage.FIT/howto.txt | 84 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 106 insertions(+), 21 deletions(-)

-- 
2.28.0

[PATCH v4 1/2] spl: Use standard FIT entries

[Michal Simek]

SPL is creating fit-images DT node when loadables are recorded in selected
configuration. Entries which are created are using entry-point and
load-addr property names. But there shouldn't be a need to use non standard
properties because entry/load are standard FIT properties. But using
standard FIT properties enables option to use generic FIT functions to
descrease SPL size. Here is result for ZynqMP virt configuration:
xilinx_zynqmp_virt: spl/u-boot-spl:all -82 spl/u-boot-spl:rodata -22 spl/u-boot-spl:text -60

The patch causes change in run time fit image record.
Before:
fit-images {
        uboot {
                os = "u-boot";
                type = "firmware";
                size = <0xfd520>;
                entry-point = <0x8000000>;
                load-addr = <0x8000000>;
        };
};

After:
fit-images {
        uboot {
                os = "u-boot";
                type = "firmware";
                size = <0xfd520>;
                entry = <0x8000000>;
                load = <0x8000000>;
        };
};

Replacing calling fdt_getprop_u32() by fit_image_get_entry/load() also
enables support for reading entry/load properties recorded in 64bit format.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v4:
- In case of missing entry point initialize image_info->entry_point to
  FDT_ERROR. This has been seen on RISCV instances. FDT_ERROR handling
  should be changed separately because entry_point is uintptr_t and there
  is no way how to pass error conditions that's why FDT_ERROR is used
  instead.

Changes in v3:
- Change example to have 64bit addresses for u-boot
- Add reviewed-by from Simon

Changes in v2:
- Also fix opensbi
- Add record to doc/uImage.FIT/howto.txt - reported by Simon

Would be good to know history of fit-images and it's property names but
there shouldn't be a need to use non standard names where we have
FIT_*_PROP recorded as macros in include/image.h.

Concern regarding backward compatibility is definitely valid but not sure
how many systems can be affected by this change.

Adding temporary support for entry-point/load-addr is also possible.
Or second way around is to create new wrappers as
fit_image_get_entry_point()/fit_image_get_load_addr() or
call fit_image_get_address() directly.

---
 common/fdt_support.c     |  4 +-
 common/spl/spl_atf.c     |  7 ++--
 common/spl/spl_fit.c     |  8 +++-
 common/spl/spl_opensbi.c |  8 ++--
 doc/uImage.FIT/howto.txt | 84 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 100 insertions(+), 11 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index a565b470f81e..b8a8768a2147 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -616,9 +616,9 @@ int fdt_record_loadable(void *blob, u32 index, const char *name,
 	 * 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);
+	fdt_setprop_u32(blob, node, "load", load_addr);
 	if (entry_point != -1)
-		fdt_setprop_u32(blob, node, "entry-point", entry_point);
+		fdt_setprop_u32(blob, node, "entry", entry_point);
 	fdt_setprop_u32(blob, node, "size", size);
 	if (type)
 		fdt_setprop_string(blob, node, "type", type);
diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
index b54b4f0d22e2..9bd25f6b32c1 100644
--- a/common/spl/spl_atf.c
+++ b/common/spl/spl_atf.c
@@ -132,10 +132,11 @@ static int spl_fit_images_find(void *blob, int os)
 uintptr_t spl_fit_images_get_entry(void *blob, int node)
 {
 	ulong  val;
+	int ret;
 
-	val = fdt_getprop_u32(blob, node, "entry-point");
-	if (val == FDT_ERROR)
-		val = fdt_getprop_u32(blob, node, "load-addr");
+	ret = fit_image_get_entry(blob, node, &val);
+	if (ret)
+		ret = fit_image_get_load(blob, node, &val);
 
 	debug("%s: entry point 0x%lx\n", __func__, val);
 	return val;
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 0e27ad1d6a55..3cbaa840dd4f 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -332,9 +332,15 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
 	}
 
 	if (image_info) {
+		ulong entry_point;
+
 		image_info->load_addr = load_addr;
 		image_info->size = length;
-		image_info->entry_point = fdt_getprop_u32(fit, node, "entry");
+
+		if (!fit_image_get_entry(fit, node, &entry_point))
+			image_info->entry_point = entry_point;
+		else
+			image_info->entry_point = FDT_ERROR;
 	}
 
 	return 0;
diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
index 14f335f75f02..41e0746bb012 100644
--- a/common/spl/spl_opensbi.c
+++ b/common/spl/spl_opensbi.c
@@ -61,11 +61,9 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image)
 	}
 
 	/* Get U-Boot entry point */
-	uboot_entry = fdt_getprop_u32(spl_image->fdt_addr, uboot_node,
-				      "entry-point");
-	if (uboot_entry == FDT_ERROR)
-		uboot_entry = fdt_getprop_u32(spl_image->fdt_addr, uboot_node,
-					      "load-addr");
+	ret = fit_image_get_entry(spl_image->fdt_addr, uboot_node, &uboot_entry);
+	if (ret)
+		ret = fit_image_get_load(spl_image->fdt_addr, uboot_node, &uboot_entry);
 
 	/* Prepare obensbi_info object */
 	opensbi_info.magic = FW_DYNAMIC_INFO_MAGIC_VALUE;
diff --git a/doc/uImage.FIT/howto.txt b/doc/uImage.FIT/howto.txt
index 8592719685eb..019dda24a081 100644
--- a/doc/uImage.FIT/howto.txt
+++ b/doc/uImage.FIT/howto.txt
@@ -66,6 +66,90 @@ can point to a script which generates this image source file during
 the build process. It gets passed a list of device tree files (taken from the
 CONFIG_OF_LIST symbol).
 
+The SPL also records to a DT all additional images (called loadables) which are
+loaded. The information about loadables locations is passed via the DT node with
+fit-images name.
+
+Loadables Example
+-----------------
+Consider the following case for an ARM64 platform where U-Boot runs in EL2
+started by ATF where SPL is loading U-Boot (as loadables) and ATF (as firmware).
+
+/dts-v1/;
+
+/ {
+	description = "Configuration to load ATF before U-Boot";
+
+	images {
+		uboot {
+			description = "U-Boot (64-bit)";
+			data = /incbin/("u-boot-nodtb.bin");
+			type = "firmware";
+			os = "u-boot";
+			arch = "arm64";
+			compression = "none";
+			load = <0x8 0x8000000>;
+			entry = <0x8 0x8000000>;
+			hash {
+				algo = "md5";
+			};
+		};
+		atf {
+			description = "ARM Trusted Firmware";
+			data = /incbin/("bl31.bin");
+			type = "firmware";
+			os = "arm-trusted-firmware";
+			arch = "arm64";
+			compression = "none";
+			load = <0xfffea000>;
+			entry = <0xfffea000>;
+			hash {
+				algo = "md5";
+			};
+		};
+		fdt_1 {
+			description = "zynqmp-zcu102-revA";
+			data = /incbin/("arch/arm/dts/zynqmp-zcu102-revA.dtb");
+			type = "flat_dt";
+			arch = "arm64";
+			compression = "none";
+			load = <0x100000>;
+			hash {
+				algo = "md5";
+			};
+		};
+	};
+	configurations {
+		default = "config_1";
+
+		config_1 {
+			description = "zynqmp-zcu102-revA";
+			firmware = "atf";
+			loadables = "uboot";
+			fdt = "fdt_1";
+		};
+	};
+};
+
+In this case the SPL records via fit-images DT node the information about
+loadables U-Boot image.
+
+ZynqMP> fdt addr $fdtcontroladdr
+ZynqMP> fdt print /fit-images
+fit-images {
+	uboot {
+		os = "u-boot";
+		type = "firmware";
+		size = <0x001017c8>;
+		entry = <0x00000008 0x08000000>;
+		load = <0x00000008 0x08000000>;
+	};
+};
+
+As you can see entry and load properties are 64bit wide to support loading
+images above 4GB (in past entry and load properties where just 32bit).
+
+
 Example 1 -- old-style (non-FDT) kernel booting
 -----------------------------------------------
 
-- 
2.28.0

[PATCH v4 2/2] spl: fdt: Record load/entry fit-images entries in 64bit format

[Michal Simek]

The commit 9f45aeb93727 ("spl: fit: implement fdt_record_loadable") which
introduced fdt_record_loadable() state there spl_fit.c is not 64bit safe.
Based on my tests on Xilinx ZynqMP zcu102 platform there shouldn't be a
problem to record these addresses in 64bit format.
The patch adds support for systems which need to load images above 4GB.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v4:
- RISCV32_SPL reports issue that two cells format (64bit value) is
  unsupported address size. But that's not accurate because two cell format
  is valid on any 32bit platform. What it is not supported is address above
  4GB. That's why code is fixed by reading value to 64bit type first and then
  check if upper 32bits are zero or not. On all 32bit platforms upper bits
  should be 0 and if not, message is shown.

 common/fdt_support.c |  9 ++-------
 common/image-fit.c   | 11 ++++++-----
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index b8a8768a2147..5ae75df3c658 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -611,14 +611,9 @@ int fdt_record_loadable(void *blob, u32 index, const char *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", load_addr);
+	fdt_setprop_u64(blob, node, "load", load_addr);
 	if (entry_point != -1)
-		fdt_setprop_u32(blob, node, "entry", entry_point);
+		fdt_setprop_u64(blob, node, "entry", entry_point);
 	fdt_setprop_u32(blob, node, "size", size);
 	if (type)
 		fdt_setprop_string(blob, node, "type", type);
diff --git a/common/image-fit.c b/common/image-fit.c
index d54eff9033cc..c82d4d8015f0 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -791,17 +791,18 @@ static int fit_image_get_address(const void *fit, int noffset, char *name,
 		return -1;
 	}
 
-	if (len > sizeof(ulong)) {
-		printf("Unsupported %s address size\n", name);
-		return -1;
-	}
-
 	cell_len = len >> 2;
 	/* Use load64 to avoid compiling warning for 32-bit target */
 	while (cell_len--) {
 		load64 = (load64 << 32) | uimage_to_cpu(*cell);
 		cell++;
 	}
+
+	if (len > sizeof(ulong) && (uint32_t)(load64 >> 32)) {
+		printf("Unsupported %s address size\n", name);
+		return -1;
+	}
+
 	*load = (ulong)load64;
 
 	return 0;
-- 
2.28.0

[PATCH v4 0/2] Add support for loading images above 4GB

[Michal Simek]

po 19. 10. 2020 v 16:35 odes?latel Michal Simek
<michal.simek@xilinx.com> napsal:
>
> Hi,
>
> We have several use cases where customers want to partition memory by
> putting NS images above 4GB. On Xilinx arm 64bit SOC 0-2GB can be used for
> others CPU in the systems (like R5) or for secure sw.
> Currently there is limitation in SPL to record load/entry addresses in
> 64bit format because they are recorded in 32bit only.
> This series add support for it.
> Patches have been tested on Xilinx ZynqMP zcu102 board in SD bootmode with
> images generated by binman. Because u-boot is using
> CONFIG_POSITION_INDEPENDENT it can be put to others 4k aligned addresses
> and there is no real need to build it to certain offset.
>
> Thanks,
> Michal
>
> Changes in v4:
> - In case of missing entry point initialize image_info->entry_point to
>   FDT_ERROR. This has been seen on RISCV instances. FDT_ERROR handling
>   should be changed separately because entry_point is uintptr_t and there
>   is no way how to pass error conditions that's why FDT_ERROR is used
>   instead.
> - RISCV32_SPL reports issue that two cells format (64bit value) is
>   unsupported address size. But that's not accurate because two cell format
>   is valid on any 32bit platform. What it is not supported is address above
>   4GB. That's why code is fixed by reading value to 64bit type first and then
>   check if upper 32bits are zero or not. On all 32bit platforms upper bits
>   should be 0 and if not, message is shown.
>
> Changes in v3:
> - Change example to have 64bit addresses for u-boot
> - Add reviewed-by from Simon
>
> Changes in v2:
> - Also fix opensbi
> - Add record to doc/uImage.FIT/howto.txt - reported by Simon
>
> Michal Simek (2):
>   spl: Use standard FIT entries
>   spl: fdt: Record load/entry fit-images entries in 64bit format
>
>  common/fdt_support.c     |  9 +----
>  common/image-fit.c       | 11 +++---
>  common/spl/spl_atf.c     |  7 ++--
>  common/spl/spl_fit.c     |  8 +++-
>  common/spl/spl_opensbi.c |  8 ++--
>  doc/uImage.FIT/howto.txt | 84 ++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 106 insertions(+), 21 deletions(-)
>
> --
> 2.28.0
>

Applied.
M

-- 
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/Versal SoCs