[Qemu-devel] [PATCH 0/3] iommu: isa dma engine support

[Qemu-devel] [PATCH 0/3] iommu: isa dma engine support

[Jason Baron]

Hi,

Based on Benjamin Herrenschmidt's iommu series - these patches provide
infrastructure for a chipset topology where an iommu can sit between
main memory and the isa dma engine. The idea is remap addresses according
to the iommu's wish when we are performing a dma engine r/w operation.

Since the iommu context is tied to the isa device structure, it can be set
independently of the bus on which the isa device resides. Thus, I'm hoping that
this patchset allows us to correctly model more complex chipset topologies.

Thanks,

-Jason


Jason Baron (3):
  iommu: remove extra 'typedef struct DMAContext'
  iommu: add support for the isa dma engine
  iommu: add generic mapping support for the isa bus

 dma.h        |    5 ++---
 hw/cs4231a.c |    2 +-
 hw/dma.c     |   19 ++++++++++++-------
 hw/fdc.c     |   10 +++++-----
 hw/gus.c     |    2 +-
 hw/isa-bus.c |   14 ++++++++++++++
 hw/isa.h     |    9 ++++++++-
 hw/sb16.c    |    4 ++--
 hw/sun4m.c   |    2 +-
 hw/sun4u.c   |    2 +-
 10 files changed, 47 insertions(+), 22 deletions(-)

[Qemu-devel] [PATCH 1/3] iommu: remove extra 'typedef struct DMAContext'

[Jason Baron]

There are several instances of 'typedef struct DMAContext DMAContext'. Remove them
all except for the one in qemu-common.h.

Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 dma.h |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/dma.h b/dma.h
index 0d57e50..862373d 100644
--- a/dma.h
+++ b/dma.h
@@ -15,7 +15,6 @@
 #include "block.h"
 #include "kvm.h"
 
