Patchwork [05/13] iommu: Add universal DMA helper functions

login
register
mail settings
Submitter David Gibson
Date March 9, 2012, 5:01 a.m.
Message ID <1331269308-22372-6-git-send-email-david@gibson.dropbear.id.au>
Download mbox | patch
Permalink /patch/145658/
State New
Headers show

Comments

David Gibson - March 9, 2012, 5:01 a.m.
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 <mst@redhat.com>
Cc: Joerg Rodel <Joerg.Rodel@amd.com>
Cc: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
Cc: Richard Henderson <rth@twiddle.net>

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 dma.h         |  103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/pci.h      |   22 +++++++-----
 qemu-common.h |    1 +
 3 files changed, 117 insertions(+), 9 deletions(-)
Paolo Bonzini - March 9, 2012, 10:06 a.m.
Il 09/03/2012 06:01, David Gibson ha scritto:
> +static inline int dma_memory_read(DMAContext *dma, dma_addr_t addr,
> +                                  void *buf, dma_addr_t len)
> +{
> +    return dma_memory_rw(dma, addr, buf, len, DMA_DIRECTION_TO_DEVICE);
> +}
> +
> +static inline int dma_memory_write(DMAContext *dma, dma_addr_t addr,
> +                                   const void *buf, dma_addr_t len)
> +{
> +    return dma_memory_rw(dma, addr, (void *)buf, len,
> +                         DMA_DIRECTION_FROM_DEVICE);
> +}

This is opposite to the convention of dma_buf_read/dma_buf_write, which
is from the point of view of the device.  Either way is okay for me,
including removing dma_buf_read/dma_buf_write altogether and using
dma_buf_rw in the few users (so that there will be an explicit
DMADirection argument that clears all confusion).

Paolo
David Gibson - March 9, 2012, 11:19 a.m.
On Fri, Mar 09, 2012 at 11:06:41AM +0100, Paolo Bonzini wrote:
> Il 09/03/2012 06:01, David Gibson ha scritto:
> > +static inline int dma_memory_read(DMAContext *dma, dma_addr_t addr,
> > +                                  void *buf, dma_addr_t len)
> > +{
> > +    return dma_memory_rw(dma, addr, buf, len, DMA_DIRECTION_TO_DEVICE);
> > +}
> > +
> > +static inline int dma_memory_write(DMAContext *dma, dma_addr_t addr,
> > +                                   const void *buf, dma_addr_t len)
> > +{
> > +    return dma_memory_rw(dma, addr, (void *)buf, len,
> > +                         DMA_DIRECTION_FROM_DEVICE);
> > +}
> 
> This is opposite to the convention of dma_buf_read/dma_buf_write, which
> is from the point of view of the device.

Uh, these are from the point of view of the device - the device writes
to memory == transfer from device.  More importantly, this is the same
convention as cpu_physical_memory_{read,write}() which is what these
generally replace.  dma_buf_read() is from the point of view of the
core/memory.
Paolo Bonzini - March 9, 2012, 11:55 a.m.
Il 09/03/2012 12:19, David Gibson ha scritto:
>> > 
>> > This is opposite to the convention of dma_buf_read/dma_buf_write, which
>> > is from the point of view of the device.
> Uh, these are from the point of view of the device - the device writes
> to memory == transfer from device.

Right, dma_buf_read = "read from device".  The idea with dma_buf_read
was to make it the same as dma_bdrv_read().

> More importantly, this is the same
> convention as cpu_physical_memory_{read,write}() which is what these
> generally replace.

Understood.

My point was not really to bikeshed, just to note that probably
dma_buf_read/dma_buf_write become very confusing now.  So it is best to
remove them and just replace it with dma_buf_rw and a DMADirection
argument).  Can you do that?

Thanks,

Paolo

Patch

diff --git a/dma.h b/dma.h
index 463095c..8b6ef44 100644
--- a/dma.h
+++ b/dma.h
@@ -35,6 +35,109 @@  typedef target_phys_addr_t dma_addr_t;
 #define DMA_ADDR_BITS TARGET_PHYS_ADDR_BITS
 #define DMA_ADDR_FMT TARGET_FMT_plx
 
