Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 11/14] ioport: Switch dispatching to memory core layer

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Fri, 2013-07-12 at 19:26 +0100, Peter Maydell wrote:

> It's not wrong when the hardware actually does a byteswap at
> some level in the memory hierarchy. You can see this for instance
> on ARMv7M systems, where byteswapping for bigendian happens at
> an intermediate level that not all accesses go through:
> 
>  [CPU] ---->  [byteswap here] --> [memory and ext. devices]
>          |
>          -->  [internal memory mapped devices]
> 
> so some things see always little endian regardless.

Ugh ? That's so completely fucked up, if that's indeed what the HW is
doing this is a piece of trash and the designers are in urgent need of
being turned into fertilizer.

Unless again you are talking about "lane swapping" which allows to
preserve the byte address invariance when the CPU decides to flip its
bus around, but I would have thought that modern CPUs do not do that
sort of shit anymore.

In any case, it cannot be represented with an "endian" attribute at the
bridge level, that doesn't mean anything. Again, the only endian
attribute that exists are the byte order of the original access (which
byte has the lowest address, regardless of significance of those bytes
in the target, ie, purely from a qemu standpoint, in the variable that
carries the access around inside qemu, which byte has the lowest
address), and the same on the target device (at which point a concept of
significance does apply, but it's a guest driver business to get it
right, qemu just need to make sure byte 0 goes to byte 0).

If a bridge flips things around in a way that breaks the model, then add
some property describing the flipping properties but don't call it "big
endian" or "little endian" at the bridge level, that has no meaning,
confuses things and introduces breakage like we have seen.

> >> The whole business of endianness in qemu is a mess. In the end what
> >> matters and the only thing that does is:
> >
> > It's not as bad as you think I suspect.
> >
> >>  * The endianness of a given memory access by the guest (which may or
> >> may not be the endianness of the guest -> MSR:LE, byteswap load/store
> >> instsructions, etc..)
> >
> > Correct.
> >
> >> vs.
> >>
> >>  * The endianness of the target device register (and I say register ...
> >> a framebuffer does NOT have endianness per-se and thus accesses to BAR
> >> mapping a "memory" range (framebuffer, ROM, ...) should go such that the
> >> *byte order* of individual bytes is preserved, which typically means
> >> untranslated).
> >
> > Yes.  To put it another way, an MMIO write is a store and depending on
> > the VCPU, that will result in a write with a certain byte order.  That
> > byte order should be preserved.
> >
> > However, what we don't model today, and why we have the silly
> > endianness in MemoryRegionOps, is the fact that I/O may pass through
> > multiple layers and those layers may change byte ordering.
> >
> > We jump through great hoops to have a flat dispatch table.  I've never
> > liked it but that's what we do.  That means that in cases where a host
> > bridge may do byte swapping, we cannot easily support that.
> 
> We could support that if we cared to -- you just have to have a
> container MemoryRegion type which is a byte-swapping container
> (or just have a flag on existing containers, I suppose).
> Then as you flatten the regions into the flat table you keep
> track of how many levels of byteswapping each region goes through,
> and you end up with a single 'byteswap or not?' flag for each
> section of your flat dispatch table.
> 
> (Our other serious endianness problem is that we don't really
> do very well at supporting a TCG CPU arbitrarily flipping
> endianness -- TARGET_WORDS_BIGENDIAN is a compile time setting
> and ideally it should not be.)

Our experience is that it actually works fine for almost everything
except virtio :-) ie mostly TARGET_WORDS_BIGENDIAN is irrelevant (and
should be).

Cheers,
Ben.