Re: [Qemu-devel] [PATCH 4/8] Add universal DMA helper functions

[Michael S. Tsirkin]

On Mon, Feb 27, 2012 at 11:22:43AM +1100, David Gibson wrote:
> On Sun, Feb 26, 2012 at 12:04:49PM +0200, Michael S. Tsirkin wrote:
> > On Fri, Feb 24, 2012 at 02:27:39PM +1100, David Gibson wrote:
> > > Not that long ago, every device implementation using DMA directly
> > > accessed guest memory using cpu_physical_memory_*().  This meant that
> > > adding support for a guest visible IOMMU would require changing every
> > > one of these devices to go through IOMMU translation.
> > > 
> > > Shortly before qemu 1.0, I made a start on fixing this by providing
> > > helper functions for PCI DMA.  These are currently just stubs which
> > > call the direct access functions, but mean that an IOMMU can be
> > > implemented in one place, rather than for every PCI device.
> > > 
> > > Clearly, this doesn't help for non PCI devices, which could also be
> > > IOMMU translated on some platforms.  It is also problematic for the
> > > devices which have both PCI and non-PCI version (e.g. OHCI, AHCI) - we
> > > cannot use the the pci_dma_*() functions, because they assume the
> > > presence of a PCIDevice, but we don't want to have to check between
> > > pci_dma_*() and cpu_physical_memory_*() every time we do a DMA in the
> > > device code.
> > > 
> > > This patch makes the first step on addressing both these problems, by
> > > introducing new (stub) dma helper functions which can be used for any
> > > DMA capable device.
> > > 
> > > These dma functions take a DMAContext *, a new (currently empty)
> > > variable describing the DMA address space in which the operation is to
> > > take place.  NULL indicates untranslated DMA directly into guest
> > > physical address space.  The intention is that in future non-NULL
> > > values will given information about any necessary IOMMU translation.
> > > 
> > > DMA using devices must obtain a DMAContext (or, potentially, contexts)
> > > from their bus or platform.  For now this patch just converts the PCI
> > > wrappers to be implemented in terms of the universal wrappers,
> > > converting other drivers can take place over time.
> > > 
> > > Cc: Michael S. Tsirkin <address@hidden>
> > > Cc: Joerg Rodel <address@hidden>
> > > Cc: Eduard - Gabriel Munteanu <address@hidden>
> > > Cc: Richard Henderson <address@hidden>
> > > 
> > > Signed-off-by: David Gibson <address@hidden>
> > 
> > I'm a bit confused with all the stubbing going on.
> > Is this the final form of the pci_* functions or just
> > a stub? If the final form, we probably should just
> > open-code them - they don't buy us much.
> > If not, let's add a comment?
> 
> Well.. it's the intended final form of pci_dma_*() - which do become
> trivial wrappers, yes.

I'd say let's drop them then (in a follow-up patch).  The topic is
confusing enough without having to wade through layers of wrappers :)

> It's _not_ the intended final form of dma_*(),
> which need to grow code to do actual IOMMU translation.  I'll add a
> comment about this in the next round.
> 
> -- 
> 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