Hi Randolph, On 2026-05-13T01:56:06, Randolph Sapp <[email protected]> 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 <[email protected]> > > test/boot/Makefile | 1 + > test/boot/image_fdt.c | 53 > +++++++++++++++++++++++++++++++++++++++++++++ > test/cmd_ut.c | 2 ++ > test/py/tests/test_suite.py | 2 +- > 4 files changed, 57 insertions(+), 1 deletion(-)
Change log? > diff --git a/test/boot/image_fdt.c b/test/boot/image_fdt.c > @@ -0,0 +1,53 @@ > +#include <fdt_support.h> > +#include <asm/global_data.h> > +#include <config.h> > +#include <image.h> > +#include <lmb.h> > +#include <malloc.h> > +#include <test/test.h> > +#include <test/ut.h> Please sort these: config.h (if needed) first, then the plain top-level headers alphabetically (fdt_support.h, image.h, lmb.h, malloc.h), then asm/global_data.h, then test/*. > diff --git a/test/boot/image_fdt.c b/test/boot/image_fdt.c > @@ -0,0 +1,53 @@ > +#define FDTDEC_MAX_SIZE (2 * 1024 * 1024) > + > +#define IMAGE_FDT_TEST(_name, _flags) UNIT_TEST(_name, _flags, image_fdt) FDTDEC_MAX_SIZE is never referenced - looks copied from test/dm/fdtdec.c - please drop it. > diff --git a/test/boot/image_fdt.c b/test/boot/image_fdt.c > @@ -0,0 +1,53 @@ > + /* Loading a new device tree should be allowed */ > + blob = malloc(fdt_sz); > + ut_assertok_ptr(blob); > + memcpy(blob, gd->fdt_blob, fdt_sz); > + > + nodeoffset = fdt_path_offset(blob, '/reserved-memory'); > + ut_assertok(fdt_del_node(blob, nodeoffset)); If /reserved-memory is missing, fdt_path_offset() returns a negative libfdt error and fdt_del_node() then fails with a confusing message. Please ut_assert(nodeoffset >= 0) first. Also, blob is malloc()'d but never freed. > diff --git a/test/boot/image_fdt.c b/test/boot/image_fdt.c > @@ -0,0 +1,53 @@ > + boot_fdt_add_mem_rsv_regions(blob); > + ut_assert_console_end(); > + boot_relocate_fdt((char **)&blob, &fdt_sz); > + > + /* Reservation should not exist now */ > + ut_asserteq(0, lmb_is_reserved_flags(start, LMB_NOMAP)); What is the boot_relocate_fdt() call testing - could you add a comment? The reservation at start is already gone after boot_fdt_add_mem_rsv_regions(blob), so the final ut_asserteq() does not depend on it. It also allocates from LMB for the relocated copy, which then leaks. I'd drop it, or split it into a separate test with its own assertions. The (char **)&blob cast is a hint that the call doesn't belong here. > diff --git a/test/boot/image_fdt.c b/test/boot/image_fdt.c > @@ -0,0 +1,53 @@ > +IMAGE_FDT_TEST(test_boot_fdt_add_mem_rsv_regions, UTF_LIVE_TREE | > UTF_CONSOLE); Just to check: the test only touches gd->fdt_blob (the flat tree), so why UTF_LIVE_TREE? If you want it to run regardless of OF_LIVE, drop the flag (or use UTF_FLAT_TREE). If the intent was to avoid running twice, please mention that in a comment. Regards, Simon
