Re: [Qemu-devel] [PATCH 2/4] memory: Single byte swap along the I/O path

[Paolo Bonzini]

On 17/07/19 08:06, address@hidden wrote:
> +
> +static inline MemOp MEMOP(unsigned size)
> +{
> +    switch (size) {
> +    case 1:
> +        return MO_8;
> +    case 2:
> +        return MO_16;
> +    case 4:
> +        return MO_32;
> +    case 8:
> +        return MO_64;
> +    default:
> +        g_assert_not_reached();
> +    }
> +}

FWIW, this is just "ctz(size)".

My main concern is that MO_BE/MO_LE/MO_TE do not really apply to the 
memory.c paths.  MO_BSWAP is never passed into the MemOp, even if target 
endianness != host endianness.

Therefore, you could return MO_TE | MO_{8,16,32,64} from this function, 
and change memory_region_endianness_inverted to test 
HOST_WORDS_BIGENDIAN instead of TARGET_WORDS_BIGENDIAN.  Then the two
MO_BSWAPs (one from MO_TE, one from adjust_endianness because
memory_region_endianness_inverted returns true) cancel out if the
memory region's endianness is the same as the host's but different
from the target's.

Some care is needed in virtio_address_space_write and zpci_write_bar.  I 
think the latter is okay, while the former could do something like this:

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ce928f2429..61885f020c 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -541,16 +541,16 @@ void virtio_address_space_write(VirtIOPCIProxy *proxy, 
hwaddr addr,
         val = pci_get_byte(buf);
         break;
     case 2:
-        val = cpu_to_le16(pci_get_word(buf));
+        val = pci_get_word(buf);
         break;
     case 4:
-        val = cpu_to_le32(pci_get_long(buf));
+        val = pci_get_long(buf);
         break;
     default:
         /* As length is under guest control, handle illegal values. */
         return;
     }
-    memory_region_dispatch_write(mr, addr, val, len, MEMTXATTRS_UNSPECIFIED);
+    memory_region_dispatch_write(mr, addr, val, size_memop(len) & ~MO_BSWAP, 
MEMTXATTRS_UNSPECIFIED);
 }
 
 static void


Paolo