dma/rc4030: translate memory accesses only when they occur

This simplifies the code a lot, and this fixes big memory leaks introduced in a3d586f704609a45b6037534cb2f34da5dfd8895

Windows NT is now able to boot without using gigabytes of ram on the host.

Signed-off-by Hervé Poussineau <[email protected]>

---
 hw/dma/rc4030.c | 158 +++++++++++++-------------------------------------------
 1 file changed, 36 insertions(+), 122 deletions(-)

hw/dma/rc4030.c +36 -122

index 41fc043..5f10b9d 100644
@@ -34,8 +34,6 @@
 /********************************************************/
 /* rc4030 emulation                                     */
 
-#define MAX_TL_ENTRIES 512
-
 typedef struct dma_pagetable_entry {
     int32_t frame;
     int32_t owner;

@@ -91,14 +89,8 @@ typedef struct rc4030State
     qemu_irq timer_irq;
     qemu_irq jazz_bus_irq;
 
-    /* biggest translation table */
-    MemoryRegion dma_tt;
-    /* translation table memory region alias, added to system RAM */
-    MemoryRegion dma_tt_alias;
     /* whole DMA memory region, root of DMA address space */
     MemoryRegion dma_mr;
-    /* translation table entry aliases, added to DMA memory region */
-    MemoryRegion dma_mrs[MAX_TL_ENTRIES];
     AddressSpace dma_as;
 
     MemoryRegion iomem_chipset;

@@ -256,96 +248,6 @@ static uint64_t rc4030_read(void *opaque, hwaddr addr, unsigned int size)
     return val;
 }
 
