Re: [PATCH for-7.0 v2 1/2] ivshmem.c: change endianness to LITTLE_ENDIAN

[Daniel Henrique Barboza <danielhb413@gmail.com>]

On 11/24/21 08:28, Philippe Mathieu-Daudé wrote:
> On 11/24/21 10:29, Daniel Henrique Barboza wrote:
>> The ivshmem device, as with most PCI devices, uses little endian byte
>> order. However, the endianness of its mmio_ops is marked as
>> DEVICE_NATIVE_ENDIAN. This presents not only the usual problems with big
>> endian hosts but also with PowerPC little endian hosts as well, since
>> the Power architecture in QEMU uses big endian hardware (XIVE controller,
>> PCI Host Bridges, etc) even if the host is in little endian byte order.
> 
> Maybe mention commit f7a199b2b44 ("ivshmem: use little-endian
> int64_t for the protocol")?


Good point. I'll send a v3.

> 
>> As it is today, the IVPosition of the device will be byte swapped when
>> running in Power BE and LE. This can be seen by changing the existing
>> qtest 'ivshmem-test' to run in ppc64 hosts and printing the IVPOSITION
>> regs in test_ivshmem_server() right after the VM ids assert. For x86_64
>> the VM id values read are '0' and '1', for ppc64 (tested in a Power8
>> RHEL 7.9 BE server) and ppc64le (tested in a Power9 RHEL 8.6 LE server)
>> the ids will be '0' and '0x1000000'.
>>
>> Change this device to LITTLE_ENDIAN fixes the issue for Power hosts of
>> both endianness, and every other big-endian architecture that might use
>> this device, without impacting x86 users.
>>
>> Fixes: cb06608e17f8 ("ivshmem: convert to memory API")
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/168
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   hw/misc/ivshmem.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
>> index 1ba4a98377..299837e5c1 100644
>> --- a/hw/misc/ivshmem.c
>> +++ b/hw/misc/ivshmem.c
>> @@ -243,7 +243,7 @@ static uint64_t ivshmem_io_read(void *opaque, hwaddr addr,
>>   static const MemoryRegionOps ivshmem_mmio_ops = {
>>       .read = ivshmem_io_read,
>>       .write = ivshmem_io_write,
>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>       .impl = {
>>           .min_access_size = 4,
>>           .max_access_size = 4,
>>
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


Thanks!


Daniel

>