From: Tomoyuki Hirose <hrstmyk811m@gmail.com>
This commit adds a test device for checking memory access. The test
device generates memory regions that covers all the legal parameter
patterns. With this device, we can check the handling of
reading/writing the MemoryRegion is correct.
Co-developed-by: CJ Chen <cjchen@igel.co.jp>
Signed-off-by: CJ Chen <cjchen@igel.co.jp>
Tested-by: CJ Chen <cjchen@igel.co.jp>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
---
v2:
- Fix the typo of ops size of big-l-valid.
- Replaced the huge macro blocks with dynamic loops that fill in
the `MemoryRegionOps` arrays at runtime.
- Remove test cases valid.unaligned = false,impl.unaligned = true.
---
hw/misc/Kconfig | 4 +
hw/misc/memaccess-testdev.c | 331 ++++++++++++++++++++++++++++
hw/misc/meson.build | 1 +
include/hw/misc/memaccess-testdev.h | 104 +++++++++
4 files changed, 440 insertions(+)
create mode 100644 hw/misc/memaccess-testdev.c
create mode 100644 include/hw/misc/memaccess-testdev.h
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index ec0fa5aa9f..ff7d7c65ef 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -25,6 +25,10 @@ config PCI_TESTDEV
default y if TEST_DEVICES
depends on PCI
+config MEMACCESS_TESTDEV
+ bool
+ default y if TEST_DEVICES
+
config EDU
bool
default y if TEST_DEVICES
diff --git a/hw/misc/memaccess-testdev.c b/hw/misc/memaccess-testdev.c
new file mode 100644
index 0000000000..1aaa52c69f
--- /dev/null
+++ b/hw/misc/memaccess-testdev.c
@@ -0,0 +1,331 @@
+/*
+ * QEMU memory access test device
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Author: Tomoyuki HIROSE <hrstmyk811m@gmail.com>
+ *
+ * This device is used to test memory acccess, like:
+ * qemu-system-x86_64 -device memaccess-testdev,address=0x10000000
+ */
+
+#include "qemu/osdep.h"
+#include "system/address-spaces.h"
+#include "system/memory.h"
+#include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
+#include "qemu/typedefs.h"
+#include "qom/object.h"
+#include "hw/misc/memaccess-testdev.h"
+
+typedef bool (*skip_func_ptr)(uint32_t valid_max, uint32_t valid_min,
+ bool valid_unaligned, uint32_t impl_max,
+ uint32_t impl_min, bool impl_unaligned);
+
+typedef struct MrOpsList {
+ const char *name;
+ MemoryRegionOps *ops_array;
+ const size_t ops_array_len;
+ const size_t offset_idx;
+ skip_func_ptr skip_fn;
+ bool is_little;
+} MrOpsList;
+
+MemoryRegionOps ops_list_little_b_valid[N_OPS_LIST_LITTLE_B_VALID];
+MemoryRegionOps ops_list_little_b_invalid[N_OPS_LIST_LITTLE_B_INVALID];
+MemoryRegionOps ops_list_little_w_valid[N_OPS_LIST_LITTLE_W_VALID];
+MemoryRegionOps ops_list_little_w_invalid[N_OPS_LIST_LITTLE_W_INVALID];
+MemoryRegionOps ops_list_little_l_valid[N_OPS_LIST_LITTLE_L_VALID];
+MemoryRegionOps ops_list_little_l_invalid[N_OPS_LIST_LITTLE_L_INVALID];
+MemoryRegionOps ops_list_little_q_valid[N_OPS_LIST_LITTLE_Q_VALID];
+MemoryRegionOps ops_list_little_q_invalid[N_OPS_LIST_LITTLE_Q_INVALID];
+MemoryRegionOps ops_list_big_b_valid[N_OPS_LIST_BIG_B_VALID];
+MemoryRegionOps ops_list_big_b_invalid[N_OPS_LIST_BIG_B_INVALID];
+MemoryRegionOps ops_list_big_w_valid[N_OPS_LIST_BIG_W_VALID];
+MemoryRegionOps ops_list_big_w_invalid[N_OPS_LIST_BIG_W_INVALID];
+MemoryRegionOps ops_list_big_l_valid[N_OPS_LIST_BIG_L_VALID];
+MemoryRegionOps ops_list_big_l_invalid[N_OPS_LIST_BIG_L_INVALID];
+MemoryRegionOps ops_list_big_q_valid[N_OPS_LIST_BIG_Q_VALID];
+MemoryRegionOps ops_list_big_q_invalid[N_OPS_LIST_BIG_Q_INVALID];
+
+static bool skip_core(uint32_t required_min, bool valid_test,
+ uint32_t valid_max, uint32_t valid_min,
+ bool valid_unaligned, uint32_t impl_max,
+ uint32_t impl_min, bool impl_unaligned)
+{
+ if (valid_min != required_min) {
+ return true;
+ }
+ if (valid_test) {
+ if (!valid_unaligned) {
+ return true;
+ }
+ } else {
+ if (valid_unaligned || impl_unaligned) {
+ return true;
+ }
+ }
+ if (valid_max < valid_min) {
+ return true;
+ }
+
+ if (impl_max < impl_min) {
+ return true;
+ }
+
+ return false;
+}
+
+#define DEFINE_SKIP_VALID_INVALID_FN(NAME, REQ_MIN) \
+ static bool skip_##NAME##_valid(uint32_t vm, uint32_t vn, bool vu, \
+ uint32_t im, uint32_t in, bool iu) \
+ { \
+ return skip_core(REQ_MIN, true, vm, vn, vu, im, in, iu); \
+ } \
+ \
+ static bool skip_##NAME##_invalid(uint32_t vm, uint32_t vn, bool vu, \
+ uint32_t im, uint32_t in, bool iu) \
+ { \
+ return skip_core(REQ_MIN, false, vm, vn, vu, im, in, iu); \
+ }
+
+DEFINE_SKIP_VALID_INVALID_FN(b, 1)
+DEFINE_SKIP_VALID_INVALID_FN(w, 2)
+DEFINE_SKIP_VALID_INVALID_FN(l, 4)
+DEFINE_SKIP_VALID_INVALID_FN(q, 8)
+
+static void testdev_init_memory_region(MemoryRegion *mr,
+ Object *owner,
+ const MemoryRegionOps *ops,
+ void *opaque,
+ const char *name,
+ uint64_t size,
+ MemoryRegion *container,
+ hwaddr container_offset)
+{
+ memory_region_init_io(mr, owner, ops, opaque, name, size);
+ memory_region_add_subregion(container, container_offset, mr);
+}
+
+static void testdev_init_from_mr_ops_list(MemAccessTestDev *testdev,
+ const MrOpsList *l)
+{
+ for (size_t i = 0; i < l->ops_array_len; i++) {
+ g_autofree gchar *name = g_strdup_printf("%s-%ld", l->name, i);
+ testdev_init_memory_region(&testdev->memory_regions[l->offset_idx + i],
+ OBJECT(testdev), &l->ops_array[i],
+ testdev->mr_data[l->offset_idx + i],
+ name,
+ MEMACCESS_TESTDEV_REGION_SIZE,
+ &testdev->container,
+ MEMACCESS_TESTDEV_REGION_SIZE *
+ (l->offset_idx + i));
+ }
+}
+
+#define LITTLE 1
+#define BIG 0
+#define _DEFINE_MR_OPS_LIST(_n, _ops, _len, _off, _skipfn, _is_little) \
+{ \
+ .name = (_n), \
+ .ops_array = (_ops), \
+ .ops_array_len = (_len), \
+ .offset_idx = (_off), \
+ .skip_fn = (_skipfn), \
+ .is_little = (_is_little), \
+}
+
+#define DEFINE_MR_OPS_LIST(e, E, w, W, v, V) \
+ _DEFINE_MR_OPS_LIST( \
+ #e "-" #w "-" #v, /* .name */ \
+ ops_list_##e##_##w##_##v, /* .ops_array */ \
+ N_OPS_LIST_##E##_##W##_##V, /* .ops_array_len */ \
+ OFF_IDX_OPS_LIST_##E##_##W##_##V,/* .offset_idx */ \
+ skip_##w##_##v, /* .skip_fn */ \
+ E /* .is_little => 1 = little endian, 0 = big endian */ \
+ )
+
+static MrOpsList mr_ops_list[] = {
+ DEFINE_MR_OPS_LIST(little, LITTLE, b, B, valid, VALID),
+ DEFINE_MR_OPS_LIST(little, LITTLE, b, B, invalid, INVALID),
+ DEFINE_MR_OPS_LIST(little, LITTLE, w, W, valid, VALID),
+ DEFINE_MR_OPS_LIST(little, LITTLE, w, W, invalid, INVALID),
+ DEFINE_MR_OPS_LIST(little, LITTLE, l, L, valid, VALID),
+ DEFINE_MR_OPS_LIST(little, LITTLE, l, L, invalid, INVALID),
+ DEFINE_MR_OPS_LIST(little, LITTLE, q, Q, valid, VALID),
+ DEFINE_MR_OPS_LIST(little, LITTLE, q, Q, invalid, INVALID),
+ DEFINE_MR_OPS_LIST(big, BIG, b, B, valid, VALID),
+ DEFINE_MR_OPS_LIST(big, BIG, b, B, invalid, INVALID),
+ DEFINE_MR_OPS_LIST(big, BIG, w, W, valid, VALID),
+ DEFINE_MR_OPS_LIST(big, BIG, w, W, invalid, INVALID),
+ DEFINE_MR_OPS_LIST(big, BIG, l, L, valid, VALID),
+ DEFINE_MR_OPS_LIST(big, BIG, l, L, invalid, INVALID),
+ DEFINE_MR_OPS_LIST(big, BIG, q, Q, valid, VALID),
+ DEFINE_MR_OPS_LIST(big, BIG, q, Q, invalid, INVALID),
+};
+#undef LITTLE
+#undef BIG
+
+static uint64_t memaccess_testdev_read_little(void *opaque, hwaddr addr,
+ unsigned int size)
+{
+ g_assert(addr + size < MEMACCESS_TESTDEV_REGION_SIZE);
+ void *s = (uint8_t *)opaque + addr;
+ return ldn_le_p(s, size);
+}
+
+static void memaccess_testdev_write_little(void *opaque, hwaddr addr,
+ uint64_t data, unsigned int size)
+{
+ g_assert(addr + size < MEMACCESS_TESTDEV_REGION_SIZE);
+ void *d = (uint8_t *)opaque + addr;
+ stn_le_p(d, size, data);
+}
+
+static uint64_t memaccess_testdev_read_big(void *opaque, hwaddr addr,
+ unsigned int size)
+{
+ g_assert(addr + size < MEMACCESS_TESTDEV_REGION_SIZE);
+ void *s = (uint8_t *)opaque + addr;
+ return ldn_be_p(s, size);
+}
+
+static void memaccess_testdev_write_big(void *opaque, hwaddr addr,
+ uint64_t data, unsigned int size)
+{
+ g_assert(addr + size < MEMACCESS_TESTDEV_REGION_SIZE);
+ void *d = (uint8_t *)opaque + addr;
+ stn_be_p(d, size, data);
+}
+
+static void fill_ops_list(MemoryRegionOps *ops,
+ skip_func_ptr fptr,
+ size_t ops_len,
+ bool is_little)
+{
+ static const uint32_t sizes[] = { 1, 2, 4, 8 };
+ static const bool bools[] = { false, true };
+ int idx = 0;
+
+ for (int vMaxIdx = 0; vMaxIdx < 4; vMaxIdx++) {
+ for (int vMinIdx = 0; vMinIdx < 4; vMinIdx++) {
+ for (int vUIdx = 0; vUIdx < 2; vUIdx++) {
+ for (int iMaxIdx = 0; iMaxIdx < 4; iMaxIdx++) {
+ for (int iMinIdx = 0; iMinIdx < 4; iMinIdx++) {
+ for (int iUIdx = 0; iUIdx < 2; iUIdx++) {
+ uint32_t valid_max = sizes[vMaxIdx];
+ uint32_t valid_min = sizes[vMinIdx];
+ bool valid_unaligned = bools[vUIdx];
+ uint32_t impl_max = sizes[iMaxIdx];
+ uint32_t impl_min = sizes[iMinIdx];
+ bool impl_unaligned = bools[iUIdx];
+
+ if (!fptr(valid_max, valid_min, valid_unaligned,
+ impl_max, impl_min, impl_unaligned))
+ {
+ const MemoryRegionOps new_op = {
+ .read = is_little ?
+ memaccess_testdev_read_little :
+ memaccess_testdev_read_big,
+ .write = is_little ?
+ memaccess_testdev_write_little :
+ memaccess_testdev_write_big,
+ .endianness = is_little ?
+ DEVICE_LITTLE_ENDIAN :
+ DEVICE_BIG_ENDIAN,
+ .valid = {
+ .max_access_size = valid_max,
+ .min_access_size = valid_min,
+ .unaligned = valid_unaligned,
+ },
+ .impl = {
+ .max_access_size = impl_max,
+ .min_access_size = impl_min,
+ .unaligned = impl_unaligned,
+ },
+ };
+
+ ops[idx] = new_op;
+ idx++;
+ if (idx > ops_len) {
+ g_assert_not_reached();
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+}
+
+#define N_MR_OPS_LIST (sizeof(mr_ops_list) / sizeof(MrOpsList))
+
+static void init_testdev(MemAccessTestDev *testdev)
+{
+ memory_region_init(&testdev->container, OBJECT(testdev), "memtest-regions",
+ MEMACCESS_TESTDEV_REGION_SIZE * N_OPS_LIST);
+ testdev->mr_data = g_malloc(MEMACCESS_TESTDEV_MR_DATA_SIZE);
+
+ for (size_t i = 0; i < N_MR_OPS_LIST; i++) {
+ fill_ops_list(
+ mr_ops_list[i].ops_array,
+ mr_ops_list[i].skip_fn,
+ mr_ops_list[i].ops_array_len,
+ mr_ops_list[i].is_little
+ );
+ testdev_init_from_mr_ops_list(testdev, &mr_ops_list[i]);
+ }
+
+ memory_region_add_subregion(get_system_memory(), testdev->base,
+ &testdev->container);
+}
+
+static void memaccess_testdev_realize(DeviceState *dev, Error **errp)
+{
+ MemAccessTestDev *d = MEM_ACCESS_TEST_DEV(dev);
+
+ if (d->base == UINT64_MAX) {
+ error_setg(errp, "base address is not assigned");
+ return;
+ }
+
+ init_testdev(d);
+}
+
+static void memaccess_testdev_unrealize(DeviceState *dev)
+{
+ MemAccessTestDev *d = MEM_ACCESS_TEST_DEV(dev);
+ g_free(d->mr_data);
+}
+
+static Property memaccess_testdev_props[] = {
+ DEFINE_PROP_UINT64("address", MemAccessTestDev, base, UINT64_MAX),
+};
+
+static void memaccess_testdev_class_init(ObjectClass *klass, const void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = memaccess_testdev_realize;
+ dc->unrealize = memaccess_testdev_unrealize;
+ device_class_set_props_n(dc,
+ memaccess_testdev_props,
+ ARRAY_SIZE(memaccess_testdev_props));
+ set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo memaccess_testdev_info = {
+ .name = TYPE_MEM_ACCESS_TEST_DEV,
+ .parent = TYPE_DEVICE,
+ .instance_size = sizeof(MemAccessTestDev),
+ .class_init = memaccess_testdev_class_init,
+};
+
+static void memaccess_testdev_register_types(void)
+{
+ type_register_static(&memaccess_testdev_info);
+}
+
+type_init(memaccess_testdev_register_types)
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 6d47de482c..f06568aaed 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -4,6 +4,7 @@ system_ss.add(when: 'CONFIG_FW_CFG_DMA', if_true: files('vmcoreinfo.c'))
system_ss.add(when: 'CONFIG_ISA_DEBUG', if_true: files('debugexit.c'))
system_ss.add(when: 'CONFIG_ISA_TESTDEV', if_true: files('pc-testdev.c'))
system_ss.add(when: 'CONFIG_PCI_TESTDEV', if_true: files('pci-testdev.c'))
+system_ss.add(when: 'CONFIG_MEMACCESS_TESTDEV', if_true: files('memaccess-testdev.c'))
system_ss.add(when: 'CONFIG_UNIMP', if_true: files('unimp.c'))
system_ss.add(when: 'CONFIG_EMPTY_SLOT', if_true: files('empty_slot.c'))
system_ss.add(when: 'CONFIG_LED', if_true: files('led.c'))
diff --git a/include/hw/misc/memaccess-testdev.h b/include/hw/misc/memaccess-testdev.h
new file mode 100644
index 0000000000..c1b17297a2
--- /dev/null
+++ b/include/hw/misc/memaccess-testdev.h
@@ -0,0 +1,104 @@
+/*
+ * QEMU memory access test device header
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Author: Tomoyuki HIROSE <hrstmyk811m@gmail.com>
+ */
+
+#ifndef HW_MISC_MEMACCESS_TESTDEV_H
+#define HW_MISC_MEMACCESS_TESTDEV_H
+
+#include "system/memory.h"
+#include "hw/qdev-core.h"
+
+#define TYPE_MEM_ACCESS_TEST_DEV "memaccess-testdev"
+
+#define MEMACCESS_TESTDEV_REGION_SIZE 32
+
+#define N_OPS_LIST_LITTLE_B_VALID 80
+#define N_OPS_LIST_LITTLE_B_INVALID 40
+#define N_OPS_LIST_LITTLE_W_VALID 60
+#define N_OPS_LIST_LITTLE_W_INVALID 30
+#define N_OPS_LIST_LITTLE_L_VALID 40
+#define N_OPS_LIST_LITTLE_L_INVALID 20
+#define N_OPS_LIST_LITTLE_Q_VALID 20
+#define N_OPS_LIST_LITTLE_Q_INVALID 10
+#define N_OPS_LIST_BIG_B_VALID 80
+#define N_OPS_LIST_BIG_B_INVALID 40
+#define N_OPS_LIST_BIG_W_VALID 60
+#define N_OPS_LIST_BIG_W_INVALID 30
+#define N_OPS_LIST_BIG_L_VALID 40
+#define N_OPS_LIST_BIG_L_INVALID 20
+#define N_OPS_LIST_BIG_Q_VALID 20
+#define N_OPS_LIST_BIG_Q_INVALID 10
+
+#define N_OPS_LIST \
+ (N_OPS_LIST_LITTLE_B_VALID + \
+ N_OPS_LIST_LITTLE_B_INVALID + \
+ N_OPS_LIST_LITTLE_W_VALID + \
+ N_OPS_LIST_LITTLE_W_INVALID + \
+ N_OPS_LIST_LITTLE_L_VALID + \
+ N_OPS_LIST_LITTLE_L_INVALID + \
+ N_OPS_LIST_LITTLE_Q_VALID + \
+ N_OPS_LIST_LITTLE_Q_INVALID + \
+ N_OPS_LIST_BIG_B_VALID + \
+ N_OPS_LIST_BIG_B_INVALID + \
+ N_OPS_LIST_BIG_W_VALID + \
+ N_OPS_LIST_BIG_W_INVALID + \
+ N_OPS_LIST_BIG_L_VALID + \
+ N_OPS_LIST_BIG_L_INVALID + \
+ N_OPS_LIST_BIG_Q_VALID + \
+ N_OPS_LIST_BIG_Q_INVALID)
+
+#define OFF_IDX_OPS_LIST_LITTLE_B_VALID \
+ (0)
+#define OFF_IDX_OPS_LIST_LITTLE_B_INVALID \
+ (OFF_IDX_OPS_LIST_LITTLE_B_VALID + N_OPS_LIST_LITTLE_B_VALID)
+#define OFF_IDX_OPS_LIST_LITTLE_W_VALID \
+ (OFF_IDX_OPS_LIST_LITTLE_B_INVALID + N_OPS_LIST_LITTLE_B_INVALID)
+#define OFF_IDX_OPS_LIST_LITTLE_W_INVALID \
+ (OFF_IDX_OPS_LIST_LITTLE_W_VALID + N_OPS_LIST_LITTLE_W_VALID)
+#define OFF_IDX_OPS_LIST_LITTLE_L_VALID \
+ (OFF_IDX_OPS_LIST_LITTLE_W_INVALID + N_OPS_LIST_LITTLE_W_INVALID)
+#define OFF_IDX_OPS_LIST_LITTLE_L_INVALID \
+ (OFF_IDX_OPS_LIST_LITTLE_L_VALID + N_OPS_LIST_LITTLE_L_VALID)
+#define OFF_IDX_OPS_LIST_LITTLE_Q_VALID \
+ (OFF_IDX_OPS_LIST_LITTLE_L_INVALID + N_OPS_LIST_LITTLE_L_INVALID)
+#define OFF_IDX_OPS_LIST_LITTLE_Q_INVALID \
+ (OFF_IDX_OPS_LIST_LITTLE_Q_VALID + N_OPS_LIST_LITTLE_Q_VALID)
+#define OFF_IDX_OPS_LIST_BIG_B_VALID \
+ (OFF_IDX_OPS_LIST_LITTLE_Q_INVALID + N_OPS_LIST_LITTLE_Q_INVALID)
+#define OFF_IDX_OPS_LIST_BIG_B_INVALID \
+ (OFF_IDX_OPS_LIST_BIG_B_VALID + N_OPS_LIST_BIG_B_VALID)
+#define OFF_IDX_OPS_LIST_BIG_W_VALID \
+ (OFF_IDX_OPS_LIST_BIG_B_INVALID + N_OPS_LIST_BIG_B_INVALID)
+#define OFF_IDX_OPS_LIST_BIG_W_INVALID \
+ (OFF_IDX_OPS_LIST_BIG_W_VALID + N_OPS_LIST_BIG_W_VALID)
+#define OFF_IDX_OPS_LIST_BIG_L_VALID \
+ (OFF_IDX_OPS_LIST_BIG_W_INVALID + N_OPS_LIST_BIG_W_INVALID)
+#define OFF_IDX_OPS_LIST_BIG_L_INVALID \
+ (OFF_IDX_OPS_LIST_BIG_L_VALID + N_OPS_LIST_BIG_L_VALID)
+#define OFF_IDX_OPS_LIST_BIG_Q_VALID \
+ (OFF_IDX_OPS_LIST_BIG_L_INVALID + N_OPS_LIST_BIG_L_INVALID)
+#define OFF_IDX_OPS_LIST_BIG_Q_INVALID \
+ (OFF_IDX_OPS_LIST_BIG_Q_VALID + N_OPS_LIST_BIG_Q_VALID)
+
+typedef uint8_t MrData[MEMACCESS_TESTDEV_REGION_SIZE];
+#define MEMACCESS_TESTDEV_MR_DATA_SIZE (sizeof(MrData) * N_OPS_LIST)
+
+typedef DeviceClass MemAccessTestDevClass;
+typedef struct MemAccessTestDev {
+ /* Private */
+ DeviceState parent_obj;
+ /* Public */
+ MemoryRegion container;
+ MemoryRegion memory_regions[N_OPS_LIST]; /* test memory regions */
+ uint64_t base; /* map base address */
+ MrData *mr_data; /* memory region data array */
+} MemAccessTestDev;
+
+#define MEM_ACCESS_TEST_DEV(obj) \
+ OBJECT_CHECK(MemAccessTestDev, obj, TYPE_MEM_ACCESS_TEST_DEV)
+
+#endif
--
2.25.1
On Fri, 22 Aug 2025 at 10:25, CJ Chen <cjchen@igel.co.jp> wrote: > > From: Tomoyuki Hirose <hrstmyk811m@gmail.com> > > This commit adds a test device for checking memory access. The test > device generates memory regions that covers all the legal parameter > patterns. With this device, we can check the handling of > reading/writing the MemoryRegion is correct. > > Co-developed-by: CJ Chen <cjchen@igel.co.jp> > Signed-off-by: CJ Chen <cjchen@igel.co.jp> > Tested-by: CJ Chen <cjchen@igel.co.jp> > Suggested-by: Peter Maydell <peter.maydell@linaro.org> > --- > v2: > - Fix the typo of ops size of big-l-valid. > - Replaced the huge macro blocks with dynamic loops that fill in > the `MemoryRegionOps` arrays at runtime. > - Remove test cases valid.unaligned = false,impl.unaligned = true. > diff --git a/include/hw/misc/memaccess-testdev.h b/include/hw/misc/memaccess-testdev.h > new file mode 100644 > index 0000000000..c1b17297a2 > --- /dev/null > +++ b/include/hw/misc/memaccess-testdev.h > @@ -0,0 +1,104 @@ > +/* > + * QEMU memory access test device header > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + * > + * Author: Tomoyuki HIROSE <hrstmyk811m@gmail.com> > + */ > + > +#ifndef HW_MISC_MEMACCESS_TESTDEV_H > +#define HW_MISC_MEMACCESS_TESTDEV_H > + > +#include "system/memory.h" > +#include "hw/qdev-core.h" > + > +#define TYPE_MEM_ACCESS_TEST_DEV "memaccess-testdev" Could we have a comment in this header file that documents what interface the test device presents to tests, please? Both this patch and the test-case patch are hard to review, because I don't know what the test device is trying to do or what the test code is able to assume about the test device. thanks -- PMM
On Mon, Sep 01, 2025 at 06:03:41PM +0100, Peter Maydell wrote:
> Could we have a comment in this header file that documents
> what interface the test device presents to tests, please?
> Both this patch and the test-case patch are hard to
> review, because I don't know what the test device is
> trying to do or what the test code is able to assume
> about the test device.
Since the series is withdrawed.. but still I feel like I'll need to read
this series when it's picked up again, I took some time (while the brain
cache is still around..) study the code, I think I get the rough idea of
what this testdev is about. If it's gonna be picked up by anyone, hope
below could help a bit. Also for future myself..
Firstly, the testdev creates a bunch of MRs, with all kinds of the
attributes to cover all max/min access sizes possible & unaligned access &
endianess. The test cases are exactly tailored for this testdev, as the
testcase needs to know exactly which offset contains which type of MR. The
tests must be run correspondingly on the paired MR to work.
There're 16 groups of MRs / tests, each group contains below num of MRs:
#define N_OPS_LIST_LITTLE_B_VALID 80
#define N_OPS_LIST_LITTLE_B_INVALID 40
#define N_OPS_LIST_LITTLE_W_VALID 60
#define N_OPS_LIST_LITTLE_W_INVALID 30
#define N_OPS_LIST_LITTLE_L_VALID 40
#define N_OPS_LIST_LITTLE_L_INVALID 20
#define N_OPS_LIST_LITTLE_Q_VALID 20
#define N_OPS_LIST_LITTLE_Q_INVALID 10
#define N_OPS_LIST_BIG_B_VALID 80
#define N_OPS_LIST_BIG_B_INVALID 40
#define N_OPS_LIST_BIG_W_VALID 60
#define N_OPS_LIST_BIG_W_INVALID 30
#define N_OPS_LIST_BIG_L_VALID 40
#define N_OPS_LIST_BIG_L_INVALID 20
#define N_OPS_LIST_BIG_Q_VALID 20
#define N_OPS_LIST_BIG_Q_INVALID 10
That's a total of 600 (which is, N_OPS_LIST) MRs at the base address of the
testdev, specified by, for example:
-device memaccess-testdev,address=0x10000000
Each MR will be 32B (MEMACCESS_TESTDEV_REGION_SIZE) in size, sequentially
appended and installed to base address offset. All of them internally
backed by:
testdev->mr_data = g_malloc(MEMACCESS_TESTDEV_MR_DATA_SIZE);
Here, BIG/LITTLE decides the endianess of the MR, B/W/L/Q decides the
min_access_size of the MR, which isn't clear at all to me.. IIUC it's
hidden inside the skip_core() check where it skips anything except when
valid_min == required_min. So only those MRs that satisfy will be created.
And just to mention, IIUC these numbers are not random either, they are
exactly how many possible MRs that we can create under the specific
category of MR group. Changing that could either causing uninitialized MRs
(under some group) or trigger assertions saying MR list too small:
fill_ops_list:
if (idx > ops_len) {
g_assert_not_reached();
}
This is not clear either.. better document this if it will be picked up one
day.
An example for definition of N_OPS_LIST_LITTLE_B_VALID: we can create 80
such MRs when the MR is (1) LITTLE (2) min_access_size=1 (3) allows
.valid.unaligned. We'll skip the rest in skip_core() when building the
list of MRs. And yes, here (3) isn't clear either: VALID here means "the
MR allows unaligned access from API level", which implies
.valid.unaligned=true. OTOH, INVALID implies .valid.unaligned=false.
NOTE: it doesn't imply at all about .impl.unaligned.
The test case should be tailored for this device, because for each test it
will run, it'll run exactly on top of the group of MRs that the test case
pairs with.
Taking example of big_w_valid(): it'll run the test on all MRs that is (1)
BIG, (2) min_access_size=2, (3) VALID to use unaligned access, aka,
.valid.unaligned=true.
Said above, I'll also raise a question that I don't understand, on the
testdev implementation. It's about endianess of the MR and how data
endianess is processed in the test dev. Please shoot if anyone knows.
Again, taking the example of BIG write() of a test MR, I wonder why the
testdev has this:
static void memaccess_testdev_write_big(void *opaque, hwaddr addr,
uint64_t data, unsigned int size)
{
g_assert(addr + size < MEMACCESS_TESTDEV_REGION_SIZE);
void *d = (uint8_t *)opaque + addr;
stn_be_p(d, size, data);
}
It means when the ->write() triggers, it assumes "data" here is host
endianess, then convert that to MR's endianess, which is BE in this case.
But haven't we already done that before reaching ->write()? Memory core
will do the endianess conversion (from HOST -> MR endianess) already here
before reaching the write() hook, AFAICT:
memory_region_dispatch_write()
adjust_endianness(mr, &data, op);
if ((op & MO_BSWAP) != devend_memop(mr->ops->endianness)) {
Here, MO_BSWAP shouldn't normally be set. devend_memop() should read BE.
On my host (x86_64) it means it'll swap once to MR endianess. IIUC, now
the whole "data" field already in MR endianess and should be directly put
into the backend memdev storage. I don't think I understand why above
stn_be_p() is not a memcpy(). Answers welcomed..
--
Peter Xu
© 2016 - 2025 Red Hat, Inc.