Re: [Qemu-devel] [RFC PATCH] memory: fix access_with_adjusted_size() on big-endian

[Philippe Mathieu-Daudé]

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é <address@hidden>
> > ---
> > 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;
> >   }
> >