[RFC PATCH v2 0/9] support unaligned access to xHCI Capability

[RFC PATCH v2 0/9] support unaligned access to xHCI Capability

[CJ Chen]

This patch set aims to support unaligned access to xHCI Capability
Registers.

To achieve this, we introduce the emulation of an unaligned access
through multiple aligned accesses. This patch set also adds a test
device and several tests using this device to verify that the
emulation functions correctly.

Using these changes, unaligned access to xHCI Capability Registers is
now supported.

During development, I required a lot of 'MemoryRegionOps' structs with
its own read/write functions for tests. In the QEMU project, a large
number of similar functions or structs are often written in '.inc'
files. I followed this approach for the test functions but would
appreciate feedback on whether this is appropriate.

---
v1 ... 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.
   - Modification to the memory document about the alignment issue.
   - Update the npcm7xx_fiu, mx_pic and risc-v-iommu configuration 
     to align with the unaligned-access policy.
   - Document memory.rst clarify that .valid=true,.impl=false causes
     split unaligned accesses (may have side effects); forbid 
	 .valid=false,.impl=true via assertion.

---
 CJ Chen (4):
  doc/devel/memory.rst: additional explanation for unaligned access
  hw/riscv: iommu-trap: remove .impl.unaligned = true
  hw: npcm7xx_fiu and mx_pic change .impl.unaligned = true
  system/memory: assert on invalid unaligned combo

Tomoyuki Hirose (5):
  hw/nvme/ctrl: specify the 'valid' field in MemoryRegionOps
  system/memory: support unaligned access
  hw/usb/hcd-xhci: allow unaligned access to Capability Registers
  hw/misc: add test device for memory access
  tests/qtest: add test for memory region access

 docs/devel/memory.rst               |  18 +
 hw/misc/Kconfig                     |   4 +
 hw/misc/memaccess-testdev.c         | 331 +++++++++++++++
 hw/misc/meson.build                 |   1 +
 hw/nvme/ctrl.c                      |   5 +
 hw/riscv/riscv-iommu.c              |   1 -
 hw/ssi/npcm7xx_fiu.c                |   3 +
 hw/usb/hcd-xhci.c                   |   4 +-
 hw/xtensa/mx_pic.c                  |   3 +
 include/hw/misc/memaccess-testdev.h | 104 +++++
 system/memory.c                     | 148 +++++--
 system/physmem.c                    |   8 -
 tests/qtest/memaccess-test.c        | 597 ++++++++++++++++++++++++++++
 tests/qtest/meson.build             |   9 +
 14 files changed, 1198 insertions(+), 38 deletions(-)
 create mode 100644 hw/misc/memaccess-testdev.c
 create mode 100644 include/hw/misc/memaccess-testdev.h
 create mode 100644 tests/qtest/memaccess-test.c

base-commit: 5836af0783213b9355a6bbf85d9e6bc4c9c9363f
-- 
2.25.1

[RFC PATCH v2 1/9] doc/devel/memory.rst: additional explanation for unaligned access

[CJ Chen]

Add documentation to clarify that if `.valid.unaligned = true` but
`.impl.unaligned = false`, QEMU’s memory core will automatically split
unaligned guest accesses into multiple aligned accesses. This helps
devices avoid implementing their own unaligned logic, but can be
problematic for devices with side-effect-heavy registers. Also note
that setting `.valid.unaligned = false` together with
`.impl.unaligned = true` is invalid, as it contradicts itself and
will trigger an assertion.

Signed-off-by: CJ Chen <cjchen@igel.co.jp>
Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
---
 docs/devel/memory.rst | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
index 57fb2aec76..71d7de7ae5 100644
--- a/docs/devel/memory.rst
+++ b/docs/devel/memory.rst
@@ -365,6 +365,24 @@ callbacks are called:
 - .impl.unaligned specifies that the *implementation* supports unaligned
   accesses; if false, unaligned accesses will be emulated by two aligned
   accesses.
+- Additionally, if .valid.unaligned = true but .impl.unaligned = false, the
+  memory core will emulate each unaligned guest access by splitting it into
+  multiple aligned sub-accesses. This ensures that devices which only handle
+  aligned requests do not need to implement unaligned logic themselves. For
+  example, see xhci_cap_ops in hw/usb/hcd-xhci.c: it sets  .valid.unaligned
+  = true so guests can do unaligned reads on the xHCI Capability Registers,
+  while keeping .impl.unaligned = false to rely on the core splitting logic.
+  However, if a device’s registers have side effects on read or write, this
+  extra splitting can introduce undesired behavior. Specifically, for devices
+  whose registers trigger state changes on each read/write, splitting an access
+  can lead to reading or writing bytes beyond the originally requested subrange
+  thereby triggering repeated or otherwise unintended register side effects.
+  In such cases, one should set .valid.unaligned = false to reject unaligned
+  accesses entirely.
+- Conversely, if .valid.unaligned = false but .impl.unaligned = true,
+  that setting is considered invalid; it claims unaligned access is allowed
+  by the implementation yet disallowed for the device. QEMU enforces this with
+  an assertion to prevent contradictory usage.
 
 API Reference
 -------------
-- 
2.25.1

Re: [RFC PATCH v2 1/9] doc/devel/memory.rst: additional explanation for unaligned access

[Peter Maydell]

On Fri, 22 Aug 2025 at 10:25, CJ Chen <cjchen@igel.co.jp> wrote:
>
> Add documentation to clarify that if `.valid.unaligned = true` but
> `.impl.unaligned = false`, QEMU’s memory core will automatically split
> unaligned guest accesses into multiple aligned accesses. This helps
> devices avoid implementing their own unaligned logic, but can be
> problematic for devices with side-effect-heavy registers. Also note
> that setting `.valid.unaligned = false` together with
> `.impl.unaligned = true` is invalid, as it contradicts itself and
> will trigger an assertion.
>
> Signed-off-by: CJ Chen <cjchen@igel.co.jp>
> Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  docs/devel/memory.rst | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
> index 57fb2aec76..71d7de7ae5 100644
> --- a/docs/devel/memory.rst
> +++ b/docs/devel/memory.rst
> @@ -365,6 +365,24 @@ callbacks are called:
>  - .impl.unaligned specifies that the *implementation* supports unaligned
>    accesses; if false, unaligned accesses will be emulated by two aligned
>    accesses.
> +- Additionally, if .valid.unaligned = true but .impl.unaligned = false, the
> +  memory core will emulate each unaligned guest access by splitting it into
> +  multiple aligned sub-accesses.

Can we say in more specific detail what the core handling is, please?
I should be able to read this documentation and know exactly what
extra accesses I will get to my device. (Then I can make the decision
about whether I will be OK with those or if I will need to do the
unaligned handling myself.)

Documenting the behaviour we intend to provide will also make it
easier to review the implementation and the tests in the later patches.

> This ensures that devices which only handle
> +  aligned requests do not need to implement unaligned logic themselves. For
> +  example, see xhci_cap_ops in hw/usb/hcd-xhci.c: it sets  .valid.unaligned
> +  = true so guests can do unaligned reads on the xHCI Capability Registers,
> +  while keeping .impl.unaligned = false to rely on the core splitting logic.
> +  However, if a device’s registers have side effects on read or write, this
> +  extra splitting can introduce undesired behavior. Specifically, for devices
> +  whose registers trigger state changes on each read/write, splitting an access
> +  can lead to reading or writing bytes beyond the originally requested subrange
> +  thereby triggering repeated or otherwise unintended register side effects.
> +  In such cases, one should set .valid.unaligned = false to reject unaligned
> +  accesses entirely.

.valid.unaligned has to match what the real hardware requires.
So we should instead say something like:

 If your device must support unaligned accesses and these extra
 accesses would cause unintended side-effects, then you must set
 .impl.unaligned and handle the unaligned access cases yourself.

> +- Conversely, if .valid.unaligned = false but .impl.unaligned = true,
> +  that setting is considered invalid; it claims unaligned access is allowed
> +  by the implementation yet disallowed for the device. QEMU enforces this with
> +  an assertion to prevent contradictory usage.
>
>  API Reference
>  -------------
> --
> 2.25.1
>

thanks
-- PMM

Re: [RFC PATCH v2 1/9] doc/devel/memory.rst: additional explanation for unaligned access

[Peter Xu]

On Fri, Aug 22, 2025 at 06:24:02PM +0900, CJ Chen wrote:
> Add documentation to clarify that if `.valid.unaligned = true` but
> `.impl.unaligned = false`, QEMU’s memory core will automatically split
> unaligned guest accesses into multiple aligned accesses. This helps
> devices avoid implementing their own unaligned logic, but can be
> problematic for devices with side-effect-heavy registers. Also note
> that setting `.valid.unaligned = false` together with
> `.impl.unaligned = true` is invalid, as it contradicts itself and
> will trigger an assertion.
> 
> Signed-off-by: CJ Chen <cjchen@igel.co.jp>
> Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  docs/devel/memory.rst | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
> index 57fb2aec76..71d7de7ae5 100644
> --- a/docs/devel/memory.rst
> +++ b/docs/devel/memory.rst
> @@ -365,6 +365,24 @@ callbacks are called:
>  - .impl.unaligned specifies that the *implementation* supports unaligned
>    accesses; if false, unaligned accesses will be emulated by two aligned
>    accesses.
> +- Additionally, if .valid.unaligned = true but .impl.unaligned = false, the
> +  memory core will emulate each unaligned guest access by splitting it into
> +  multiple aligned sub-accesses. This ensures that devices which only handle
> +  aligned requests do not need to implement unaligned logic themselves. For
> +  example, see xhci_cap_ops in hw/usb/hcd-xhci.c: it sets  .valid.unaligned
> +  = true so guests can do unaligned reads on the xHCI Capability Registers,
> +  while keeping .impl.unaligned = false to rely on the core splitting logic.
> +  However, if a device’s registers have side effects on read or write, this
> +  extra splitting can introduce undesired behavior. Specifically, for devices
> +  whose registers trigger state changes on each read/write, splitting an access
> +  can lead to reading or writing bytes beyond the originally requested subrange
> +  thereby triggering repeated or otherwise unintended register side effects.
> +  In such cases, one should set .valid.unaligned = false to reject unaligned
> +  accesses entirely.
> +- Conversely, if .valid.unaligned = false but .impl.unaligned = true,
> +  that setting is considered invalid; it claims unaligned access is allowed
> +  by the implementation yet disallowed for the device. QEMU enforces this with
> +  an assertion to prevent contradictory usage.