-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_alias);
-        cpu_physical_memory_write(s->dma_tl_limit & 0x7fffffff,
-                                  memory_region_get_ram_ptr(&s->dma_tt),
-                                  memory_region_size(&s->dma_tt_alias));
-    }
-    object_unparent(OBJECT(&s->dma_tt_alias));
-
-    s->dma_tl_base = new_tl_base;
-    s->dma_tl_limit = new_tl_limit;
-    new_tl_base &= 0x7fffffff;
-
-    if (s->dma_tl_limit) {
-        uint64_t dma_tt_size;
-        if (s->dma_tl_limit <= memory_region_size(&s->dma_tt)) {
-            dma_tt_size = s->dma_tl_limit;
-        } else {
-            dma_tt_size = memory_region_size(&s->dma_tt);
-        }
-        memory_region_init_alias(&s->dma_tt_alias, OBJECT(s),
-                                 "dma-table-alias",
-                                 &s->dma_tt, 0, dma_tt_size);
-        dma_tl_contents = memory_region_get_ram_ptr(&s->dma_tt);
-        cpu_physical_memory_read(new_tl_base, dma_tl_contents, dma_tt_size);
-
-        memory_region_transaction_begin();
-        entries = dma_tt_size / 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_alias);
-        memory_region_transaction_commit();
-    } else {
-        memory_region_init(&s->dma_tt_alias, OBJECT(s),
-                           "dma-table-alias", 0);
-    }
-}
-
 static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
                          unsigned int size)
 {

@@ -362,11 +264,11 @@ static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
         break;
     /* DMA transl. table base */
     case 0x0018:
-        rc4030_dma_tt_update(s, val, s->dma_tl_limit);
+        s->dma_tl_base = val;
         break;
     /* DMA transl. table limit */
     case 0x0020:
-        rc4030_dma_tt_update(s, s->dma_tl_base, val);
+        s->dma_tl_limit = val;
         break;
     /* DMA transl. table invalidated */
     case 0x0028:

@@ -586,6 +488,38 @@ static const MemoryRegionOps jazzio_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static IOMMUTLBEntry rc4030_dma_translate(MemoryRegion *iommu, hwaddr addr,
+                                          bool is_write)
+{
+    rc4030State *s = container_of(iommu, rc4030State, dma_mr);
+    IOMMUTLBEntry ret = {
+        .target_as = &address_space_memory,
+        .iova = addr & ~(DMA_PAGESIZE - 1),
+        .translated_addr = 0,
+        .addr_mask = DMA_PAGESIZE - 1,
+        .perm = IOMMU_NONE,
+    };
+    uint64_t i, entry_address;
+    dma_pagetable_entry entry;
+
+    i = addr / DMA_PAGESIZE;
+    if (i < s->dma_tl_limit / sizeof(entry)) {
+        entry_address = (s->dma_tl_base & 0x7fffffff) + i * sizeof(entry);
+        if (address_space_read(ret.target_as, entry_address,
+                               MEMTXATTRS_UNSPECIFIED, (unsigned char *)&entry,
+                               sizeof(entry)) == MEMTX_OK) {
+            ret.translated_addr = entry.frame & ~(DMA_PAGESIZE - 1);
+            ret.perm = IOMMU_RW;
+        }
+    }
+
+    return ret;
+}
+
+static const MemoryRegionIOMMUOps rc4030_dma_ops = {
+    .translate = rc4030_dma_translate,
+};
+
 static void rc4030_reset(DeviceState *dev)
 {
     rc4030State *s = RC4030(dev);

@@ -596,7 +530,6 @@ static void rc4030_reset(DeviceState *dev)
     s->invalid_address_register = 0;
 
     memset(s->dma_regs, 0, sizeof(s->dma_regs));
-    rc4030_dma_tt_update(s, 0, 0);
 
     s->remote_failed_address = s->memory_failed_address = 0;
     s->cache_maint = 0;

@@ -735,7 +668,6 @@ static void rc4030_realize(DeviceState *dev, Error **errp)
 {
     rc4030State *s = RC4030(dev);
     Object *o = OBJECT(dev);
-    int i;
 
     s->periodic_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
                                      rc4030_periodic_timer, s);

@@ -745,37 +677,19 @@ static void rc4030_realize(DeviceState *dev, Error **errp)
     memory_region_init_io(&s->iomem_jazzio, NULL, &jazzio_ops, s,
                           "rc4030.jazzio", 0x00001000);
 
-    memory_region_init_rom_device(&s->dma_tt, o,
-                                  &rc4030_dma_tt_ops, s, "dma-table",
-                                  MAX_TL_ENTRIES * sizeof(dma_pagetable_entry),
-                                  NULL);
-    memory_region_init(&s->dma_tt_alias, o, "dma-table-alias", 0);
-    memory_region_init(&s->dma_mr, o, "dma", INT32_MAX);
-    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
-        memory_region_init_alias(&s->dma_mrs[i], o, "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]);
-    }
+    memory_region_init_iommu(&s->dma_mr, o, &rc4030_dma_ops,
+                             "rc4030.dma", UINT32_MAX);
     address_space_init(&s->dma_as, &s->dma_mr, "rc4030-dma");
 }
 
 static void rc4030_unrealize(DeviceState *dev, Error **errp)
 {
     rc4030State *s = RC4030(dev);
-    int i;
 
     timer_free(s->periodic_timer);
 
     address_space_destroy(&s->dma_as);
-    object_unparent(OBJECT(&s->dma_tt));
-    object_unparent(OBJECT(&s->dma_tt_alias));
     object_unparent(OBJECT(&s->dma_mr));
-    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
-        memory_region_del_subregion(&s->dma_mr, &s->dma_mrs[i]);
-        object_unparent(OBJECT(&s->dma_mrs[i]));
-    }
 }
 
 static void rc4030_class_init(ObjectClass *klass, void *class_data)

Ping?

Quoted 245 lines · Hervé Poussineau

This simplifies the code a lot, and this fixes big memory leaks introduced in a3d586f704609a45b6037534cb2f34da5dfd8895

Windows NT is now able to boot without using gigabytes of ram on the host.

Signed-off-by Hervé Poussineau <[email protected]>

---
 hw/dma/rc4030.c | 158 +++++++++++++-------------------------------------------
 1 file changed, 36 insertions(+), 122 deletions(-)

hw/dma/rc4030.c +36 -122

index 41fc043..5f10b9d 100644
@@ -34,8 +34,6 @@
 /********************************************************/
 /* rc4030 emulation                                     */
 
-#define MAX_TL_ENTRIES 512
-
 typedef struct dma_pagetable_entry {
     int32_t frame;
     int32_t owner;

@@ -91,14 +89,8 @@ typedef struct rc4030State
     qemu_irq timer_irq;
     qemu_irq jazz_bus_irq;
 
-    /* biggest translation table */
-    MemoryRegion dma_tt;
-    /* translation table memory region alias, added to system RAM */
-    MemoryRegion dma_tt_alias;
     /* whole DMA memory region, root of DMA address space */
     MemoryRegion dma_mr;
-    /* translation table entry aliases, added to DMA memory region */
-    MemoryRegion dma_mrs[MAX_TL_ENTRIES];
     AddressSpace dma_as;
 
     MemoryRegion iomem_chipset;

@@ -256,96 +248,6 @@ static uint64_t rc4030_read(void *opaque, hwaddr addr, unsigned int size)
     return val;
 }
 
