Re: [Qemu-devel] [PATCH 1/7] virtio: allow byte swapping for vring and config access

[Rusty Russell]

Anthony Liguori <address@hidden> writes:
> "Daniel P. Berrange" <address@hidden> writes:
>
>> On Thu, Aug 08, 2013 at 10:40:28AM -0500, Anthony Liguori wrote:
>>> Andreas Färber <address@hidden> writes:
>>> >> We have a mechanism to do weak functions via stubs/.  I think it would
>>> >> be better to do cpu_get_byteswap() as a stub function and then overload
>>> >> it in the ppc64 code.
>>> >
>>> > If this as your name indicates is a per-CPU function then it should go
>>> > into CPUClass. Interesting question is, what is virtio supposed to do if
>>> > we have two ppc CPUs, one is Big Endian, the other is Little Endian.
>>> 
>>> PPC64 is big endian.  AFAIK, there is no such thing as a little endian
>>> PPC64 processor.
>>
>> Unless I'm misunderstanding, this thread seems to suggest otherwise:
>>
>>   "[Qemu-devel] [PATCH 0/5] 64bit PowerPC little endian support"
>>
>>   https://lists.nongnu.org/archive/html/qemu-devel/2013-08/msg00813.html
>
> Yeah, it's confusing.  It feels like little endian to most software but
> the distinction in hardware (and therefore QEMU) is important.
>
> It's the same processor.  It still starts executing big endian
> instructions.  A magic register value is tweaked and loads/stores are
> swapped.  CPU data structures are still read as big endian though.  It's
> really just load/stores that are affected.
>
> The distinction is important in QEMU.  ppc64 is still
> TARGET_WORDS_BIGENDIAN.  We still want most stl_phys to treat integers
> as big endian.  There's just this extra concept that CPU loads/stores
> are sometimes byte swapped.  That affects virtio but not a lot else.

You've redefined endian here; please don't do that.  Endian is the order
in memory which a CPU does loads and stores.  From any reasonable
definition, PPC is bi-endian.

It's actually a weird thing for the qemu core to know at all: almost
everything which cares is in target-specific code.  The exceptions are
gdb stubs and virtio, both of which are "native endian" (and that weird
code in exec.c: what is notdirty_mem_write?).

Your argument that we shouldn't fix stl_* might be justifiable (ie. just
hack virtio and gdb as one-offs), but it's neither clear nor "least
surprise".

Chers,
Rusty.