Some cosmetic comments only..

This shouldn't be another bullet, but rather a separate sub-section,
because it describes the relationship of above two entries.

IMO it can be better like this:

MMIO Operations
---------------

...

- .valid.min_access_size, .valid.max_access_size...

- .valid.unaligned...  See :ref:`unaligned-mmio-accesses` for details.

- .impl.min_access_size, .impl.max_access_size...

- .impl.unaligned...  See :ref:`unaligned-mmio-accesses` for details.

.. _unaligned-mmio-accesses:

Unaligned MMIO Accesses
~~~~~~~~~~~~~~~~~~~~~~~

...

-- 
Peter Xu

[RFC PATCH v2 2/9] hw/riscv: iommu-trap: remove .impl.unaligned = true

[CJ Chen]

Since riscv-iommu does not support unaligned accesses, drop
`.impl.unaligned = true` to avoid the contradictory pairing with
`.valid.unaligned = false`.  This makes QEMU reject unaligned accesses
for this device and prevents the assertion in memory.c that previously
caused `make check` to fail.

Signed-off-by: CJ Chen <cjchen@igel.co.jp>
Tested-by: CJ Chen <cjchen@igel.co.jp>
Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
---
 hw/riscv/riscv-iommu.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index a877e5da84..277746598a 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -2288,7 +2288,6 @@ static const MemoryRegionOps riscv_iommu_trap_ops = {
     .impl = {
         .min_access_size = 4,
         .max_access_size = 8,
-        .unaligned = true,
     },
     .valid = {
         .min_access_size = 4,
-- 
2.25.1

Re: [RFC PATCH v2 2/9] hw/riscv: iommu-trap: remove .impl.unaligned = true

[Daniel Henrique Barboza]

On 8/22/25 6:24 AM, CJ Chen wrote:
> Since riscv-iommu does not support unaligned accesses, drop
> `.impl.unaligned = true` to avoid the contradictory pairing with
> `.valid.unaligned = false`.  This makes QEMU reject unaligned accesses
> for this device and prevents the assertion in memory.c that previously
> caused `make check` to fail.
> 
> Signed-off-by: CJ Chen <cjchen@igel.co.jp>
> Tested-by: CJ Chen <cjchen@igel.co.jp>
> Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   hw/riscv/riscv-iommu.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index a877e5da84..277746598a 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -2288,7 +2288,6 @@ static const MemoryRegionOps riscv_iommu_trap_ops = {
>       .impl = {
>           .min_access_size = 4,
>           .max_access_size = 8,
> -        .unaligned = true,
>       },
>       .valid = {
>           .min_access_size = 4,

[RFC PATCH v2 3/9] hw: npcm7xx_fiu and mx_pic change .impl.unaligned = true

[CJ Chen]

By setting .impl.unaligned = true, we allow QEMU to pass along
unaligned requests directly as-is, rather than splitting them into
multiple aligned sub-requests that might cause repeated device
callbacks or unintended side effects.

Signed-off-by: CJ Chen <cjchen@igel.co.jp>
Tested-by: CJ Chen <cjchen@igel.co.jp>
Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
---
 hw/ssi/npcm7xx_fiu.c | 3 +++
 hw/xtensa/mx_pic.c   | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/hw/ssi/npcm7xx_fiu.c b/hw/ssi/npcm7xx_fiu.c
index 056ce13394..10ee4deb31 100644
--- a/hw/ssi/npcm7xx_fiu.c
+++ b/hw/ssi/npcm7xx_fiu.c
@@ -255,6 +255,9 @@ static const MemoryRegionOps npcm7xx_fiu_flash_ops = {
         .max_access_size = 8,
         .unaligned = true,
     },
+    .impl = {
+        .unaligned = true,
+    },
 };
 
 /* Control register read handler. */
diff --git a/hw/xtensa/mx_pic.c b/hw/xtensa/mx_pic.c
index 8211c993eb..6bf524a918 100644
--- a/hw/xtensa/mx_pic.c
+++ b/hw/xtensa/mx_pic.c
@@ -270,6 +270,9 @@ static const MemoryRegionOps xtensa_mx_pic_ops = {
     .valid = {
         .unaligned = true,
     },
+    .impl = {
+        .unaligned = true,
+    },
 };
 
 MemoryRegion *xtensa_mx_pic_register_cpu(XtensaMxPic *mx,
-- 
2.25.1

Re: [RFC PATCH v2 3/9] hw: npcm7xx_fiu and mx_pic change .impl.unaligned = true

[Philippe Mathieu-Daudé]

Hi,

On 22/8/25 11:24, CJ Chen wrote:
> By setting .impl.unaligned = true, we allow QEMU to pass along
> unaligned requests directly as-is, rather than splitting them into
> multiple aligned sub-requests that might cause repeated device
> callbacks or unintended side effects.
> 
> Signed-off-by: CJ Chen <cjchen@igel.co.jp>
> Tested-by: CJ Chen <cjchen@igel.co.jp>
> Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> ---
>   hw/ssi/npcm7xx_fiu.c | 3 +++
>   hw/xtensa/mx_pic.c   | 3 +++
>   2 files changed, 6 insertions(+)


> diff --git a/hw/xtensa/mx_pic.c b/hw/xtensa/mx_pic.c
> index 8211c993eb..6bf524a918 100644
> --- a/hw/xtensa/mx_pic.c
> +++ b/hw/xtensa/mx_pic.c
> @@ -270,6 +270,9 @@ static const MemoryRegionOps xtensa_mx_pic_ops = {
>       .valid = {
>           .unaligned = true,
>       },
> +    .impl = {
> +        .unaligned = true,
> +    },
>   };

Surely a distinct patch.

Re: [RFC PATCH v2 3/9] hw: npcm7xx_fiu and mx_pic change .impl.unaligned = true

[Peter Xu]

On Mon, Aug 25, 2025 at 01:00:21PM +0200, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 22/8/25 11:24, CJ Chen wrote:
> > By setting .impl.unaligned = true, we allow QEMU to pass along
> > unaligned requests directly as-is, rather than splitting them into
> > multiple aligned sub-requests that might cause repeated device
> > callbacks or unintended side effects.
> > 
> > Signed-off-by: CJ Chen <cjchen@igel.co.jp>
> > Tested-by: CJ Chen <cjchen@igel.co.jp>
> > Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> > Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> > ---
> >   hw/ssi/npcm7xx_fiu.c | 3 +++
> >   hw/xtensa/mx_pic.c   | 3 +++
> >   2 files changed, 6 insertions(+)
> 
> 
> > diff --git a/hw/xtensa/mx_pic.c b/hw/xtensa/mx_pic.c
> > index 8211c993eb..6bf524a918 100644
> > --- a/hw/xtensa/mx_pic.c
> > +++ b/hw/xtensa/mx_pic.c
> > @@ -270,6 +270,9 @@ static const MemoryRegionOps xtensa_mx_pic_ops = {
> >       .valid = {
> >           .unaligned = true,
> >       },
> > +    .impl = {
> > +        .unaligned = true,
> > +    },
> >   };
> 
> Surely a distinct patch.

Besides that, I also don't understand how it used to work even before this
change..

E.g., both of the xtensa mx_pic read/write ignores size, that doesn't look
right already when there's totally no limitation of impl.*_access_size.

Meanwhile, taking xtensa_mx_pic_ext_reg_read() as example, it'll return a
u32 by offset smaller than MX_MAX_IRQ (where MIROUT==0):

    if (offset < MIROUT + MX_MAX_IRQ) {
        return mx->mirout[offset - MIROUT];
    }

But it returns different u32 for different offsets.. so reading 0x0 returns
the 1st u32, then 0x1 returns the 2nd (rather than reading 0x4 returns
that).

Even if there might be a driver that works with it, it still doesn't look
like the traditional way of doing MMIOs.. irrelevant of setting unaligned
or not in its .impl.

-- 
Peter Xu

[RFC PATCH v2 4/9] hw/nvme/ctrl: specify the 'valid' field in MemoryRegionOps

[CJ Chen]

From: Tomoyuki Hirose <hrstmyk811m@gmail.com>

'valid' field in MemoryRegionOps struct indicates how the MemoryRegion
can be accessed by the guest. In the previous code, the 'valid' field
was not specified explicitly. As a result, the CMB area could only be
accessed in units of 4 bytes.

This commit specifies the 'valid' field in MemoryRegionOps of CMB and
the CMB area can be accessed in units of 8 bytes.

Signed-off-by: CJ Chen <cjchen@igel.co.jp>
Based-on-a-patch-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
Tested-by: CJ Chen <cjchen@igel.co.jp>
Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
---
 hw/nvme/ctrl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index fd935507bc..9dca718ca1 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8272,6 +8272,11 @@ static const MemoryRegionOps nvme_cmb_ops = {
         .min_access_size = 1,
         .max_access_size = 8,
     },
+    .valid = {
+        .unaligned = true,
+        .min_access_size = 1,
+        .max_access_size = 8,
+    },
 };
 
 static bool nvme_check_params(NvmeCtrl *n, Error **errp)
-- 
2.25.1

[RFC PATCH v2 5/9] system/memory: support unaligned access

[CJ Chen]

From: Tomoyuki Hirose <hrstmyk811m@gmail.com>

The previous code ignored 'impl.unaligned' and handled unaligned
accesses as-is. But this implementation could not emulate specific
registers of some devices that allow unaligned access such as xHCI
Host Controller Capability Registers.

This commit emulates an unaligned access with multiple aligned
accesses. Additionally, the overwriting of the max access size is
removed to retrieve the actual max access size.

Based-on-a-patch-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
Signed-off-by: CJ Chen <cjchen@igel.co.jp>
Tested-by: CJ Chen <cjchen@igel.co.jp>
Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
---
 system/memory.c  | 147 ++++++++++++++++++++++++++++++++++++++---------
 system/physmem.c |   8 ---
 2 files changed, 119 insertions(+), 36 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 63b983efcd..d6071b4414 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -509,27 +509,118 @@ static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,
     return mr->ops->write_with_attrs(mr->opaque, addr, tmp, size, attrs);
 }
 
+typedef MemTxResult (*MemoryRegionAccessFn)(MemoryRegion *mr,
+                                            hwaddr addr,
+                                            uint64_t *value,
+                                            unsigned size,
+                                            signed shift,
+                                            uint64_t mask,
+                                            MemTxAttrs attrs);
+
+static MemTxResult access_emulation(hwaddr addr,
+                                    uint64_t *value,
+                                    unsigned int size,
+                                    unsigned int access_size_min,
+                                    unsigned int access_size_max,
+                                    MemoryRegion *mr,
+                                    MemTxAttrs attrs,
+                                    MemoryRegionAccessFn access_fn_read,
+                                    MemoryRegionAccessFn access_fn_write,
+                                    bool is_write)
+{
+    hwaddr a;
+    uint8_t *d;
+    uint64_t v;
+    MemTxResult r = MEMTX_OK;
+    bool is_big_endian = devend_big_endian(mr->ops->endianness);
+    void (*store)(void *, int, uint64_t) = is_big_endian ? stn_be_p : stn_le_p;
+    uint64_t (*load)(const void *, int) = is_big_endian ? ldn_be_p : ldn_le_p;
+    size_t access_size = MAX(MIN(size, access_size_max), access_size_min);
+    uint64_t access_mask = MAKE_64BIT_MASK(0, access_size * 8);
+    hwaddr round_down = mr->ops->impl.unaligned && addr + size <= mr->size ?
+        0 : addr % access_size;
+    hwaddr start = addr - round_down;
+    hwaddr tail = addr + size <= mr->size ? addr + size : mr->size;
+    uint8_t data[16] = {0};
+    g_assert(size <= 8);
+
+    for (a = start, d = data, v = 0; a < tail;
+         a += access_size, d += access_size, v = 0) {
+        r |= access_fn_read(mr, a, &v, access_size, 0, access_mask,
+                            attrs);
+        store(d, access_size, v);
+    }
+    if (is_write) {
+        stn_he_p(&data[round_down], size, load(value, size));
+        for (a = start, d = data; a < tail;
+             a += access_size, d += access_size) {
+            v = load(d, access_size);
+            r |= access_fn_write(mr, a, &v, access_size, 0, access_mask,
+                                 attrs);
+        }
+    } else {
+        store(value, size, ldn_he_p(&data[round_down], size));
+    }
+
+    return r;
+}
+
+static bool is_access_fastpath(hwaddr addr,
+                               unsigned int size,
+                               unsigned int access_size_min,
+                               unsigned int access_size_max,
+                               MemoryRegion *mr)
+{
+    size_t access_size = MAX(MIN(size, access_size_max), access_size_min);
+    hwaddr round_down = mr->ops->impl.unaligned && addr + size <= mr->size ?
+        0 : addr % access_size;
+
+    return round_down == 0 && access_size <= size;
+}
+
+static MemTxResult access_fastpath(hwaddr addr,
+                                   uint64_t *value,
+                                   unsigned int size,
+                                   unsigned int access_size_min,
+                                   unsigned int access_size_max,
+                                   MemoryRegion *mr,
+                                   MemTxAttrs attrs,
+                                   MemoryRegionAccessFn fastpath)
+{
+    MemTxResult r = MEMTX_OK;
+    size_t access_size = MAX(MIN(size, access_size_max), access_size_min);
+    uint64_t access_mask = MAKE_64BIT_MASK(0, access_size * 8);
+
+    if (devend_big_endian(mr->ops->endianness)) {
+        for (size_t i = 0; i < size; i += access_size) {
+            r |= fastpath(mr, addr + i, value, access_size,
+                          (size - access_size - i) * 8, access_mask, attrs);
+        }
+    } else {
+        for (size_t i = 0; i < size; i += access_size) {
+            r |= fastpath(mr, addr + i, value, access_size,
+                          i * 8, access_mask, attrs);
+        }
+    }
+
+    return r;
+}
+
 static MemTxResult access_with_adjusted_size(hwaddr addr,
                                       uint64_t *value,
                                       unsigned size,
                                       unsigned access_size_min,
                                       unsigned access_size_max,
-                                      MemTxResult (*access_fn)
-                                                  (MemoryRegion *mr,
-                                                   hwaddr addr,
-                                                   uint64_t *value,
-                                                   unsigned size,
-                                                   signed shift,
-                                                   uint64_t mask,
-                                                   MemTxAttrs attrs),
+                                      MemoryRegionAccessFn access_fn_read,
+                                      MemoryRegionAccessFn access_fn_write,
+                                      bool is_write,
                                       MemoryRegion *mr,
                                       MemTxAttrs attrs)
 {
-    uint64_t access_mask;
-    unsigned access_size;
-    unsigned i;
     MemTxResult r = MEMTX_OK;
     bool reentrancy_guard_applied = false;
+    MemoryRegionAccessFn access_fn_fastpath =
+        is_write ? access_fn_write : access_fn_read;
 
     if (!access_size_min) {
         access_size_min = 1;
@@ -551,20 +642,16 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
         reentrancy_guard_applied = true;
     }
 
-    /* FIXME: support unaligned access? */
-    access_size = MAX(MIN(size, access_size_max), access_size_min);
-    access_mask = MAKE_64BIT_MASK(0, access_size * 8);
-    if (devend_big_endian(mr->ops->endianness)) {
-        for (i = 0; i < size; i += access_size) {
-            r |= access_fn(mr, addr + i, value, access_size,
-                        (size - access_size - i) * 8, access_mask, attrs);
-        }
+    if (is_access_fastpath(addr, size, access_size_min, access_size_max, mr)) {
+        r |= access_fastpath(addr, value, size,
+                             access_size_min, access_size_max, mr, attrs,
+                             access_fn_fastpath);
     } else {
-        for (i = 0; i < size; i += access_size) {
-            r |= access_fn(mr, addr + i, value, access_size, i * 8,
-                        access_mask, attrs);
-        }
+        r |= access_emulation(addr, value, size,
+                              access_size_min, access_size_max, mr, attrs,
+                              access_fn_read, access_fn_write, is_write);
     }
+
     if (mr->dev && reentrancy_guard_applied) {
         mr->dev->mem_reentrancy_guard.engaged_in_io = false;
     }
@@ -1450,13 +1537,15 @@ static MemTxResult memory_region_dispatch_read1(MemoryRegion *mr,
                                          mr->ops->impl.min_access_size,
                                          mr->ops->impl.max_access_size,
                                          memory_region_read_accessor,
-                                         mr, attrs);
+                                         memory_region_write_accessor,
+                                         false, mr, attrs);
     } else {
         return access_with_adjusted_size(addr, pval, size,
                                          mr->ops->impl.min_access_size,
                                          mr->ops->impl.max_access_size,
                                          memory_region_read_with_attrs_accessor,
-                                         mr, attrs);
+                                         memory_region_write_with_attrs_accessor,
+                                         false, mr, attrs);
     }
 }
 
