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

[Philippe Mathieu-Daudé]

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")?

> 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>