From: Tom Rini Date: Wed, 24 Sep 2025 13:50:44 +0000 (-0600) Subject: Revert "Merge patch series "mkimage: Detect FIT image load address overlaps and fix... X-Git-Url: https://git.openpandora.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e482fdbbca935de32400054eb532de45b1cc01cb;p=pandora-u-boot.git Revert "Merge patch series "mkimage: Detect FIT image load address overlaps and fix related test/DTS issues"" This reverts commit 4d84fa1261eb27d57687f2e4c404a78b8653c183, reversing changes made to b82a1fa7ddc7f3be2f3b75898d5dc44c34420bdd. I had missed some feedback on this series from earlier, and we have since had reports of regressions due to this as well. For now, revert this. Signed-off-by: Tom Rini --- diff --git a/arch/arm/dts/k3-am625-phycore-som-binman.dtsi b/arch/arm/dts/k3-am625-phycore-som-binman.dtsi index 4344cefeba3..a9bd5a2be84 100644 --- a/arch/arm/dts/k3-am625-phycore-som-binman.dtsi +++ b/arch/arm/dts/k3-am625-phycore-som-binman.dtsi @@ -234,8 +234,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-fs"; - load = <0x9dc10000>; - entry = <0x9dc10000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_fs"; }; @@ -247,8 +247,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-gp"; - load = <0x9dc20000>; - entry = <0x9dc20000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_gp"; }; @@ -322,7 +322,7 @@ description = "k3-am6xx-phycore-disable-spi-nor"; type = "flat_dt"; compression = "none"; - load = <0x8F002000>; + load = <0x8F001000>; arch = "arm"; ti-secure { content = <&am6xx_phycore_disable_spi_not_dtbo>; @@ -337,7 +337,7 @@ description = "k3-am6xx-phycore-disable-eth-phy"; type = "flat_dt"; compression = "none"; - load = <0x8F004000>; + load = <0x8F002000>; arch = "arm"; ti-secure { content = <&am6xx_phycore_disable_eth_phy_dtbo>; @@ -352,7 +352,7 @@ description = "k3-am6xx-phycore-qspi-nor"; type = "flat_dt"; compression = "none"; - load = <0x8F006000>; + load = <0x8F003000>; arch = "arm"; ti-secure { content = <&am6xx_phycore_disable_qspi_nor_dtbo>; @@ -479,8 +479,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-fs"; - load = <0x9dc10000>; - entry = <0x9dc10000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_fs"; }; @@ -492,8 +492,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-gp"; - load = <0x9dc20000>; - entry = <0x9dc20000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_gp"; }; diff --git a/arch/arm/dts/k3-am625-sk-binman.dtsi b/arch/arm/dts/k3-am625-sk-binman.dtsi index 1619f733a0d..f743c4353b4 100644 --- a/arch/arm/dts/k3-am625-sk-binman.dtsi +++ b/arch/arm/dts/k3-am625-sk-binman.dtsi @@ -231,8 +231,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-fs"; - load = <0x9dc10000>; - entry = <0x9dc10000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_fs"; }; @@ -244,8 +244,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-gp"; - load = <0x9dc20000>; - entry = <0x9dc20000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_gp"; }; @@ -362,8 +362,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-fs"; - load = <0x9dc10000>; - entry = <0x9dc10000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_fs"; }; @@ -375,8 +375,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-gp"; - load = <0x9dc20000>; - entry = <0x9dc20000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_gp"; }; diff --git a/arch/arm/dts/k3-am625-verdin-wifi-dev-binman.dtsi b/arch/arm/dts/k3-am625-verdin-wifi-dev-binman.dtsi index 6c4ad72d936..65fef6e4790 100644 --- a/arch/arm/dts/k3-am625-verdin-wifi-dev-binman.dtsi +++ b/arch/arm/dts/k3-am625-verdin-wifi-dev-binman.dtsi @@ -219,8 +219,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-fs"; - load = <0x9dc10000>; - entry = <0x9dc10000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_fs"; }; @@ -232,8 +232,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-gp"; - load = <0x9dc20000>; - entry = <0x9dc20000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_gp"; }; @@ -346,8 +346,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-fs"; - load = <0x9dc10000>; - entry = <0x9dc10000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_fs"; }; @@ -359,8 +359,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-gp"; - load = <0x9dc20000>; - entry = <0x9dc20000>; + load = <0x9dc00000>; + entry = <0x9dc00000>; blob-ext { filename = "tifsstub.bin_gp"; }; diff --git a/arch/arm/dts/k3-am62a-phycore-som-binman.dtsi b/arch/arm/dts/k3-am62a-phycore-som-binman.dtsi index 786c7a2d458..a284226320c 100644 --- a/arch/arm/dts/k3-am62a-phycore-som-binman.dtsi +++ b/arch/arm/dts/k3-am62a-phycore-som-binman.dtsi @@ -184,8 +184,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-fs"; - load = <0x9ca10000>; - entry = <0x9ca10000>; + load = <0x9ca00000>; + entry = <0x9ca00000>; blob-ext { filename = "tifsstub.bin_fs"; }; @@ -260,7 +260,7 @@ description = "k3-am6xx-phycore-disable-spi-nor"; type = "flat_dt"; compression = "none"; - load = <0x8F002000>; + load = <0x8F001000>; arch = "arm"; ti-secure { content = <&am6xx_phycore_disable_spi_not_dtbo>; @@ -275,7 +275,7 @@ description = "k3-am6xx-phycore-disable-eth-phy"; type = "flat_dt"; compression = "none"; - load = <0x8F004000>; + load = <0x8F002000>; arch = "arm"; ti-secure { content = <&am6xx_phycore_disable_eth_phy_dtbo>; @@ -290,7 +290,7 @@ description = "k3-am6xx-phycore-qspi-nor"; type = "flat_dt"; compression = "none"; - load = <0x8F006000>; + load = <0x8F003000>; arch = "arm"; ti-secure { content = <&am6xx_phycore_disable_qspi_nor_dtbo>; diff --git a/arch/arm/dts/k3-am62a-sk-binman.dtsi b/arch/arm/dts/k3-am62a-sk-binman.dtsi index 214acd7f0f7..e64c165ecbf 100644 --- a/arch/arm/dts/k3-am62a-sk-binman.dtsi +++ b/arch/arm/dts/k3-am62a-sk-binman.dtsi @@ -168,8 +168,8 @@ arch = "arm32"; compression = "none"; os = "tifsstub-fs"; - load = <0x9ca10000>; - entry = <0x9ca10000>; + load = <0x9ca00000>; + entry = <0x9ca00000>; blob-ext { filename = "tifsstub.bin_fs"; }; diff --git a/arch/arm/dts/k3-am642-phycore-som-binman.dtsi b/arch/arm/dts/k3-am642-phycore-som-binman.dtsi index 59d8902bf48..966905bd64d 100644 --- a/arch/arm/dts/k3-am642-phycore-som-binman.dtsi +++ b/arch/arm/dts/k3-am642-phycore-som-binman.dtsi @@ -371,7 +371,7 @@ description = "k3-am6xx-phycore-disable-spi-nor"; type = "flat_dt"; compression = "none"; - load = <0x8F002000>; + load = <0x8F001000>; arch = "arm"; ti-secure { content = <&am6xx_phycore_disable_spi_not_dtbo>; @@ -386,7 +386,7 @@ description = "k3-am6xx-phycore-disable-eth-phy"; type = "flat_dt"; compression = "none"; - load = <0x8F004000>; + load = <0x8F002000>; arch = "arm"; ti-secure { content = <&am6xx_phycore_disable_eth_phy_dtbo>; @@ -401,7 +401,7 @@ description = "k3-am6xx-phycore-qspi-nor"; type = "flat_dt"; compression = "none"; - load = <0x8F006000>; + load = <0x8F003000>; arch = "arm"; ti-secure { content = <&am6xx_phycore_disable_qspi_nor_dtbo>; diff --git a/test/py/tests/test_fit_mkimage_validate.py b/test/py/tests/test_fit_mkimage_validate.py index 27299a58f33..170b2a8cbbb 100644 --- a/test/py/tests/test_fit_mkimage_validate.py +++ b/test/py/tests/test_fit_mkimage_validate.py @@ -103,68 +103,3 @@ def test_fit_invalid_default_config(ubman): assert result.returncode != 0, "mkimage should fail due to missing default config" assert re.search(r"Default configuration '.*' not found under /configurations", result.stderr) - -def test_fit_load_addr_overlap(ubman): - """Test that mkimage fails when load address overlap""" - - its_fname = fit_util.make_fname(ubman, "invalid.its") - itb_fname = fit_util.make_fname(ubman, "invalid.itb") - kernel = fit_util.make_kernel(ubman, 'kernel.bin', 'kernel') - fdt = fit_util.make_dtb(ubman, ''' -/dts-v1/; -/ { - model = "Test FDT"; - compatible = "test"; -}; -''', 'test') - - # Write ITS with an invalid reference to a nonexistent default config - its_text = ''' -/dts-v1/; - -/ { - images { - kernel@1 { - description = "Test Kernel"; - data = /incbin/("kernel.bin"); - type = "kernel"; - arch = "sandbox"; - os = "linux"; - compression = "none"; - load = <0x40000>; - entry = <0x40000>; - }; - fdt@1 { - description = "Test FDT"; - data = /incbin/("test.dtb"); - type = "flat_dt"; - arch = "sandbox"; - os = "linux"; - compression = "none"; - load = <0x40000>; - entry = <0x40000>; - }; - }; - - configurations { - default = "conf@1"; - conf@1 { - kernel = "kernel@1"; - fdt = "fdt@1"; - }; - }; -}; -''' - - with open(its_fname, 'w') as f: - f.write(its_text) - - mkimage = os.path.join(ubman.config.build_dir, 'tools/mkimage') - cmd = [mkimage, '-f', its_fname, itb_fname] - - result = subprocess.run(cmd, capture_output=True, text=True) - - assert result.returncode != 0, "mkimage should fail due to memory overlap" - assert "Error: Overlap detected:" in result.stderr - # Check that it identifies the specific overlapping components - assert "kernel@1" in result.stderr and "fdt@1" in result.stderr diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 173b7eef6cc..8922d6cd070 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -1050,24 +1050,6 @@ split-elf Generates a `load = <...>` property with the load address of the segment - Note: The load address comes from the ELF file's program header or - linker script. To determine where an ELF file will be loaded, you can: - - 1. Use readelf to examine the program headers: - ``readelf -l your_elf_file.elf`` - Look for the LOAD segments and their VirtAddr (Virtual Address) - - 2. Check the linker script (.lds file) used to build the ELF: - Look for the `. =
;` statements which set the location - counter and determine load addresses for different sections - - 3. Use objdump to see section addresses: - ``objdump -h your_elf_file.elf`` - - For example, in binman tests, elf_sections.lds sets ATF load address - to 0x00000010, while elf_sections_tee.lds sets TEE load address to - 0x00100010 to avoid memory overlap conflicts. - fit,entry Generates a `entry = <...>` property with the entry address of the ELF. This is only produced for the first entry diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 0c2dbf333c0..925c39a530e 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -252,7 +252,7 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('bl31.elf', tools.read_file(cls.ElfTestFile('elf_sections'))) TestFunctional.tee_elf_path = TestFunctional._MakeInputFile('tee.elf', - tools.read_file(cls.ElfTestFile('elf_sections_tee'))) + tools.read_file(cls.ElfTestFile('elf_sections'))) # Newer OP_TEE file in v1 binary format cls.make_tee_bin('tee.bin') @@ -7997,7 +7997,7 @@ fdt fdtmap Extract the devicetree blob from the fdtmap 'Node \'/binman/fit\': multiple key paths found', str(e.exception)) - def testFitSignNoSignatureNodes(self): + def testFitSignNoSingatureNodes(self): """Test that fit,sign doens't raise error if no signature nodes found""" if not elf.ELF_TOOLS: self.skipTest('Python elftools not available') diff --git a/tools/binman/test/276_fit_firmware_loadables.dts b/tools/binman/test/276_fit_firmware_loadables.dts index d344036a11a..2f79cdc9bb8 100644 --- a/tools/binman/test/276_fit_firmware_loadables.dts +++ b/tools/binman/test/276_fit_firmware_loadables.dts @@ -19,8 +19,8 @@ arch = "arm64"; os = "u-boot"; compression = "none"; - load = <0x00002000>; - entry = <0x00002000>; + load = <0x00000000>; + entry = <0x00000000>; u-boot-nodtb { }; diff --git a/tools/binman/test/340_fit_signature.dts b/tools/binman/test/340_fit_signature.dts index 1c25d52cba4..9dce62e52de 100644 --- a/tools/binman/test/340_fit_signature.dts +++ b/tools/binman/test/340_fit_signature.dts @@ -20,8 +20,8 @@ arch = "arm64"; os = "u-boot"; compression = "none"; - load = <0x00002000>; - entry = <0x00002000>; + load = <0x00000000>; + entry = <0x00000000>; u-boot-nodtb { }; diff --git a/tools/binman/test/342_fit_signature.dts b/tools/binman/test/342_fit_signature.dts index 2ac600b1c70..267105d0f68 100644 --- a/tools/binman/test/342_fit_signature.dts +++ b/tools/binman/test/342_fit_signature.dts @@ -20,8 +20,8 @@ arch = "arm64"; os = "u-boot"; compression = "none"; - load = <0x00002000>; - entry = <0x00002000>; + load = <0x00000000>; + entry = <0x00000000>; u-boot-nodtb { }; diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index 66279e0e207..4d152eee9c0 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -30,13 +30,12 @@ LDS_BINMAN_BAD := -T $(SRC)u_boot_binman_syms_bad.lds LDS_BINMAN_X86 := -T $(SRC)u_boot_binman_syms_x86.lds LDS_BINMAN_EMBED := -T $(SRC)u_boot_binman_embed.lds LDS_EFL_SECTIONS := -T $(SRC)elf_sections.lds -LDS_EFL_SECTIONS_TEE := -T $(SRC)elf_sections_tee.lds LDS_BLOB := -T $(SRC)blob_syms.lds TARGETS = u_boot_ucode_ptr u_boot_no_ucode_ptr bss_data bss_data_zero \ u_boot_binman_syms u_boot_binman_syms.bin u_boot_binman_syms_bad \ u_boot_binman_syms_size u_boot_binman_syms_x86 embed_data \ - u_boot_binman_embed u_boot_binman_embed_sm elf_sections elf_sections_tee blob_syms.bin + u_boot_binman_embed u_boot_binman_embed_sm elf_sections blob_syms.bin all: $(TARGETS) @@ -85,9 +84,6 @@ blob_syms: blob_syms.c elf_sections: CFLAGS += $(LDS_EFL_SECTIONS) elf_sections: elf_sections.c -elf_sections_tee: CFLAGS += $(LDS_EFL_SECTIONS_TEE) -elf_sections_tee: elf_sections_tee.c - clean: rm -f $(TARGETS) diff --git a/tools/binman/test/elf_sections_tee.c b/tools/binman/test/elf_sections_tee.c deleted file mode 120000 index 01b200a365e..00000000000 --- a/tools/binman/test/elf_sections_tee.c +++ /dev/null @@ -1 +0,0 @@ -elf_sections.c \ No newline at end of file diff --git a/tools/binman/test/elf_sections_tee.lds b/tools/binman/test/elf_sections_tee.lds deleted file mode 100644 index 97e5e5f5d94..00000000000 --- a/tools/binman/test/elf_sections_tee.lds +++ /dev/null @@ -1,32 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0+ */ -/* - * Copyright (c) 2016 Google, Inc - * Copyright (c) 2025 Canonical Ltd. - */ - -OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386") -OUTPUT_ARCH(i386) -ENTRY(_start) - -SECTIONS -{ - . = 0x00100010; - _start = .; - - . = ALIGN(4); - .text : - { - *(.text*) - } - - . = 0x00101000; - .sram : - { - *(.sram*) - } - - /DISCARD/ : { - *(.comment) - *(.dyn*) - } -} diff --git a/tools/fit_image.c b/tools/fit_image.c index 12f4cdb2875..10849733816 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -22,20 +22,6 @@ #include #include -struct fit_region { - ulong load; - ulong size; - const char *name; -}; - -static int regions_overlap(const struct fit_region *a, const struct fit_region *b) -{ - ulong a_end = a->load + a->size; - ulong b_end = b->load + b->size; - - return !(a_end <= b->load || b_end <= a->load); -} - static struct legacy_img_hdr header; static int fit_estimate_hash_sig_size(struct image_tool_params *params, const char *fname) @@ -837,12 +823,9 @@ static int fit_import_data(struct image_tool_params *params, const char *fname) } fdt_for_each_subnode(node, fdt, confs) { - struct fit_region *regions = NULL; - unsigned int img_count = 0; - unsigned int regions_allocated = 0; const char *conf_name = fdt_get_name(fdt, node, NULL); - for (unsigned int i = 0; i < ARRAY_SIZE(props); i++) { + for (int i = 0; i < ARRAY_SIZE(props); i++) { int count = fdt_stringlist_count(fdt, node, props[i]); if (count < 0) @@ -863,79 +846,8 @@ static int fit_import_data(struct image_tool_params *params, const char *fname) ret = FDT_ERR_NOTFOUND; goto err_munmap; } - - ulong img_load = 0; - int img_size = 0; - - if (fit_image_get_load(fdt, img, &img_load)) { - fprintf(stderr, - "Warning: not able to get `load` of node '%s'\n", - img_name); - // Skip checking the components that do not have a - // definition for `load` - continue; - } - const char *img_data = fdt_getprop(fdt, img, - FIT_DATA_PROP, - &img_size); - - if (!img_data || !img_size) - continue; - - // Check if we've already added this image to avoid duplicates - for (unsigned int k = 0; k < img_count; k++) { - if (!strcmp(regions[k].name, img_name)) - goto next_node; - } - - // Expand regions array if needed - if (img_count >= regions_allocated) { - unsigned int new_size = regions_allocated ? - regions_allocated * 2 : 8; - struct fit_region *new_regions = realloc(regions, - new_size * sizeof(struct fit_region)); - if (!new_regions) { - fprintf(stderr, - "Failed to allocate memory for regions in config %s\n", - fdt_get_name(fdt, node, NULL)); - free(regions); - ret = -ENOMEM; - goto err_munmap; - } - regions = new_regions; - regions_allocated = new_size; - } - - regions[img_count].load = img_load; - regions[img_count].size = img_size; - regions[img_count].name = img_name; - img_count++; -next_node:; - } - } - - // Check for overlap within this config only - for (unsigned int i = 0; i < img_count; i++) { - for (unsigned int j = i + 1; j < img_count; j++) { - if (regions_overlap(®ions[i], ®ions[j])) { - fprintf(stderr, - "[Config: %s] Error: Overlap detected:\n" - " - %s: [0x%lx - 0x%lx]\n" - " - %s: [0x%lx - 0x%lx]\n", - fdt_get_name(fdt, node, NULL), - regions[i].name, regions[i].load, - regions[i].load + regions[i].size, - regions[j].name, regions[j].load, - regions[j].load + regions[j].size); - ret = FDT_ERR_BADSTRUCTURE; - free(regions); - goto err_munmap; - } } } - - // Clean up allocated memory for this configuration - free(regions); } munmap(old_fdt, sbuf.st_size); diff --git a/tools/mkimage.c b/tools/mkimage.c index e96fb7e42db..12183270776 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -533,8 +533,7 @@ int main(int argc, char **argv) retval = tparams->fflag_handle(¶ms); if (retval != EXIT_SUCCESS) { - if (retval == FDT_ERR_NOTFOUND || - retval == FDT_ERR_BADSTRUCTURE) { + if (retval == FDT_ERR_NOTFOUND) { // Already printed error, exit cleanly exit(EXIT_FAILURE); }