@@ -1544,15 +1633,17 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,
         return access_with_adjusted_size(addr, &data, size,
                                          mr->ops->impl.min_access_size,
                                          mr->ops->impl.max_access_size,
-                                         memory_region_write_accessor, mr,
-                                         attrs);
+                                         memory_region_read_accessor,
+                                         memory_region_write_accessor,
+                                         true, mr, attrs);
     } else {
         return
             access_with_adjusted_size(addr, &data, size,
                                       mr->ops->impl.min_access_size,
                                       mr->ops->impl.max_access_size,
+                                      memory_region_read_with_attrs_accessor,
                                       memory_region_write_with_attrs_accessor,
-                                      mr, attrs);
+                                      true, mr, attrs);
     }
 }
 
diff --git a/system/physmem.c b/system/physmem.c
index a8a9ca309e..9c5f3fbef1 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2864,14 +2864,6 @@ int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
         access_size_max = 4;
     }
 
-    /* Bound the maximum access by the alignment of the address.  */
-    if (!mr->ops->impl.unaligned) {
-        unsigned align_size_max = addr & -addr;
-        if (align_size_max != 0 && align_size_max < access_size_max) {
-            access_size_max = align_size_max;
-        }
-    }
-
     /* Don't attempt accesses larger than the maximum.  */
     if (l > access_size_max) {
         l = access_size_max;
-- 
2.25.1

Re: [RFC PATCH v2 5/9] system/memory: support unaligned access

[Peter Maydell]

On Fri, 22 Aug 2025 at 10:25, CJ Chen <cjchen@igel.co.jp> wrote:
>
> From: Tomoyuki Hirose <hrstmyk811m@gmail.com>
>
> The previous code ignored 'impl.unaligned' and handled unaligned
> accesses as-is. But this implementation could not emulate specific
> registers of some devices that allow unaligned access such as xHCI
> Host Controller Capability Registers.
>
> This commit emulates an unaligned access with multiple aligned
> accesses. Additionally, the overwriting of the max access size is
> removed to retrieve the actual max access size.
>
> Based-on-a-patch-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> Signed-off-by: CJ Chen <cjchen@igel.co.jp>
> Tested-by: CJ Chen <cjchen@igel.co.jp>
> Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> ---
>  system/memory.c  | 147 ++++++++++++++++++++++++++++++++++++++---------
>  system/physmem.c |   8 ---
>  2 files changed, 119 insertions(+), 36 deletions(-)
>
> diff --git a/system/memory.c b/system/memory.c
> index 63b983efcd..d6071b4414 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -509,27 +509,118 @@ static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,
>      return mr->ops->write_with_attrs(mr->opaque, addr, tmp, size, attrs);
>  }
>
> +typedef MemTxResult (*MemoryRegionAccessFn)(MemoryRegion *mr,
> +                                            hwaddr addr,
> +                                            uint64_t *value,
> +                                            unsigned size,
> +                                            signed shift,
> +                                            uint64_t mask,
> +                                            MemTxAttrs attrs);