-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_alias);
-        cpu_physical_memory_write(s->dma_tl_limit & 0x7fffffff,
-                                  memory_region_get_ram_ptr(&s->dma_tt),
-                                  memory_region_size(&s->dma_tt_alias));
-    }
-    object_unparent(OBJECT(&s->dma_tt_alias));
-
-    s->dma_tl_base = new_tl_base;
-    s->dma_tl_limit = new_tl_limit;
-    new_tl_base &= 0x7fffffff;
-
-    if (s->dma_tl_limit) {
-        uint64_t dma_tt_size;
-        if (s->dma_tl_limit <= memory_region_size(&s->dma_tt)) {
-            dma_tt_size = s->dma_tl_limit;
-        } else {
-            dma_tt_size = memory_region_size(&s->dma_tt);
-        }
-        memory_region_init_alias(&s->dma_tt_alias, OBJECT(s),
-                                 "dma-table-alias",
-                                 &s->dma_tt, 0, dma_tt_size);
-        dma_tl_contents = memory_region_get_ram_ptr(&s->dma_tt);
-        cpu_physical_memory_read(new_tl_base, dma_tl_contents, dma_tt_size);
-
-        memory_region_transaction_begin();
-        entries = dma_tt_size / 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_alias);
-        memory_region_transaction_commit();
-    } else {
-        memory_region_init(&s->dma_tt_alias, OBJECT(s),
-                           "dma-table-alias", 0);
-    }
-}
-
 static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
                          unsigned int size)
 {

@@ -362,11 +264,11 @@ static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
         break;
     /* DMA transl. table base */
     case 0x0018:
-        rc4030_dma_tt_update(s, val, s->dma_tl_limit);
+        s->dma_tl_base = val;
         break;
     /* DMA transl. table limit */
     case 0x0020:
-        rc4030_dma_tt_update(s, s->dma_tl_base, val);
+        s->dma_tl_limit = val;
         break;
     /* DMA transl. table invalidated */
     case 0x0028:

@@ -586,6 +488,38 @@ static const MemoryRegionOps jazzio_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static IOMMUTLBEntry rc4030_dma_translate(MemoryRegion *iommu, hwaddr addr,
+                                          bool is_write)
+{
+    rc4030State *s = container_of(iommu, rc4030State, dma_mr);
+    IOMMUTLBEntry ret = {
+        .target_as = &address_space_memory,
+        .iova = addr & ~(DMA_PAGESIZE - 1),
+        .translated_addr = 0,
+        .addr_mask = DMA_PAGESIZE - 1,
+        .perm = IOMMU_NONE,
+    };
+    uint64_t i, entry_address;
+    dma_pagetable_entry entry;
+
+    i = addr / DMA_PAGESIZE;
+    if (i < s->dma_tl_limit / sizeof(entry)) {
+        entry_address = (s->dma_tl_base & 0x7fffffff) + i * sizeof(entry);
+        if (address_space_read(ret.target_as, entry_address,
+                               MEMTXATTRS_UNSPECIFIED, (unsigned char *)&entry,
+                               sizeof(entry)) == MEMTX_OK) {
+            ret.translated_addr = entry.frame & ~(DMA_PAGESIZE - 1);
+            ret.perm = IOMMU_RW;
+        }
+    }
+
+    return ret;
+}
+
+static const MemoryRegionIOMMUOps rc4030_dma_ops = {
+    .translate = rc4030_dma_translate,
+};
+
 static void rc4030_reset(DeviceState *dev)
 {
     rc4030State *s = RC4030(dev);

@@ -596,7 +530,6 @@ static void rc4030_reset(DeviceState *dev)
     s->invalid_address_register = 0;
 
     memset(s->dma_regs, 0, sizeof(s->dma_regs));
-    rc4030_dma_tt_update(s, 0, 0);
 
     s->remote_failed_address = s->memory_failed_address = 0;
     s->cache_maint = 0;

@@ -735,7 +668,6 @@ static void rc4030_realize(DeviceState *dev, Error **errp)
 {
     rc4030State *s = RC4030(dev);
     Object *o = OBJECT(dev);
-    int i;
 
     s->periodic_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
                                      rc4030_periodic_timer, s);

@@ -745,37 +677,19 @@ static void rc4030_realize(DeviceState *dev, Error **errp)
     memory_region_init_io(&s->iomem_jazzio, NULL, &jazzio_ops, s,
                           "rc4030.jazzio", 0x00001000);
 
-    memory_region_init_rom_device(&s->dma_tt, o,
-                                  &rc4030_dma_tt_ops, s, "dma-table",
-                                  MAX_TL_ENTRIES * sizeof(dma_pagetable_entry),
-                                  NULL);
-    memory_region_init(&s->dma_tt_alias, o, "dma-table-alias", 0);
-    memory_region_init(&s->dma_mr, o, "dma", INT32_MAX);
-    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
-        memory_region_init_alias(&s->dma_mrs[i], o, "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]);
-    }
+    memory_region_init_iommu(&s->dma_mr, o, &rc4030_dma_ops,
+                             "rc4030.dma", UINT32_MAX);
     address_space_init(&s->dma_as, &s->dma_mr, "rc4030-dma");
 }
 
 static void rc4030_unrealize(DeviceState *dev, Error **errp)
 {
     rc4030State *s = RC4030(dev);
-    int i;
 
     timer_free(s->periodic_timer);
 
     address_space_destroy(&s->dma_as);
-    object_unparent(OBJECT(&s->dma_tt));
-    object_unparent(OBJECT(&s->dma_tt_alias));
     object_unparent(OBJECT(&s->dma_mr));
-    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
-        memory_region_del_subregion(&s->dma_mr, &s->dma_mrs[i]);
-        object_unparent(OBJECT(&s->dma_mrs[i]));
-    }
 }
 
 static void rc4030_class_init(ObjectClass *klass, void *class_data)

