On 23/02/2017 16:29, Peter Maydell wrote: >> No, they look entirely the same. The only difference is that they go >> through MemoryRegionOps instead of memcpy. > Then we have a different problem, because the thing this patch > is claiming to fix is that the memory the device is backed by > (from vfio) is little-endian and we're not accessing it right. > > RAM of the usual sort is target-endian (by which I mean "when the guest > does a write of 32-bits 0x12345678, and you look at the memory byte > by byte then the order of bytes is either 0x12 0x34 0x56 0x78 if > TARGET_LITTLE_ENDIAN or 0x78 0x56 0x34 0x12 if TARGET_BIG_ENDIAN").
Okay, I think I see what you mean. MMIO regions do not expect their datum in any endianness; they just get a uint64_t representing an 8/16/32/64-bit value. The datum undergoes a conversion if necessary, according to target and MemoryRegionOps endiannesses, before the callback is invoked. In the ramd case, the ops convert between bunch-of-bytes-stored-into-RAM-by-host and uint64_t (to uint64_t for read, from uint64_t for write). The conversion from/to target endianness has been done already by the memory.c core code. The ops' task is to ensure that this bunch of bytes has the exact order that the guest desired. There's more than one way to do it---all that's needed is that the order used by the ops matches the one specified by the ops' endianness, resulting in an even number of swaps. However, DEVICE_NATIVE_ENDIAN would have to be paired with tswap, which the current code does not do, hence the bug. To have no swap at all, you'd need DEVICE_HOST_ENDIAN. > AIUI what we want for this VFIO case is "when the guest does > a 32-bit write of 0x12345678 then the bytes are 0x12 0x34 0x56 0x78 > regardless of whether TARGET_BIG_ENDIAN or not". No, I don't think so. This is not specific to VFIO. You can do it with any device, albeit VFIO is currently the only one using ramd regions. Paolo
