All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements
@ 2015-05-27 12:19 Hervé Poussineau
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 01/17] mips jazz: compile only in 64 bit little endian Hervé Poussineau
                   ` (16 more replies)
  0 siblings, 17 replies; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Hi,

This patchset improves dp8393x network card emulation to current QEMU standards,
mostly decouples it from MIPS rc4030 chipset emulation, adds PROM and load/save
functionalities, and fixes some bugs seen with NetBSD.
It also converts rc4030 to QOM, and does some cleanup.

Patchset has been tested on MIPS Jazz emulation and on (yet unpublished)
m68k Quadra 800 emulation.

Changes v1->v2:
- added patches 1, 5-7, 14-16
- fixed memory region life cycle in rc4030 (patch 2)
- removed a loop around address_space_rw (patch 2)
- added RFC patch 17, which highlights a bug somewhere

Hervé Poussineau (17):
  mips jazz: compile only in 64 bit little endian
  dma/rc4030: create custom DMA address space
  dma/rc4030: use AddressSpace and address_space_rw in users
  dma/rc4030: do not use old_mmio accesses
  dma/rc4030: document register at offset 0x210
  dma/rc4030: use trace events instead of custom logging
  dma/rc4030: convert to QOM
  net/dp8393x: always calculate proper checksums
  net/dp8393x: do not use old_mmio accesses
  net/dp8393x: use dp8393x_ prefix for all functions
  net/dp8393x: QOM'ify
  net/dp8393x: add PROM to store MAC address
  net/dp8393x: add load/save support
  net/dp8393x: correctly reset in_use field
  net/dp8393x: fix hardware reset
  net/dp8393x: repair can_receive() method
  [RFC] dma/rc4030: do multiple calls to address_space_rw when doing DMA
    transfers

 default-configs/mips-softmmu.mak     |   5 -
 default-configs/mips64-softmmu.mak   |   5 -
 default-configs/mips64el-softmmu.mak |   1 +
 default-configs/mipsel-softmmu.mak   |   5 -
 hw/dma/rc4030.c                      | 472 ++++++++++++++++++-----------------
 hw/mips/Makefile.objs                |   3 +-
 hw/mips/mips_jazz.c                  |  53 ++--
 hw/net/dp8393x.c                     | 377 ++++++++++++++--------------
 include/hw/mips/mips.h               |  11 +-
 tests/endianness-test.c              |   4 -
 trace-events                         |   6 +
 11 files changed, 481 insertions(+), 461 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 01/17] mips jazz: compile only in 64 bit little endian
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:02   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 02/17] dma/rc4030: create custom DMA address space Hervé Poussineau
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Remove now useless device models from other MIPS configurations

We're now compiling 18 files less than before.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 default-configs/mips-softmmu.mak     | 5 -----
 default-configs/mips64-softmmu.mak   | 5 -----
 default-configs/mips64el-softmmu.mak | 1 +
 default-configs/mipsel-softmmu.mak   | 5 -----
 hw/mips/Makefile.objs                | 3 ++-
 hw/mips/mips_jazz.c                  | 5 -----
 tests/endianness-test.c              | 4 ----
 7 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak
index cce2c81..f62a21a 100644
--- a/default-configs/mips-softmmu.mak
+++ b/default-configs/mips-softmmu.mak
@@ -21,14 +21,9 @@ CONFIG_PIIX4=y
 CONFIG_IDE_ISA=y
 CONFIG_IDE_PIIX=y
 CONFIG_NE2000_ISA=y
-CONFIG_RC4030=y
-CONFIG_DP8393X=y
-CONFIG_DS1225Y=y
 CONFIG_MIPSNET=y
 CONFIG_PFLASH_CFI01=y
-CONFIG_G364FB=y
 CONFIG_I8259=y
-CONFIG_JAZZ_LED=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_EMPTY_SLOT=y
diff --git a/default-configs/mips64-softmmu.mak b/default-configs/mips64-softmmu.mak
index 7a88a08..accedca 100644
--- a/default-configs/mips64-softmmu.mak
+++ b/default-configs/mips64-softmmu.mak
@@ -21,14 +21,9 @@ CONFIG_PIIX4=y
 CONFIG_IDE_ISA=y
 CONFIG_IDE_PIIX=y
 CONFIG_NE2000_ISA=y
-CONFIG_RC4030=y
-CONFIG_DP8393X=y
-CONFIG_DS1225Y=y
 CONFIG_MIPSNET=y
 CONFIG_PFLASH_CFI01=y
-CONFIG_G364FB=y
 CONFIG_I8259=y
-CONFIG_JAZZ_LED=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_EMPTY_SLOT=y
diff --git a/default-configs/mips64el-softmmu.mak b/default-configs/mips64el-softmmu.mak
index 095de43..24ff6b9 100644
--- a/default-configs/mips64el-softmmu.mak
+++ b/default-configs/mips64el-softmmu.mak
@@ -28,6 +28,7 @@ CONFIG_DS1225Y=y
 CONFIG_MIPSNET=y
 CONFIG_PFLASH_CFI01=y
 CONFIG_FULONG=y
+CONFIG_JAZZ=y
 CONFIG_G364FB=y
 CONFIG_I8259=y
 CONFIG_JAZZ_LED=y
diff --git a/default-configs/mipsel-softmmu.mak b/default-configs/mipsel-softmmu.mak
index 0e25108..9fbee82 100644
--- a/default-configs/mipsel-softmmu.mak
+++ b/default-configs/mipsel-softmmu.mak
@@ -21,14 +21,9 @@ CONFIG_PIIX4=y
 CONFIG_IDE_ISA=y
 CONFIG_IDE_PIIX=y
 CONFIG_NE2000_ISA=y
-CONFIG_RC4030=y
-CONFIG_DP8393X=y
-CONFIG_DS1225Y=y
 CONFIG_MIPSNET=y
 CONFIG_PFLASH_CFI01=y
-CONFIG_G364FB=y
 CONFIG_I8259=y
-CONFIG_JAZZ_LED=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_EMPTY_SLOT=y
diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
index 0a652f8..9633f3a 100644
--- a/hw/mips/Makefile.objs
+++ b/hw/mips/Makefile.objs
@@ -1,4 +1,5 @@
-obj-y += mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
+obj-y += mips_r4k.o mips_malta.o mips_mipssim.o
 obj-y += addr.o cputimer.o mips_int.o
+obj-$(CONFIG_JAZZ) += mips_jazz.o
 obj-$(CONFIG_FULONG) += mips_fulong2e.o
 obj-y += gt64xxx_pci.o
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 2c153e0..f16070e 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -157,12 +157,7 @@ static void mips_jazz_init(MachineState *machine,
 
     /* init CPUs */
     if (cpu_model == NULL) {
-#ifdef TARGET_MIPS64
         cpu_model = "R4000";
-#else
-        /* FIXME: All wrong, this maybe should be R3000 for the older JAZZs. */
-        cpu_model = "24Kf";
-#endif
     }
     cpu = cpu_mips_init(cpu_model);
     if (cpu == NULL) {
diff --git a/tests/endianness-test.c b/tests/endianness-test.c
index 92e17d2..9506da0 100644
--- a/tests/endianness-test.c
+++ b/tests/endianness-test.c
@@ -31,12 +31,8 @@ struct TestCase {
 
 static const TestCase test_cases[] = {
     { "i386", "pc", -1 },
-    { "mips", "magnum", 0x90000000, .bswap = true },
-    { "mips", "pica61", 0x90000000, .bswap = true },
     { "mips", "mips", 0x14000000, .bswap = true },
     { "mips", "malta", 0x10000000, .bswap = true },
-    { "mips64", "magnum", 0x90000000, .bswap = true },
-    { "mips64", "pica61", 0x90000000, .bswap = true },
     { "mips64", "mips", 0x14000000, .bswap = true },
     { "mips64", "malta", 0x10000000, .bswap = true },
     { "mips64el", "fulong2e", 0x1fd00000 },
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 02/17] dma/rc4030: create custom DMA address space
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 01/17] mips jazz: compile only in 64 bit little endian Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:02   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 03/17] dma/rc4030: use AddressSpace and address_space_rw in users Hervé Poussineau
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno, Paolo Bonzini

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>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
 hw/dma/rc4030.c | 163 +++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 126 insertions(+), 37 deletions(-)

diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index af26632..84039dc 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,16 @@ 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;
     MemoryRegion iomem_jazzio;
 } rc4030State;
@@ -265,6 +278,97 @@ 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_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, NULL,
+                                 "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, NULL,
+                           "dma-table-alias", 0);
+    }
+}
+
+
 static void rc4030_writel(void *opaque, hwaddr addr, uint32_t val)
 {
     rc4030State *s = opaque;
@@ -279,11 +383,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 +694,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 +779,8 @@ 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, MEMTXATTRS_UNSPECIFIED, buf, len,
+                     is_write);
 }
 
 static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_write)
@@ -733,7 +806,8 @@ 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);
+    address_space_rw(&s->dma_as, dma_addr, MEMTXATTRS_UNSPECIFIED,
+                     buf, len, is_write);
 
     s->dma_regs[n][DMA_REG_ENABLE] |= DMA_FLAG_TC_INTR;
     s->dma_regs[n][DMA_REG_COUNT] -= len;
@@ -800,6 +874,7 @@ void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
                   MemoryRegion *sysmem)
 {
     rc4030State *s;
+    int i;
 
     s = g_malloc0(sizeof(rc4030State));
 
@@ -821,5 +896,19 @@ 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_rom_device(&s->dma_tt, NULL,
+                                  &rc4030_dma_tt_ops, s, "dma-table",
+                                  MAX_TL_ENTRIES * sizeof(dma_pagetable_entry),
+                                  NULL);
+    memory_region_init(&s->dma_tt_alias, NULL, "dma-table-alias", 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;
 }
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 03/17] dma/rc4030: use AddressSpace and address_space_rw in users
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 01/17] mips jazz: compile only in 64 bit little endian Hervé Poussineau
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 02/17] dma/rc4030: create custom DMA address space Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:02   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 04/17] dma/rc4030: do not use old_mmio accesses Hervé Poussineau
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Now that rc4030 internally uses an AddressSpace for DMA handling, make its root
memory region public. This is especially usefull for dp8393x netcard, which now
uses well known QEMU types and methods.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/dma/rc4030.c        | 15 ++++---------
 hw/mips/mips_jazz.c    |  6 ++---
 hw/net/dp8393x.c       | 61 +++++++++++++++++++++++++-------------------------
 include/hw/mips/mips.h | 10 ++++-----
 4 files changed, 42 insertions(+), 50 deletions(-)

diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index 84039dc..a0b617f 100644
--- a/hw/dma/rc4030.c
+++ b/hw/dma/rc4030.c
@@ -776,13 +776,6 @@ static void rc4030_save(QEMUFile *f, void *opaque)
     qemu_put_be32(f, s->itr);
 }
 
-void rc4030_dma_memory_rw(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write)
-{
-    rc4030State *s = opaque;
-    address_space_rw(&s->dma_as, addr, MEMTXATTRS_UNSPECIFIED, buf, len,
-                     is_write);
-}
-
 static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_write)
 {
     rc4030State *s = opaque;
@@ -869,9 +862,9 @@ static rc4030_dma *rc4030_allocate_dmas(void *opaque, int n)
     return s;
 }
 
-void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
-                  qemu_irq **irqs, rc4030_dma **dmas,
-                  MemoryRegion *sysmem)
+MemoryRegion *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
+                          qemu_irq **irqs, rc4030_dma **dmas,
+                          MemoryRegion *sysmem)
 {
     rc4030State *s;
     int i;
@@ -910,5 +903,5 @@ void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
                                     &s->dma_mrs[i]);
     }
     address_space_init(&s->dma_as, &s->dma_mr, "rc4030-dma");
-    return s;
+    return &s->dma_mr;
 }
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index f16070e..05cad6b 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -137,7 +137,7 @@ static void mips_jazz_init(MachineState *machine,
     CPUMIPSState *env;
     qemu_irq *rc4030, *i8259;
     rc4030_dma *dmas;
-    void* rc4030_opaque;
+    MemoryRegion *rc4030_dma_mr;
     MemoryRegion *isa_mem = g_new(MemoryRegion, 1);
     MemoryRegion *isa_io = g_new(MemoryRegion, 1);
     MemoryRegion *rtc = g_new(MemoryRegion, 1);
@@ -213,7 +213,7 @@ static void mips_jazz_init(MachineState *machine,
     cpu_mips_clock_init(env);
 
     /* Chipset */
-    rc4030_opaque = rc4030_init(env->irq[6], env->irq[3], &rc4030, &dmas,
+    rc4030_dma_mr = rc4030_init(env->irq[6], env->irq[3], &rc4030, &dmas,
                                 address_space);
     memory_region_init_io(dma_dummy, NULL, &dma_dummy_ops, NULL, "dummy_dma", 0x1000);
     memory_region_add_subregion(address_space, 0x8000d000, dma_dummy);
@@ -268,7 +268,7 @@ static void mips_jazz_init(MachineState *machine,
             nd->model = g_strdup("dp83932");
         if (strcmp(nd->model, "dp83932") == 0) {
             dp83932_init(nd, 0x80001000, 2, get_system_memory(), rc4030[4],
-                         rc4030_opaque, rc4030_dma_memory_rw);
+                         rc4030_dma_mr);
             break;
         } else if (is_help_option(nd->model)) {
             fprintf(stderr, "qemu: Supported NICs: dp83932\n");
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 7ce13d2..2297231 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -168,8 +168,7 @@ typedef struct dp8393xState {
     int loopback_packet;
 
     /* Memory access */
-    void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write);
-    void* mem_opaque;
+    AddressSpace as;
 } dp8393xState;
 
 static void dp8393x_update_irq(dp8393xState *s)
@@ -201,9 +200,9 @@ static void do_load_cam(dp8393xState *s)
 
     while (s->regs[SONIC_CDC] & 0x1f) {
         /* Fill current entry */
-        s->memory_rw(s->mem_opaque,
+        address_space_rw(&s->as,
             (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
-            (uint8_t *)data, size, 0);
+            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
         s->cam[index][0] = data[1 * width] & 0xff;
         s->cam[index][1] = data[1 * width] >> 8;
         s->cam[index][2] = data[2 * width] & 0xff;
@@ -220,9 +219,9 @@ static void do_load_cam(dp8393xState *s)
     }
 
     /* Read CAM enable */
-    s->memory_rw(s->mem_opaque,
+    address_space_rw(&s->as,
         (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
-        (uint8_t *)data, size, 0);
+        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
     s->regs[SONIC_CE] = data[0 * width];
     DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]);
 
@@ -240,9 +239,9 @@ static void do_read_rra(dp8393xState *s)
     /* Read memory */
     width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
     size = sizeof(uint16_t) * 4 * width;
-    s->memory_rw(s->mem_opaque,
+    address_space_rw(&s->as,
         (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_RRP],
-        (uint8_t *)data, size, 0);
+        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
 
     /* Update SONIC registers */
     s->regs[SONIC_CRBA0] = data[0 * width];
@@ -353,9 +352,9 @@ static void do_transmit_packets(dp8393xState *s)
                 (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_CTDA]);
         size = sizeof(uint16_t) * 6 * width;
         s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA];
-        s->memory_rw(s->mem_opaque,
+        address_space_rw(&s->as,
             ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * width,
-            (uint8_t *)data, size, 0);
+            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
         tx_len = 0;
 
         /* Update registers */
@@ -379,18 +378,18 @@ static void do_transmit_packets(dp8393xState *s)
             if (tx_len + len > sizeof(s->tx_buffer)) {
                 len = sizeof(s->tx_buffer) - tx_len;
             }
-            s->memory_rw(s->mem_opaque,
+            address_space_rw(&s->as,
                 (s->regs[SONIC_TSA1] << 16) | s->regs[SONIC_TSA0],
-                &s->tx_buffer[tx_len], len, 0);
+                MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0);
             tx_len += len;
 
             i++;
             if (i != s->regs[SONIC_TFC]) {
                 /* Read next fragment details */
                 size = sizeof(uint16_t) * 3 * width;
-                s->memory_rw(s->mem_opaque,
+                address_space_rw(&s->as,
                     ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * i) * width,
-                    (uint8_t *)data, size, 0);
+                    MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
                 s->regs[SONIC_TSA0] = data[0 * width];
                 s->regs[SONIC_TSA1] = data[1 * width];
                 s->regs[SONIC_TFS] = data[2 * width];
@@ -422,16 +421,16 @@ static void do_transmit_packets(dp8393xState *s)
         /* Write status */
         data[0 * width] = s->regs[SONIC_TCR] & 0x0fff; /* status */
         size = sizeof(uint16_t) * width;
-        s->memory_rw(s->mem_opaque,
+        address_space_rw(&s->as,
             (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA],
-            (uint8_t *)data, size, 1);
+            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
 
         if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) {
             /* Read footer of packet */
             size = sizeof(uint16_t) * width;
-            s->memory_rw(s->mem_opaque,
+            address_space_rw(&s->as,
                 ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) * width,
-                (uint8_t *)data, size, 0);
+                MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
             s->regs[SONIC_CTDA] = data[0 * width] & ~0x1;
             if (data[0 * width] & 0x1) {
                 /* EOL detected */
@@ -750,7 +749,8 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
         /* Are we still in resource exhaustion? */
         size = sizeof(uint16_t) * 1 * width;
         address = ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width;
-        s->memory_rw(s->mem_opaque, address, (uint8_t*)data, size, 0);
+        address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED,
+                         (uint8_t *)data, size, 0);
         if (data[0 * width] & 0x1) {
             /* Still EOL ; stop reception */
             return -1;
@@ -773,9 +773,11 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
     /* Put packet into RBA */
     DPRINTF("Receive packet at %08x\n", (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0]);
     address = (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0];
-    s->memory_rw(s->mem_opaque, address, (uint8_t*)buf, rx_len, 1);
+    address_space_rw(&s->as, address,
+        MEMTXATTRS_UNSPECIFIED, (uint8_t *)buf, rx_len, 1);
     address += rx_len;
-    s->memory_rw(s->mem_opaque, address, (uint8_t*)&checksum, 4, 1);
+    address_space_rw(&s->as, address,
+        MEMTXATTRS_UNSPECIFIED, (uint8_t *)&checksum, 4, 1);
     rx_len += 4;
     s->regs[SONIC_CRBA1] = address >> 16;
     s->regs[SONIC_CRBA0] = address & 0xffff;
@@ -803,22 +805,23 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
     data[3 * width] = s->regs[SONIC_TRBA1]; /* pkt_ptr1 */
     data[4 * width] = s->regs[SONIC_RSC]; /* seq_no */
     size = sizeof(uint16_t) * 5 * width;
-    s->memory_rw(s->mem_opaque, (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA], (uint8_t *)data, size, 1);
+    address_space_rw(&s->as, (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA],
+        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
 
     /* Move to next descriptor */
     size = sizeof(uint16_t) * width;
-    s->memory_rw(s->mem_opaque,
+    address_space_rw(&s->as,
         ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width,
-        (uint8_t *)data, size, 0);
+        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
     s->regs[SONIC_LLFA] = data[0 * width];
     if (s->regs[SONIC_LLFA] & 0x1) {
         /* EOL detected */
         s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
     } else {
         data[0 * width] = 0; /* in_use */
-        s->memory_rw(s->mem_opaque,
+        address_space_rw(&s->as,
             ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 6 * width,
-            (uint8_t *)data, size, 1);
+            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
         s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
         s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX;
         s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff);
@@ -868,8 +871,7 @@ static NetClientInfo net_dp83932_info = {
 
 void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
                   MemoryRegion *address_space,
-                  qemu_irq irq, void* mem_opaque,
-                  void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write))
+                  qemu_irq irq, MemoryRegion *dma_mr)
 {
     dp8393xState *s;
 
@@ -878,8 +880,7 @@ void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
     s = g_malloc0(sizeof(dp8393xState));
 
     s->address_space = address_space;
-    s->mem_opaque = mem_opaque;
-    s->memory_rw = memory_rw;
+    address_space_init(&s->as, dma_mr, "dp8393x-dma");
     s->it_shift = it_shift;
     s->irq = irq;
     s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s);
diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
index 2a7a9c9..47eb31f 100644
--- a/include/hw/mips/mips.h
+++ b/include/hw/mips/mips.h
@@ -15,18 +15,16 @@ PCIBus *bonito_init(qemu_irq *pic);
 
 /* rc4030.c */
 typedef struct rc4030DMAState *rc4030_dma;
-void rc4030_dma_memory_rw(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write);
 void rc4030_dma_read(void *dma, uint8_t *buf, int len);
 void rc4030_dma_write(void *dma, uint8_t *buf, int len);
 
-void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
-                  qemu_irq **irqs, rc4030_dma **dmas,
-                  MemoryRegion *sysmem);
+MemoryRegion *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
+                          qemu_irq **irqs, rc4030_dma **dmas,
+                          MemoryRegion *sysmem);
 
 /* dp8393x.c */
 void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
                   MemoryRegion *address_space,
-                  qemu_irq irq, void* mem_opaque,
-                  void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write));
+                  qemu_irq irq, MemoryRegion *dma_mr);
 
 #endif
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 04/17] dma/rc4030: do not use old_mmio accesses
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (2 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 03/17] dma/rc4030: use AddressSpace and address_space_rw in users Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:03   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 05/17] dma/rc4030: document register at offset 0x210 Hervé Poussineau
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

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

diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index a0b617f..96f796b 100644
--- a/hw/dma/rc4030.c
+++ b/hw/dma/rc4030.c
@@ -125,7 +125,7 @@ static void set_next_tick(rc4030State *s)
 }
 
 /* called for accesses to rc4030 */
-static uint32_t rc4030_readl(void *opaque, hwaddr addr)
+static uint64_t rc4030_read(void *opaque, hwaddr addr, unsigned int size)
 {
     rc4030State *s = opaque;
     uint32_t val;
@@ -263,21 +263,6 @@ static uint32_t rc4030_readl(void *opaque, hwaddr addr)
     return val;
 }
 
-static uint32_t rc4030_readw(void *opaque, hwaddr addr)
-{
-    uint32_t v = rc4030_readl(opaque, addr & ~0x3);
-    if (addr & 0x2)
-        return v >> 16;
-    else
-        return v & 0xffff;
-}
-
-static uint32_t rc4030_readb(void *opaque, hwaddr addr)
-{
-    uint32_t v = rc4030_readl(opaque, addr & ~0x3);
-    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) {
@@ -368,10 +353,11 @@ static void rc4030_dma_tt_update(rc4030State *s, uint32_t new_tl_base,
     }
 }
 
-
-static void rc4030_writel(void *opaque, hwaddr addr, uint32_t val)
+static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
+                         unsigned int size)
 {
     rc4030State *s = opaque;
+    uint32_t val = data;
     addr &= 0x3fff;
 
     DPRINTF("write 0x%02x at " TARGET_FMT_plx "\n", val, addr);
@@ -494,43 +480,11 @@ static void rc4030_writel(void *opaque, hwaddr addr, uint32_t val)
     }
 }
 
-static void rc4030_writew(void *opaque, hwaddr addr, uint32_t val)
-{
-    uint32_t old_val = rc4030_readl(opaque, addr & ~0x3);
-
-    if (addr & 0x2)
-        val = (val << 16) | (old_val & 0x0000ffff);
-    else
-        val = val | (old_val & 0xffff0000);
-    rc4030_writel(opaque, addr & ~0x3, val);
-}
-
-static void rc4030_writeb(void *opaque, hwaddr addr, uint32_t val)
-{
-    uint32_t old_val = rc4030_readl(opaque, addr & ~0x3);
-
-    switch (addr & 3) {
-    case 0:
-        val = val | (old_val & 0xffffff00);
-        break;
-    case 1:
-        val = (val << 8) | (old_val & 0xffff00ff);
-        break;
-    case 2:
-        val = (val << 16) | (old_val & 0xff00ffff);
-        break;
-    case 3:
-        val = (val << 24) | (old_val & 0x00ffffff);
-        break;
-    }
-    rc4030_writel(opaque, addr & ~0x3, val);
-}
-
 static const MemoryRegionOps rc4030_ops = {
-    .old_mmio = {
-        .read = { rc4030_readb, rc4030_readw, rc4030_readl, },
-        .write = { rc4030_writeb, rc4030_writew, rc4030_writel, },
-    },
+    .read = rc4030_read,
+    .write = rc4030_write,
+    .impl.min_access_size = 4,
+    .impl.max_access_size = 4,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
@@ -583,7 +537,7 @@ static void rc4030_periodic_timer(void *opaque)
     qemu_irq_raise(s->timer_irq);
 }
 
-static uint32_t jazzio_readw(void *opaque, hwaddr addr)
+static uint64_t jazzio_read(void *opaque, hwaddr addr, unsigned int size)
 {
     rc4030State *s = opaque;
     uint32_t val;
@@ -621,24 +575,11 @@ static uint32_t jazzio_readw(void *opaque, hwaddr addr)
     return val;
 }
 
-static uint32_t jazzio_readb(void *opaque, hwaddr addr)
-{
-    uint32_t v;
-    v = jazzio_readw(opaque, addr & ~0x1);
-    return (v >> (8 * (addr & 0x1))) & 0xff;
-}
-
-static uint32_t jazzio_readl(void *opaque, hwaddr addr)
-{
-    uint32_t v;
-    v = jazzio_readw(opaque, addr);
-    v |= jazzio_readw(opaque, addr + 2) << 16;
-    return v;
-}
-
-static void jazzio_writew(void *opaque, hwaddr addr, uint32_t val)
+static void jazzio_write(void *opaque, hwaddr addr, uint64_t data,
+                         unsigned int size)
 {
     rc4030State *s = opaque;
+    uint32_t val = data;
     addr &= 0xfff;
 
     DPRINTF("(jazz io controller) write 0x%04x at " TARGET_FMT_plx "\n", val, addr);
@@ -655,32 +596,11 @@ static void jazzio_writew(void *opaque, hwaddr addr, uint32_t val)
     }
 }
 
-static void jazzio_writeb(void *opaque, hwaddr addr, uint32_t val)
-{
-    uint32_t old_val = jazzio_readw(opaque, addr & ~0x1);
-
-    switch (addr & 1) {
-    case 0:
-        val = val | (old_val & 0xff00);
-        break;
-    case 1:
-        val = (val << 8) | (old_val & 0x00ff);
-        break;
-    }
-    jazzio_writew(opaque, addr & ~0x1, val);
-}
-
-static void jazzio_writel(void *opaque, hwaddr addr, uint32_t val)
-{
-    jazzio_writew(opaque, addr, val & 0xffff);
-    jazzio_writew(opaque, addr + 2, (val >> 16) & 0xffff);
-}
-
 static const MemoryRegionOps jazzio_ops = {
-    .old_mmio = {
-        .read = { jazzio_readb, jazzio_readw, jazzio_readl, },
-        .write = { jazzio_writeb, jazzio_writew, jazzio_writel, },
-    },
+    .read = jazzio_read,
+    .write = jazzio_write,
+    .impl.min_access_size = 2,
+    .impl.max_access_size = 2,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 05/17] dma/rc4030: document register at offset 0x210
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (3 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 04/17] dma/rc4030: do not use old_mmio accesses Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:03   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 06/17] dma/rc4030: use trace events instead of custom logging Hervé Poussineau
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

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

diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index 96f796b..bf82eed 100644
--- a/hw/dma/rc4030.c
+++ b/hw/dma/rc4030.c
@@ -86,7 +86,7 @@ typedef struct rc4030State
     uint32_t cache_bmask; /* 0x0058: I/O Cache Byte Mask */
 
     uint32_t nmi_interrupt; /* 0x0200: interrupt source */
-    uint32_t offset210;
+    uint32_t memory_refresh_rate; /* 0x0210: memory refresh rate */
     uint32_t nvram_protect; /* 0x0220: NV ram protect register */
     uint32_t rem_speed[16];
     uint32_t imr_jazz; /* Local bus int enable mask */
@@ -233,9 +233,9 @@ static uint64_t rc4030_read(void *opaque, hwaddr addr, unsigned int size)
     case 0x0208:
         val = 0;
         break;
-    /* Offset 0x0210 */
+    /* Memory refresh rate */
     case 0x0210:
-        val = s->offset210;
+        val = s->memory_refresh_rate;
         break;
     /* NV ram protect register */
     case 0x0220:
@@ -461,9 +461,9 @@ static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
             s->dma_regs[entry][idx] = val;
         }
         break;
-    /* Offset 0x0210 */
+    /* Memory refresh rate */
     case 0x0210:
-        s->offset210 = val;
+        s->memory_refresh_rate = val;
         break;
     /* Interval timer reload */
     case 0x0228:
@@ -621,7 +621,7 @@ static void rc4030_reset(void *opaque)
     s->cache_ptag = s->cache_ltag = 0;
     s->cache_bmask = 0;
 
-    s->offset210 = 0x18186;
+    s->memory_refresh_rate = 0x18186;
     s->nvram_protect = 7;
     for (i = 0; i < 15; i++)
         s->rem_speed[i] = 7;
@@ -655,7 +655,7 @@ static int rc4030_load(QEMUFile *f, void *opaque, int version_id)
     s->cache_ptag = qemu_get_be32(f);
     s->cache_ltag = qemu_get_be32(f);
     s->cache_bmask = qemu_get_be32(f);
-    s->offset210 = qemu_get_be32(f);
+    s->memory_refresh_rate = qemu_get_be32(f);
     s->nvram_protect = qemu_get_be32(f);
     for (i = 0; i < 15; i++)
         s->rem_speed[i] = qemu_get_be32(f);
@@ -687,7 +687,7 @@ static void rc4030_save(QEMUFile *f, void *opaque)
     qemu_put_be32(f, s->cache_ptag);
     qemu_put_be32(f, s->cache_ltag);
     qemu_put_be32(f, s->cache_bmask);
-    qemu_put_be32(f, s->offset210);
+    qemu_put_be32(f, s->memory_refresh_rate);
     qemu_put_be32(f, s->nvram_protect);
     for (i = 0; i < 15; i++)
         qemu_put_be32(f, s->rem_speed[i]);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 06/17] dma/rc4030: use trace events instead of custom logging
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (4 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 05/17] dma/rc4030: document register at offset 0x210 Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:03   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 07/17] dma/rc4030: convert to QOM Hervé Poussineau
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Remove also unneeded debug logs.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/dma/rc4030.c | 81 ++++++++++++---------------------------------------------
 trace-events    |  6 +++++
 2 files changed, 22 insertions(+), 65 deletions(-)

diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index bf82eed..55844ed 100644
--- a/hw/dma/rc4030.c
+++ b/hw/dma/rc4030.c
@@ -26,24 +26,7 @@
 #include "hw/mips/mips.h"
 #include "qemu/timer.h"
 #include "exec/address-spaces.h"
-
-/********************************************************/
-/* debug rc4030 */
-
-//#define DEBUG_RC4030
-//#define DEBUG_RC4030_DMA
-
-#ifdef DEBUG_RC4030
-#define DPRINTF(fmt, ...) \
-do { printf("rc4030: " fmt , ## __VA_ARGS__); } while (0)
-static const char* irq_names[] = { "parallel", "floppy", "sound", "video",
-            "network", "scsi", "keyboard", "mouse", "serial0", "serial1" };
-#else
-#define DPRINTF(fmt, ...)
-#endif
-
-#define RC4030_ERROR(fmt, ...) \
-do { fprintf(stderr, "rc4030 ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); } while (0)
+#include "trace.h"
 
 /********************************************************/
 /* rc4030 emulation                                     */
@@ -251,13 +234,14 @@ static uint64_t rc4030_read(void *opaque, hwaddr addr, unsigned int size)
         val = 7; /* FIXME: should be read from EISA controller */
         break;
     default:
-        RC4030_ERROR("invalid read [" TARGET_FMT_plx "]\n", addr);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "rc4030: invalid read at 0x%x", (int)addr);
         val = 0;
         break;
     }
 
     if ((addr & ~3) != 0x230) {
-        DPRINTF("read 0x%02x at " TARGET_FMT_plx "\n", val, addr);
+        trace_rc4030_read(addr, val);
     }
 
     return val;
@@ -360,7 +344,7 @@ static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
     uint32_t val = data;
     addr &= 0x3fff;
 
-    DPRINTF("write 0x%02x at " TARGET_FMT_plx "\n", val, addr);
+    trace_rc4030_write(addr, val);
 
     switch (addr & ~0x3) {
     /* Global config register */
@@ -475,7 +459,9 @@ static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
     case 0x0238:
         break;
     default:
-        RC4030_ERROR("invalid write of 0x%02x at [" TARGET_FMT_plx "]\n", val, addr);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "rc4030: invalid write of 0x%02x at 0x%x",
+                      val, (int)addr);
         break;
     }
 }
@@ -494,22 +480,6 @@ static void update_jazz_irq(rc4030State *s)
 
     pending = s->isr_jazz & s->imr_jazz;
 
-#ifdef DEBUG_RC4030
-    if (s->isr_jazz != 0) {
-        uint32_t irq = 0;
-        DPRINTF("pending irqs:");
-        for (irq = 0; irq < ARRAY_SIZE(irq_names); irq++) {
-            if (s->isr_jazz & (1 << irq)) {
-                printf(" %s", irq_names[irq]);
-                if (!(s->imr_jazz & (1 << irq))) {
-                    printf("(ignored)");
-                }
-            }
-        }
-        printf("\n");
-    }
-#endif
-
     if (pending != 0)
         qemu_irq_raise(s->jazz_bus_irq);
     else
@@ -552,7 +522,6 @@ static uint64_t jazzio_read(void *opaque, hwaddr addr, unsigned int size)
         irq = 0;
         while (pending) {
             if (pending & 1) {
-                DPRINTF("returning irq %s\n", irq_names[irq]);
                 val = (irq + 1) << 2;
                 break;
             }
@@ -566,11 +535,13 @@ static uint64_t jazzio_read(void *opaque, hwaddr addr, unsigned int size)
         val = s->imr_jazz;
         break;
     default:
-        RC4030_ERROR("(jazz io controller) invalid read [" TARGET_FMT_plx "]\n", addr);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "rc4030/jazzio: invalid read at 0x%x", (int)addr);
         val = 0;
+        break;
     }
 
-    DPRINTF("(jazz io controller) read 0x%04x at " TARGET_FMT_plx "\n", val, addr);
+    trace_jazzio_read(addr, val);
 
     return val;
 }
@@ -582,7 +553,7 @@ static void jazzio_write(void *opaque, hwaddr addr, uint64_t data,
     uint32_t val = data;
     addr &= 0xfff;
 
-    DPRINTF("(jazz io controller) write 0x%04x at " TARGET_FMT_plx "\n", val, addr);
+    trace_jazzio_write(addr, val);
 
     switch (addr) {
     /* Local bus int enable mask */
@@ -591,7 +562,9 @@ static void jazzio_write(void *opaque, hwaddr addr, uint64_t data,
         update_jazz_irq(s);
         break;
     default:
-        RC4030_ERROR("(jazz io controller) invalid write of 0x%04x at [" TARGET_FMT_plx "]\n", val, addr);
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "rc4030/jazzio: invalid write of 0x%02x at 0x%x",
+                      val, (int)addr);
         break;
     }
 }
@@ -724,28 +697,6 @@ static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_wri
 
     s->dma_regs[n][DMA_REG_ENABLE] |= DMA_FLAG_TC_INTR;
     s->dma_regs[n][DMA_REG_COUNT] -= len;
-
-#ifdef DEBUG_RC4030_DMA
-    {
-        int i, j;
-        printf("rc4030 dma: Copying %d bytes %s host %p\n",
-            len, is_write ? "from" : "to", buf);
-        for (i = 0; i < len; i += 16) {
-            int n = 16;
-            if (n > len - i) {
-                n = len - i;
-            }
-            for (j = 0; j < n; j++)
-                printf("%02x ", buf[i + j]);
-            while (j++ < 16)
-                printf("   ");
-            printf("| ");
-            for (j = 0; j < n; j++)
-                printf("%c", isprint(buf[i + j]) ? buf[i + j] : '.');
-            printf("\n");
-        }
-    }
-#endif
 }
 
 struct rc4030DMAState {
diff --git a/trace-events b/trace-events
index 11387c3..1443e84 100644
--- a/trace-events
+++ b/trace-events
@@ -282,6 +282,12 @@ slavio_timer_mem_writel_mode_counter(unsigned int timer_index) "processor %d cha
 slavio_timer_mem_writel_mode_invalid(void) "not system timer"
 slavio_timer_mem_writel_invalid(uint64_t addr) "invalid write address %"PRIx64
 
+# hw/dma/rc4030.c
+jazzio_read(uint64_t addr, uint32_t ret) "read reg[0x%"PRIx64"] = 0x%x"
+jazzio_write(uint64_t addr, uint32_t val) "write reg[0x%"PRIx64"] = 0x%x"
+rc4030_read(uint64_t addr, uint32_t ret) "read reg[0x%"PRIx64"] = 0x%x"
+rc4030_write(uint64_t addr, uint32_t val) "write reg[0x%"PRIx64"] = 0x%x"
+
 # hw/dma/sparc32_dma.c
 ledma_memory_read(uint64_t addr) "DMA read addr 0x%"PRIx64
 ledma_memory_write(uint64_t addr) "DMA write addr 0x%"PRIx64
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 07/17] dma/rc4030: convert to QOM
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (5 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 06/17] dma/rc4030: use trace events instead of custom logging Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:03   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 08/17] net/dp8393x: always calculate proper checksums Hervé Poussineau
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/dma/rc4030.c        | 115 ++++++++++++++++++++++++++++++++++++++-----------
 hw/mips/mips_jazz.c    |  37 ++++++++++------
 include/hw/mips/mips.h |   4 +-
 3 files changed, 113 insertions(+), 43 deletions(-)

diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index 55844ed..3efa6de 100644
--- a/hw/dma/rc4030.c
+++ b/hw/dma/rc4030.c
@@ -1,7 +1,7 @@
 /*
  * QEMU JAZZ RC4030 chipset
  *
- * Copyright (c) 2007-2009 Herve Poussineau
+ * Copyright (c) 2007-2013 Hervé Poussineau
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -24,6 +24,7 @@
 
 #include "hw/hw.h"
 #include "hw/mips/mips.h"
+#include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "exec/address-spaces.h"
 #include "trace.h"
@@ -49,8 +50,14 @@ typedef struct dma_pagetable_entry {
 #define DMA_FLAG_MEM_INTR   0x0200
 #define DMA_FLAG_ADDR_INTR  0x0400
 
+#define TYPE_RC4030 "rc4030"
+#define RC4030(obj) \
+    OBJECT_CHECK(rc4030State, (obj), TYPE_RC4030)
+
 typedef struct rc4030State
 {
+    SysBusDevice parent;
+
     uint32_t config; /* 0x0000: RC4030 config register */
     uint32_t revision; /* 0x0008: RC4030 Revision register */
     uint32_t invalid_address_register; /* 0x0010: Invalid Address register */
@@ -317,7 +324,7 @@ static void rc4030_dma_tt_update(rc4030State *s, uint32_t new_tl_base,
         } else {
             dma_tt_size = memory_region_size(&s->dma_tt);
         }
-        memory_region_init_alias(&s->dma_tt_alias, NULL,
+        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);
@@ -332,7 +339,7 @@ static void rc4030_dma_tt_update(rc4030State *s, uint32_t new_tl_base,
                                     &s->dma_tt_alias);
         memory_region_transaction_commit();
     } else {
-        memory_region_init(&s->dma_tt_alias, NULL,
+        memory_region_init(&s->dma_tt_alias, OBJECT(s),
                            "dma-table-alias", 0);
     }
 }
@@ -577,9 +584,9 @@ static const MemoryRegionOps jazzio_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void rc4030_reset(void *opaque)
+static void rc4030_reset(DeviceState *dev)
 {
-    rc4030State *s = opaque;
+    rc4030State *s = RC4030(dev);
     int i;
 
     s->config = 0x410; /* some boards seem to accept 0x104 too */
@@ -733,46 +740,102 @@ static rc4030_dma *rc4030_allocate_dmas(void *opaque, int n)
     return s;
 }
 
-MemoryRegion *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
-                          qemu_irq **irqs, rc4030_dma **dmas,
-                          MemoryRegion *sysmem)
+static void rc4030_initfn(Object *obj)
 {
-    rc4030State *s;
-    int i;
-
-    s = g_malloc0(sizeof(rc4030State));
+    DeviceState *dev = DEVICE(obj);
+    rc4030State *s = RC4030(obj);
+    SysBusDevice *sysbus = SYS_BUS_DEVICE(obj);
 
-    *irqs = qemu_allocate_irqs(rc4030_irq_jazz_request, s, 16);
-    *dmas = rc4030_allocate_dmas(s, 4);
+    qdev_init_gpio_in(dev, rc4030_irq_jazz_request, 16);
 
-    s->periodic_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, rc4030_periodic_timer, s);
-    s->timer_irq = timer;
-    s->jazz_bus_irq = jazz_bus;
+    sysbus_init_irq(sysbus, &s->timer_irq);
+    sysbus_init_irq(sysbus, &s->jazz_bus_irq);
 
-    qemu_register_reset(rc4030_reset, s);
     register_savevm(NULL, "rc4030", 0, 2, rc4030_save, rc4030_load, s);
-    rc4030_reset(s);
+
+    sysbus_init_mmio(sysbus, &s->iomem_chipset);
+    sysbus_init_mmio(sysbus, &s->iomem_jazzio);
+}
+
+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);
 
     memory_region_init_io(&s->iomem_chipset, NULL, &rc4030_ops, s,
                           "rc4030.chipset", 0x300);
-    memory_region_add_subregion(sysmem, 0x80000000, &s->iomem_chipset);
     memory_region_init_io(&s->iomem_jazzio, NULL, &jazzio_ops, s,
                           "rc4030.jazzio", 0x00001000);
-    memory_region_add_subregion(sysmem, 0xf0000000, &s->iomem_jazzio);
 
-    memory_region_init_rom_device(&s->dma_tt, NULL,
+    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, NULL, "dma-table-alias", 0);
-    memory_region_init(&s->dma_mr, NULL, "dma", INT32_MAX);
+    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], NULL, "dma-alias",
+        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]);
     }
     address_space_init(&s->dma_as, &s->dma_mr, "rc4030-dma");
-    return &s->dma_mr;
+}
+
+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)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = rc4030_realize;
+    dc->unrealize = rc4030_unrealize;
+    dc->reset = rc4030_reset;
+}
+
+static const TypeInfo rc4030_info = {
+    .name = TYPE_RC4030,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(rc4030State),
+    .instance_init = rc4030_initfn,
+    .class_init = rc4030_class_init,
+};
+
+static void rc4030_register_types(void)
+{
+    type_register_static(&rc4030_info);
+}
+
+type_init(rc4030_register_types)
+
+DeviceState *rc4030_init(rc4030_dma **dmas, MemoryRegion **dma_mr)
+{
+    DeviceState *dev;
+
+    dev = qdev_create(NULL, TYPE_RC4030);
+    qdev_init_nofail(dev);
+
+    *dmas = rc4030_allocate_dmas(dev, 4);
+    *dma_mr = &RC4030(dev)->dma_mr;
+    return dev;
 }
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 05cad6b..29a13c0 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -135,7 +135,7 @@ static void mips_jazz_init(MachineState *machine,
     MIPSCPU *cpu;
     CPUClass *cc;
     CPUMIPSState *env;
-    qemu_irq *rc4030, *i8259;
+    qemu_irq *i8259;
     rc4030_dma *dmas;
     MemoryRegion *rc4030_dma_mr;
     MemoryRegion *isa_mem = g_new(MemoryRegion, 1);
@@ -144,7 +144,7 @@ static void mips_jazz_init(MachineState *machine,
     MemoryRegion *i8042 = g_new(MemoryRegion, 1);
     MemoryRegion *dma_dummy = g_new(MemoryRegion, 1);
     NICInfo *nd;
-    DeviceState *dev;
+    DeviceState *dev, *rc4030;
     SysBusDevice *sysbus;
     ISABus *isa_bus;
     ISADevice *pit;
@@ -213,8 +213,14 @@ static void mips_jazz_init(MachineState *machine,
     cpu_mips_clock_init(env);
 
     /* Chipset */
-    rc4030_dma_mr = rc4030_init(env->irq[6], env->irq[3], &rc4030, &dmas,
-                                address_space);
+    rc4030 = rc4030_init(&dmas, &rc4030_dma_mr);
+    sysbus = SYS_BUS_DEVICE(rc4030);
+    sysbus_connect_irq(sysbus, 0, env->irq[6]);
+    sysbus_connect_irq(sysbus, 1, env->irq[3]);
+    memory_region_add_subregion(address_space, 0x80000000,
+                                sysbus_mmio_get_region(sysbus, 0));
+    memory_region_add_subregion(address_space, 0xf0000000,
+                                sysbus_mmio_get_region(sysbus, 1));
     memory_region_init_io(dma_dummy, NULL, &dma_dummy_ops, NULL, "dummy_dma", 0x1000);
     memory_region_add_subregion(address_space, 0x8000d000, dma_dummy);
 
@@ -241,7 +247,7 @@ static void mips_jazz_init(MachineState *machine,
         sysbus = SYS_BUS_DEVICE(dev);
         sysbus_mmio_map(sysbus, 0, 0x60080000);
         sysbus_mmio_map(sysbus, 1, 0x40000000);
-        sysbus_connect_irq(sysbus, 0, rc4030[3]);
+        sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(rc4030, 3));
         {
             /* Simple ROM, so user doesn't have to provide one */
             MemoryRegion *rom_mr = g_new(MemoryRegion, 1);
@@ -267,8 +273,8 @@ static void mips_jazz_init(MachineState *machine,
         if (!nd->model)
             nd->model = g_strdup("dp83932");
         if (strcmp(nd->model, "dp83932") == 0) {
-            dp83932_init(nd, 0x80001000, 2, get_system_memory(), rc4030[4],
-                         rc4030_dma_mr);
+            dp83932_init(nd, 0x80001000, 2, get_system_memory(),
+                         qdev_get_gpio_in(rc4030, 4), rc4030_dma_mr);
             break;
         } else if (is_help_option(nd->model)) {
             fprintf(stderr, "qemu: Supported NICs: dp83932\n");
@@ -282,7 +288,7 @@ static void mips_jazz_init(MachineState *machine,
     /* SCSI adapter */
     esp_init(0x80002000, 0,
              rc4030_dma_read, rc4030_dma_write, dmas[0],
-             rc4030[5], &esp_reset, &dma_enable);
+             qdev_get_gpio_in(rc4030, 5), &esp_reset, &dma_enable);
 
     /* Floppy */
     if (drive_get_max_bus(IF_FLOPPY) >= MAX_FD) {
@@ -292,7 +298,7 @@ static void mips_jazz_init(MachineState *machine,
     for (n = 0; n < MAX_FD; n++) {
         fds[n] = drive_get(IF_FLOPPY, 0, n);
     }
-    fdctrl_init_sysbus(rc4030[1], 0, 0x80003000, fds);
+    fdctrl_init_sysbus(qdev_get_gpio_in(rc4030, 1), 0, 0x80003000, fds);
 
     /* Real time clock */
     rtc_init(isa_bus, 1980, NULL);
@@ -300,23 +306,26 @@ static void mips_jazz_init(MachineState *machine,
     memory_region_add_subregion(address_space, 0x80004000, rtc);
 
     /* Keyboard (i8042) */
-    i8042_mm_init(rc4030[6], rc4030[7], i8042, 0x1000, 0x1);
+    i8042_mm_init(qdev_get_gpio_in(rc4030, 6), qdev_get_gpio_in(rc4030, 7),
+                  i8042, 0x1000, 0x1);
     memory_region_add_subregion(address_space, 0x80005000, i8042);
 
     /* Serial ports */
     if (serial_hds[0]) {
-        serial_mm_init(address_space, 0x80006000, 0, rc4030[8], 8000000/16,
+        serial_mm_init(address_space, 0x80006000, 0,
+                       qdev_get_gpio_in(rc4030, 8), 8000000/16,
                        serial_hds[0], DEVICE_NATIVE_ENDIAN);
     }
     if (serial_hds[1]) {
-        serial_mm_init(address_space, 0x80007000, 0, rc4030[9], 8000000/16,
+        serial_mm_init(address_space, 0x80007000, 0,
+                       qdev_get_gpio_in(rc4030, 9), 8000000/16,
                        serial_hds[1], DEVICE_NATIVE_ENDIAN);
     }
 
     /* Parallel port */
     if (parallel_hds[0])
-        parallel_mm_init(address_space, 0x80008000, 0, rc4030[0],
-                         parallel_hds[0]);
+        parallel_mm_init(address_space, 0x80008000, 0,
+                         qdev_get_gpio_in(rc4030, 0), parallel_hds[0]);
 
     /* FIXME: missing Jazz sound at 0x8000c000, rc4030[2] */
 
diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
index 47eb31f..31b4729 100644
--- a/include/hw/mips/mips.h
+++ b/include/hw/mips/mips.h
@@ -18,9 +18,7 @@ typedef struct rc4030DMAState *rc4030_dma;
 void rc4030_dma_read(void *dma, uint8_t *buf, int len);
 void rc4030_dma_write(void *dma, uint8_t *buf, int len);
 
-MemoryRegion *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
-                          qemu_irq **irqs, rc4030_dma **dmas,
-                          MemoryRegion *sysmem);
+DeviceState *rc4030_init(rc4030_dma **dmas, MemoryRegion **dma_mr);
 
 /* dp8393x.c */
 void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 08/17] net/dp8393x: always calculate proper checksums
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (6 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 07/17] dma/rc4030: convert to QOM Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:03   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 09/17] net/dp8393x: do not use old_mmio accesses Hervé Poussineau
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/net/dp8393x.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 2297231..093f0cc 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -21,16 +21,10 @@
 #include "qemu/timer.h"
 #include "net/net.h"
 #include "hw/mips/mips.h"
+#include <zlib.h>
 
 //#define DEBUG_SONIC
 
-/* Calculate CRCs properly on Rx packets */
-#define SONIC_CALCULATE_RXCRC
-
-#if defined(SONIC_CALCULATE_RXCRC)
-/* For crc32 */
-#include <zlib.h>
-#endif
 
 #ifdef DEBUG_SONIC
 #define DPRINTF(fmt, ...) \
@@ -764,11 +758,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
     s->regs[SONIC_TRBA0] = s->regs[SONIC_CRBA0];
 
     /* Calculate the ethernet checksum */
-#ifdef SONIC_CALCULATE_RXCRC
     checksum = cpu_to_le32(crc32(0, buf, rx_len));
-#else
-    checksum = 0;
-#endif
 
     /* Put packet into RBA */
     DPRINTF("Receive packet at %08x\n", (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0]);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 09/17] net/dp8393x: do not use old_mmio accesses
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (7 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 08/17] net/dp8393x: always calculate proper checksums Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:03   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 10/17] net/dp8393x: use dp8393x_ prefix for all functions Hervé Poussineau
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/net/dp8393x.c | 114 ++++++++++++++-----------------------------------------
 1 file changed, 29 insertions(+), 85 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 093f0cc..5cc1e6b 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -473,8 +473,10 @@ static void do_command(dp8393xState *s, uint16_t command)
         do_load_cam(s);
 }
 
-static uint16_t read_register(dp8393xState *s, int reg)
+static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size)
 {
+    dp8393xState *s = opaque;
+    int reg = addr >> s->it_shift;
     uint16_t val = 0;
 
     switch (reg) {
@@ -503,14 +505,18 @@ static uint16_t read_register(dp8393xState *s, int reg)
     return val;
 }
 
-static void write_register(dp8393xState *s, int reg, uint16_t val)
+static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
+                          unsigned int size)
 {
-    DPRINTF("write 0x%04x to reg %s\n", val, reg_names[reg]);
+    dp8393xState *s = opaque;
+    int reg = addr >> s->it_shift;
+
+    DPRINTF("write 0x%04x to reg %s\n", (uint16_t)data, reg_names[reg]);
 
     switch (reg) {
         /* Command register */
         case SONIC_CR:
-            do_command(s, val);
+            do_command(s, data);
             break;
         /* Prevent write to read-only registers */
         case SONIC_CAP2:
@@ -523,36 +529,36 @@ static void write_register(dp8393xState *s, int reg, uint16_t val)
         /* Accept write to some registers only when in reset mode */
         case SONIC_DCR:
             if (s->regs[SONIC_CR] & SONIC_CR_RST) {
-                s->regs[reg] = val & 0xbfff;
+                s->regs[reg] = data & 0xbfff;
             } else {
                 DPRINTF("writing to DCR invalid\n");
             }
             break;
         case SONIC_DCR2:
             if (s->regs[SONIC_CR] & SONIC_CR_RST) {
-                s->regs[reg] = val & 0xf017;
+                s->regs[reg] = data & 0xf017;
             } else {
                 DPRINTF("writing to DCR2 invalid\n");
             }
             break;
         /* 12 lower bytes are Read Only */
         case SONIC_TCR:
-            s->regs[reg] = val & 0xf000;
+            s->regs[reg] = data & 0xf000;
             break;
         /* 9 lower bytes are Read Only */
         case SONIC_RCR:
-            s->regs[reg] = val & 0xffe0;
+            s->regs[reg] = data & 0xffe0;
             break;
         /* Ignore most significant bit */
         case SONIC_IMR:
-            s->regs[reg] = val & 0x7fff;
+            s->regs[reg] = data & 0x7fff;
             dp8393x_update_irq(s);
             break;
         /* Clear bits by writing 1 to them */
         case SONIC_ISR:
-            val &= s->regs[reg];
-            s->regs[reg] &= ~val;
-            if (val & SONIC_ISR_RBE) {
+            data &= s->regs[reg];
+            s->regs[reg] &= ~data;
+            if (data & SONIC_ISR_RBE) {
                 do_read_rra(s);
             }
             dp8393x_update_irq(s);
@@ -562,17 +568,17 @@ static void write_register(dp8393xState *s, int reg, uint16_t val)
         case SONIC_REA:
         case SONIC_RRP:
         case SONIC_RWP:
-            s->regs[reg] = val & 0xfffe;
+            s->regs[reg] = data & 0xfffe;
             break;
         /* Invert written value for some registers */
         case SONIC_CRCT:
         case SONIC_FAET:
         case SONIC_MPT:
-            s->regs[reg] = val ^ 0xffff;
+            s->regs[reg] = data ^ 0xffff;
             break;
         /* All other registers have no special contrainst */
         default:
-            s->regs[reg] = val;
+            s->regs[reg] = data;
     }
 
     if (reg == SONIC_WT0 || reg == SONIC_WT1) {
@@ -580,6 +586,14 @@ static void write_register(dp8393xState *s, int reg, uint16_t val)
     }
 }
 
+static const MemoryRegionOps dp8393x_ops = {
+    .read = dp8393x_read,
+    .write = dp8393x_write,
+    .impl.min_access_size = 2,
+    .impl.max_access_size = 2,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
 static void dp8393x_watchdog(void *opaque)
 {
     dp8393xState *s = opaque;
@@ -597,76 +611,6 @@ static void dp8393x_watchdog(void *opaque)
     dp8393x_update_irq(s);
 }
 
-static uint32_t dp8393x_readw(void *opaque, hwaddr addr)
-{
-    dp8393xState *s = opaque;
-    int reg;
-
-    if ((addr & ((1 << s->it_shift) - 1)) != 0) {
-        return 0;
-    }
-
-    reg = addr >> s->it_shift;
-    return read_register(s, reg);
-}
-
-static uint32_t dp8393x_readb(void *opaque, hwaddr addr)
-{
-    uint16_t v = dp8393x_readw(opaque, addr & ~0x1);
-    return (v >> (8 * (addr & 0x1))) & 0xff;
-}
-
-static uint32_t dp8393x_readl(void *opaque, hwaddr addr)
-{
-    uint32_t v;
-    v = dp8393x_readw(opaque, addr);
-    v |= dp8393x_readw(opaque, addr + 2) << 16;
-    return v;
-}
-
-static void dp8393x_writew(void *opaque, hwaddr addr, uint32_t val)
-{
-    dp8393xState *s = opaque;
-    int reg;
-
-    if ((addr & ((1 << s->it_shift) - 1)) != 0) {
-        return;
-    }
-
-    reg = addr >> s->it_shift;
-
-    write_register(s, reg, (uint16_t)val);
-}
-
-static void dp8393x_writeb(void *opaque, hwaddr addr, uint32_t val)
-{
-    uint16_t old_val = dp8393x_readw(opaque, addr & ~0x1);
-
-    switch (addr & 3) {
-    case 0:
-        val = val | (old_val & 0xff00);
-        break;
-    case 1:
-        val = (val << 8) | (old_val & 0x00ff);
-        break;
-    }
-    dp8393x_writew(opaque, addr & ~0x1, val);
-}
-
-static void dp8393x_writel(void *opaque, hwaddr addr, uint32_t val)
-{
-    dp8393x_writew(opaque, addr, val & 0xffff);
-    dp8393x_writew(opaque, addr + 2, (val >> 16) & 0xffff);
-}
-
-static const MemoryRegionOps dp8393x_ops = {
-    .old_mmio = {
-        .read = { dp8393x_readb, dp8393x_readw, dp8393x_readl, },
-        .write = { dp8393x_writeb, dp8393x_writew, dp8393x_writel, },
-    },
-    .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
 static int nic_can_receive(NetClientState *nc)
 {
     dp8393xState *s = qemu_get_nic_opaque(nc);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 10/17] net/dp8393x: use dp8393x_ prefix for all functions
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (8 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 09/17] net/dp8393x: do not use old_mmio accesses Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:04   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 11/17] net/dp8393x: QOM'ify Hervé Poussineau
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/net/dp8393x.c | 80 +++++++++++++++++++++++++++++---------------------------
 1 file changed, 41 insertions(+), 39 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 5cc1e6b..0aff04f 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -183,7 +183,7 @@ static void dp8393x_update_irq(dp8393xState *s)
     qemu_set_irq(s->irq, level);
 }
 
-static void do_load_cam(dp8393xState *s)
+static void dp8393x_do_load_cam(dp8393xState *s)
 {
     uint16_t data[8];
     int width, size;
@@ -225,7 +225,7 @@ static void do_load_cam(dp8393xState *s)
     dp8393x_update_irq(s);
 }
 
-static void do_read_rra(dp8393xState *s)
+static void dp8393x_do_read_rra(dp8393xState *s)
 {
     uint16_t data[8];
     int width, size;
@@ -265,7 +265,7 @@ static void do_read_rra(dp8393xState *s)
     s->regs[SONIC_CR] &= ~SONIC_CR_RRRA;
 }
 
-static void do_software_reset(dp8393xState *s)
+static void dp8393x_do_software_reset(dp8393xState *s)
 {
     timer_del(s->watchdog);
 
@@ -273,7 +273,7 @@ static void do_software_reset(dp8393xState *s)
     s->regs[SONIC_CR] |= SONIC_CR_RST | SONIC_CR_RXDIS;
 }
 
-static void set_next_tick(dp8393xState *s)
+static void dp8393x_set_next_tick(dp8393xState *s)
 {
     uint32_t ticks;
     int64_t delay;
@@ -289,7 +289,7 @@ static void set_next_tick(dp8393xState *s)
     timer_mod(s->watchdog, s->wt_last_update + delay);
 }
 
-static void update_wt_regs(dp8393xState *s)
+static void dp8393x_update_wt_regs(dp8393xState *s)
 {
     int64_t elapsed;
     uint32_t val;
@@ -304,33 +304,33 @@ static void update_wt_regs(dp8393xState *s)
     val -= elapsed / 5000000;
     s->regs[SONIC_WT1] = (val >> 16) & 0xffff;
     s->regs[SONIC_WT0] = (val >> 0)  & 0xffff;
-    set_next_tick(s);
+    dp8393x_set_next_tick(s);
 
 }
 
-static void do_start_timer(dp8393xState *s)
+static void dp8393x_do_start_timer(dp8393xState *s)
 {
     s->regs[SONIC_CR] &= ~SONIC_CR_STP;
-    set_next_tick(s);
+    dp8393x_set_next_tick(s);
 }
 
-static void do_stop_timer(dp8393xState *s)
+static void dp8393x_do_stop_timer(dp8393xState *s)
 {
     s->regs[SONIC_CR] &= ~SONIC_CR_ST;
-    update_wt_regs(s);
+    dp8393x_update_wt_regs(s);
 }
 
-static void do_receiver_enable(dp8393xState *s)
+static void dp8393x_do_receiver_enable(dp8393xState *s)
 {
     s->regs[SONIC_CR] &= ~SONIC_CR_RXDIS;
 }
 
-static void do_receiver_disable(dp8393xState *s)
+static void dp8393x_do_receiver_disable(dp8393xState *s)
 {
     s->regs[SONIC_CR] &= ~SONIC_CR_RXEN;
 }
 
-static void do_transmit_packets(dp8393xState *s)
+static void dp8393x_do_transmit_packets(dp8393xState *s)
 {
     NetClientState *nc = qemu_get_queue(s->nic);
     uint16_t data[12];
@@ -439,12 +439,12 @@ static void do_transmit_packets(dp8393xState *s)
     dp8393x_update_irq(s);
 }
 
-static void do_halt_transmission(dp8393xState *s)
+static void dp8393x_do_halt_transmission(dp8393xState *s)
 {
     /* Nothing to do */
 }
 
-static void do_command(dp8393xState *s, uint16_t command)
+static void dp8393x_do_command(dp8393xState *s, uint16_t command)
 {
     if ((s->regs[SONIC_CR] & SONIC_CR_RST) && !(command & SONIC_CR_RST)) {
         s->regs[SONIC_CR] &= ~SONIC_CR_RST;
@@ -454,23 +454,23 @@ static void do_command(dp8393xState *s, uint16_t command)
     s->regs[SONIC_CR] |= (command & SONIC_CR_MASK);
 
     if (command & SONIC_CR_HTX)
-        do_halt_transmission(s);
+        dp8393x_do_halt_transmission(s);
     if (command & SONIC_CR_TXP)
-        do_transmit_packets(s);
+        dp8393x_do_transmit_packets(s);
     if (command & SONIC_CR_RXDIS)
-        do_receiver_disable(s);
+        dp8393x_do_receiver_disable(s);
     if (command & SONIC_CR_RXEN)
-        do_receiver_enable(s);
+        dp8393x_do_receiver_enable(s);
     if (command & SONIC_CR_STP)
-        do_stop_timer(s);
+        dp8393x_do_stop_timer(s);
     if (command & SONIC_CR_ST)
-        do_start_timer(s);
+        dp8393x_do_start_timer(s);
     if (command & SONIC_CR_RST)
-        do_software_reset(s);
+        dp8393x_do_software_reset(s);
     if (command & SONIC_CR_RRRA)
-        do_read_rra(s);
+        dp8393x_do_read_rra(s);
     if (command & SONIC_CR_LCAM)
-        do_load_cam(s);
+        dp8393x_do_load_cam(s);
 }
 
 static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size)
@@ -483,7 +483,7 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size)
         /* Update data before reading it */
         case SONIC_WT0:
         case SONIC_WT1:
-            update_wt_regs(s);
+            dp8393x_update_wt_regs(s);
             val = s->regs[reg];
             break;
         /* Accept read to some registers only when in reset mode */
@@ -516,7 +516,7 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
     switch (reg) {
         /* Command register */
         case SONIC_CR:
-            do_command(s, data);
+            dp8393x_do_command(s, data);
             break;
         /* Prevent write to read-only registers */
         case SONIC_CAP2:
@@ -559,7 +559,7 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
             data &= s->regs[reg];
             s->regs[reg] &= ~data;
             if (data & SONIC_ISR_RBE) {
-                do_read_rra(s);
+                dp8393x_do_read_rra(s);
             }
             dp8393x_update_irq(s);
             break;
@@ -582,7 +582,7 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
     }
 
     if (reg == SONIC_WT0 || reg == SONIC_WT1) {
-        set_next_tick(s);
+        dp8393x_set_next_tick(s);
     }
 }
 
@@ -604,14 +604,14 @@ static void dp8393x_watchdog(void *opaque)
 
     s->regs[SONIC_WT1] = 0xffff;
     s->regs[SONIC_WT0] = 0xffff;
-    set_next_tick(s);
+    dp8393x_set_next_tick(s);
 
     /* Signal underflow */
     s->regs[SONIC_ISR] |= SONIC_ISR_TC;
     dp8393x_update_irq(s);
 }
 
-static int nic_can_receive(NetClientState *nc)
+static int dp8393x_can_receive(NetClientState *nc)
 {
     dp8393xState *s = qemu_get_nic_opaque(nc);
 
@@ -622,7 +622,8 @@ static int nic_can_receive(NetClientState *nc)
     return 1;
 }
 
-static int receive_filter(dp8393xState *s, const uint8_t * buf, int size)
+static int dp8393x_receive_filter(dp8393xState *s, const uint8_t * buf,
+                                  int size)
 {
     static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
     int i;
@@ -660,7 +661,8 @@ static int receive_filter(dp8393xState *s, const uint8_t * buf, int size)
     return -1;
 }
 
-static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
+static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
+                               size_t size)
 {
     dp8393xState *s = qemu_get_nic_opaque(nc);
     uint16_t data[10];
@@ -674,7 +676,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
     s->regs[SONIC_RCR] &= ~(SONIC_RCR_PRX | SONIC_RCR_LBK | SONIC_RCR_FAER |
         SONIC_RCR_CRCR | SONIC_RCR_LPKT | SONIC_RCR_BC | SONIC_RCR_MC);
 
-    packet_type = receive_filter(s, buf, size);
+    packet_type = dp8393x_receive_filter(s, buf, size);
     if (packet_type < 0) {
         DPRINTF("packet not for netcard\n");
         return -1;
@@ -762,7 +764,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
 
         if (s->regs[SONIC_RCR] & SONIC_RCR_LPKT) {
             /* Read next RRA */
-            do_read_rra(s);
+            dp8393x_do_read_rra(s);
         }
     }
 
@@ -772,7 +774,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
     return size;
 }
 
-static void nic_reset(void *opaque)
+static void dp8393x_reset(void *opaque)
 {
     dp8393xState *s = opaque;
     timer_del(s->watchdog);
@@ -799,8 +801,8 @@ static void nic_reset(void *opaque)
 static NetClientInfo net_dp83932_info = {
     .type = NET_CLIENT_OPTIONS_KIND_NIC,
     .size = sizeof(NICState),
-    .can_receive = nic_can_receive,
-    .receive = nic_receive,
+    .can_receive = dp8393x_can_receive,
+    .receive = dp8393x_receive,
 };
 
 void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
@@ -826,8 +828,8 @@ void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
     s->nic = qemu_new_nic(&net_dp83932_info, &s->conf, nd->model, nd->name, s);
 
     qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
-    qemu_register_reset(nic_reset, s);
-    nic_reset(s);
+    qemu_register_reset(dp8393x_reset, s);
+    dp8393x_reset(s);
 
     memory_region_init_io(&s->mmio, NULL, &dp8393x_ops, s,
                           "dp8393x", 0x40 << it_shift);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 11/17] net/dp8393x: QOM'ify
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (9 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 10/17] net/dp8393x: use dp8393x_ prefix for all functions Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:04   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 12/17] net/dp8393x: add PROM to store MAC address Hervé Poussineau
                   ` (5 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Hervé Poussineau, Leon Alrae, Laurent Vivier, Aurelien Jarno

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/mips/mips_jazz.c    | 12 ++++++--
 hw/net/dp8393x.c       | 83 ++++++++++++++++++++++++++++++++++----------------
 include/hw/mips/mips.h |  5 ---
 3 files changed, 67 insertions(+), 33 deletions(-)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 29a13c0..648654e 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -273,8 +273,16 @@ static void mips_jazz_init(MachineState *machine,
         if (!nd->model)
             nd->model = g_strdup("dp83932");
         if (strcmp(nd->model, "dp83932") == 0) {
-            dp83932_init(nd, 0x80001000, 2, get_system_memory(),
-                         qdev_get_gpio_in(rc4030, 4), rc4030_dma_mr);
+            qemu_check_nic_model(nd, "dp83932");
+
+            dev = qdev_create(NULL, "dp8393x");
+            qdev_set_nic_properties(dev, nd);
+            qdev_prop_set_uint8(dev, "it_shift", 2);
+            qdev_prop_set_ptr(dev, "dma_mr", rc4030_dma_mr);
+            qdev_init_nofail(dev);
+            sysbus = SYS_BUS_DEVICE(dev);
+            sysbus_mmio_map(sysbus, 0, 0x80001000);
+            sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(rc4030, 4));
             break;
         } else if (is_help_option(nd->model)) {
             fprintf(stderr, "qemu: Supported NICs: dp83932\n");
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 0aff04f..51e728b 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -17,10 +17,10 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "hw/hw.h"
-#include "qemu/timer.h"
+#include "hw/sysbus.h"
+#include "hw/devices.h"
 #include "net/net.h"
-#include "hw/mips/mips.h"
+#include "qemu/timer.h"
 #include <zlib.h>
 
 //#define DEBUG_SONIC
@@ -139,9 +139,14 @@ do { printf("sonic ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); } while (0)
 #define SONIC_ISR_PINT   0x0800
 #define SONIC_ISR_LCD    0x1000
 
+#define TYPE_DP8393X "dp8393x"
+#define DP8393X(obj) OBJECT_CHECK(dp8393xState, (obj), TYPE_DP8393X)
+
 typedef struct dp8393xState {
+    SysBusDevice parent_obj;
+
     /* Hardware */
-    int it_shift;
+    uint8_t it_shift;
     qemu_irq irq;
 #ifdef DEBUG_SONIC
     int irq_level;
@@ -150,7 +155,6 @@ typedef struct dp8393xState {
     int64_t wt_last_update;
     NICConf conf;
     NICState *nic;
-    MemoryRegion *address_space;
     MemoryRegion mmio;
 
     /* Registers */
@@ -162,6 +166,7 @@ typedef struct dp8393xState {
     int loopback_packet;
 
     /* Memory access */
+    void *dma_mr;
     AddressSpace as;
 } dp8393xState;
 
@@ -774,9 +779,9 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     return size;
 }
 
-static void dp8393x_reset(void *opaque)
+static void dp8393x_reset(DeviceState *dev)
 {
-    dp8393xState *s = opaque;
+    dp8393xState *s = DP8393X(dev);
     timer_del(s->watchdog);
 
     s->regs[SONIC_CR] = SONIC_CR_RST | SONIC_CR_STP | SONIC_CR_RXDIS;
@@ -805,33 +810,59 @@ static NetClientInfo net_dp83932_info = {
     .receive = dp8393x_receive,
 };
 
-void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
-                  MemoryRegion *address_space,
-                  qemu_irq irq, MemoryRegion *dma_mr)
+static void dp8393x_instance_init(Object *obj)
 {
-    dp8393xState *s;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    dp8393xState *s = DP8393X(obj);
 
-    qemu_check_nic_model(nd, "dp83932");
+    sysbus_init_mmio(sbd, &s->mmio);
+    sysbus_init_irq(sbd, &s->irq);
+}
+
+static void dp8393x_realize(DeviceState *dev, Error **errp)
+{
+    dp8393xState *s = DP8393X(dev);
 
-    s = g_malloc0(sizeof(dp8393xState));
+    address_space_init(&s->as, s->dma_mr, "dp8393x");
+    memory_region_init_io(&s->mmio, OBJECT(dev), &dp8393x_ops, s,
+                          "dp8393x-regs", 0x40 << s->it_shift);
+
+    s->nic = qemu_new_nic(&net_dp83932_info, &s->conf,
+                          object_get_typename(OBJECT(dev)), dev->id, s);
+    qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
 
-    s->address_space = address_space;
-    address_space_init(&s->as, dma_mr, "dp8393x-dma");
-    s->it_shift = it_shift;
-    s->irq = irq;
     s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s);
     s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux */
+}
 
-    s->conf.macaddr = nd->macaddr;
-    s->conf.peers.ncs[0] = nd->netdev;
+static Property dp8393x_properties[] = {
+    DEFINE_NIC_PROPERTIES(dp8393xState, conf),
+    DEFINE_PROP_PTR("dma_mr", dp8393xState, dma_mr),
+    DEFINE_PROP_UINT8("it_shift", dp8393xState, it_shift, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
 
-    s->nic = qemu_new_nic(&net_dp83932_info, &s->conf, nd->model, nd->name, s);
+static void dp8393x_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
-    qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
-    qemu_register_reset(dp8393x_reset, s);
-    dp8393x_reset(s);
+    set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
+    dc->realize = dp8393x_realize;
+    dc->reset = dp8393x_reset;
+    dc->props = dp8393x_properties;
+}
 
-    memory_region_init_io(&s->mmio, NULL, &dp8393x_ops, s,
-                          "dp8393x", 0x40 << it_shift);
-    memory_region_add_subregion(address_space, base, &s->mmio);
+static const TypeInfo dp8393x_info = {
+    .name          = TYPE_DP8393X,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(dp8393xState),
+    .instance_init = dp8393x_instance_init,
+    .class_init    = dp8393x_class_init,
+};
+
+static void dp8393x_register_types(void)
+{
+    type_register_static(&dp8393x_info);
 }
+
+type_init(dp8393x_register_types)
diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
index 31b4729..e0065ce 100644
--- a/include/hw/mips/mips.h
+++ b/include/hw/mips/mips.h
@@ -20,9 +20,4 @@ void rc4030_dma_write(void *dma, uint8_t *buf, int len);
 
 DeviceState *rc4030_init(rc4030_dma **dmas, MemoryRegion **dma_mr);
 
-/* dp8393x.c */
-void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
-                  MemoryRegion *address_space,
-                  qemu_irq irq, MemoryRegion *dma_mr);
-
 #endif
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 12/17] net/dp8393x: add PROM to store MAC address
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (10 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 11/17] net/dp8393x: QOM'ify Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:04   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 13/17] net/dp8393x: add load/save support Hervé Poussineau
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel
  Cc: Hervé Poussineau, Leon Alrae, Laurent Vivier, Aurelien Jarno

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/mips/mips_jazz.c |  1 +
 hw/net/dp8393x.c    | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 648654e..9d60633 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -282,6 +282,7 @@ static void mips_jazz_init(MachineState *machine,
             qdev_init_nofail(dev);
             sysbus = SYS_BUS_DEVICE(dev);
             sysbus_mmio_map(sysbus, 0, 0x80001000);
+            sysbus_mmio_map(sysbus, 1, 0x8000b000);
             sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(rc4030, 4));
             break;
         } else if (is_help_option(nd->model)) {
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 51e728b..ef1fb0e 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -25,6 +25,7 @@
 
 //#define DEBUG_SONIC
 
+#define SONIC_PROM_SIZE 0x1000
 
 #ifdef DEBUG_SONIC
 #define DPRINTF(fmt, ...) \
@@ -156,6 +157,7 @@ typedef struct dp8393xState {
     NICConf conf;
     NICState *nic;
     MemoryRegion mmio;
+    MemoryRegion prom;
 
     /* Registers */
     uint8_t cam[16][6];
@@ -816,12 +818,15 @@ static void dp8393x_instance_init(Object *obj)
     dp8393xState *s = DP8393X(obj);
 
     sysbus_init_mmio(sbd, &s->mmio);
+    sysbus_init_mmio(sbd, &s->prom);
     sysbus_init_irq(sbd, &s->irq);
 }
 
 static void dp8393x_realize(DeviceState *dev, Error **errp)
 {
     dp8393xState *s = DP8393X(dev);
+    int i, checksum;
+    uint8_t *prom;
 
     address_space_init(&s->as, s->dma_mr, "dp8393x");
     memory_region_init_io(&s->mmio, OBJECT(dev), &dp8393x_ops, s,
@@ -833,6 +838,19 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
 
     s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s);
     s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux */
+
+    memory_region_init_rom_device(&s->prom, OBJECT(dev), NULL, NULL,
+                                  "dp8393x-prom", SONIC_PROM_SIZE, NULL);
+    prom = memory_region_get_ram_ptr(&s->prom);
+    checksum = 0;
+    for (i = 0; i < 6; i++) {
+        prom[i] = s->conf.macaddr.a[i];
+        checksum += prom[i];
+        if (checksum > 0xff) {
+            checksum = (checksum + 1) & 0xff;
+        }
+    }
+    prom[7] = 0xff - checksum;
 }
 
 static Property dp8393x_properties[] = {
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 13/17] net/dp8393x: add load/save support
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (11 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 12/17] net/dp8393x: add PROM to store MAC address Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:04   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 14/17] net/dp8393x: correctly reset in_use field Hervé Poussineau
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/net/dp8393x.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index ef1fb0e..4184045 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -853,6 +853,17 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
     prom[7] = 0xff - checksum;
 }
 
+static const VMStateDescription vmstate_dp8393x = {
+    .name = "dp8393x",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField []) {
+        VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 6),
+        VMSTATE_UINT16_ARRAY(regs, dp8393xState, 0x40),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static Property dp8393x_properties[] = {
     DEFINE_NIC_PROPERTIES(dp8393xState, conf),
     DEFINE_PROP_PTR("dma_mr", dp8393xState, dma_mr),
@@ -867,6 +878,7 @@ static void dp8393x_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
     dc->realize = dp8393x_realize;
     dc->reset = dp8393x_reset;
+    dc->vmsd = &vmstate_dp8393x;
     dc->props = dp8393x_properties;
 }
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 14/17] net/dp8393x: correctly reset in_use field
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (12 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 13/17] net/dp8393x: add load/save support Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:04   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 15/17] net/dp8393x: fix hardware reset Hervé Poussineau
                   ` (2 subsequent siblings)
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Don't write more than the field width, which is always 16 bit.
Fixes network in NetBSD 5.1/arc

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/net/dp8393x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 4184045..b72b0b1 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -761,10 +761,10 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
         /* EOL detected */
         s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
     } else {
-        data[0 * width] = 0; /* in_use */
+        uint16_t in_use = 0;
         address_space_rw(&s->as,
             ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 6 * width,
-            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
+            MEMTXATTRS_UNSPECIFIED, (uint8_t *)&in_use, sizeof(uint16_t), 1);
         s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
         s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX;
         s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 15/17] net/dp8393x: fix hardware reset
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (13 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 14/17] net/dp8393x: correctly reset in_use field Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:05   ` Aurelien Jarno
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 16/17] net/dp8393x: repair can_receive() method Hervé Poussineau
  2015-05-27 12:20 ` [Qemu-devel] [PATCH v2 17/17] [RFC] dma/rc4030: do multiple calls to address_space_rw when doing DMA transfers Hervé Poussineau
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Documentation is not clear of what happens when doing a hardware reset,
but firmware expect all registers to be zero unless specified otherwise.

This fixes reboot on MIPS Magnum.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/net/dp8393x.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index b72b0b1..95a4d3d 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -786,6 +786,7 @@ static void dp8393x_reset(DeviceState *dev)
     dp8393xState *s = DP8393X(dev);
     timer_del(s->watchdog);
 
+    memset(s->regs, 0, sizeof(s->regs));
     s->regs[SONIC_CR] = SONIC_CR_RST | SONIC_CR_STP | SONIC_CR_RXDIS;
     s->regs[SONIC_DCR] &= ~(SONIC_DCR_EXBUS | SONIC_DCR_LBR);
     s->regs[SONIC_RCR] &= ~(SONIC_RCR_LB0 | SONIC_RCR_LB1 | SONIC_RCR_BRD | SONIC_RCR_RNT);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 16/17] net/dp8393x: repair can_receive() method
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (14 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 15/17] net/dp8393x: fix hardware reset Hervé Poussineau
@ 2015-05-27 12:19 ` Hervé Poussineau
  2015-06-02 11:05   ` Aurelien Jarno
  2015-05-27 12:20 ` [Qemu-devel] [PATCH v2 17/17] [RFC] dma/rc4030: do multiple calls to address_space_rw when doing DMA transfers Hervé Poussineau
  16 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno

