diff mbox

[1/8] rc4030: create custom DMA address space

Message ID 1425593606-5909-2-git-send-email-hpoussin@reactos.org
State New
Headers show

Commit Message

Hervé Poussineau March 5, 2015, 10:13 p.m. UTC
Add a new memory region in system address space where DMA address space
definition (the 'translation table') belongs, so we can update on the fly
the DMA address space.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/dma/rc4030.c |  154 ++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 117 insertions(+), 37 deletions(-)

Comments

Paolo Bonzini March 25, 2015, 2:45 p.m. UTC | #1
On 05/03/2015 23:13, Hervé Poussineau wrote:
> Add a new memory region in system address space where DMA address space
> definition (the 'translation table') belongs, so we can update on the fly
> the DMA address space.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

Would it make sense to just use an IOMMU region for the DMA address space?

Paolo
Hervé Poussineau March 25, 2015, 7:10 p.m. UTC | #2
Le 25/03/2015 15:45, Paolo Bonzini a écrit :
>
>
> On 05/03/2015 23:13, Hervé Poussineau wrote:
>> Add a new memory region in system address space where DMA address space
>> definition (the 'translation table') belongs, so we can update on the fly
>> the DMA address space.
>>
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>
> Would it make sense to just use an IOMMU region for the DMA address space?

I don't really know. Currently, the first user is the dp8393x network card (a sysbus device), which is connected through the DMA address space.
It means that *every* read or write access needs to be translated, even those accessing 1 or 2 bytes. dp8393x has no direct connection to the system memory.
On some other machines, like the m68k Quadra 800, the dp8393x is only connected to the system memory.

A second user is the ESP SCSI card in DMA mode, using the DMA address space with the help of the DMA controller.

As DMA address space mapping doesn't change much (except initialization phases), and that it is used for small (1 or 2 bytes) and big transfers (network packets and disk accesses), I thought that an 
address space was better.

Hervé
Paolo Bonzini March 26, 2015, 2:15 p.m. UTC | #3
On 25/03/2015 20:10, Hervé Poussineau wrote:
> Le 25/03/2015 15:45, Paolo Bonzini a écrit :
>>
>>
>> On 05/03/2015 23:13, Hervé Poussineau wrote:
>>> Add a new memory region in system address space where DMA address space
>>> definition (the 'translation table') belongs, so we can update on the
>>> fly
>>> the DMA address space.
>>>
>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>
>> Would it make sense to just use an IOMMU region for the DMA address
>> space?
> 
> I don't really know. Currently, the first user is the dp8393x network
> card (a sysbus device), which is connected through the DMA address space.
> It means that *every* read or write access needs to be translated, even
> those accessing 1 or 2 bytes. dp8393x has no direct connection to the
> system memory.
> On some other machines, like the m68k Quadra 800, the dp8393x is only
> connected to the system memory.
> 
> A second user is the ESP SCSI card in DMA mode, using the DMA address
> space with the help of the DMA controller.
> 
> As DMA address space mapping doesn't change much (except initialization
> phases), and that it is used for small (1 or 2 bytes) and big transfers
> (network packets and disk accesses), I thought that an address space was
> better.

Both are okay.  The IOMMU makes address space changes faster; your
scheme is basically a form of caching, it trades update performance for
improved translation performance.

The trick you're using with shadowing the RAM is nifty, but it leaks a
memory region.  I'll reply to the main patch.