So we now have access_emulation and access_fastpath and
the function is_access_fastpath() to select between them.
Can we have a comment please that says what the two are
doing and what the criterion is that lets us pick the fast path ?

> +
> +static MemTxResult access_emulation(hwaddr addr,
> +                                    uint64_t *value,
> +                                    unsigned int size,
> +                                    unsigned int access_size_min,
> +                                    unsigned int access_size_max,
> +                                    MemoryRegion *mr,
> +                                    MemTxAttrs attrs,
> +                                    MemoryRegionAccessFn access_fn_read,
> +                                    MemoryRegionAccessFn access_fn_write,
> +                                    bool is_write)
> +{
> +    hwaddr a;
> +    uint8_t *d;
> +    uint64_t v;
> +    MemTxResult r = MEMTX_OK;
> +    bool is_big_endian = devend_big_endian(mr->ops->endianness);
> +    void (*store)(void *, int, uint64_t) = is_big_endian ? stn_be_p : stn_le_p;
> +    uint64_t (*load)(const void *, int) = is_big_endian ? ldn_be_p : ldn_le_p;

Please use a typedef for all function pointers: it makes it
much easier to read.

> +    size_t access_size = MAX(MIN(size, access_size_max), access_size_min);
> +    uint64_t access_mask = MAKE_64BIT_MASK(0, access_size * 8);
> +    hwaddr round_down = mr->ops->impl.unaligned && addr + size <= mr->size ?
> +        0 : addr % access_size;
> +    hwaddr start = addr - round_down;
> +    hwaddr tail = addr + size <= mr->size ? addr + size : mr->size;
> +    uint8_t data[16] = {0};
> +    g_assert(size <= 8);

There should be a blank line after the last variable definition
and before the g_assert() here.

> +
> +    for (a = start, d = data, v = 0; a < tail;
> +         a += access_size, d += access_size, v = 0) {
> +        r |= access_fn_read(mr, a, &v, access_size, 0, access_mask,
> +                            attrs);
> +        store(d, access_size, v);
> +    }
> +    if (is_write) {
> +        stn_he_p(&data[round_down], size, load(value, size));
> +        for (a = start, d = data; a < tail;
> +             a += access_size, d += access_size) {
> +            v = load(d, access_size);
> +            r |= access_fn_write(mr, a, &v, access_size, 0, access_mask,
> +                                 attrs);
> +        }
> +    } else {
> +        store(value, size, ldn_he_p(&data[round_down], size));
> +    }

This would be much easier to review if there were comments
that said what the intention/design of the code was.

> +
> +    return r;
> +}
> +
> +static bool is_access_fastpath(hwaddr addr,
> +                               unsigned int size,
> +                               unsigned int access_size_min,
> +                               unsigned int access_size_max,
> +                               MemoryRegion *mr)
> +{
> +    size_t access_size = MAX(MIN(size, access_size_max), access_size_min);
> +    hwaddr round_down = mr->ops->impl.unaligned && addr + size <= mr->size ?
> +        0 : addr % access_size;
> +
> +    return round_down == 0 && access_size <= size;
> +}
> +
> +static MemTxResult access_fastpath(hwaddr addr,
> +                                   uint64_t *value,
> +                                   unsigned int size,
> +                                   unsigned int access_size_min,
> +                                   unsigned int access_size_max,
> +                                   MemoryRegion *mr,
> +                                   MemTxAttrs attrs,
> +                                   MemoryRegionAccessFn fastpath)
> +{
> +    MemTxResult r = MEMTX_OK;
> +    size_t access_size = MAX(MIN(size, access_size_max), access_size_min);
> +    uint64_t access_mask = MAKE_64BIT_MASK(0, access_size * 8);
> +
> +    if (devend_big_endian(mr->ops->endianness)) {
> +        for (size_t i = 0; i < size; i += access_size) {
> +            r |= fastpath(mr, addr + i, value, access_size,
> +                          (size - access_size - i) * 8, access_mask, attrs);
> +        }
> +    } else {
> +        for (size_t i = 0; i < size; i += access_size) {
> +            r |= fastpath(mr, addr + i, value, access_size,
> +                          i * 8, access_mask, attrs);
> +        }
> +    }
> +
> +    return r;
> +}
> +
>  static MemTxResult access_with_adjusted_size(hwaddr addr,
>                                        uint64_t *value,
>                                        unsigned size,
>                                        unsigned access_size_min,
>                                        unsigned access_size_max,
> -                                      MemTxResult (*access_fn)
> -                                                  (MemoryRegion *mr,
> -                                                   hwaddr addr,
> -                                                   uint64_t *value,
> -                                                   unsigned size,
> -                                                   signed shift,
> -                                                   uint64_t mask,
> -                                                   MemTxAttrs attrs),
> +                                      MemoryRegionAccessFn access_fn_read,
> +                                      MemoryRegionAccessFn access_fn_write,
> +                                      bool is_write,
>                                        MemoryRegion *mr,
>                                        MemTxAttrs attrs)
>  {
> -    uint64_t access_mask;
> -    unsigned access_size;
> -    unsigned i;
>      MemTxResult r = MEMTX_OK;
>      bool reentrancy_guard_applied = false;
> +    MemoryRegionAccessFn access_fn_fastpath =
> +        is_write ? access_fn_write : access_fn_read;
>
>      if (!access_size_min) {
>          access_size_min = 1;
> @@ -551,20 +642,16 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
>          reentrancy_guard_applied = true;
>      }
>
> -    /* FIXME: support unaligned access? */
> -    access_size = MAX(MIN(size, access_size_max), access_size_min);
> -    access_mask = MAKE_64BIT_MASK(0, access_size * 8);
> -    if (devend_big_endian(mr->ops->endianness)) {
> -        for (i = 0; i < size; i += access_size) {
> -            r |= access_fn(mr, addr + i, value, access_size,
> -                        (size - access_size - i) * 8, access_mask, attrs);
> -        }
> +    if (is_access_fastpath(addr, size, access_size_min, access_size_max, mr)) {
> +        r |= access_fastpath(addr, value, size,
> +                             access_size_min, access_size_max, mr, attrs,
> +                             access_fn_fastpath);
>      } else {
> -        for (i = 0; i < size; i += access_size) {
> -            r |= access_fn(mr, addr + i, value, access_size, i * 8,
> -                        access_mask, attrs);
> -        }
> +        r |= access_emulation(addr, value, size,
> +                              access_size_min, access_size_max, mr, attrs,
> +                              access_fn_read, access_fn_write, is_write);
>      }

Because you've removed the loops from this function, we don't
any longer need to set r to MEMTX_OK and then OR in the
return value from access_whatever; we can just set r = ...

> +
>      if (mr->dev && reentrancy_guard_applied) {
>          mr->dev->mem_reentrancy_guard.engaged_in_io = false;
>      }

thanks
-- PMM

[RFC PATCH v2 6/9] hw/usb/hcd-xhci: allow unaligned access to Capability Registers

[CJ Chen]

From: Tomoyuki Hirose <hrstmyk811m@gmail.com>

According to xHCI spec rev 1.2, unaligned access to xHCI Host
Controller Capability Registers is not prohibited. In addition, the
limit of access size is also unspecified. Actually, some real devices
allow unaligned access and 8-byte access to these registers.

This commit makes it possible to unaligned access and 8-byte access to
Host Controller Capability Registers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/143

Based-on-a-patch-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
Signed-off-by: CJ Chen <cjchen@igel.co.jp>
Tested-by: CJ Chen <cjchen@igel.co.jp>
Reported-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
---
 hw/usb/hcd-xhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 292c378bfc..81e91e6ffb 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3190,9 +3190,11 @@ static const MemoryRegionOps xhci_cap_ops = {
     .read = xhci_cap_read,
     .write = xhci_cap_write,
     .valid.min_access_size = 1,
-    .valid.max_access_size = 4,
+    .valid.max_access_size = 8,
+    .valid.unaligned = true,
     .impl.min_access_size = 4,
     .impl.max_access_size = 4,
+    .impl.unaligned = false,
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-- 
2.25.1

[RFC PATCH v2 7/9] system/memory: assert on invalid unaligned combo

[CJ Chen]

When it comes to this pattern: .valid.unaligned = false and
impl.unaligned = true, is effectlvely contradictory. The .valid
structure indicates that unaligned access should be rejected at
the access validation phase, yet .impl suggests the underlying
device implementation can handle unaligned operations. As a result,
the upper-layer code will never even reach the .impl logic, leading
to confusion.

Signed-off-by: CJ Chen <cjchen@igel.co.jp>
Tested-by: CJ Chen <cjchen@igel.co.jp>
Suggested-by: Peter Xu <peterx@redhat.com>
Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
---
 system/memory.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/system/memory.c b/system/memory.c
index d6071b4414..b536a62ce9 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1654,6 +1654,7 @@ void memory_region_init_io(MemoryRegion *mr,
                            const char *name,
                            uint64_t size)
 {
+    g_assert(!ops || !(ops->impl.unaligned && !ops->valid.unaligned));
     memory_region_init(mr, owner, name, size);
     mr->ops = ops ? ops : &unassigned_mem_ops;
     mr->opaque = opaque;
-- 
2.25.1

Re: [RFC PATCH v2 7/9] system/memory: assert on invalid unaligned combo

[Philippe Mathieu-Daudé]

On 22/8/25 11:24, CJ Chen wrote:
> When it comes to this pattern: .valid.unaligned = false and
> impl.unaligned = true, is effectlvely contradictory. The .valid
> structure indicates that unaligned access should be rejected at
> the access validation phase, yet .impl suggests the underlying
> device implementation can handle unaligned operations. As a result,
> the upper-layer code will never even reach the .impl logic, leading
> to confusion.
> 
> Signed-off-by: CJ Chen <cjchen@igel.co.jp>
> Tested-by: CJ Chen <cjchen@igel.co.jp>

It is normal for contributors to test their patches ;)

> Suggested-by: Peter Xu <peterx@redhat.com>
> Acked-by: Tomoyuki Hirose <hrstmyk811m@gmail.com>
> ---
>   system/memory.c | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[RFC PATCH v2 8/9] hw/misc: add test device for memory access

[CJ Chen]

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

Re: [RFC PATCH v2 8/9] hw/misc: add test device for memory access

[Peter Maydell]

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

Re: [RFC PATCH v2 8/9] hw/misc: add test device for memory access

[Peter Xu]

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

[PATCH RFC v2 9/9] tests/qtest: add test for memory region access

[CJ Chen]

From: Tomoyuki Hirose <hrstmyk811m@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 <cjchen@igel.co.jp>
Co-developed-by: CJ Chen <cjchen@igel.co.jp>
Reported-by: Tomoyuki Hirose <hrstmyk811m@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 <hrstmyk811m@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" },
+};
+
+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;
+}
+
+static void little_b_valid(QTestState *qts, uint64_t offset)
+{
+    qtest_writeb(qts, base + offset + 0, 0x00);
+    qtest_writeb(qts, base + offset + 1, 0x11);
+    qtest_writeb(qts, base + offset + 2, 0x22);
+    qtest_writeb(qts, base + offset + 3, 0x33);
+    qtest_writeb(qts, base + offset + 4, 0x44);
+    qtest_writeb(qts, base + offset + 5, 0x55);
+    qtest_writeb(qts, base + offset + 6, 0x66);
+    qtest_writeb(qts, base + offset + 7, 0x77);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 0), ==, 0x00);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 1), ==, 0x11);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 2), ==, 0x22);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 3), ==, 0x33);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 4), ==, 0x44);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 5), ==, 0x55);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 6), ==, 0x66);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 7), ==, 0x77);
+}
+
+static void little_b_invalid(QTestState *qts, uint64_t offset)
+{
+    qtest_writeb(qts, base + offset + 0, 0x00);
+    qtest_writeb(qts, base + offset + 1, 0x11);
+    qtest_writeb(qts, base + offset + 2, 0x22);
+    qtest_writeb(qts, base + offset + 3, 0x33);
+    qtest_writeb(qts, base + offset + 4, 0x44);
+    qtest_writeb(qts, base + offset + 5, 0x55);
+    qtest_writeb(qts, base + offset + 6, 0x66);
+    qtest_writeb(qts, base + offset + 7, 0x77);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 0), ==, 0x00);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 1), ==, 0x11);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 2), ==, 0x22);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 3), ==, 0x33);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 4), ==, 0x44);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 5), ==, 0x55);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 6), ==, 0x66);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 7), ==, 0x77);
+}
+
+static void little_w_valid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writew(qts, base + offset + 0, 0x1100);
+        qtest_writew(qts, base + offset + 1, 0x3322);
+        qtest_writew(qts, base + offset + 2, 0x5544);
+        qtest_writew(qts, base + offset + 3, 0x7766);
+        qtest_writew(qts, base + offset + 4, 0x9988);
+        qtest_writew(qts, base + offset + 5, 0xbbaa);
+        qtest_writew(qts, base + offset + 6, 0xddcc);
+        qtest_writew(qts, base + offset + 7, 0xffee);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 0), ==, 0x1133);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 1), ==, 0x3355);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 2), ==, 0x5577);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 3), ==, 0x7799);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 4), ==, 0x99bb);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 5), ==, 0xbbdd);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 6), ==, 0xddff);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 7), ==, 0xffee);
+    } else {
+        qtest_writew(qts, base + offset + 0, 0x1100);
+        qtest_writew(qts, base + offset + 1, 0x3322);
+        qtest_writew(qts, base + offset + 2, 0x5544);
+        qtest_writew(qts, base + offset + 3, 0x7766);
+        qtest_writew(qts, base + offset + 4, 0x9988);
+        qtest_writew(qts, base + offset + 5, 0xbbaa);
+        qtest_writew(qts, base + offset + 6, 0xddcc);
+        qtest_writew(qts, base + offset + 7, 0xffee);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 0), ==, 0x2200);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 1), ==, 0x4422);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 2), ==, 0x6644);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 3), ==, 0x8866);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 4), ==, 0xaa88);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 5), ==, 0xccaa);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 6), ==, 0xeecc);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 7), ==, 0xffee);
+    }
+}
+
+static void little_w_invalid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writew(qts, base + offset + 0, 0x1100);
+        qtest_writew(qts, base + offset + 2, 0x3322);
+        qtest_writew(qts, base + offset + 4, 0x5544);
+        qtest_writew(qts, base + offset + 6, 0x7766);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 0), ==, 0x1100);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 2), ==, 0x3322);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 4), ==, 0x5544);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 6), ==, 0x7766);
+    } else {
+        qtest_writew(qts, base + offset + 0, 0x1100);
+        qtest_writew(qts, base + offset + 2, 0x3322);
+        qtest_writew(qts, base + offset + 4, 0x5544);
+        qtest_writew(qts, base + offset + 6, 0x7766);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 0), ==, 0x1100);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 2), ==, 0x3322);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 4), ==, 0x5544);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 6), ==, 0x7766);
+    }
+}
+
+static void little_l_valid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writel(qts, base + offset + 0, 0x33221100);
+        qtest_writel(qts, base + offset + 1, 0x77665544);
+        qtest_writel(qts, base + offset + 2, 0xbbaa9988);
+        qtest_writel(qts, base + offset + 3, 0xffeeddcc);
+        qtest_writel(qts, base + offset + 4, 0x01234567);
+        qtest_writel(qts, base + offset + 5, 0x89abcdef);
+        qtest_writel(qts, base + offset + 6, 0xfedcba98);
+        qtest_writel(qts, base + offset + 7, 0x76543210);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 0), ==, 0x3377bbff);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 1), ==, 0x77bbff01);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 2), ==, 0xbbff0189);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 3), ==, 0xff0189fe);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 4), ==, 0x0189fe76);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 5), ==, 0x89fe7654);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 6), ==, 0xfe765432);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 7), ==, 0x76543210);
+    } else {
+        qtest_writel(qts, base + offset + 0, 0x33221100);
+        qtest_writel(qts, base + offset + 1, 0x77665544);
+        qtest_writel(qts, base + offset + 2, 0xbbaa9988);
+        qtest_writel(qts, base + offset + 3, 0xffeeddcc);
+        qtest_writel(qts, base + offset + 4, 0x01234567);
+        qtest_writel(qts, base + offset + 5, 0x89abcdef);
+        qtest_writel(qts, base + offset + 6, 0xfedcba98);
+        qtest_writel(qts, base + offset + 7, 0x76543210);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 0), ==, 0xcc884400);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 1), ==, 0x67cc8844);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 2), ==, 0xef67cc88);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 3), ==, 0x98ef67cc);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 4), ==, 0x1098ef67);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 5), ==, 0x321098ef);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 6), ==, 0x54321098);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 7), ==, 0x76543210);
+    }
+}
+
+static void little_l_invalid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writel(qts, base + offset + 0, 0x33221100);
+        qtest_writel(qts, base + offset + 4, 0x77665544);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 0), ==, 0x33221100);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 4), ==, 0x77665544);
+    } else {
+        qtest_writel(qts, base + offset + 0, 0x33221100);
+        qtest_writel(qts, base + offset + 4, 0x77665544);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 0), ==, 0x33221100);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 4), ==, 0x77665544);
+    }
+}
+
+static void little_q_valid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writeq(qts, base + offset + 0, 0x7766554433221100);
+        qtest_writeq(qts, base + offset + 1, 0xffeeddccbbaa9988);
+        qtest_writeq(qts, base + offset + 2, 0xfedcba9876543210);
+        qtest_writeq(qts, base + offset + 3, 0x0123456789abcdef);
+        qtest_writeq(qts, base + offset + 4, 0xdeadbeefdeadbeef);
+        qtest_writeq(qts, base + offset + 5, 0xcafebabecafebabe);
+        qtest_writeq(qts, base + offset + 6, 0xbeefcafebeefcafe);
+        qtest_writeq(qts, base + offset + 7, 0xfacefeedfacefeed);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 0), ==,
+                        0x77fffe01decabefa);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 1), ==,
+                        0xfffe01decabeface);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 2), ==,
+                        0xfe01decabefacefe);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 3), ==,
+                        0x01decabefacefeed);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 4), ==,
+                        0xdecabefacefeedfa);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 5), ==,
+                        0xcabefacefeedface);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 6), ==,
+                        0xbefacefeedfacefe);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 7), ==,
+                        0xfacefeedfacefeed);
+    } else {
+        qtest_writeq(qts, base + offset + 0, 0x7766554433221100);
+        qtest_writeq(qts, base + offset + 1, 0xffeeddccbbaa9988);
+        qtest_writeq(qts, base + offset + 2, 0xfedcba9876543210);
+        qtest_writeq(qts, base + offset + 3, 0x0123456789abcdef);
+        qtest_writeq(qts, base + offset + 4, 0xdeadbeefdeadbeef);
+        qtest_writeq(qts, base + offset + 5, 0xcafebabecafebabe);
+        qtest_writeq(qts, base + offset + 6, 0xbeefcafebeefcafe);
+        qtest_writeq(qts, base + offset + 7, 0xfacefeedfacefeed);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 0), ==,
+                        0xedfebeefef108800);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 1), ==,
+                        0xfeedfebeefef1088);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 2), ==,
+                        0xcefeedfebeefef10);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 3), ==,
+                        0xfacefeedfebeefef);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 4), ==,
+                        0xedfacefeedfebeef);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 5), ==,
+                        0xfeedfacefeedfebe);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 6), ==,
+                        0xcefeedfacefeedfe);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 7), ==,
+                        0xfacefeedfacefeed);
+    }
+}
+
+static void little_q_invalid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writeq(qts, base + offset + 0, 0x7766554433221100);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 0), ==,
+                        0x7766554433221100);
+    } else {
+        qtest_writeq(qts, base + offset + 0, 0x7766554433221100);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 0), ==,
+                        0x7766554433221100);
+    }
+}
+
+static void big_b_valid(QTestState *qts, uint64_t offset)
+{
+    qtest_writeb(qts, base + offset + 0, 0x00);
+    qtest_writeb(qts, base + offset + 1, 0x11);
+    qtest_writeb(qts, base + offset + 2, 0x22);
+    qtest_writeb(qts, base + offset + 3, 0x33);
+    qtest_writeb(qts, base + offset + 4, 0x44);
+    qtest_writeb(qts, base + offset + 5, 0x55);
+    qtest_writeb(qts, base + offset + 6, 0x66);
+    qtest_writeb(qts, base + offset + 7, 0x77);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 0), ==, 0x00);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 1), ==, 0x11);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 2), ==, 0x22);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 3), ==, 0x33);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 4), ==, 0x44);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 5), ==, 0x55);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 6), ==, 0x66);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 7), ==, 0x77);
+}
+
+static void big_b_invalid(QTestState *qts, uint64_t offset)
+{
+    qtest_writeb(qts, base + offset + 0, 0x00);
+    qtest_writeb(qts, base + offset + 1, 0x11);
+    qtest_writeb(qts, base + offset + 2, 0x22);
+    qtest_writeb(qts, base + offset + 3, 0x33);
+    qtest_writeb(qts, base + offset + 4, 0x44);
+    qtest_writeb(qts, base + offset + 5, 0x55);
+    qtest_writeb(qts, base + offset + 6, 0x66);
+    qtest_writeb(qts, base + offset + 7, 0x77);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 0), ==, 0x00);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 1), ==, 0x11);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 2), ==, 0x22);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 3), ==, 0x33);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 4), ==, 0x44);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 5), ==, 0x55);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 6), ==, 0x66);
+    g_assert_cmphex(qtest_readb(qts, base + offset + 7), ==, 0x77);
+}
+
+static void big_w_valid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writew(qts, base + offset + 0, 0x1100);
+        qtest_writew(qts, base + offset + 1, 0x3322);
+        qtest_writew(qts, base + offset + 2, 0x5544);
+        qtest_writew(qts, base + offset + 3, 0x7766);
+        qtest_writew(qts, base + offset + 4, 0x9988);
+        qtest_writew(qts, base + offset + 5, 0xbbaa);
+        qtest_writew(qts, base + offset + 6, 0xddcc);
+        qtest_writew(qts, base + offset + 7, 0xffee);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 0), ==, 0x1133);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 1), ==, 0x3355);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 2), ==, 0x5577);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 3), ==, 0x7799);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 4), ==, 0x99bb);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 5), ==, 0xbbdd);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 6), ==, 0xddff);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 7), ==, 0xffee);
+    } else {
+        qtest_writew(qts, base + offset + 0, 0x1100);
+        qtest_writew(qts, base + offset + 1, 0x3322);
+        qtest_writew(qts, base + offset + 2, 0x5544);
+        qtest_writew(qts, base + offset + 3, 0x7766);
+        qtest_writew(qts, base + offset + 4, 0x9988);
+        qtest_writew(qts, base + offset + 5, 0xbbaa);
+        qtest_writew(qts, base + offset + 6, 0xddcc);
+        qtest_writew(qts, base + offset + 7, 0xffee);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 0), ==, 0x2200);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 1), ==, 0x4422);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 2), ==, 0x6644);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 3), ==, 0x8866);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 4), ==, 0xaa88);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 5), ==, 0xccaa);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 6), ==, 0xeecc);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 7), ==, 0xffee);
+    }
+}
+
+static void big_w_invalid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writew(qts, base + offset + 0, 0x1100);
+        qtest_writew(qts, base + offset + 2, 0x3322);
+        qtest_writew(qts, base + offset + 4, 0x5544);
+        qtest_writew(qts, base + offset + 6, 0x7766);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 0), ==, 0x1100);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 2), ==, 0x3322);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 4), ==, 0x5544);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 6), ==, 0x7766);
+    } else {
+        qtest_writew(qts, base + offset + 0, 0x1100);
+        qtest_writew(qts, base + offset + 2, 0x3322);
+        qtest_writew(qts, base + offset + 4, 0x5544);
+        qtest_writew(qts, base + offset + 6, 0x7766);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 0), ==, 0x1100);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 2), ==, 0x3322);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 4), ==, 0x5544);
+        g_assert_cmphex(qtest_readw(qts, base + offset + 6), ==, 0x7766);
+    }
+}
+
+static void big_l_valid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writel(qts, base + offset + 0, 0x33221100);
+        qtest_writel(qts, base + offset + 1, 0x77665544);
+        qtest_writel(qts, base + offset + 2, 0xbbaa9988);
+        qtest_writel(qts, base + offset + 3, 0xffeeddcc);
+        qtest_writel(qts, base + offset + 4, 0x01234567);
+        qtest_writel(qts, base + offset + 5, 0x89abcdef);
+        qtest_writel(qts, base + offset + 6, 0xfedcba98);
+        qtest_writel(qts, base + offset + 7, 0x76543210);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 0), ==, 0x3377bbff);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 1), ==, 0x77bbff01);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 2), ==, 0xbbff0189);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 3), ==, 0xff0189fe);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 4), ==, 0x0189fe76);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 5), ==, 0x89fe7654);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 6), ==, 0xfe765432);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 7), ==, 0x76543210);
+    } else {
+        qtest_writel(qts, base + offset + 0, 0x33221100);
+        qtest_writel(qts, base + offset + 1, 0x77665544);
+        qtest_writel(qts, base + offset + 2, 0xbbaa9988);
+        qtest_writel(qts, base + offset + 3, 0xffeeddcc);
+        qtest_writel(qts, base + offset + 4, 0x01234567);
+        qtest_writel(qts, base + offset + 5, 0x89abcdef);
+        qtest_writel(qts, base + offset + 6, 0xfedcba98);
+        qtest_writel(qts, base + offset + 7, 0x76543210);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 0), ==, 0xcc884400);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 1), ==, 0x67cc8844);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 2), ==, 0xef67cc88);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 3), ==, 0x98ef67cc);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 4), ==, 0x1098ef67);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 5), ==, 0x321098ef);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 6), ==, 0x54321098);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 7), ==, 0x76543210);
+    }
+}
+
+static void big_l_invalid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writel(qts, base + offset + 0, 0x33221100);
+        qtest_writel(qts, base + offset + 4, 0x77665544);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 0), ==, 0x33221100);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 4), ==, 0x77665544);
+    } else {
+        qtest_writel(qts, base + offset + 0, 0x33221100);
+        qtest_writel(qts, base + offset + 4, 0x77665544);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 0), ==, 0x33221100);
+        g_assert_cmphex(qtest_readl(qts, base + offset + 4), ==, 0x77665544);
+    }
+}
+
+static void big_q_valid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writeq(qts, base + offset + 0, 0x7766554433221100);
+        qtest_writeq(qts, base + offset + 1, 0xffeeddccbbaa9988);
+        qtest_writeq(qts, base + offset + 2, 0xfedcba9876543210);
+        qtest_writeq(qts, base + offset + 3, 0x0123456789abcdef);
+        qtest_writeq(qts, base + offset + 4, 0xdeadbeefdeadbeef);
+        qtest_writeq(qts, base + offset + 5, 0xcafebabecafebabe);
+        qtest_writeq(qts, base + offset + 6, 0xbeefcafebeefcafe);
+        qtest_writeq(qts, base + offset + 7, 0xfacefeedfacefeed);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 0), ==,
+                        0x77fffe01decabefa);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 1), ==,
+                        0xfffe01decabeface);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 2), ==,
+                        0xfe01decabefacefe);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 3), ==,
+                        0x01decabefacefeed);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 4), ==,
+                        0xdecabefacefeedfa);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 5), ==,
+                        0xcabefacefeedface);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 6), ==,
+                        0xbefacefeedfacefe);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 7), ==,
+                        0xfacefeedfacefeed);
+    } else {
+        qtest_writeq(qts, base + offset + 0, 0x7766554433221100);
+        qtest_writeq(qts, base + offset + 1, 0xffeeddccbbaa9988);
+        qtest_writeq(qts, base + offset + 2, 0xfedcba9876543210);
+        qtest_writeq(qts, base + offset + 3, 0x0123456789abcdef);
+        qtest_writeq(qts, base + offset + 4, 0xdeadbeefdeadbeef);
+        qtest_writeq(qts, base + offset + 5, 0xcafebabecafebabe);
+        qtest_writeq(qts, base + offset + 6, 0xbeefcafebeefcafe);
+        qtest_writeq(qts, base + offset + 7, 0xfacefeedfacefeed);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 0), ==,
+                        0xedfebeefef108800);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 1), ==,
+                        0xfeedfebeefef1088);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 2), ==,
+                        0xcefeedfebeefef10);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 3), ==,
+                        0xfacefeedfebeefef);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 4), ==,
+                        0xedfacefeedfebeef);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 5), ==,
+                        0xfeedfacefeedfebe);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 6), ==,
+                        0xcefeedfacefeedfe);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 7), ==,
+                        0xfacefeedfacefeed);
+    }
+}
+
+static void big_q_invalid(QTestState *qts, hwaddr offset)
+{
+    if (qtest_big_endian(qts)) {
+        qtest_writeq(qts, base + offset + 0, 0x7766554433221100);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 0), ==,
+                        0x7766554433221100);
+    } else {
+        qtest_writeq(qts, base + offset + 0, 0x7766554433221100);
+        g_assert_cmphex(qtest_readq(qts, base + offset + 0), ==,
+                        0x7766554433221100);
+    }
+}
+
+#define DEFINE_test_memaccess(e, e_u, w, w_u, v, v_u)                   \
+    static void                                                         \
+    test_memaccess_##e##_##w##_##v(void)                                \
+    {                                                                   \
+        QTestState *qts;                                                \
+        qts = create_memaccess_qtest();                                 \
+        if (!qts) {                                                     \
+            return;                                                     \
+        }                                                               \
+                                                                        \
+        for (size_t i = OFF_IDX_OPS_LIST_##e_u##_##w_u##_##v_u;         \
+             i < OFF_IDX_OPS_LIST_##e_u##_##w_u##_##v_u +               \
+                 N_OPS_LIST_##e_u##_##w_u##_##v_u;                      \
+             i++) {                                                     \
+            e##_##w##_##v(qts, MEMACCESS_TESTDEV_REGION_SIZE * i);      \
+        }                                                               \
+                                                                        \
+        qtest_quit(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},
+};
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    arch = qtest_get_arch();
+
+    for (int i = 0; i < ARRAY_SIZE(tests); i++) {
+        g_autofree gchar *path = g_strdup_printf("memaccess/%s", tests[i].name);
+        qtest_add_func(path, tests[i].test);
+    }
+
+    return g_test_run();
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 669d07c06b..5d721b2c60 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -95,6 +95,7 @@ qtests_i386 = \
   (config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : []) +            \
   (config_all_devices.has_key('CONFIG_ESP_PCI') ? ['am53c974-test'] : []) +                 \
   (config_all_devices.has_key('CONFIG_VTD') ? ['intel-iommu-test'] : []) +                 \