Datasheet clearly says that RXDIS flag prevents reception, but says
nothing about a required presence of RXEN flag.

While at it, fix the style of the following if statement.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/net/dp8393x.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 95a4d3d..b81036a 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -622,10 +622,12 @@ static int dp8393x_can_receive(NetClientState *nc)
 {
     dp8393xState *s = qemu_get_nic_opaque(nc);
 
-    if (!(s->regs[SONIC_CR] & SONIC_CR_RXEN))
+    if (s->regs[SONIC_CR] & SONIC_CR_RXDIS) {
         return 0;
-    if (s->regs[SONIC_ISR] & SONIC_ISR_RBE)
+    }
+    if (s->regs[SONIC_ISR] & SONIC_ISR_RBE) {
         return 0;
+    }
     return 1;
 }
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [Qemu-devel] [PATCH v2 17/17] [RFC] dma/rc4030: do multiple calls to address_space_rw when doing DMA transfers
  2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
                   ` (15 preceding siblings ...)
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 16/17] net/dp8393x: repair can_receive() method Hervé Poussineau
@ 2015-05-27 12:20 ` Hervé Poussineau
  16 siblings, 0 replies; 38+ messages in thread
From: Hervé Poussineau @ 2015-05-27 12:20 UTC (permalink / raw)
  To: qemu-devel
  Cc: Hervé Poussineau, Leon Alrae, Aurelien Jarno, Paolo Bonzini

