[PATCHv7 2/3] test: boot: add a fdt reserved region check

[Simon Glass]

Hi Randolph,

On 2026-05-13T01:56:06, Randolph Sapp <rs at 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 at ti.com>
>
> 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