-typedef struct DMAContext DMAContext;
 typedef struct ScatterGatherEntry ScatterGatherEntry;
 
 typedef enum {
@@ -65,11 +64,11 @@ typedef void DMAUnmapFunc(DMAContext *dma,
                           DMADirection dir,
                           dma_addr_t access_len);
 
-typedef struct DMAContext {
+struct DMAContext {
     DMATranslateFunc *translate;
     DMAMapFunc *map;
     DMAUnmapFunc *unmap;
-} DMAContext;
+};
 
 static inline void dma_barrier(DMAContext *dma, DMADirection dir)
 {
-- 
1.7.1

[Qemu-devel] [PATCH 2/3] iommu: add support for the isa dma engine

[Jason Baron]

Provide infastructure for iommu support to the existing isa dma engine. This
allows chipsets to support an iommu sitting between memory and the dma engine.
Code is layered on top of Benjamin Herrenschmidt's generic iommu infastructure.

In addition, the patch more tightly couples isa devices with the existing
dma engine by changing the prototype of DMA_register channel:

-void DMA_register_channel (int nchan,
+void DMA_register_channel (ISADevice *dev, int nchan,
                            DMA_transfer_handler transfer_handler,
                            void *opaque)

Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 hw/cs4231a.c |    2 +-
 hw/dma.c     |   19 ++++++++++++-------
 hw/fdc.c     |   10 +++++-----
 hw/gus.c     |    2 +-
 hw/isa-bus.c |    3 +++
 hw/isa.h     |    4 +++-
 hw/sb16.c    |    4 ++--
 hw/sun4m.c   |    2 +-
 hw/sun4u.c   |    2 +-
 9 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/hw/cs4231a.c b/hw/cs4231a.c
index e07b9d6..81163f9 100644
--- a/hw/cs4231a.c
+++ b/hw/cs4231a.c
@@ -650,7 +650,7 @@ static int cs4231a_initfn (ISADevice *dev)
     memory_region_init_io (&s->ioports, &cs_ioport_ops, s, "cs4231a", 4);
     isa_register_ioport (dev, &s->ioports, s->port);
 
-    DMA_register_channel (s->dma, cs_dma_read, s);
+    DMA_register_channel (dev, s->dma, cs_dma_read, s);
 
     qemu_register_reset (cs_reset, s);
     cs_reset (s);
diff --git a/hw/dma.c b/hw/dma.c
index 0a9322d..0a323c4 100644
--- a/hw/dma.c
+++ b/hw/dma.c
@@ -45,6 +45,7 @@ struct dma_regs {
     uint8_t eop;
     DMA_transfer_handler transfer_handler;
     void *opaque;
+    DMAContext *dma_ctx;
 };
 
 #define ADDR 0
@@ -393,7 +394,7 @@ static void DMA_run_bh(void *unused)
     DMA_run();
 }
 
-void DMA_register_channel (int nchan,
+void DMA_register_channel (ISADevice *dev, int nchan,
                            DMA_transfer_handler transfer_handler,
                            void *opaque)
 {
@@ -406,18 +407,22 @@ void DMA_register_channel (int nchan,
     r = dma_controllers[ncont].regs + ichan;
     r->transfer_handler = transfer_handler;
     r->opaque = opaque;
+    if (dev)
+        r->dma_ctx = dev->dma;
+    else
+        r->dma_ctx = NULL;
 }
 
 int DMA_read_memory (int nchan, void *buf, int pos, int len)
 {
     struct dma_regs *r = &dma_controllers[nchan > 3].regs[nchan & 3];
-    target_phys_addr_t addr = ((r->pageh & 0x7f) << 24) | (r->page << 16) | r->now[ADDR];
+    dma_addr_t addr = ((r->pageh & 0x7f) << 24) | (r->page << 16) | r->now[ADDR];
 
     if (r->mode & 0x20) {
         int i;
         uint8_t *p = buf;
 
-        cpu_physical_memory_read (addr - pos - len, buf, len);
+        dma_memory_read(r->dma_ctx, addr - pos - len, buf, len);
         /* What about 16bit transfers? */
         for (i = 0; i < len >> 1; i++) {
             uint8_t b = p[len - i - 1];
@@ -425,7 +430,7 @@ int DMA_read_memory (int nchan, void *buf, int pos, int len)
         }
     }
     else
-        cpu_physical_memory_read (addr + pos, buf, len);
+        dma_memory_read(r->dma_ctx,  addr + pos, buf, len);
 
     return len;
 }
@@ -433,13 +438,13 @@ int DMA_read_memory (int nchan, void *buf, int pos, int len)
 int DMA_write_memory (int nchan, void *buf, int pos, int len)
 {
     struct dma_regs *r = &dma_controllers[nchan > 3].regs[nchan & 3];
-    target_phys_addr_t addr = ((r->pageh & 0x7f) << 24) | (r->page << 16) | r->now[ADDR];
+    dma_addr_t addr = ((r->pageh & 0x7f) << 24) | (r->page << 16) | r->now[ADDR];
 
     if (r->mode & 0x20) {
         int i;
         uint8_t *p = buf;
 
-        cpu_physical_memory_write (addr - pos - len, buf, len);
+        dma_memory_write(r->dma_ctx, addr - pos - len, buf, len);
         /* What about 16bit transfers? */
         for (i = 0; i < len; i++) {
             uint8_t b = p[len - i - 1];
@@ -447,7 +452,7 @@ int DMA_write_memory (int nchan, void *buf, int pos, int len)
         }
     }
     else
-        cpu_physical_memory_write (addr + pos, buf, len);
+        dma_memory_write(r->dma_ctx, addr + pos, buf, len);
 
     return len;
 }
diff --git a/hw/fdc.c b/hw/fdc.c
index 78b4e33..8afc467 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1951,7 +1951,7 @@ void sun4m_fdctrl_init(qemu_irq irq, target_phys_addr_t io_base,
     *fdc_tc = qdev_get_gpio_in(dev, 0);
 }
 
-static int fdctrl_init_common(FDCtrl *fdctrl)
+static int fdctrl_init_common(FDCtrl *fdctrl, ISADevice *dev)
 {
     int i, j;
     static int command_tables_inited = 0;
@@ -1979,7 +1979,7 @@ static int fdctrl_init_common(FDCtrl *fdctrl)
     fdctrl->num_floppies = MAX_FD;
 
     if (fdctrl->dma_chann != -1)
-        DMA_register_channel(fdctrl->dma_chann, &fdctrl_transfer_handler, fdctrl);
+        DMA_register_channel(dev, fdctrl->dma_chann, &fdctrl_transfer_handler, fdctrl);
     return fdctrl_connect_drives(fdctrl);
 }
 
@@ -2001,7 +2001,7 @@ static int isabus_fdc_init1(ISADevice *dev)
     fdctrl->dma_chann = isa->dma;
 
     qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 2);
-    ret = fdctrl_init_common(fdctrl);
+    ret = fdctrl_init_common(fdctrl, dev);
 
     add_boot_device_path(isa->bootindexA, &dev->qdev, "/floppy@0");
     add_boot_device_path(isa->bootindexB, &dev->qdev, "/floppy@1");
@@ -2022,7 +2022,7 @@ static int sysbus_fdc_init1(SysBusDevice *dev)
     fdctrl->dma_chann = -1;
 
     qdev_set_legacy_instance_id(&dev->qdev, 0 /* io */, 2); /* FIXME */
-    ret = fdctrl_init_common(fdctrl);
+    ret = fdctrl_init_common(fdctrl, NULL);
 
     return ret;
 }
@@ -2039,7 +2039,7 @@ static int sun4m_fdc_init1(SysBusDevice *dev)
 
     fdctrl->sun4m = 1;
     qdev_set_legacy_instance_id(&dev->qdev, 0 /* io */, 2); /* FIXME */
-    return fdctrl_init_common(fdctrl);
+    return fdctrl_init_common(fdctrl, NULL);
 }
 
 void fdc_get_bs(BlockDriverState *bs[], ISADevice *dev)
diff --git a/hw/gus.c b/hw/gus.c
index 840d098..d827337 100644
--- a/hw/gus.c
+++ b/hw/gus.c
@@ -282,7 +282,7 @@ static int gus_initfn (ISADevice *dev)
     isa_register_portio_list (dev, (s->port + 0x100) & 0xf00,
                               gus_portio_list2, s, "gus");
 
-    DMA_register_channel (s->emu.gusdma, GUS_read_DMA, s);
+    DMA_register_channel (dev, s->emu.gusdma, GUS_read_DMA, s);
     s->emu.himemaddr = s->himem;
     s->emu.gusdatapos = s->emu.himemaddr + 1024 * 1024 + 32;
     s->emu.opaque = s;
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index f9b2373..cfd7501 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -130,6 +130,9 @@ static int isa_qdev_init(DeviceState *qdev)
         return klass->init(dev);
     }
 
