On 17/07/19 08:06, tony.ngu...@bt.com 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