[Qemu-devel] [PULL 74/79] memory: Fix access_with_adjusted_size(small size) on big-endian memory regions

[Paolo Bonzini]

From: Philippe Mathieu-Daudé <address@hidden>

Memory regions configured as DEVICE_BIG_ENDIAN (or DEVICE_NATIVE_ENDIAN on
big-endian guest) behave incorrectly when the memory access 'size' is smaller
than the implementation 'access_size'.

In the following code segment from access_with_adjusted_size():

    if (memory_region_big_endian(mr)) {
        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);
        }

(size - access_size - i) * 8 is the number of bits that will arithmetic
shift the current value.

Currently we can only 'left' shift a read() access, and 'right' shift a write().

When the access 'size' is smaller than the implementation, we get a negative
number of bits to shift.

For the read() case, a negative 'left' shift is a 'right' shift :)
However since the 'shift' type is unsigned, there is currently no way to
right shift.

Fix this by changing the access_fn() prototype to handle signed shift values,
and modify the memory_region_shift_read|write_access() helpers to correctly
arithmetic shift the opposite direction when the 'shift' value is negative.

Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
---
 memory.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/memory.c b/memory.c
index a4d3fa7..b96aec7 100644
--- a/memory.c
+++ b/memory.c
@@ -375,18 +375,30 @@ static void adjust_endianness(MemoryRegion *mr, uint64_t 
*data, unsigned size)
 }
 
 static inline void memory_region_shift_read_access(uint64_t *value,
-                                                   unsigned shift,
+                                                   signed shift,
                                                    uint64_t mask,
                                                    uint64_t tmp)
 {
-    *value |= (tmp & mask) << shift;
+    if (shift >= 0) {
+        *value |= (tmp & mask) << shift;
+    } else {
+        *value |= (tmp & mask) >> -shift;
+    }
 }
 
 static inline uint64_t memory_region_shift_write_access(uint64_t *value,
-                                                        unsigned shift,
+                                                        signed shift,
                                                         uint64_t mask)
 {
-    return (*value >> shift) & mask;
+    uint64_t tmp;
+
+    if (shift >= 0) {
+        tmp = (*value >> shift) & mask;
+    } else {
+        tmp = (*value << -shift) & mask;
+    }
+
+    return tmp;
 }
 
 static hwaddr memory_region_to_absolute_addr(MemoryRegion *mr, hwaddr offset)
@@ -415,7 +427,7 @@ static MemTxResult 
memory_region_oldmmio_read_accessor(MemoryRegion *mr,
                                                        hwaddr addr,
                                                        uint64_t *value,
                                                        unsigned size,
-                                                       unsigned shift,
+                                                       signed shift,
                                                        uint64_t mask,
                                                        MemTxAttrs attrs)
 {
@@ -441,7 +453,7 @@ static MemTxResult  
memory_region_read_accessor(MemoryRegion *mr,
                                                 hwaddr addr,
                                                 uint64_t *value,
                                                 unsigned size,
-                                                unsigned shift,
+                                                signed shift,
                                                 uint64_t mask,
                                                 MemTxAttrs attrs)
 {
@@ -467,7 +479,7 @@ static MemTxResult 
memory_region_read_with_attrs_accessor(MemoryRegion *mr,
                                                           hwaddr addr,
                                                           uint64_t *value,
                                                           unsigned size,
-                                                          unsigned shift,
+                                                          signed shift,
                                                           uint64_t mask,
                                                           MemTxAttrs attrs)
 {
@@ -494,7 +506,7 @@ static MemTxResult 
memory_region_oldmmio_write_accessor(MemoryRegion *mr,
                                                         hwaddr addr,
                                                         uint64_t *value,
                                                         unsigned size,
-                                                        unsigned shift,
+                                                        signed shift,
                                                         uint64_t mask,
                                                         MemTxAttrs attrs)
 {
@@ -519,7 +531,7 @@ static MemTxResult 
memory_region_write_accessor(MemoryRegion *mr,
                                                 hwaddr addr,
                                                 uint64_t *value,
                                                 unsigned size,
-                                                unsigned shift,
+                                                signed shift,
                                                 uint64_t mask,
                                                 MemTxAttrs attrs)
 {
@@ -544,7 +556,7 @@ static MemTxResult 
memory_region_write_with_attrs_accessor(MemoryRegion *mr,
                                                            hwaddr addr,
                                                            uint64_t *value,
                                                            unsigned size,
-                                                           unsigned shift,
+                                                           signed shift,
                                                            uint64_t mask,
                                                            MemTxAttrs attrs)
 {
@@ -574,7 +586,7 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
                                                    hwaddr addr,
                                                    uint64_t *value,
                                                    unsigned size,
-                                                   unsigned shift,
+                                                   signed shift,
                                                    uint64_t mask,
                                                    MemTxAttrs attrs),
                                       MemoryRegion *mr,
-- 
1.8.3.1