Fix access_with_adjusted_size() on big-endian

[Qemu-devel] [PATCH 2/3] memory: Refactor common shifting code from accessors

Posted by Philippe Mathieu-Daudé 7 years, 4 months ago

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 memory.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/memory.c b/memory.c
index 385b9d3590..48edf7dc23 100644
--- a/memory.c
+++ b/memory.c
@@ -374,6 +374,21 @@ static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
     }
 }
 
+static inline void memory_region_shift_read_access(uint64_t *value,
+                                                   unsigned shift,
+                                                   uint64_t mask,
+                                                   uint64_t tmp)
+{
+    *value |= (tmp & mask) << shift;
+}
+
+static inline uint64_t memory_region_shift_write_access(uint64_t *value,
+                                                        unsigned shift,
+                                                        uint64_t mask)
+{
+    return (*value >> shift) & mask;
+}
+
 static hwaddr memory_region_to_absolute_addr(MemoryRegion *mr, hwaddr offset)
 {
     MemoryRegion *root;
@@ -418,7 +433,7 @@ static MemTxResult memory_region_oldmmio_read_accessor(MemoryRegion *mr,
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
     }
-    *value |= (tmp & mask) << shift;
+    memory_region_shift_read_access(value, shift, mask, tmp);
     return MEMTX_OK;
 }
 
@@ -444,7 +459,7 @@ static MemTxResult  memory_region_read_accessor(MemoryRegion *mr,
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
     }
-    *value |= (tmp & mask) << shift;
+    memory_region_shift_read_access(value, shift, mask, tmp);
     return MEMTX_OK;
 }
 
@@ -471,7 +486,7 @@ static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr,
         hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
         trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
     }
-    *value |= (tmp & mask) << shift;
+    memory_region_shift_read_access(value, shift, mask, tmp);
     return r;
 }
 
@@ -483,9 +498,8 @@ static MemTxResult memory_region_oldmmio_write_accessor(MemoryRegion *mr,
                                                         uint64_t mask,
                                                         MemTxAttrs attrs)
 {
-    uint64_t tmp;
+    uint64_t tmp = memory_region_shift_write_access(value, shift, mask);
 
-    tmp = (*value >> shift) & mask;
     if (mr->subpage) {
         trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);
     } else if (mr == &io_mem_notdirty) {
@@ -509,9 +523,8 @@ static MemTxResult memory_region_write_accessor(MemoryRegion *mr,
                                                 uint64_t mask,
                                                 MemTxAttrs attrs)
 {
-    uint64_t tmp;
+    uint64_t tmp = memory_region_shift_write_access(value, shift, mask);
 
-    tmp = (*value >> shift) & mask;
     if (mr->subpage) {
         trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);
     } else if (mr == &io_mem_notdirty) {
@@ -535,9 +548,8 @@ static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,
                                                            uint64_t mask,
                                                            MemTxAttrs attrs)
 {
-    uint64_t tmp;
+    uint64_t tmp = memory_region_shift_write_access(value, shift, mask);
 
-    tmp = (*value >> shift) & mask;
     if (mr->subpage) {
         trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);
     } else if (mr == &io_mem_notdirty) {
-- 
2.19.0

Re: [Qemu-devel] [PATCH 2/3] memory: Refactor common shifting code from accessors

Posted by Cédric Le Goater 7 years, 4 months ago


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

On 9/27/18 2:24 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  memory.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/memory.c b/memory.c
> index 385b9d3590..48edf7dc23 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -374,6 +374,21 @@ static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
>      }
>  }
>  
> +static inline void memory_region_shift_read_access(uint64_t *value,
> +                                                   unsigned shift,
> +                                                   uint64_t mask,
> +                                                   uint64_t tmp)
> +{
> +    *value |= (tmp & mask) << shift;
> +}
> +
> +static inline uint64_t memory_region_shift_write_access(uint64_t *value,
> +                                                        unsigned shift,
> +                                                        uint64_t mask)
> +{
> +    return (*value >> shift) & mask;
> +}
> +
>  static hwaddr memory_region_to_absolute_addr(MemoryRegion *mr, hwaddr offset)
>  {
>      MemoryRegion *root;
> @@ -418,7 +433,7 @@ static MemTxResult memory_region_oldmmio_read_accessor(MemoryRegion *mr,
>          hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
>          trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
>      }
> -    *value |= (tmp & mask) << shift;
> +    memory_region_shift_read_access(value, shift, mask, tmp);
>      return MEMTX_OK;
>  }
>  
> @@ -444,7 +459,7 @@ static MemTxResult  memory_region_read_accessor(MemoryRegion *mr,
>          hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
>          trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
>      }
> -    *value |= (tmp & mask) << shift;
> +    memory_region_shift_read_access(value, shift, mask, tmp);
>      return MEMTX_OK;
>  }
>  
> @@ -471,7 +486,7 @@ static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr,
>          hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
>          trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size);
>      }
> -    *value |= (tmp & mask) << shift;
> +    memory_region_shift_read_access(value, shift, mask, tmp);
>      return r;
>  }
>  
> @@ -483,9 +498,8 @@ static MemTxResult memory_region_oldmmio_write_accessor(MemoryRegion *mr,
>                                                          uint64_t mask,
>                                                          MemTxAttrs attrs)
>  {
> -    uint64_t tmp;
> +    uint64_t tmp = memory_region_shift_write_access(value, shift, mask);
>  
> -    tmp = (*value >> shift) & mask;
>      if (mr->subpage) {
>          trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);
>      } else if (mr == &io_mem_notdirty) {
> @@ -509,9 +523,8 @@ static MemTxResult memory_region_write_accessor(MemoryRegion *mr,
>                                                  uint64_t mask,
>                                                  MemTxAttrs attrs)
>  {
> -    uint64_t tmp;
> +    uint64_t tmp = memory_region_shift_write_access(value, shift, mask);
>  
> -    tmp = (*value >> shift) & mask;
>      if (mr->subpage) {
>          trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);
>      } else if (mr == &io_mem_notdirty) {
> @@ -535,9 +548,8 @@ static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,
>                                                             uint64_t mask,
>                                                             MemTxAttrs attrs)
>  {
> -    uint64_t tmp;
> +    uint64_t tmp = memory_region_shift_write_access(value, shift, mask);
>  
> -    tmp = (*value >> shift) & mask;
>      if (mr->subpage) {
>          trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size);
>      } else if (mr == &io_mem_notdirty) {
>

Re: [Qemu-devel] [PATCH 2/3] memory: Refactor common shifting code from accessors

Posted by Peter Maydell 7 years, 4 months ago

On 27 September 2018 at 01:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  memory.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
>

NB that this patch will have a (fairly trivial) conflict with
the "remove oldmmio accessors" patches, because it patches
a function that those patches delete.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/3] memory: Refactor common shifting code from accessors

Posted by Philippe Mathieu-Daudé 7 years, 4 months ago

On 9/27/18 9:00 AM, Peter Maydell wrote:
> On 27 September 2018 at 01:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  memory.c | 30 +++++++++++++++++++++---------
>>  1 file changed, 21 insertions(+), 9 deletions(-)
>>
> 
> NB that this patch will have a (fairly trivial) conflict with
> the "remove oldmmio accessors" patches, because it patches
> a function that those patches delete.

Yes, I thought about adding a comment about it ("to be respin once
Paolo's next PR lands") and then forgot :|

As you said this is here trivial, the meat of this series being patch #3.