+    /* iommu setup */
+    dev->dma = NULL;
+
     return 0;
 }
 
diff --git a/hw/isa.h b/hw/isa.h
index dc97052..a1c3f25 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -6,6 +6,7 @@
 #include "ioport.h"
 #include "memory.h"
 #include "qdev.h"
+#include "dma.h"
 
 #define ISA_NUM_IRQS 16
 
@@ -36,6 +37,7 @@ struct ISADevice {
     uint32_t isairq[2];
     int nirqs;
     int ioport_id;
+    DMAContext *dma;
 };
 
 ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space_io);
@@ -95,7 +97,7 @@ void DMA_hold_DREQ (int nchan);
 void DMA_release_DREQ (int nchan);
 void DMA_schedule(int nchan);
 void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit);
-void DMA_register_channel (int nchan,
+void DMA_register_channel (ISADevice *dev, int nchan,
                            DMA_transfer_handler transfer_handler,
                            void *opaque);
 #endif
diff --git a/hw/sb16.c b/hw/sb16.c
index c81455d..d52dc57 100644
--- a/hw/sb16.c
+++ b/hw/sb16.c
@@ -1377,8 +1377,8 @@ static int sb16_initfn (ISADevice *dev)
 
     isa_register_portio_list (dev, s->port, sb16_ioport_list, s, "sb16");
 
-    DMA_register_channel (s->hdma, SB_read_DMA, s);
-    DMA_register_channel (s->dma, SB_read_DMA, s);
+    DMA_register_channel (dev, s->hdma, SB_read_DMA, s);
+    DMA_register_channel (dev, s->dma, SB_read_DMA, s);
     s->can_write = 1;
 
     AUD_register_card ("sb16", &s->card);
diff --git a/hw/sun4m.c b/hw/sun4m.c
index a959261..7a01c94 100644
--- a/hw/sun4m.c
+++ b/hw/sun4m.c
@@ -155,7 +155,7 @@ void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit)
 {
 }
 