Paolo
Paolo Bonzini March 26, 2015, 2:27 p.m. UTC | #4
On 05/03/2015 23:13, Hervé Poussineau wrote:
> +static const MemoryRegionOps rc4030_dma_tt_ops = {
> +    .impl.min_access_size = 4,
> +    .impl.max_access_size = 4,
> +    .impl.max_access_size = 4,
> +};
> +
> +static void rc4030_dma_tt_update(rc4030State *s, uint32_t new_tl_base,
> +                                 uint32_t new_tl_limit)
> +{
> +    int entries, i;
> +    dma_pagetable_entry *dma_tl_contents;
> +
> +    if (s->dma_tl_limit) {
> +        /* write old dma tl table to physical memory */
> +        memory_region_del_subregion(get_system_memory(), &s->dma_tt);
> +        cpu_physical_memory_write(s->dma_tl_limit & 0x7fffffff,
> +                                  memory_region_get_ram_ptr(&s->dma_tt),
> +                                  s->dma_tl_limit);

You would need object_unparent(&s->dma_tt) here.

However, this breaks the rules for memory region lifetime (see
docs/memory.txt).

One solution is to warn here for a large dma_tl_limit and always create
a 4K ROMD region.  Here you can create an alias into the actual dma_tt
region and add/remove/unparent that alias.  Aliases can be created and
unparented at will.

> @@ -733,7 +793,16 @@ static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_wri
>      dma_addr = s->dma_regs[n][DMA_REG_ADDRESS];
>  
>      /* Read/write data at right place */
> -    rc4030_dma_memory_rw(opaque, dma_addr, buf, len, is_write);
> +    for (i = 0; i < len; ) {
> +        int ncpy = DMA_PAGESIZE - (dma_addr & (DMA_PAGESIZE - 1));
> +        if (ncpy > len - i) {
> +            ncpy = len - i;
> +        }
> +        address_space_rw(&s->dma_as, dma_addr, buf + i, ncpy, is_write);
> +
> +        dma_addr += ncpy;
> +        i += ncpy;
> +    }
>  
>      s->dma_regs[n][DMA_REG_ENABLE] |= DMA_FLAG_TC_INTR;
>      s->dma_regs[n][DMA_REG_COUNT] -= len;

The loop should not be necessary, address_space_rw does the same.

Paolo

> @@ -800,6 +869,7 @@ void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
>                    MemoryRegion *sysmem)
>  {
>      rc4030State *s;
> +    int i;
>  
>      s = g_malloc0(sizeof(rc4030State));
>  
> @@ -821,5 +891,15 @@ void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
>                            "rc4030.jazzio", 0x00001000);
>      memory_region_add_subregion(sysmem, 0xf0000000, &s->iomem_jazzio);
>  
> +    memory_region_init(&s->dma_tt, NULL, "dma_tt", 0);
> +    memory_region_init(&s->dma_mr, NULL, "dma", INT32_MAX);
> +    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
> +        memory_region_init_alias(&s->dma_mrs[i], NULL, "dma-alias",
> +                                 get_system_memory(), 0, DMA_PAGESIZE);
> +        memory_region_set_enabled(&s->dma_mrs[i], false);
> +        memory_region_add_subregion(&s->dma_mr, i * DMA_PAGESIZE,
> +                                    &s->dma_mrs[i]);
> +    }
> +    address_space_init(&s->dma_as, &s->dma_mr, "rc4030_dma");
>      return s;
>  }
>
diff mbox

Patch

diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index af26632..93a52f6 100644
--- a/hw/dma/rc4030.c
+++ b/hw/dma/rc4030.c
@@ -25,6 +25,7 @@ 
 #include "hw/hw.h"
 #include "hw/mips/mips.h"
 #include "qemu/timer.h"
+#include "exec/address-spaces.h"
 
 /********************************************************/
 /* debug rc4030 */