+  (config_all_devices.has_key('CONFIG_MEMACCESS_TESTDEV') ? ['memaccess-test'] : []) +      \
   (host_os != 'windows' and                                                                \
    config_all_devices.has_key('CONFIG_ACPI_ERST') ? ['erst-test'] : []) +                   \
   (config_all_devices.has_key('CONFIG_PCIE_PORT') and                                       \
@@ -138,6 +139,7 @@ qtests_x86_64 = qtests_i386
 
 qtests_alpha = ['boot-serial-test'] + \
   qtests_filter + \
+  (config_all_devices.has_key('CONFIG_MEMACCESS_TESTDEV') ? ['memaccess-test'] : []) +       \
   (config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : [])
 
 qtests_avr = [ 'boot-serial-test' ]
@@ -162,6 +164,7 @@ qtests_microblazeel = qtests_microblaze
 
 qtests_mips = \
   qtests_filter + \
+  (config_all_devices.has_key('CONFIG_MEMACCESS_TESTDEV') ? ['memaccess-test'] : []) +       \
   (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : []) +            \
   (config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : [])
 
@@ -172,6 +175,7 @@ qtests_mips64el = qtests_mips
 qtests_ppc = \
   qtests_filter + \
   (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : []) +            \
+  (config_all_devices.has_key('CONFIG_MEMACCESS_TESTDEV') ? ['memaccess-test'] : []) +       \
   (config_all_accel.has_key('CONFIG_TCG') ? ['prom-env-test'] : []) +                              \
   (config_all_accel.has_key('CONFIG_TCG') ? ['boot-serial-test'] : []) +                           \
   ['boot-order-test']
@@ -198,6 +202,7 @@ qtests_sparc = ['prom-env-test', 'm48t59-test', 'boot-serial-test'] + \
 
 qtests_sparc64 = \
   (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : []) +            \
+  (config_all_devices.has_key('CONFIG_MEMACCESS_TESTDEV') ? ['memaccess-test'] : []) +       \
   qtests_filter + \
   ['prom-env-test', 'boot-serial-test']
 
@@ -248,6 +253,7 @@ qtests_arm = \
   (config_all_devices.has_key('CONFIG_FSI_APB2OPB_ASPEED') ? ['aspeed_fsi-test'] : []) + \
   (config_all_devices.has_key('CONFIG_STM32L4X5_SOC') and
    config_all_devices.has_key('CONFIG_DM163')? ['dm163-test'] : []) + \
+  (config_all_devices.has_key('CONFIG_MEMACCESS_TESTDEV') ? ['memaccess-test'] : []) + \
   ['arm-cpu-features',
    'boot-serial-test']
 
@@ -263,6 +269,7 @@ qtests_aarch64 = \
    config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
   (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
   (config_all_devices.has_key('CONFIG_NPCM8XX') ? qtests_npcm8xx : []) + \
+  (config_all_devices.has_key('CONFIG_MEMACCESS_TESTDEV') ? ['memaccess-test'] : []) + \
   qtests_cxl +                                                                                  \
   ['arm-cpu-features',
    'numa-test',
@@ -279,9 +286,11 @@ qtests_s390x = \
    'migration-test']
 
 qtests_riscv32 = \
+  (config_all_devices.has_key('CONFIG_MEMACCESS_TESTDEV') ? ['memaccess-test'] : []) + \
   (config_all_devices.has_key('CONFIG_SIFIVE_E_AON') ? ['sifive-e-aon-watchdog-test'] : [])
 
 qtests_riscv64 = ['riscv-csr-test'] + \
+  (config_all_devices.has_key('CONFIG_MEMACCESS_TESTDEV') ? ['memaccess-test'] : []) + \
   (unpack_edk2_blobs ? ['bios-tables-test'] : [])
 
 qos_test_ss = ss.source_set()
-- 
2.25.1

Re: [PATCH RFC v2 9/9] tests/qtest: add test for memory region access

[Philippe Mathieu-Daudé]

On 22/8/25 11:24, CJ Chen wrote:
> From: Tomoyuki Hirose <hrstmyk811m@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 <cjchen@igel.co.jp>
> Co-developed-by: CJ Chen <cjchen@igel.co.jp>
> Reported-by: Tomoyuki Hirose <hrstmyk811m@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 <hrstmyk811m@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-f4bug@amsat.org/ 
;)

Re: [PATCH RFC v2 9/9] tests/qtest: add test for memory region access

[chen CJ]

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é <philmd@linaro.org> 於 2025年8月25日 週一 下午8:16寫道：
>
> On 22/8/25 11:24, CJ Chen wrote:
> > From: Tomoyuki Hirose <hrstmyk811m@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 <cjchen@igel.co.jp>
> > Co-developed-by: CJ Chen <cjchen@igel.co.jp>
> > Reported-by: Tomoyuki Hirose <hrstmyk811m@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 <hrstmyk811m@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-f4bug@amsat.org/
> ;)

