* [PATCHv8 0/3] various memory related fixups
@ 2026-05-13 20:49 rs
2026-05-13 20:49 ` [PATCHv8 1/3] boot: image-fdt: free old dtb reservations rs
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: rs @ 2026-05-13 20:49 UTC (permalink / raw)
To: robertcnelson, ayush, Erik.Welsh, anshuld, bb, trini, afd,
xypron.glpk, ilias.apalodimas, sjg
Cc: u-boot
From: Randolph Sapp <rs@ti.com>
Nitpicks and fixes from the discovery thread on adding PocketBeagle2 support
[1]. This does a lot of general setup required for the device, but these
modifications themselves aren't device specific. For those specifically
interested in PocketBeagle2 support and don't care about these details, my
development branch is public [2].
That first patch may provoke some opinions, but honestly if that warning was
still present I wouldn't have spent a week poking holes in both the EFI and LMB
allocations systems. Please let me know if there is a specific usecase that it
breaks though.
[1] https://lore.kernel.org/all/DHHC66BBMD27.YHGIH43C6XBK@ti.com/
[2] https://github.com/StaticRocket/u-boot/tree/feature/pocketbeagle2
v2:
- Remove additional increment and decrement in lmb_free_fdt_regions
- Drop the patch to backfill EFI_CONVENTIONAL_MEMORY
- Adjust the removal loop nitpick patch description
- Change the reserve memory patch to use new end_addr_sp
v3:
- Update lmb flags to use the macro documentation for constants
- Change efi_mem_sort to use list_for_each_entry_safe
v4:
- Fix typos in LMB allocation flags macro documentation
- Rename end_addr_sp to initial_relocaddr
- Keep the map_sysmem dance in the efi u-boot reservation
- Use the active device tree pointed to by gd->fdt_blob to clean up old
reservations
v5:
- Keep return value as long in boot_fdt_reserve_region
- Fix formatting in initial_relocaddr patch
v6:
- Drop patches that have been picked up already
- Add board_get_usable_ram_top for the sandbox
- s/boot_fdt_reserve_region/boot_fdt_handle_region/
v7:
- Remove board_get_usable_ram_top for the sandbox
- Reinstate bank hopping logic for U-Boot reserved region
- Update description for gd->initial_relocaddr, make it clear this is
also an exclusive value
- Add disclaimer to boot_fdt_add_mem_rsv_regions
- Add static boot_fdt_handle_mem_rsv_regions as a generic walker for the
two calls into boot_fdt_add_mem_rsv_regions
- Make boot_fdt_add_mem_rsv_regions fdt pointer const
- Leave the PRAM region out of the reservation. Previous commit messages
indicate that this is intended.
- Add a test for boot_fdt_add_mem_rsv_regions
v8:
- Adjust initial_relocaddr doc string
- Convert ref to fixes tag as requested
- Rework test_boot_fdt_add_mem_rsv_regions to mitigate lingering state
Randolph Sapp (3):
boot: image-fdt: free old dtb reservations
test: boot: add a fdt reserved region check
memory: reserve from start_addr_sp to initial_relocaddr
boot/image-fdt.c | 77 +++++++++++++++++++++++--------
common/board_f.c | 9 +++-
include/asm-generic/global_data.h | 9 ++++
include/image.h | 2 +-
lib/efi_loader/efi_memory.c | 2 +-
lib/lmb.c | 7 +--
test/boot/Makefile | 1 +
test/boot/image_fdt.c | 70 ++++++++++++++++++++++++++++
test/cmd_ut.c | 2 +
test/py/tests/test_suite.py | 2 +-
10 files changed, 154 insertions(+), 27 deletions(-)
create mode 100644 test/boot/image_fdt.c
--
2.54.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCHv8 1/3] boot: image-fdt: free old dtb reservations
2026-05-13 20:49 [PATCHv8 0/3] various memory related fixups rs
@ 2026-05-13 20:49 ` rs
2026-05-13 20:49 ` [PATCHv8 2/3] test: boot: add a fdt reserved region check rs
2026-05-13 20:49 ` [PATCHv8 3/3] memory: reserve from start_addr_sp to initial_relocaddr rs
2 siblings, 0 replies; 7+ messages in thread
From: rs @ 2026-05-13 20:49 UTC (permalink / raw)
To: robertcnelson, ayush, Erik.Welsh, anshuld, bb, trini, afd,
xypron.glpk, ilias.apalodimas, sjg
Cc: u-boot
From: Randolph Sapp <rs@ti.com>
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 <rs@ti.com>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v7:
- Add disclaimer to boot_fdt_add_mem_rsv_regions
- Add static boot_fdt_handle_mem_rsv_regions as a generic walker for the
two calls into boot_fdt_add_mem_rsv_regions
- Make boot_fdt_add_mem_rsv_regions fdt pointer const
v8:
- Convert ref to fixes tag as requested
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,
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCHv8 2/3] test: boot: add a fdt reserved region check
2026-05-13 20:49 [PATCHv8 0/3] various memory related fixups rs
2026-05-13 20:49 ` [PATCHv8 1/3] boot: image-fdt: free old dtb reservations rs
@ 2026-05-13 20:49 ` rs
2026-05-15 7:33 ` Ilias Apalodimas
2026-05-15 14:03 ` Simon Glass
2026-05-13 20:49 ` [PATCHv8 3/3] memory: reserve from start_addr_sp to initial_relocaddr rs
2 siblings, 2 replies; 7+ messages in thread
From: rs @ 2026-05-13 20:49 UTC (permalink / raw)
To: robertcnelson, ayush, Erik.Welsh, anshuld, bb, trini, afd,
xypron.glpk, ilias.apalodimas, sjg
Cc: u-boot
From: Randolph Sapp <rs@ti.com>
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 <rs@ti.com>
---
v8:
- Rework test_boot_fdt_add_mem_rsv_regions to mitigate lingering state
test/boot/Makefile | 1 +
test/boot/image_fdt.c | 70 +++++++++++++++++++++++++++++++++++++
test/cmd_ut.c | 2 ++
test/py/tests/test_suite.py | 2 +-
4 files changed, 74 insertions(+), 1 deletion(-)
create mode 100644 test/boot/image_fdt.c
diff --git a/test/boot/Makefile b/test/boot/Makefile
index 89538d4f0a6..640af6593cd 100644
--- a/test/boot/Makefile
+++ b/test/boot/Makefile
@@ -14,6 +14,7 @@ endif
ifdef CONFIG_SANDBOX
obj-$(CONFIG_$(PHASE_)CMDLINE) += bootm.o
+obj-$(CONFIG_$(PHASE_)OF_LIBFDT) += image_fdt.o
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..778951c1290
--- /dev/null
+++ b/test/boot/image_fdt.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2026 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+#include <config.h>
+
+#include <fdt_support.h>
+#include <image.h>
+#include <lmb.h>
+#include <malloc.h>
+
+#include <asm/global_data.h>
+
+#include <test/test.h>
+#include <test/ut.h>
+
+#define IMAGE_FDT_TEST(_name, _flags) UNIT_TEST(_name, _flags, image_fdt)
+
+DECLARE_GLOBAL_DATA_PTR;
+
+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_assertok_ptr(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_assert_console_end())
+ 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_LIVE_TREE | 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',
]
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCHv8 3/3] memory: reserve from start_addr_sp to initial_relocaddr
2026-05-13 20:49 [PATCHv8 0/3] various memory related fixups rs
2026-05-13 20:49 ` [PATCHv8 1/3] boot: image-fdt: free old dtb reservations rs
2026-05-13 20:49 ` [PATCHv8 2/3] test: boot: add a fdt reserved region check rs
@ 2026-05-13 20:49 ` rs
2026-05-15 7:38 ` Ilias Apalodimas
2 siblings, 1 reply; 7+ messages in thread
From: rs @ 2026-05-13 20:49 UTC (permalink / raw)
To: robertcnelson, ayush, Erik.Welsh, anshuld, bb, trini, afd,
xypron.glpk, ilias.apalodimas, sjg
Cc: u-boot
From: Randolph Sapp <rs@ti.com>
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 <rs@ti.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v7:
- Reinstate bank hopping logic for U-Boot reserved region
- Update description for gd->initial_relocaddr, make it clear this is
also an exclusive value
- Leave the PRAM region out of the reservation. Previous commit messages
indicate that this is intended.
v8:
- Adjust initial_relocaddr doc string
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 <relocaddr>' 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 046a2bb4641..9d7eda9422f 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -869,7 +869,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);
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCHv8 2/3] test: boot: add a fdt reserved region check
2026-05-13 20:49 ` [PATCHv8 2/3] test: boot: add a fdt reserved region check rs
@ 2026-05-15 7:33 ` Ilias Apalodimas
2026-05-15 14:03 ` Simon Glass
1 sibling, 0 replies; 7+ messages in thread
From: Ilias Apalodimas @ 2026-05-15 7:33 UTC (permalink / raw)
To: rs
Cc: robertcnelson, ayush, Erik.Welsh, anshuld, bb, trini, afd,
xypron.glpk, sjg, u-boot
On Wed, 13 May 2026 at 23:49, <rs@ti.com> wrote:
>
> From: Randolph Sapp <rs@ti.com>
>
> 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 <rs@ti.com>
> ---
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> v8:
> - Rework test_boot_fdt_add_mem_rsv_regions to mitigate lingering state
>
> test/boot/Makefile | 1 +
> test/boot/image_fdt.c | 70 +++++++++++++++++++++++++++++++++++++
> test/cmd_ut.c | 2 ++
> test/py/tests/test_suite.py | 2 +-
> 4 files changed, 74 insertions(+), 1 deletion(-)
> create mode 100644 test/boot/image_fdt.c
>
> diff --git a/test/boot/Makefile b/test/boot/Makefile
> index 89538d4f0a6..640af6593cd 100644
> --- a/test/boot/Makefile
> +++ b/test/boot/Makefile
> @@ -14,6 +14,7 @@ endif
>
> ifdef CONFIG_SANDBOX
> obj-$(CONFIG_$(PHASE_)CMDLINE) += bootm.o
> +obj-$(CONFIG_$(PHASE_)OF_LIBFDT) += image_fdt.o
> 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..778951c1290
> --- /dev/null
> +++ b/test/boot/image_fdt.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2026 Texas Instruments Incorporated - https://www.ti.com/
> + */
> +
> +#include <config.h>
> +
> +#include <fdt_support.h>
> +#include <image.h>
> +#include <lmb.h>
> +#include <malloc.h>
> +
> +#include <asm/global_data.h>
> +
> +#include <test/test.h>
> +#include <test/ut.h>
> +
> +#define IMAGE_FDT_TEST(_name, _flags) UNIT_TEST(_name, _flags, image_fdt)
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +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_assertok_ptr(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_assert_console_end())
> + 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_LIVE_TREE | 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',
> ]
> --
> 2.54.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv8 3/3] memory: reserve from start_addr_sp to initial_relocaddr
2026-05-13 20:49 ` [PATCHv8 3/3] memory: reserve from start_addr_sp to initial_relocaddr rs
@ 2026-05-15 7:38 ` Ilias Apalodimas
0 siblings, 0 replies; 7+ messages in thread
From: Ilias Apalodimas @ 2026-05-15 7:38 UTC (permalink / raw)
To: rs
Cc: robertcnelson, ayush, Erik.Welsh, anshuld, bb, trini, afd,
xypron.glpk, sjg, u-boot
On Wed, 13 May 2026 at 23:49, <rs@ti.com> wrote:
>
> From: Randolph Sapp <rs@ti.com>
>
> 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 <rs@ti.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> v7:
> - Reinstate bank hopping logic for U-Boot reserved region
> - Update description for gd->initial_relocaddr, make it clear this is
> also an exclusive value
> - Leave the PRAM region out of the reservation. Previous commit messages
> indicate that this is intended.
> v8:
> - Adjust initial_relocaddr doc string
>
> 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 <relocaddr>' 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 046a2bb4641..9d7eda9422f 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -869,7 +869,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);
> --
> 2.54.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv8 2/3] test: boot: add a fdt reserved region check
2026-05-13 20:49 ` [PATCHv8 2/3] test: boot: add a fdt reserved region check rs
2026-05-15 7:33 ` Ilias Apalodimas
@ 2026-05-15 14:03 ` Simon Glass
1 sibling, 0 replies; 7+ messages in thread
From: Simon Glass @ 2026-05-15 14:03 UTC (permalink / raw)
To: rs
Cc: robertcnelson, ayush, Erik.Welsh, anshuld, bb, trini, afd,
xypron.glpk, ilias.apalodimas, sjg, u-boot
Hi Randolph,
On 2026-05-13T20:49:42, Randolph Sapp <rs@ti.com> wrote:
> 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 <rs@ti.com>
> Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> test/boot/Makefile | 1 +
> test/boot/image_fdt.c | 70 +++++++++++++++++++++++++++++++++++++++++++++
> test/cmd_ut.c | 2 ++
> test/py/tests/test_suite.py | 2 +-
> 4 files changed, 74 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
> diff --git a/test/boot/image_fdt.c b/test/boot/image_fdt.c
> @@ -0,0 +1,70 @@
> + fdt_sz = fdt_totalsize(gd->fdt_blob);
> + new_blob = malloc(fdt_sz);
> + ut_assertok_ptr(new_blob);
> + memcpy(new_blob, gd->fdt_blob, fdt_sz);
ut_assertok_ptr() only checks IS_ERR(), but malloc() returns NULL on
failure. IS_ERR(NULL) is false, so a failed allocation sails through
and crashes in the memcpy() below. Please use
ut_assertnonnull(new_blob) here.
> diff --git a/test/boot/image_fdt.c b/test/boot/image_fdt.c
> @@ -0,0 +1,70 @@
> + boot_fdt_add_mem_rsv_regions(new_blob);
> + gd->fdt_blob = new_blob;
> +
> + if (ut_assert_console_end())
> + goto switch_fdt;
This doesn't do what it looks like. ut_assert_console_end() expands to
a block that does 'return CMD_RET_FAILURE' on failure, so the if
condition is always 0 and the goto is dead code. If the assertion
fails, the function returns immediately, leaving gd->fdt_blob pointing
at new_blob and leaking it.
To defer cleanup, either restructure so gd->fdt_blob isn't changed
until after the check, or use ut_check_console_end() and handle the
result yourself. The same applies to the
ut_assertok_ptr()/ut_assertnonnull() above - once gd->fdt_blob is
mutated, any macro that early-returns skips the cleanup labels. We
have a similar problem in the bootstd tests...
Regards,
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-15 14:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13 20:49 [PATCHv8 0/3] various memory related fixups rs
2026-05-13 20:49 ` [PATCHv8 1/3] boot: image-fdt: free old dtb reservations rs
2026-05-13 20:49 ` [PATCHv8 2/3] test: boot: add a fdt reserved region check rs
2026-05-15 7:33 ` Ilias Apalodimas
2026-05-15 14:03 ` Simon Glass
2026-05-13 20:49 ` [PATCHv8 3/3] memory: reserve from start_addr_sp to initial_relocaddr rs
2026-05-15 7:38 ` Ilias Apalodimas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.