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

[David Gibson]

On Wed, Nov 02, 2011 at 09:17:37AM +0200, Michael S. Tsirkin wrote:
> On Mon, Oct 31, 2011 at 05:06:47PM +1100, 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.  Stubs are included which are analogous to
> > cpu_physical_memory_{read,write}(), the stX_phys() and ldX_phys()
> > functions and cpu_physical_memory_{map,unmap}().
> > 
> > In addition, a wrapper around qemu_sglist_init() is provided, which
> > also takes a PCIDevice *.  It's assumed that _init() is the only
> > sglist function which will need wrapping, the idea being that once we
> > have IOMMU support whatever IOMMU context handle the wrapper derives
> > from the PCI device will be stored within the sglist structure for
> > later use.
> > 
> > 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>
> > Signed-off-by: Alexey Kardashevskiy <address@hidden>
> > ---
> >  hw/pci.h |   67 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 67 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/pci.h b/hw/pci.h
> > index 86a81c8..c449a90 100644
> > --- a/hw/pci.h
> > +++ b/hw/pci.h
> > @@ -6,6 +6,7 @@
> >  
> >  #include "qdev.h"
> >  #include "memory.h"
> > +#include "dma.h"
> >  
> >  /* PCI includes legacy ISA access.  */
> >  #include "isa.h"
> > @@ -487,4 +488,70 @@ static inline uint32_t pci_config_size(const PCIDevice 
> > *d)
> >      return pci_is_express(d) ? PCIE_CONFIG_SPACE_SIZE : 
> > PCI_CONFIG_SPACE_SIZE;
> >  }
> >  
> > +/* DMA access functions */
> > +static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr,
> > +                             void *buf, dma_addr_t len, DMADirection dir)
> > +{
> > +    cpu_physical_memory_rw(addr, buf, len, dir == 
> > DMA_DIRECTION_FROM_DEVICE);
> > +    return 0;
> > +}
> > +
> > +static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr,
> > +                               void *buf, dma_addr_t len)
> > +{
> > +    return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE);
> > +}
> > +
> > +static inline int pci_dma_write(PCIDevice *dev, dma_addr_t addr,
> > +                                const void *buf, dma_addr_t len)
> > +{
> > +    return pci_dma_rw(dev, addr, (void *) buf, len, 
> > DMA_DIRECTION_FROM_DEVICE);
> > +}
> > +
> > +#define PCI_DMA_DEFINE_LDST(_l, _s, _bits)                              \
> > +    static inline uint##_bits##_t ld##_l##_pci_dma(PCIDevice *dev,      \
> > +                                                   dma_addr_t addr)     \
> > +    {                                                                   \
> > +        return ld##_l##_phys(addr);                                     \
> > +    }                                                                   \
> > +    static inline void st##_s##_pci_dma(PCIDevice *dev,                 \
> > +                          dma_addr_t addr, uint##_bits##_t val)         \
> > +    {                                                                   \
> > +        st##_s##_phys(addr, val);                                       \
> > +    }
> > +
> 
> I'd much rather name APIs pci_dma_XXX.
> why use a suffix and not a prefix?

Because the analogous stX_phys/ldX_phys functions do.

-- 
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