| Message ID | 20180927002416.1781-3-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix access_with_adjusted_size() on big-endian | expand |
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) { >
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
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.
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) {
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- memory.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-)