Re: [PATCH RFC v2 9/9] tests/qtest: add test for memory region access

[Peter Maydell]

On Fri, 22 Aug 2025 at 10:26, CJ Chen <cjchen@igel.co.jp> wrote:
>
> From: Tomoyuki Hirose <hrstmyk811m@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 <cjchen@igel.co.jp>
> Co-developed-by: CJ Chen <cjchen@igel.co.jp>
> Reported-by: Tomoyuki Hirose <hrstmyk811m@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

There seems to be a lot of duplication in these test functions
(for instance, aren't all of little_b_valid(), little_b_invalid(),
big_b_valid() and big_b_invalid() identical?  and the various
_invalid functions seem to have if() blocks where the code in
the if and the else halves is the same).

But also, I feel like we could improve what we're testing.
If I understand the memaccess-testdev correctly, it has
one underlying block of memory, and it exposes access to that
via various memory regions with the different possible
valid/impl/etc configurations. So I think the way to
test that our memory access handling code is correct would be:

 * for testing reads, we first fill the test device's memory
   with a known pattern, always using the "just permit byte
   accesses" memory region. Then the test of each of the
   "some particular config" MemoryRegions only does reads,
   and checks that reads from various offsets do what we
   expect
 * for testing writes, we first clear the test device's
   memory to a known pattern, and then the test of each
   "some config" MR only does writes. We check that the
   writes did what we expect by doing reads from the
   "just permit byte accesses" region.