This fixes Windows NT 4.0/MIPS, which was always bugchecking with
IRQL_NOT_LESS_OR_EQUAL.

Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/dma/rc4030.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index 3efa6de..deac0a8 100644
--- a/hw/dma/rc4030.c
+++ b/hw/dma/rc4030.c
@@ -681,6 +681,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);
 
@@ -699,8 +700,17 @@ 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 */
-    address_space_rw(&s->dma_as, dma_addr, MEMTXATTRS_UNSPECIFIED,
-                     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, MEMTXATTRS_UNSPECIFIED,
+                         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;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/17] mips jazz: compile only in 64 bit little endian
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 01/17] mips jazz: compile only in 64 bit little endian Hervé Poussineau
@ 2015-06-02 11:02   ` Aurelien Jarno
  2015-06-02 18:04     ` Hervé Poussineau
  0 siblings, 1 reply; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:02 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Remove now useless device models from other MIPS configurations
> 
> We're now compiling 18 files less than before.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  default-configs/mips-softmmu.mak     | 5 -----
>  default-configs/mips64-softmmu.mak   | 5 -----
>  default-configs/mips64el-softmmu.mak | 1 +
>  default-configs/mipsel-softmmu.mak   | 5 -----
>  hw/mips/Makefile.objs                | 3 ++-
>  hw/mips/mips_jazz.c                  | 5 -----
>  tests/endianness-test.c              | 4 ----
>  7 files changed, 3 insertions(+), 25 deletions(-)
> 
> diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak
> index cce2c81..f62a21a 100644
> --- a/default-configs/mips-softmmu.mak
> +++ b/default-configs/mips-softmmu.mak
> @@ -21,14 +21,9 @@ CONFIG_PIIX4=y
>  CONFIG_IDE_ISA=y
>  CONFIG_IDE_PIIX=y
>  CONFIG_NE2000_ISA=y
> -CONFIG_RC4030=y
> -CONFIG_DP8393X=y
> -CONFIG_DS1225Y=y
>  CONFIG_MIPSNET=y
>  CONFIG_PFLASH_CFI01=y
> -CONFIG_G364FB=y
>  CONFIG_I8259=y
> -CONFIG_JAZZ_LED=y
>  CONFIG_MC146818RTC=y
>  CONFIG_ISA_TESTDEV=y
>  CONFIG_EMPTY_SLOT=y
> diff --git a/default-configs/mips64-softmmu.mak b/default-configs/mips64-softmmu.mak
> index 7a88a08..accedca 100644
> --- a/default-configs/mips64-softmmu.mak
> +++ b/default-configs/mips64-softmmu.mak
> @@ -21,14 +21,9 @@ CONFIG_PIIX4=y
>  CONFIG_IDE_ISA=y
>  CONFIG_IDE_PIIX=y
>  CONFIG_NE2000_ISA=y
> -CONFIG_RC4030=y
> -CONFIG_DP8393X=y
> -CONFIG_DS1225Y=y
>  CONFIG_MIPSNET=y
>  CONFIG_PFLASH_CFI01=y
> -CONFIG_G364FB=y
>  CONFIG_I8259=y
> -CONFIG_JAZZ_LED=y
>  CONFIG_MC146818RTC=y
>  CONFIG_ISA_TESTDEV=y
>  CONFIG_EMPTY_SLOT=y

