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

[Michael S. Tsirkin]

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?

> +PCI_DMA_DEFINE_LDST(ub, b, 8);
> +PCI_DMA_DEFINE_LDST(uw_le, w_le, 16)
> +PCI_DMA_DEFINE_LDST(l_le, l_le, 32);
> +PCI_DMA_DEFINE_LDST(q_le, q_le, 64);
> +PCI_DMA_DEFINE_LDST(uw_be, w_be, 16)
> +PCI_DMA_DEFINE_LDST(l_be, l_be, 32);
> +PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
> +
> +#undef PCI_DMA_DEFINE_LDST
> +
> +static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
> +                                dma_addr_t *plen, DMADirection dir)
> +{
> +    target_phys_addr_t len = *plen;
> +    void *buf;
> +
> +    buf = cpu_physical_memory_map(addr, &len, dir == 
> DMA_DIRECTION_FROM_DEVICE);
> +    *plen = len;
> +    return buf;
> +}
> +
> +static inline void pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t 
> len,
> +                                 DMADirection dir, dma_addr_t access_len)
> +{
> +    cpu_physical_memory_unmap(buffer, len, dir == DMA_DIRECTION_FROM_DEVICE,
> +                              access_len);
> +}
> +
> +static inline void pci_dma_sglist_init(QEMUSGList *qsg, PCIDevice *dev,
> +                                       int alloc_hint)
> +{
> +    qemu_sglist_init(qsg, alloc_hint);
> +}
> +
>  #endif
> -- 
> 1.7.7