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

[PATCH v3 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 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/spl/spl_atf.c     |  7 ++--
 common/spl/spl_fit.c     |  6 ++-
 common/spl/spl_opensbi.c |  8 ++--
 doc/uImage.FIT/howto.txt | 84 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 98 insertions(+), 16 deletions(-)

-- 
2.28.0

[PATCH v3 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 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     |  6 ++-
 common/spl/spl_opensbi.c |  8 ++--
 doc/uImage.FIT/howto.txt | 84 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 98 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..aeb8aeda2b19 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -332,9 +332,13 @@ 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;
 	}
 
 	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 v3 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>
---

(no changes since v1)

 common/fdt_support.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 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);
-- 
2.28.0

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

[Simon Glass]

On Mon, 12 Oct 2020 at 01:51, Michal Simek <michal.simek@xilinx.com> wrote:
>
> 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>
> ---
>
> (no changes since v1)
>
>  common/fdt_support.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)

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

I am wondering whether we should use #size-cells etc. to select this?

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

[Michal Simek]

On 15. 10. 20 17:05, Simon Glass wrote:
> On Mon, 12 Oct 2020 at 01:51, Michal Simek <michal.simek@xilinx.com> wrote:
>>
>> 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>
>> ---
>>
>> (no changes since v1)
>>
>>  common/fdt_support.c | 9 ++-------
>>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> I am wondering whether we should use #size-cells etc. to select this?
> 

size-cells/address-cells are used for reg property. We are not using it
and it is really just property with two cells which are pointing to
address. That's why I don't think we need this property there.

Thanks,
Michal