TTBOMK, MIPS Magnum machines are dual endian, so why remove the 64-bit
big endian version?

On the other hand, I am all for removing the 32-bit versions.

> diff --git a/default-configs/mips64el-softmmu.mak b/default-configs/mips64el-softmmu.mak
> index 095de43..24ff6b9 100644
> --- a/default-configs/mips64el-softmmu.mak
> +++ b/default-configs/mips64el-softmmu.mak
> @@ -28,6 +28,7 @@ CONFIG_DS1225Y=y
>  CONFIG_MIPSNET=y
>  CONFIG_PFLASH_CFI01=y
>  CONFIG_FULONG=y
> +CONFIG_JAZZ=y
>  CONFIG_G364FB=y
>  CONFIG_I8259=y
>  CONFIG_JAZZ_LED=y
> diff --git a/default-configs/mipsel-softmmu.mak b/default-configs/mipsel-softmmu.mak
> index 0e25108..9fbee82 100644
> --- a/default-configs/mipsel-softmmu.mak
> +++ b/default-configs/mipsel-softmmu.mak
> @@ -21,14 +21,9 @@ CONFIG_PIIX4=y
>  CONFIG_IDE_ISA=y
>  CONFIG_IDE_PIIX=y
>  CONFIG_NE2000_ISA=y
> -CONFIG_RC4030=y
> -CONFIG_DP8393X=y
> -CONFIG_DS1225Y=y
>  CONFIG_MIPSNET=y
>  CONFIG_PFLASH_CFI01=y
> -CONFIG_G364FB=y
>  CONFIG_I8259=y
> -CONFIG_JAZZ_LED=y
>  CONFIG_MC146818RTC=y
>  CONFIG_ISA_TESTDEV=y
>  CONFIG_EMPTY_SLOT=y
> diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
> index 0a652f8..9633f3a 100644
> --- a/hw/mips/Makefile.objs
> +++ b/hw/mips/Makefile.objs
> @@ -1,4 +1,5 @@
> -obj-y += mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
> +obj-y += mips_r4k.o mips_malta.o mips_mipssim.o
>  obj-y += addr.o cputimer.o mips_int.o
> +obj-$(CONFIG_JAZZ) += mips_jazz.o
>  obj-$(CONFIG_FULONG) += mips_fulong2e.o
>  obj-y += gt64xxx_pci.o
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 2c153e0..f16070e 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -157,12 +157,7 @@ static void mips_jazz_init(MachineState *machine,
>  
>      /* init CPUs */
>      if (cpu_model == NULL) {
> -#ifdef TARGET_MIPS64
>          cpu_model = "R4000";
> -#else
> -        /* FIXME: All wrong, this maybe should be R3000 for the older JAZZs. */
> -        cpu_model = "24Kf";
> -#endif
>      }
>      cpu = cpu_mips_init(cpu_model);
>      if (cpu == NULL) {
> diff --git a/tests/endianness-test.c b/tests/endianness-test.c
> index 92e17d2..9506da0 100644
> --- a/tests/endianness-test.c
> +++ b/tests/endianness-test.c
> @@ -31,12 +31,8 @@ struct TestCase {
>  
>  static const TestCase test_cases[] = {
>      { "i386", "pc", -1 },
> -    { "mips", "magnum", 0x90000000, .bswap = true },
> -    { "mips", "pica61", 0x90000000, .bswap = true },
>      { "mips", "mips", 0x14000000, .bswap = true },
>      { "mips", "malta", 0x10000000, .bswap = true },
> -    { "mips64", "magnum", 0x90000000, .bswap = true },
> -    { "mips64", "pica61", 0x90000000, .bswap = true },
>      { "mips64", "mips", 0x14000000, .bswap = true },
>      { "mips64", "malta", 0x10000000, .bswap = true },
>      { "mips64el", "fulong2e", 0x1fd00000 },
> -- 
> 2.1.4
> 
> 

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 02/17] dma/rc4030: create custom DMA address space
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 02/17] dma/rc4030: create custom DMA address space Hervé Poussineau
@ 2015-06-02 11:02   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:02 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Paolo Bonzini, Leon Alrae, qemu-devel

On 2015-05-27 14:19, 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>
> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
>  hw/dma/rc4030.c | 163 +++++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 126 insertions(+), 37 deletions(-)
> 
> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
> index af26632..84039dc 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,16 @@ 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;
>      MemoryRegion iomem_jazzio;
>  } rc4030State;
> @@ -265,6 +278,97 @@ 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_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, NULL,
> +                                 "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, NULL,
> +                           "dma-table-alias", 0);
> +    }
> +}
> +
> +
>  static void rc4030_writel(void *opaque, hwaddr addr, uint32_t val)
>  {
>      rc4030State *s = opaque;
> @@ -279,11 +383,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 +694,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 +779,8 @@ 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, MEMTXATTRS_UNSPECIFIED, buf, len,
> +                     is_write);
>  }
>  
>  static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_write)
> @@ -733,7 +806,8 @@ 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);
> +    address_space_rw(&s->dma_as, dma_addr, MEMTXATTRS_UNSPECIFIED,
> +                     buf, len, is_write);
>  
>      s->dma_regs[n][DMA_REG_ENABLE] |= DMA_FLAG_TC_INTR;
>      s->dma_regs[n][DMA_REG_COUNT] -= len;
> @@ -800,6 +874,7 @@ void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
>                    MemoryRegion *sysmem)
>  {
>      rc4030State *s;
> +    int i;
>  
>      s = g_malloc0(sizeof(rc4030State));
>  
> @@ -821,5 +896,19 @@ 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_rom_device(&s->dma_tt, NULL,
> +                                  &rc4030_dma_tt_ops, s, "dma-table",
> +                                  MAX_TL_ENTRIES * sizeof(dma_pagetable_entry),
> +                                  NULL);
> +    memory_region_init(&s->dma_tt_alias, NULL, "dma-table-alias", 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;
>  }

I don't have a big knowledge of the memory API, but it clearly looks
cleaner than the current code.

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 03/17] dma/rc4030: use AddressSpace and address_space_rw in users
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 03/17] dma/rc4030: use AddressSpace and address_space_rw in users Hervé Poussineau
@ 2015-06-02 11:02   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:02 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Now that rc4030 internally uses an AddressSpace for DMA handling, make its root
> memory region public. This is especially usefull for dp8393x netcard, which now
> uses well known QEMU types and methods.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/dma/rc4030.c        | 15 ++++---------
>  hw/mips/mips_jazz.c    |  6 ++---
>  hw/net/dp8393x.c       | 61 +++++++++++++++++++++++++-------------------------
>  include/hw/mips/mips.h | 10 ++++-----
>  4 files changed, 42 insertions(+), 50 deletions(-)
> 
> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
> index 84039dc..a0b617f 100644
> --- a/hw/dma/rc4030.c
> +++ b/hw/dma/rc4030.c
> @@ -776,13 +776,6 @@ static void rc4030_save(QEMUFile *f, void *opaque)
>      qemu_put_be32(f, s->itr);
>  }
>  
> -void rc4030_dma_memory_rw(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write)
> -{
> -    rc4030State *s = opaque;
> -    address_space_rw(&s->dma_as, addr, MEMTXATTRS_UNSPECIFIED, buf, len,
> -                     is_write);
> -}
> -
>  static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_write)
>  {
>      rc4030State *s = opaque;
> @@ -869,9 +862,9 @@ static rc4030_dma *rc4030_allocate_dmas(void *opaque, int n)
>      return s;
>  }
>  
> -void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
> -                  qemu_irq **irqs, rc4030_dma **dmas,
> -                  MemoryRegion *sysmem)
> +MemoryRegion *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
> +                          qemu_irq **irqs, rc4030_dma **dmas,
> +                          MemoryRegion *sysmem)
>  {
>      rc4030State *s;
>      int i;
> @@ -910,5 +903,5 @@ void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
>                                      &s->dma_mrs[i]);
>      }
>      address_space_init(&s->dma_as, &s->dma_mr, "rc4030-dma");
> -    return s;
> +    return &s->dma_mr;
>  }
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index f16070e..05cad6b 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -137,7 +137,7 @@ static void mips_jazz_init(MachineState *machine,
>      CPUMIPSState *env;
>      qemu_irq *rc4030, *i8259;
>      rc4030_dma *dmas;
> -    void* rc4030_opaque;
> +    MemoryRegion *rc4030_dma_mr;
>      MemoryRegion *isa_mem = g_new(MemoryRegion, 1);
>      MemoryRegion *isa_io = g_new(MemoryRegion, 1);
>      MemoryRegion *rtc = g_new(MemoryRegion, 1);
> @@ -213,7 +213,7 @@ static void mips_jazz_init(MachineState *machine,
>      cpu_mips_clock_init(env);
>  
>      /* Chipset */
> -    rc4030_opaque = rc4030_init(env->irq[6], env->irq[3], &rc4030, &dmas,
> +    rc4030_dma_mr = rc4030_init(env->irq[6], env->irq[3], &rc4030, &dmas,
>                                  address_space);
>      memory_region_init_io(dma_dummy, NULL, &dma_dummy_ops, NULL, "dummy_dma", 0x1000);
>      memory_region_add_subregion(address_space, 0x8000d000, dma_dummy);
> @@ -268,7 +268,7 @@ static void mips_jazz_init(MachineState *machine,
>              nd->model = g_strdup("dp83932");
>          if (strcmp(nd->model, "dp83932") == 0) {
>              dp83932_init(nd, 0x80001000, 2, get_system_memory(), rc4030[4],
> -                         rc4030_opaque, rc4030_dma_memory_rw);
> +                         rc4030_dma_mr);
>              break;
>          } else if (is_help_option(nd->model)) {
>              fprintf(stderr, "qemu: Supported NICs: dp83932\n");
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 7ce13d2..2297231 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -168,8 +168,7 @@ typedef struct dp8393xState {
>      int loopback_packet;
>  
>      /* Memory access */
> -    void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write);
> -    void* mem_opaque;
> +    AddressSpace as;
>  } dp8393xState;
>  
>  static void dp8393x_update_irq(dp8393xState *s)
> @@ -201,9 +200,9 @@ static void do_load_cam(dp8393xState *s)
>  
>      while (s->regs[SONIC_CDC] & 0x1f) {
>          /* Fill current entry */
> -        s->memory_rw(s->mem_opaque,
> +        address_space_rw(&s->as,
>              (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
> -            (uint8_t *)data, size, 0);
> +            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
>          s->cam[index][0] = data[1 * width] & 0xff;
>          s->cam[index][1] = data[1 * width] >> 8;
>          s->cam[index][2] = data[2 * width] & 0xff;
> @@ -220,9 +219,9 @@ static void do_load_cam(dp8393xState *s)
>      }
>  
>      /* Read CAM enable */
> -    s->memory_rw(s->mem_opaque,
> +    address_space_rw(&s->as,
>          (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
> -        (uint8_t *)data, size, 0);
> +        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
>      s->regs[SONIC_CE] = data[0 * width];
>      DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]);
>  
> @@ -240,9 +239,9 @@ static void do_read_rra(dp8393xState *s)
>      /* Read memory */
>      width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
>      size = sizeof(uint16_t) * 4 * width;
> -    s->memory_rw(s->mem_opaque,
> +    address_space_rw(&s->as,
>          (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_RRP],
> -        (uint8_t *)data, size, 0);
> +        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
>  
>      /* Update SONIC registers */
>      s->regs[SONIC_CRBA0] = data[0 * width];
> @@ -353,9 +352,9 @@ static void do_transmit_packets(dp8393xState *s)
>                  (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_CTDA]);
>          size = sizeof(uint16_t) * 6 * width;
>          s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA];
> -        s->memory_rw(s->mem_opaque,
> +        address_space_rw(&s->as,
>              ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * width,
> -            (uint8_t *)data, size, 0);
> +            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
>          tx_len = 0;
>  
>          /* Update registers */
> @@ -379,18 +378,18 @@ static void do_transmit_packets(dp8393xState *s)
>              if (tx_len + len > sizeof(s->tx_buffer)) {
>                  len = sizeof(s->tx_buffer) - tx_len;
>              }
> -            s->memory_rw(s->mem_opaque,
> +            address_space_rw(&s->as,
>                  (s->regs[SONIC_TSA1] << 16) | s->regs[SONIC_TSA0],
> -                &s->tx_buffer[tx_len], len, 0);
> +                MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0);
>              tx_len += len;
>  
>              i++;
>              if (i != s->regs[SONIC_TFC]) {
>                  /* Read next fragment details */
>                  size = sizeof(uint16_t) * 3 * width;
> -                s->memory_rw(s->mem_opaque,
> +                address_space_rw(&s->as,
>                      ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * i) * width,
> -                    (uint8_t *)data, size, 0);
> +                    MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
>                  s->regs[SONIC_TSA0] = data[0 * width];
>                  s->regs[SONIC_TSA1] = data[1 * width];
>                  s->regs[SONIC_TFS] = data[2 * width];
> @@ -422,16 +421,16 @@ static void do_transmit_packets(dp8393xState *s)
>          /* Write status */
>          data[0 * width] = s->regs[SONIC_TCR] & 0x0fff; /* status */
>          size = sizeof(uint16_t) * width;
> -        s->memory_rw(s->mem_opaque,
> +        address_space_rw(&s->as,
>              (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA],
> -            (uint8_t *)data, size, 1);
> +            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
>  
>          if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) {
>              /* Read footer of packet */
>              size = sizeof(uint16_t) * width;
> -            s->memory_rw(s->mem_opaque,
> +            address_space_rw(&s->as,
>                  ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) * width,
> -                (uint8_t *)data, size, 0);
> +                MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
>              s->regs[SONIC_CTDA] = data[0 * width] & ~0x1;
>              if (data[0 * width] & 0x1) {
>                  /* EOL detected */
> @@ -750,7 +749,8 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
>          /* Are we still in resource exhaustion? */
>          size = sizeof(uint16_t) * 1 * width;
>          address = ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width;
> -        s->memory_rw(s->mem_opaque, address, (uint8_t*)data, size, 0);
> +        address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED,
> +                         (uint8_t *)data, size, 0);
>          if (data[0 * width] & 0x1) {
>              /* Still EOL ; stop reception */
>              return -1;
> @@ -773,9 +773,11 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
>      /* Put packet into RBA */
>      DPRINTF("Receive packet at %08x\n", (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0]);
>      address = (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0];
> -    s->memory_rw(s->mem_opaque, address, (uint8_t*)buf, rx_len, 1);
> +    address_space_rw(&s->as, address,
> +        MEMTXATTRS_UNSPECIFIED, (uint8_t *)buf, rx_len, 1);
>      address += rx_len;
> -    s->memory_rw(s->mem_opaque, address, (uint8_t*)&checksum, 4, 1);
> +    address_space_rw(&s->as, address,
> +        MEMTXATTRS_UNSPECIFIED, (uint8_t *)&checksum, 4, 1);
>      rx_len += 4;
>      s->regs[SONIC_CRBA1] = address >> 16;
>      s->regs[SONIC_CRBA0] = address & 0xffff;
> @@ -803,22 +805,23 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
>      data[3 * width] = s->regs[SONIC_TRBA1]; /* pkt_ptr1 */
>      data[4 * width] = s->regs[SONIC_RSC]; /* seq_no */
>      size = sizeof(uint16_t) * 5 * width;
> -    s->memory_rw(s->mem_opaque, (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA], (uint8_t *)data, size, 1);
> +    address_space_rw(&s->as, (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA],
> +        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
>  
>      /* Move to next descriptor */
>      size = sizeof(uint16_t) * width;
> -    s->memory_rw(s->mem_opaque,
> +    address_space_rw(&s->as,
>          ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width,
> -        (uint8_t *)data, size, 0);
> +        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0);
>      s->regs[SONIC_LLFA] = data[0 * width];
>      if (s->regs[SONIC_LLFA] & 0x1) {
>          /* EOL detected */
>          s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
>      } else {
>          data[0 * width] = 0; /* in_use */
> -        s->memory_rw(s->mem_opaque,
> +        address_space_rw(&s->as,
>              ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 6 * width,
> -            (uint8_t *)data, size, 1);
> +            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
>          s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
>          s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX;
>          s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff);
> @@ -868,8 +871,7 @@ static NetClientInfo net_dp83932_info = {
>  
>  void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
>                    MemoryRegion *address_space,
> -                  qemu_irq irq, void* mem_opaque,
> -                  void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write))
> +                  qemu_irq irq, MemoryRegion *dma_mr)
>  {
>      dp8393xState *s;
>  
> @@ -878,8 +880,7 @@ void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
>      s = g_malloc0(sizeof(dp8393xState));
>  
>      s->address_space = address_space;
> -    s->mem_opaque = mem_opaque;
> -    s->memory_rw = memory_rw;
> +    address_space_init(&s->as, dma_mr, "dp8393x-dma");
>      s->it_shift = it_shift;
>      s->irq = irq;
>      s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s);
> diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
> index 2a7a9c9..47eb31f 100644
> --- a/include/hw/mips/mips.h
> +++ b/include/hw/mips/mips.h
> @@ -15,18 +15,16 @@ PCIBus *bonito_init(qemu_irq *pic);
>  
>  /* rc4030.c */
>  typedef struct rc4030DMAState *rc4030_dma;
> -void rc4030_dma_memory_rw(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write);
>  void rc4030_dma_read(void *dma, uint8_t *buf, int len);
>  void rc4030_dma_write(void *dma, uint8_t *buf, int len);
>  
> -void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
> -                  qemu_irq **irqs, rc4030_dma **dmas,
> -                  MemoryRegion *sysmem);
> +MemoryRegion *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
> +                          qemu_irq **irqs, rc4030_dma **dmas,
> +                          MemoryRegion *sysmem);
>  
>  /* dp8393x.c */
>  void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
>                    MemoryRegion *address_space,
> -                  qemu_irq irq, void* mem_opaque,
> -                  void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write));
> +                  qemu_irq irq, MemoryRegion *dma_mr);
>  
>  #endif

As for the previous patch, I am not very comfortable with the memory
API, but it looks fine to me.

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 04/17] dma/rc4030: do not use old_mmio accesses
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 04/17] dma/rc4030: do not use old_mmio accesses Hervé Poussineau
@ 2015-06-02 11:03   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:03 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/dma/rc4030.c | 112 ++++++++------------------------------------------------
>  1 file changed, 16 insertions(+), 96 deletions(-)
> 
> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
> index a0b617f..96f796b 100644
> --- a/hw/dma/rc4030.c
> +++ b/hw/dma/rc4030.c
> @@ -125,7 +125,7 @@ static void set_next_tick(rc4030State *s)
>  }
>  
>  /* called for accesses to rc4030 */
> -static uint32_t rc4030_readl(void *opaque, hwaddr addr)
> +static uint64_t rc4030_read(void *opaque, hwaddr addr, unsigned int size)
>  {
>      rc4030State *s = opaque;
>      uint32_t val;
> @@ -263,21 +263,6 @@ static uint32_t rc4030_readl(void *opaque, hwaddr addr)
>      return val;
>  }
>  
> -static uint32_t rc4030_readw(void *opaque, hwaddr addr)
> -{
> -    uint32_t v = rc4030_readl(opaque, addr & ~0x3);
> -    if (addr & 0x2)
> -        return v >> 16;
> -    else
> -        return v & 0xffff;
> -}
> -
> -static uint32_t rc4030_readb(void *opaque, hwaddr addr)
> -{
> -    uint32_t v = rc4030_readl(opaque, addr & ~0x3);
> -    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) {
> @@ -368,10 +353,11 @@ static void rc4030_dma_tt_update(rc4030State *s, uint32_t new_tl_base,
>      }
>  }
>  
> -
> -static void rc4030_writel(void *opaque, hwaddr addr, uint32_t val)
> +static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
> +                         unsigned int size)
>  {
>      rc4030State *s = opaque;
> +    uint32_t val = data;
>      addr &= 0x3fff;
>  
>      DPRINTF("write 0x%02x at " TARGET_FMT_plx "\n", val, addr);
> @@ -494,43 +480,11 @@ static void rc4030_writel(void *opaque, hwaddr addr, uint32_t val)
>      }
>  }
>  
> -static void rc4030_writew(void *opaque, hwaddr addr, uint32_t val)
> -{
> -    uint32_t old_val = rc4030_readl(opaque, addr & ~0x3);
> -
> -    if (addr & 0x2)
> -        val = (val << 16) | (old_val & 0x0000ffff);
> -    else
> -        val = val | (old_val & 0xffff0000);
> -    rc4030_writel(opaque, addr & ~0x3, val);
> -}
> -
> -static void rc4030_writeb(void *opaque, hwaddr addr, uint32_t val)
> -{
> -    uint32_t old_val = rc4030_readl(opaque, addr & ~0x3);
> -
> -    switch (addr & 3) {
> -    case 0:
> -        val = val | (old_val & 0xffffff00);
> -        break;
> -    case 1:
> -        val = (val << 8) | (old_val & 0xffff00ff);
> -        break;
> -    case 2:
> -        val = (val << 16) | (old_val & 0xff00ffff);
> -        break;
> -    case 3:
> -        val = (val << 24) | (old_val & 0x00ffffff);
> -        break;
> -    }
> -    rc4030_writel(opaque, addr & ~0x3, val);
> -}
> -
>  static const MemoryRegionOps rc4030_ops = {
> -    .old_mmio = {
> -        .read = { rc4030_readb, rc4030_readw, rc4030_readl, },
> -        .write = { rc4030_writeb, rc4030_writew, rc4030_writel, },
> -    },
> +    .read = rc4030_read,
> +    .write = rc4030_write,
> +    .impl.min_access_size = 4,
> +    .impl.max_access_size = 4,
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> @@ -583,7 +537,7 @@ static void rc4030_periodic_timer(void *opaque)
>      qemu_irq_raise(s->timer_irq);
>  }
>  
> -static uint32_t jazzio_readw(void *opaque, hwaddr addr)
> +static uint64_t jazzio_read(void *opaque, hwaddr addr, unsigned int size)
>  {
>      rc4030State *s = opaque;
>      uint32_t val;
> @@ -621,24 +575,11 @@ static uint32_t jazzio_readw(void *opaque, hwaddr addr)
>      return val;
>  }
>  
> -static uint32_t jazzio_readb(void *opaque, hwaddr addr)
> -{
> -    uint32_t v;
> -    v = jazzio_readw(opaque, addr & ~0x1);
> -    return (v >> (8 * (addr & 0x1))) & 0xff;
> -}
> -
> -static uint32_t jazzio_readl(void *opaque, hwaddr addr)
> -{
> -    uint32_t v;
> -    v = jazzio_readw(opaque, addr);
> -    v |= jazzio_readw(opaque, addr + 2) << 16;
> -    return v;
> -}
> -
> -static void jazzio_writew(void *opaque, hwaddr addr, uint32_t val)
> +static void jazzio_write(void *opaque, hwaddr addr, uint64_t data,
> +                         unsigned int size)
>  {
>      rc4030State *s = opaque;
> +    uint32_t val = data;
>      addr &= 0xfff;
>  
>      DPRINTF("(jazz io controller) write 0x%04x at " TARGET_FMT_plx "\n", val, addr);
> @@ -655,32 +596,11 @@ static void jazzio_writew(void *opaque, hwaddr addr, uint32_t val)
>      }
>  }
>  
> -static void jazzio_writeb(void *opaque, hwaddr addr, uint32_t val)
> -{
> -    uint32_t old_val = jazzio_readw(opaque, addr & ~0x1);
> -
> -    switch (addr & 1) {
> -    case 0:
> -        val = val | (old_val & 0xff00);
> -        break;
> -    case 1:
> -        val = (val << 8) | (old_val & 0x00ff);
> -        break;
> -    }
> -    jazzio_writew(opaque, addr & ~0x1, val);
> -}
> -
> -static void jazzio_writel(void *opaque, hwaddr addr, uint32_t val)
> -{
> -    jazzio_writew(opaque, addr, val & 0xffff);
> -    jazzio_writew(opaque, addr + 2, (val >> 16) & 0xffff);
> -}
> -
>  static const MemoryRegionOps jazzio_ops = {
> -    .old_mmio = {
> -        .read = { jazzio_readb, jazzio_readw, jazzio_readl, },
> -        .write = { jazzio_writeb, jazzio_writew, jazzio_writel, },
> -    },
> +    .read = jazzio_read,
> +    .write = jazzio_write,
> +    .impl.min_access_size = 2,
> +    .impl.max_access_size = 2,
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };

