From 623f6c5b6ab7fa270a9e36db0c6136c5983a45a0 Mon Sep 17 00:00:00 2001 From: Randolph Sapp Date: Thu, 4 Jun 2026 10:50:35 -0500 Subject: boot: image-fdt: free old dtb reservations Add a free flag and an initial call to free allocations covered by the global FDT. This assumes that all calls to boot_fdt_add_mem_rsv_regions occur before the transition to the new device tree, thus we can access the currently active device tree through the global data pointer. This allows us to clearly indicate to the user when a device tree reservation fails. How we handle this can still use some improvement. Right now we'll keep the default behavior and try to boot anyway. Fixes: 5a6aa7d5913 ("boot: fdt: Handle already reserved memory in boot_fdt_reserve_region()") Signed-off-by: Randolph Sapp Acked-by: Ilias Apalodimas Reviewed-by: Simon Glass Fixes: tag with a 12-char hash: Fixes: 5a6aa7d59133 ("boot: fdt: Handle already reserved memory in --- boot/image-fdt.c | 77 +++++++++++++++++++++++++++++++++++++++++--------------- include/image.h | 2 +- 2 files changed, 58 insertions(+), 21 deletions(-) diff --git a/boot/image-fdt.c b/boot/image-fdt.c index a3a4fb8b558..1150131a11e 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -69,35 +69,50 @@ static const struct legacy_img_hdr *image_get_fdt(ulong fdt_addr) } #endif -static void boot_fdt_reserve_region(u64 addr, u64 size, u32 flags) +/** + * boot_fdt_handle_region - Reserve or free a given FDT region in LMB + * @addr: Reservation base address + * @size: Reservation size + * @flags: Reservation flags + * @free: Indicate if region is being freed or allocated + * + * Add or free a given reservation from LMB. This reports to the user if any + * errors occurred during either operation. + */ +static void boot_fdt_handle_region(u64 addr, u64 size, u32 flags, bool free) { long ret; phys_addr_t rsv_addr; rsv_addr = (phys_addr_t)addr; - ret = lmb_alloc_mem(LMB_MEM_ALLOC_ADDR, 0, &rsv_addr, size, flags); + if (free) + ret = lmb_free(rsv_addr, size, flags); + else + ret = lmb_alloc_mem(LMB_MEM_ALLOC_ADDR, 0, &rsv_addr, size, + flags); + if (!ret) { - debug(" reserving fdt memory region: addr=%llx size=%llx flags=%x\n", - (unsigned long long)addr, + debug(" %s fdt memory region: addr=%llx size=%llx flags=%x\n", + free ? "freed" : "reserved", (unsigned long long)addr, (unsigned long long)size, flags); - } else if (ret != -EEXIST && ret != -EINVAL) { - puts("ERROR: reserving fdt memory region failed "); - printf("(addr=%llx size=%llx flags=%x)\n", - (unsigned long long)addr, - (unsigned long long)size, flags); + } else { + printf("ERROR: %s fdt memory region failed (addr=%llx size=%llx flags=%x): %ld\n", + free ? "freeing" : "reserving", (unsigned long long)addr, + (unsigned long long)size, flags, ret); } } /** - * boot_fdt_add_mem_rsv_regions - Mark the memreserve and reserved-memory - * sections as unusable + * boot_fdt_handle_mem_rsv_regions - Handle FDT memreserve and reserved-memory + * sections * @fdt_blob: pointer to fdt blob base address + * @free: indicate if regions are being freed * - * Adds the and reserved-memorymemreserve regions in the dtb to the lmb block. - * Adding the memreserve regions prevents u-boot from using them to store the - * initrd or the fdt blob. + * Adds or removes reserved-memory and memreserve regions in the dtb to the lmb + * block. Adding the memreserve regions prevents u-boot from using them to store + * the initrd or the fdt blob. */ -void boot_fdt_add_mem_rsv_regions(void *fdt_blob) +static void boot_fdt_handle_mem_rsv_regions(const void *fdt_blob, bool free) { uint64_t addr, size; int i, total, ret; @@ -105,15 +120,12 @@ void boot_fdt_add_mem_rsv_regions(void *fdt_blob) struct fdt_resource res; u32 flags; - if (fdt_check_header(fdt_blob) != 0) - return; - /* process memreserve sections */ total = fdt_num_mem_rsv(fdt_blob); for (i = 0; i < total; i++) { if (fdt_get_mem_rsv(fdt_blob, i, &addr, &size) != 0) continue; - boot_fdt_reserve_region(addr, size, LMB_NOOVERWRITE); + boot_fdt_handle_region(addr, size, LMB_NOOVERWRITE, free); } /* process reserved-memory */ @@ -131,7 +143,7 @@ void boot_fdt_add_mem_rsv_regions(void *fdt_blob) flags = LMB_NOMAP; addr = res.start; size = res.end - res.start + 1; - boot_fdt_reserve_region(addr, size, flags); + boot_fdt_handle_region(addr, size, flags, free); } subnode = fdt_next_subnode(fdt_blob, subnode); @@ -139,6 +151,31 @@ void boot_fdt_add_mem_rsv_regions(void *fdt_blob) } } +/** + * boot_fdt_add_mem_rsv_regions - Add FDT memreserve and reserved-memory + * sections + * @fdt_blob: pointer to fdt blob base address + * + * Adds reserved-memory and memreserve regions in the dtb to the lmb block. + * Adding the memreserve regions prevents u-boot from using them to store the + * initrd or the fdt blob. + * + * This function will attempt to clean the currently active reservations if a + * new device tree blob is given. This must be called before gd->fdt_blob is + * switched. + */ +void boot_fdt_add_mem_rsv_regions(const void *fdt_blob) +{ + if (fdt_check_header(fdt_blob) != 0) + return; + + /* Remove old regions */ + if (gd->fdt_blob != fdt_blob) + boot_fdt_handle_mem_rsv_regions(gd->fdt_blob, true); + + boot_fdt_handle_mem_rsv_regions(fdt_blob, false); +} + /** * boot_relocate_fdt - relocate flat device tree * @of_flat_tree: pointer to a char* variable, will hold fdt start address diff --git a/include/image.h b/include/image.h index 34efac6056d..151619f42bf 100644 --- a/include/image.h +++ b/include/image.h @@ -827,7 +827,7 @@ int boot_get_fdt(void *buf, const char *select, uint arch, struct bootm_headers *images, char **of_flat_tree, ulong *of_size); -void boot_fdt_add_mem_rsv_regions(void *fdt_blob); +void boot_fdt_add_mem_rsv_regions(const void *fdt_blob); int boot_relocate_fdt(char **of_flat_tree, ulong *of_size); int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start, -- cgit v1.3.1 From 5a17d02223b942f97bd3df9d6acb835fded4ad28 Mon Sep 17 00:00:00 2001 From: Randolph Sapp Date: Thu, 4 Jun 2026 10:50:36 -0500 Subject: test_ut: add a ut_ubman fixture to clean up tests Add a ut_ubman fixture to clean up after certain problematic tests without negatively affecting the current assert based testing. Currently this catches "bootstd bootflow_cmd_boot" and "bootstd bootflow_scan_boot" ut_subtests, as these will change the sandbox state a little too much to be recoverable from. Signed-off-by: Randolph Sapp Reviewed-by: Simon Glass --- test/py/tests/test_ut.py | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py index dce5a37dd35..fa50c8008a5 100644 --- a/test/py/tests/test_ut.py +++ b/test/py/tests/test_ut.py @@ -631,7 +631,23 @@ def test_ut_dm_init_bootstd(ubman): ubman.restart_uboot() -def test_ut(ubman, ut_subtest): +@pytest.fixture(name="ut_ubman") +def ut_ubman_fixture(ubman, ut_subtest): + """Fixture to restart the sandbox after known problematic tests. + + Args: + ubman (ConsoleBase): U-Boot console + ut_subtest (str): test to be executed via command ut, e.g 'foo bar' to + execute command 'ut foo bar' + """ + + yield ubman + + if ut_subtest in ("bootstd bootflow_cmd_boot", "bootstd bootflow_scan_boot"): + ubman.restart_uboot() + + +def test_ut(ut_ubman, ut_subtest): """Execute a "ut" subtest. The subtests are collected in function generate_ut_subtest() from linker @@ -644,18 +660,18 @@ def test_ut(ubman, ut_subtest): implemented in C function foo_test_bar(). Args: - ubman (ConsoleBase): U-Boot console + ut_ubman (ConsoleBase): U-Boot console ut_subtest (str): test to be executed via command ut, e.g 'foo bar' to execute command 'ut foo bar' """ if ut_subtest == 'hush hush_test_simple_dollar': # ut hush hush_test_simple_dollar prints "Unknown command" on purpose. - with ubman.disable_check('unknown_command'): - output = ubman.run_command('ut ' + ut_subtest) + with ut_ubman.disable_check('unknown_command'): + output = ut_ubman.run_command('ut ' + ut_subtest) assert 'Unknown command \'quux\' - try \'help\'' in output else: - output = ubman.run_command('ut ' + ut_subtest) + output = ut_ubman.run_command('ut ' + ut_subtest) assert output.endswith('failures: 0') lastline = output.splitlines()[-1] if "skipped: 0," not in lastline: -- cgit v1.3.1 From de12fa4df89277460aacaea7c9a43421190ee04d Mon Sep 17 00:00:00 2001 From: Randolph Sapp Date: Thu, 4 Jun 2026 10:50:37 -0500 Subject: test: boot: add a fdt reserved region check Add a image_fdt suite and a check for boot_fdt_add_mem_rsv_regions. This will ensure the user is properly informed of any reservation failures. It will also validate that reservations are cleaned up correctly when switching FDTs. Signed-off-by: Randolph Sapp Reviewed-by: Simon Glass Acked-by: Ilias Apalodimas --- test/boot/Makefile | 3 ++ test/boot/image_fdt.c | 83 +++++++++++++++++++++++++++++++++++++++++++++ test/cmd_ut.c | 2 ++ test/py/tests/test_suite.py | 2 +- 4 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 test/boot/image_fdt.c diff --git a/test/boot/Makefile b/test/boot/Makefile index 89538d4f0a6..12904f7f508 100644 --- a/test/boot/Makefile +++ b/test/boot/Makefile @@ -14,6 +14,9 @@ endif ifdef CONFIG_SANDBOX obj-$(CONFIG_$(PHASE_)CMDLINE) += bootm.o +ifdef CONFIG_UT_DM +obj-$(CONFIG_$(PHASE_)OF_LIBFDT) += image_fdt.o +endif endif obj-$(CONFIG_MEASURED_BOOT) += measurement.o diff --git a/test/boot/image_fdt.c b/test/boot/image_fdt.c new file mode 100644 index 00000000000..5417689a683 --- /dev/null +++ b/test/boot/image_fdt.c @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2026 Texas Instruments Incorporated - https://www.ti.com/ + */ + +#include + +#include +#include +#include +#include + +#include + +#include +#include + +#define IMAGE_FDT_TEST(_name, _flags) UNIT_TEST(_name, _flags, image_fdt) + +DECLARE_GLOBAL_DATA_PTR; + +/** + * test_boot_fdt_add_mem_rsv_regions - Make sure dt reservations are created and + * destroyed correctly + * @uts: Test state + * + * This test depends on the UT_DM device tree and ensures the following + * statements hold true: The default reservation in test.dtb exists. + * Re-reserving that region will result in an error. Loading a new device tree + * will remove old reservations. + */ +static int test_boot_fdt_add_mem_rsv_regions(struct unit_test_state *uts) +{ + phys_addr_t start = CFG_SYS_SDRAM_BASE + 0x100000; + const void *old_blob = gd->fdt_blob; + int ret = CMD_RET_FAILURE; + ulong fdt_sz; + int nodeoffset; + void *new_blob; + + /* Default reservation should exist */ + ut_asserteq(1, lmb_is_reserved_flags(start, LMB_NOMAP)); + + /* Attempting to re-reserve should warn the user */ + boot_fdt_add_mem_rsv_regions(gd->fdt_blob); + ut_assert_nextlinen("ERROR: reserving"); + ut_assert_console_end(); + + /* Loading a new_blob device tree should be allowed */ + fdt_sz = fdt_totalsize(gd->fdt_blob); + new_blob = malloc(fdt_sz); + ut_assertnonnull(new_blob); + memcpy(new_blob, gd->fdt_blob, fdt_sz); + + nodeoffset = fdt_path_offset(new_blob, "/reserved-memory"); + if (nodeoffset < 0) + goto free_blob; + + if (fdt_del_node(new_blob, nodeoffset)) + goto free_blob; + + boot_fdt_add_mem_rsv_regions(new_blob); + gd->fdt_blob = new_blob; + + if (ut_check_console_end(uts)) { + ut_failf(uts, __FILE__, __LINE__, __func__, "console", + "Expected no more output, got '%s'", uts->actual_str); + goto switch_fdt; + } + + /* Reservation should not exist now */ + if (!lmb_is_reserved_flags(start, LMB_NOMAP)) + ret = 0; + + /* Cleanup */ +switch_fdt: + boot_fdt_add_mem_rsv_regions(old_blob); + gd->fdt_blob = old_blob; +free_blob: + free(new_blob); + return ret; +} +IMAGE_FDT_TEST(test_boot_fdt_add_mem_rsv_regions, UTF_CONSOLE); diff --git a/test/cmd_ut.c b/test/cmd_ut.c index 44e5fdfdaa6..363ed4eab30 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -61,6 +61,7 @@ SUITE_DECL(fdt); SUITE_DECL(fdt_overlay); SUITE_DECL(font); SUITE_DECL(hush); +SUITE_DECL(image_fdt); SUITE_DECL(lib); SUITE_DECL(loadm); SUITE_DECL(log); @@ -88,6 +89,7 @@ static struct suite suites[] = { SUITE(fdt_overlay, "device tree overlays"), SUITE(font, "font command"), SUITE(hush, "hush behaviour"), + SUITE(image_fdt, "image fdt parsing"), SUITE(lib, "library functions"), SUITE(loadm, "loadm command parameters and loading memory blob"), SUITE(log, "logging functions"), diff --git a/test/py/tests/test_suite.py b/test/py/tests/test_suite.py index 7fe9a90dfd3..08285f12a5f 100644 --- a/test/py/tests/test_suite.py +++ b/test/py/tests/test_suite.py @@ -8,7 +8,7 @@ import re EXPECTED_SUITES = [ 'addrmap', 'bdinfo', 'bloblist', 'bootm', 'bootstd', 'cmd', 'common', 'dm', 'env', 'exit', 'fdt_overlay', - 'fdt', 'font', 'hush', 'lib', + 'fdt', 'font', 'hush', 'image_fdt', 'lib', 'loadm', 'log', 'mbr', 'measurement', 'mem', 'pci_mps', 'setexpr', 'upl', ] -- cgit v1.3.1 From 48412f8f2962e27abe3b9a4a73221cebbfd73333 Mon Sep 17 00:00:00 2001 From: Randolph Sapp Date: Thu, 4 Jun 2026 10:50:38 -0500 Subject: memory: reserve from start_addr_sp to initial_relocaddr Add a new global data struct member called initial_relocaddr. This stores the original value of relocaddr, directly from setup_dest_addr. This is specifically to avoid any adjustments made by other init functions. Reserve the memory from gd->start_addr_sp - CONFIG_STACK_SIZE to gd->initial_relocaddr instead of gd->ram_top. This allows platform specific relocation addresses to work without unnecessarily painting over a large range. Signed-off-by: Randolph Sapp Reviewed-by: Simon Glass Reviewed-by: Ilias Apalodimas --- common/board_f.c | 9 ++++++++- include/asm-generic/global_data.h | 9 +++++++++ lib/efi_loader/efi_memory.c | 2 +- lib/lmb.c | 7 ++++--- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/common/board_f.c b/common/board_f.c index ce87c619e68..aeb53b4c274 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -330,6 +330,8 @@ __weak int arch_setup_dest_addr(void) static int setup_dest_addr(void) { + int ret; + debug("Monitor len: %08x\n", gd->mon_len); /* * Ram is setup, size stored in gd !! @@ -356,7 +358,12 @@ static int setup_dest_addr(void) gd->relocaddr = gd->ram_top; debug("Ram top: %08llX\n", (unsigned long long)gd->ram_top); - return arch_setup_dest_addr(); + ret = arch_setup_dest_addr(); + if (ret) + return ret; + + gd->initial_relocaddr = gd->relocaddr; + return 0; } #ifdef CFG_PRAM diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index 745d2c3a966..8d1d49b1133 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -107,6 +107,15 @@ struct global_data { * GDB using the 'add-symbol-file u-boot ' command. */ unsigned long relocaddr; + /** + * @initial_relocaddr: top address of U-Boot in RAM + * + * This should be the value of relocaddr after setup_dest_addr() and + * before reserve_pram() or any other allocations or reservations shift + * it. This address will, depending on the platform, be equivalent to + * ram_top and should also be considered an exclusive address. + */ + unsigned long initial_relocaddr; /** * @irq_sp: IRQ stack pointer */ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 2feb29f0a2c..c3da7c20cb2 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -871,7 +871,7 @@ static void add_u_boot_and_runtime(void) /* Add U-Boot */ uboot_start = ((uintptr_t)map_sysmem(gd->start_addr_sp, 0) - uboot_stack_size) & ~EFI_PAGE_MASK; - uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) - + uboot_pages = ((uintptr_t)map_sysmem(gd->initial_relocaddr - 1, 0) - uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; efi_update_memory_map(uboot_start, uboot_pages, EFI_BOOT_SERVICES_CODE, false, false); diff --git a/lib/lmb.c b/lib/lmb.c index 8f12c6ad8e5..27c8565e590 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -540,13 +540,14 @@ static void lmb_reserve_uboot_region(void) ulong pram = 0; rsv_start = gd->start_addr_sp - CONFIG_STACK_SIZE; - end = gd->ram_top; + end = gd->initial_relocaddr; /* * Reserve memory from aligned address below the bottom of U-Boot stack - * until end of RAM area to prevent LMB from overwriting that memory. + * until the original relocation address to prevent LMB from + * overwriting that memory. */ - debug("## Current stack ends at 0x%08lx ", (ulong)rsv_start); + debug("## Current stack ends at 0x%08lx\n", (ulong)rsv_start); #ifdef CFG_PRAM pram = env_get_ulong("pram", 10, CFG_PRAM); -- cgit v1.3.1