Hi Philippe,
Thanks for the review and the pointer.
You’re right — the CPUs are not involved in the test path, and the only
difference that matters is the binary endianness. I will re-spin the
series and reduce the test matrix to one little-endian and one big-endian
target to avoid redundant runs.
Specifically, I will keep:
- x86_64 (cpu: "qemu64,apic-id=0") — little-endian
- s390x (cpu: "qemu") — big-endian
Thanks again for the feedback. If you have any further comments, please
let me know.
Best regards,
CJ
Philippe Mathieu-Daudé <phi...@linaro.org> 於 2025年8月25日 週一 下午8:16寫道：
>
> On 22/8/25 11:24, CJ Chen wrote:
> > From: Tomoyuki Hirose <hrstmyk8...@gmail.com>
> >
> > This commit adds a qtest for accessing various memory regions. The
> > qtest checks the correctness of handling the access to memory regions
> > by using 'memaccess-testdev'.
> >
> > Signed-off-by: CJ Chen <cjc...@igel.co.jp>
> > Co-developed-by: CJ Chen <cjc...@igel.co.jp>
> > Reported-by: Tomoyuki Hirose <hrstmyk8...@gmail.com>
> > ---
> > tests/qtest/memaccess-test.c | 597 +++++++++++++++++++++++++++++++++++
> > tests/qtest/meson.build | 9 +
> > 2 files changed, 606 insertions(+)
> > create mode 100644 tests/qtest/memaccess-test.c
> >
> > diff --git a/tests/qtest/memaccess-test.c b/tests/qtest/memaccess-test.c
> > new file mode 100644
> > index 0000000000..7e90028ea0
> > --- /dev/null
> > +++ b/tests/qtest/memaccess-test.c
> > @@ -0,0 +1,597 @@
> > +/*
> > + * QEMU memory region access test
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * Author: Tomoyuki HIROSE <hrstmyk8...@gmail.com>
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "libqtest.h"
> > +
> > +#include "hw/misc/memaccess-testdev.h"
> > +
> > +static const char *arch = "";
> > +static const hwaddr base = 0x200000000;
> > +
> > +struct arch2cpu {
> > + const char *arch;
> > + const char *cpu_model;
> > +};
> > +
> > +static struct arch2cpu cpus_map[] = {
> > + /* tested targets list */
> > + { "arm", "cortex-a15" },
> > + { "aarch64", "cortex-a57" },
> > + { "avr", "avr6-avr-cpu" },
> > + { "x86_64", "qemu64,apic-id=0" },
> > + { "i386", "qemu32,apic-id=0" },
> > + { "alpha", "ev67" },
> > + { "cris", "crisv32" },
> > + { "m68k", "m5206" },
> > + { "microblaze", "any" },
> > + { "microblazeel", "any" },
> > + { "mips", "4Kc" },
> > + { "mipsel", "I7200" },
> > + { "mips64", "20Kc" },
> > + { "mips64el", "I6500" },
> > + { "or1k", "or1200" },
> > + { "ppc", "604" },
> > + { "ppc64", "power8e_v2.1" },
> > + { "s390x", "qemu" },
> > + { "sh4", "sh7750r" },
> > + { "sh4eb", "sh7751r" },
> > + { "sparc", "LEON2" },
> > + { "sparc64", "Fujitsu Sparc64" },
> > + { "tricore", "tc1796" },
> > + { "xtensa", "dc233c" },
> > + { "xtensaeb", "fsf" },
> > + { "hppa", "hppa" },
> > + { "riscv64", "rv64" },
> > + { "riscv32", "rv32" },
> > + { "rx", "rx62n" },
> > + { "loongarch64", "la464" },
>
> IIUC CPUs are not involved in the test path. The only difference
> is the binary endianness. So we are testing 2 distinct code path
> duplicated as ARRAY_SIZE(cpus_map) = 31 times.
>
> Let's run the tests with a pair of common targets and save 29
> pointless tests:
>
> ... cpus_map[] = {
> /* One little endian and one big endian target */
> { "x86_64", "qemu64,apic-id=0" },
> { "s390x", "qemu" }
> }
>
> > +};
> > +
> > +static const char *get_cpu_model_by_arch(const char *arch)
> > +{
> > + for (int i = 0; i < ARRAY_SIZE(cpus_map); i++) {
> > + if (!strcmp(arch, cpus_map[i].arch)) {
> > + return cpus_map[i].cpu_model;
> > + }
> > + }
> > + return NULL;
> > +}
> > +
> > +static QTestState *create_memaccess_qtest(void)
> > +{
> > + QTestState *qts;
> > +
> > + qts = qtest_initf("-machine none -cpu \"%s\" "
> > + "-device memaccess-testdev,address=0x%" PRIx64,
> > + get_cpu_model_by_arch(arch), base);
> > + return qts;
> > +}
>
>
> > +DEFINE_test_memaccess(little, LITTLE, b, B, valid, VALID)
> > +DEFINE_test_memaccess(little, LITTLE, w, W, valid, VALID)
> > +DEFINE_test_memaccess(little, LITTLE, l, L, valid, VALID)
> > +DEFINE_test_memaccess(little, LITTLE, q, Q, valid, VALID)
> > +DEFINE_test_memaccess(little, LITTLE, b, B, invalid, INVALID)
> > +DEFINE_test_memaccess(little, LITTLE, w, W, invalid, INVALID)
> > +DEFINE_test_memaccess(little, LITTLE, l, L, invalid, INVALID)
> > +DEFINE_test_memaccess(little, LITTLE, q, Q, invalid, INVALID)
> > +DEFINE_test_memaccess(big, BIG, b, B, valid, VALID)
> > +DEFINE_test_memaccess(big, BIG, w, W, valid, VALID)
> > +DEFINE_test_memaccess(big, BIG, l, L, valid, VALID)
> > +DEFINE_test_memaccess(big, BIG, q, Q, valid, VALID)
> > +DEFINE_test_memaccess(big, BIG, b, B, invalid, INVALID)
> > +DEFINE_test_memaccess(big, BIG, w, W, invalid, INVALID)
> > +DEFINE_test_memaccess(big, BIG, l, L, invalid, INVALID)
> > +DEFINE_test_memaccess(big, BIG, q, Q, invalid, INVALID)
> > +
> > +#undef DEFINE_test_memaccess
> > +
> > +static struct {
> > + const char *name;
> > + void (*test)(void);
> > +} tests[] = {
> > + {"little_b_valid", test_memaccess_little_b_valid},
> > + {"little_w_valid", test_memaccess_little_w_valid},
> > + {"little_l_valid", test_memaccess_little_l_valid},
> > + {"little_q_valid", test_memaccess_little_q_valid},
> > + {"little_b_invalid", test_memaccess_little_b_invalid},
> > + {"little_w_invalid", test_memaccess_little_w_invalid},
> > + {"little_l_invalid", test_memaccess_little_l_invalid},
> > + {"little_q_invalid", test_memaccess_little_q_invalid},
> > + {"big_b_valid", test_memaccess_big_b_valid},
> > + {"big_w_valid", test_memaccess_big_w_valid},
> > + {"big_l_valid", test_memaccess_big_l_valid},
> > + {"big_q_valid", test_memaccess_big_q_valid},
> > + {"big_b_invalid", test_memaccess_big_b_invalid},
> > + {"big_w_invalid", test_memaccess_big_w_invalid},
> > + {"big_l_invalid", test_memaccess_big_l_invalid},
> > + {"big_q_invalid", test_memaccess_big_q_invalid},
> > +};
> BTW this reminds me of
> https://lore.kernel.org/qemu-devel/20200817161853.593247-8-f4...@amsat.org/
> ;)