Nice cleanup :)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 05/17] dma/rc4030: document register at offset 0x210
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 05/17] dma/rc4030: document register at offset 0x210 Hervé Poussineau
@ 2015-06-02 11:03   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:03 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/dma/rc4030.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
> index 96f796b..bf82eed 100644
> --- a/hw/dma/rc4030.c
> +++ b/hw/dma/rc4030.c
> @@ -86,7 +86,7 @@ typedef struct rc4030State
>      uint32_t cache_bmask; /* 0x0058: I/O Cache Byte Mask */
>  
>      uint32_t nmi_interrupt; /* 0x0200: interrupt source */
> -    uint32_t offset210;
> +    uint32_t memory_refresh_rate; /* 0x0210: memory refresh rate */
>      uint32_t nvram_protect; /* 0x0220: NV ram protect register */
>      uint32_t rem_speed[16];
>      uint32_t imr_jazz; /* Local bus int enable mask */
> @@ -233,9 +233,9 @@ static uint64_t rc4030_read(void *opaque, hwaddr addr, unsigned int size)
>      case 0x0208:
>          val = 0;
>          break;
> -    /* Offset 0x0210 */
> +    /* Memory refresh rate */
>      case 0x0210:
> -        val = s->offset210;
> +        val = s->memory_refresh_rate;
>          break;
>      /* NV ram protect register */
>      case 0x0220:
> @@ -461,9 +461,9 @@ static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
>              s->dma_regs[entry][idx] = val;
>          }
>          break;
> -    /* Offset 0x0210 */
> +    /* Memory refresh rate */
>      case 0x0210:
> -        s->offset210 = val;
> +        s->memory_refresh_rate = val;
>          break;
>      /* Interval timer reload */
>      case 0x0228:
> @@ -621,7 +621,7 @@ static void rc4030_reset(void *opaque)
>      s->cache_ptag = s->cache_ltag = 0;
>      s->cache_bmask = 0;
>  
> -    s->offset210 = 0x18186;
> +    s->memory_refresh_rate = 0x18186;
>      s->nvram_protect = 7;
>      for (i = 0; i < 15; i++)
>          s->rem_speed[i] = 7;
> @@ -655,7 +655,7 @@ static int rc4030_load(QEMUFile *f, void *opaque, int version_id)
>      s->cache_ptag = qemu_get_be32(f);
>      s->cache_ltag = qemu_get_be32(f);
>      s->cache_bmask = qemu_get_be32(f);
> -    s->offset210 = qemu_get_be32(f);
> +    s->memory_refresh_rate = qemu_get_be32(f);
>      s->nvram_protect = qemu_get_be32(f);
>      for (i = 0; i < 15; i++)
>          s->rem_speed[i] = qemu_get_be32(f);
> @@ -687,7 +687,7 @@ static void rc4030_save(QEMUFile *f, void *opaque)
>      qemu_put_be32(f, s->cache_ptag);
>      qemu_put_be32(f, s->cache_ltag);
>      qemu_put_be32(f, s->cache_bmask);
> -    qemu_put_be32(f, s->offset210);
> +    qemu_put_be32(f, s->memory_refresh_rate);
>      qemu_put_be32(f, s->nvram_protect);
>      for (i = 0; i < 15; i++)
>          qemu_put_be32(f, s->rem_speed[i]);

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 06/17] dma/rc4030: use trace events instead of custom logging
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 06/17] dma/rc4030: use trace events instead of custom logging Hervé Poussineau
@ 2015-06-02 11:03   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:03 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Remove also unneeded debug logs.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/dma/rc4030.c | 81 ++++++++++++---------------------------------------------
>  trace-events    |  6 +++++
>  2 files changed, 22 insertions(+), 65 deletions(-)
> 
> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
> index bf82eed..55844ed 100644
> --- a/hw/dma/rc4030.c
> +++ b/hw/dma/rc4030.c
> @@ -26,24 +26,7 @@
>  #include "hw/mips/mips.h"
>  #include "qemu/timer.h"
>  #include "exec/address-spaces.h"
> -
> -/********************************************************/
> -/* debug rc4030 */
> -
> -//#define DEBUG_RC4030
> -//#define DEBUG_RC4030_DMA
> -
> -#ifdef DEBUG_RC4030
> -#define DPRINTF(fmt, ...) \
> -do { printf("rc4030: " fmt , ## __VA_ARGS__); } while (0)
> -static const char* irq_names[] = { "parallel", "floppy", "sound", "video",
> -            "network", "scsi", "keyboard", "mouse", "serial0", "serial1" };
> -#else
> -#define DPRINTF(fmt, ...)
> -#endif
> -
> -#define RC4030_ERROR(fmt, ...) \
> -do { fprintf(stderr, "rc4030 ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); } while (0)
> +#include "trace.h"
>  
>  /********************************************************/
>  /* rc4030 emulation                                     */
> @@ -251,13 +234,14 @@ static uint64_t rc4030_read(void *opaque, hwaddr addr, unsigned int size)
>          val = 7; /* FIXME: should be read from EISA controller */
>          break;
>      default:
> -        RC4030_ERROR("invalid read [" TARGET_FMT_plx "]\n", addr);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "rc4030: invalid read at 0x%x", (int)addr);
>          val = 0;
>          break;
>      }
>  
>      if ((addr & ~3) != 0x230) {
> -        DPRINTF("read 0x%02x at " TARGET_FMT_plx "\n", val, addr);
> +        trace_rc4030_read(addr, val);
>      }
>  
>      return val;
> @@ -360,7 +344,7 @@ static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
>      uint32_t val = data;
>      addr &= 0x3fff;
>  
> -    DPRINTF("write 0x%02x at " TARGET_FMT_plx "\n", val, addr);
> +    trace_rc4030_write(addr, val);
>  
>      switch (addr & ~0x3) {
>      /* Global config register */
> @@ -475,7 +459,9 @@ static void rc4030_write(void *opaque, hwaddr addr, uint64_t data,
>      case 0x0238:
>          break;
>      default:
> -        RC4030_ERROR("invalid write of 0x%02x at [" TARGET_FMT_plx "]\n", val, addr);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "rc4030: invalid write of 0x%02x at 0x%x",
> +                      val, (int)addr);
>          break;
>      }
>  }
> @@ -494,22 +480,6 @@ static void update_jazz_irq(rc4030State *s)
>  
>      pending = s->isr_jazz & s->imr_jazz;
>  
> -#ifdef DEBUG_RC4030
> -    if (s->isr_jazz != 0) {
> -        uint32_t irq = 0;
> -        DPRINTF("pending irqs:");
> -        for (irq = 0; irq < ARRAY_SIZE(irq_names); irq++) {
> -            if (s->isr_jazz & (1 << irq)) {
> -                printf(" %s", irq_names[irq]);
> -                if (!(s->imr_jazz & (1 << irq))) {
> -                    printf("(ignored)");
> -                }
> -            }
> -        }
> -        printf("\n");
> -    }
> -#endif
> -
>      if (pending != 0)
>          qemu_irq_raise(s->jazz_bus_irq);
>      else
> @@ -552,7 +522,6 @@ static uint64_t jazzio_read(void *opaque, hwaddr addr, unsigned int size)
>          irq = 0;
>          while (pending) {
>              if (pending & 1) {
> -                DPRINTF("returning irq %s\n", irq_names[irq]);
>                  val = (irq + 1) << 2;
>                  break;
>              }
> @@ -566,11 +535,13 @@ static uint64_t jazzio_read(void *opaque, hwaddr addr, unsigned int size)
>          val = s->imr_jazz;
>          break;
>      default:
> -        RC4030_ERROR("(jazz io controller) invalid read [" TARGET_FMT_plx "]\n", addr);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "rc4030/jazzio: invalid read at 0x%x", (int)addr);
>          val = 0;
> +        break;
>      }
>  
> -    DPRINTF("(jazz io controller) read 0x%04x at " TARGET_FMT_plx "\n", val, addr);
> +    trace_jazzio_read(addr, val);
>  
>      return val;
>  }
> @@ -582,7 +553,7 @@ static void jazzio_write(void *opaque, hwaddr addr, uint64_t data,
>      uint32_t val = data;
>      addr &= 0xfff;
>  
> -    DPRINTF("(jazz io controller) write 0x%04x at " TARGET_FMT_plx "\n", val, addr);
> +    trace_jazzio_write(addr, val);
>  
>      switch (addr) {
>      /* Local bus int enable mask */
> @@ -591,7 +562,9 @@ static void jazzio_write(void *opaque, hwaddr addr, uint64_t data,
>          update_jazz_irq(s);
>          break;
>      default:
> -        RC4030_ERROR("(jazz io controller) invalid write of 0x%04x at [" TARGET_FMT_plx "]\n", val, addr);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "rc4030/jazzio: invalid write of 0x%02x at 0x%x",
> +                      val, (int)addr);
>          break;
>      }
>  }
> @@ -724,28 +697,6 @@ static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_wri
>  
>      s->dma_regs[n][DMA_REG_ENABLE] |= DMA_FLAG_TC_INTR;
>      s->dma_regs[n][DMA_REG_COUNT] -= len;
> -
> -#ifdef DEBUG_RC4030_DMA
> -    {
> -        int i, j;
> -        printf("rc4030 dma: Copying %d bytes %s host %p\n",
> -            len, is_write ? "from" : "to", buf);
> -        for (i = 0; i < len; i += 16) {
> -            int n = 16;
> -            if (n > len - i) {
> -                n = len - i;
> -            }
> -            for (j = 0; j < n; j++)
> -                printf("%02x ", buf[i + j]);
> -            while (j++ < 16)
> -                printf("   ");
> -            printf("| ");
> -            for (j = 0; j < n; j++)
> -                printf("%c", isprint(buf[i + j]) ? buf[i + j] : '.');
> -            printf("\n");
> -        }
> -    }
> -#endif
>  }
>  
>  struct rc4030DMAState {
> diff --git a/trace-events b/trace-events
> index 11387c3..1443e84 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -282,6 +282,12 @@ slavio_timer_mem_writel_mode_counter(unsigned int timer_index) "processor %d cha
>  slavio_timer_mem_writel_mode_invalid(void) "not system timer"
>  slavio_timer_mem_writel_invalid(uint64_t addr) "invalid write address %"PRIx64
>  
> +# hw/dma/rc4030.c
> +jazzio_read(uint64_t addr, uint32_t ret) "read reg[0x%"PRIx64"] = 0x%x"
> +jazzio_write(uint64_t addr, uint32_t val) "write reg[0x%"PRIx64"] = 0x%x"
> +rc4030_read(uint64_t addr, uint32_t ret) "read reg[0x%"PRIx64"] = 0x%x"
> +rc4030_write(uint64_t addr, uint32_t val) "write reg[0x%"PRIx64"] = 0x%x"
> +
>  # hw/dma/sparc32_dma.c
>  ledma_memory_read(uint64_t addr) "DMA read addr 0x%"PRIx64
>  ledma_memory_write(uint64_t addr) "DMA write addr 0x%"PRIx64

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 07/17] dma/rc4030: convert to QOM
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 07/17] dma/rc4030: convert to QOM Hervé Poussineau
@ 2015-06-02 11:03   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:03 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/dma/rc4030.c        | 115 ++++++++++++++++++++++++++++++++++++++-----------
>  hw/mips/mips_jazz.c    |  37 ++++++++++------
>  include/hw/mips/mips.h |   4 +-
>  3 files changed, 113 insertions(+), 43 deletions(-)
> 
> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
> index 55844ed..3efa6de 100644
> --- a/hw/dma/rc4030.c
> +++ b/hw/dma/rc4030.c
> @@ -1,7 +1,7 @@
>  /*
>   * QEMU JAZZ RC4030 chipset
>   *
> - * Copyright (c) 2007-2009 Herve Poussineau
> + * Copyright (c) 2007-2013 Hervé Poussineau
>   *
>   * Permission is hereby granted, free of charge, to any person obtaining a copy
>   * of this software and associated documentation files (the "Software"), to deal
> @@ -24,6 +24,7 @@
>  
>  #include "hw/hw.h"
>  #include "hw/mips/mips.h"
> +#include "hw/sysbus.h"
>  #include "qemu/timer.h"
>  #include "exec/address-spaces.h"
>  #include "trace.h"
> @@ -49,8 +50,14 @@ typedef struct dma_pagetable_entry {
>  #define DMA_FLAG_MEM_INTR   0x0200
>  #define DMA_FLAG_ADDR_INTR  0x0400
>  
> +#define TYPE_RC4030 "rc4030"
> +#define RC4030(obj) \
> +    OBJECT_CHECK(rc4030State, (obj), TYPE_RC4030)
> +
>  typedef struct rc4030State
>  {
> +    SysBusDevice parent;
> +
>      uint32_t config; /* 0x0000: RC4030 config register */
>      uint32_t revision; /* 0x0008: RC4030 Revision register */
>      uint32_t invalid_address_register; /* 0x0010: Invalid Address register */
> @@ -317,7 +324,7 @@ static void rc4030_dma_tt_update(rc4030State *s, uint32_t new_tl_base,
>          } else {
>              dma_tt_size = memory_region_size(&s->dma_tt);
>          }
> -        memory_region_init_alias(&s->dma_tt_alias, NULL,
> +        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);
> @@ -332,7 +339,7 @@ static void rc4030_dma_tt_update(rc4030State *s, uint32_t new_tl_base,
>                                      &s->dma_tt_alias);
>          memory_region_transaction_commit();
>      } else {
> -        memory_region_init(&s->dma_tt_alias, NULL,
> +        memory_region_init(&s->dma_tt_alias, OBJECT(s),
>                             "dma-table-alias", 0);
>      }
>  }
> @@ -577,9 +584,9 @@ static const MemoryRegionOps jazzio_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> -static void rc4030_reset(void *opaque)
> +static void rc4030_reset(DeviceState *dev)
>  {
> -    rc4030State *s = opaque;
> +    rc4030State *s = RC4030(dev);
>      int i;
>  
>      s->config = 0x410; /* some boards seem to accept 0x104 too */
> @@ -733,46 +740,102 @@ static rc4030_dma *rc4030_allocate_dmas(void *opaque, int n)
>      return s;
>  }
>  
> -MemoryRegion *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
> -                          qemu_irq **irqs, rc4030_dma **dmas,
> -                          MemoryRegion *sysmem)
> +static void rc4030_initfn(Object *obj)
>  {
> -    rc4030State *s;
> -    int i;
> -
> -    s = g_malloc0(sizeof(rc4030State));
> +    DeviceState *dev = DEVICE(obj);
> +    rc4030State *s = RC4030(obj);
> +    SysBusDevice *sysbus = SYS_BUS_DEVICE(obj);
>  
> -    *irqs = qemu_allocate_irqs(rc4030_irq_jazz_request, s, 16);
> -    *dmas = rc4030_allocate_dmas(s, 4);
> +    qdev_init_gpio_in(dev, rc4030_irq_jazz_request, 16);
>  
> -    s->periodic_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, rc4030_periodic_timer, s);
> -    s->timer_irq = timer;
> -    s->jazz_bus_irq = jazz_bus;
> +    sysbus_init_irq(sysbus, &s->timer_irq);
> +    sysbus_init_irq(sysbus, &s->jazz_bus_irq);
>  
> -    qemu_register_reset(rc4030_reset, s);
>      register_savevm(NULL, "rc4030", 0, 2, rc4030_save, rc4030_load, s);
> -    rc4030_reset(s);
> +
> +    sysbus_init_mmio(sysbus, &s->iomem_chipset);
> +    sysbus_init_mmio(sysbus, &s->iomem_jazzio);
> +}
> +
> +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);
>  
>      memory_region_init_io(&s->iomem_chipset, NULL, &rc4030_ops, s,
>                            "rc4030.chipset", 0x300);
> -    memory_region_add_subregion(sysmem, 0x80000000, &s->iomem_chipset);
>      memory_region_init_io(&s->iomem_jazzio, NULL, &jazzio_ops, s,
>                            "rc4030.jazzio", 0x00001000);
> -    memory_region_add_subregion(sysmem, 0xf0000000, &s->iomem_jazzio);
>  
> -    memory_region_init_rom_device(&s->dma_tt, NULL,
> +    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, NULL, "dma-table-alias", 0);
> -    memory_region_init(&s->dma_mr, NULL, "dma", INT32_MAX);
> +    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], NULL, "dma-alias",
> +        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]);
>      }
>      address_space_init(&s->dma_as, &s->dma_mr, "rc4030-dma");
> -    return &s->dma_mr;
> +}
> +
> +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)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = rc4030_realize;
> +    dc->unrealize = rc4030_unrealize;
> +    dc->reset = rc4030_reset;
> +}
> +
> +static const TypeInfo rc4030_info = {
> +    .name = TYPE_RC4030,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(rc4030State),
> +    .instance_init = rc4030_initfn,
> +    .class_init = rc4030_class_init,
> +};
> +
> +static void rc4030_register_types(void)
> +{
> +    type_register_static(&rc4030_info);
> +}
> +
> +type_init(rc4030_register_types)
> +
> +DeviceState *rc4030_init(rc4030_dma **dmas, MemoryRegion **dma_mr)
> +{
> +    DeviceState *dev;
> +
> +    dev = qdev_create(NULL, TYPE_RC4030);
> +    qdev_init_nofail(dev);
> +
> +    *dmas = rc4030_allocate_dmas(dev, 4);
> +    *dma_mr = &RC4030(dev)->dma_mr;
> +    return dev;
>  }
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 05cad6b..29a13c0 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -135,7 +135,7 @@ static void mips_jazz_init(MachineState *machine,
>      MIPSCPU *cpu;
>      CPUClass *cc;
>      CPUMIPSState *env;
> -    qemu_irq *rc4030, *i8259;
> +    qemu_irq *i8259;
>      rc4030_dma *dmas;
>      MemoryRegion *rc4030_dma_mr;
>      MemoryRegion *isa_mem = g_new(MemoryRegion, 1);
> @@ -144,7 +144,7 @@ static void mips_jazz_init(MachineState *machine,
>      MemoryRegion *i8042 = g_new(MemoryRegion, 1);
>      MemoryRegion *dma_dummy = g_new(MemoryRegion, 1);
>      NICInfo *nd;
> -    DeviceState *dev;
> +    DeviceState *dev, *rc4030;
>      SysBusDevice *sysbus;
>      ISABus *isa_bus;
>      ISADevice *pit;
> @@ -213,8 +213,14 @@ static void mips_jazz_init(MachineState *machine,
>      cpu_mips_clock_init(env);
>  
>      /* Chipset */
> -    rc4030_dma_mr = rc4030_init(env->irq[6], env->irq[3], &rc4030, &dmas,
> -                                address_space);
> +    rc4030 = rc4030_init(&dmas, &rc4030_dma_mr);
> +    sysbus = SYS_BUS_DEVICE(rc4030);
> +    sysbus_connect_irq(sysbus, 0, env->irq[6]);
> +    sysbus_connect_irq(sysbus, 1, env->irq[3]);
> +    memory_region_add_subregion(address_space, 0x80000000,
> +                                sysbus_mmio_get_region(sysbus, 0));
> +    memory_region_add_subregion(address_space, 0xf0000000,
> +                                sysbus_mmio_get_region(sysbus, 1));
>      memory_region_init_io(dma_dummy, NULL, &dma_dummy_ops, NULL, "dummy_dma", 0x1000);
>      memory_region_add_subregion(address_space, 0x8000d000, dma_dummy);
>  
> @@ -241,7 +247,7 @@ static void mips_jazz_init(MachineState *machine,
>          sysbus = SYS_BUS_DEVICE(dev);
>          sysbus_mmio_map(sysbus, 0, 0x60080000);
>          sysbus_mmio_map(sysbus, 1, 0x40000000);
> -        sysbus_connect_irq(sysbus, 0, rc4030[3]);
> +        sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(rc4030, 3));
>          {
>              /* Simple ROM, so user doesn't have to provide one */
>              MemoryRegion *rom_mr = g_new(MemoryRegion, 1);
> @@ -267,8 +273,8 @@ static void mips_jazz_init(MachineState *machine,
>          if (!nd->model)
>              nd->model = g_strdup("dp83932");
>          if (strcmp(nd->model, "dp83932") == 0) {
> -            dp83932_init(nd, 0x80001000, 2, get_system_memory(), rc4030[4],
> -                         rc4030_dma_mr);
> +            dp83932_init(nd, 0x80001000, 2, get_system_memory(),
> +                         qdev_get_gpio_in(rc4030, 4), rc4030_dma_mr);
>              break;
>          } else if (is_help_option(nd->model)) {
>              fprintf(stderr, "qemu: Supported NICs: dp83932\n");
> @@ -282,7 +288,7 @@ static void mips_jazz_init(MachineState *machine,
>      /* SCSI adapter */
>      esp_init(0x80002000, 0,
>               rc4030_dma_read, rc4030_dma_write, dmas[0],
> -             rc4030[5], &esp_reset, &dma_enable);
> +             qdev_get_gpio_in(rc4030, 5), &esp_reset, &dma_enable);
>  
>      /* Floppy */
>      if (drive_get_max_bus(IF_FLOPPY) >= MAX_FD) {
> @@ -292,7 +298,7 @@ static void mips_jazz_init(MachineState *machine,
>      for (n = 0; n < MAX_FD; n++) {
>          fds[n] = drive_get(IF_FLOPPY, 0, n);
>      }
> -    fdctrl_init_sysbus(rc4030[1], 0, 0x80003000, fds);
> +    fdctrl_init_sysbus(qdev_get_gpio_in(rc4030, 1), 0, 0x80003000, fds);
>  
>      /* Real time clock */
>      rtc_init(isa_bus, 1980, NULL);
> @@ -300,23 +306,26 @@ static void mips_jazz_init(MachineState *machine,
>      memory_region_add_subregion(address_space, 0x80004000, rtc);
>  
>      /* Keyboard (i8042) */
> -    i8042_mm_init(rc4030[6], rc4030[7], i8042, 0x1000, 0x1);
> +    i8042_mm_init(qdev_get_gpio_in(rc4030, 6), qdev_get_gpio_in(rc4030, 7),
> +                  i8042, 0x1000, 0x1);
>      memory_region_add_subregion(address_space, 0x80005000, i8042);
>  
>      /* Serial ports */
>      if (serial_hds[0]) {
> -        serial_mm_init(address_space, 0x80006000, 0, rc4030[8], 8000000/16,
> +        serial_mm_init(address_space, 0x80006000, 0,
> +                       qdev_get_gpio_in(rc4030, 8), 8000000/16,
>                         serial_hds[0], DEVICE_NATIVE_ENDIAN);
>      }
>      if (serial_hds[1]) {
> -        serial_mm_init(address_space, 0x80007000, 0, rc4030[9], 8000000/16,
> +        serial_mm_init(address_space, 0x80007000, 0,
> +                       qdev_get_gpio_in(rc4030, 9), 8000000/16,
>                         serial_hds[1], DEVICE_NATIVE_ENDIAN);
>      }
>  
>      /* Parallel port */
>      if (parallel_hds[0])
> -        parallel_mm_init(address_space, 0x80008000, 0, rc4030[0],
> -                         parallel_hds[0]);
> +        parallel_mm_init(address_space, 0x80008000, 0,
> +                         qdev_get_gpio_in(rc4030, 0), parallel_hds[0]);
>  
>      /* FIXME: missing Jazz sound at 0x8000c000, rc4030[2] */
>  
> diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
> index 47eb31f..31b4729 100644
> --- a/include/hw/mips/mips.h
> +++ b/include/hw/mips/mips.h
> @@ -18,9 +18,7 @@ typedef struct rc4030DMAState *rc4030_dma;
>  void rc4030_dma_read(void *dma, uint8_t *buf, int len);
>  void rc4030_dma_write(void *dma, uint8_t *buf, int len);
>  
> -MemoryRegion *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
> -                          qemu_irq **irqs, rc4030_dma **dmas,
> -                          MemoryRegion *sysmem);
> +DeviceState *rc4030_init(rc4030_dma **dmas, MemoryRegion **dma_mr);
>  
>  /* dp8393x.c */
>  void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,

I am not very comfortable with the QOM API, but it looks fine to me.

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>


-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 08/17] net/dp8393x: always calculate proper checksums
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 08/17] net/dp8393x: always calculate proper checksums Hervé Poussineau
@ 2015-06-02 11:03   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:03 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/net/dp8393x.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 2297231..093f0cc 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -21,16 +21,10 @@
>  #include "qemu/timer.h"
>  #include "net/net.h"
>  #include "hw/mips/mips.h"
> +#include <zlib.h>
>  
>  //#define DEBUG_SONIC
>  
> -/* Calculate CRCs properly on Rx packets */
> -#define SONIC_CALCULATE_RXCRC
> -
> -#if defined(SONIC_CALCULATE_RXCRC)
> -/* For crc32 */
> -#include <zlib.h>
> -#endif
>  
>  #ifdef DEBUG_SONIC
>  #define DPRINTF(fmt, ...) \
> @@ -764,11 +758,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
>      s->regs[SONIC_TRBA0] = s->regs[SONIC_CRBA0];
>  
>      /* Calculate the ethernet checksum */
> -#ifdef SONIC_CALCULATE_RXCRC
>      checksum = cpu_to_le32(crc32(0, buf, rx_len));
> -#else
> -    checksum = 0;
> -#endif
>  
>      /* Put packet into RBA */
>      DPRINTF("Receive packet at %08x\n", (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0]);

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 09/17] net/dp8393x: do not use old_mmio accesses
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 09/17] net/dp8393x: do not use old_mmio accesses Hervé Poussineau
@ 2015-06-02 11:03   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:03 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/net/dp8393x.c | 114 ++++++++++++++-----------------------------------------
>  1 file changed, 29 insertions(+), 85 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 093f0cc..5cc1e6b 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -473,8 +473,10 @@ static void do_command(dp8393xState *s, uint16_t command)
>          do_load_cam(s);
>  }
>  
> -static uint16_t read_register(dp8393xState *s, int reg)
> +static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size)
>  {
> +    dp8393xState *s = opaque;
> +    int reg = addr >> s->it_shift;
>      uint16_t val = 0;
>  
>      switch (reg) {
> @@ -503,14 +505,18 @@ static uint16_t read_register(dp8393xState *s, int reg)
>      return val;
>  }
>  
> -static void write_register(dp8393xState *s, int reg, uint16_t val)
> +static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
> +                          unsigned int size)
>  {
> -    DPRINTF("write 0x%04x to reg %s\n", val, reg_names[reg]);
> +    dp8393xState *s = opaque;
> +    int reg = addr >> s->it_shift;
> +
> +    DPRINTF("write 0x%04x to reg %s\n", (uint16_t)data, reg_names[reg]);
>  
>      switch (reg) {
>          /* Command register */
>          case SONIC_CR:
> -            do_command(s, val);
> +            do_command(s, data);
>              break;
>          /* Prevent write to read-only registers */
>          case SONIC_CAP2:
> @@ -523,36 +529,36 @@ static void write_register(dp8393xState *s, int reg, uint16_t val)
>          /* Accept write to some registers only when in reset mode */
>          case SONIC_DCR:
>              if (s->regs[SONIC_CR] & SONIC_CR_RST) {
> -                s->regs[reg] = val & 0xbfff;
> +                s->regs[reg] = data & 0xbfff;
>              } else {
>                  DPRINTF("writing to DCR invalid\n");
>              }
>              break;
>          case SONIC_DCR2:
>              if (s->regs[SONIC_CR] & SONIC_CR_RST) {
> -                s->regs[reg] = val & 0xf017;
> +                s->regs[reg] = data & 0xf017;
>              } else {
>                  DPRINTF("writing to DCR2 invalid\n");
>              }
>              break;
>          /* 12 lower bytes are Read Only */
>          case SONIC_TCR:
> -            s->regs[reg] = val & 0xf000;
> +            s->regs[reg] = data & 0xf000;
>              break;
>          /* 9 lower bytes are Read Only */
>          case SONIC_RCR:
> -            s->regs[reg] = val & 0xffe0;
> +            s->regs[reg] = data & 0xffe0;
>              break;
>          /* Ignore most significant bit */
>          case SONIC_IMR:
> -            s->regs[reg] = val & 0x7fff;
> +            s->regs[reg] = data & 0x7fff;
>              dp8393x_update_irq(s);
>              break;
>          /* Clear bits by writing 1 to them */
>          case SONIC_ISR:
> -            val &= s->regs[reg];
> -            s->regs[reg] &= ~val;
> -            if (val & SONIC_ISR_RBE) {
> +            data &= s->regs[reg];
> +            s->regs[reg] &= ~data;
> +            if (data & SONIC_ISR_RBE) {
>                  do_read_rra(s);
>              }
>              dp8393x_update_irq(s);
> @@ -562,17 +568,17 @@ static void write_register(dp8393xState *s, int reg, uint16_t val)
>          case SONIC_REA:
>          case SONIC_RRP:
>          case SONIC_RWP:
> -            s->regs[reg] = val & 0xfffe;
> +            s->regs[reg] = data & 0xfffe;
>              break;
>          /* Invert written value for some registers */
>          case SONIC_CRCT:
>          case SONIC_FAET:
>          case SONIC_MPT:
> -            s->regs[reg] = val ^ 0xffff;
> +            s->regs[reg] = data ^ 0xffff;
>              break;
>          /* All other registers have no special contrainst */
>          default:
> -            s->regs[reg] = val;
> +            s->regs[reg] = data;
>      }
>  
>      if (reg == SONIC_WT0 || reg == SONIC_WT1) {
> @@ -580,6 +586,14 @@ static void write_register(dp8393xState *s, int reg, uint16_t val)
>      }
>  }
>  
> +static const MemoryRegionOps dp8393x_ops = {
> +    .read = dp8393x_read,
> +    .write = dp8393x_write,
> +    .impl.min_access_size = 2,
> +    .impl.max_access_size = 2,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
>  static void dp8393x_watchdog(void *opaque)
>  {
>      dp8393xState *s = opaque;
> @@ -597,76 +611,6 @@ static void dp8393x_watchdog(void *opaque)
>      dp8393x_update_irq(s);
>  }
>  
> -static uint32_t dp8393x_readw(void *opaque, hwaddr addr)
> -{
> -    dp8393xState *s = opaque;
> -    int reg;
> -
> -    if ((addr & ((1 << s->it_shift) - 1)) != 0) {
> -        return 0;
> -    }
> -
> -    reg = addr >> s->it_shift;
> -    return read_register(s, reg);
> -}
> -
> -static uint32_t dp8393x_readb(void *opaque, hwaddr addr)
> -{
> -    uint16_t v = dp8393x_readw(opaque, addr & ~0x1);
> -    return (v >> (8 * (addr & 0x1))) & 0xff;
> -}
> -
> -static uint32_t dp8393x_readl(void *opaque, hwaddr addr)
> -{
> -    uint32_t v;
> -    v = dp8393x_readw(opaque, addr);
> -    v |= dp8393x_readw(opaque, addr + 2) << 16;
> -    return v;
> -}
> -
> -static void dp8393x_writew(void *opaque, hwaddr addr, uint32_t val)
> -{
> -    dp8393xState *s = opaque;
> -    int reg;
> -
> -    if ((addr & ((1 << s->it_shift) - 1)) != 0) {
> -        return;
> -    }
> -
> -    reg = addr >> s->it_shift;
> -
> -    write_register(s, reg, (uint16_t)val);
> -}
> -
> -static void dp8393x_writeb(void *opaque, hwaddr addr, uint32_t val)
> -{
> -    uint16_t old_val = dp8393x_readw(opaque, addr & ~0x1);
> -
> -    switch (addr & 3) {
> -    case 0:
> -        val = val | (old_val & 0xff00);
> -        break;
> -    case 1:
> -        val = (val << 8) | (old_val & 0x00ff);
> -        break;
> -    }
> -    dp8393x_writew(opaque, addr & ~0x1, val);
> -}
> -
> -static void dp8393x_writel(void *opaque, hwaddr addr, uint32_t val)
> -{
> -    dp8393x_writew(opaque, addr, val & 0xffff);
> -    dp8393x_writew(opaque, addr + 2, (val >> 16) & 0xffff);
> -}
> -
> -static const MemoryRegionOps dp8393x_ops = {
> -    .old_mmio = {
> -        .read = { dp8393x_readb, dp8393x_readw, dp8393x_readl, },
> -        .write = { dp8393x_writeb, dp8393x_writew, dp8393x_writel, },
> -    },
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> -};
> -
>  static int nic_can_receive(NetClientState *nc)
>  {
>      dp8393xState *s = qemu_get_nic_opaque(nc);

Nice cleanup :)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 10/17] net/dp8393x: use dp8393x_ prefix for all functions
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 10/17] net/dp8393x: use dp8393x_ prefix for all functions Hervé Poussineau
@ 2015-06-02 11:04   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:04 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/net/dp8393x.c | 80 +++++++++++++++++++++++++++++---------------------------
>  1 file changed, 41 insertions(+), 39 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 5cc1e6b..0aff04f 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -183,7 +183,7 @@ static void dp8393x_update_irq(dp8393xState *s)
>      qemu_set_irq(s->irq, level);
>  }
>  
> -static void do_load_cam(dp8393xState *s)
> +static void dp8393x_do_load_cam(dp8393xState *s)
>  {
>      uint16_t data[8];
>      int width, size;
> @@ -225,7 +225,7 @@ static void do_load_cam(dp8393xState *s)
>      dp8393x_update_irq(s);
>  }
>  
> -static void do_read_rra(dp8393xState *s)
> +static void dp8393x_do_read_rra(dp8393xState *s)
>  {
>      uint16_t data[8];
>      int width, size;
> @@ -265,7 +265,7 @@ static void do_read_rra(dp8393xState *s)
>      s->regs[SONIC_CR] &= ~SONIC_CR_RRRA;
>  }
>  
> -static void do_software_reset(dp8393xState *s)
> +static void dp8393x_do_software_reset(dp8393xState *s)
>  {
>      timer_del(s->watchdog);
>  
> @@ -273,7 +273,7 @@ static void do_software_reset(dp8393xState *s)
>      s->regs[SONIC_CR] |= SONIC_CR_RST | SONIC_CR_RXDIS;
>  }
>  
> -static void set_next_tick(dp8393xState *s)
> +static void dp8393x_set_next_tick(dp8393xState *s)
>  {
>      uint32_t ticks;
>      int64_t delay;
> @@ -289,7 +289,7 @@ static void set_next_tick(dp8393xState *s)
>      timer_mod(s->watchdog, s->wt_last_update + delay);
>  }
>  
> -static void update_wt_regs(dp8393xState *s)
> +static void dp8393x_update_wt_regs(dp8393xState *s)
>  {
>      int64_t elapsed;
>      uint32_t val;
> @@ -304,33 +304,33 @@ static void update_wt_regs(dp8393xState *s)
>      val -= elapsed / 5000000;
>      s->regs[SONIC_WT1] = (val >> 16) & 0xffff;
>      s->regs[SONIC_WT0] = (val >> 0)  & 0xffff;
> -    set_next_tick(s);
> +    dp8393x_set_next_tick(s);
>  
>  }
>  
> -static void do_start_timer(dp8393xState *s)
> +static void dp8393x_do_start_timer(dp8393xState *s)
>  {
>      s->regs[SONIC_CR] &= ~SONIC_CR_STP;
> -    set_next_tick(s);
> +    dp8393x_set_next_tick(s);
>  }
>  
> -static void do_stop_timer(dp8393xState *s)
> +static void dp8393x_do_stop_timer(dp8393xState *s)
>  {
>      s->regs[SONIC_CR] &= ~SONIC_CR_ST;
> -    update_wt_regs(s);
> +    dp8393x_update_wt_regs(s);
>  }
>  
> -static void do_receiver_enable(dp8393xState *s)
> +static void dp8393x_do_receiver_enable(dp8393xState *s)
>  {
>      s->regs[SONIC_CR] &= ~SONIC_CR_RXDIS;
>  }
>  
> -static void do_receiver_disable(dp8393xState *s)
> +static void dp8393x_do_receiver_disable(dp8393xState *s)
>  {
>      s->regs[SONIC_CR] &= ~SONIC_CR_RXEN;
>  }
>  
> -static void do_transmit_packets(dp8393xState *s)
> +static void dp8393x_do_transmit_packets(dp8393xState *s)
>  {
>      NetClientState *nc = qemu_get_queue(s->nic);
>      uint16_t data[12];
> @@ -439,12 +439,12 @@ static void do_transmit_packets(dp8393xState *s)
>      dp8393x_update_irq(s);
>  }
>  
> -static void do_halt_transmission(dp8393xState *s)
> +static void dp8393x_do_halt_transmission(dp8393xState *s)
>  {
>      /* Nothing to do */
>  }
>  
> -static void do_command(dp8393xState *s, uint16_t command)
> +static void dp8393x_do_command(dp8393xState *s, uint16_t command)
>  {
>      if ((s->regs[SONIC_CR] & SONIC_CR_RST) && !(command & SONIC_CR_RST)) {
>          s->regs[SONIC_CR] &= ~SONIC_CR_RST;
> @@ -454,23 +454,23 @@ static void do_command(dp8393xState *s, uint16_t command)
>      s->regs[SONIC_CR] |= (command & SONIC_CR_MASK);
>  
>      if (command & SONIC_CR_HTX)
> -        do_halt_transmission(s);
> +        dp8393x_do_halt_transmission(s);
>      if (command & SONIC_CR_TXP)
> -        do_transmit_packets(s);
> +        dp8393x_do_transmit_packets(s);
>      if (command & SONIC_CR_RXDIS)
> -        do_receiver_disable(s);
> +        dp8393x_do_receiver_disable(s);
>      if (command & SONIC_CR_RXEN)
> -        do_receiver_enable(s);
> +        dp8393x_do_receiver_enable(s);
>      if (command & SONIC_CR_STP)
> -        do_stop_timer(s);
> +        dp8393x_do_stop_timer(s);
>      if (command & SONIC_CR_ST)
> -        do_start_timer(s);
> +        dp8393x_do_start_timer(s);
>      if (command & SONIC_CR_RST)
> -        do_software_reset(s);
> +        dp8393x_do_software_reset(s);
>      if (command & SONIC_CR_RRRA)
> -        do_read_rra(s);
> +        dp8393x_do_read_rra(s);
>      if (command & SONIC_CR_LCAM)
> -        do_load_cam(s);
> +        dp8393x_do_load_cam(s);
>  }
>  
>  static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size)
> @@ -483,7 +483,7 @@ static uint64_t dp8393x_read(void *opaque, hwaddr addr, unsigned int size)
>          /* Update data before reading it */
>          case SONIC_WT0:
>          case SONIC_WT1:
> -            update_wt_regs(s);
> +            dp8393x_update_wt_regs(s);
>              val = s->regs[reg];
>              break;
>          /* Accept read to some registers only when in reset mode */
> @@ -516,7 +516,7 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
>      switch (reg) {
>          /* Command register */
>          case SONIC_CR:
> -            do_command(s, data);
> +            dp8393x_do_command(s, data);
>              break;
>          /* Prevent write to read-only registers */
>          case SONIC_CAP2:
> @@ -559,7 +559,7 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
>              data &= s->regs[reg];
>              s->regs[reg] &= ~data;
>              if (data & SONIC_ISR_RBE) {
> -                do_read_rra(s);
> +                dp8393x_do_read_rra(s);
>              }
>              dp8393x_update_irq(s);
>              break;
> @@ -582,7 +582,7 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
>      }
>  
>      if (reg == SONIC_WT0 || reg == SONIC_WT1) {
> -        set_next_tick(s);
> +        dp8393x_set_next_tick(s);
>      }
>  }
>  
> @@ -604,14 +604,14 @@ static void dp8393x_watchdog(void *opaque)
>  
>      s->regs[SONIC_WT1] = 0xffff;
>      s->regs[SONIC_WT0] = 0xffff;
> -    set_next_tick(s);
> +    dp8393x_set_next_tick(s);
>  
>      /* Signal underflow */
>      s->regs[SONIC_ISR] |= SONIC_ISR_TC;
>      dp8393x_update_irq(s);
>  }
>  
> -static int nic_can_receive(NetClientState *nc)
> +static int dp8393x_can_receive(NetClientState *nc)
>  {
>      dp8393xState *s = qemu_get_nic_opaque(nc);
>  
> @@ -622,7 +622,8 @@ static int nic_can_receive(NetClientState *nc)
>      return 1;
>  }
>  
> -static int receive_filter(dp8393xState *s, const uint8_t * buf, int size)
> +static int dp8393x_receive_filter(dp8393xState *s, const uint8_t * buf,
> +                                  int size)
>  {
>      static const uint8_t bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
>      int i;
> @@ -660,7 +661,8 @@ static int receive_filter(dp8393xState *s, const uint8_t * buf, int size)
>      return -1;
>  }
>  
> -static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
> +static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
> +                               size_t size)
>  {
>      dp8393xState *s = qemu_get_nic_opaque(nc);
>      uint16_t data[10];
> @@ -674,7 +676,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
>      s->regs[SONIC_RCR] &= ~(SONIC_RCR_PRX | SONIC_RCR_LBK | SONIC_RCR_FAER |
>          SONIC_RCR_CRCR | SONIC_RCR_LPKT | SONIC_RCR_BC | SONIC_RCR_MC);
>  
> -    packet_type = receive_filter(s, buf, size);
> +    packet_type = dp8393x_receive_filter(s, buf, size);
>      if (packet_type < 0) {
>          DPRINTF("packet not for netcard\n");
>          return -1;
> @@ -762,7 +764,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
>  
>          if (s->regs[SONIC_RCR] & SONIC_RCR_LPKT) {
>              /* Read next RRA */
> -            do_read_rra(s);
> +            dp8393x_do_read_rra(s);
>          }
>      }
>  
> @@ -772,7 +774,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
>      return size;
>  }
>  
> -static void nic_reset(void *opaque)
> +static void dp8393x_reset(void *opaque)
>  {
>      dp8393xState *s = opaque;
>      timer_del(s->watchdog);
> @@ -799,8 +801,8 @@ static void nic_reset(void *opaque)
>  static NetClientInfo net_dp83932_info = {
>      .type = NET_CLIENT_OPTIONS_KIND_NIC,
>      .size = sizeof(NICState),
> -    .can_receive = nic_can_receive,
> -    .receive = nic_receive,
> +    .can_receive = dp8393x_can_receive,
> +    .receive = dp8393x_receive,
>  };
>  
>  void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
> @@ -826,8 +828,8 @@ void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
>      s->nic = qemu_new_nic(&net_dp83932_info, &s->conf, nd->model, nd->name, s);
>  
>      qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
> -    qemu_register_reset(nic_reset, s);
> -    nic_reset(s);
> +    qemu_register_reset(dp8393x_reset, s);
> +    dp8393x_reset(s);
>  
>      memory_region_init_io(&s->mmio, NULL, &dp8393x_ops, s,
>                            "dp8393x", 0x40 << it_shift);

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 11/17] net/dp8393x: QOM'ify
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 11/17] net/dp8393x: QOM'ify Hervé Poussineau
@ 2015-06-02 11:04   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:04 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel, Laurent Vivier

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/mips/mips_jazz.c    | 12 ++++++--
>  hw/net/dp8393x.c       | 83 ++++++++++++++++++++++++++++++++++----------------
>  include/hw/mips/mips.h |  5 ---
>  3 files changed, 67 insertions(+), 33 deletions(-)
> 
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 29a13c0..648654e 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -273,8 +273,16 @@ static void mips_jazz_init(MachineState *machine,
>          if (!nd->model)
>              nd->model = g_strdup("dp83932");
>          if (strcmp(nd->model, "dp83932") == 0) {
> -            dp83932_init(nd, 0x80001000, 2, get_system_memory(),
> -                         qdev_get_gpio_in(rc4030, 4), rc4030_dma_mr);
> +            qemu_check_nic_model(nd, "dp83932");
> +
> +            dev = qdev_create(NULL, "dp8393x");
> +            qdev_set_nic_properties(dev, nd);
> +            qdev_prop_set_uint8(dev, "it_shift", 2);
> +            qdev_prop_set_ptr(dev, "dma_mr", rc4030_dma_mr);
> +            qdev_init_nofail(dev);
> +            sysbus = SYS_BUS_DEVICE(dev);
> +            sysbus_mmio_map(sysbus, 0, 0x80001000);
> +            sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(rc4030, 4));
>              break;
>          } else if (is_help_option(nd->model)) {
>              fprintf(stderr, "qemu: Supported NICs: dp83932\n");
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 0aff04f..51e728b 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -17,10 +17,10 @@
>   * with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>  
> -#include "hw/hw.h"
> -#include "qemu/timer.h"
> +#include "hw/sysbus.h"
> +#include "hw/devices.h"
>  #include "net/net.h"
> -#include "hw/mips/mips.h"
> +#include "qemu/timer.h"
>  #include <zlib.h>
>  
>  //#define DEBUG_SONIC
> @@ -139,9 +139,14 @@ do { printf("sonic ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); } while (0)
>  #define SONIC_ISR_PINT   0x0800
>  #define SONIC_ISR_LCD    0x1000
>  
> +#define TYPE_DP8393X "dp8393x"
> +#define DP8393X(obj) OBJECT_CHECK(dp8393xState, (obj), TYPE_DP8393X)
> +
>  typedef struct dp8393xState {
> +    SysBusDevice parent_obj;
> +
>      /* Hardware */
> -    int it_shift;
> +    uint8_t it_shift;
>      qemu_irq irq;
>  #ifdef DEBUG_SONIC
>      int irq_level;
> @@ -150,7 +155,6 @@ typedef struct dp8393xState {
>      int64_t wt_last_update;
>      NICConf conf;
>      NICState *nic;
> -    MemoryRegion *address_space;
>      MemoryRegion mmio;
>  
>      /* Registers */
> @@ -162,6 +166,7 @@ typedef struct dp8393xState {
>      int loopback_packet;
>  
>      /* Memory access */
> +    void *dma_mr;
>      AddressSpace as;
>  } dp8393xState;
>  
> @@ -774,9 +779,9 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
>      return size;
>  }
>  
> -static void dp8393x_reset(void *opaque)
> +static void dp8393x_reset(DeviceState *dev)
>  {
> -    dp8393xState *s = opaque;
> +    dp8393xState *s = DP8393X(dev);
>      timer_del(s->watchdog);
>  
>      s->regs[SONIC_CR] = SONIC_CR_RST | SONIC_CR_STP | SONIC_CR_RXDIS;
> @@ -805,33 +810,59 @@ static NetClientInfo net_dp83932_info = {
>      .receive = dp8393x_receive,
>  };
>  
> -void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
> -                  MemoryRegion *address_space,
> -                  qemu_irq irq, MemoryRegion *dma_mr)
> +static void dp8393x_instance_init(Object *obj)
>  {
> -    dp8393xState *s;
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    dp8393xState *s = DP8393X(obj);
>  
> -    qemu_check_nic_model(nd, "dp83932");
> +    sysbus_init_mmio(sbd, &s->mmio);
> +    sysbus_init_irq(sbd, &s->irq);
> +}
> +
> +static void dp8393x_realize(DeviceState *dev, Error **errp)
> +{
> +    dp8393xState *s = DP8393X(dev);
>  
> -    s = g_malloc0(sizeof(dp8393xState));
> +    address_space_init(&s->as, s->dma_mr, "dp8393x");
> +    memory_region_init_io(&s->mmio, OBJECT(dev), &dp8393x_ops, s,
> +                          "dp8393x-regs", 0x40 << s->it_shift);
> +
> +    s->nic = qemu_new_nic(&net_dp83932_info, &s->conf,
> +                          object_get_typename(OBJECT(dev)), dev->id, s);
> +    qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
>  
> -    s->address_space = address_space;
> -    address_space_init(&s->as, dma_mr, "dp8393x-dma");
> -    s->it_shift = it_shift;
> -    s->irq = irq;
>      s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s);
>      s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux */
> +}
>  
> -    s->conf.macaddr = nd->macaddr;
> -    s->conf.peers.ncs[0] = nd->netdev;
> +static Property dp8393x_properties[] = {
> +    DEFINE_NIC_PROPERTIES(dp8393xState, conf),
> +    DEFINE_PROP_PTR("dma_mr", dp8393xState, dma_mr),
> +    DEFINE_PROP_UINT8("it_shift", dp8393xState, it_shift, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
>  
> -    s->nic = qemu_new_nic(&net_dp83932_info, &s->conf, nd->model, nd->name, s);
> +static void dp8393x_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
> -    qemu_register_reset(dp8393x_reset, s);
> -    dp8393x_reset(s);
> +    set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
> +    dc->realize = dp8393x_realize;
> +    dc->reset = dp8393x_reset;
> +    dc->props = dp8393x_properties;
> +}
>  
> -    memory_region_init_io(&s->mmio, NULL, &dp8393x_ops, s,
> -                          "dp8393x", 0x40 << it_shift);
> -    memory_region_add_subregion(address_space, base, &s->mmio);
> +static const TypeInfo dp8393x_info = {
> +    .name          = TYPE_DP8393X,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(dp8393xState),
> +    .instance_init = dp8393x_instance_init,
> +    .class_init    = dp8393x_class_init,
> +};
> +
> +static void dp8393x_register_types(void)
> +{
> +    type_register_static(&dp8393x_info);
>  }
> +
> +type_init(dp8393x_register_types)
> diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
> index 31b4729..e0065ce 100644
> --- a/include/hw/mips/mips.h
> +++ b/include/hw/mips/mips.h
> @@ -20,9 +20,4 @@ void rc4030_dma_write(void *dma, uint8_t *buf, int len);
>  
>  DeviceState *rc4030_init(rc4030_dma **dmas, MemoryRegion **dma_mr);
>  
> -/* dp8393x.c */
> -void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
> -                  MemoryRegion *address_space,
> -                  qemu_irq irq, MemoryRegion *dma_mr);
> -
>  #endif

I am not very comfortable with the QOM API, but it looks fine to me.

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 12/17] net/dp8393x: add PROM to store MAC address
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 12/17] net/dp8393x: add PROM to store MAC address Hervé Poussineau
@ 2015-06-02 11:04   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:04 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel, Laurent Vivier

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/mips/mips_jazz.c |  1 +
>  hw/net/dp8393x.c    | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 648654e..9d60633 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -282,6 +282,7 @@ static void mips_jazz_init(MachineState *machine,
>              qdev_init_nofail(dev);
>              sysbus = SYS_BUS_DEVICE(dev);
>              sysbus_mmio_map(sysbus, 0, 0x80001000);
> +            sysbus_mmio_map(sysbus, 1, 0x8000b000);
>              sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(rc4030, 4));
>              break;
>          } else if (is_help_option(nd->model)) {
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 51e728b..ef1fb0e 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -25,6 +25,7 @@
>  
>  //#define DEBUG_SONIC
>  
> +#define SONIC_PROM_SIZE 0x1000
>  
>  #ifdef DEBUG_SONIC
>  #define DPRINTF(fmt, ...) \
> @@ -156,6 +157,7 @@ typedef struct dp8393xState {
>      NICConf conf;
>      NICState *nic;
>      MemoryRegion mmio;
> +    MemoryRegion prom;
>  
>      /* Registers */
>      uint8_t cam[16][6];
> @@ -816,12 +818,15 @@ static void dp8393x_instance_init(Object *obj)
>      dp8393xState *s = DP8393X(obj);
>  
>      sysbus_init_mmio(sbd, &s->mmio);
> +    sysbus_init_mmio(sbd, &s->prom);
>      sysbus_init_irq(sbd, &s->irq);
>  }
>  
>  static void dp8393x_realize(DeviceState *dev, Error **errp)
>  {
>      dp8393xState *s = DP8393X(dev);
> +    int i, checksum;
> +    uint8_t *prom;
>  
>      address_space_init(&s->as, s->dma_mr, "dp8393x");
>      memory_region_init_io(&s->mmio, OBJECT(dev), &dp8393x_ops, s,
> @@ -833,6 +838,19 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
>  
>      s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s);
>      s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux */
> +
> +    memory_region_init_rom_device(&s->prom, OBJECT(dev), NULL, NULL,
> +                                  "dp8393x-prom", SONIC_PROM_SIZE, NULL);
> +    prom = memory_region_get_ram_ptr(&s->prom);
> +    checksum = 0;
> +    for (i = 0; i < 6; i++) {
> +        prom[i] = s->conf.macaddr.a[i];
> +        checksum += prom[i];
> +        if (checksum > 0xff) {
> +            checksum = (checksum + 1) & 0xff;
> +        }
> +    }
> +    prom[7] = 0xff - checksum;
>  }
>  
>  static Property dp8393x_properties[] = {

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 13/17] net/dp8393x: add load/save support
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 13/17] net/dp8393x: add load/save support Hervé Poussineau
@ 2015-06-02 11:04   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:04 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/net/dp8393x.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index ef1fb0e..4184045 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -853,6 +853,17 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
>      prom[7] = 0xff - checksum;
>  }
>  
> +static const VMStateDescription vmstate_dp8393x = {
> +    .name = "dp8393x",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .fields = (VMStateField []) {
> +        VMSTATE_BUFFER_UNSAFE(cam, dp8393xState, 0, 16 * 6),
> +        VMSTATE_UINT16_ARRAY(regs, dp8393xState, 0x40),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static Property dp8393x_properties[] = {
>      DEFINE_NIC_PROPERTIES(dp8393xState, conf),
>      DEFINE_PROP_PTR("dma_mr", dp8393xState, dma_mr),
> @@ -867,6 +878,7 @@ static void dp8393x_class_init(ObjectClass *klass, void *data)
>      set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
>      dc->realize = dp8393x_realize;
>      dc->reset = dp8393x_reset;
> +    dc->vmsd = &vmstate_dp8393x;
>      dc->props = dp8393x_properties;
>  }

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 14/17] net/dp8393x: correctly reset in_use field
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 14/17] net/dp8393x: correctly reset in_use field Hervé Poussineau
@ 2015-06-02 11:04   ` Aurelien Jarno
  2015-06-02 18:05     ` Hervé Poussineau
  0 siblings, 1 reply; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:04 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Don't write more than the field width, which is always 16 bit.
