[RFC PATCH 0/9] hw/misc: Add support for interleaved memory accesses

[RFC PATCH 0/9] hw/misc: Add support for interleaved memory accesses

[Philippe Mathieu-Daudé]

This is a follow up on last year patch from Stephen Checkoway:
"block/pflash_cfi02: Implement intereleaved flash devices"
https://www.mail-archive.com/qemu-devel@nongnu.org/msg613077.html

Instead of making the PFLASH device more complex, I went
implementing a generic interleaver device (that can be re-used
for other MMIO devices, probably pointless, still useful for
simple testing).

The series is available on [1] and is organized as:

1/ Fix a pair of issues in memory.c as we are not exercing much
the memory_region_[read/write]_with_attrs_accessor functions (see
also [2]).

2/ Allow to use qtest helpers with the 'none' machine (without any
CPU).

3/ Add the interleaver device

4/ Add a device to test the interleaver device (later I'll expand
it to other MMIO accesses, such access_with_adjusted_size, endianess
and unaligned accesses). Let the 'none' machine create this device
from command line.

5/ Add qtests using 4/ to test 3/.

6/ Add a way to display the interleaved memory (sub)regions in
the HMP 'info mtree' command.

Based-on: <20200816173051.16274-1-f4bug@amsat.org>
[1] https://gitlab.com/philmd/qemu/-/tree/interleaved_device-v1
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg730279.html

Philippe Mathieu-Daudé (9):
  memory: Initialize MemoryRegionOps for RAM memory regions
  qtest: Add local qtest_mem_as() getter
  qtest: Directly use global address_space_memory when no CPU available
  hw/misc: Add interleaver device to make interleaved memory accesses
  hw/misc: Add MMIO test device
  hw/core/null-machine: Allow to use the MMIO 'test' device
  tests/qtest: Add generic MMIO tests
  memory: Allow memory region to display its subregions own descriptions
  hw/misc/interleaver: Display subregions in 'info mtree'

 include/exec/memory.h         |   6 +
 include/hw/misc/interleaver.h |  40 +++++
 include/hw/misc/testdev.h     |  15 ++
 hw/core/null-machine.c        |   2 +
 hw/misc/interleaver.c         | 283 ++++++++++++++++++++++++++++++++++
 hw/misc/mmio-testdev.c        | 150 ++++++++++++++++++
 softmmu/memory.c              |  32 ++++
 softmmu/qtest.c               |  40 +++--
 tests/qtest/mmio-test.c       | 146 ++++++++++++++++++
 MAINTAINERS                   |   9 ++
 hw/misc/Kconfig               |   9 ++
 hw/misc/Makefile.objs         |   2 +
 hw/misc/trace-events          |   6 +
 tests/qtest/Makefile.include  |   1 +
 14 files changed, 728 insertions(+), 13 deletions(-)
 create mode 100644 include/hw/misc/interleaver.h
 create mode 100644 include/hw/misc/testdev.h
 create mode 100644 hw/misc/interleaver.c
 create mode 100644 hw/misc/mmio-testdev.c
 create mode 100644 tests/qtest/mmio-test.c

-- 
2.26.2

[RFC PATCH 1/9] memory: Initialize MemoryRegionOps for RAM memory regions

[Philippe Mathieu-Daudé]

There is an issue when using memory_region_dispatch_read() or
memory_region_dispatch_write() on RAM memory regions.

RAM memory regions are initialized as:

  memory_region_init_ram()
  -> memory_region_init_ram_nomigrate()
     -> memory_region_init_ram_shared_nomigrate()
        -> memory_region_init()
           -> object_initialize(TYPE_MEMORY_REGION)
              -> memory_region_initfn()
                 -> mr->ops = &unassigned_mem_ops;

Later when accessing the alias, the memory_region_dispatch_read()
flow is:

  memory_region_dispatch_read()
  -> memory_region_dispatch_read1()
     -> if (mr->ops->read) { /* not taken */ } else ...
        -> access_with_adjusted_size
           -> memory_region_read_with_attrs_accessor
              -> memory_region_dispatch_read
                 -> unassigned_mem_read
              <- MEMTX_DECODE_ERROR

The caller gets a MEMTX_DECODE_ERROR while the access is OK.
(Similar flow with memory_region_dispatch_write).

Fix by initializing the MemoryRegionOps to ram_device_mem_ops,
this way the memory accesses are properly dispatched using
memory_region_ram_device_read() / memory_region_ram_device_write().

