On Wed, 2012-10-31 at 12:32 +0200, Avi Kivity wrote: > This has nothing to do with device endianness; we're translating from a > byte buffer (address_space_rw()) to a uint64_t > (MemoryRegionOps::read/write()) so we need to take host endianess into > account. > > This code cleverly makes use of memory core endian support to do the > conversion for us. But it's probably too clever and should be replaced > by an explicit call to a helper.
Well, the whole idea of providing sized-type accessors (u8/u16/...) is very fishy for DMA. I understand it comes from the MemoryRegion ops, and It made somewhat sense in CPU to device (though even then endianness had always gotten wrong) but for DMA it's going to be a can of worms. Taking "host endianness" into account means nothing if you don't define what endianness those are expected to use to write to memory. If you add yet another case of "guest endianness" in the mix, then you make yet another mistake akin to the virtio trainwreck since things like powerpc or ARM can operate in different endianness. The best thing to do is to forbid use of those functions :-) And basically mandate the use of endian explicit accessor that specifically indicate what endianness the value should have once written in target memory. The most sensible expectation when using the "raw" Memory ops is to have the value go "as is" ie in host endianness. Cheers, Ben.