If you only test e.g. word writes by doing word reads,
then it's possible to have bugs which cancel each other
out in the read and write paths, especially in the
"aligned access only" case.

thanks
-- PMM

Re: [PATCH RFC v2 9/9] tests/qtest: add test for memory region access

[Peter Xu]

On Mon, Sep 01, 2025 at 05:57:57PM +0100, Peter Maydell wrote:
> On Fri, 22 Aug 2025 at 10:26, CJ Chen <cjchen@igel.co.jp> wrote:
> >
> > From: Tomoyuki Hirose <hrstmyk811m@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 <cjchen@igel.co.jp>
> > Co-developed-by: CJ Chen <cjchen@igel.co.jp>
> > Reported-by: Tomoyuki Hirose <hrstmyk811m@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
> 
> There seems to be a lot of duplication in these test functions
> (for instance, aren't all of little_b_valid(), little_b_invalid(),
> big_b_valid() and big_b_invalid() identical?  and the various
> _invalid functions seem to have if() blocks where the code in
> the if and the else halves is the same).

Besides that, I don't yet understand some of the test code on endianess,
this might be relevant to the question I raised in the other reply.

Taking example of big_w_valid() test:

static void big_w_valid(QTestState *qts, hwaddr offset)
{
    if (qtest_big_endian(qts)) {
        qtest_writew(qts, base + offset + 0, 0x1100);                     <--- [1]
        qtest_writew(qts, base + offset + 1, 0x3322);                     <--- [2]
        qtest_writew(qts, base + offset + 2, 0x5544);
        qtest_writew(qts, base + offset + 3, 0x7766);
        qtest_writew(qts, base + offset + 4, 0x9988);
        qtest_writew(qts, base + offset + 5, 0xbbaa);
        qtest_writew(qts, base + offset + 6, 0xddcc);
        qtest_writew(qts, base + offset + 7, 0xffee);
        g_assert_cmphex(qtest_readw(qts, base + offset + 0), ==, 0x1133); <--- [3]
        g_assert_cmphex(qtest_readw(qts, base + offset + 1), ==, 0x3355);
        g_assert_cmphex(qtest_readw(qts, base + offset + 2), ==, 0x5577);
        g_assert_cmphex(qtest_readw(qts, base + offset + 3), ==, 0x7799);
        g_assert_cmphex(qtest_readw(qts, base + offset + 4), ==, 0x99bb);
        g_assert_cmphex(qtest_readw(qts, base + offset + 5), ==, 0xbbdd);
        g_assert_cmphex(qtest_readw(qts, base + offset + 6), ==, 0xddff);
        g_assert_cmphex(qtest_readw(qts, base + offset + 7), ==, 0xffee);
    } else {
        qtest_writew(qts, base + offset + 0, 0x1100);                     <--- [4]
        qtest_writew(qts, base + offset + 1, 0x3322);                     <--- [5]
        qtest_writew(qts, base + offset + 2, 0x5544);
        qtest_writew(qts, base + offset + 3, 0x7766);
        qtest_writew(qts, base + offset + 4, 0x9988);
        qtest_writew(qts, base + offset + 5, 0xbbaa);
        qtest_writew(qts, base + offset + 6, 0xddcc);
        qtest_writew(qts, base + offset + 7, 0xffee);
        g_assert_cmphex(qtest_readw(qts, base + offset + 0), ==, 0x2200); <--- [6]
        g_assert_cmphex(qtest_readw(qts, base + offset + 1), ==, 0x4422);
        g_assert_cmphex(qtest_readw(qts, base + offset + 2), ==, 0x6644);
        g_assert_cmphex(qtest_readw(qts, base + offset + 3), ==, 0x8866);
        g_assert_cmphex(qtest_readw(qts, base + offset + 4), ==, 0xaa88);
        g_assert_cmphex(qtest_readw(qts, base + offset + 5), ==, 0xccaa);
        g_assert_cmphex(qtest_readw(qts, base + offset + 6), ==, 0xeecc);
        g_assert_cmphex(qtest_readw(qts, base + offset + 7), ==, 0xffee);
    }
}

