Re: [PATCH v2] KVM: Specify byte order for KVM_EXIT_MMIO

[Scott Wood <scottwood@freescale.com>]

On Sat, 2014-01-25 at 03:15 +0100, Alexander Graf wrote:
> Ok, let's go through the combinations for a 32-bit write of 0x01020304 on PPC and what data[] looks like
> 
> your proposal:
> 
>   BE guest, BE host: { 0x01, 0x02, 0x03, 0x04 }
>   LE guest, BE host: { 0x04, 0x03, 0x02, 0x01 }
>   BE guest, LE host:  { 0x01, 0x02, 0x03, 0x04 }
>   LE guest, LE host:  { 0x04, 0x03, 0x02, 0x01 }
> 
> -> ldw_p() will give us the correct value to work with

Do you mean ldl_p(), which just does a memcpy()?  How is that different
from *(uint32_t*)data, other than complying with C99 aliasing rules?

What you get from doing ldl_p() is the value as interpreted by the host
endianness.  If the memory region endianness is different from the host
endianness, you need to swap.  adjust_endianness() sort of does this,
but it's using TARGET_WORDS_BIGENDIAN instead of the host endianness
which will break if they are not the same.  It works for TCG because it
passes the MMIO value in target endianness rather than host --
effectively the TARGET_WORDS_BIGENDIAN usages in memory.c and
include/exec/softmmu_template.h cancel each other.

> current proposal:
> 
>   BE guest, BE host: { 0x01, 0x02, 0x03, 0x04 }
>   LE guest, BE host: { 0x04, 0x03, 0x02, 0x01 }
>   BE guest, LE host:  { 0x04, 0x03, 0x02, 0x01 }
>   LE guest, LE host:  { 0x01, 0x02, 0x03, 0x04 }
> 
> -> *(uint32_t*)data will give us the correct value to work with

*(uint32_t*)data in the "LE guest, LE host" case will get you 0x04030201
-- how is that the correct value to work with?  Did you make this table
with the assumption that big-endian interpretation is inherently "the
correct value" on PPC?

> There are pros and cons for both approaches.
> 
> Pro approach 1 is that it fits the way data[] is read today, so no QEMU
> changes are required. However, it means that user space needs to have
> awareness of the "default endianness".

Normally the endianness is well specified by the particular device/bus
(PCI is little-endian, e500 CCSR is big-endian, etc).

I suppose you could define one endianness as "natural" for a particular
platform, and the other as swapped, but I don't think that's a useful
way to look at it except for things like virtio that are defined as
being native-endian (and I don't think KVM_EXIT_MMIO is the right layer
to try to sort that problem out, as it just introduces confusion for
non-virtio MMIO).

Unfortunately, that appears to currently be how QEMU works, so either
QEMU needs to be fixed in a way that is compatible with currently
working scenarios (e.g. by changing adjust_endianness() to use host
endianness and changing the TCG path to match), or it needs to be
explicitly defined in the API what assumptions are being made regarding
the default endianness on each architecture.  I believe these
assumptions are present but implicit in the current proposal (if Alex's
intepretation of it matches what was intended).

> With approach 2 you don't care about endianness at all anymore - you
> just get a payload that the host process can read in.

You always need to care about endianness -- something has to result in
PCI registers appearing swapped compared to e500 CCSR registers.

-Scott