* [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian @ 2017-08-11 14:06 Philippe Mathieu-Daudé 2017-08-11 14:25 ` Paolo Bonzini 0 siblings, 1 reply; 4+ messages in thread From: Philippe Mathieu-Daudé @ 2017-08-11 14:06 UTC (permalink / raw) To: Peter Maydell, KONRAD Frederic, Peter Crosthwaite, Paolo Bonzini Cc: Philippe Mathieu-Daudé, qemu-devel remove unnecessary memory_region_big_endian() Signed-off-by: Philippe Mathieu-Daudé <f4bug@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; } -- 2.13.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian 2017-08-11 14:06 [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian Philippe Mathieu-Daudé @ 2017-08-11 14:25 ` Paolo Bonzini 2017-08-11 14:49 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 4+ messages in thread From: Paolo Bonzini @ 2017-08-11 14:25 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Peter Maydell, KONRAD Frederic, Peter Crosthwaite Cc: qemu-devel On 11/08/2017 16:06, Philippe Mathieu-Daudé wrote: > remove unnecessary memory_region_big_endian() Can you explain why it's unnecessary?... Paolo > Signed-off-by: Philippe Mathieu-Daudé <f4bug@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; > } > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian 2017-08-11 14:25 ` Paolo Bonzini @ 2017-08-11 14:49 ` Philippe Mathieu-Daudé 2017-08-11 17:13 ` KONRAD Frederic 0 siblings, 1 reply; 4+ messages in thread From: Philippe Mathieu-Daudé @ 2017-08-11 14:49 UTC (permalink / raw) To: Paolo Bonzini, Peter Maydell, KONRAD Frederic, Peter Crosthwaite Cc: qemu-devel 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é <f4bug@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; >> } >> >> > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian 2017-08-11 14:49 ` Philippe Mathieu-Daudé @ 2017-08-11 17:13 ` KONRAD Frederic 0 siblings, 0 replies; 4+ messages in thread From: KONRAD Frederic @ 2017-08-11 17:13 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Paolo Bonzini, Peter Maydell, Peter Crosthwaite, qemu-devel Hi Philippe, Thanks for that, I'll try this out when I'm back. The 15th is blank holiday here. Thanks, Fred On 08/11/2017 04:49 PM, Philippe Mathieu-Daudé wrote: > 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é <f4bug@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; >>> } >>> >> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-08-11 17:14 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-11 14:06 [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian Philippe Mathieu-Daudé 2017-08-11 14:25 ` Paolo Bonzini 2017-08-11 14:49 ` Philippe Mathieu-Daudé 2017-08-11 17:13 ` KONRAD Frederic
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).