> Fixes network in NetBSD 5.1/arc
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/net/dp8393x.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 4184045..b72b0b1 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -761,10 +761,10 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
>          /* EOL detected */
>          s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
>      } else {
> -        data[0 * width] = 0; /* in_use */
> +        uint16_t in_use = 0;
>          address_space_rw(&s->as,
>              ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 6 * width,
> -            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
> +            MEMTXATTRS_UNSPECIFIED, (uint8_t *)&in_use, sizeof(uint16_t), 1);

Why not initialising both data[0] and data[1] to 0 and a fixed size of 2
bytes instead of using a new variable?

>          s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
>          s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX;
>          s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff);

That said:
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 15/17] net/dp8393x: fix hardware reset
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 15/17] net/dp8393x: fix hardware reset Hervé Poussineau
@ 2015-06-02 11:05   ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:05 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Documentation is not clear of what happens when doing a hardware reset,
> but firmware expect all registers to be zero unless specified otherwise.
> 
> This fixes reboot on MIPS Magnum.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/net/dp8393x.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index b72b0b1..95a4d3d 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -786,6 +786,7 @@ static void dp8393x_reset(DeviceState *dev)
>      dp8393xState *s = DP8393X(dev);
>      timer_del(s->watchdog);
>  
> +    memset(s->regs, 0, sizeof(s->regs));
>      s->regs[SONIC_CR] = SONIC_CR_RST | SONIC_CR_STP | SONIC_CR_RXDIS;
>      s->regs[SONIC_DCR] &= ~(SONIC_DCR_EXBUS | SONIC_DCR_LBR);
>      s->regs[SONIC_RCR] &= ~(SONIC_RCR_LB0 | SONIC_RCR_LB1 | SONIC_RCR_BRD | SONIC_RCR_RNT);

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 16/17] net/dp8393x: repair can_receive() method
  2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 16/17] net/dp8393x: repair can_receive() method Hervé Poussineau