@@ -47,6 +48,8 @@  do { fprintf(stderr, "rc4030 ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); } whi
 /********************************************************/
 /* rc4030 emulation                                     */
 
+#define MAX_TL_ENTRIES 512
+
 typedef struct dma_pagetable_entry {
     int32_t frame;
     int32_t owner;
@@ -96,6 +99,11 @@  typedef struct rc4030State
     qemu_irq timer_irq;
     qemu_irq jazz_bus_irq;
 
+    MemoryRegion dma_tt; /* translation table */
+    MemoryRegion dma_mrs[MAX_TL_ENTRIES]; /* translation aliases */
+    MemoryRegion dma_mr; /* whole DMA memory region */
+    AddressSpace dma_as;
+
     MemoryRegion iomem_chipset;
     MemoryRegion iomem_jazzio;
 } rc4030State;
@@ -265,6 +273,89 @@  static uint32_t rc4030_readb(void *opaque, hwaddr addr)
     return (v >> (8 * (addr & 0x3))) & 0xff;
 }
 
+static void rc4030_dma_as_update_one(rc4030State *s, int index, uint32_t frame)
+{
+    if (index < MAX_TL_ENTRIES) {
+        memory_region_set_enabled(&s->dma_mrs[index], false);
+    }
+
+    if (!frame) {
+        return;
+    }
+
+    if (index >= MAX_TL_ENTRIES) {
+        qemu_log_mask(LOG_UNIMP,
+                      "rc4030: trying to use too high "
+                      "translation table entry %d (max allowed=%d)",
+                      index, MAX_TL_ENTRIES);
+        return;
+    }
+    memory_region_set_alias_offset(&s->dma_mrs[index], frame);
+    memory_region_set_enabled(&s->dma_mrs[index], true);
+}
+
+static void rc4030_dma_tt_write(void *opaque, hwaddr addr, uint64_t data,
+                                unsigned int size)
+{
+    rc4030State *s = opaque;
+
+    /* write memory */
+    memcpy(memory_region_get_ram_ptr(&s->dma_tt) + addr, &data, size);
+
+    /* update dma address space (only if frame field has been written) */
+    if (addr % sizeof(dma_pagetable_entry) == 0) {
+        int index = addr / sizeof(dma_pagetable_entry);
+        memory_region_transaction_begin();
+        rc4030_dma_as_update_one(s, index, (uint32_t)data);
+        memory_region_transaction_commit();
+    }
+}
+
+static const MemoryRegionOps rc4030_dma_tt_ops = {
+    .write = rc4030_dma_tt_write,
+    .impl.min_access_size = 4,
+    .impl.max_access_size = 4,
+};
+
+static void rc4030_dma_tt_update(rc4030State *s, uint32_t new_tl_base,
+                                 uint32_t new_tl_limit)
+{
+    int entries, i;
+    dma_pagetable_entry *dma_tl_contents;
+
+    if (s->dma_tl_limit) {
+        /* write old dma tl table to physical memory */
+        memory_region_del_subregion(get_system_memory(), &s->dma_tt);
+        cpu_physical_memory_write(s->dma_tl_limit & 0x7fffffff,
+                                  memory_region_get_ram_ptr(&s->dma_tt),
+                                  s->dma_tl_limit);
+    }
+
+    s->dma_tl_base = new_tl_base;
+    s->dma_tl_limit = new_tl_limit;
+    new_tl_base &= 0x7fffffff;
+
+    if (s->dma_tl_limit) {
+        memory_region_init_rom_device(&s->dma_tt, NULL,
+                                      &rc4030_dma_tt_ops, s, "dma_tt",
+                                      s->dma_tl_limit, NULL);
+        dma_tl_contents = memory_region_get_ram_ptr(&s->dma_tt);
+        cpu_physical_memory_read(new_tl_base, dma_tl_contents, s->dma_tl_limit);
+
+        memory_region_transaction_begin();
+        entries = s->dma_tl_limit / sizeof(dma_pagetable_entry);
+        for (i = 0; i < entries; i++) {
+            rc4030_dma_as_update_one(s, i, dma_tl_contents[i].frame);
+        }
+        memory_region_add_subregion(get_system_memory(), new_tl_base,
+                                    &s->dma_tt);
+        memory_region_transaction_commit();
+    } else {
+        memory_region_init(&s->dma_tt, NULL, "dma_tt", 0);
+    }
+}
+
+
 static void rc4030_writel(void *opaque, hwaddr addr, uint32_t val)
 {
     rc4030State *s = opaque;
@@ -279,11 +370,11 @@  static void rc4030_writel(void *opaque, hwaddr addr, uint32_t val)
         break;
     /* DMA transl. table base */
     case 0x0018:
-        s->dma_tl_base = val;
+        rc4030_dma_tt_update(s, val, s->dma_tl_limit);
         break;
     /* DMA transl. table limit */
     case 0x0020:
-        s->dma_tl_limit = val;
+        rc4030_dma_tt_update(s, s->dma_tl_base, val);
         break;
     /* DMA transl. table invalidated */
     case 0x0028:
@@ -590,7 +681,7 @@  static void rc4030_reset(void *opaque)
     s->invalid_address_register = 0;
 
     memset(s->dma_regs, 0, sizeof(s->dma_regs));
-    s->dma_tl_base = s->dma_tl_limit = 0;
+    rc4030_dma_tt_update(s, 0, 0);
 
     s->remote_failed_address = s->memory_failed_address = 0;
     s->cache_maint = 0;
@@ -675,39 +766,7 @@  static void rc4030_save(QEMUFile *f, void *opaque)
 void rc4030_dma_memory_rw(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write)
 {
     rc4030State *s = opaque;
-    hwaddr entry_addr;
-    hwaddr phys_addr;
-    dma_pagetable_entry entry;
-    int index;
-    int ncpy, i;
-
-    i = 0;
-    for (;;) {
-        if (i == len) {
-            break;
-        }
-
-        ncpy = DMA_PAGESIZE - (addr & (DMA_PAGESIZE - 1));
-        if (ncpy > len - i)
-            ncpy = len - i;
-
-        /* Get DMA translation table entry */
-        index = addr / DMA_PAGESIZE;
-        if (index >= s->dma_tl_limit / sizeof(dma_pagetable_entry)) {
-            break;
-        }
-        entry_addr = s->dma_tl_base + index * sizeof(dma_pagetable_entry);
-        /* XXX: not sure. should we really use only lowest bits? */
-        entry_addr &= 0x7fffffff;
-        cpu_physical_memory_read(entry_addr, &entry, sizeof(entry));
-
-        /* Read/write data at right place */
-        phys_addr = entry.frame + (addr & (DMA_PAGESIZE - 1));
-        cpu_physical_memory_rw(phys_addr, &buf[i], ncpy, is_write);
-
-        i += ncpy;
-        addr += ncpy;
-    }
+    address_space_rw(&s->dma_as, addr, buf, len, is_write);
 }
 
 static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_write)
@@ -715,6 +774,7 @@  static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_wri
     rc4030State *s = opaque;
     hwaddr dma_addr;
     int dev_to_mem;
+    int i;
 
     s->dma_regs[n][DMA_REG_ENABLE] &= ~(DMA_FLAG_TC_INTR | DMA_FLAG_MEM_INTR | DMA_FLAG_ADDR_INTR);
 
@@ -733,7 +793,16 @@  static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_wri
     dma_addr = s->dma_regs[n][DMA_REG_ADDRESS];
 
     /* Read/write data at right place */
-    rc4030_dma_memory_rw(opaque, dma_addr, buf, len, is_write);
+    for (i = 0; i < len; ) {
+        int ncpy = DMA_PAGESIZE - (dma_addr & (DMA_PAGESIZE - 1));
+        if (ncpy > len - i) {
+            ncpy = len - i;
+        }
+        address_space_rw(&s->dma_as, dma_addr, buf + i, ncpy, is_write);
+
+        dma_addr += ncpy;
+        i += ncpy;
+    }
 
     s->dma_regs[n][DMA_REG_ENABLE] |= DMA_FLAG_TC_INTR;
     s->dma_regs[n][DMA_REG_COUNT] -= len;
@@ -800,6 +869,7 @@  void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
                   MemoryRegion *sysmem)
 {
     rc4030State *s;
+    int i;
 
     s = g_malloc0(sizeof(rc4030State));
 
@@ -821,5 +891,15 @@  void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
                           "rc4030.jazzio", 0x00001000);
     memory_region_add_subregion(sysmem, 0xf0000000, &s->iomem_jazzio);
 
+    memory_region_init(&s->dma_tt, NULL, "dma_tt", 0);
+    memory_region_init(&s->dma_mr, NULL, "dma", INT32_MAX);
+    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
+        memory_region_init_alias(&s->dma_mrs[i], NULL, "dma-alias",
+                                 get_system_memory(), 0, DMA_PAGESIZE);
+        memory_region_set_enabled(&s->dma_mrs[i], false);
+        memory_region_add_subregion(&s->dma_mr, i * DMA_PAGESIZE,
+                                    &s->dma_mrs[i]);
+    }
+    address_space_init(&s->dma_as, &s->dma_mr, "rc4030_dma");
     return s;
 }