[PATCH 0/9] CI: test qemu-riscv64_smode[_acpi]

[PATCH 0/9] CI: test qemu-riscv64_smode[_acpi]

[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

[PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY

[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

Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY

[Ilias Apalodimas]

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

Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY

[Tom Rini]

[-- 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 --]

Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY

[Heinrich Schuchardt]

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

Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY

[Heinrich Schuchardt]

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

Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY

[Tom Rini]

[-- 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 --]

Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY

[Tom Rini]

[-- 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 --]

Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY

[Heinrich Schuchardt]

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

Re: [PATCH RESEND 1/9] test: fdt_test_apply requires CONFIG_OF_LIBFDT_OVERLAY

[Tom Rini]

[-- 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 --]

[PATCH 2/9] test: cmd/fdt: do not assume RNG device exists

[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

Re: [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists

[Ilias Apalodimas]

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>

Re: [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists

[Simon Glass]

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

Re: [PATCH 2/9] test: cmd/fdt: do not assume RNG device exists

[Tom Rini]

[-- 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 --]

[PATCH 3/9] test: cmd/bdinfo: make no flash assumption

[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

Re: [PATCH 3/9] test: cmd/bdinfo: make no flash assumption

[Ilias Apalodimas]

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>

Re: [PATCH 3/9] test: cmd/bdinfo: make no flash assumption

[Simon Glass]

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

[PATCH 4/9] test: cmd/bdinfo: consider arch_print_bdinfo() output

[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

Re: [PATCH 4/9] test: cmd/bdinfo: consider arch_print_bdinfo() output

[Simon Glass]

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

[PATCH 5/9] test: common/print: do not use fixed buffer addresses

[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

Re: [PATCH 5/9] test: common/print: do not use fixed buffer addresses

[Ilias Apalodimas]

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
>

Re: [PATCH 5/9] test: common/print: do not use fixed buffer addresses

[Simon Glass]

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

[PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses

[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

Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses

[Ilias Apalodimas]

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>

Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses

[Simon Glass]

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

Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses

[Tom Rini]

[-- 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 --]

Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses

[Simon Glass]

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

Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses

[Tom Rini]

[-- 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 --]

Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses

[Simon Glass]

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

Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses

[Tom Rini]

[-- 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 --]

Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses

[Simon Glass]

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

Re: [PATCH 6/9] test: cmd/fdt: do not use fixed buffer addresses

[Heinrich Schuchardt]

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

[PATCH 7/9] common: default CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000

[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

Re: [PATCH 7/9] common: default CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000

[Ilias Apalodimas]

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>

Re: [PATCH 7/9] common: default CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000

[Simon Glass]

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.

[PATCH 8/9] configs: CONFIG_CONSOLE_RECORD=y on qemu-riscv64_smode_acpi

[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

[PATCH RESEND 9/9] CI: test qemu-riscv64_smode[_acpi]

[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