It tests on all MRs that are (1) device using big endianess, (2)
.valid.min_access_size=2, (3) .valid.unaligned=true.

First of all, I don't understand why a test case needs to behave
differently according to the TARGET endianess, aka, qtest_big_endian().
IIUC, each of the qtest_writew() should request a WRITE with an integer
value to be applied to the MMIO region, when we know the endianess of the
region (in this case, big endian), we know exactly how it will be read out.

Taking above steps [1-3] as example.  Here [1+2] will write two words to
offset 0x0, 0x1 correspondingly:

  - [1] WRITE(addr=0x0, size=2, data=0x1100)
  - [2] WRITE(addr=0x1, size=2, data=0x3322)

Here, IMHO the result should not depend on the internal property of the
systems (e.g. MR .impl values, after we have unaligned support memory core
should resolve all of these issues by either split 2B MMIO into two 1B, or
do proper padding on start/end to amplify the write if necessary).  Because
we know the device / MR is big endianess, so we should know the result of
the write already, as below:

  - After [1] WRITE(addr=0x0, size=2, data=0x1100), data should look like:

    [0x11, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, ...]
     ^^^^^^^^^^^
    Here it should always follow device's endianess.

  - After [2] WRITE(addr=0x1, size=2, data=0x3322), data should look like:

    [0x11, 0x33, 0x22, 0x00, 0x00, 0x00, 0x00, 0x00, ...]
           ^^^^^^^^^^^
    Here it should always follow device's endianess.

Above would be verified by step [3].  Basically it verifies READ(addr=0x0,
size=2) would result in 0x1133, which looks correct.

However the problem is, when GUEST is little endian, the test, even if
written the same data [4-5], expecting different results [6].  That's the
part I don't understand.  I think it would make sense if [6] should also
verify the same as [3].  IOW, the chunk in the "if" section looks like the
right thing to test for both big/little GUEST endianess.

-- 
Peter Xu

Re: [RFC PATCH v2 0/9] support unaligned access to xHCI Capability

[Peter Maydell]

On Fri, 22 Aug 2025 at 10:25, CJ Chen <cjchen@igel.co.jp> wrote:
>
> This patch set aims to support unaligned access to xHCI Capability
> Registers.
>
> To achieve this, we introduce the emulation of an unaligned access
> through multiple aligned accesses. This patch set also adds a test
> device and several tests using this device to verify that the
> emulation functions correctly.

I think it would be good if we can land this series early in
the 10.2 cycle so we have plenty of time to fix any
unexpected issues that might crop up.

I've left my review comments on various of the patches.

thanks
-- PMM

[Withdrawn] [RFC PATCH v2 0/9] support unaligned access to xHCI Capability

[chen CJ]

I would like to withdraw this patch series.

Sorry for the inconvenience, and thank you for your understanding.

CJ Chen <cjchen@igel.co.jp> 於 2025年8月22日 週五 下午6:25寫道：
>
> This patch set aims to support unaligned access to xHCI Capability
> Registers.
>
> To achieve this, we introduce the emulation of an unaligned access
> through multiple aligned accesses. This patch set also adds a test
> device and several tests using this device to verify that the
> emulation functions correctly.
>
> Using these changes, unaligned access to xHCI Capability Registers is
> now supported.
>
> During development, I required a lot of 'MemoryRegionOps' structs with
> its own read/write functions for tests. In the QEMU project, a large
> number of similar functions or structs are often written in '.inc'
> files. I followed this approach for the test functions but would
> appreciate feedback on whether this is appropriate.
>
> ---
> v1 ... 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.
>    - Modification to the memory document about the alignment issue.
>    - Update the npcm7xx_fiu, mx_pic and risc-v-iommu configuration
>      to align with the unaligned-access policy.
>    - Document memory.rst clarify that .valid=true,.impl=false causes
>      split unaligned accesses (may have side effects); forbid
>          .valid=false,.impl=true via assertion.
>
> ---
>  CJ Chen (4):
>   doc/devel/memory.rst: additional explanation for unaligned access
>   hw/riscv: iommu-trap: remove .impl.unaligned = true
>   hw: npcm7xx_fiu and mx_pic change .impl.unaligned = true
>   system/memory: assert on invalid unaligned combo
>
> Tomoyuki Hirose (5):
>   hw/nvme/ctrl: specify the 'valid' field in MemoryRegionOps
>   system/memory: support unaligned access
>   hw/usb/hcd-xhci: allow unaligned access to Capability Registers
>   hw/misc: add test device for memory access
>   tests/qtest: add test for memory region access
>
>  docs/devel/memory.rst               |  18 +
>  hw/misc/Kconfig                     |   4 +
>  hw/misc/memaccess-testdev.c         | 331 +++++++++++++++
>  hw/misc/meson.build                 |   1 +
>  hw/nvme/ctrl.c                      |   5 +
>  hw/riscv/riscv-iommu.c              |   1 -
>  hw/ssi/npcm7xx_fiu.c                |   3 +
>  hw/usb/hcd-xhci.c                   |   4 +-
>  hw/xtensa/mx_pic.c                  |   3 +
>  include/hw/misc/memaccess-testdev.h | 104 +++++
>  system/memory.c                     | 148 +++++--
>  system/physmem.c                    |   8 -
>  tests/qtest/memaccess-test.c        | 597 ++++++++++++++++++++++++++++
>  tests/qtest/meson.build             |   9 +
>  14 files changed, 1198 insertions(+), 38 deletions(-)
>  create mode 100644 hw/misc/memaccess-testdev.c
>  create mode 100644 include/hw/misc/memaccess-testdev.h
>  create mode 100644 tests/qtest/memaccess-test.c
>
> base-commit: 5836af0783213b9355a6bbf85d9e6bc4c9c9363f
> --
> 2.25.1

Re: [Withdrawn] [RFC PATCH v2 0/9] support unaligned access to xHCI Capability

[Peter Maydell]

On Wed, 3 Sept 2025 at 06:03, chen CJ <cjchen@igel.co.jp> wrote:
>
> I would like to withdraw this patch series.
>
> Sorry for the inconvenience, and thank you for your understanding.

That's unfortunate; I think it's an issue we really do need to fix,
but I entirely understand if you don't have the time to work
on it further.

I might pick it up if I have the time to do so.

thanks
-- PMM

Re: [Withdrawn] [RFC PATCH v2 0/9] support unaligned access to xHCI Capability

[Peter Xu]

On Wed, Sep 03, 2025 at 10:47:17AM +0100, Peter Maydell wrote:
> On Wed, 3 Sept 2025 at 06:03, chen CJ <cjchen@igel.co.jp> wrote:
> >
> > I would like to withdraw this patch series.
> >
> > Sorry for the inconvenience, and thank you for your understanding.
> 
> That's unfortunate; I think it's an issue we really do need to fix,
> but I entirely understand if you don't have the time to work
> on it further.
> 
> I might pick it up if I have the time to do so.

I worked on this problem a bit more in the past few days while almost
everyone will be at the forum.  It's almost because I saw similar issues
that I have commented before on old versions, but they still existed in the
core patch 5.  Then I found more issues.  Keep commenting on that might be
awkward because there will be quite a few dependency changes.  One example
is, I kept thinking we should not worry about MMIO out-of-bound over
mr->size when reaching as deep as access_with_adjusted_size().  There are
still quite a few places in patch 5 of this series that does the
calculation and it's not obvious what happens if mr->size violated.

Peter, if you want to pick it up, please consider reading the replies I
left in this series, alone with this version below as comparison reading
material.  The hope is the reworked patchset below _might_ be easier to
read (at least I did add rich comments, because the unaligned changes are
tricky and not easy to follow):

https://gitlab.com/peterx/qemu/-/commits/mem-unaligned-fix-v0.1?ref_type=tags

Especially this patch:

https://gitlab.com/peterx/qemu/-/commit/8a8f0f5728a7adc6ecb2cf4358366d2d663a5ed9

However that won't pass the test cases.  I still doubt the test case is
wrong but I didn't go further modifying the test cases yet (or any better
way to test this as you suggested in the other reply).  I think that can be
the 1st thing we figure out, not the best way to test, but the correctness
of the current test case, because IIUC it shouldn't be relevant to impl of
unaligned access.  To me, if we can reach a consensus on what is the
correct (test) behavior on all kinds of unaligned access emulations, fixing
the impl should be relatively easy.

Thanks,

-- 
Peter Xu