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

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

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

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

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?

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
participants (2)
-
Michal Simek
-
Simon Glass