@ 2015-06-02 11:05   ` Aurelien Jarno
  2015-06-03 20:33     ` Hervé Poussineau
  0 siblings, 1 reply; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 11:05 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Datasheet clearly says that RXDIS flag prevents reception, but says
> nothing about a required presence of RXEN flag.
> 
> While at it, fix the style of the following if statement.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/net/dp8393x.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 95a4d3d..b81036a 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -622,10 +622,12 @@ static int dp8393x_can_receive(NetClientState *nc)
>  {
>      dp8393xState *s = qemu_get_nic_opaque(nc);
>  
> -    if (!(s->regs[SONIC_CR] & SONIC_CR_RXEN))
> +    if (s->regs[SONIC_CR] & SONIC_CR_RXDIS) {
>          return 0;
> -    if (s->regs[SONIC_ISR] & SONIC_ISR_RBE)
> +    }
> +    if (s->regs[SONIC_ISR] & SONIC_ISR_RBE) {
>          return 0;
> +    }
>      return 1;
>  }

I don't have the datasheet at hand, but what would be the point of such
a RXEN flag if it is ignored? Are you sure it's not because it's enabled
by default?

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/17] mips jazz: compile only in 64 bit little endian
  2015-06-02 11:02   ` Aurelien Jarno
@ 2015-06-02 18:04     ` Hervé Poussineau
  2015-06-02 19:08       ` Aurelien Jarno
  0 siblings, 1 reply; 38+ messages in thread
From: Hervé Poussineau @ 2015-06-02 18:04 UTC (permalink / raw)
  To: qemu-devel, Leon Alrae

Le 02/06/2015 13:02, Aurelien Jarno a écrit :
> On 2015-05-27 14:19, Hervé Poussineau wrote:
>> Remove now useless device models from other MIPS configurations
>>
>> We're now compiling 18 files less than before.
>>
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>>   default-configs/mips-softmmu.mak     | 5 -----
>>   default-configs/mips64-softmmu.mak   | 5 -----
>>   default-configs/mips64el-softmmu.mak | 1 +
>>   default-configs/mipsel-softmmu.mak   | 5 -----
>>   hw/mips/Makefile.objs                | 3 ++-
>>   hw/mips/mips_jazz.c                  | 5 -----
>>   tests/endianness-test.c              | 4 ----
>>   7 files changed, 3 insertions(+), 25 deletions(-)
>>
>> diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak
>> index cce2c81..f62a21a 100644
>> --- a/default-configs/mips-softmmu.mak
>> +++ b/default-configs/mips-softmmu.mak
>> @@ -21,14 +21,9 @@ CONFIG_PIIX4=y
>>   CONFIG_IDE_ISA=y
>>   CONFIG_IDE_PIIX=y
>>   CONFIG_NE2000_ISA=y
>> -CONFIG_RC4030=y
>> -CONFIG_DP8393X=y
>> -CONFIG_DS1225Y=y
>>   CONFIG_MIPSNET=y
>>   CONFIG_PFLASH_CFI01=y
>> -CONFIG_G364FB=y
>>   CONFIG_I8259=y
>> -CONFIG_JAZZ_LED=y
>>   CONFIG_MC146818RTC=y
>>   CONFIG_ISA_TESTDEV=y
>>   CONFIG_EMPTY_SLOT=y
>> diff --git a/default-configs/mips64-softmmu.mak b/default-configs/mips64-softmmu.mak
>> index 7a88a08..accedca 100644
>> --- a/default-configs/mips64-softmmu.mak
>> +++ b/default-configs/mips64-softmmu.mak
>> @@ -21,14 +21,9 @@ CONFIG_PIIX4=y
>>   CONFIG_IDE_ISA=y
>>   CONFIG_IDE_PIIX=y
>>   CONFIG_NE2000_ISA=y
>> -CONFIG_RC4030=y
>> -CONFIG_DP8393X=y
>> -CONFIG_DS1225Y=y
>>   CONFIG_MIPSNET=y
>>   CONFIG_PFLASH_CFI01=y
>> -CONFIG_G364FB=y
>>   CONFIG_I8259=y
>> -CONFIG_JAZZ_LED=y
>>   CONFIG_MC146818RTC=y
>>   CONFIG_ISA_TESTDEV=y
>>   CONFIG_EMPTY_SLOT=y
>
> TTBOMK, MIPS Magnum machines are dual endian, so why remove the 64-bit
> big endian version?
>
> On the other hand, I am all for removing the 32-bit versions.
>

Yes, MIPS Magnum are dual endian. However, they always start in little-endian mode, and firmware switches CPU to big-endian if required.
If you prefer that I keep the 64 bit big-endian variant, I can do it.

Regards,

Hervé

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 14/17] net/dp8393x: correctly reset in_use field
  2015-06-02 11:04   ` Aurelien Jarno