Ping again.

Quoted 249 lines · Hervé Poussineau

Ping?

Quoted 245 lines · Hervé Poussineau

This simplifies the code a lot, and this fixes big memory leaks introduced in a3d586f704609a45b6037534cb2f34da5dfd8895

Windows NT is now able to boot without using gigabytes of ram on the host.

Signed-off-by Hervé Poussineau <[email protected]>

---
 hw/dma/rc4030.c | 158 +++++++++++++-------------------------------------------
 1 file changed, 36 insertions(+), 122 deletions(-)

hw/dma/rc4030.c +36 -122

index 41fc043..5f10b9d 100644
@@ -34,8 +34,6 @@
 /********************************************************/
 /* rc4030 emulation                                     */
 
-#define MAX_TL_ENTRIES 512
-
 typedef struct dma_pagetable_entry {
     int32_t frame;
     int32_t owner;

@@ -91,14 +89,8 @@ typedef struct rc4030State
     qemu_irq timer_irq;
     qemu_irq jazz_bus_irq;
 
-    /* biggest translation table */
-    MemoryRegion dma_tt;
-    /* translation table memory region alias, added to system RAM */
-    MemoryRegion dma_tt_alias;
     /* whole DMA memory region, root of DMA address space */
     MemoryRegion dma_mr;
-    /* translation table entry aliases, added to DMA memory region */
-    MemoryRegion dma_mrs[MAX_TL_ENTRIES];
     AddressSpace dma_as;
 
     MemoryRegion iomem_chipset;

@@ -256,96 +248,6 @@ static uint64_t rc4030_read(void *opaque, hwaddr addr, unsigned int size)
     return val;
 }
 
