Re: [Qemu-devel] [PATCH 1/9] Add stub functions for PCI device models to do PCI DMA

[Michael S. Tsirkin]

On Sun, Oct 02, 2011 at 12:29:08PM +0200, Avi Kivity wrote:
> On 10/02/2011 12:25 PM, Michael S. Tsirkin wrote:
> >On Mon, Sep 05, 2011 at 02:34:56PM +1000, David Gibson wrote:
> >>  This patch adds functions to pci.[ch] to perform PCI DMA operations.  At
> >>  present, these are just stubs which perform directly cpu physical memory
> >>  accesses.
> >>
> >>  Using these stubs, however, distinguishes PCI device DMA transactions from
> >>  other accesses to physical memory, which will allow PCI IOMMU support to
> >>  be added in one place, rather than updating every PCI driver at that time.
> >>
> >>  That is, it allows us to update individual PCI drivers to support an IOMMU
> >>  without having yet determined the details of how the IOMMU emulation will
> >>  operate.  This will let us remove the most bitrot-sensitive part of an
> >>  IOMMU patch in advance.
> >>
> >>  Signed-off-by: David Gibson<address@hidden>
> >
> >So something I just thought about:
> >
> >all wrappers now go through cpu_physical_memory_rw.
> >This is a problem as e.g. virtio assumes that
> >accesses such as stw are atomic. cpu_physical_memory_rw
> >is a memcpy which makes no such guarantees.
> >
> 
> Let's change cpu_physical_memory_rw() to provide that guarantee for
> aligned two and four byte accesses.  Having separate paths just for
> that is not maintainable.

Well, we also have stX_phys convert to target native endian-ness
(nop for KVM but not necessarily for qemu).

So if we do what you suggest, this patch will become more correct, but
it would still need to duplicate the endian-ness work.

For that reason, I think calling stX_phys and friends from pci
makes more sense - we get more simple inline wrappers
but that code duplication worries me much less than tricky
endian-ness hidden within a macro.

> -- 
> error compiling committee.c: too many arguments to function