On Sun, Oct 02, 2011 at 02:14:28PM +0200, Michael S. Tsirkin wrote:
> On Sun, Oct 02, 2011 at 02:01:10PM +0200, Avi Kivity wrote:
> > On 10/02/2011 01:17 PM, Michael S. Tsirkin wrote:
> > >On Sun, Oct 02, 2011 at 12:58:35PM +0200, Avi Kivity wrote:
> > >> On 10/02/2011 12:52 PM, Michael S. Tsirkin wrote:
> > >> >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<da...@gibson.dropbear.id.au>
> > >> >> >
> > >> >> >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.
> > >> >
> > >>
> > >> Good point. Though this is really a virtio specific issue since
> > >> other devices have explicit endianness (not guest dependent).
> > >
> > >Hmm, not entirely virtio specific, some devices use stX macros to do the
> > >conversion. E.g. stw_be_phys and stl_le_phys are used in several
> > >places.
> >
> > These are fine - explicit endianness.
>
> Right. So changing these to e.g. stl_dma and assuming
> LE is default seems like a step backwards.
Um.. why? PCI is defined by the spec to be LE, so I don't see that we
need explicit endianness versions for PCI helpers.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