-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_alias);
-        cpu_physical_memory_write(s->dma_tl_limit & 0x7fffffff,
-                                  memory_region_get_ram_ptr(&s->dma_tt),
-                                  memory_region_size(&s->dma_tt_alias));
-    }
-    object_unparent(OBJECT(&s->dma_tt_alias));
-
-    s->dma_tl_base = new_tl_base;
-    s->dma_tl_limit = new_tl_limit;
-    new_tl_base &= 0x7fffffff;
-
-    if (s->dma_tl_limit) {
-        uint64_t dma_tt_size;
-        if (s->dma_tl_limit <= memory_region_size(&s->dma_tt)) {
-            dma_tt_size = s->dma_tl_limit;
-        } else {
-            dma_tt_size = memory_region_size(&s->dma_tt);
-        }
-        memory_region_init_alias(&s->dma_tt_alias, OBJECT(s),
-                                 "dma-table-alias",
-                                 &s->dma_tt, 0, dma_tt_size);
-        dma_tl_contents = memory_region_get_ram_ptr(&s->dma_tt);
-        cpu_physical_memory_read(new_tl_base, dma_tl_contents, dma_tt_size);
-
-        memory_region_transaction_begin();
-        entries = dma_tt_size / 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_alias);
-        memory_region_transaction_commit();
-    } else {
-        memory_region_init(&s->dma_tt_alias, OBJECT(s),
-                           "dma-table-alias", 0);
-    }
-}
-
 static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
                          unsigned int size)
 {

@@ -362,11 +264,11 @@ static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
         break;
     /* DMA transl. table base */
     case 0x0018:
-        rc4030_dma_tt_update(s, val, s->dma_tl_limit);
+        s->dma_tl_base = val;
         break;
     /* DMA transl. table limit */
     case 0x0020:
-        rc4030_dma_tt_update(s, s->dma_tl_base, val);
+        s->dma_tl_limit = val;
         break;
     /* DMA transl. table invalidated */
     case 0x0028:

@@ -586,6 +488,38 @@ static const MemoryRegionOps jazzio_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static IOMMUTLBEntry rc4030_dma_translate(MemoryRegion *iommu, hwaddr addr,
+                                          bool is_write)
+{
+    rc4030State *s = container_of(iommu, rc4030State, dma_mr);
+    IOMMUTLBEntry ret = {
+        .target_as = &address_space_memory,
+        .iova = addr & ~(DMA_PAGESIZE - 1),
+        .translated_addr = 0,
+        .addr_mask = DMA_PAGESIZE - 1,
+        .perm = IOMMU_NONE,
+    };
+    uint64_t i, entry_address;
+    dma_pagetable_entry entry;
+
+    i = addr / DMA_PAGESIZE;
+    if (i < s->dma_tl_limit / sizeof(entry)) {
+        entry_address = (s->dma_tl_base & 0x7fffffff) + i * sizeof(entry);
+        if (address_space_read(ret.target_as, entry_address,
+                               MEMTXATTRS_UNSPECIFIED, (unsigned char *)&entry,
+                               sizeof(entry)) == MEMTX_OK) {
+            ret.translated_addr = entry.frame & ~(DMA_PAGESIZE - 1);
+            ret.perm = IOMMU_RW;
+        }
+    }
+
+    return ret;
+}
+
+static const MemoryRegionIOMMUOps rc4030_dma_ops = {
+    .translate = rc4030_dma_translate,
+};
+
 static void rc4030_reset(DeviceState *dev)
 {
     rc4030State *s = RC4030(dev);

@@ -596,7 +530,6 @@ static void rc4030_reset(DeviceState *dev)
     s->invalid_address_register = 0;
 
     memset(s->dma_regs, 0, sizeof(s->dma_regs));
-    rc4030_dma_tt_update(s, 0, 0);
 
     s->remote_failed_address = s->memory_failed_address = 0;
     s->cache_maint = 0;

@@ -735,7 +668,6 @@ static void rc4030_realize(DeviceState *dev, Error **errp)
 {
     rc4030State *s = RC4030(dev);
     Object *o = OBJECT(dev);
-    int i;
 
     s->periodic_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
                                      rc4030_periodic_timer, s);

@@ -745,37 +677,19 @@ static void rc4030_realize(DeviceState *dev, Error **errp)
     memory_region_init_io(&s->iomem_jazzio, NULL, &jazzio_ops, s,
                           "rc4030.jazzio", 0x00001000);
 
-    memory_region_init_rom_device(&s->dma_tt, o,
-                                  &rc4030_dma_tt_ops, s, "dma-table",
-                                  MAX_TL_ENTRIES * sizeof(dma_pagetable_entry),
-                                  NULL);
-    memory_region_init(&s->dma_tt_alias, o, "dma-table-alias", 0);
-    memory_region_init(&s->dma_mr, o, "dma", INT32_MAX);
-    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
-        memory_region_init_alias(&s->dma_mrs[i], o, "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]);
-    }
+    memory_region_init_iommu(&s->dma_mr, o, &rc4030_dma_ops,
+                             "rc4030.dma", UINT32_MAX);
     address_space_init(&s->dma_as, &s->dma_mr, "rc4030-dma");
 }
 
 static void rc4030_unrealize(DeviceState *dev, Error **errp)
 {
     rc4030State *s = RC4030(dev);
-    int i;
 
     timer_free(s->periodic_timer);
 
     address_space_destroy(&s->dma_as);
-    object_unparent(OBJECT(&s->dma_tt));
-    object_unparent(OBJECT(&s->dma_tt_alias));
     object_unparent(OBJECT(&s->dma_mr));
-    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
-        memory_region_del_subregion(&s->dma_mr, &s->dma_mrs[i]);
-        object_unparent(OBJECT(&s->dma_mrs[i]));
-    }
 }
 
 static void rc4030_class_init(ObjectClass *klass, void *class_data)

