[PATCH RFC] image: apply FDTOs on FDT image node without a load property
From: Quentin Schulz quentin.schulz@cherry.de
A FIT image which is NOT using -E when created by mkimage - that is with image data within the FIT - will fail to apply FDTO if the base FDT image node does not specify a load property (which points to an address in DRAM). This is because we check that the FDT address we want to apply overlay to (i.e. modify and likely increase in size) is not inside the FIT and give up otherwise. This is assumed necessary because we may then overwrite other data when applying in-place.
However, we can do better than giving up: relocating the FDT in another place in DRAM where it's safe to increase its size and apply FDTOs.
Suggested-by: Marek Vasut marex@denx.de Signed-off-by: Quentin Schulz quentin.schulz@cherry.de --- Tested with extlinux with: - conf node with multiple fdt entries - conf node with base fdt + conf node with fdto (stored in fdt property) - FDT image node with load property (behavior unchanged) - FDT image node without load property
I'll be frank, I don't really know what I'm doing here and I'm on holidays for a month in a few hours, so take this with a bigger grain of salt than usual (hence the RFC). It works (tm) for now.
Cc: Heiko Stuebner heiko@sntech.de --- boot/image-fit.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/boot/image-fit.c b/boot/image-fit.c index 7d56f0b5e6ed39a75e029b09dc95d605835c0f91..088482c4d5d2e194cce66f10b0acd9e586f826b2 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -2392,13 +2392,18 @@ int boot_get_fdt_fit(struct bootm_headers *images, ulong addr, image_end = addr + fit_get_size(fit); /* verify that relocation took place by load address not being in fit */ if (load >= image_start && load < image_end) { - /* check is simplified; fit load checks for overlaps */ - printf("Overlayed FDT requires relocation\n"); - fdt_noffset = -EBADF; - goto out; - } + /* Cannot overlay an FDT stored inside the fitImage, so let's relocate */ + char *of_flat_tree = map_sysmem(load, len);
- base = map_sysmem(load, len); + len = ALIGN(fdt_totalsize(load), SZ_4K); + if (boot_relocate_fdt(&of_flat_tree, &len)) { + printf("Required FDT relocation for applying overlays failed..."); + fdt_noffset = -EBADF; + goto out; + } + + load = (ulong)of_flat_tree; + }
/* apply extra configs in FIT first, followed by args */ for (i = 1; ; i++) {
--- base-commit: 69bd83568c57813cd23bc2d100c066a17e7e349d change-id: 20241219-extlinux-relocate-dtb-when-dtbo-4c7536f880b2
Best regards,
On 12/19/24 4:03 PM, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@cherry.de
A FIT image which is NOT using -E when created by mkimage - that is with image data within the FIT - will fail to apply FDTO if the base FDT image node does not specify a load property (which points to an address in DRAM). This is because we check that the FDT address we want to apply overlay to (i.e. modify and likely increase in size) is not inside the FIT and give up otherwise. This is assumed necessary because we may then overwrite other data when applying in-place.
However, we can do better than giving up: relocating the FDT in another place in DRAM where it's safe to increase its size and apply FDTOs.
Suggested-by: Marek Vasut marex@denx.de Signed-off-by: Quentin Schulz quentin.schulz@cherry.de
Tested with extlinux with:
- conf node with multiple fdt entries
- conf node with base fdt + conf node with fdto (stored in fdt property)
- FDT image node with load property (behavior unchanged)
- FDT image node without load property
I'll be frank, I don't really know what I'm doing here and I'm on holidays for a month in a few hours, so take this with a bigger grain of salt than usual (hence the RFC). It works (tm) for now.
Cc: Heiko Stuebner heiko@sntech.de
boot/image-fit.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/boot/image-fit.c b/boot/image-fit.c index 7d56f0b5e6ed39a75e029b09dc95d605835c0f91..088482c4d5d2e194cce66f10b0acd9e586f826b2 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -2392,13 +2392,18 @@ int boot_get_fdt_fit(struct bootm_headers *images, ulong addr, image_end = addr + fit_get_size(fit); /* verify that relocation took place by load address not being in fit */ if (load >= image_start && load < image_end) {
This has to be updated to handle the mkimage -E case too. Basically, if the FDT is outside of the fitImage tree area, but still in the trailing blob area at the end of fitImage generated with mkimage -E, then any modification to the DT which would result in resize may either fail or corrupt the surrounding blobs.
The safe but heavy handed option would be to relocate unconditionally .
/* check is simplified; fit load checks for overlaps */printf("Overlayed FDT requires relocation\n");fdt_noffset = -EBADF;goto out;- }
/* Cannot overlay an FDT stored inside the fitImage, so let's relocate */char *of_flat_tree = map_sysmem(load, len);
- base = map_sysmem(load, len);
len = ALIGN(fdt_totalsize(load), SZ_4K);if (boot_relocate_fdt(&of_flat_tree, &len)) {
Please print the error value in case of failure here, it is useful for debugging.
printf("Required FDT relocation for applying overlays failed...");fdt_noffset = -EBADF;goto out;}load = (ulong)of_flat_tree;- }
Hi Marek,
On 12/19/24 7:26 PM, Marek Vasut wrote:
On 12/19/24 4:03 PM, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@cherry.de
A FIT image which is NOT using -E when created by mkimage - that is with image data within the FIT - will fail to apply FDTO if the base FDT image node does not specify a load property (which points to an address in DRAM). This is because we check that the FDT address we want to apply overlay to (i.e. modify and likely increase in size) is not inside the FIT and give up otherwise. This is assumed necessary because we may then overwrite other data when applying in-place.
However, we can do better than giving up: relocating the FDT in another place in DRAM where it's safe to increase its size and apply FDTOs.
Suggested-by: Marek Vasut marex@denx.de Signed-off-by: Quentin Schulz quentin.schulz@cherry.de
Tested with extlinux with:
- conf node with multiple fdt entries
- conf node with base fdt + conf node with fdto (stored in fdt property)
- FDT image node with load property (behavior unchanged)
- FDT image node without load property
I'll be frank, I don't really know what I'm doing here and I'm on holidays for a month in a few hours, so take this with a bigger grain of salt than usual (hence the RFC). It works (tm) for now.
Cc: Heiko Stuebner heiko@sntech.de
boot/image-fit.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/boot/image-fit.c b/boot/image-fit.c index 7d56f0b5e6ed39a75e029b09dc95d605835c0f91..088482c4d5d2e194cce66f10b0acd9e586f826b2 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -2392,13 +2392,18 @@ int boot_get_fdt_fit(struct bootm_headers *images, ulong addr, image_end = addr + fit_get_size(fit); /* verify that relocation took place by load address not being in fit */ if (load >= image_start && load < image_end) {
This has to be updated to handle the mkimage -E case too. Basically, if the FDT is outside of the fitImage tree area, but still in the trailing blob area at the end of fitImage generated with mkimage -E, then any modification to the DT which would result in resize may either fail or corrupt the surrounding blobs.
The safe but heavy handed option would be to relocate unconditionally .
Will do.
- /* check is simplified; fit load checks for overlaps */ - printf("Overlayed FDT requires relocation\n"); - fdt_noffset = -EBADF; - goto out; - } + /* Cannot overlay an FDT stored inside the fitImage, so let's relocate */ + char *of_flat_tree = map_sysmem(load, len); - base = map_sysmem(load, len); + len = ALIGN(fdt_totalsize(load), SZ_4K); + if (boot_relocate_fdt(&of_flat_tree, &len)) {
Please print the error value in case of failure here, it is useful for debugging.
The function returns 0 on success, 1 on any error, not sure it's that useful as it'll always be 1. Do we want to anticipate this function being updated to have other error codes in the future?
Cheers, Quentin
On 1/14/25 12:04 PM, Quentin Schulz wrote: [...]
This has to be updated to handle the mkimage -E case too. Basically, if the FDT is outside of the fitImage tree area, but still in the trailing blob area at the end of fitImage generated with mkimage -E, then any modification to the DT which would result in resize may either fail or corrupt the surrounding blobs.
The safe but heavy handed option would be to relocate unconditionally .
Will do.
Thank you
- /* check is simplified; fit load checks for overlaps */ - printf("Overlayed FDT requires relocation\n"); - fdt_noffset = -EBADF; - goto out; - } + /* Cannot overlay an FDT stored inside the fitImage, so let's relocate */ + char *of_flat_tree = map_sysmem(load, len); - base = map_sysmem(load, len); + len = ALIGN(fdt_totalsize(load), SZ_4K); + if (boot_relocate_fdt(&of_flat_tree, &len)) {
Please print the error value in case of failure here, it is useful for debugging.
The function returns 0 on success, 1 on any error, not sure it's that useful as it'll always be 1. Do we want to anticipate this function being updated to have other error codes in the future?
The other functions in boot/image-fdt.c already return proper error codes, so it is possible this one will be updated to return them too, eventually.
participants (3)
-
Marek Vasut -
Quentin Schulz -
Quentin Schulz