Fixes: 4a2e242bbb ("memory: Don't use memcpy for ram_device regions")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Since v1: Corrected description (PJP)
Cc: P J P <ppandit@redhat.com>
---
 softmmu/memory.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index 651705b7d1..8139da1a58 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1517,6 +1517,8 @@ void memory_region_init_ram_shared_nomigrate(MemoryRegion *mr,
     Error *err = NULL;
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
+    mr->ops = &ram_device_mem_ops;
+    mr->opaque = mr;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     mr->ram_block = qemu_ram_alloc(size, share, mr, &err);
@@ -1541,6 +1543,8 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
     Error *err = NULL;
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
+    mr->ops = &ram_device_mem_ops;
+    mr->opaque = mr;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     mr->ram_block = qemu_ram_alloc_resizeable(size, max_size, resized,
@@ -1566,6 +1570,8 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
     Error *err = NULL;
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
+    mr->ops = &ram_device_mem_ops;
+    mr->opaque = mr;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     mr->align = align;
@@ -1589,6 +1595,8 @@ void memory_region_init_ram_from_fd(MemoryRegion *mr,
     Error *err = NULL;
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
+    mr->ops = &ram_device_mem_ops;
+    mr->opaque = mr;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     mr->ram_block = qemu_ram_alloc_from_fd(size, mr,
@@ -1611,6 +1619,8 @@ void memory_region_init_ram_ptr(MemoryRegion *mr,
 {
     memory_region_init(mr, owner, name, size);
     mr->ram = true;
+    mr->ops = &ram_device_mem_ops;
+    mr->opaque = mr;
     mr->terminates = true;
     mr->destructor = memory_region_destructor_ram;
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
-- 
2.26.2

Re: [RFC PATCH 1/9] memory: Initialize MemoryRegionOps for RAM memory regions

[P J P]

[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]

+-- On Mon, 17 Aug 2020, Philippe Mathieu-Daudé wrote --+
| Fix by initializing the MemoryRegionOps to ram_device_mem_ops, this way the 
| memory accesses are properly dispatched using 
| memory_region_ram_device_read() / memory_region_ram_device_write().
| 
| Fixes: 4a2e242bbb ("memory: Don't use memcpy for ram_device regions")
| Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
| ---
| Since v1: Corrected description (PJP)
| Cc: P J P <ppandit@redhat.com>
| ---
|  softmmu/memory.c | 10 ++++++++++
|  1 file changed, 10 insertions(+)
| 
| diff --git a/softmmu/memory.c b/softmmu/memory.c
| index 651705b7d1..8139da1a58 100644
| --- a/softmmu/memory.c
| +++ b/softmmu/memory.c
| @@ -1517,6 +1517,8 @@ void memory_region_init_ram_shared_nomigrate(MemoryRegion *mr,
|      Error *err = NULL;
|      memory_region_init(mr, owner, name, size);
|      mr->ram = true;
| +    mr->ops = &ram_device_mem_ops;
| +    mr->opaque = mr;
|      mr->terminates = true;


I wonder if 'mr->ops = &ram_device_mem_ops' could be done in 
memory_region_init() instead?

Otherwise looks okay.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D

Re: [RFC PATCH 1/9] memory: Initialize MemoryRegionOps for RAM memory regions

[Philippe Mathieu-Daudé]

On 8/18/20 11:52 AM, P J P wrote:
> +-- On Mon, 17 Aug 2020, Philippe Mathieu-Daudé wrote --+
> | Fix by initializing the MemoryRegionOps to ram_device_mem_ops, this way the 
> | memory accesses are properly dispatched using 
> | memory_region_ram_device_read() / memory_region_ram_device_write().
> | 
> | Fixes: 4a2e242bbb ("memory: Don't use memcpy for ram_device regions")
> | Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> | ---
> | Since v1: Corrected description (PJP)
> | Cc: P J P <ppandit@redhat.com>
> | ---
> |  softmmu/memory.c | 10 ++++++++++
> |  1 file changed, 10 insertions(+)
> | 
> | diff --git a/softmmu/memory.c b/softmmu/memory.c
> | index 651705b7d1..8139da1a58 100644
> | --- a/softmmu/memory.c
> | +++ b/softmmu/memory.c
> | @@ -1517,6 +1517,8 @@ void memory_region_init_ram_shared_nomigrate(MemoryRegion *mr,
> |      Error *err = NULL;
> |      memory_region_init(mr, owner, name, size);
> |      mr->ram = true;
> | +    mr->ops = &ram_device_mem_ops;
> | +    mr->opaque = mr;
> |      mr->terminates = true;
> 
> 
> I wonder if 'mr->ops = &ram_device_mem_ops' could be done in 
> memory_region_init() instead?

I don't think so, because to use &ram_device_mem_ops handlers use
mr->opaque and mr->ram_block, so alias/io/iommu/rom types would
directly crash accessing these NULL fields.

> 
> Otherwise looks okay.

Thanks!

> 
> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
> 

[RFC PATCH 2/9] qtest: Add local qtest_mem_as() getter

[Philippe Mathieu-Daudé]

Refactor the access to the default address space introducing the
qtest_mem_as() getter. This will help us to use another address
space in the next commit.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/qtest.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 5672b75c35..81b5110783 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -273,6 +273,12 @@ static void qtest_irq_handler(void *opaque, int n, int level)
     }
 }
 
+/* Default address space for MMIO accesses */
+static AddressSpace *qtest_mem_as(void)
+{
+    return first_cpu->as;
+}
+
 static void qtest_process_command(CharBackend *chr, gchar **words)
 {
     const gchar *command;
@@ -434,22 +440,22 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
 
         if (words[0][5] == 'b') {
             uint8_t data = value;
-            address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+            address_space_write(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED,
                                 &data, 1);
         } else if (words[0][5] == 'w') {
             uint16_t data = value;
             tswap16s(&data);
-            address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+            address_space_write(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED,
                                 &data, 2);
         } else if (words[0][5] == 'l') {
             uint32_t data = value;
             tswap32s(&data);
-            address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+            address_space_write(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED,
                                 &data, 4);
         } else if (words[0][5] == 'q') {
             uint64_t data = value;
             tswap64s(&data);
-            address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+            address_space_write(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED,
                                 &data, 8);
         }
         qtest_send_prefix(chr);
@@ -468,21 +474,21 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
 
         if (words[0][4] == 'b') {
             uint8_t data;
-            address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+            address_space_read(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED,
                                &data, 1);
             value = data;
         } else if (words[0][4] == 'w') {
             uint16_t data;
-            address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+            address_space_read(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED,
                                &data, 2);
             value = tswap16(data);
         } else if (words[0][4] == 'l') {
             uint32_t data;
-            address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+            address_space_read(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED,
                                &data, 4);
             value = tswap32(data);
         } else if (words[0][4] == 'q') {
-            address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+            address_space_read(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED,
                                &value, 8);
             tswap64s(&value);
         }
@@ -503,7 +509,7 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         g_assert(len);
 
         data = g_malloc(len);
-        address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
+        address_space_read(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED, data,
                            len);
 
         enc = g_malloc(2 * len + 1);
@@ -529,7 +535,7 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         g_assert(ret == 0);
 
         data = g_malloc(len);
-        address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
+        address_space_read(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED, data,
                            len);
         b64_data = g_base64_encode(data, len);
         qtest_send_prefix(chr);
@@ -564,7 +570,7 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
                 data[i] = 0;
             }
         }
-        address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
+        address_space_write(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED, data,
                             len);
         g_free(data);
 
@@ -587,7 +593,7 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         if (len) {
             data = g_malloc(len);
             memset(data, pattern, len);
-            address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+            address_space_write(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED,
                                 data, len);
             g_free(data);
         }
@@ -621,7 +627,7 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
             out_len = MIN(out_len, len);
         }
 
-        address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
+        address_space_write(qtest_mem_as(), addr, MEMTXATTRS_UNSPECIFIED, data,
                             len);
 
         qtest_send_prefix(chr);
-- 
2.26.2

[RFC PATCH 3/9] qtest: Directly use global address_space_memory when no CPU available

[Philippe Mathieu-Daudé]

Currently all qtest MMIO access are dispatched via the first CPU
available. This limitation doesn't allow use to use the 'none'
machine to run tests. Unleash this limitation by selecting the
global address_space_memory when no CPU is available.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/qtest.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 81b5110783..7f018d2faf 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -19,6 +19,7 @@
 #include "chardev/char-fe.h"
 #include "exec/ioport.h"
 #include "exec/memory.h"
+#include "exec/address-spaces.h"
 #include "hw/irq.h"
 #include "sysemu/accel.h"
 #include "sysemu/cpus.h"
@@ -276,6 +277,13 @@ static void qtest_irq_handler(void *opaque, int n, int level)
 /* Default address space for MMIO accesses */
 static AddressSpace *qtest_mem_as(void)
 {
+    if (!first_cpu) {
+        /*
+         * The 'none' machine doesn't have any CPU,
+         * directly access the main address space.
+         */
+        return &address_space_memory;
+    }
     return first_cpu->as;
 }
 
-- 
2.26.2

[RFC PATCH 4/9] hw/misc: Add interleaver device to make interleaved memory accesses

[Philippe Mathieu-Daudé]

Some slow devices might be arranged in an interleaved setup to reduce
waiting for memory banks and improve memory throughput.  Classical
examples are NOR flashes.

Add an 'interleaver' device to allow making such interleaved memory
accesses. This device support using the 16x8, 32x8, 32x16, 64x8,
64x16 and 64x32 configurations.

Example of 32x16 interleaver accesses (32-bit bus, 2x 16-bit banks):

Each interleaved 32-bit access on the bus results in contiguous 16-bit
access on each banked device:

                   ____________________________________________________
 Bus accesses      |        1st 32-bit       |        2nd 32-bit       |
                   -----------------------------------------------------
                          |            |            |            |
                          v            |            v            |
                   ______________      |     ______________      |
 1st bank accesses | 1st 16-bit |      |     | 2nd 16-bit |      |
                   --------------      |     --------------      |
                                       v                         v
                                ______________            ______________
 2nd bank accesses              | 1st 16-bit |            | 2nd 16-bit |
                                --------------            --------------

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
How to simplify idx/addr initialization in the read/write handlers?
---
 include/hw/misc/interleaver.h |  40 ++++++
 hw/misc/interleaver.c         | 254 ++++++++++++++++++++++++++++++++++
 MAINTAINERS                   |   6 +
 hw/misc/Kconfig               |   4 +
 hw/misc/Makefile.objs         |   1 +
 hw/misc/trace-events          |   6 +
 6 files changed, 311 insertions(+)
 create mode 100644 include/hw/misc/interleaver.h
 create mode 100644 hw/misc/interleaver.c

diff --git a/include/hw/misc/interleaver.h b/include/hw/misc/interleaver.h
new file mode 100644
index 0000000000..953d987556
--- /dev/null
+++ b/include/hw/misc/interleaver.h
@@ -0,0 +1,40 @@
+/*
+ * QEMU Interleaver device
+ *
+ * Copyright (C) 2020 Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_MISC_INTERLEAVER_H
+#define HW_MISC_INTERLEAVER_H
+
+/*
+ * Example of 32x16 interleaver accesses (32-bit bus, 2x 16-bit banks):
+ *
+ * Each interleaved 32-bit access on the bus results in contiguous 16-bit
+ * access on each banked device:
+ *
+ *                      ____________________________________________________
+ *   Bus accesses       |        1st 32-bit       |        2nd 32-bit       |
+ *                      -----------------------------------------------------
+ *                             |            |            |            |
+ *                             v            |            v            |
+ *                      ______________      |     ______________      |
+ *   1st bank accesses  | 1st 16-bit |      |     | 2nd 16-bit |      |
+ *                      --------------      |     --------------      |
+ *                                          v                         v
+ *                                   ______________            ______________
+ *   2nd bank accesses               | 1st 16-bit |            | 2nd 16-bit |
+ *                                   --------------            --------------
+ */
+
+#define TYPE_INTERLEAVER_16X8_DEVICE    "interleaver-16x8-device"
+#define TYPE_INTERLEAVER_32X8_DEVICE    "interleaver-32x8-device"
+#define TYPE_INTERLEAVER_32X16_DEVICE   "interleaver-32x16-device"
+#define TYPE_INTERLEAVER_64X8_DEVICE    "interleaver-64x8-device"
+#define TYPE_INTERLEAVER_64X16_DEVICE   "interleaver-64x16-device"
+#define TYPE_INTERLEAVER_64X32_DEVICE   "interleaver-64x32-device"
+
+#endif
+
diff --git a/hw/misc/interleaver.c b/hw/misc/interleaver.c
new file mode 100644
index 0000000000..46099e9e11
--- /dev/null
+++ b/hw/misc/interleaver.c
@@ -0,0 +1,254 @@
+/*
+ * QEMU Interleaver device
+ *
+ * The interleaver device to allow making interleaved memory accesses.
+ *
+ * This device support using the following configurations (INPUT x OUTPUT):
+ * 16x8, 32x8, 32x16, 64x8, 64x16 and 64x32.
+ *
+ * Copyright (C) 2020 Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "hw/qdev-properties.h"
+#include "hw/misc/interleaver.h"
+#include "trace.h"
+
+#define TYPE_INTERLEAVER_DEVICE "interleaver-device"
+
+typedef struct InterleaverDeviceClass {
+    /*< private >*/
+    SysBusDeviceClass parent_class;
+    /*< public >*/
+    MemoryRegionOps ops;
+    unsigned input_access_size;
+    unsigned output_access_size;
+    MemOp output_memop;
+    unsigned mr_count;
+    char *name;
+} InterleaverDeviceClass;
+
+#define INTERLEAVER_DEVICE_CLASS(klass) \
+    OBJECT_CLASS_CHECK(InterleaverDeviceClass, (klass), TYPE_INTERLEAVER_DEVICE)
+#define INTERLEAVER_DEVICE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(InterleaverDeviceClass, (obj), TYPE_INTERLEAVER_DEVICE)
+
+#define INTERLEAVER_REGIONS_MAX 8 /* 64x8 */
+
+typedef struct {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+    MemoryRegion iomem;
+    uint64_t size;
+    MemoryRegion *mr[INTERLEAVER_REGIONS_MAX];
+} InterleaverDeviceState;
+
+#define INTERLEAVER_DEVICE(obj) \
+    OBJECT_CHECK(InterleaverDeviceState, (obj), TYPE_INTERLEAVER_DEVICE)
+
+static const char *memresult_str[] = {"OK", "ERROR", "DECODE_ERROR"};
+
+static const char *emtpy_mr_name = "EMPTY";
+
+static MemTxResult interleaver_read(void *opaque,
+                                    hwaddr offset, uint64_t *data,
+                                    unsigned size, MemTxAttrs attrs)
+{
+    InterleaverDeviceState *s = INTERLEAVER_DEVICE(opaque);
+    InterleaverDeviceClass *idc = INTERLEAVER_DEVICE_GET_CLASS(s);
+    unsigned idx = (offset / idc->output_access_size) & (idc->mr_count - 1);
+    hwaddr addr = (offset & ~(idc->input_access_size - 1)) / idc->mr_count;
+    MemTxResult r = MEMTX_ERROR;
+
+    trace_interleaver_read_enter(idc->input_access_size,
+                                 idc->output_access_size, size,
+                                 idc->mr_count, idx,
+                                 s->mr[idx] ? memory_region_name(s->mr[idx])
+                                            : emtpy_mr_name,
+                                 offset, addr);
+    if (s->mr[idx]) {
+        r = memory_region_dispatch_read(s->mr[idx],
+                                        addr,
+                                        data,
+                                        idc->output_memop,
+                                        attrs);
+    }
+    trace_interleaver_read_exit(size, *data, memresult_str[r]);
+
+    return r;
+}
+
+static MemTxResult interleaver_write(void *opaque,
+                                     hwaddr offset, uint64_t data,
+                                     unsigned size, MemTxAttrs attrs)
+{
+    InterleaverDeviceState *s = INTERLEAVER_DEVICE(opaque);
+    InterleaverDeviceClass *idc = INTERLEAVER_DEVICE_GET_CLASS(s);
+    unsigned idx = (offset / idc->output_access_size) & (idc->mr_count - 1);
+    hwaddr addr = (offset & ~(idc->input_access_size - 1)) / idc->mr_count;
+    MemTxResult r = MEMTX_ERROR;
+
+    trace_interleaver_write_enter(idc->input_access_size,
+                                  idc->output_access_size, size,
+                                  idc->mr_count, idx,
+                                  s->mr[idx] ? memory_region_name(s->mr[idx])
+                                             : emtpy_mr_name,
+                                  offset, addr);
+    if (s->mr[idx]) {
+        r = memory_region_dispatch_write(s->mr[idx],
+                                         addr,
+                                         data,
+                                         idc->output_memop,
+                                         attrs);
+    }
+    trace_interleaver_write_exit(size, data, memresult_str[r]);
+
+    return r;
+}
+
+static void interleaver_realize(DeviceState *dev, Error **errp)
+{
+    InterleaverDeviceState *s = INTERLEAVER_DEVICE(dev);
+    InterleaverDeviceClass *idc = INTERLEAVER_DEVICE_GET_CLASS(dev);
+    uint64_t expected_mr_size;
+
+    if (s->size == 0) {
+        error_setg(errp, "property 'size' not specified or zero");
+        return;
+    }
+    if (!QEMU_IS_ALIGNED(s->size, idc->input_access_size)) {
+        error_setg(errp, "property 'size' must be multiple of %u",
+                   idc->input_access_size);
+        return;
+    }
+
+    expected_mr_size = s->size / idc->mr_count;
+    for (unsigned i = 0; i < idc->mr_count; i++) {
+        if (s->mr[i] && memory_region_size(s->mr[i]) != expected_mr_size) {
+            error_setg(errp,
+                       "memory region #%u (%s) size mismatches interleaver",
+                       i, memory_region_name(s->mr[i]));
+            return;
+        }
+    }
+    memory_region_init_io(&s->iomem, OBJECT(s), &idc->ops, s,
+                          idc->name, s->size);
+    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
+}
+
+static Property interleaver_properties[] = {
+    DEFINE_PROP_UINT64("size", InterleaverDeviceState, size, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void interleaver_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = interleaver_realize;
+    device_class_set_props(dc, interleaver_properties);
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static void interleaver_class_add_properties(ObjectClass *oc,
+                                             unsigned input_bits,
+                                             unsigned output_bits)
+{
+    InterleaverDeviceClass *idc = INTERLEAVER_DEVICE_CLASS(oc);
+
+    idc->name = g_strdup_printf("interleaver-%ux%u", input_bits, output_bits);
+    idc->input_access_size = input_bits >> 3;
+    idc->output_access_size = output_bits >> 3;
+    idc->output_memop = size_memop(idc->output_access_size);
+    idc->mr_count = input_bits / output_bits;
+    idc->ops = (MemoryRegionOps){
+        .read_with_attrs = interleaver_read,
+        .write_with_attrs = interleaver_write,
+        .valid.min_access_size = 1,
+        .valid.max_access_size = idc->input_access_size,
+        .impl.min_access_size = idc->output_access_size,
+        .impl.max_access_size = idc->output_access_size,
+        .endianness = DEVICE_NATIVE_ENDIAN,
+    };
+
+    for (unsigned i = 0; i < idc->mr_count; i++) {
+        g_autofree char *name = g_strdup_printf("mr%u", i);
+        object_class_property_add_link(oc, name, TYPE_MEMORY_REGION,
+                                       offsetof(InterleaverDeviceState, mr[i]),
+                                       qdev_prop_allow_set_link_before_realize,
+                                       OBJ_PROP_LINK_STRONG);
+    }
+}
+
+static void interleaver_16x8_class_init(ObjectClass *oc, void *data)
+{
+    interleaver_class_add_properties(oc, 16, 8);
+};
+
+static void interleaver_32x8_class_init(ObjectClass *oc, void *data)
+{
+    interleaver_class_add_properties(oc, 32, 8);
+};
+
+static void interleaver_32x16_class_init(ObjectClass *oc, void *data)
+{
+    interleaver_class_add_properties(oc, 32, 16);
+};
+
+static void interleaver_64x8_class_init(ObjectClass *oc, void *data)
+{
+    interleaver_class_add_properties(oc, 64, 8);
+};
+
+static void interleaver_64x16_class_init(ObjectClass *oc, void *data)
+{
+    interleaver_class_add_properties(oc, 64, 16);
+};
+
+static void interleaver_64x32_class_init(ObjectClass *oc, void *data)
+{
+    interleaver_class_add_properties(oc, 64, 32);
+};
+
+static const TypeInfo interleaver_device_types[] = {
+    {
+        .name           = TYPE_INTERLEAVER_16X8_DEVICE,
+        .parent         = TYPE_INTERLEAVER_DEVICE,
+        .class_init     = interleaver_16x8_class_init,
+    }, {
+        .name           = TYPE_INTERLEAVER_32X8_DEVICE,
+        .parent         = TYPE_INTERLEAVER_DEVICE,
+        .class_init     = interleaver_32x8_class_init,
+    }, {
+        .name           = TYPE_INTERLEAVER_32X16_DEVICE,
+        .parent         = TYPE_INTERLEAVER_DEVICE,
+        .class_init     = interleaver_32x16_class_init,
+    }, {
+        .name           = TYPE_INTERLEAVER_64X8_DEVICE,
+        .parent         = TYPE_INTERLEAVER_DEVICE,
+        .class_init     = interleaver_64x8_class_init,
+    }, {
+        .name           = TYPE_INTERLEAVER_64X16_DEVICE,
+        .parent         = TYPE_INTERLEAVER_DEVICE,
+        .class_init     = interleaver_64x16_class_init,
+    }, {
+        .name           = TYPE_INTERLEAVER_64X32_DEVICE,
+        .parent         = TYPE_INTERLEAVER_DEVICE,
+        .class_init     = interleaver_64x32_class_init,
+    }, {
+        .name           = TYPE_INTERLEAVER_DEVICE,
+        .parent         = TYPE_SYS_BUS_DEVICE,
+        .instance_size  = sizeof(InterleaverDeviceState),
+        .class_size     = sizeof(InterleaverDeviceClass),
+        .class_init     = interleaver_class_init,
+        .abstract       = true,
+    }
+};
+
+DEFINE_TYPES(interleaver_device_types)
diff --git a/MAINTAINERS b/MAINTAINERS
index 0886eb3d2b..1efce3dd27 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1961,6 +1961,12 @@ S: Maintained
 F: include/hw/misc/empty_slot.h
 F: hw/misc/empty_slot.c
 
+Interleaver device
+M: Philippe Mathieu-Daudé <f4bug@amsat.org>
+S: Maintained
+F: include/hw/misc/interleaver.h
+F: hw/misc/interleaver.c
+
 Standard VGA
 M: Gerd Hoffmann <kraxel@redhat.com>
 S: Maintained
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 92c397ca07..7ed0f4ccc7 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -21,6 +21,10 @@ config SGA
     bool
     depends on ISA_BUS
 
+config INTERLEAVER
+    bool
+    default y
+
 config ISA_TESTDEV
     bool
     default y if TEST_DEVICES
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 6be3d255ab..aa753a847f 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -12,6 +12,7 @@ common-obj-$(CONFIG_PCA9552) += pca9552.o
 common-obj-$(CONFIG_UNIMP) += unimp.o
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 common-obj-$(CONFIG_FW_CFG_DMA) += vmcoreinfo.o
+common-obj-$(CONFIG_INTERLEAVER) += interleaver.o
 
 # ARM devices
 common-obj-$(CONFIG_PL310) += arm_l2x0.o
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 066752aa90..1b0db146b4 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -217,3 +217,9 @@ grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read addr:0x%03"PRIx6
 # pca9552.c
 pca955x_gpio_status(const char *description, const char *buf) "%s GPIOs 0-15 [%s]"
 pca955x_gpio_change(const char *description, unsigned id, unsigned prev_state, unsigned current_state) "%s GPIO id:%u status: %u -> %u"
+
+# interleaver.c
+interleaver_read_enter(unsigned input_access_size, unsigned output_access_size, unsigned size, unsigned mr_count, unsigned index, const char *mr_name, uint64_t in_addr, uint64_t out_addr) "rd ixs:%u oxs:%u sz:%u mr_cnt:%u mr_idx:%u mr_name:'%s' iadr:0x%"PRIx64" oadr:0x%"PRIx64
+interleaver_read_exit(unsigned size, uint64_t value, const char *result) "rd size:%u value:0x%08"PRIx64" result: %s"
+interleaver_write_enter(unsigned input_access_size, unsigned output_access_size, unsigned size, unsigned mr_count, unsigned index, const char *mr_name, uint64_t in_addr, uint64_t out_addr) "wr ixs:%u oxs:%u sz:%u mr_cnt:%u mr_idx:%u mr_name:'%s' iadr:0x%"PRIx64" oadr:0x%"PRIx64
+interleaver_write_exit(unsigned size, uint64_t value, const char *result) "wr size:%u value:0x%08"PRIx64" result: %s"
-- 
2.26.2

[RFC PATCH 5/9] hw/misc: Add MMIO test device

[Philippe Mathieu-Daudé]

Add a MMIO test device handy to test QEMU internal devices via MMIO
accesses.

This device is meant to be run by the 'none' machine, thus no CPU
is required.

So far it is only useful to test the interleaver device.
A SRAM region is split into 256B subregions, and these subregions
are mapped at different addresses in an interleaved setup.
All the following (INPUT x OUTPUT) configurations can be tested:
16x8, 32x8, 32x16, 64x8, 64x16 and 64x32.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/misc/testdev.h |  15 ++++
 hw/misc/mmio-testdev.c    | 146 ++++++++++++++++++++++++++++++++++++++
 MAINTAINERS               |   2 +
 hw/misc/Kconfig           |   5 ++
 hw/misc/Makefile.objs     |   1 +
 5 files changed, 169 insertions(+)
 create mode 100644 include/hw/misc/testdev.h
 create mode 100644 hw/misc/mmio-testdev.c

diff --git a/include/hw/misc/testdev.h b/include/hw/misc/testdev.h
new file mode 100644
index 0000000000..2ff47d2766
--- /dev/null
+++ b/include/hw/misc/testdev.h
@@ -0,0 +1,15 @@
+/*
+ * QEMU MMIO test device
+ *
+ * Copyright (C) 2020 Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_MISC_TESTDEV_H
+#define HW_MISC_TESTDEV_H
+
+#define TYPE_MMIO_TESTDEV "mmio-testdev"
+
+#endif
+
diff --git a/hw/misc/mmio-testdev.c b/hw/misc/mmio-testdev.c
new file mode 100644
index 0000000000..3b7a8057b2
--- /dev/null
+++ b/hw/misc/mmio-testdev.c
@@ -0,0 +1,146 @@
+/*
+ * QEMU MMIO test device
+ *
+ * Copyright (C) 2020 Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+/*
+ * This device is mostly used to test QEMU internal MMIO devices.
+ * Accesses using CPU core are not allowed.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "exec/address-spaces.h"
+#include "hw/misc/testdev.h"
+#include "hw/misc/interleaver.h"
+
+/*
+ * Device Memory Map:
+ *
+ *   offset       size         description
+ * ----------  ----------  --------------------
+ * 0x00000000  [   2 KiB]  SRAM (8 banks of 256B)
+ * 0x10000000  [ 128 MiB]  interleaved-container
+ * 0x11608000  [   4 KiB]  interleaved-16x8  (each device interleaves the sram)
+ * 0x13208000  [   8 KiB]  interleaved-32x8   "
+ * 0x13216000  [   4 KiB]  interleaved-32x16  "
+ * 0x16408000  [  16 KiB]  interleaved-64x8   "
+ * 0x16416000  [   8 KiB]  interleaved-64x16  "
+ * 0x16432000  [   4 KiB]  interleaved-64x32  "
+ * 0x20000000  [ 256 MiB]  container
+ *
+ * All gap regions are reserved.
+ */
+
+typedef struct MmioTestDevice {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    MemoryRegion container;
+    MemoryRegion sram;
+    MemoryRegion sram_alias[8];
+    MemoryRegion interleaver_container;
+    MemoryRegion iomem;
+
+    uint64_t base;
+} MmioTestDevice;
+
+#define TESTDEV(obj) \
+     OBJECT_CHECK(MmioTestDevice, (obj), TYPE_MMIO_TESTDEV)
+
+static void mmio_testdev_realize(DeviceState *dev, Error **errp)
+{
+    static const unsigned bhexs[] = {
+        [8] = 0x8, [16] = 0x16, [32] = 0x32, [64] = 0x64,
+    };
+    static const struct {
+        unsigned in, out;
+        const char *typename;
+    } cfg[] = {
+        {16, 8,  TYPE_INTERLEAVER_16X8_DEVICE},
+        {32, 8,  TYPE_INTERLEAVER_32X8_DEVICE},
+        {32, 16, TYPE_INTERLEAVER_32X16_DEVICE},
+        {64, 8,  TYPE_INTERLEAVER_64X8_DEVICE},
+        {64, 16, TYPE_INTERLEAVER_64X16_DEVICE},
+        {64, 32, TYPE_INTERLEAVER_64X32_DEVICE},
+    };
+    MmioTestDevice *s = TESTDEV(dev);
+    DeviceState *interleaver;
+
+    if (s->base == UINT64_MAX) {
+        error_setg(errp, "property 'address' not specified or zero");
+        return;
+    }
+
+    memory_region_init(&s->container, OBJECT(s), "testdev", 0x20000000);
+
+    memory_region_init_ram(&s->sram, OBJECT(s), "testdev-sram",
+                           0x800, &error_fatal);
+    memory_region_add_subregion(&s->container, 0x000000, &s->sram);
+
+    /* interleaved memory */
+    memory_region_init(&s->interleaver_container, OBJECT(s),
+                       "interleaver-container", 0x8000000);
+    memory_region_add_subregion(&s->container, 0x10000000,
+                                &s->interleaver_container);
+    for (unsigned i = 0; i < 8; i++) {
+        g_autofree char *name = g_strdup_printf("sram-p%u", i);
+        /* Each alias access a 256B region of the SRAM */
+        memory_region_init_alias(&s->sram_alias[i], OBJECT(s), name,
+                                 &s->sram, i * 0x100, 0x100);
+    }
+    for (size_t i = 0; i < ARRAY_SIZE(cfg); i++) {
+        unsigned count = cfg[i].in / cfg[i].out;
+
+        interleaver = qdev_new(cfg[i].typename);
+        qdev_prop_set_uint64(interleaver, "size", count * 0x100);
+        /* Map 256B SRAM regions on interleaver banks */
+        for (unsigned c = 0; c < count; c++) {
+            g_autofree char *prop_name = g_strdup_printf("mr%u", c);
+            object_property_set_link(OBJECT(interleaver), prop_name,
+                                     OBJECT(&s->sram_alias[c]), &error_abort);
+        }
+        sysbus_realize_and_unref(SYS_BUS_DEVICE(interleaver), &error_fatal);
+        memory_region_add_subregion(&s->interleaver_container,
+                (bhexs[cfg[i].in] << 20) | (bhexs[cfg[i].out] << 12),
+                sysbus_mmio_get_region(SYS_BUS_DEVICE(interleaver), 0));
+    }
+
+    memory_region_add_subregion(get_system_memory(), s->base, &s->container);
+}
+
+static Property mmio_testdev_properties[] = {
+    DEFINE_PROP_UINT64("address", MmioTestDevice, base, UINT64_MAX),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void mmio_testdev_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = mmio_testdev_realize;
+    dc->user_creatable = true;
+    device_class_set_props(dc, mmio_testdev_properties);
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo mmio_testdev_info = {
+    .name           = TYPE_MMIO_TESTDEV,
+    .parent         = TYPE_SYS_BUS_DEVICE,
+    .instance_size  = sizeof(MmioTestDevice),
+    .class_init     = mmio_testdev_class_init,
+};
+
+static void mmio_testdev_register_types(void)
+{
+    type_register_static(&mmio_testdev_info);
+}
+
+type_init(mmio_testdev_register_types)
diff --git a/MAINTAINERS b/MAINTAINERS
index 1efce3dd27..f75b8c984a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1966,6 +1966,8 @@ M: Philippe Mathieu-Daudé <f4bug@amsat.org>
 S: Maintained
 F: include/hw/misc/interleaver.h
 F: hw/misc/interleaver.c
+F: hw/misc/mmio-testdev.c
+F: include/hw/misc/testdev.h
 
 Standard VGA
 M: Gerd Hoffmann <kraxel@redhat.com>
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 7ed0f4ccc7..5b101abeea 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -30,6 +30,11 @@ config ISA_TESTDEV
     default y if TEST_DEVICES
     depends on ISA_BUS
 
+config MMIO_TESTDEV
+    bool
+    default y if TEST_DEVICES
+    depends on INTERLEAVER
+
 config PCI_TESTDEV
     bool
     default y if TEST_DEVICES
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index aa753a847f..b3e7da7177 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -5,6 +5,7 @@ common-obj-$(CONFIG_TMP421) += tmp421.o
 common-obj-$(CONFIG_ISA_DEBUG) += debugexit.o
 common-obj-$(CONFIG_SGA) += sga.o
 common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o
+common-obj-$(CONFIG_MMIO_TESTDEV) += mmio-testdev.o
 common-obj-$(CONFIG_PCI_TESTDEV) += pci-testdev.o
 common-obj-$(CONFIG_EDU) += edu.o
 common-obj-$(CONFIG_PCA9552) += pca9552.o
-- 
2.26.2

[RFC PATCH 6/9] hw/core/null-machine: Allow to use the MMIO 'test' device

[Philippe Mathieu-Daudé]

The MMIO 'test' device (TYPE_MMIO_TESTDEV) can be mapped almost
anywhere on the sysbus memory. Allow the 'none' machine to
instantiate it from the command line, such:

  $ qemu-system-sh4 -M none -monitor stdio \
    -device mmio-testdev,address=0x00000000 -monitor stdio
  (qemu) info mtree
  address-space: memory
    0000000000000000-ffffffffffffffff (prio 0, i/o): system
      0000000000000000-000000001fffffff (prio 0, i/o): testdev
        0000000000000000-00000000000007ff (prio 0, ram): testdev-sram
        0000000010000000-0000000017ffffff (prio 0, i/o): interleaver-container
          0000000011608000-00000000116081ff (prio 0, i/o): interleaver-16x8
          0000000013208000-00000000132083ff (prio 0, i/o): interleaver-32x8
          0000000013216000-00000000132161ff (prio 0, i/o): interleaver-32x16
          0000000016408000-00000000164087ff (prio 0, i/o): interleaver-64x8
          0000000016416000-00000000164163ff (prio 0, i/o): interleaver-64x16
          0000000016432000-00000000164321ff (prio 0, i/o): interleaver-64x32

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/core/null-machine.c | 2 ++
 hw/misc/mmio-testdev.c | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
index 7e693523d7..d8be17092f 100644
--- a/hw/core/null-machine.c
+++ b/hw/core/null-machine.c
@@ -17,6 +17,7 @@
 #include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
 #include "hw/core/cpu.h"
+#include "hw/misc/testdev.h"
 
 static void machine_none_init(MachineState *mch)
 {
@@ -55,6 +56,7 @@ static void machine_none_machine_init(MachineClass *mc)
     mc->no_floppy = 1;
     mc->no_cdrom = 1;
     mc->no_sdcard = 1;
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_MMIO_TESTDEV);
 }
 
 DEFINE_MACHINE("none", machine_none_machine_init)
diff --git a/hw/misc/mmio-testdev.c b/hw/misc/mmio-testdev.c
index 3b7a8057b2..42eed16f2d 100644
--- a/hw/misc/mmio-testdev.c
+++ b/hw/misc/mmio-testdev.c
@@ -9,6 +9,10 @@
 /*
  * This device is mostly used to test QEMU internal MMIO devices.
  * Accesses using CPU core are not allowed.
+ *
+ * This device is meant to be used for testing, like:
+ *
+ *   qemu-system-sh4 -M none -device mmio-testdev,address=0x10000000
  */
 
 #include "qemu/osdep.h"
-- 
2.26.2

[RFC PATCH 7/9] tests/qtest: Add generic MMIO tests

[Philippe Mathieu-Daudé]

Add generic MMIO tests (which don't need any particular knowledge
about the architecture) and can be run without any CPU (using the
'none' machine).

Start testing the interleaved memory accesses.

Cross-endianess is not tested (host and target must have the same
endianess).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/qtest/mmio-test.c      | 146 +++++++++++++++++++++++++++++++++++
 MAINTAINERS                  |   1 +
 tests/qtest/Makefile.include |   1 +
 3 files changed, 148 insertions(+)
 create mode 100644 tests/qtest/mmio-test.c

diff --git a/tests/qtest/mmio-test.c b/tests/qtest/mmio-test.c
new file mode 100644
index 0000000000..7a31eb8e9d
--- /dev/null
+++ b/tests/qtest/mmio-test.c
@@ -0,0 +1,146 @@
+/*
+ * QTest testcases for generic MMIO accesses
+ *
+ * Copyright (C) 2020 Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+
+#include "libqtest.h"
+#include "qemu/bswap.h"
+
+/* Must fit in arch address space */
+static const uint64_t base = 0x20000000ul;
+
+static bool is_cross_endian(QTestState *qts)
+{
+    bool te = qtest_big_endian(qts);
+#ifdef HOST_WORDS_BIGENDIAN
+    te = !te;
+#endif
+    return te;
+}
+
+static QTestState *create_interleaver_qtest(void)
+{
+    QTestState *qts;
+
+    qts = qtest_initf("-M none -device mmio-testdev,address=0x%" PRIx64, base);
+    if (is_cross_endian(qts)) {
+        g_test_skip("Skipping on cross-endian targets");
+        qtest_quit(qts);
+        return NULL;
+    }
+    return qts;
+}
+
+static void test_interleaver_rd32x8a(void)
+{
+    QTestState *qts = create_interleaver_qtest();
+
+    if (!qts) {
+        return;
+    }
+
+    /* write sram directly */
+    qtest_writeb(qts, base + 0x000, 0x10);
+    qtest_writeb(qts, base + 0x100, 0x32);
+    qtest_writeb(qts, base + 0x200, 0x54);
+    qtest_writeb(qts, base + 0x300, 0x76);
+    /* read via interleaver */
+    g_assert_cmphex(qtest_readl(qts, base + 0x13208000 + 0x00), ==, 0x76543210);
+    qtest_quit(qts);
+}
+
+static void test_interleaver_rd32x8b(void)
+{
+    QTestState *qts = create_interleaver_qtest();
+
+    if (!qts) {
+        return;
+    }
+
+    /* write sram directly */
+    qtest_writeb(qts, base + 0x003, 0x10);
+    qtest_writeb(qts, base + 0x103, 0x32);
+    qtest_writeb(qts, base + 0x203, 0x54);
+    qtest_writeb(qts, base + 0x303, 0x76);
+    /* read via interleaver */
+    g_assert_cmphex(qtest_readl(qts, base + 0x13208000 + 0x0c), ==, 0x76543210);
+    qtest_quit(qts);
+}
+
+static void test_interleaver_rd32x16(void)
+{
+    QTestState *qts = create_interleaver_qtest();
+
+    if (!qts) {
+        return;
+    }
+
+    /* write sram directly */
+    qtest_writew(qts, base + 0x002, 0x3210);
+    qtest_writew(qts, base + 0x102, 0x7654);
+    /* read via interleaver */
+    g_assert_cmphex(qtest_readl(qts, base + 0x13216000 + 0x04), ==, 0x76543210);
+    qtest_quit(qts);
+}
+
+static void test_interleaver_wr32x16(void)
+{
+    QTestState *qts = create_interleaver_qtest();
+
+    if (!qts) {
+        return;
+    }
+
+    /* write via interleaver */
+    qtest_writel(qts, base + 0x13216000 + 0x04, 0x76543210);
+    /* read sram directly */
+    g_assert_cmphex(qtest_readw(qts, base + 0x002), ==, 0x3210);
+    g_assert_cmphex(qtest_readw(qts, base + 0x102), ==, 0x7654);
+    qtest_quit(qts);
+}
+
+static void test_interleaver_wr64x8(void)
+{
+    QTestState *qts = create_interleaver_qtest();
+
+    if (!qts) {
+        return;
+    }
+
+    /* write via interleaver */
+    qtest_writeq(qts, base + 0x16408000 + 0x08, 0x9876543210);
+    /* read sram directly */
+    g_assert_cmphex(qtest_readb(qts, base + 0x001), ==, 0x10);
+    g_assert_cmphex(qtest_readb(qts, base + 0x101), ==, 0x32);
+    g_assert_cmphex(qtest_readb(qts, base + 0x401), ==, 0x98);
+    qtest_quit(qts);
+}
+
+static struct {
+    const char *name;
+    void (*test)(void);
+} tests[] = {
+    {"interleaver/rd32x8a", test_interleaver_rd32x8a},
+    {"interleaver/rd32x8b", test_interleaver_rd32x8b},
+    {"interleaver/rd32x16", test_interleaver_rd32x16},
+    {"interleaver/wr32x16", test_interleaver_wr32x16},
+    {"interleaver/wr64x8",  test_interleaver_wr64x8},
+};
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    for (size_t i = 0; i < ARRAY_SIZE(tests); i++) {
+        g_autofree gchar *path = g_strdup_printf("mmio/%s",
+                                                 tests[i].name);
+        qtest_add_func(path, tests[i].test);
+    }
+
+    return g_test_run();
+}
diff --git a/MAINTAINERS b/MAINTAINERS
index f75b8c984a..93efef1b87 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1968,6 +1968,7 @@ F: include/hw/misc/interleaver.h
 F: hw/misc/interleaver.c
 F: hw/misc/mmio-testdev.c
 F: include/hw/misc/testdev.h
+F: tests/qtest/mmio-test.c
 
 Standard VGA
 M: Gerd Hoffmann <kraxel@redhat.com>
diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
index b0204e44f2..29dbeb5d05 100644
--- a/tests/qtest/Makefile.include
+++ b/tests/qtest/Makefile.include
@@ -9,6 +9,7 @@ check-qtest-generic-y += qmp-cmd-test
 check-qtest-generic-y += qom-test
 check-qtest-generic-$(CONFIG_MODULES) += modules-test
 check-qtest-generic-y += test-hmp
+check-qtest-generic-$(CONFIG_MMIO_TESTDEV) += mmio-test
 
 check-qtest-pci-$(CONFIG_RTL8139_PCI) += rtl8139-test
 check-qtest-pci-$(CONFIG_VGA) += display-vga-test
-- 
2.26.2

[RFC PATCH 8/9] memory: Allow memory region to display its subregions own descriptions

[Philippe Mathieu-Daudé]

If a MemoryRegion has subregion linked (but NOT mapped), these
subregions won't be displayed in the 'info mtree' HMP command.

Add the possibility to display such subregion descriptions.
It will result useful for the Interleaver memory device.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Any clever idea?
---
 include/exec/memory.h |  6 ++++++
 softmmu/memory.c      | 22 ++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 307e527835..8bcacfc79e 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -404,6 +404,12 @@ struct MemoryRegion {
     const char *name;
     unsigned ioeventfd_nb;
     MemoryRegionIoeventfd *ioeventfds;
+    /*
+     * If a memory region has subregions linked, it can use this
+     * handler to return an array of string, each string holding
+     * the subregion description.
+     */
+    GStrv (*subregions_description)(const MemoryRegion *mr);
 };
 
 struct IOMMUMemoryRegion {
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 8139da1a58..f8e27edbe2 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -2967,6 +2967,28 @@ static void mtree_print_mr(const MemoryRegion *mr, unsigned int level,
                 mtree_print_mr_owner(mr);
             }
             qemu_printf("\n");
+
+            if (mr->subregions_description) {
+                GStrv s = mr->subregions_description(mr);
+                for (int j = 0; s[j]; j++) {
+                    for (i = 0; i < level; i++) {
+                        qemu_printf(MTREE_INDENT);
+                    }
+                    qemu_printf(TARGET_FMT_plx "-" TARGET_FMT_plx
+                                " (prio %d, %s%s): %s%s",
+                                cur_start, cur_end,
+                                mr->priority,
+                                mr->nonvolatile ? "nv-" : "",
+                                memory_region_type((MemoryRegion *)mr),
+                                s[j],
+                                mr->enabled ? "" : " [disabled]");
+                    if (owner) {
+                        mtree_print_mr_owner(mr);
+                    }
+                    qemu_printf("\n");
+                }
+                g_strfreev(s);
+            }
         }
     }
 
-- 
2.26.2

[RFC PATCH 9/9] hw/misc/interleaver: Display subregions in 'info mtree'

[Philippe Mathieu-Daudé]

Implement the MemoryRegion::subregions_description() handler to
be able to display the inverleaved memory regions:

  $ qemu-system-sh4 -M none -monitor stdio \
    -device mmio-testdev,address=0x00000000 -monitor stdio
  address-space: memory
    0000000000000000-ffffffffffffffff (prio 0, i/o): system
      0000000000000000-000000001fffffff (prio 0, i/o): testdev
        0000000000000000-00000000000007ff (prio 0, ram): testdev-sram
        0000000010000000-0000000017ffffff (prio 0, i/o): interleaver-container
          0000000011608000-00000000116081ff (prio 0, i/o): interleaver-16x8
          0000000011608000-00000000116081ff (prio 0, i/o):   8-bit access on 'sram-p0'
          0000000011608000-00000000116081ff (prio 0, i/o):   8-bit access on 'sram-p1' (8-bit shifted)
          0000000013208000-00000000132083ff (prio 0, i/o): interleaver-32x8
          0000000013208000-00000000132083ff (prio 0, i/o):   8-bit access on 'sram-p0'
          0000000013208000-00000000132083ff (prio 0, i/o):   8-bit access on 'sram-p1' (8-bit shifted)
          0000000013208000-00000000132083ff (prio 0, i/o):   8-bit access on 'sram-p2' (16-bit shifted)
          0000000013208000-00000000132083ff (prio 0, i/o):   8-bit access on 'sram-p3' (24-bit shifted)
          0000000013216000-00000000132161ff (prio 0, i/o): interleaver-32x16
          0000000013216000-00000000132161ff (prio 0, i/o):   16-bit access on 'sram-p0'
          0000000013216000-00000000132161ff (prio 0, i/o):   16-bit access on 'sram-p1' (16-bit shifted)
          0000000016408000-00000000164087ff (prio 0, i/o): interleaver-64x8
          0000000016408000-00000000164087ff (prio 0, i/o):   8-bit access on 'sram-p0'
          0000000016408000-00000000164087ff (prio 0, i/o):   8-bit access on 'sram-p1' (8-bit shifted)
          0000000016408000-00000000164087ff (prio 0, i/o):   8-bit access on 'sram-p2' (16-bit shifted)
          0000000016408000-00000000164087ff (prio 0, i/o):   8-bit access on 'sram-p3' (24-bit shifted)
          0000000016408000-00000000164087ff (prio 0, i/o):   8-bit access on 'sram-p4' (32-bit shifted)
          0000000016408000-00000000164087ff (prio 0, i/o):   8-bit access on 'sram-p5' (40-bit shifted)
          0000000016408000-00000000164087ff (prio 0, i/o):   8-bit access on 'sram-p6' (48-bit shifted)
          0000000016408000-00000000164087ff (prio 0, i/o):   8-bit access on 'sram-p7' (56-bit shifted)
          0000000016416000-00000000164163ff (prio 0, i/o): interleaver-64x16
          0000000016416000-00000000164163ff (prio 0, i/o):   16-bit access on 'sram-p0'
          0000000016416000-00000000164163ff (prio 0, i/o):   16-bit access on 'sram-p1' (16-bit shifted)
          0000000016416000-00000000164163ff (prio 0, i/o):   16-bit access on 'sram-p2' (32-bit shifted)
          0000000016416000-00000000164163ff (prio 0, i/o):   16-bit access on 'sram-p3' (48-bit shifted)
          0000000016432000-00000000164321ff (prio 0, i/o): interleaver-64x32
          0000000016432000-00000000164321ff (prio 0, i/o):   32-bit access on 'sram-p0'
          0000000016432000-00000000164321ff (prio 0, i/o):   32-bit access on 'sram-p1' (32-bit shifted)

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/interleaver.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/hw/misc/interleaver.c b/hw/misc/interleaver.c
index 46099e9e11..5cedcb3541 100644
--- a/hw/misc/interleaver.c
+++ b/hw/misc/interleaver.c
@@ -112,6 +112,34 @@ static MemTxResult interleaver_write(void *opaque,
     return r;
 }
 
+static GStrv interleaver_subregions_description(const MemoryRegion *mr)
+{
+    InterleaverDeviceState *s = container_of(mr, InterleaverDeviceState, iomem);
+    InterleaverDeviceClass *idc = INTERLEAVER_DEVICE_GET_CLASS(s);
+    gchar **descs = g_new(gchar *, idc->mr_count + 1);
+    unsigned output_access_bits = idc->output_access_size << 3;
+    size_t i;
+
+    for (i = 0; i < idc->mr_count; i++) {
+        if (i) {
+            descs[i] = g_strdup_printf("  %u-bit access on '%s'"
+                                       " (%zu-bit shifted)",
+                                       output_access_bits,
+                                       s->mr[i] ? memory_region_name(s->mr[i])
+                                                : emtpy_mr_name,
+                                       i * output_access_bits);
+       } else {
+            descs[i] = g_strdup_printf("  %u-bit access on '%s'",
+                                       output_access_bits,
+                                       s->mr[i] ? memory_region_name(s->mr[i])
+                                                : emtpy_mr_name);
+        }
+    }
+    descs[i] = NULL;
+
+    return descs;
+}
+
 static void interleaver_realize(DeviceState *dev, Error **errp)
 {
     InterleaverDeviceState *s = INTERLEAVER_DEVICE(dev);
@@ -139,6 +167,7 @@ static void interleaver_realize(DeviceState *dev, Error **errp)
     }
     memory_region_init_io(&s->iomem, OBJECT(s), &idc->ops, s,
                           idc->name, s->size);
+    s->iomem.subregions_description = interleaver_subregions_description;
     sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
 }
 
-- 
2.26.2

Re: [RFC PATCH 0/9] hw/misc: Add support for interleaved memory accesses

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200817161853.593247-1-f4bug@amsat.org/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      tests/qtest/e1000-test.o
  CC      tests/qtest/e1000e-test.o
  CC      tests/qtest/eepro100-test.o
/tmp/qemu-test/src/tests/qtest/mmio-test.c:9:24: fatal error: qemu/osdep.h: No such file or directory
 #include "qemu/osdep.h"
                        ^
compilation terminated.
  CC      tests/qtest/es1370-test.o
  CC      tests/qtest/ipoctal232-test.o
  CC      tests/qtest/megasas-test.o
make: *** [tests/qtest/mmio-test] Error 1
make: *** Waiting for unfinished jobs....
  CC      test_abcz_f32.o
  CC      test_ab_f32_z_bool.o
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=84ffc0d808a34b62824e6e470c7b703a', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-4imsyimq/src/docker-src.2020-08-17-16.08.23.7359:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=84ffc0d808a34b62824e6e470c7b703a
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-4imsyimq/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    5m5.999s
user    0m6.598s


The full log is available at
http://patchew.org/logs/20200817161853.593247-1-f4bug@amsat.org/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [RFC PATCH 0/9] hw/misc: Add support for interleaved memory accesses

[Philippe Mathieu-Daudé]

On 8/17/20 6:18 PM, Philippe Mathieu-Daudé wrote:
> This is a follow up on last year patch from Stephen Checkoway:
> "block/pflash_cfi02: Implement intereleaved flash devices"
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg613077.html
> 
> Instead of making the PFLASH device more complex, I went
> implementing a generic interleaver device (that can be re-used
> for other MMIO devices, probably pointless, still useful for
> simple testing).
> 
> The series is available on [1] and is organized as:
> 
> 1/ Fix a pair of issues in memory.c as we are not exercing much
> the memory_region_[read/write]_with_attrs_accessor functions (see
> also [2]).
> 
> 2/ Allow to use qtest helpers with the 'none' machine (without any
> CPU).
> 
> 3/ Add the interleaver device
> 
> 4/ Add a device to test the interleaver device (later I'll expand
> it to other MMIO accesses, such access_with_adjusted_size, endianess
> and unaligned accesses). Let the 'none' machine create this device
> from command line.
> 
> 5/ Add qtests using 4/ to test 3/.
> 
> 6/ Add a way to display the interleaved memory (sub)regions in
> the HMP 'info mtree' command.
> 
> Based-on: <20200816173051.16274-1-f4bug@amsat.org>
> [1] https://gitlab.com/philmd/qemu/-/tree/interleaved_device-v1
> [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg730279.html

I followed Paolo's suggestion to use qtest_memread/qtest_memwrite to
avoid endianess swapping, and now cross-endianess works :)

Review still welcome, but I'll respin in September.

Regards,

Phil.

Re: [RFC PATCH 0/9] hw/misc: Add support for interleaved memory accesses

[Philippe Mathieu-Daudé]

On 8/17/20 6:18 PM, Philippe Mathieu-Daudé wrote:
> This is a follow up on last year patch from Stephen Checkoway:
> "block/pflash_cfi02: Implement intereleaved flash devices"
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg613077.html
> 
> Instead of making the PFLASH device more complex, I went
> implementing a generic interleaver device (that can be re-used
> for other MMIO devices, probably pointless, still useful for
> simple testing).
> 
> The series is available on [1] and is organized as:
> 
> 1/ Fix a pair of issues in memory.c as we are not exercing much
> the memory_region_[read/write]_with_attrs_accessor functions (see
> also [2]).
> 
> 2/ Allow to use qtest helpers with the 'none' machine (without any
> CPU).
> 
> 3/ Add the interleaver device
> 
> 4/ Add a device to test the interleaver device (later I'll expand
> it to other MMIO accesses, such access_with_adjusted_size, endianess
> and unaligned accesses). Let the 'none' machine create this device
> from command line.
> 
> 5/ Add qtests using 4/ to test 3/.
> 
> 6/ Add a way to display the interleaved memory (sub)regions in
> the HMP 'info mtree' command.
> 
> Based-on: <20200816173051.16274-1-f4bug@amsat.org>
> [1] https://gitlab.com/philmd/qemu/-/tree/interleaved_device-v1
> [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg730279.html

I followed Paolo's suggestion to use qtest_memread/qtest_memwrite to
avoid endianess swapping, and now cross-endianess works :)

Review still welcome, but I'll respin in September.

Regards,

Phil.