Re: [Qemu-devel] RFC: reverse-endian softmmu memory accessors

[J. Mayer]

On Mon, 2007-10-15 at 19:02 +0300, Blue Swirl wrote:
> On 10/15/07, J. Mayer <address@hidden> wrote:
> > On Sun, 2007-10-14 at 15:59 +0300, Blue Swirl wrote:
> > > On 10/14/07, J. Mayer <address@hidden> wrote:
> > > > Here's an updated version of the patch against current CVS.
> > > > This patches provides reverse-endian, little-endian and big-endian
> > > > memory accessors, available with and without softmmu. It also provides
> > > > an IO_MEM_REVERSE TLB flag to allow future support of per-page
> > > > endianness control, which is required by some targets CPU emulations.
> > > > Having reverse-endian memory accessors also make it possible to optimise
> > > > reverse-endian memory access when the target CPU has dedicated
> > > > instructions. For now, it includes optimisations for the PowerPC target.
> > >
> > > This breaks Sparc32 softmmu, I get a black screen. Your changes to
> > > target-sparc and hw/sun4m.c look fine, so the problem could be in IO?
> >
> > Did it worked before my commits ? I may have done something wrong during
> > the merge...
> > I will do more checks and more tests...
> 
> If I disable the IOSWAP code, black screen is gone. I think this is
> logical: the io accessors return host CPU values, therefore no byte
> swapping need to be performed.

Memory mapped I/O access function hopefully return data in the target
endianness.
This is the reason why there are so many #ifdef TARGET_WORDS_BIGENDIAN
in the emulated devices memory mapped accesses routines and also in
io_read and io_write functions for 64 bits accesses.
And the emulated CPU is expecting data to always come in its endiannes
when doing a "load from memory", even if the access is a device one.

Your patch works as long as you don't use load/store with reverse endian 
accessor routines nor TLB wih reverse endian bit set.
On PowerPC, using reverse-endian load and stores, the byteswap in I/O routines 
is needed for most MMIO device accesses (like IDE, which always returns 
little-endian data) could ever be accessed.
The bug you report just means there's a logical error somewhere in my code. I 
did download the Sparc test and was able to reproduce it. I'm working to find 
the bug.
And I finally found it. The bug is just that I did something completelly 
stupid, defining IO_MEM_REVERSE as 3 instead of 4: it's obvious that it has to 
be a power of 2 to be combined with the other TB bits. I wonder how the PowerPC 
case was able to run with such a huge bug... Please apologive.
I'm going to do more test with this fix and try to merge the 
sparc_reverse_endian in my code and repost an updated patch.

-- 
J. Mayer <address@hidden>
Never organized