+typedef void DMAInvalidateMapFunc(void *);
+
+/* Checks that the given range of addresses is valid for DMA.  This is
+ * useful for certain cases, but usually you should just use
+ * dma_memory_{read,write}() and check for errors */
+static inline bool dma_memory_valid(DMAContext *dma, dma_addr_t addr,
+                                    dma_addr_t len, DMADirection dir)
+{
+    /* Stub version, with no iommu we assume all bus addresses are valid */
+    return true;
+}
+
+static inline int dma_memory_rw(DMAContext *dma, dma_addr_t addr,
+                                void *buf, dma_addr_t len, DMADirection dir)
+{
+    /* Stub version when we have no iommu support */
+    cpu_physical_memory_rw(addr, buf, (target_phys_addr_t)len,
+                           dir == DMA_DIRECTION_FROM_DEVICE);
+    return 0;
+}
+
+static inline int dma_memory_read(DMAContext *dma, dma_addr_t addr,
+                                  void *buf, dma_addr_t len)
+{
+    return dma_memory_rw(dma, addr, buf, len, DMA_DIRECTION_TO_DEVICE);
+}
+
+static inline int dma_memory_write(DMAContext *dma, dma_addr_t addr,
+                                   const void *buf, dma_addr_t len)
+{
+    return dma_memory_rw(dma, addr, (void *)buf, len,
+                         DMA_DIRECTION_FROM_DEVICE);
+}
+
+static inline int dma_memory_zero(DMAContext *dma, dma_addr_t addr,
+                                  dma_addr_t len)
+{
+    /* Stub version when we have no iommu support */
+    cpu_physical_memory_zero(addr, len);
+    return 0;
+}
+
+static inline void *dma_memory_map(DMAContext *dma,
+                                   DMAInvalidateMapFunc *cb, void *opaque,
+                                   dma_addr_t addr, dma_addr_t *len,
+                                   DMADirection dir)
+{
+    target_phys_addr_t xlen = *len;
+    void *p;
+
+    p = cpu_physical_memory_map(addr, &xlen,
+                                dir == DMA_DIRECTION_FROM_DEVICE);
+    *len = xlen;
+    return p;
+}
+
+static inline void dma_memory_unmap(DMAContext *dma,
+                                    void *buffer, dma_addr_t len,
+                                    DMADirection dir, dma_addr_t access_len)
+{
+    return cpu_physical_memory_unmap(buffer, (target_phys_addr_t)len,
+                                     dir == DMA_DIRECTION_FROM_DEVICE,
+                                     access_len);
+}
+
+#define DEFINE_LDST_DMA(_lname, _sname, _bits, _end) \
+    static inline uint##_bits##_t ld##_lname##_##_end##_dma(DMAContext *dma, \
+                                                            dma_addr_t addr) \
+    {                                                                   \
+        uint##_bits##_t val;                                            \
+        dma_memory_read(dma, addr, &val, (_bits) / 8);                  \
+        return _end##_bits##_to_cpu(val);                               \
+    }                                                                   \
+    static inline void st##_sname##_##_end##_dma(DMAContext *dma,       \
+                                                 dma_addr_t addr,       \
+                                                 uint##_bits##_t val)   \
+    {                                                                   \
+        val = cpu_to_##_end##_bits(val);                                \
+        dma_memory_write(dma, addr, &val, (_bits) / 8);                 \
+    }
+
+static inline uint8_t ldub_dma(DMAContext *dma, dma_addr_t addr)
+{
+    uint8_t val;
+
+    dma_memory_read(dma, addr, &val, 1);
+    return val;
+}
+
+static inline void stb_dma(DMAContext *dma, dma_addr_t addr, uint8_t val)
+{
+    dma_memory_write(dma, addr, &val, 1);
+}
+
+DEFINE_LDST_DMA(uw, w, 16, le);
+DEFINE_LDST_DMA(l, l, 32, le);
+DEFINE_LDST_DMA(q, q, 64, le);
+DEFINE_LDST_DMA(uw, w, 16, be);
+DEFINE_LDST_DMA(l, l, 32, be);
+DEFINE_LDST_DMA(q, q, 64, be);
+
+#undef DEFINE_LDST_DMA
+
 struct ScatterGatherEntry {
     dma_addr_t base;
     dma_addr_t len;
diff --git a/hw/pci.h b/hw/pci.h
index 4f19fdb..c021805 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -545,10 +545,16 @@  static inline uint32_t pci_config_size(const PCIDevice *d)
 }
 
 /* DMA access functions */
+static inline DMAContext *pci_dma_context(PCIDevice *dev)
+{
+    /* Stub for when we have no PCI iommu support */
+    return NULL;
+}
+
 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);
+    dma_memory_rw(pci_dma_context(dev), addr, buf, len, dir);
     return 0;
 }
 
@@ -568,12 +574,12 @@  static inline int pci_dma_write(PCIDevice *dev, dma_addr_t addr,
     static inline uint##_bits##_t ld##_l##_pci_dma(PCIDevice *dev,      \
                                                    dma_addr_t addr)     \
     {                                                                   \
-        return ld##_l##_phys(addr);                                     \
+        return ld##_l##_dma(pci_dma_context(dev), addr);                \
     }                                                                   \
     static inline void st##_s##_pci_dma(PCIDevice *dev,                 \
-                          dma_addr_t addr, uint##_bits##_t val)         \
+                                        dma_addr_t addr, uint##_bits##_t val) \
     {                                                                   \
-        st##_s##_phys(addr, val);                                       \
+        st##_s##_dma(pci_dma_context(dev), addr, val);                  \
     }
 
 PCI_DMA_DEFINE_LDST(ub, b, 8);
@@ -587,21 +593,19 @@  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,
+                                DMAInvalidateMapFunc *cb, void *opaque,
                                 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;
+    buf = dma_memory_map(pci_dma_context(dev), cb, opaque, addr, plen, dir);
     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);
+    dma_memory_unmap(pci_dma_context(dev), buffer, len, dir, access_len);
 }
 
 static inline void pci_dma_sglist_init(QEMUSGList *qsg, PCIDevice *dev,
diff --git a/qemu-common.h b/qemu-common.h
index dbfce6f..af2dd51 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -256,6 +256,7 @@  typedef struct SSIBus SSIBus;
 typedef struct EventNotifier EventNotifier;
 typedef struct VirtIODevice VirtIODevice;
 typedef struct QEMUSGList QEMUSGList;
+typedef struct DMAContext DMAContext;
 
 typedef uint64_t pcibus_t;