From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42593) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TTdVn-0004Lk-J4 for qemu-devel@nongnu.org; Wed, 31 Oct 2012 14:59:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TTdVj-00046r-Gz for qemu-devel@nongnu.org; Wed, 31 Oct 2012 14:59:35 -0400 Received: from gate.crashing.org ([63.228.1.57]:47329) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TTdVj-000460-7p for qemu-devel@nongnu.org; Wed, 31 Oct 2012 14:59:31 -0400 Message-ID: <1351709961.12271.206.camel@pasglop> From: Benjamin Herrenschmidt Date: Thu, 01 Nov 2012 05:59:21 +1100 In-Reply-To: <5090FE22.9080403@redhat.com> References: <1351597670-23031-1-git-send-email-avi@redhat.com> <1351597670-23031-4-git-send-email-avi@redhat.com> <5090FE22.9080403@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Mime-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v2 3/7] memory: iommu support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: "Michael S. Tsirkin" , qemu-devel@nongnu.org, Alexander Graf , Blue Swirl , Alex Williamson , Anthony Liguori 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.