Re: [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian

[Philippe Mathieu-Daudé]

On 08/11/2017 11:25 AM, Paolo Bonzini wrote:
On 11/08/2017 16:06, Philippe Mathieu-Daudé wrote:
remove unnecessary memory_region_big_endian()

Can you explain why it's unnecessary?...

I might have missed a lot here, I'm focusing on something else and 
thought this might help Frederic so I only cherry-picked this patch from 
a bigger series, since I memory_region_big_endian() is static/unused I 
just dropped it, which is surely wrong.

I see in this series I then call "adjust_endianness(mr, value, size)" 
before returning. And then lot of unit-tests to understand failures I'm 
having accessing PCI bus on armeb.

This branch is few months old and I'm out of context, I shouldn't have 
rushed to send this patch, I'll try to get some time during the week-end 
to remember.

Cheers,

Phil.


Paolo

Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
---
This is part of a branch with many cross-endianness tests for 2.11

Frederic does it helps your issues on armeb?

  memory.c | 28 ++++++++++------------------
  1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/memory.c b/memory.c
index c0adc35410..600f5d5b1a 100644
--- a/memory.c
+++ b/memory.c
@@ -338,15 +338,6 @@ static void flatview_simplify(FlatView *view)
      }
  }
  
-static bool memory_region_big_endian(MemoryRegion *mr)
-{
-#ifdef TARGET_WORDS_BIGENDIAN
-    return mr->ops->endianness != DEVICE_LITTLE_ENDIAN;
-#else
-    return mr->ops->endianness == DEVICE_BIG_ENDIAN;
-#endif
-}
-
  static bool memory_region_wrong_endianness(MemoryRegion *mr)
  {
  #ifdef TARGET_WORDS_BIGENDIAN
@@ -572,7 +563,8 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
  {
      uint64_t access_mask;
      unsigned access_size;
-    unsigned i;
+    hwaddr access_addr;
+    unsigned offset;
      MemTxResult r = MEMTX_OK;
  
      if (!access_size_min) {
@@ -585,17 +577,17 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
      /* FIXME: support unaligned access? */
      access_size = MAX(MIN(size, access_size_max), access_size_min);
      access_mask = -1ULL >> (64 - access_size * 8);
-    if (memory_region_big_endian(mr)) {
-        for (i = 0; i < size; i += access_size) {
-            r |= access(mr, addr + i, value, access_size,
-                        (size - access_size - i) * 8, access_mask, attrs);
+    access_addr = addr & ~(access_size - 1);
+    if (access_size < size) {
+        for (offset = 0; offset < size; offset += access_size) {
+            r |= access(mr, access_addr + offset, value, access_size,
+                        offset * 8, access_mask, attrs);
          }
      } else {
-        for (i = 0; i < size; i += access_size) {
-            r |= access(mr, addr + i, value, access_size, i * 8,
-                        access_mask, attrs);
-        }
+        r = access(mr, access_addr, value, access_size,
+                    0, access_mask, attrs);
      }
+
      return r;
  }