[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/7] memory: iommu support
From: |
Avi Kivity |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/7] memory: iommu support |
Date: |
Thu, 01 Nov 2012 15:44:19 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121016 Thunderbird/16.0.1 |
On 10/31/2012 08:59 PM, Benjamin Herrenschmidt wrote:
> 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.
Endianness here has no effect on the result. An address_space_rw()
causes a lookup of a memory region, which happens to be an iommu memory
region. Because of the way MemoryRegionOps are done, it is converted to
a 1/2/4/8 accessor, and then converted immediately back to a byte array.
As long as we're consistent there's no change to the data path.
However we do have a problem with non-1/2/4/8 byte writes. Right now
any mismatched access ends up as an 8 byte write, we need an extra
accessor for arbitrary writes, or rather better use of the .impl members
of MemoryRegionOps.
>
> 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.
--
error compiling committee.c: too many arguments to function
- Re: [Qemu-devel] [PATCH v2 3/7] memory: iommu support,
Avi Kivity <=