Re: [Qemu-devel] [PATCH v2 3/7] memory: iommu support

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

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.