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