Ping again.

Quoted 265 lines · Hervé Poussineau

Ping again.

Quoted 261 lines · Hervé Poussineau

Ping?

Quoted 257 lines · Hervé Poussineau

This simplifies the code a lot, and this fixes big memory leaks introduced in a3d586f704609a45b6037534cb2f34da5dfd8895

Windows NT is now able to boot without using gigabytes of ram on the host.

Signed-off-by Hervé Poussineau <[email protected]>

---
 hw/dma/rc4030.c | 158
+++++++++++++-------------------------------------------
 1 file changed, 36 insertions(+), 122 deletions(-)

hw/dma/rc4030.c +0 -83

index 41fc043..5f10b9d 100644
@@ -34,8 +34,6 @@
 /********************************************************/
 /* rc4030 emulation                                     */
 
-#define MAX_TL_ENTRIES 512
-
 typedef struct dma_pagetable_entry {
     int32_t frame;
     int32_t owner;

@@ -91,14 +89,8 @@ typedef struct rc4030State
     qemu_irq timer_irq;
     qemu_irq jazz_bus_irq;
 
-    /* biggest translation table */
-    MemoryRegion dma_tt;
-    /* translation table memory region alias, added to system RAM */
-    MemoryRegion dma_tt_alias;
     /* whole DMA memory region, root of DMA address space */
     MemoryRegion dma_mr;
-    /* translation table entry aliases, added to DMA memory region */
-    MemoryRegion dma_mrs[MAX_TL_ENTRIES];
     AddressSpace dma_as;
 
     MemoryRegion iomem_chipset;

@@ -256,96 +248,6 @@ static uint64_t rc4030_read(void *opaque, hwaddr
 addr, unsigned int size)
     return val;
 }
 
