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
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; > } > >
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; >> } >> >> >
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; >>> } >>> >>
© 2016 - 2024 Red Hat, Inc.