[PATCH v3 03/25] system/physmem: Use explicit endianness in subpage_ops::read/write()

Philippe Mathieu-Daudé posted 25 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v3 03/25] system/physmem: Use explicit endianness in subpage_ops::read/write()
Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago
Replace the ldn_p/stn_p() calls by their explicit endianness
variants. Duplicate the MemoryRegionOps, replacing the single
DEVICE_NATIVE_ENDIAN entry by a pair of LITTLE and BIG ones.
Select the proper MemoryRegionOps in subpage_init().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 system/physmem.c | 76 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 59 insertions(+), 17 deletions(-)

diff --git a/system/physmem.c b/system/physmem.c
index 7e914ecf648..9fe84679cac 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2894,8 +2894,8 @@ static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
 static bool flatview_access_valid(FlatView *fv, hwaddr addr, hwaddr len,
                                   bool is_write, MemTxAttrs attrs);
 
-static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
-                                unsigned len, MemTxAttrs attrs)
+static MemTxResult subpage_read_le(void *opaque, hwaddr addr, uint64_t *data,
+                                   unsigned len, MemTxAttrs attrs)
 {
     subpage_t *subpage = opaque;
     uint8_t buf[8];
@@ -2906,18 +2906,49 @@ static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
     if (res) {
         return res;
     }
-    *data = ldn_p(buf, len);
+    *data = ldn_le_p(buf, len);
     return MEMTX_OK;
 }
 
-static MemTxResult subpage_write(void *opaque, hwaddr addr,
-                                 uint64_t value, unsigned len, MemTxAttrs attrs)
+static MemTxResult subpage_read_be(void *opaque, hwaddr addr, uint64_t *data,
+                                   unsigned len, MemTxAttrs attrs)
+{
+    subpage_t *subpage = opaque;
+    uint8_t buf[8];
+    MemTxResult res;
+
+    trace_subpage_read(subpage, len, addr);
+    res = flatview_read(subpage->fv, addr + subpage->base, attrs, buf, len);
+    if (res) {
+        return res;
+    }
+    *data = ldn_be_p(buf, len);
+    return MEMTX_OK;
+}
+
+static MemTxResult subpage_write_le(void *opaque, hwaddr addr,
+                                    uint64_t value, unsigned len,
+                                    MemTxAttrs attrs)
 {
     subpage_t *subpage = opaque;
     uint8_t buf[8];
 
     trace_subpage_write(subpage, len, addr, value);
-    stn_p(buf, len, value);
+    stn_le_p(buf, len, value);
+
+    return flatview_write(subpage->fv, addr + subpage->base, attrs, buf, len);
+}
+
+static MemTxResult subpage_write_be(void *opaque, hwaddr addr,
+                                    uint64_t value, unsigned len,
+                                    MemTxAttrs attrs)
+{
+    subpage_t *subpage = opaque;
+    uint8_t buf[8];
+
+    trace_subpage_write(subpage, len, addr, value);
+    stn_be_p(buf, len, value);
+
     return flatview_write(subpage->fv, addr + subpage->base, attrs, buf, len);
 }
 
@@ -2933,15 +2964,26 @@ static bool subpage_accepts(void *opaque, hwaddr addr,
                                  len, is_write, attrs);
 }
 
-static const MemoryRegionOps subpage_ops = {
-    .read_with_attrs = subpage_read,
-    .write_with_attrs = subpage_write,
-    .impl.min_access_size = 1,
-    .impl.max_access_size = 8,
-    .valid.min_access_size = 1,
-    .valid.max_access_size = 8,
-    .valid.accepts = subpage_accepts,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+static const MemoryRegionOps subpage_ops[2] = {
+    [0 ... 1] = {
+        .impl = {
+            .min_access_size = 1,
+            .max_access_size = 8,
+        },
+        .valid = {
+            .min_access_size = 1,
+            .max_access_size = 8,
+            .accepts = subpage_accepts,
+        },
+    },
+
+    [0].endianness = DEVICE_LITTLE_ENDIAN,
+    [0].read_with_attrs = subpage_read_le,
+    [0].write_with_attrs = subpage_write_le,
+
+    [1].endianness = DEVICE_BIG_ENDIAN,
+    [1].read_with_attrs = subpage_read_be,
+    [1].write_with_attrs = subpage_write_be,
 };
 
 static int subpage_register(subpage_t *mmio, uint32_t start, uint32_t end,
@@ -2969,8 +3011,8 @@ static subpage_t *subpage_init(FlatView *fv, hwaddr base)
     mmio = g_malloc0(sizeof(subpage_t) + TARGET_PAGE_SIZE * sizeof(uint16_t));
     mmio->fv = fv;
     mmio->base = base;
-    memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio,
-                          NULL, TARGET_PAGE_SIZE);
+    memory_region_init_io(&mmio->iomem, NULL, &subpage_ops[target_big_endian()],
+                          mmio, NULL, TARGET_PAGE_SIZE);
     mmio->iomem.subpage = true;
     trace_subpage_init(mmio, base, TARGET_PAGE_SIZE);
 
-- 
2.52.0


Re: [PATCH v3 03/25] system/physmem: Use explicit endianness in subpage_ops::read/write()
Posted by Paolo Bonzini 1 month, 1 week ago
Il mer 24 dic 2025, 16:22 Philippe Mathieu-Daudé <philmd@linaro.org> ha
scritto:

> Replace the ldn_p/stn_p() calls by their explicit endianness
> variants. Duplicate the MemoryRegionOps, replacing the single
> DEVICE_NATIVE_ENDIAN entry by a pair of LITTLE and BIG ones.
> Select the proper MemoryRegionOps in subpage_init().
>

This would need further adjustment when the target endianness is not fixed,
but luckily the extra complexity you're introducing is not needed. The two
ops add either 0 or 2 swaps to the underlying flatview_read/flatview_write
so you only need one copy. As long as the ldn_*_p is consistent with the
endianness of the MemoryRegionOps the result is the same.

Paolo
Re: [PATCH v3 03/25] system/physmem: Use explicit endianness in subpage_ops::read/write()
Posted by Philippe Mathieu-Daudé 1 month, 1 week ago
On 28/12/25 11:54, Paolo Bonzini wrote:
> 
> 
> Il mer 24 dic 2025, 16:22 Philippe Mathieu-Daudé <philmd@linaro.org 
> <mailto:philmd@linaro.org>> ha scritto:
> 
>     Replace the ldn_p/stn_p() calls by their explicit endianness
>     variants. Duplicate the MemoryRegionOps, replacing the single
>     DEVICE_NATIVE_ENDIAN entry by a pair of LITTLE and BIG ones.
>     Select the proper MemoryRegionOps in subpage_init().
> 
> 
> This would need further adjustment when the target endianness is not 
> fixed, but luckily the extra complexity you're introducing is not 
> needed. The two ops add either 0 or 2 swaps to the underlying 
> flatview_read/flatview_write so you only need one copy. As long as the 
> ldn_*_p is consistent with the endianness of the MemoryRegionOps the 
> result is the same.

Ack, patch dropped.