Re: [Qemu-devel] [PATCH v2] memory: Introduce DEVICE_HOST_ENDIAN for ram device

[Yongji Xie <xyjxie@linux.vnet.ibm.com>]

on 2017/2/28 8:41, David Gibson wrote:

> On Mon, Feb 27, 2017 at 12:52:44PM +0800, Yongji Xie wrote:
>> At the moment ram device's memory regions are DEVICE_NATIVE_ENDIAN. It's
>> incorrect. This memory region is backed by a MMIO area in host, so the
>> uint64_t data that MemoryRegionOps read from/write to this area should be
>> host-endian rather than target-endian. Hence, current code does not work
>> when target and host endianness are different which is the most common case
>> on PPC64. To fix it, this introduces DEVICE_HOST_ENDIAN for the ram device.
>>
>> This has been tested on PPC64 BE/LE host/guest in all possible combinations
>> including TCG.
>>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>
> The effect of the patch is certainly correct.  I remain a little
> concerned that the name "host endian" might cause more confusion than
> it resolves, but a better term isn't immediately obvious to me.

If the memory region's endianness indicates the endianness of multi-byte 
value that
MemoryRegionOps read from/write to this memory region, should "host endian"
be reasonable?

For a mmio store, QEMU just get a bunch of bytes in the memory at the 
beginning.
Then we use ldX_p to load a target-endian multi-byte value from the 
memory.  Then
adjust_endianness() change the endianness of the multi-byte value from 
target-endian
to memory region's endianness.

For the mmap MMIO area, we should use host-endian multi-byte value to 
access it.

*(uint32_t *)(mr->ram_block->host + addr) = (uint32_t)data;

Here it is the same as stl_he_p().

The "host-endian" means we load a bunch of bytes as a host-endian value, 
and write the
value to the mmap MMIO area. That's my understanding. Not sure if it's 
correct.

Thanks,
Yongji