* [PATCH 0/9] CI: test qemu-riscv64_smode[_acpi]
@ 2025-11-09 10:10 Heinrich Schuchardt
2025-11-09 10:10 ` [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY Heinrich Schuchardt
` (8 more replies)
0 siblings, 9 replies; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-09 10:10 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot, Heinrich Schuchardt
ACPI via QFW was broken since the 2025.01 release and we did not observe
it due to missing CI testing. This bug has been fixed in the meanwhile.
This series
* introduces prerequisite corrections of unit tests
* provides the necessary console record size for command line tests
* enables CI testing for qemu-riscv64_smode and qemu-riscv64_smode_acpi
The following patches are just resent to show how all belongs together:
* test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY
* CI: test qemu-riscv64_smode[_acpi]
Heinrich Schuchardt (9):
test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY
test: cmd/fdt: do not assume RNG device exists
test: cmd/bdinfo: make no flash assumption
test: cmd/bdinfo: consider arch_print_bdinfo() output
test: common/print: do not use fixed buffer addresses
test: cmd/fdt: do not use fixed buffer addresses
common: default CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000
configs: CONFIG_CONSOLE_RECORD=y on qemu-riscv64_smode_acpi
CI: test qemu-riscv64_smode[_acpi]
.azure-pipelines.yml | 6 ++
.gitlab-ci.yml | 12 ++++
common/Kconfig | 2 +-
configs/qemu-riscv64_smode_acpi_defconfig | 1 +
test/cmd/bdinfo.c | 15 +++--
test/cmd/fdt.c | 31 ++++++++--
test/common/print.c | 71 +++++++++++------------
7 files changed, 91 insertions(+), 47 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY
2025-11-09 10:10 [PATCH 0/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
@ 2025-11-09 10:10 ` Heinrich Schuchardt
2025-11-11 8:29 ` Ilias Apalodimas
2025-11-22 16:23 ` Tom Rini
2025-11-09 10:10 ` [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists Heinrich Schuchardt
` (7 subsequent siblings)
8 siblings, 2 replies; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-09 10:10 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot, Heinrich Schuchardt
The `fdt apply` sub-command is only available if CONFIG_OF_LIBFDT_OVERLAY
is enabled.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
test/cmd/fdt.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
index 96a8488e172..a36f2dcdda8 100644
--- a/test/cmd/fdt.c
+++ b/test/cmd/fdt.c
@@ -1319,6 +1319,9 @@ static int fdt_test_apply(struct unit_test_state *uts)
char fdt[8192], fdto[8192];
ulong addr, addro;
+ if (!IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY))
+ return -EAGAIN;
+
/* Create base DT with __symbols__ node */
ut_assertok(fdt_create(fdt, sizeof(fdt)));
ut_assertok(fdt_finish_reservemap(fdt));
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists
2025-11-09 10:10 [PATCH 0/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
2025-11-09 10:10 ` [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY Heinrich Schuchardt
@ 2025-11-09 10:10 ` Heinrich Schuchardt
2025-11-11 8:33 ` Ilias Apalodimas
2025-11-14 12:44 ` Simon Glass
2025-11-09 10:10 ` [PATCH 3/9] test: cmd/bdinfo: make no flash assumption Heinrich Schuchardt
` (6 subsequent siblings)
8 siblings, 2 replies; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-09 10:10 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot, Heinrich Schuchardt
In fdt_test_chosen() currently we test if DM_RNG is configured.
CONFIG_DM_RNG=y does not imply that a RNG device actually exists.
For instance QEMU may be called with -device virtio-rng-device or not.
The current test framework evicts the virtio RNG device even if QEMU is
called with -device virtio-rng-device.
In the fdt_test_chosen() check if a RNG device exists.
Ignore 'No RNG device' messages.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
test/cmd/fdt.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
index a36f2dcdda8..b950123b6da 100644
--- a/test/cmd/fdt.c
+++ b/test/cmd/fdt.c
@@ -10,6 +10,7 @@
#include <fdt_support.h>
#include <mapmem.h>
#include <asm/global_data.h>
+#include <dm/uclass.h>
#include <linux/libfdt.h>
#include <test/ut.h>
@@ -1267,6 +1268,7 @@ static int fdt_test_chosen(struct unit_test_state *uts)
{
const char *env_bootargs = env_get("bootargs");
char fdt[8192];
+ struct udevice *dev;
ulong addr;
ut_assertok(make_test_fdt(uts, fdt, sizeof(fdt), &addr));
@@ -1280,11 +1282,16 @@ static int fdt_test_chosen(struct unit_test_state *uts)
/* Test add new chosen node without initrd */
ut_assertok(run_commandf("fdt chosen"));
ut_assertok(run_commandf("fdt print /chosen"));
- ut_assert_nextline("chosen {");
+ ut_assert(0 < console_record_readline(uts->actual_str,
+ sizeof(uts->actual_str)));
+ if (!strcmp("No RNG device", uts->actual_str))
+ ut_assert(0 < console_record_readline(uts->actual_str,
+ sizeof(uts->actual_str)));
+ ut_asserteq_str("chosen {", uts->actual_str);
ut_assert_nextlinen("\tu-boot,version = "); /* Ignore the version string */
if (env_bootargs)
ut_assert_nextline("\tbootargs = \"%s\";", env_bootargs);
- if (IS_ENABLED(CONFIG_DM_RNG) &&
+ if (!uclass_get_device(UCLASS_RNG, 0, &dev) &&
!IS_ENABLED(CONFIG_MEASURED_BOOT) &&
!IS_ENABLED(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT))
ut_assert_nextlinen("\tkaslr-seed = ");
@@ -1294,7 +1301,12 @@ static int fdt_test_chosen(struct unit_test_state *uts)
/* Test add new chosen node with initrd */
ut_assertok(run_commandf("fdt chosen 0x1234 0x5678"));
ut_assertok(run_commandf("fdt print /chosen"));
- ut_assert_nextline("chosen {");
+ ut_assert(0 < console_record_readline(uts->actual_str,
+ sizeof(uts->actual_str)));
+ if (!strcmp("No RNG device", uts->actual_str))
+ ut_assert(0 < console_record_readline(uts->actual_str,
+ sizeof(uts->actual_str)));
+ ut_asserteq_str("chosen {", uts->actual_str);
ut_assert_nextline("\tlinux,initrd-end = <0x%08x 0x%08x>;",
upper_32_bits(0x1234 + 0x5678 - 1),
lower_32_bits(0x1234 + 0x5678 - 1));
@@ -1303,7 +1315,7 @@ static int fdt_test_chosen(struct unit_test_state *uts)
ut_assert_nextlinen("\tu-boot,version = "); /* Ignore the version string */
if (env_bootargs)
ut_assert_nextline("\tbootargs = \"%s\";", env_bootargs);
- if (IS_ENABLED(CONFIG_DM_RNG) &&
+ if (!uclass_get_device(UCLASS_RNG, 0, &dev) &&
!IS_ENABLED(CONFIG_MEASURED_BOOT) &&
!IS_ENABLED(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT))
ut_assert_nextlinen("\tkaslr-seed = ");
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 3/9] test: cmd/bdinfo: make no flash assumption
2025-11-09 10:10 [PATCH 0/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
2025-11-09 10:10 ` [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY Heinrich Schuchardt
2025-11-09 10:10 ` [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists Heinrich Schuchardt
@ 2025-11-09 10:10 ` Heinrich Schuchardt
2025-11-11 8:30 ` Ilias Apalodimas
2025-11-14 12:44 ` Simon Glass
2025-11-09 10:10 ` [PATCH 4/9] test: cmd/bdinfo: consider arch_print_bdinfo() output Heinrich Schuchardt
` (5 subsequent siblings)
8 siblings, 2 replies; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-09 10:10 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot, Heinrich Schuchardt
The location and size of flash is device-dependent. Do not make any
assumption about the location and size.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
test/cmd/bdinfo.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c
index 09f44ee41ed..214b237152b 100644
--- a/test/cmd/bdinfo.c
+++ b/test/cmd/bdinfo.c
@@ -161,9 +161,9 @@ static int bdinfo_test_all(struct unit_test_state *uts)
ut_assertok(bdinfo_check_mem(uts));
/* CONFIG_SYS_HAS_SRAM testing not supported */
- ut_assertok(test_num_l(uts, "flashstart", 0));
- ut_assertok(test_num_l(uts, "flashsize", 0));
- ut_assertok(test_num_l(uts, "flashoffset", 0));
+ ut_check_console_linen(uts, "flashstart");
+ ut_check_console_linen(uts, "flashsize");
+ ut_check_console_linen(uts, "flashoffset");
ut_assert_nextline("baudrate = %lu bps",
env_get_ulong("baudrate", 10, 1234));
ut_assertok(test_num_l(uts, "relocaddr", gd->relocaddr));
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 4/9] test: cmd/bdinfo: consider arch_print_bdinfo() output
2025-11-09 10:10 [PATCH 0/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
` (2 preceding siblings ...)
2025-11-09 10:10 ` [PATCH 3/9] test: cmd/bdinfo: make no flash assumption Heinrich Schuchardt
@ 2025-11-09 10:10 ` Heinrich Schuchardt
2025-11-14 12:44 ` Simon Glass
2025-11-09 10:10 ` [PATCH 5/9] test: common/print: do not use fixed buffer addresses Heinrich Schuchardt
` (4 subsequent siblings)
8 siblings, 1 reply; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-09 10:10 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot, Heinrich Schuchardt
On x86 commit 9b35dbc93fd4 ("x86: Show the timestamp counter with bdinfo")
has added another bdinfo output line.
On RISC-V commit 66b5ee9c558e ("riscv: add RISC-V fields to bdinfo
command") implemented arch_print_bdinfo().
Update the bdinfo test accordingly.
Fixes: 9b35dbc93fd4 ("x86: Show the timestamp counter with bdinfo")
Fixes: 66b5ee9c558e ("riscv: add RISC-V fields to bdinfo command")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
test/cmd/bdinfo.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c
index 214b237152b..28d448a0866 100644
--- a/test/cmd/bdinfo.c
+++ b/test/cmd/bdinfo.c
@@ -215,8 +215,15 @@ static int bdinfo_test_all(struct unit_test_state *uts)
ut_assertok(test_num_l(uts, "malloc base", gd_malloc_start()));
}
+ /* Check arch_print_bdinfo() output */
if (IS_ENABLED(CONFIG_X86))
- ut_check_skip_to_linen(uts, " high end =");
+ ut_check_skip_to_linen(uts, "tsc");
+
+#ifdef CONFIG_RISCV
+ ut_check_console_linen(uts, "boot hart");
+ if (gd->arch.firmware_fdt_addr)
+ ut_check_console_linen(uts, "firmware fdt");
+#endif
return 0;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 5/9] test: common/print: do not use fixed buffer addresses
2025-11-09 10:10 [PATCH 0/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
` (3 preceding siblings ...)
2025-11-09 10:10 ` [PATCH 4/9] test: cmd/bdinfo: consider arch_print_bdinfo() output Heinrich Schuchardt
@ 2025-11-09 10:10 ` Heinrich Schuchardt
2025-11-11 8:48 ` Ilias Apalodimas
2025-11-14 12:44 ` Simon Glass
2025-11-09 10:10 ` [PATCH 6/9] test: cmd/fdt: " Heinrich Schuchardt
` (3 subsequent siblings)
8 siblings, 2 replies; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-09 10:10 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot, Heinrich Schuchardt
The location of memory depends on the board. Do not assume memory at fixed
memory locations. Use calloc() instead to allocate buffers.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
test/common/print.c | 76 ++++++++++++++++++++++++---------------------
1 file changed, 40 insertions(+), 36 deletions(-)
diff --git a/test/common/print.c b/test/common/print.c
index 76ee851fe6a..2ac7eeed2c1 100644
--- a/test/common/print.c
+++ b/test/common/print.c
@@ -168,12 +168,8 @@ static int print_display_buffer(struct unit_test_state *uts)
u8 *buf;
int i;
- /* This test requires writable memory at zero */
- if (IS_ENABLED(CONFIG_X86))
- return -EAGAIN;
-
- buf = map_sysmem(0, BUF_SIZE);
- memset(buf, '\0', BUF_SIZE);
+ buf = calloc(1, BUF_SIZE);
+ ut_assertnonnull(buf);
for (i = 0; i < 0x11; i++)
buf[i] = i * 0x11;
@@ -232,7 +228,7 @@ static int print_display_buffer(struct unit_test_state *uts)
ut_assert_nextline("00000000: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~.....");
ut_assert_console_end();
- unmap_sysmem(buf);
+ free(buf);
return 0;
}
@@ -244,13 +240,14 @@ static int print_hexdump_line(struct unit_test_state *uts)
u8 *buf;
int i;
- buf = map_sysmem(0, BUF_SIZE);
- memset(buf, '\0', BUF_SIZE);
+ buf = calloc(1, BUF_SIZE);
+ ut_assertnonnull(buf);
for (i = 0; i < 0x11; i++)
buf[i] = i * 0x11;
/* Check buffer size calculations */
- linebuf = map_sysmem(0x400, BUF_SIZE);
+ linebuf = calloc(1, BUF_SIZE);
+ ut_assertnonnull(buf);
memset(linebuf, '\xff', BUF_SIZE);
ut_asserteq(-ENOSPC, hexdump_line(0, buf, 1, 0x10, 0, linebuf, 75));
ut_asserteq(0xff, linebuf[0]);
@@ -258,7 +255,8 @@ static int print_hexdump_line(struct unit_test_state *uts)
ut_asserteq('\0', linebuf[75]);
ut_asserteq(0xff, linebuf[76]);
- unmap_sysmem(buf);
+ free(linebuf);
+ free(buf);
return 0;
}
@@ -269,69 +267,74 @@ static int print_do_hex_dump(struct unit_test_state *uts)
u8 *buf;
int i;
- /* This test requires writable memory at zero */
- if (IS_ENABLED(CONFIG_X86))
- return -EAGAIN;
-
- buf = map_sysmem(0, BUF_SIZE);
- memset(buf, '\0', BUF_SIZE);
+ buf = calloc(1, BUF_SIZE);
+ ut_assertnonnull(buf);
for (i = 0; i < 0x11; i++)
buf[i] = i * 0x11;
/* bytes */
print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf);
ut_assert_nextline("%0*lx: 10 00 ..",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x10UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf + 0x10UL);
ut_assert_console_end();
/* line length */
print_hex_dump("", DUMP_PREFIX_ADDRESS, 8, 1, buf, 0x12, true);
ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 ..\"3DUfw",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf);
ut_assert_nextline("%0*lx: 88 99 aa bb cc dd ee ff ........",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x8UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf + 0x8UL);
ut_assert_nextline("%0*lx: 10 00 ..",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x10UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf + 0x10UL);
ut_assert_console_end();
- unmap_sysmem(buf);
/* long line */
buf[0x41] = 0x41;
print_hex_dump("", DUMP_PREFIX_ADDRESS, 0x40, 1, buf, 0x42, true);
ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ..\"3DUfw........................................................",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf);
ut_assert_nextline("%0*lx: 00 41 .A",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x40UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf + 0x40UL);
ut_assert_console_end();
/* 16-bit */
print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 2, buf, 0x12, true);
ut_assert_nextline("%0*lx: 1100 3322 5544 7766 9988 bbaa ddcc ffee ..\"3DUfw........",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf);
ut_assert_nextline("%0*lx: 0010 ..",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x10UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf + 0x10UL);
ut_assert_console_end();
- unmap_sysmem(buf);
/* 32-bit */
print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 4, buf, 0x14, true);
ut_assert_nextline("%0*lx: 33221100 77665544 bbaa9988 ffeeddcc ..\"3DUfw........",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf);
ut_assert_nextline("%0*lx: 00000010 ....",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x10UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf + 0x10UL);
ut_assert_console_end();
- unmap_sysmem(buf);
/* 64-bit */
print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 8, buf, 0x18, true);
ut_assert_nextline("%0*lx: 7766554433221100 ffeeddccbbaa9988 ..\"3DUfw........",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf);
ut_assert_nextline("%0*lx: 0000000000000010 ........",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x10UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf + 0x10UL);
ut_assert_console_end();
- unmap_sysmem(buf);
/* ASCII */
buf[1] = 31;
@@ -342,9 +345,10 @@ static int print_do_hex_dump(struct unit_test_state *uts)
buf[8] = 255;
print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 1, buf, 10, true);
ut_assert_nextline("%0*lx: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~.....",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
+ (uintptr_t)buf);
ut_assert_console_end();
- unmap_sysmem(buf);
+ free(buf);
return 0;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses
2025-11-09 10:10 [PATCH 0/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
` (4 preceding siblings ...)
2025-11-09 10:10 ` [PATCH 5/9] test: common/print: do not use fixed buffer addresses Heinrich Schuchardt
@ 2025-11-09 10:10 ` Heinrich Schuchardt
2025-11-11 8:43 ` Ilias Apalodimas
2025-11-14 12:31 ` Simon Glass
2025-11-09 10:10 ` [PATCH 7/9] common: default CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000 Heinrich Schuchardt
` (2 subsequent siblings)
8 siblings, 2 replies; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-09 10:10 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot, Heinrich Schuchardt
The location of memory depends on the board. Do not assume memory at fixed
memory locations. Use memalign() instead to allocate a buffer.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
test/cmd/fdt.c | 8 ++++++--
test/common/print.c | 37 ++++++++++++++++---------------------
2 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
index b950123b6da..4c3c6308ab4 100644
--- a/test/cmd/fdt.c
+++ b/test/cmd/fdt.c
@@ -265,7 +265,7 @@ FDT_TEST(fdt_test_addr_resize, UTF_CONSOLE);
static int fdt_test_move(struct unit_test_state *uts)
{
char fdt[256];
- ulong addr, newaddr = 0x10000;
+ ulong addr, newaddr;
const int size = sizeof(fdt);
uint32_t ts;
void *buf;
@@ -275,8 +275,10 @@ static int fdt_test_move(struct unit_test_state *uts)
ts = fdt_totalsize(fdt);
/* Moved target DT location */
- buf = map_sysmem(newaddr, size);
+ buf = memalign(8, size);
+ ut_assertnonnull(buf);
memset(buf, 0, size);
+ newaddr = map_to_sysmem(buf);
/* Test moving the working FDT to a new location */
ut_assertok(run_commandf("fdt move %08lx %08lx %x", addr, newaddr, ts));
@@ -288,6 +290,8 @@ static int fdt_test_move(struct unit_test_state *uts)
ut_assert_nextline("Total of %d byte(s) were the same", ts);
ut_assert_console_end();
+ free(buf);
+
return 0;
}
FDT_TEST(fdt_test_move, UTF_CONSOLE);
diff --git a/test/common/print.c b/test/common/print.c
index 2ac7eeed2c1..f33476a36b4 100644
--- a/test/common/print.c
+++ b/test/common/print.c
@@ -266,74 +266,70 @@ static int print_do_hex_dump(struct unit_test_state *uts)
{
u8 *buf;
int i;
+ ulong addr;
buf = calloc(1, BUF_SIZE);
ut_assertnonnull(buf);
+ addr = map_to_sysmem(buf);
for (i = 0; i < 0x11; i++)
buf[i] = i * 0x11;
/* bytes */
print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
ut_assert_nextline("%0*lx: 10 00 ..",
IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf + 0x10UL);
+ addr + 0x10UL);
ut_assert_console_end();
/* line length */
print_hex_dump("", DUMP_PREFIX_ADDRESS, 8, 1, buf, 0x12, true);
ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 ..\"3DUfw",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
ut_assert_nextline("%0*lx: 88 99 aa bb cc dd ee ff ........",
IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf + 0x8UL);
+ addr + 0x8UL);
ut_assert_nextline("%0*lx: 10 00 ..",
IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf + 0x10UL);
+ addr + 0x10UL);
ut_assert_console_end();
/* long line */
buf[0x41] = 0x41;
print_hex_dump("", DUMP_PREFIX_ADDRESS, 0x40, 1, buf, 0x42, true);
ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ..\"3DUfw........................................................",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
ut_assert_nextline("%0*lx: 00 41 .A",
IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf + 0x40UL);
+ addr + 0x40UL);
ut_assert_console_end();
/* 16-bit */
print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 2, buf, 0x12, true);
ut_assert_nextline("%0*lx: 1100 3322 5544 7766 9988 bbaa ddcc ffee ..\"3DUfw........",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
ut_assert_nextline("%0*lx: 0010 ..",
IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf + 0x10UL);
+ addr + 0x10UL);
ut_assert_console_end();
/* 32-bit */
print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 4, buf, 0x14, true);
ut_assert_nextline("%0*lx: 33221100 77665544 bbaa9988 ffeeddcc ..\"3DUfw........",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
ut_assert_nextline("%0*lx: 00000010 ....",
IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf + 0x10UL);
+ addr + 0x10UL);
ut_assert_console_end();
/* 64-bit */
print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 8, buf, 0x18, true);
ut_assert_nextline("%0*lx: 7766554433221100 ffeeddccbbaa9988 ..\"3DUfw........",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
ut_assert_nextline("%0*lx: 0000000000000010 ........",
IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf + 0x10UL);
+ addr + 0x10UL);
ut_assert_console_end();
/* ASCII */
@@ -345,8 +341,7 @@ static int print_do_hex_dump(struct unit_test_state *uts)
buf[8] = 255;
print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 1, buf, 10, true);
ut_assert_nextline("%0*lx: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~.....",
- IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
- (uintptr_t)buf);
+ IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
ut_assert_console_end();
free(buf);
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 7/9] common: default CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000
2025-11-09 10:10 [PATCH 0/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
` (5 preceding siblings ...)
2025-11-09 10:10 ` [PATCH 6/9] test: cmd/fdt: " Heinrich Schuchardt
@ 2025-11-09 10:10 ` Heinrich Schuchardt
2025-11-11 8:31 ` Ilias Apalodimas
2025-11-09 10:10 ` [PATCH 8/9] configs: CONFIG_CONSOLE_RECORD=y on qemu-riscv64_smode_acpi Heinrich Schuchardt
2025-11-09 10:10 ` [PATCH RESEND 9/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
8 siblings, 1 reply; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-09 10:10 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot, Heinrich Schuchardt
For some tests the current default of 0x400 for
CONFIG_CONSOLE_RECORD_OUT_SIZE is too small.
Raise the value to 0x6000 which is already the most common value.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
common/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/Kconfig b/common/Kconfig
index 0c25a8fc866..47d17f4e7c6 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -26,7 +26,7 @@ config CONSOLE_RECORD_INIT_F
config CONSOLE_RECORD_OUT_SIZE
hex "Output buffer size"
depends on CONSOLE_RECORD
- default 0x400 if CONSOLE_RECORD
+ default 0x6000
help
Set the size of the console recording output buffer. When this fills
up, no more data will be recorded until some is removed. The buffer
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 8/9] configs: CONFIG_CONSOLE_RECORD=y on qemu-riscv64_smode_acpi
2025-11-09 10:10 [PATCH 0/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
` (6 preceding siblings ...)
2025-11-09 10:10 ` [PATCH 7/9] common: default CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000 Heinrich Schuchardt
@ 2025-11-09 10:10 ` Heinrich Schuchardt
2025-11-09 10:10 ` [PATCH RESEND 9/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
8 siblings, 0 replies; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-09 10:10 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot, Heinrich Schuchardt
For testing ACPI on QEMU we need a defconfig that supports acpi command
test.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
configs/qemu-riscv64_smode_acpi_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/configs/qemu-riscv64_smode_acpi_defconfig b/configs/qemu-riscv64_smode_acpi_defconfig
index e78e5ffb390..11592091443 100644
--- a/configs/qemu-riscv64_smode_acpi_defconfig
+++ b/configs/qemu-riscv64_smode_acpi_defconfig
@@ -1,2 +1,3 @@
#include <configs/qemu-riscv64_smode_defconfig>
#include <board/emulation/configs/acpi.config>
+CONFIG_CONSOLE_RECORD=y
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH RESEND 9/9] CI: test qemu-riscv64_smode[_acpi]
2025-11-09 10:10 [PATCH 0/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
` (7 preceding siblings ...)
2025-11-09 10:10 ` [PATCH 8/9] configs: CONFIG_CONSOLE_RECORD=y on qemu-riscv64_smode_acpi Heinrich Schuchardt
@ 2025-11-09 10:10 ` Heinrich Schuchardt
8 siblings, 0 replies; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-09 10:10 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot, Heinrich Schuchardt
QEMU comes with its own OpenSBI. For running RISC-V virtual machine
using one of qemu-riscv64_smode_defconfig or
qemu-riscv64_smode_acpi_defconfig is the natural choice.
Add the riscv64 smode configurations to the test scope.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
.azure-pipelines.yml | 6 ++++++
.gitlab-ci.yml | 12 ++++++++++++
2 files changed, 18 insertions(+)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 3b684ec73ba..06f999aab1a 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -522,6 +522,12 @@ stages:
qemu_riscv64_spl:
TEST_PY_BD: "qemu-riscv64_spl"
TEST_PY_TEST_SPEC: "not sleep"
+ qemu_riscv64_smode:
+ TEST_PY_BD: "qemu-riscv64_smode"
+ TEST_PY_TEST_SPEC: "not sleep"
+ qemu_riscv64_smode_acpi:
+ TEST_PY_BD: "qemu-riscv64_smode_acpi"
+ TEST_PY_TEST_SPEC: "not sleep"
qemu_x86:
TEST_PY_BD: "qemu-x86"
TEST_PY_TEST_SPEC: "not sleep"
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 0337380f98e..7cd0e521049 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -474,6 +474,18 @@ qemu-riscv64_spl test.py:
TEST_PY_TEST_SPEC: "not sleep"
<<: *buildman_and_testpy_dfn
+qemu-riscv64_smode test.py:
+ variables:
+ TEST_PY_BD: "qemu-riscv64_smode"
+ TEST_PY_TEST_SPEC: "not sleep"
+ <<: *buildman_and_testpy_dfn
+
+qemu-riscv64_smode_acpi test.py:
+ variables:
+ TEST_PY_BD: "qemu-riscv64_smode_acpi"
+ TEST_PY_TEST_SPEC: "not sleep"
+ <<: *buildman_and_testpy_dfn
+
qemu-x86 test.py:
variables:
TEST_PY_BD: "qemu-x86"
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY
2025-11-09 10:10 ` [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY Heinrich Schuchardt
@ 2025-11-11 8:29 ` Ilias Apalodimas
2025-11-11 13:32 ` Tom Rini
2025-11-11 21:12 ` Heinrich Schuchardt
2025-11-22 16:23 ` Tom Rini
1 sibling, 2 replies; 37+ messages in thread
From: Ilias Apalodimas @ 2025-11-11 8:29 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Bin Meng, Simon Glass, Raymond Mao, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
Hi Heinrich
On Sun, 9 Nov 2025 at 12:10, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> The `fdt apply` sub-command is only available if CONFIG_OF_LIBFDT_OVERLAY
> is enabled.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> test/cmd/fdt.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> index 96a8488e172..a36f2dcdda8 100644
> --- a/test/cmd/fdt.c
> +++ b/test/cmd/fdt.c
> @@ -1319,6 +1319,9 @@ static int fdt_test_apply(struct unit_test_state *uts)
> char fdt[8192], fdto[8192];
> ulong addr, addro;
>
> + if (!IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY))
> + return -EAGAIN;
The asserts return CMD_RET_FAILURE on failures. Should't we use the same here?
> +
> /* Create base DT with __symbols__ node */
> ut_assertok(fdt_create(fdt, sizeof(fdt)));
> ut_assertok(fdt_finish_reservemap(fdt));
> --
> 2.51.0
>
Regards
/Ilias
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 3/9] test: cmd/bdinfo: make no flash assumption
2025-11-09 10:10 ` [PATCH 3/9] test: cmd/bdinfo: make no flash assumption Heinrich Schuchardt
@ 2025-11-11 8:30 ` Ilias Apalodimas
2025-11-14 12:44 ` Simon Glass
1 sibling, 0 replies; 37+ messages in thread
From: Ilias Apalodimas @ 2025-11-11 8:30 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Bin Meng, Simon Glass, Raymond Mao, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
On Sun, 9 Nov 2025 at 12:10, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> The location and size of flash is device-dependent. Do not make any
> assumption about the location and size.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> test/cmd/bdinfo.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c
> index 09f44ee41ed..214b237152b 100644
> --- a/test/cmd/bdinfo.c
> +++ b/test/cmd/bdinfo.c
> @@ -161,9 +161,9 @@ static int bdinfo_test_all(struct unit_test_state *uts)
> ut_assertok(bdinfo_check_mem(uts));
>
> /* CONFIG_SYS_HAS_SRAM testing not supported */
> - ut_assertok(test_num_l(uts, "flashstart", 0));
> - ut_assertok(test_num_l(uts, "flashsize", 0));
> - ut_assertok(test_num_l(uts, "flashoffset", 0));
> + ut_check_console_linen(uts, "flashstart");
> + ut_check_console_linen(uts, "flashsize");
> + ut_check_console_linen(uts, "flashoffset");
> ut_assert_nextline("baudrate = %lu bps",
> env_get_ulong("baudrate", 10, 1234));
> ut_assertok(test_num_l(uts, "relocaddr", gd->relocaddr));
> --
> 2.51.0
>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 7/9] common: default CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000
2025-11-09 10:10 ` [PATCH 7/9] common: default CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000 Heinrich Schuchardt
@ 2025-11-11 8:31 ` Ilias Apalodimas
2025-11-14 12:44 ` Simon Glass
0 siblings, 1 reply; 37+ messages in thread
From: Ilias Apalodimas @ 2025-11-11 8:31 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Bin Meng, Simon Glass, Raymond Mao, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
On Sun, 9 Nov 2025 at 12:10, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> For some tests the current default of 0x400 for
> CONFIG_CONSOLE_RECORD_OUT_SIZE is too small.
>
> Raise the value to 0x6000 which is already the most common value.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> common/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 0c25a8fc866..47d17f4e7c6 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -26,7 +26,7 @@ config CONSOLE_RECORD_INIT_F
> config CONSOLE_RECORD_OUT_SIZE
> hex "Output buffer size"
> depends on CONSOLE_RECORD
> - default 0x400 if CONSOLE_RECORD
> + default 0x6000
> help
> Set the size of the console recording output buffer. When this fills
> up, no more data will be recorded until some is removed. The buffer
> --
> 2.51.0
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists
2025-11-09 10:10 ` [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists Heinrich Schuchardt
@ 2025-11-11 8:33 ` Ilias Apalodimas
2025-11-14 12:44 ` Simon Glass
1 sibling, 0 replies; 37+ messages in thread
From: Ilias Apalodimas @ 2025-11-11 8:33 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Bin Meng, Simon Glass, Raymond Mao, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
On Sun, 9 Nov 2025 at 12:10, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> In fdt_test_chosen() currently we test if DM_RNG is configured.
> CONFIG_DM_RNG=y does not imply that a RNG device actually exists.
> For instance QEMU may be called with -device virtio-rng-device or not.
> The current test framework evicts the virtio RNG device even if QEMU is
> called with -device virtio-rng-device.
>
> In the fdt_test_chosen() check if a RNG device exists.
> Ignore 'No RNG device' messages.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> test/cmd/fdt.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> index a36f2dcdda8..b950123b6da 100644
> --- a/test/cmd/fdt.c
> +++ b/test/cmd/fdt.c
> @@ -10,6 +10,7 @@
> #include <fdt_support.h>
> #include <mapmem.h>
> #include <asm/global_data.h>
> +#include <dm/uclass.h>
> #include <linux/libfdt.h>
> #include <test/ut.h>
>
> @@ -1267,6 +1268,7 @@ static int fdt_test_chosen(struct unit_test_state *uts)
> {
> const char *env_bootargs = env_get("bootargs");
> char fdt[8192];
> + struct udevice *dev;
> ulong addr;
>
> ut_assertok(make_test_fdt(uts, fdt, sizeof(fdt), &addr));
> @@ -1280,11 +1282,16 @@ static int fdt_test_chosen(struct unit_test_state *uts)
> /* Test add new chosen node without initrd */
> ut_assertok(run_commandf("fdt chosen"));
> ut_assertok(run_commandf("fdt print /chosen"));
> - ut_assert_nextline("chosen {");
> + ut_assert(0 < console_record_readline(uts->actual_str,
> + sizeof(uts->actual_str)));
> + if (!strcmp("No RNG device", uts->actual_str))
> + ut_assert(0 < console_record_readline(uts->actual_str,
> + sizeof(uts->actual_str)));
> + ut_asserteq_str("chosen {", uts->actual_str);
> ut_assert_nextlinen("\tu-boot,version = "); /* Ignore the version string */
> if (env_bootargs)
> ut_assert_nextline("\tbootargs = \"%s\";", env_bootargs);
> - if (IS_ENABLED(CONFIG_DM_RNG) &&
> + if (!uclass_get_device(UCLASS_RNG, 0, &dev) &&
> !IS_ENABLED(CONFIG_MEASURED_BOOT) &&
> !IS_ENABLED(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT))
> ut_assert_nextlinen("\tkaslr-seed = ");
> @@ -1294,7 +1301,12 @@ static int fdt_test_chosen(struct unit_test_state *uts)
> /* Test add new chosen node with initrd */
> ut_assertok(run_commandf("fdt chosen 0x1234 0x5678"));
> ut_assertok(run_commandf("fdt print /chosen"));
> - ut_assert_nextline("chosen {");
> + ut_assert(0 < console_record_readline(uts->actual_str,
> + sizeof(uts->actual_str)));
> + if (!strcmp("No RNG device", uts->actual_str))
> + ut_assert(0 < console_record_readline(uts->actual_str,
> + sizeof(uts->actual_str)));
> + ut_asserteq_str("chosen {", uts->actual_str);
> ut_assert_nextline("\tlinux,initrd-end = <0x%08x 0x%08x>;",
> upper_32_bits(0x1234 + 0x5678 - 1),
> lower_32_bits(0x1234 + 0x5678 - 1));
> @@ -1303,7 +1315,7 @@ static int fdt_test_chosen(struct unit_test_state *uts)
> ut_assert_nextlinen("\tu-boot,version = "); /* Ignore the version string */
> if (env_bootargs)
> ut_assert_nextline("\tbootargs = \"%s\";", env_bootargs);
> - if (IS_ENABLED(CONFIG_DM_RNG) &&
> + if (!uclass_get_device(UCLASS_RNG, 0, &dev) &&
> !IS_ENABLED(CONFIG_MEASURED_BOOT) &&
> !IS_ENABLED(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT))
> ut_assert_nextlinen("\tkaslr-seed = ");
> --
> 2.51.0
>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses
2025-11-09 10:10 ` [PATCH 6/9] test: cmd/fdt: " Heinrich Schuchardt
@ 2025-11-11 8:43 ` Ilias Apalodimas
2025-11-14 12:31 ` Simon Glass
1 sibling, 0 replies; 37+ messages in thread
From: Ilias Apalodimas @ 2025-11-11 8:43 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Bin Meng, Simon Glass, Raymond Mao, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
On Sun, 9 Nov 2025 at 12:10, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> The location of memory depends on the board. Do not assume memory at fixed
> memory locations. Use memalign() instead to allocate a buffer.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> test/cmd/fdt.c | 8 ++++++--
> test/common/print.c | 37 ++++++++++++++++---------------------
> 2 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> index b950123b6da..4c3c6308ab4 100644
> --- a/test/cmd/fdt.c
> +++ b/test/cmd/fdt.c
> @@ -265,7 +265,7 @@ FDT_TEST(fdt_test_addr_resize, UTF_CONSOLE);
> static int fdt_test_move(struct unit_test_state *uts)
> {
> char fdt[256];
> - ulong addr, newaddr = 0x10000;
> + ulong addr, newaddr;
> const int size = sizeof(fdt);
> uint32_t ts;
> void *buf;
> @@ -275,8 +275,10 @@ static int fdt_test_move(struct unit_test_state *uts)
> ts = fdt_totalsize(fdt);
>
> /* Moved target DT location */
> - buf = map_sysmem(newaddr, size);
> + buf = memalign(8, size);
> + ut_assertnonnull(buf);
> memset(buf, 0, size);
> + newaddr = map_to_sysmem(buf);
>
> /* Test moving the working FDT to a new location */
> ut_assertok(run_commandf("fdt move %08lx %08lx %x", addr, newaddr, ts));
> @@ -288,6 +290,8 @@ static int fdt_test_move(struct unit_test_state *uts)
> ut_assert_nextline("Total of %d byte(s) were the same", ts);
> ut_assert_console_end();
>
> + free(buf);
> +
> return 0;
> }
> FDT_TEST(fdt_test_move, UTF_CONSOLE);
> diff --git a/test/common/print.c b/test/common/print.c
> index 2ac7eeed2c1..f33476a36b4 100644
> --- a/test/common/print.c
> +++ b/test/common/print.c
> @@ -266,74 +266,70 @@ static int print_do_hex_dump(struct unit_test_state *uts)
> {
> u8 *buf;
> int i;
> + ulong addr;
>
> buf = calloc(1, BUF_SIZE);
> ut_assertnonnull(buf);
> + addr = map_to_sysmem(buf);
> for (i = 0; i < 0x11; i++)
> buf[i] = i * 0x11;
>
> /* bytes */
> print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 10 00 ..",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x10UL);
> + addr + 0x10UL);
> ut_assert_console_end();
>
> /* line length */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 8, 1, buf, 0x12, true);
> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 ..\"3DUfw",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 88 99 aa bb cc dd ee ff ........",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x8UL);
> + addr + 0x8UL);
> ut_assert_nextline("%0*lx: 10 00 ..",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x10UL);
> + addr + 0x10UL);
> ut_assert_console_end();
>
> /* long line */
> buf[0x41] = 0x41;
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0x40, 1, buf, 0x42, true);
> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ..\"3DUfw........................................................",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 00 41 .A",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x40UL);
> + addr + 0x40UL);
> ut_assert_console_end();
>
> /* 16-bit */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 2, buf, 0x12, true);
> ut_assert_nextline("%0*lx: 1100 3322 5544 7766 9988 bbaa ddcc ffee ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 0010 ..",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x10UL);
> + addr + 0x10UL);
> ut_assert_console_end();
>
> /* 32-bit */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 4, buf, 0x14, true);
> ut_assert_nextline("%0*lx: 33221100 77665544 bbaa9988 ffeeddcc ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 00000010 ....",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x10UL);
> + addr + 0x10UL);
> ut_assert_console_end();
>
> /* 64-bit */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 8, buf, 0x18, true);
> ut_assert_nextline("%0*lx: 7766554433221100 ffeeddccbbaa9988 ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 0000000000000010 ........",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x10UL);
> + addr + 0x10UL);
> ut_assert_console_end();
>
> /* ASCII */
> @@ -345,8 +341,7 @@ static int print_do_hex_dump(struct unit_test_state *uts)
> buf[8] = 255;
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 1, buf, 10, true);
> ut_assert_nextline("%0*lx: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~.....",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_console_end();
> free(buf);
>
> --
> 2.51.0
>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 5/9] test: common/print: do not use fixed buffer addresses
2025-11-09 10:10 ` [PATCH 5/9] test: common/print: do not use fixed buffer addresses Heinrich Schuchardt
@ 2025-11-11 8:48 ` Ilias Apalodimas
2025-11-14 12:44 ` Simon Glass
1 sibling, 0 replies; 37+ messages in thread
From: Ilias Apalodimas @ 2025-11-11 8:48 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Bin Meng, Simon Glass, Raymond Mao, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
On Sun, 9 Nov 2025 at 12:10, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> The location of memory depends on the board. Do not assume memory at fixed
> memory locations. Use calloc() instead to allocate buffers.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> test/common/print.c | 76 ++++++++++++++++++++++++---------------------
> 1 file changed, 40 insertions(+), 36 deletions(-)
>
> diff --git a/test/common/print.c b/test/common/print.c
> index 76ee851fe6a..2ac7eeed2c1 100644
> --- a/test/common/print.c
> +++ b/test/common/print.c
> @@ -168,12 +168,8 @@ static int print_display_buffer(struct unit_test_state *uts)
> u8 *buf;
> int i;
>
> - /* This test requires writable memory at zero */
> - if (IS_ENABLED(CONFIG_X86))
> - return -EAGAIN;
> -
> - buf = map_sysmem(0, BUF_SIZE);
> - memset(buf, '\0', BUF_SIZE);
> + buf = calloc(1, BUF_SIZE);
> + ut_assertnonnull(buf);
> for (i = 0; i < 0x11; i++)
> buf[i] = i * 0x11;
>
> @@ -232,7 +228,7 @@ static int print_display_buffer(struct unit_test_state *uts)
> ut_assert_nextline("00000000: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~.....");
> ut_assert_console_end();
>
> - unmap_sysmem(buf);
> + free(buf);
>
> return 0;
> }
> @@ -244,13 +240,14 @@ static int print_hexdump_line(struct unit_test_state *uts)
> u8 *buf;
> int i;
>
> - buf = map_sysmem(0, BUF_SIZE);
> - memset(buf, '\0', BUF_SIZE);
> + buf = calloc(1, BUF_SIZE);
> + ut_assertnonnull(buf);
> for (i = 0; i < 0x11; i++)
> buf[i] = i * 0x11;
>
> /* Check buffer size calculations */
> - linebuf = map_sysmem(0x400, BUF_SIZE);
> + linebuf = calloc(1, BUF_SIZE);
> + ut_assertnonnull(buf);
> memset(linebuf, '\xff', BUF_SIZE);
> ut_asserteq(-ENOSPC, hexdump_line(0, buf, 1, 0x10, 0, linebuf, 75));
> ut_asserteq(0xff, linebuf[0]);
> @@ -258,7 +255,8 @@ static int print_hexdump_line(struct unit_test_state *uts)
> ut_asserteq('\0', linebuf[75]);
> ut_asserteq(0xff, linebuf[76]);
>
> - unmap_sysmem(buf);
> + free(linebuf);
> + free(buf);
>
> return 0;
> }
> @@ -269,69 +267,74 @@ static int print_do_hex_dump(struct unit_test_state *uts)
> u8 *buf;
> int i;
>
> - /* This test requires writable memory at zero */
> - if (IS_ENABLED(CONFIG_X86))
> - return -EAGAIN;
> -
> - buf = map_sysmem(0, BUF_SIZE);
> - memset(buf, '\0', BUF_SIZE);
> + buf = calloc(1, BUF_SIZE);
> + ut_assertnonnull(buf);
> for (i = 0; i < 0x11; i++)
> buf[i] = i * 0x11;
>
> /* bytes */
> print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf);
> ut_assert_nextline("%0*lx: 10 00 ..",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x10UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf + 0x10UL);
> ut_assert_console_end();
>
> /* line length */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 8, 1, buf, 0x12, true);
> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 ..\"3DUfw",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf);
> ut_assert_nextline("%0*lx: 88 99 aa bb cc dd ee ff ........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x8UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf + 0x8UL);
> ut_assert_nextline("%0*lx: 10 00 ..",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x10UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf + 0x10UL);
> ut_assert_console_end();
> - unmap_sysmem(buf);
>
> /* long line */
> buf[0x41] = 0x41;
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0x40, 1, buf, 0x42, true);
> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ..\"3DUfw........................................................",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf);
> ut_assert_nextline("%0*lx: 00 41 .A",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x40UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf + 0x40UL);
> ut_assert_console_end();
>
> /* 16-bit */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 2, buf, 0x12, true);
> ut_assert_nextline("%0*lx: 1100 3322 5544 7766 9988 bbaa ddcc ffee ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf);
> ut_assert_nextline("%0*lx: 0010 ..",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x10UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf + 0x10UL);
> ut_assert_console_end();
> - unmap_sysmem(buf);
>
> /* 32-bit */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 4, buf, 0x14, true);
> ut_assert_nextline("%0*lx: 33221100 77665544 bbaa9988 ffeeddcc ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf);
> ut_assert_nextline("%0*lx: 00000010 ....",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x10UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf + 0x10UL);
> ut_assert_console_end();
> - unmap_sysmem(buf);
>
> /* 64-bit */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 8, buf, 0x18, true);
> ut_assert_nextline("%0*lx: 7766554433221100 ffeeddccbbaa9988 ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf);
> ut_assert_nextline("%0*lx: 0000000000000010 ........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x10UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf + 0x10UL);
> ut_assert_console_end();
> - unmap_sysmem(buf);
>
> /* ASCII */
> buf[1] = 31;
> @@ -342,9 +345,10 @@ static int print_do_hex_dump(struct unit_test_state *uts)
> buf[8] = 255;
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 1, buf, 10, true);
> ut_assert_nextline("%0*lx: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~.....",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, 0x0UL);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> + (uintptr_t)buf);
> ut_assert_console_end();
> - unmap_sysmem(buf);
> + free(buf);
>
> return 0;
> }
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY
2025-11-11 8:29 ` Ilias Apalodimas
@ 2025-11-11 13:32 ` Tom Rini
2025-11-11 21:18 ` Heinrich Schuchardt
2025-11-11 21:12 ` Heinrich Schuchardt
1 sibling, 1 reply; 37+ messages in thread
From: Tom Rini @ 2025-11-11 13:32 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: Heinrich Schuchardt, Bin Meng, Simon Glass, Raymond Mao,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]
On Tue, Nov 11, 2025 at 10:29:23AM +0200, Ilias Apalodimas wrote:
> Hi Heinrich
>
> On Sun, 9 Nov 2025 at 12:10, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > The `fdt apply` sub-command is only available if CONFIG_OF_LIBFDT_OVERLAY
> > is enabled.
> >
> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > ---
> > test/cmd/fdt.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> > index 96a8488e172..a36f2dcdda8 100644
> > --- a/test/cmd/fdt.c
> > +++ b/test/cmd/fdt.c
> > @@ -1319,6 +1319,9 @@ static int fdt_test_apply(struct unit_test_state *uts)
> > char fdt[8192], fdto[8192];
> > ulong addr, addro;
> >
> > + if (!IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY))
> > + return -EAGAIN;
>
> The asserts return CMD_RET_FAILURE on failures. Should't we use the same here?
>
> > +
> > /* Create base DT with __symbols__ node */
> > ut_assertok(fdt_create(fdt, sizeof(fdt)));
> > ut_assertok(fdt_finish_reservemap(fdt));
Or just guard the whole test if #ifdef CONFIG_OF_LIBFDT_OVERLAY ...
#endif.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY
2025-11-11 8:29 ` Ilias Apalodimas
2025-11-11 13:32 ` Tom Rini
@ 2025-11-11 21:12 ` Heinrich Schuchardt
2025-11-11 21:26 ` Tom Rini
1 sibling, 1 reply; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-11 21:12 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: Tom Rini, Bin Meng, Simon Glass, Raymond Mao, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
On 11/11/25 09:29, Ilias Apalodimas wrote:
> Hi Heinrich
>
> On Sun, 9 Nov 2025 at 12:10, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> The `fdt apply` sub-command is only available if CONFIG_OF_LIBFDT_OVERLAY
>> is enabled.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>> test/cmd/fdt.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
>> index 96a8488e172..a36f2dcdda8 100644
>> --- a/test/cmd/fdt.c
>> +++ b/test/cmd/fdt.c
>> @@ -1319,6 +1319,9 @@ static int fdt_test_apply(struct unit_test_state *uts)
>> char fdt[8192], fdto[8192];
>> ulong addr, addro;
>>
>> + if (!IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY))
>> + return -EAGAIN;
>
> The asserts return CMD_RET_FAILURE on failures. Should't we use the same here?
There is nothing wrong in CONFIG_OF_LIBFDT_OVERLAY=n. This is not a failure.
-EAGAIN signals a skipped test.
Look for EAGAIN in test/test-main.c.
We should describe this in doc/develop/tests_writing.rst.
Best regards
Heinrich
>
>> +
>> /* Create base DT with __symbols__ node */
>> ut_assertok(fdt_create(fdt, sizeof(fdt)));
>> ut_assertok(fdt_finish_reservemap(fdt));
>> --
>> 2.51.0
>>
>
> Regards
> /Ilias
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY
2025-11-11 13:32 ` Tom Rini
@ 2025-11-11 21:18 ` Heinrich Schuchardt
0 siblings, 0 replies; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-11 21:18 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Michal Simek, Adriano Cordova,
Harrison Mutai, Peng Fan, Ben Dooks, Svyatoslav Ryhel,
Sam Protsenko, Mattijs Korpershoek, Jerome Forissier,
Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen, u-boot,
Ilias Apalodimas
On 11/11/25 14:32, Tom Rini wrote:
> On Tue, Nov 11, 2025 at 10:29:23AM +0200, Ilias Apalodimas wrote:
>> Hi Heinrich
>>
>> On Sun, 9 Nov 2025 at 12:10, Heinrich Schuchardt
>> <heinrich.schuchardt@canonical.com> wrote:
>>>
>>> The `fdt apply` sub-command is only available if CONFIG_OF_LIBFDT_OVERLAY
>>> is enabled.
>>>
>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>> ---
>>> test/cmd/fdt.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
>>> index 96a8488e172..a36f2dcdda8 100644
>>> --- a/test/cmd/fdt.c
>>> +++ b/test/cmd/fdt.c
>>> @@ -1319,6 +1319,9 @@ static int fdt_test_apply(struct unit_test_state *uts)
>>> char fdt[8192], fdto[8192];
>>> ulong addr, addro;
>>>
>>> + if (!IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY))
>>> + return -EAGAIN;
>>
>> The asserts return CMD_RET_FAILURE on failures. Should't we use the same here?
>>
>>> +
>>> /* Create base DT with __symbols__ node */
>>> ut_assertok(fdt_create(fdt, sizeof(fdt)));
>>> ut_assertok(fdt_finish_reservemap(fdt));
>
> Or just guard the whole test if #ifdef CONFIG_OF_LIBFDT_OVERLAY ...
> #endif.
>
-EGAIN is used for skipping in a good number of tests.
See also the usage in test/test-main.c.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY
2025-11-11 21:12 ` Heinrich Schuchardt
@ 2025-11-11 21:26 ` Tom Rini
0 siblings, 0 replies; 37+ messages in thread
From: Tom Rini @ 2025-11-11 21:26 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Ilias Apalodimas, Bin Meng, Simon Glass, Raymond Mao,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
[-- Attachment #1: Type: text/plain, Size: 1417 bytes --]
On Tue, Nov 11, 2025 at 10:12:58PM +0100, Heinrich Schuchardt wrote:
> On 11/11/25 09:29, Ilias Apalodimas wrote:
> > Hi Heinrich
> >
> > On Sun, 9 Nov 2025 at 12:10, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> > >
> > > The `fdt apply` sub-command is only available if CONFIG_OF_LIBFDT_OVERLAY
> > > is enabled.
> > >
> > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > ---
> > > test/cmd/fdt.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> > > index 96a8488e172..a36f2dcdda8 100644
> > > --- a/test/cmd/fdt.c
> > > +++ b/test/cmd/fdt.c
> > > @@ -1319,6 +1319,9 @@ static int fdt_test_apply(struct unit_test_state *uts)
> > > char fdt[8192], fdto[8192];
> > > ulong addr, addro;
> > >
> > > + if (!IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY))
> > > + return -EAGAIN;
> >
> > The asserts return CMD_RET_FAILURE on failures. Should't we use the same here?
>
> There is nothing wrong in CONFIG_OF_LIBFDT_OVERLAY=n. This is not a failure.
>
> -EAGAIN signals a skipped test.
> Look for EAGAIN in test/test-main.c.
>
> We should describe this in doc/develop/tests_writing.rst.
We should document it, yes. I don't have a strong preference over using
this rather than just not compiling the test in to start with.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses
2025-11-09 10:10 ` [PATCH 6/9] test: cmd/fdt: " Heinrich Schuchardt
2025-11-11 8:43 ` Ilias Apalodimas
@ 2025-11-14 12:31 ` Simon Glass
2025-11-14 14:19 ` Tom Rini
2025-11-14 14:29 ` Heinrich Schuchardt
1 sibling, 2 replies; 37+ messages in thread
From: Simon Glass @ 2025-11-14 12:31 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Bin Meng, Raymond Mao, Ilias Apalodimas, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
Hi Heinrich,
On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> The location of memory depends on the board. Do not assume memory at fixed
> memory locations. Use memalign() instead to allocate a buffer.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> test/cmd/fdt.c | 8 ++++++--
> test/common/print.c | 37 ++++++++++++++++---------------------
> 2 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> index b950123b6da..4c3c6308ab4 100644
> --- a/test/cmd/fdt.c
> +++ b/test/cmd/fdt.c
> @@ -265,7 +265,7 @@ FDT_TEST(fdt_test_addr_resize, UTF_CONSOLE);
> static int fdt_test_move(struct unit_test_state *uts)
> {
> char fdt[256];
> - ulong addr, newaddr = 0x10000;
> + ulong addr, newaddr;
> const int size = sizeof(fdt);
> uint32_t ts;
> void *buf;
> @@ -275,8 +275,10 @@ static int fdt_test_move(struct unit_test_state *uts)
> ts = fdt_totalsize(fdt);
>
> /* Moved target DT location */
> - buf = map_sysmem(newaddr, size);
> + buf = memalign(8, size);
> + ut_assertnonnull(buf);
> memset(buf, 0, size);
> + newaddr = map_to_sysmem(buf);
>
> /* Test moving the working FDT to a new location */
> ut_assertok(run_commandf("fdt move %08lx %08lx %x", addr, newaddr, ts));
> @@ -288,6 +290,8 @@ static int fdt_test_move(struct unit_test_state *uts)
> ut_assert_nextline("Total of %d byte(s) were the same", ts);
> ut_assert_console_end();
>
> + free(buf);
> +
> return 0;
> }
> FDT_TEST(fdt_test_move, UTF_CONSOLE);
> diff --git a/test/common/print.c b/test/common/print.c
> index 2ac7eeed2c1..f33476a36b4 100644
> --- a/test/common/print.c
> +++ b/test/common/print.c
> @@ -266,74 +266,70 @@ static int print_do_hex_dump(struct unit_test_state *uts)
> {
> u8 *buf;
> int i;
> + ulong addr;
>
> buf = calloc(1, BUF_SIZE);
> ut_assertnonnull(buf);
> + addr = map_to_sysmem(buf);
> for (i = 0; i < 0x11; i++)
> buf[i] = i * 0x11;
>
> /* bytes */
> print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 10 00 ..",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x10UL);
> + addr + 0x10UL);
> ut_assert_console_end();
>
> /* line length */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 8, 1, buf, 0x12, true);
> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 ..\"3DUfw",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 88 99 aa bb cc dd ee ff ........",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x8UL);
> + addr + 0x8UL);
> ut_assert_nextline("%0*lx: 10 00 ..",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x10UL);
> + addr + 0x10UL);
> ut_assert_console_end();
>
> /* long line */
> buf[0x41] = 0x41;
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0x40, 1, buf, 0x42, true);
> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ..\"3DUfw........................................................",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 00 41 .A",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x40UL);
> + addr + 0x40UL);
> ut_assert_console_end();
>
> /* 16-bit */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 2, buf, 0x12, true);
> ut_assert_nextline("%0*lx: 1100 3322 5544 7766 9988 bbaa ddcc ffee ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 0010 ..",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x10UL);
> + addr + 0x10UL);
> ut_assert_console_end();
>
> /* 32-bit */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 4, buf, 0x14, true);
> ut_assert_nextline("%0*lx: 33221100 77665544 bbaa9988 ffeeddcc ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 00000010 ....",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x10UL);
> + addr + 0x10UL);
> ut_assert_console_end();
>
> /* 64-bit */
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 8, buf, 0x18, true);
> ut_assert_nextline("%0*lx: 7766554433221100 ffeeddccbbaa9988 ..\"3DUfw........",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_nextline("%0*lx: 0000000000000010 ........",
> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf + 0x10UL);
> + addr + 0x10UL);
> ut_assert_console_end();
>
> /* ASCII */
> @@ -345,8 +341,7 @@ static int print_do_hex_dump(struct unit_test_state *uts)
> buf[8] = 255;
> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 1, buf, 10, true);
> ut_assert_nextline("%0*lx: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~.....",
> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> - (uintptr_t)buf);
> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> ut_assert_console_end();
> free(buf);
>
> --
> 2.51.0
>
This is adding memory allocations to a test for hex dumping.
It would be better and simpler to use a fixed address and make this a
sandbox-only test. I struggle to see the value of running these sorts
of tests under QEMU?
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 7/9] common: default CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000
2025-11-11 8:31 ` Ilias Apalodimas
@ 2025-11-14 12:44 ` Simon Glass
0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2025-11-14 12:44 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: Heinrich Schuchardt, Tom Rini, Bin Meng, Raymond Mao,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
On Tue, 11 Nov 2025 at 01:31, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Sun, 9 Nov 2025 at 12:10, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > For some tests the current default of 0x400 for
> > CONFIG_CONSOLE_RECORD_OUT_SIZE is too small.
> >
> > Raise the value to 0x6000 which is already the most common value.
> >
> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > ---
> > common/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
This doesn't affect the pre-relocation value so seems fine to me.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 3/9] test: cmd/bdinfo: make no flash assumption
2025-11-09 10:10 ` [PATCH 3/9] test: cmd/bdinfo: make no flash assumption Heinrich Schuchardt
2025-11-11 8:30 ` Ilias Apalodimas
@ 2025-11-14 12:44 ` Simon Glass
1 sibling, 0 replies; 37+ messages in thread
From: Simon Glass @ 2025-11-14 12:44 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Bin Meng, Raymond Mao, Ilias Apalodimas, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
Hi Heinrich,
On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> The location and size of flash is device-dependent. Do not make any
> assumption about the location and size.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> test/cmd/bdinfo.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c
> index 09f44ee41ed..214b237152b 100644
> --- a/test/cmd/bdinfo.c
> +++ b/test/cmd/bdinfo.c
> @@ -161,9 +161,9 @@ static int bdinfo_test_all(struct unit_test_state *uts)
> ut_assertok(bdinfo_check_mem(uts));
>
> /* CONFIG_SYS_HAS_SRAM testing not supported */
> - ut_assertok(test_num_l(uts, "flashstart", 0));
> - ut_assertok(test_num_l(uts, "flashsize", 0));
> - ut_assertok(test_num_l(uts, "flashoffset", 0));
> + ut_check_console_linen(uts, "flashstart");
> + ut_check_console_linen(uts, "flashsize");
> + ut_check_console_linen(uts, "flashoffset");
> ut_assert_nextline("baudrate = %lu bps",
> env_get_ulong("baudrate", 10, 1234));
> ut_assertok(test_num_l(uts, "relocaddr", gd->relocaddr));
> --
> 2.51.0
>
Shouldn't we check the actual value, then?
Reviewed-by: Simon Glass <sjg@chromium.org>
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists
2025-11-09 10:10 ` [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists Heinrich Schuchardt
2025-11-11 8:33 ` Ilias Apalodimas
@ 2025-11-14 12:44 ` Simon Glass
2025-11-14 14:22 ` Tom Rini
1 sibling, 1 reply; 37+ messages in thread
From: Simon Glass @ 2025-11-14 12:44 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Bin Meng, Raymond Mao, Ilias Apalodimas, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
Hi Heinrich,
On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> In fdt_test_chosen() currently we test if DM_RNG is configured.
> CONFIG_DM_RNG=y does not imply that a RNG device actually exists.
> For instance QEMU may be called with -device virtio-rng-device or not.
> The current test framework evicts the virtio RNG device even if QEMU is
> called with -device virtio-rng-device.
>
> In the fdt_test_chosen() check if a RNG device exists.
> Ignore 'No RNG device' messages.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> test/cmd/fdt.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> index a36f2dcdda8..b950123b6da 100644
> --- a/test/cmd/fdt.c
> +++ b/test/cmd/fdt.c
> @@ -10,6 +10,7 @@
> #include <fdt_support.h>
> #include <mapmem.h>
> #include <asm/global_data.h>
> +#include <dm/uclass.h>
> #include <linux/libfdt.h>
> #include <test/ut.h>
>
> @@ -1267,6 +1268,7 @@ static int fdt_test_chosen(struct unit_test_state *uts)
> {
> const char *env_bootargs = env_get("bootargs");
> char fdt[8192];
> + struct udevice *dev;
> ulong addr;
>
> ut_assertok(make_test_fdt(uts, fdt, sizeof(fdt), &addr));
> @@ -1280,11 +1282,16 @@ static int fdt_test_chosen(struct unit_test_state *uts)
> /* Test add new chosen node without initrd */
> ut_assertok(run_commandf("fdt chosen"));
> ut_assertok(run_commandf("fdt print /chosen"));
> - ut_assert_nextline("chosen {");
> + ut_assert(0 < console_record_readline(uts->actual_str,
> + sizeof(uts->actual_str)));
> + if (!strcmp("No RNG device", uts->actual_str))
> + ut_assert(0 < console_record_readline(uts->actual_str,
> + sizeof(uts->actual_str)));
But then this test isn't really making sure that the RNG code works.
We need to know whether there is supposed to be a device (as you do
below), rather than making the test vague.
For sandbox, we can assert that the device exists, so at least we keep
some coverage.
> + ut_asserteq_str("chosen {", uts->actual_str);
> ut_assert_nextlinen("\tu-boot,version = "); /* Ignore the version string */
> if (env_bootargs)
> ut_assert_nextline("\tbootargs = \"%s\";", env_bootargs);
> - if (IS_ENABLED(CONFIG_DM_RNG) &&
> + if (!uclass_get_device(UCLASS_RNG, 0, &dev) &&
> !IS_ENABLED(CONFIG_MEASURED_BOOT) &&
> !IS_ENABLED(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT))
> ut_assert_nextlinen("\tkaslr-seed = ");
> @@ -1294,7 +1301,12 @@ static int fdt_test_chosen(struct unit_test_state *uts)
> /* Test add new chosen node with initrd */
> ut_assertok(run_commandf("fdt chosen 0x1234 0x5678"));
> ut_assertok(run_commandf("fdt print /chosen"));
> - ut_assert_nextline("chosen {");
> + ut_assert(0 < console_record_readline(uts->actual_str,
> + sizeof(uts->actual_str)));
> + if (!strcmp("No RNG device", uts->actual_str))
> + ut_assert(0 < console_record_readline(uts->actual_str,
> + sizeof(uts->actual_str)));
> + ut_asserteq_str("chosen {", uts->actual_str);
> ut_assert_nextline("\tlinux,initrd-end = <0x%08x 0x%08x>;",
> upper_32_bits(0x1234 + 0x5678 - 1),
> lower_32_bits(0x1234 + 0x5678 - 1));
> @@ -1303,7 +1315,7 @@ static int fdt_test_chosen(struct unit_test_state *uts)
> ut_assert_nextlinen("\tu-boot,version = "); /* Ignore the version string */
> if (env_bootargs)
> ut_assert_nextline("\tbootargs = \"%s\";", env_bootargs);
> - if (IS_ENABLED(CONFIG_DM_RNG) &&
> + if (!uclass_get_device(UCLASS_RNG, 0, &dev) &&
> !IS_ENABLED(CONFIG_MEASURED_BOOT) &&
> !IS_ENABLED(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT))
> ut_assert_nextlinen("\tkaslr-seed = ");
> --
> 2.51.0
>
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 4/9] test: cmd/bdinfo: consider arch_print_bdinfo() output
2025-11-09 10:10 ` [PATCH 4/9] test: cmd/bdinfo: consider arch_print_bdinfo() output Heinrich Schuchardt
@ 2025-11-14 12:44 ` Simon Glass
0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2025-11-14 12:44 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Bin Meng, Raymond Mao, Ilias Apalodimas, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
Hi Heinrich,
On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On x86 commit 9b35dbc93fd4 ("x86: Show the timestamp counter with bdinfo")
> has added another bdinfo output line.
>
> On RISC-V commit 66b5ee9c558e ("riscv: add RISC-V fields to bdinfo
> command") implemented arch_print_bdinfo().
>
> Update the bdinfo test accordingly.
>
> Fixes: 9b35dbc93fd4 ("x86: Show the timestamp counter with bdinfo")
> Fixes: 66b5ee9c558e ("riscv: add RISC-V fields to bdinfo command")
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> test/cmd/bdinfo.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c
> index 214b237152b..28d448a0866 100644
> --- a/test/cmd/bdinfo.c
> +++ b/test/cmd/bdinfo.c
> @@ -215,8 +215,15 @@ static int bdinfo_test_all(struct unit_test_state *uts)
> ut_assertok(test_num_l(uts, "malloc base", gd_malloc_start()));
> }
>
> + /* Check arch_print_bdinfo() output */
> if (IS_ENABLED(CONFIG_X86))
> - ut_check_skip_to_linen(uts, " high end =");
> + ut_check_skip_to_linen(uts, "tsc");
> +
> +#ifdef CONFIG_RISCV
> + ut_check_console_linen(uts, "boot hart");
> + if (gd->arch.firmware_fdt_addr)
> + ut_check_console_linen(uts, "firmware fdt");
> +#endif
How about creating an accessor so the #ifdef is in global_data.h and
you can use if (IS_ENABLED()) here?
>
> return 0;
> }
> --
> 2.51.0
>
Reviewed-by: Simon Glass <sjg@chromium.org>
Regards,
SImon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 5/9] test: common/print: do not use fixed buffer addresses
2025-11-09 10:10 ` [PATCH 5/9] test: common/print: do not use fixed buffer addresses Heinrich Schuchardt
2025-11-11 8:48 ` Ilias Apalodimas
@ 2025-11-14 12:44 ` Simon Glass
1 sibling, 0 replies; 37+ messages in thread
From: Simon Glass @ 2025-11-14 12:44 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Bin Meng, Raymond Mao, Ilias Apalodimas, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
Hi Heinrich,
On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> The location of memory depends on the board. Do not assume memory at fixed
> memory locations. Use calloc() instead to allocate buffers.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> test/common/print.c | 76 ++++++++++++++++++++++++---------------------
> 1 file changed, 40 insertions(+), 36 deletions(-)
>
I have the same question, as to the value of this test when it is
already covered by sandbox.
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses
2025-11-14 12:31 ` Simon Glass
@ 2025-11-14 14:19 ` Tom Rini
2025-11-14 14:24 ` Simon Glass
2025-11-14 14:29 ` Heinrich Schuchardt
1 sibling, 1 reply; 37+ messages in thread
From: Tom Rini @ 2025-11-14 14:19 UTC (permalink / raw)
To: Simon Glass
Cc: Heinrich Schuchardt, Bin Meng, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]
On Fri, Nov 14, 2025 at 05:31:40AM -0700, Simon Glass wrote:
> Hi Heinrich,
>
> On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > The location of memory depends on the board. Do not assume memory at fixed
> > memory locations. Use memalign() instead to allocate a buffer.
> >
> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
[snip]
> > /* bytes */
> > print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
> > ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
> > - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> > - (uintptr_t)buf);
> > + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
[snip]
> This is adding memory allocations to a test for hex dumping.
>
> It would be better and simpler to use a fixed address and make this a
> sandbox-only test. I struggle to see the value of running these sorts
> of tests under QEMU?
Removing context to highlight value of running tests on multiple
platforms.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists
2025-11-14 12:44 ` Simon Glass
@ 2025-11-14 14:22 ` Tom Rini
0 siblings, 0 replies; 37+ messages in thread
From: Tom Rini @ 2025-11-14 14:22 UTC (permalink / raw)
To: Simon Glass
Cc: Heinrich Schuchardt, Bin Meng, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
[-- Attachment #1: Type: text/plain, Size: 2331 bytes --]
On Fri, Nov 14, 2025 at 05:44:08AM -0700, Simon Glass wrote:
> Hi Heinrich,
>
> On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > In fdt_test_chosen() currently we test if DM_RNG is configured.
> > CONFIG_DM_RNG=y does not imply that a RNG device actually exists.
> > For instance QEMU may be called with -device virtio-rng-device or not.
> > The current test framework evicts the virtio RNG device even if QEMU is
> > called with -device virtio-rng-device.
> >
> > In the fdt_test_chosen() check if a RNG device exists.
> > Ignore 'No RNG device' messages.
> >
> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > ---
> > test/cmd/fdt.c | 20 ++++++++++++++++----
> > 1 file changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> > index a36f2dcdda8..b950123b6da 100644
> > --- a/test/cmd/fdt.c
> > +++ b/test/cmd/fdt.c
> > @@ -10,6 +10,7 @@
> > #include <fdt_support.h>
> > #include <mapmem.h>
> > #include <asm/global_data.h>
> > +#include <dm/uclass.h>
> > #include <linux/libfdt.h>
> > #include <test/ut.h>
> >
> > @@ -1267,6 +1268,7 @@ static int fdt_test_chosen(struct unit_test_state *uts)
> > {
> > const char *env_bootargs = env_get("bootargs");
> > char fdt[8192];
> > + struct udevice *dev;
> > ulong addr;
> >
> > ut_assertok(make_test_fdt(uts, fdt, sizeof(fdt), &addr));
> > @@ -1280,11 +1282,16 @@ static int fdt_test_chosen(struct unit_test_state *uts)
> > /* Test add new chosen node without initrd */
> > ut_assertok(run_commandf("fdt chosen"));
> > ut_assertok(run_commandf("fdt print /chosen"));
> > - ut_assert_nextline("chosen {");
> > + ut_assert(0 < console_record_readline(uts->actual_str,
> > + sizeof(uts->actual_str)));
> > + if (!strcmp("No RNG device", uts->actual_str))
> > + ut_assert(0 < console_record_readline(uts->actual_str,
> > + sizeof(uts->actual_str)));
>
> But then this test isn't really making sure that the RNG code works.
This isn't an RNG test. That would be test/dm/rng.c or
test/dm/virtio_rng.c
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses
2025-11-14 14:19 ` Tom Rini
@ 2025-11-14 14:24 ` Simon Glass
2025-11-14 14:43 ` Tom Rini
0 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2025-11-14 14:24 UTC (permalink / raw)
To: Tom Rini
Cc: Heinrich Schuchardt, Bin Meng, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
Hi Tom,
On Fri, 14 Nov 2025 at 07:19, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Nov 14, 2025 at 05:31:40AM -0700, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> > >
> > > The location of memory depends on the board. Do not assume memory at fixed
> > > memory locations. Use memalign() instead to allocate a buffer.
> > >
> > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> [snip]
> > > /* bytes */
> > > print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
> > > ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
> > > - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> > > - (uintptr_t)buf);
> > > + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> [snip]
> > This is adding memory allocations to a test for hex dumping.
> >
> > It would be better and simpler to use a fixed address and make this a
> > sandbox-only test. I struggle to see the value of running these sorts
> > of tests under QEMU?
>
> Removing context to highlight value of running tests on multiple
> platforms.
$ ./tools/qconfig.py -f ~PHYS_64BIT -l |grep sandbox
sandbox
sandbox_flattree
sandbox_nocmdline
sandbox_noinst
sandbox_spl
sandbox_vpl
$ ./tools/qconfig.py -f PHYS_64BIT -l |grep sandbox
sandbox64
sandbox64_lwip
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses
2025-11-14 12:31 ` Simon Glass
2025-11-14 14:19 ` Tom Rini
@ 2025-11-14 14:29 ` Heinrich Schuchardt
1 sibling, 0 replies; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-14 14:29 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Bin Meng, Raymond Mao, Ilias Apalodimas, Michal Simek,
Adriano Cordova, Harrison Mutai, Peng Fan, Ben Dooks,
Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
On 11/14/25 13:31, Simon Glass wrote:
> Hi Heinrich,
>
> On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> The location of memory depends on the board. Do not assume memory at fixed
>> memory locations. Use memalign() instead to allocate a buffer.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>> test/cmd/fdt.c | 8 ++++++--
>> test/common/print.c | 37 ++++++++++++++++---------------------
>> 2 files changed, 22 insertions(+), 23 deletions(-)
>>
>> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
>> index b950123b6da..4c3c6308ab4 100644
>> --- a/test/cmd/fdt.c
>> +++ b/test/cmd/fdt.c
>> @@ -265,7 +265,7 @@ FDT_TEST(fdt_test_addr_resize, UTF_CONSOLE);
>> static int fdt_test_move(struct unit_test_state *uts)
>> {
>> char fdt[256];
>> - ulong addr, newaddr = 0x10000;
>> + ulong addr, newaddr;
>> const int size = sizeof(fdt);
>> uint32_t ts;
>> void *buf;
>> @@ -275,8 +275,10 @@ static int fdt_test_move(struct unit_test_state *uts)
>> ts = fdt_totalsize(fdt);
>>
>> /* Moved target DT location */
>> - buf = map_sysmem(newaddr, size);
>> + buf = memalign(8, size);
>> + ut_assertnonnull(buf);
>> memset(buf, 0, size);
>> + newaddr = map_to_sysmem(buf);
>>
>> /* Test moving the working FDT to a new location */
>> ut_assertok(run_commandf("fdt move %08lx %08lx %x", addr, newaddr, ts));
>> @@ -288,6 +290,8 @@ static int fdt_test_move(struct unit_test_state *uts)
>> ut_assert_nextline("Total of %d byte(s) were the same", ts);
>> ut_assert_console_end();
>>
>> + free(buf);
>> +
>> return 0;
>> }
>> FDT_TEST(fdt_test_move, UTF_CONSOLE);
>> diff --git a/test/common/print.c b/test/common/print.c
>> index 2ac7eeed2c1..f33476a36b4 100644
>> --- a/test/common/print.c
>> +++ b/test/common/print.c
>> @@ -266,74 +266,70 @@ static int print_do_hex_dump(struct unit_test_state *uts)
>> {
>> u8 *buf;
>> int i;
>> + ulong addr;
>>
>> buf = calloc(1, BUF_SIZE);
>> ut_assertnonnull(buf);
>> + addr = map_to_sysmem(buf);
>> for (i = 0; i < 0x11; i++)
>> buf[i] = i * 0x11;
>>
>> /* bytes */
>> print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
>> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
>> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf);
>> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
>> ut_assert_nextline("%0*lx: 10 00 ..",
>> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf + 0x10UL);
>> + addr + 0x10UL);
>> ut_assert_console_end();
>>
>> /* line length */
>> print_hex_dump("", DUMP_PREFIX_ADDRESS, 8, 1, buf, 0x12, true);
>> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 ..\"3DUfw",
>> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf);
>> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
>> ut_assert_nextline("%0*lx: 88 99 aa bb cc dd ee ff ........",
>> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf + 0x8UL);
>> + addr + 0x8UL);
>> ut_assert_nextline("%0*lx: 10 00 ..",
>> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf + 0x10UL);
>> + addr + 0x10UL);
>> ut_assert_console_end();
>>
>> /* long line */
>> buf[0x41] = 0x41;
>> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0x40, 1, buf, 0x42, true);
>> ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ..\"3DUfw........................................................",
>> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf);
>> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
>> ut_assert_nextline("%0*lx: 00 41 .A",
>> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf + 0x40UL);
>> + addr + 0x40UL);
>> ut_assert_console_end();
>>
>> /* 16-bit */
>> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 2, buf, 0x12, true);
>> ut_assert_nextline("%0*lx: 1100 3322 5544 7766 9988 bbaa ddcc ffee ..\"3DUfw........",
>> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf);
>> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
>> ut_assert_nextline("%0*lx: 0010 ..",
>> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf + 0x10UL);
>> + addr + 0x10UL);
>> ut_assert_console_end();
>>
>> /* 32-bit */
>> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 4, buf, 0x14, true);
>> ut_assert_nextline("%0*lx: 33221100 77665544 bbaa9988 ffeeddcc ..\"3DUfw........",
>> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf);
>> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
>> ut_assert_nextline("%0*lx: 00000010 ....",
>> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf + 0x10UL);
>> + addr + 0x10UL);
>> ut_assert_console_end();
>>
>> /* 64-bit */
>> print_hex_dump("", DUMP_PREFIX_ADDRESS, 16, 8, buf, 0x18, true);
>> ut_assert_nextline("%0*lx: 7766554433221100 ffeeddccbbaa9988 ..\"3DUfw........",
>> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf);
>> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
>> ut_assert_nextline("%0*lx: 0000000000000010 ........",
>> IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf + 0x10UL);
>> + addr + 0x10UL);
>> ut_assert_console_end();
>>
>> /* ASCII */
>> @@ -345,8 +341,7 @@ static int print_do_hex_dump(struct unit_test_state *uts)
>> buf[8] = 255;
>> print_hex_dump("", DUMP_PREFIX_ADDRESS, 0, 1, buf, 10, true);
>> ut_assert_nextline("%0*lx: 00 1f 20 21 7e 7f 80 81 ff 99 .. !~.....",
>> - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
>> - (uintptr_t)buf);
>> + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
>> ut_assert_console_end();
>> free(buf);
>>
>> --
>> 2.51.0
>>
>
> This is adding memory allocations to a test for hex dumping.
>
> It would be better and simpler to use a fixed address and make this a
> sandbox-only test. I struggle to see the value of running these sorts
> of tests under QEMU?
I cannot see any added value in restricting a test such that it can only
be run on the sandbox.
My expectation is that tests should be executable on any platform be it
a physical board or a virtual machine. There are few use cases for the
sandbox in real life.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses
2025-11-14 14:24 ` Simon Glass
@ 2025-11-14 14:43 ` Tom Rini
2025-11-14 18:10 ` Simon Glass
0 siblings, 1 reply; 37+ messages in thread
From: Tom Rini @ 2025-11-14 14:43 UTC (permalink / raw)
To: Simon Glass
Cc: Heinrich Schuchardt, Bin Meng, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
[-- Attachment #1: Type: text/plain, Size: 1791 bytes --]
On Fri, Nov 14, 2025 at 07:24:53AM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Fri, 14 Nov 2025 at 07:19, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Nov 14, 2025 at 05:31:40AM -0700, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
> > > <heinrich.schuchardt@canonical.com> wrote:
> > > >
> > > > The location of memory depends on the board. Do not assume memory at fixed
> > > > memory locations. Use memalign() instead to allocate a buffer.
> > > >
> > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > [snip]
> > > > /* bytes */
> > > > print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
> > > > ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
> > > > - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> > > > - (uintptr_t)buf);
> > > > + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> > [snip]
> > > This is adding memory allocations to a test for hex dumping.
> > >
> > > It would be better and simpler to use a fixed address and make this a
> > > sandbox-only test. I struggle to see the value of running these sorts
> > > of tests under QEMU?
> >
> > Removing context to highlight value of running tests on multiple
> > platforms.
>
> $ ./tools/qconfig.py -f ~PHYS_64BIT -l |grep sandbox
> sandbox
> sandbox_flattree
> sandbox_nocmdline
> sandbox_noinst
> sandbox_spl
> sandbox_vpl
> $ ./tools/qconfig.py -f PHYS_64BIT -l |grep sandbox
> sandbox64
> sandbox64_lwip
Good, and as soon as you restrict to "sandbox" we stop running
"sandbox64", it's why sandbox64 runs so much quicker in CI.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses
2025-11-14 14:43 ` Tom Rini
@ 2025-11-14 18:10 ` Simon Glass
2025-11-14 18:54 ` Tom Rini
0 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2025-11-14 18:10 UTC (permalink / raw)
To: Tom Rini
Cc: Heinrich Schuchardt, Bin Meng, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
On Fri, 14 Nov 2025 at 07:43, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Nov 14, 2025 at 07:24:53AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 14 Nov 2025 at 07:19, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Nov 14, 2025 at 05:31:40AM -0700, Simon Glass wrote:
> > > > Hi Heinrich,
> > > >
> > > > On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
> > > > <heinrich.schuchardt@canonical.com> wrote:
> > > > >
> > > > > The location of memory depends on the board. Do not assume memory at fixed
> > > > > memory locations. Use memalign() instead to allocate a buffer.
> > > > >
> > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > [snip]
> > > > > /* bytes */
> > > > > print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
> > > > > ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
> > > > > - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> > > > > - (uintptr_t)buf);
> > > > > + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> > > [snip]
> > > > This is adding memory allocations to a test for hex dumping.
> > > >
> > > > It would be better and simpler to use a fixed address and make this a
> > > > sandbox-only test. I struggle to see the value of running these sorts
> > > > of tests under QEMU?
> > >
> > > Removing context to highlight value of running tests on multiple
> > > platforms.
> >
> > $ ./tools/qconfig.py -f ~PHYS_64BIT -l |grep sandbox
> > sandbox
> > sandbox_flattree
> > sandbox_nocmdline
> > sandbox_noinst
> > sandbox_spl
> > sandbox_vpl
> > $ ./tools/qconfig.py -f PHYS_64BIT -l |grep sandbox
> > sandbox64
> > sandbox64_lwip
If at any point you have made up your mind, please say so, rather than
continuing what I intended to be a discussion of the pros and cons of
this patch.
> Good, and as soon as you restrict to "sandbox" we stop running
> "sandbox64", it's why sandbox64 runs so much quicker in CI.
This is a C test - test/cmd/fdt.c so it is built for all sandbox
boards. The restriction you are referring to here is for pytests, I
believe. The core of my argument is that running sandbox tests on
other architectures is mostly a waste of time, assuming the compiler
is functioning correctly. This patch is also devaluing sandbox, the
major advantages of which is its fast, native code execution and fixed
execution environment (memory map, etc.).
At the very least, it would help to be clear what bugs we are hoping
to find with this change.
- Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses
2025-11-14 18:10 ` Simon Glass
@ 2025-11-14 18:54 ` Tom Rini
2025-11-17 18:16 ` Simon Glass
0 siblings, 1 reply; 37+ messages in thread
From: Tom Rini @ 2025-11-14 18:54 UTC (permalink / raw)
To: Simon Glass
Cc: Heinrich Schuchardt, Bin Meng, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
[-- Attachment #1: Type: text/plain, Size: 3942 bytes --]
On Fri, Nov 14, 2025 at 11:10:59AM -0700, Simon Glass wrote:
> On Fri, 14 Nov 2025 at 07:43, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Nov 14, 2025 at 07:24:53AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Fri, 14 Nov 2025 at 07:19, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Fri, Nov 14, 2025 at 05:31:40AM -0700, Simon Glass wrote:
> > > > > Hi Heinrich,
> > > > >
> > > > > On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
> > > > > <heinrich.schuchardt@canonical.com> wrote:
> > > > > >
> > > > > > The location of memory depends on the board. Do not assume memory at fixed
> > > > > > memory locations. Use memalign() instead to allocate a buffer.
> > > > > >
> > > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > > [snip]
> > > > > > /* bytes */
> > > > > > print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
> > > > > > ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
> > > > > > - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> > > > > > - (uintptr_t)buf);
> > > > > > + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> > > > [snip]
> > > > > This is adding memory allocations to a test for hex dumping.
> > > > >
> > > > > It would be better and simpler to use a fixed address and make this a
> > > > > sandbox-only test. I struggle to see the value of running these sorts
> > > > > of tests under QEMU?
> > > >
> > > > Removing context to highlight value of running tests on multiple
> > > > platforms.
> > >
> > > $ ./tools/qconfig.py -f ~PHYS_64BIT -l |grep sandbox
> > > sandbox
> > > sandbox_flattree
> > > sandbox_nocmdline
> > > sandbox_noinst
> > > sandbox_spl
> > > sandbox_vpl
> > > $ ./tools/qconfig.py -f PHYS_64BIT -l |grep sandbox
> > > sandbox64
> > > sandbox64_lwip
>
> If at any point you have made up your mind, please say so, rather than
> continuing what I intended to be a discussion of the pros and cons of
> this patch.
Yes, both Heinrich and I agree we should be running these tests on
hardware. Don't run them on hardware as a position has been rejected.
You can stop reading here if you don't want an explanation.
> > Good, and as soon as you restrict to "sandbox" we stop running
> > "sandbox64", it's why sandbox64 runs so much quicker in CI.
>
> This is a C test - test/cmd/fdt.c so it is built for all sandbox
> boards. The restriction you are referring to here is for pytests, I
> believe. The core of my argument is that running sandbox tests on
> other architectures is mostly a waste of time, assuming the compiler
> is functioning correctly. This patch is also devaluing sandbox, the
> major advantages of which is its fast, native code execution and fixed
> execution environment (memory map, etc.).
>
> At the very least, it would help to be clear what bugs we are hoping
> to find with this change.
The position of "don't run tests on hardware, only on sandbox" does not
sound sensible in general. In practice we need to do more, not less, on
device testing and saving milliseconds by skipping tests is noise lost
in the time it takes to acquire a runner and clone the source code or
even which runner we test things on. Spending time on "should this run
on hardware or only sandbox" is time not well spent. Finally, "do a
bunch of stuff on hardware" is still the best overall method for finding
unexpected platform bugs. It's how we catch "some clocks are wrong" or
"we configured thermals wrong" or "we configured memory wrong" and so
forth.
You mention QEMU, and yes, we will be executing these on QEMU in CI, on
some platforms, which are our fastest pytest pipelines. But we will also
being running these on hardware, which is my point and I believe
Heinrich's as well.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses
2025-11-14 18:54 ` Tom Rini
@ 2025-11-17 18:16 ` Simon Glass
0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2025-11-17 18:16 UTC (permalink / raw)
To: Tom Rini
Cc: Heinrich Schuchardt, Bin Meng, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
Hi Tom,
On Fri, 14 Nov 2025 at 11:54, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Nov 14, 2025 at 11:10:59AM -0700, Simon Glass wrote:
> > On Fri, 14 Nov 2025 at 07:43, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Fri, Nov 14, 2025 at 07:24:53AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Fri, 14 Nov 2025 at 07:19, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Fri, Nov 14, 2025 at 05:31:40AM -0700, Simon Glass wrote:
> > > > > > Hi Heinrich,
> > > > > >
> > > > > > On Sun, 9 Nov 2025 at 03:10, Heinrich Schuchardt
> > > > > > <heinrich.schuchardt@canonical.com> wrote:
> > > > > > >
> > > > > > > The location of memory depends on the board. Do not assume memory at fixed
> > > > > > > memory locations. Use memalign() instead to allocate a buffer.
> > > > > > >
> > > > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > > > [snip]
> > > > > > > /* bytes */
> > > > > > > print_hex_dump_bytes("", DUMP_PREFIX_ADDRESS, buf, 0x12);
> > > > > > > ut_assert_nextline("%0*lx: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff ..\"3DUfw........",
> > > > > > > - IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8,
> > > > > > > - (uintptr_t)buf);
> > > > > > > + IS_ENABLED(CONFIG_PHYS_64BIT) ? 16 : 8, addr);
> > > > > [snip]
> > > > > > This is adding memory allocations to a test for hex dumping.
> > > > > >
> > > > > > It would be better and simpler to use a fixed address and make this a
> > > > > > sandbox-only test. I struggle to see the value of running these sorts
> > > > > > of tests under QEMU?
> > > > >
> > > > > Removing context to highlight value of running tests on multiple
> > > > > platforms.
> > > >
> > > > $ ./tools/qconfig.py -f ~PHYS_64BIT -l |grep sandbox
> > > > sandbox
> > > > sandbox_flattree
> > > > sandbox_nocmdline
> > > > sandbox_noinst
> > > > sandbox_spl
> > > > sandbox_vpl
> > > > $ ./tools/qconfig.py -f PHYS_64BIT -l |grep sandbox
> > > > sandbox64
> > > > sandbox64_lwip
> >
> > If at any point you have made up your mind, please say so, rather than
> > continuing what I intended to be a discussion of the pros and cons of
> > this patch.
>
> Yes, both Heinrich and I agree we should be running these tests on
> hardware. Don't run them on hardware as a position has been rejected.
> You can stop reading here if you don't want an explanation.
OK. I'll discuss this with Heinrich at some point to understand his POV.
>
> > > Good, and as soon as you restrict to "sandbox" we stop running
> > > "sandbox64", it's why sandbox64 runs so much quicker in CI.
> >
> > This is a C test - test/cmd/fdt.c so it is built for all sandbox
> > boards. The restriction you are referring to here is for pytests, I
> > believe. The core of my argument is that running sandbox tests on
> > other architectures is mostly a waste of time, assuming the compiler
> > is functioning correctly. This patch is also devaluing sandbox, the
> > major advantages of which is its fast, native code execution and fixed
> > execution environment (memory map, etc.).
> >
> > At the very least, it would help to be clear what bugs we are hoping
> > to find with this change.
>
> The position of "don't run tests on hardware, only on sandbox" does not
> sound sensible in general. In practice we need to do more, not less, on
> device testing and saving milliseconds by skipping tests is noise lost
> in the time it takes to acquire a runner and clone the source code or
> even which runner we test things on. Spending time on "should this run
> on hardware or only sandbox" is time not well spent. Finally, "do a
> bunch of stuff on hardware" is still the best overall method for finding
> unexpected platform bugs. It's how we catch "some clocks are wrong" or
> "we configured thermals wrong" or "we configured memory wrong" and so
> forth.
Those bugs will not be found by this test. If the platform is unable
to reliably execute code, we should have a test which boots to a
prompt, runs dhrystone, or something like that.
>
> You mention QEMU, and yes, we will be executing these on QEMU in CI, on
> some platforms, which are our fastest pytest pipelines. But we will also
> being running these on hardware, which is my point and I believe
> Heinrich's as well.
OK I read this far.
1. If a test has *never* failed on a board and never can, what benefit
are we getting? This is not just about test time. It is also about
making the tests more complicated to deal with non-sandbox
environments. It's about the poor people who have to maintain all this
stuff.
2. Sandbox is basically the unit-test platform. Some of the tests do
quite a bit more than simple unit testing. But they are unit tests.
3. We should be clear about the goals of testing, not just blindly
enable all our unit tests on all boards.
Regards,
Simon
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY
2025-11-09 10:10 ` [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY Heinrich Schuchardt
2025-11-11 8:29 ` Ilias Apalodimas
@ 2025-11-22 16:23 ` Tom Rini
2025-11-22 23:52 ` Heinrich Schuchardt
1 sibling, 1 reply; 37+ messages in thread
From: Tom Rini @ 2025-11-22 16:23 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]
On Sun, Nov 09, 2025 at 11:10:02AM +0100, Heinrich Schuchardt wrote:
> The `fdt apply` sub-command is only available if CONFIG_OF_LIBFDT_OVERLAY
> is enabled.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> test/cmd/fdt.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> index 96a8488e172..a36f2dcdda8 100644
> --- a/test/cmd/fdt.c
> +++ b/test/cmd/fdt.c
> @@ -1319,6 +1319,9 @@ static int fdt_test_apply(struct unit_test_state *uts)
> char fdt[8192], fdto[8192];
> ulong addr, addro;
>
> + if (!IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY))
> + return -EAGAIN;
> +
> /* Create base DT with __symbols__ node */
> ut_assertok(fdt_create(fdt, sizeof(fdt)));
> ut_assertok(fdt_finish_reservemap(fdt));
So, this is interesting. As part of merging this, I see that on
qemu_arm64 for example:
u-boot: add: 0/0, grow: 4/-4 bytes: 284/-9691 (-9407)
function old new delta
print_hexdump_line 472 556 +84
fdt_test_move 568 652 +84
print_do_hex_dump 1704 1764 +60
print_display_buffer 1736 1792 +56
static.__func__ 5670 5655 -15
bdinfo_test_all 3064 2884 -180
fdt_test_chosen 1496 516 -980
fdt_test_apply 8524 8 -8516
So we're as expected shrinking fdt_test_apply with a build time
optimization. But we've been building and running this test as can be
seen in older pipelines and their published artifacts. Should it not
have been a failing test? Did it fail for you somewhere or was this
found by inspection?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY
2025-11-22 16:23 ` Tom Rini
@ 2025-11-22 23:52 ` Heinrich Schuchardt
2025-11-23 14:02 ` Tom Rini
0 siblings, 1 reply; 37+ messages in thread
From: Heinrich Schuchardt @ 2025-11-22 23:52 UTC (permalink / raw)
To: Tom Rini
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
On 11/22/25 17:23, Tom Rini wrote:
> On Sun, Nov 09, 2025 at 11:10:02AM +0100, Heinrich Schuchardt wrote:
>
>> The `fdt apply` sub-command is only available if CONFIG_OF_LIBFDT_OVERLAY
>> is enabled.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>> test/cmd/fdt.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
>> index 96a8488e172..a36f2dcdda8 100644
>> --- a/test/cmd/fdt.c
>> +++ b/test/cmd/fdt.c
>> @@ -1319,6 +1319,9 @@ static int fdt_test_apply(struct unit_test_state *uts)
>> char fdt[8192], fdto[8192];
>> ulong addr, addro;
>>
>> + if (!IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY))
>> + return -EAGAIN;
>> +
>> /* Create base DT with __symbols__ node */
>> ut_assertok(fdt_create(fdt, sizeof(fdt)));
>> ut_assertok(fdt_finish_reservemap(fdt));
>
> So, this is interesting. As part of merging this, I see that on
> qemu_arm64 for example:
> u-boot: add: 0/0, grow: 4/-4 bytes: 284/-9691 (-9407)
> function old new delta
> print_hexdump_line 472 556 +84
> fdt_test_move 568 652 +84
> print_do_hex_dump 1704 1764 +60
> print_display_buffer 1736 1792 +56
> static.__func__ 5670 5655 -15
> bdinfo_test_all 3064 2884 -180
> fdt_test_chosen 1496 516 -980
> fdt_test_apply 8524 8 -8516
>
> So we're as expected shrinking fdt_test_apply with a build time
> optimization. But we've been building and running this test as can be
> seen in older pipelines and their published artifacts. Should it not
> have been a failing test? Did it fail for you somewhere or was this
> found by inspection?
>
Hello Tom,
The target of the series was to test QFW and ACPI passthrough in the CI
using qemu-riscv64_smode_acpi_defconfig.
fdt_test_apply and other tests failed on
qemu-riscv64_smode_acpi_defconfig. This is why I touched the tests.
CONFIG_OF_LIBFDT_OVERLAY is not enabled on qemu_arm64_defconfig in
origin/master. Without this setting the `fdt apply` sub-command is not
available. So the test would not pass on this board.
But running `fdt ut` for qemu_arm64_defconfig shows:
=> ut fdt
Running 19 fdt tests
Test: addr: fdt.c
Skipping: Console recording disabled
Test: addr_resize: fdt.c
Skipping: Console recording disabled
Test: apply: fdt.c
Skipping: Console recording disabled
Test: bootcpu: fdt.c
The fdt_test_apply_test was compiled but is always skipped.
We should enable console recording on boards where we want proper testing.
Furthermore we should change test/py/tests/test_ut.py to show skipped C
tests by calling pytest.skip(), see
[PATCH 1/1] test: Let pytest indicate skipped C unit tests
https://lore.kernel.org/u-boot/20251122234920.80114-1-xypron.glpk@gmx.de/T/#u
Best regards
Heinrich
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY
2025-11-22 23:52 ` Heinrich Schuchardt
@ 2025-11-23 14:02 ` Tom Rini
0 siblings, 0 replies; 37+ messages in thread
From: Tom Rini @ 2025-11-23 14:02 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Bin Meng, Simon Glass, Raymond Mao, Ilias Apalodimas,
Michal Simek, Adriano Cordova, Harrison Mutai, Peng Fan,
Ben Dooks, Svyatoslav Ryhel, Sam Protsenko, Mattijs Korpershoek,
Jerome Forissier, Patrick Delaunay, Leo Yu-Chi Liang, Rick Chen,
u-boot
[-- Attachment #1: Type: text/plain, Size: 3592 bytes --]
On Sun, Nov 23, 2025 at 12:52:55AM +0100, Heinrich Schuchardt wrote:
> On 11/22/25 17:23, Tom Rini wrote:
> > On Sun, Nov 09, 2025 at 11:10:02AM +0100, Heinrich Schuchardt wrote:
> >
> > > The `fdt apply` sub-command is only available if CONFIG_OF_LIBFDT_OVERLAY
> > > is enabled.
> > >
> > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > ---
> > > test/cmd/fdt.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> > > index 96a8488e172..a36f2dcdda8 100644
> > > --- a/test/cmd/fdt.c
> > > +++ b/test/cmd/fdt.c
> > > @@ -1319,6 +1319,9 @@ static int fdt_test_apply(struct unit_test_state *uts)
> > > char fdt[8192], fdto[8192];
> > > ulong addr, addro;
> > > + if (!IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY))
> > > + return -EAGAIN;
> > > +
> > > /* Create base DT with __symbols__ node */
> > > ut_assertok(fdt_create(fdt, sizeof(fdt)));
> > > ut_assertok(fdt_finish_reservemap(fdt));
> >
> > So, this is interesting. As part of merging this, I see that on
> > qemu_arm64 for example:
> > u-boot: add: 0/0, grow: 4/-4 bytes: 284/-9691 (-9407)
> > function old new delta
> > print_hexdump_line 472 556 +84
> > fdt_test_move 568 652 +84
> > print_do_hex_dump 1704 1764 +60
> > print_display_buffer 1736 1792 +56
> > static.__func__ 5670 5655 -15
> > bdinfo_test_all 3064 2884 -180
> > fdt_test_chosen 1496 516 -980
> > fdt_test_apply 8524 8 -8516
> >
> > So we're as expected shrinking fdt_test_apply with a build time
> > optimization. But we've been building and running this test as can be
> > seen in older pipelines and their published artifacts. Should it not
> > have been a failing test? Did it fail for you somewhere or was this
> > found by inspection?
> >
>
> Hello Tom,
>
> The target of the series was to test QFW and ACPI passthrough in the CI
> using qemu-riscv64_smode_acpi_defconfig.
>
> fdt_test_apply and other tests failed on qemu-riscv64_smode_acpi_defconfig.
> This is why I touched the tests.
>
> CONFIG_OF_LIBFDT_OVERLAY is not enabled on qemu_arm64_defconfig in
> origin/master. Without this setting the `fdt apply` sub-command is not
> available. So the test would not pass on this board.
>
> But running `fdt ut` for qemu_arm64_defconfig shows:
>
> => ut fdt
> Running 19 fdt tests
> Test: addr: fdt.c
> Skipping: Console recording disabled
> Test: addr_resize: fdt.c
> Skipping: Console recording disabled
> Test: apply: fdt.c
> Skipping: Console recording disabled
> Test: bootcpu: fdt.c
>
> The fdt_test_apply_test was compiled but is always skipped.
>
> We should enable console recording on boards where we want proper testing.
>
> Furthermore we should change test/py/tests/test_ut.py to show skipped C
> tests by calling pytest.skip(), see
>
> [PATCH 1/1] test: Let pytest indicate skipped C unit tests
> https://lore.kernel.org/u-boot/20251122234920.80114-1-xypron.glpk@gmx.de/T/#u
Thanks for looking in to this more, your patches look like the right
direction to me!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2025-11-23 14:02 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-09 10:10 [PATCH 0/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
2025-11-09 10:10 ` [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY Heinrich Schuchardt
2025-11-11 8:29 ` Ilias Apalodimas
2025-11-11 13:32 ` Tom Rini
2025-11-11 21:18 ` Heinrich Schuchardt
2025-11-11 21:12 ` Heinrich Schuchardt
2025-11-11 21:26 ` Tom Rini
2025-11-22 16:23 ` Tom Rini
2025-11-22 23:52 ` Heinrich Schuchardt
2025-11-23 14:02 ` Tom Rini
2025-11-09 10:10 ` [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists Heinrich Schuchardt
2025-11-11 8:33 ` Ilias Apalodimas
2025-11-14 12:44 ` Simon Glass
2025-11-14 14:22 ` Tom Rini
2025-11-09 10:10 ` [PATCH 3/9] test: cmd/bdinfo: make no flash assumption Heinrich Schuchardt
2025-11-11 8:30 ` Ilias Apalodimas
2025-11-14 12:44 ` Simon Glass
2025-11-09 10:10 ` [PATCH 4/9] test: cmd/bdinfo: consider arch_print_bdinfo() output Heinrich Schuchardt
2025-11-14 12:44 ` Simon Glass
2025-11-09 10:10 ` [PATCH 5/9] test: common/print: do not use fixed buffer addresses Heinrich Schuchardt
2025-11-11 8:48 ` Ilias Apalodimas
2025-11-14 12:44 ` Simon Glass
2025-11-09 10:10 ` [PATCH 6/9] test: cmd/fdt: " Heinrich Schuchardt
2025-11-11 8:43 ` Ilias Apalodimas
2025-11-14 12:31 ` Simon Glass
2025-11-14 14:19 ` Tom Rini
2025-11-14 14:24 ` Simon Glass
2025-11-14 14:43 ` Tom Rini
2025-11-14 18:10 ` Simon Glass
2025-11-14 18:54 ` Tom Rini
2025-11-17 18:16 ` Simon Glass
2025-11-14 14:29 ` Heinrich Schuchardt
2025-11-09 10:10 ` [PATCH 7/9] common: default CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000 Heinrich Schuchardt
2025-11-11 8:31 ` Ilias Apalodimas
2025-11-14 12:44 ` Simon Glass
2025-11-09 10:10 ` [PATCH 8/9] configs: CONFIG_CONSOLE_RECORD=y on qemu-riscv64_smode_acpi Heinrich Schuchardt
2025-11-09 10:10 ` [PATCH RESEND 9/9] CI: test qemu-riscv64_smode[_acpi] Heinrich Schuchardt
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.