-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_alias);
-        cpu_physical_memory_write(s->dma_tl_limit & 0x7fffffff,
-                                  memory_region_get_ram_ptr(&s->dma_tt),
-                                  memory_region_size(&s->dma_tt_alias));
-    }
-    object_unparent(OBJECT(&s->dma_tt_alias));
-
-    s->dma_tl_base = new_tl_base;
-    s->dma_tl_limit = new_tl_limit;
-    new_tl_base &= 0x7fffffff;
-
-    if (s->dma_tl_limit) {
-        uint64_t dma_tt_size;
-        if (s->dma_tl_limit <= memory_region_size(&s->dma_tt)) {
-            dma_tt_size = s->dma_tl_limit;
-        } else {
-            dma_tt_size = memory_region_size(&s->dma_tt);
-        }
-        memory_region_init_alias(&s->dma_tt_alias, OBJECT(s),
-                                 "dma-table-alias",
-                                 &s->dma_tt, 0, dma_tt_size);
-        dma_tl_contents = memory_region_get_ram_ptr(&s->dma_tt);
-        cpu_physical_memory_read(new_tl_base, dma_tl_contents,

dma_tt_size);

hw/dma/rc4030.c +2 -17

-
-        memory_region_transaction_begin();
-        entries = dma_tt_size / 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_alias);
-        memory_region_transaction_commit();
-    } else {
-        memory_region_init(&s->dma_tt_alias, OBJECT(s),
-                           "dma-table-alias", 0);
-    }
-}
-
 static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
                          unsigned int size)
 {

@@ -362,11 +264,11 @@ static void rc4030_write(void *opaque, hwaddr
 addr, uint64_t data,
         break;
     /* DMA transl. table base */
     case 0x0018:
-        rc4030_dma_tt_update(s, val, s->dma_tl_limit);
+        s->dma_tl_base = val;
         break;
     /* DMA transl. table limit */
     case 0x0020:
-        rc4030_dma_tt_update(s, s->dma_tl_base, val);
+        s->dma_tl_limit = val;
         break;
     /* DMA transl. table invalidated */

     case 0x0028:

hw/dma/rc4030.c +32 -0

@@ -586,6 +488,38 @@ static const MemoryRegionOps jazzio_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static IOMMUTLBEntry rc4030_dma_translate(MemoryRegion *iommu, hwaddr
 addr,
+                                          bool is_write)
+{
+    rc4030State *s = container_of(iommu, rc4030State, dma_mr);
+    IOMMUTLBEntry ret = {
+        .target_as = &address_space_memory,
+        .iova = addr & ~(DMA_PAGESIZE - 1),
+        .translated_addr = 0,
+        .addr_mask = DMA_PAGESIZE - 1,
+        .perm = IOMMU_NONE,
+    };
+    uint64_t i, entry_address;
+    dma_pagetable_entry entry;
+
+    i = addr / DMA_PAGESIZE;
+    if (i < s->dma_tl_limit / sizeof(entry)) {
+        entry_address = (s->dma_tl_base & 0x7fffffff) + i *
 sizeof(entry);
+        if (address_space_read(ret.target_as, entry_address,
+                               MEMTXATTRS_UNSPECIFIED, (unsigned char
 *)&entry,
+                               sizeof(entry)) == MEMTX_OK) {
+            ret.translated_addr = entry.frame & ~(DMA_PAGESIZE - 1);
+            ret.perm = IOMMU_RW;
+        }
+    }
+
+    return ret;
+}
+
+static const MemoryRegionIOMMUOps rc4030_dma_ops = {
+    .translate = rc4030_dma_translate,
+};
+

static void rc4030_reset(DeviceState *dev)

 {
     rc4030State *s = RC4030(dev);

hw/dma/rc4030.c +0 -2

@@ -596,7 +530,6 @@ static void rc4030_reset(DeviceState *dev)
     s->invalid_address_register = 0;
 
     memset(s->dma_regs, 0, sizeof(s->dma_regs));
-    rc4030_dma_tt_update(s, 0, 0);
 
     s->remote_failed_address = s->memory_failed_address = 0;
     s->cache_maint = 0;

@@ -735,7 +668,6 @@ static void rc4030_realize(DeviceState *dev, Error
 **errp)
 {
     rc4030State *s = RC4030(dev);
     Object *o = OBJECT(dev);
-    int i;
 
     s->periodic_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,

                                      rc4030_periodic_timer, s);

hw/dma/rc4030.c +2 -20

@@ -745,37 +677,19 @@ static void rc4030_realize(DeviceState *dev,
 Error **errp)
     memory_region_init_io(&s->iomem_jazzio, NULL, &jazzio_ops, s,
                           "rc4030.jazzio", 0x00001000);
 
-    memory_region_init_rom_device(&s->dma_tt, o,
-                                  &rc4030_dma_tt_ops, s, "dma-table",
-                                  MAX_TL_ENTRIES *
 sizeof(dma_pagetable_entry),
-                                  NULL);
-    memory_region_init(&s->dma_tt_alias, o, "dma-table-alias", 0);
-    memory_region_init(&s->dma_mr, o, "dma", INT32_MAX);
-    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
-        memory_region_init_alias(&s->dma_mrs[i], o, "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]);
-    }
+    memory_region_init_iommu(&s->dma_mr, o, &rc4030_dma_ops,
+                             "rc4030.dma", UINT32_MAX);
     address_space_init(&s->dma_as, &s->dma_mr, "rc4030-dma");
 }
 
 static void rc4030_unrealize(DeviceState *dev, Error **errp)
 {
     rc4030State *s = RC4030(dev);
-    int i;
 
     timer_free(s->periodic_timer);
 
     address_space_destroy(&s->dma_as);
-    object_unparent(OBJECT(&s->dma_tt));
-    object_unparent(OBJECT(&s->dma_tt_alias));
     object_unparent(OBJECT(&s->dma_mr));
-    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
-        memory_region_del_subregion(&s->dma_mr, &s->dma_mrs[i]);
-        object_unparent(OBJECT(&s->dma_mrs[i]));
-    }
 }

static void rc4030_class_init(ObjectClass *klass, void *class_data)