@ 2015-06-02 18:05     ` Hervé Poussineau
  0 siblings, 0 replies; 38+ messages in thread
From: Hervé Poussineau @ 2015-06-02 18:05 UTC (permalink / raw)
  To: qemu-devel, Leon Alrae

Le 02/06/2015 13:04, Aurelien Jarno a écrit :
> On 2015-05-27 14:19, Hervé Poussineau wrote:
>> Don't write more than the field width, which is always 16 bit.
>> Fixes network in NetBSD 5.1/arc
>>
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>>   hw/net/dp8393x.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
>> index 4184045..b72b0b1 100644
>> --- a/hw/net/dp8393x.c
>> +++ b/hw/net/dp8393x.c
>> @@ -761,10 +761,10 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
>>           /* EOL detected */
>>           s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
>>       } else {
>> -        data[0 * width] = 0; /* in_use */
>> +        uint16_t in_use = 0;
>>           address_space_rw(&s->as,
>>               ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 6 * width,
>> -            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1);
>> +            MEMTXATTRS_UNSPECIFIED, (uint8_t *)&in_use, sizeof(uint16_t), 1);
>
> Why not initialising both data[0] and data[1] to 0 and a fixed size of 2
> bytes instead of using a new variable?

I'll do that if a v3 is required.

>
>>           s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
>>           s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX;
>>           s->regs[SONIC_RSC] = (s->regs[SONIC_RSC] & 0xff00) | (((s->regs[SONIC_RSC] & 0x00ff) + 1) & 0x00ff);
>
> That said:
> Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
>

Hervé

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 01/17] mips jazz: compile only in 64 bit little endian
  2015-06-02 18:04     ` Hervé Poussineau
@ 2015-06-02 19:08       ` Aurelien Jarno
  0 siblings, 0 replies; 38+ messages in thread
From: Aurelien Jarno @ 2015-06-02 19:08 UTC (permalink / raw)
  To: Hervé Poussineau; +Cc: Leon Alrae, qemu-devel

On 2015-06-02 20:04, Hervé Poussineau wrote:
> Le 02/06/2015 13:02, Aurelien Jarno a écrit :
> >On 2015-05-27 14:19, Hervé Poussineau wrote:
> >>Remove now useless device models from other MIPS configurations
> >>
> >>We're now compiling 18 files less than before.
> >>
> >>Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> >>---
> >>  default-configs/mips-softmmu.mak     | 5 -----
> >>  default-configs/mips64-softmmu.mak   | 5 -----
> >>  default-configs/mips64el-softmmu.mak | 1 +
> >>  default-configs/mipsel-softmmu.mak   | 5 -----
> >>  hw/mips/Makefile.objs                | 3 ++-
> >>  hw/mips/mips_jazz.c                  | 5 -----
> >>  tests/endianness-test.c              | 4 ----
> >>  7 files changed, 3 insertions(+), 25 deletions(-)
> >>
> >>diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak
> >>index cce2c81..f62a21a 100644
> >>--- a/default-configs/mips-softmmu.mak
> >>+++ b/default-configs/mips-softmmu.mak
> >>@@ -21,14 +21,9 @@ CONFIG_PIIX4=y
> >>  CONFIG_IDE_ISA=y
> >>  CONFIG_IDE_PIIX=y
> >>  CONFIG_NE2000_ISA=y
> >>-CONFIG_RC4030=y
> >>-CONFIG_DP8393X=y
> >>-CONFIG_DS1225Y=y
> >>  CONFIG_MIPSNET=y
> >>  CONFIG_PFLASH_CFI01=y
> >>-CONFIG_G364FB=y
> >>  CONFIG_I8259=y
> >>-CONFIG_JAZZ_LED=y
> >>  CONFIG_MC146818RTC=y
> >>  CONFIG_ISA_TESTDEV=y
> >>  CONFIG_EMPTY_SLOT=y
> >>diff --git a/default-configs/mips64-softmmu.mak b/default-configs/mips64-softmmu.mak
> >>index 7a88a08..accedca 100644
> >>--- a/default-configs/mips64-softmmu.mak
> >>+++ b/default-configs/mips64-softmmu.mak
> >>@@ -21,14 +21,9 @@ CONFIG_PIIX4=y
> >>  CONFIG_IDE_ISA=y
> >>  CONFIG_IDE_PIIX=y
> >>  CONFIG_NE2000_ISA=y
> >>-CONFIG_RC4030=y
> >>-CONFIG_DP8393X=y
> >>-CONFIG_DS1225Y=y
> >>  CONFIG_MIPSNET=y
> >>  CONFIG_PFLASH_CFI01=y
> >>-CONFIG_G364FB=y
> >>  CONFIG_I8259=y
> >>-CONFIG_JAZZ_LED=y
> >>  CONFIG_MC146818RTC=y
> >>  CONFIG_ISA_TESTDEV=y
> >>  CONFIG_EMPTY_SLOT=y
> >
> >TTBOMK, MIPS Magnum machines are dual endian, so why remove the 64-bit
> >big endian version?
> >
> >On the other hand, I am all for removing the 32-bit versions.
> >
> 
> Yes, MIPS Magnum are dual endian. However, they always start in little-endian mode, and firmware switches CPU to big-endian if required.
> If you prefer that I keep the 64 bit big-endian variant, I can do it.

I guess QEMU doesn't support the endian switch, so we have to keep the
two versions so that people can use either a little or a big endian
guest.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [Qemu-devel] [PATCH v2 16/17] net/dp8393x: repair can_receive() method
  2015-06-02 11:05   ` Aurelien Jarno
@ 2015-06-03 20:33     ` Hervé Poussineau
  0 siblings, 0 replies; 38+ messages in thread
From: Hervé Poussineau @ 2015-06-03 20:33 UTC (permalink / raw)
  To: qemu-devel, Leon Alrae

Le 02/06/2015 13:05, Aurelien Jarno a écrit :
> On 2015-05-27 14:19, Hervé Poussineau wrote:
>> Datasheet clearly says that RXDIS flag prevents reception, but says
>> nothing about a required presence of RXEN flag.
>>
>> While at it, fix the style of the following if statement.
>>
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>>   hw/net/dp8393x.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
>> index 95a4d3d..b81036a 100644
>> --- a/hw/net/dp8393x.c
>> +++ b/hw/net/dp8393x.c
>> @@ -622,10 +622,12 @@ static int dp8393x_can_receive(NetClientState *nc)
>>   {
>>       dp8393xState *s = qemu_get_nic_opaque(nc);
>>
>> -    if (!(s->regs[SONIC_CR] & SONIC_CR_RXEN))
>> +    if (s->regs[SONIC_CR] & SONIC_CR_RXDIS) {
>>           return 0;
>> -    if (s->regs[SONIC_ISR] & SONIC_ISR_RBE)
>> +    }
>> +    if (s->regs[SONIC_ISR] & SONIC_ISR_RBE) {
>>           return 0;
>> +    }
>>       return 1;
>>   }
>
> I don't have the datasheet at hand, but what would be the point of such
> a RXEN flag if it is ignored? Are you sure it's not because it's enabled
> by default?
>

After double checking the datasheet:
- RXEN "enables the receive buffer management [...]. Setting this bit resets the RXDIS bit"
- "Setting [RXDIS] disables the receiver from buffering data to memory or the Receive FIFO. [...] The RXEN bit is reset when the receiver is disabled."

So RXEN and RXDIS are mutually exclusive, except in transition phases when a packet is currently received and you're setting RXEN/RXDIS.
As QEMU doesn't emulate the period of reception of a packet (packet is received at once), both flag checks are equivalent.

So, I'll withdraw this patch in v3.

Hervé

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2015-06-03 20:37 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-27 12:19 [Qemu-devel] [PATCH v2 00/17] net/dp8393x and dma/rc4030 improvements Hervé Poussineau
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 01/17] mips jazz: compile only in 64 bit little endian Hervé Poussineau
2015-06-02 11:02   ` Aurelien Jarno
2015-06-02 18:04     ` Hervé Poussineau
2015-06-02 19:08       ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 02/17] dma/rc4030: create custom DMA address space Hervé Poussineau
2015-06-02 11:02   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 03/17] dma/rc4030: use AddressSpace and address_space_rw in users Hervé Poussineau
2015-06-02 11:02   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 04/17] dma/rc4030: do not use old_mmio accesses Hervé Poussineau
2015-06-02 11:03   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 05/17] dma/rc4030: document register at offset 0x210 Hervé Poussineau
2015-06-02 11:03   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 06/17] dma/rc4030: use trace events instead of custom logging Hervé Poussineau
2015-06-02 11:03   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 07/17] dma/rc4030: convert to QOM Hervé Poussineau
2015-06-02 11:03   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 08/17] net/dp8393x: always calculate proper checksums Hervé Poussineau
2015-06-02 11:03   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 09/17] net/dp8393x: do not use old_mmio accesses Hervé Poussineau
2015-06-02 11:03   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 10/17] net/dp8393x: use dp8393x_ prefix for all functions Hervé Poussineau
2015-06-02 11:04   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 11/17] net/dp8393x: QOM'ify Hervé Poussineau
2015-06-02 11:04   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 12/17] net/dp8393x: add PROM to store MAC address Hervé Poussineau
2015-06-02 11:04   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 13/17] net/dp8393x: add load/save support Hervé Poussineau
2015-06-02 11:04   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 14/17] net/dp8393x: correctly reset in_use field Hervé Poussineau
2015-06-02 11:04   ` Aurelien Jarno
2015-06-02 18:05     ` Hervé Poussineau
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 15/17] net/dp8393x: fix hardware reset Hervé Poussineau
2015-06-02 11:05   ` Aurelien Jarno
2015-05-27 12:19 ` [Qemu-devel] [PATCH v2 16/17] net/dp8393x: repair can_receive() method Hervé Poussineau
2015-06-02 11:05   ` Aurelien Jarno
2015-06-03 20:33     ` Hervé Poussineau
2015-05-27 12:20 ` [Qemu-devel] [PATCH v2 17/17] [RFC] dma/rc4030: do multiple calls to address_space_rw when doing DMA transfers Hervé Poussineau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.