-void DMA_register_channel (int nchan,
+void DMA_register_channel (ISADevice *dev, int nchan,
                            DMA_transfer_handler transfer_handler,
                            void *opaque)
 {
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 137a7c6..5964559 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -117,7 +117,7 @@ void DMA_init(int high_page_enable, qemu_irq *cpu_request_exit)
 {
 }
 
-void DMA_register_channel (int nchan,
+void DMA_register_channel (ISADevice *dev, int nchan,
                            DMA_transfer_handler transfer_handler,
                            void *opaque)
 {
-- 
1.7.1

[Qemu-devel] [PATCH 3/3] iommu: add generic mapping support for the isa bus

[Jason Baron]

Introduce 'isa_setup_iommu()' to allow chipsets to associate an iommu mapping
function with the isa bus. This allows isa devices which sit behind an iommu
to be correctly configured. This is based on Benjamin Herrenschmidt's iommu
series. It has no consumers for now.

Signed-off-by: Jason Baron <jbaron@redhat.com>
---
 hw/isa-bus.c |   11 +++++++++++
 hw/isa.h     |    5 +++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index cfd7501..0232799 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -118,10 +118,17 @@ void isa_register_portio_list(ISADevice *dev, uint16_t start,
     portio_list_add(piolist, isabus->address_space_io, start);
 }
 
+void isa_setup_iommu(ISABus *bus, ISADMAContextFunc fn, void *opaque)
+{
+    bus->dma_context_fn = fn;
+    bus->dma_context_opaque = opaque;
+}
+
 static int isa_qdev_init(DeviceState *qdev)
 {
     ISADevice *dev = ISA_DEVICE(qdev);
     ISADeviceClass *klass = ISA_DEVICE_GET_CLASS(dev);
+    ISABus *isabus;
 
     dev->isairq[0] = -1;
     dev->isairq[1] = -1;
@@ -132,6 +139,10 @@ static int isa_qdev_init(DeviceState *qdev)
 
     /* iommu setup */
     dev->dma = NULL;
+    isabus = FROM_QBUS(ISABus, qdev_get_parent_bus(qdev));
+    if (isabus->dma_context_fn) {
+        dev->dma = isabus->dma_context_fn(isabus, isabus->dma_context_opaque);
+    }
 
     return 0;
 }
diff --git a/hw/isa.h b/hw/isa.h
index a1c3f25..e6a0bb1 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -26,10 +26,15 @@ typedef struct ISADeviceClass {
     int (*init)(ISADevice *dev);
 } ISADeviceClass;
 
+typedef DMAContext *(*ISADMAContextFunc)(ISABus *, void *);
+void isa_setup_iommu(ISABus *bus, ISADMAContextFunc fn, void *opaque);
+
 struct ISABus {
     BusState qbus;
     MemoryRegion *address_space_io;
     qemu_irq *irqs;
+    ISADMAContextFunc dma_context_fn;
+    void *dma_context_opaque;
 };
 
 struct ISADevice {
-- 
1.7.1

Re: [Qemu-devel] [PATCH 1/3] iommu: remove extra 'typedef struct DMAContext'

[Benjamin Herrenschmidt]

On Wed, 2012-06-20 at 16:11 -0400, Jason Baron wrote:
> There are several instances of 'typedef struct DMAContext DMAContext'. Remove them
> all except for the one in qemu-common.h.

Actually, the other way around :-) I'm removing the one in qemu-common.h
in favor of the one in dma.h (at Anthony's request). I'll have a new
series soon...

Cheers,
Ben.

> Signed-off-by: Jason Baron <jbaron@redhat.com>
> ---
>  dma.h |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/dma.h b/dma.h
> index 0d57e50..862373d 100644
> --- a/dma.h
> +++ b/dma.h
> @@ -15,7 +15,6 @@
>  #include "block.h"
>  #include "kvm.h"
>  
> -typedef struct DMAContext DMAContext;
>  typedef struct ScatterGatherEntry ScatterGatherEntry;
>  
>  typedef enum {
> @@ -65,11 +64,11 @@ typedef void DMAUnmapFunc(DMAContext *dma,
>                            DMADirection dir,
>                            dma_addr_t access_len);
>  
> -typedef struct DMAContext {
> +struct DMAContext {
>      DMATranslateFunc *translate;
>      DMAMapFunc *map;
>      DMAUnmapFunc *unmap;
> -} DMAContext;
> +};
>  
>  static inline void dma_barrier(DMAContext *dma, DMADirection dir)
>  {