[Qemu-devel] [PATCH 0/3] dp8393x update

[Qemu-devel] [PATCH 0/3] dp8393x update

[Laurent Vivier]

This is a series of patches I wrote to use dp8393x (SONIC) with
Quadra 800 emulation. I think it is interesting to share them with the
mainline.

Qdev'ifying allows to remove the annoying warning:
"requested NIC (anonymous, model dp83932) was not created
 (not supported by this machine?)"

[PATCH 1/3] dp8393x: add registers offset
[PATCH 2/3] dp8393x: add PROM to store MAC address
[PATCH 3/3] qdev'ify dp8393x

[Qemu-devel] [PATCH 1/3] dp8393x: add registers offset

[Laurent Vivier]

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/mips/mips_jazz.c      | 3 ++-
 hw/net/dp8393x.c         | 8 ++++++--
 include/hw/mips/mips.h   | 6 ------
 include/hw/net/dp8393x.h | 6 ++++++
 4 files changed, 14 insertions(+), 9 deletions(-)
 create mode 100644 include/hw/net/dp8393x.h

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 3f33093..ecfaacb 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -44,6 +44,7 @@
 #include "exec/address-spaces.h"
 #include "sysemu/qtest.h"
 #include "qemu/error-report.h"
+#include "hw/net/dp8393x.h"
 
 enum jazz_model_e
 {
@@ -267,7 +268,7 @@ static void mips_jazz_init(MemoryRegion *address_space,
         if (!nd->model)
             nd->model = g_strdup("dp83932");
         if (strcmp(nd->model, "dp83932") == 0) {
-            dp83932_init(nd, 0x80001000, 2, get_system_memory(), rc4030[4],
+            dp83932_init(nd, 0x80001000, 2, 0, get_system_memory(), rc4030[4],
                          rc4030_opaque, rc4030_dma_memory_rw);
             break;
         } else if (is_help_option(nd->model)) {
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 7eab7ad..343191f 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -20,7 +20,7 @@
 #include "hw/hw.h"
 #include "qemu/timer.h"
 #include "net/net.h"
-#include "hw/mips/mips.h"
+#include "hw/net/dp8393x.h"
 
 //#define DEBUG_SONIC
 
@@ -148,6 +148,7 @@ do { printf("sonic ERROR: %s: " fmt, __func__ , ## __VA_ARGS__); } while (0)
 typedef struct dp8393xState {
     /* Hardware */
     int it_shift;
+    int regs_offset;
     qemu_irq irq;
 #ifdef DEBUG_SONIC
     int irq_level;
@@ -609,6 +610,7 @@ static uint32_t dp8393x_readw(void *opaque, hwaddr addr)
     dp8393xState *s = opaque;
     int reg;
 
+    addr -= s->regs_offset;
     if ((addr & ((1 << s->it_shift) - 1)) != 0) {
         return 0;
     }
@@ -636,6 +638,7 @@ static void dp8393x_writew(void *opaque, hwaddr addr, uint32_t val)
     dp8393xState *s = opaque;
     int reg;
 
+    addr -= s->regs_offset;
     if ((addr & ((1 << s->it_shift) - 1)) != 0) {
         return;
     }
@@ -877,7 +880,7 @@ static NetClientInfo net_dp83932_info = {
     .cleanup = nic_cleanup,
 };
 
-void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
+void dp83932_init(NICInfo *nd, hwaddr base, int it_shift, int regs_offset,
                   MemoryRegion *address_space,
                   qemu_irq irq, void* mem_opaque,
                   void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write))
@@ -892,6 +895,7 @@ void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
     s->mem_opaque = mem_opaque;
     s->memory_rw = memory_rw;
     s->it_shift = it_shift;
+    s->regs_offset = regs_offset;
     s->irq = irq;
     s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s);
     s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux */
diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
index 2a7a9c9..d57e318 100644
--- a/include/hw/mips/mips.h
+++ b/include/hw/mips/mips.h
@@ -23,10 +23,4 @@ void *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));
-
 #endif
diff --git a/include/hw/net/dp8393x.h b/include/hw/net/dp8393x.h
new file mode 100644
index 0000000..21aa0f7
--- /dev/null
+++ b/include/hw/net/dp8393x.h
@@ -0,0 +1,6 @@
+void dp83932_init(NICInfo *nd, hwaddr base, int it_shift, int regs_offset,
+                  MemoryRegion *address_space,
+                  qemu_irq irq, void *mem_opaque,
+                  void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf,
+                                    int len, int is_write));
+
-- 
1.9.1

[Qemu-devel] [PATCH 2/3] dp8393x: add PROM to store MAC address

[Laurent Vivier]

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/mips/mips_jazz.c      |  4 ++--
 hw/net/dp8393x.c         | 18 +++++++++++++++++-
 include/hw/net/dp8393x.h |  3 ++-
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index ecfaacb..4d556ab 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -268,8 +268,8 @@ static void mips_jazz_init(MemoryRegion *address_space,
         if (!nd->model)
             nd->model = g_strdup("dp83932");
         if (strcmp(nd->model, "dp83932") == 0) {
-            dp83932_init(nd, 0x80001000, 2, 0, get_system_memory(), rc4030[4],
-                         rc4030_opaque, rc4030_dma_memory_rw);
+            dp83932_init(nd, 0x80001000, 0, 2, 0, get_system_memory(),
+                         rc4030[4], rc4030_opaque, rc4030_dma_memory_rw);
             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 343191f..bd97851 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -24,6 +24,8 @@
 
 //#define DEBUG_SONIC
 
+#define SONIC_PROM_SIZE 8192
+
 /* Calculate CRCs properly on Rx packets */
 #define SONIC_CALCULATE_RXCRC
 
@@ -159,6 +161,7 @@ typedef struct dp8393xState {
     NICState *nic;
     MemoryRegion *address_space;
     MemoryRegion mmio;
+    MemoryRegion prom;
 
     /* Registers */
     uint8_t cam[16][6];
@@ -880,12 +883,15 @@ static NetClientInfo net_dp83932_info = {
     .cleanup = nic_cleanup,
 };
 
-void dp83932_init(NICInfo *nd, hwaddr base, int it_shift, int regs_offset,
+void dp83932_init(NICInfo *nd, hwaddr base, hwaddr prombase,
+                  int it_shift, int regs_offset,
                   MemoryRegion *address_space,
                   qemu_irq irq, void* mem_opaque,
                   void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write))
 {
     dp8393xState *s;
+    int i;
+    uint8_t *prom;
 
     qemu_check_nic_model(nd, "dp83932");
 
@@ -912,4 +918,14 @@ void dp83932_init(NICInfo *nd, hwaddr base, int it_shift, int regs_offset,
     memory_region_init_io(&s->mmio, NULL, &dp8393x_ops, s,
                           "dp8393x", 0x40 << it_shift);
     memory_region_add_subregion(address_space, base, &s->mmio);
+
+    if (prombase) {
+        memory_region_init_rom_device(&s->prom, NULL, NULL, NULL,
+                                      "dp8393x-prom", SONIC_PROM_SIZE, NULL);
+        prom = memory_region_get_ram_ptr(&s->prom);
+        for (i = 0; i < 6; i++) {
+            prom[i] = s->conf.macaddr.a[i];
+        }
+        memory_region_add_subregion(address_space, prombase, &s->prom);
+    }
 }
diff --git a/include/hw/net/dp8393x.h b/include/hw/net/dp8393x.h
index 21aa0f7..47eb187 100644
--- a/include/hw/net/dp8393x.h
+++ b/include/hw/net/dp8393x.h
@@ -1,4 +1,5 @@
-void dp83932_init(NICInfo *nd, hwaddr base, int it_shift, int regs_offset,
+void dp83932_init(NICInfo *nd, hwaddr base, hwaddr rombase,
+                  int it_shift, int regs_offset,
                   MemoryRegion *address_space,
                   qemu_irq irq, void *mem_opaque,
                   void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf,
-- 
1.9.1

[Qemu-devel] [PATCH 3/3] qdev'ify dp8393x

[Laurent Vivier]

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/mips/mips_jazz.c      |   2 +-
 hw/net/dp8393x.c         | 182 +++++++++++++++++++++++++++++++----------------
 include/hw/net/dp8393x.h |   1 -
 3 files changed, 121 insertions(+), 64 deletions(-)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 4d556ab..8eca5a2 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -268,7 +268,7 @@ static void mips_jazz_init(MemoryRegion *address_space,
         if (!nd->model)
             nd->model = g_strdup("dp83932");
         if (strcmp(nd->model, "dp83932") == 0) {
-            dp83932_init(nd, 0x80001000, 0, 2, 0, get_system_memory(),
+            dp83932_init(nd, 0x80001000, 0, 2, 0,
                          rc4030[4], rc4030_opaque, rc4030_dma_memory_rw);
             break;
         } else if (is_help_option(nd->model)) {
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index bd97851..b81e68c 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -17,9 +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 "net/net.h"
+#include "hw/devices.h"
+#include "qemu/timer.h"
 #include "hw/net/dp8393x.h"
 
 //#define DEBUG_SONIC
@@ -147,10 +148,15 @@ 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;
-    int regs_offset;
+    int32_t it_shift;
+    int32_t regs_offset;
     qemu_irq irq;
 #ifdef DEBUG_SONIC
     int irq_level;
@@ -172,10 +178,13 @@ 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;
+    void *memory_rw;
+    void *mem_opaque;
 } dp8393xState;
 
+#define MEMORY_RW(func) void (*func)(void *opaque, hwaddr addr, \
+                        uint8_t *buf, int len, int is_write) = s->memory_rw
+
 static void dp8393x_update_irq(dp8393xState *s)
 {
     int level = (s->regs[SONIC_IMR] & s->regs[SONIC_ISR]) ? 1 : 0;
@@ -199,13 +208,14 @@ static void do_load_cam(dp8393xState *s)
     uint16_t data[8];
     int width, size;
     uint16_t index = 0;
+    MEMORY_RW(memory_rw);
 
     width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
     size = sizeof(uint16_t) * 4 * width;
 
     while (s->regs[SONIC_CDC] & 0x1f) {
         /* Fill current entry */
-        s->memory_rw(s->mem_opaque,
+        memory_rw(s->mem_opaque,
             (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
             (uint8_t *)data, size, 0);
         s->cam[index][0] = data[1 * width] & 0xff;
@@ -224,7 +234,7 @@ static void do_load_cam(dp8393xState *s)
     }
 
     /* Read CAM enable */
-    s->memory_rw(s->mem_opaque,
+    memory_rw(s->mem_opaque,
         (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
         (uint8_t *)data, size, 0);
     s->regs[SONIC_CE] = data[0 * width];
@@ -240,11 +250,14 @@ static void do_read_rra(dp8393xState *s)
 {
     uint16_t data[8];
     int width, size;
+    MEMORY_RW(memory_rw);
 
     /* Read memory */
     width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
     size = sizeof(uint16_t) * 4 * width;
-    s->memory_rw(s->mem_opaque,
+    DPRINTF("READ from %08x\n",
+            (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_RRP]);
+    memory_rw(s->mem_opaque,
         (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_RRP],
         (uint8_t *)data, size, 0);
 
@@ -348,6 +361,7 @@ static void do_transmit_packets(dp8393xState *s)
     int width, size;
     int tx_len, len;
     uint16_t i;
+    MEMORY_RW(memory_rw);
 
     width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
 
@@ -357,7 +371,7 @@ 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,
+        memory_rw(s->mem_opaque,
             ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * width,
             (uint8_t *)data, size, 0);
         tx_len = 0;
@@ -383,7 +397,7 @@ 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,
+            memory_rw(s->mem_opaque,
                 (s->regs[SONIC_TSA1] << 16) | s->regs[SONIC_TSA0],
                 &s->tx_buffer[tx_len], len, 0);
             tx_len += len;
@@ -392,7 +406,7 @@ static void do_transmit_packets(dp8393xState *s)
             if (i != s->regs[SONIC_TFC]) {
                 /* Read next fragment details */
                 size = sizeof(uint16_t) * 3 * width;
-                s->memory_rw(s->mem_opaque,
+                memory_rw(s->mem_opaque,
                     ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * i) * width,
                     (uint8_t *)data, size, 0);
                 s->regs[SONIC_TSA0] = data[0 * width];
@@ -426,14 +440,14 @@ 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,
+        memory_rw(s->mem_opaque,
             (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA],
             (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,
+            memory_rw(s->mem_opaque,
                 ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) * width,
                 (uint8_t *)data, size, 0);
             s->regs[SONIC_CTDA] = data[0 * width] & ~0x1;
@@ -680,7 +694,7 @@ static const MemoryRegionOps dp8393x_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int nic_can_receive(NetClientState *nc)
+static int dp8393x_can_receive(NetClientState *nc)
 {
     dp8393xState *s = qemu_get_nic_opaque(nc);
 
@@ -729,7 +743,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];
@@ -737,6 +752,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
     uint32_t available, address;
     int width, rx_len = size;
     uint32_t checksum;
+    MEMORY_RW(memory_rw);
 
     width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
 
@@ -755,8 +771,9 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
     if (s->regs[SONIC_LLFA] & 0x1) {
         /* 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 = ((s->regs[SONIC_URDA] << 16) |
+                   s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width;
+        memory_rw(s->mem_opaque, address, (uint8_t *)data, size, 0);
         if (data[0 * width] & 0x1) {
             /* Still EOL ; stop reception */
             return -1;
@@ -779,9 +796,9 @@ 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);
+    memory_rw(s->mem_opaque, address, (uint8_t *)buf, rx_len, 1);
     address += rx_len;
-    s->memory_rw(s->mem_opaque, address, (uint8_t*)&checksum, 4, 1);
+    memory_rw(s->mem_opaque, address, (uint8_t *)&checksum, 4, 1);
     rx_len += 4;
     s->regs[SONIC_CRBA1] = address >> 16;
     s->regs[SONIC_CRBA0] = address & 0xffff;
@@ -809,11 +826,12 @@ 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);
+    memory_rw(s->mem_opaque, (s->regs[SONIC_URDA] << 16) |
+              s->regs[SONIC_CRDA], (uint8_t *)data, size, 1);
 
     /* Move to next descriptor */
     size = sizeof(uint16_t) * width;
-    s->memory_rw(s->mem_opaque,
+    memory_rw(s->mem_opaque,
         ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width,
         (uint8_t *)data, size, 0);
     s->regs[SONIC_LLFA] = data[0 * width];
@@ -822,7 +840,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
         s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
     } else {
         data[0 * width] = 0; /* in_use */
-        s->memory_rw(s->mem_opaque,
+        memory_rw(s->mem_opaque,
             ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 6 * width,
             (uint8_t *)data, size, 1);
         s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
@@ -841,9 +859,9 @@ 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(DeviceState *d)
 {
-    dp8393xState *s = opaque;
+    dp8393xState *s = DP8393X(d);
     timer_del(s->watchdog);
 
     s->regs[SONIC_CR] = SONIC_CR_RST | SONIC_CR_STP | SONIC_CR_RXDIS;
@@ -865,7 +883,7 @@ static void nic_reset(void *opaque)
     dp8393x_update_irq(s);
 }
 
-static void nic_cleanup(NetClientState *nc)
+static void dp8393x_cleanup(NetClientState *nc)
 {
     dp8393xState *s = qemu_get_nic_opaque(nc);
 
@@ -878,54 +896,94 @@ static void nic_cleanup(NetClientState *nc)
 static NetClientInfo net_dp83932_info = {
     .type = NET_CLIENT_OPTIONS_KIND_NIC,
     .size = sizeof(NICState),
-    .can_receive = nic_can_receive,
-    .receive = nic_receive,
-    .cleanup = nic_cleanup,
+    .can_receive = dp8393x_can_receive,
+    .receive = dp8393x_receive,
+    .cleanup = dp8393x_cleanup,
 };
 
-void dp83932_init(NICInfo *nd, hwaddr base, hwaddr prombase,
-                  int it_shift, int regs_offset,
-                  MemoryRegion *address_space,
-                  qemu_irq irq, void* mem_opaque,
-                  void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf, int len, int is_write))
+static int dp8393x_init1(SysBusDevice *sbd)
 {
-    dp8393xState *s;
+    DeviceState *dev = DEVICE(sbd);
+    dp8393xState *s = DP8393X(dev);
     int i;
     uint8_t *prom;
 
-    qemu_check_nic_model(nd, "dp83932");
+    memory_region_init_io(&s->mmio, NULL, &dp8393x_ops, s,
+                          "dp8393x", 0x40 << s->it_shift);
+    sysbus_init_mmio(sbd, &s->mmio);
+    sysbus_init_irq(sbd, &s->irq);
 
-    s = g_malloc0(sizeof(dp8393xState));
+    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;
-    s->mem_opaque = mem_opaque;
-    s->memory_rw = memory_rw;
-    s->it_shift = it_shift;
-    s->regs_offset = regs_offset;
-    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;
+    memory_region_init_rom_device(&s->prom, NULL, NULL, NULL,
+                                  "dp8393x-prom", SONIC_PROM_SIZE, NULL);
+    prom = memory_region_get_ram_ptr(&s->prom);
+    for (i = 0; i < 6; i++) {
+        prom[i] = s->conf.macaddr.a[i];
+    }
+    sysbus_init_mmio(sbd, &s->prom);
 
-    s->nic = qemu_new_nic(&net_dp83932_info, &s->conf, nd->model, nd->name, s);
+    return 0;
+}
 
-    qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
-    qemu_register_reset(nic_reset, s);
-    nic_reset(s);
+static Property dp8393x_properties[] = {
+    DEFINE_NIC_PROPERTIES(dp8393xState, conf),
+    DEFINE_PROP_PTR("memory_rw", dp8393xState, memory_rw),
+    DEFINE_PROP_PTR("mem_opaque", dp8393xState, mem_opaque),
+    DEFINE_PROP_INT32("it_shift", dp8393xState, it_shift, 0),
+    DEFINE_PROP_INT32("regs_offset", dp8393xState, regs_offset, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
 
-    memory_region_init_io(&s->mmio, NULL, &dp8393x_ops, s,
-                          "dp8393x", 0x40 << it_shift);
-    memory_region_add_subregion(address_space, base, &s->mmio);
-
-    if (prombase) {
-        memory_region_init_rom_device(&s->prom, NULL, NULL, NULL,
-                                      "dp8393x-prom", SONIC_PROM_SIZE, NULL);
-        prom = memory_region_get_ram_ptr(&s->prom);
-        for (i = 0; i < 6; i++) {
-            prom[i] = s->conf.macaddr.a[i];
-        }
-        memory_region_add_subregion(address_space, prombase, &s->prom);
-    }
+static void dp8393x_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = dp8393x_init1;
+    dc->reset = dp8393x_reset;
+    dc->props = dp8393x_properties;
 }
+
+static const TypeInfo dp8393x_info = {
+    .name   = TYPE_DP8393X,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(dp8393xState),
+    .class_init    = dp8393x_class_init,
+};
+
+static void dp8393x_register_types(void)
+{
+    type_register_static(&dp8393x_info);
+}
+
+void dp83932_init(NICInfo *nd, hwaddr base, hwaddr prombase,
+                  int it_shift, int regs_offset,
+                  qemu_irq irq, void *mem_opaque,
+                  void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf,
+                  int len, int is_write))
+{
+    DeviceState *dev;
+    SysBusDevice *s;
+
+    qemu_check_nic_model(nd, "dp83932");
+
+    dev = qdev_create(NULL, TYPE_DP8393X);
+    qdev_set_nic_properties(dev, nd);
+    qdev_prop_set_int32(dev, "it_shift", it_shift);
+    qdev_prop_set_int32(dev, "regs_offset", regs_offset);
+    qdev_prop_set_ptr(dev, "mem_opaque", mem_opaque);
+    qdev_prop_set_ptr(dev, "memory_rw", memory_rw);
+    qdev_init_nofail(dev);
+    s = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(s, 0, base);
+    sysbus_mmio_map(s, 1, prombase);
+    sysbus_connect_irq(s, 0, irq);
+}
+
+type_init(dp8393x_register_types)
diff --git a/include/hw/net/dp8393x.h b/include/hw/net/dp8393x.h
index 47eb187..178ff5c 100644
--- a/include/hw/net/dp8393x.h
+++ b/include/hw/net/dp8393x.h
@@ -1,6 +1,5 @@
 void dp83932_init(NICInfo *nd, hwaddr base, hwaddr rombase,
                   int it_shift, int regs_offset,
-                  MemoryRegion *address_space,
                   qemu_irq irq, void *mem_opaque,
                   void (*memory_rw)(void *opaque, hwaddr addr, uint8_t *buf,
                                     int len, int is_write));
-- 
1.9.1

Re: [Qemu-devel] [PATCH 3/3] qdev'ify dp8393x

[Andreas Färber]

Hi Laurent,

Am 29.12.2014 um 01:39 schrieb Laurent Vivier:
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  hw/mips/mips_jazz.c      |   2 +-
>  hw/net/dp8393x.c         | 182 +++++++++++++++++++++++++++++++----------------
>  include/hw/net/dp8393x.h |   1 -
>  3 files changed, 121 insertions(+), 64 deletions(-)

Thanks for doing this cleanup! "QOM'ify" would be more correct
(subject). Patch looks good except for one nit: Can you distinguish
between instance_init and realize function rather than using the old
SysBusDeviceClass::init?

Also, can dp83932_init() be inlined in the call site, or is it used in
multiple places?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 3/3] qdev'ify dp8393x

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]

Hi Andreas,

Thank you for your comments.

You're right: I will change the subject and distinguish the
instance_init and realize.

dp83932_init() is used in a single place, so as you propose I will
inline it and remove the function definition.

Regards,
Laurent

Le 01/01/2015 17:15, Andreas Färber a écrit :
> Hi Laurent,
> 
> Am 29.12.2014 um 01:39 schrieb Laurent Vivier:
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>  hw/mips/mips_jazz.c      |   2 +-
>>  hw/net/dp8393x.c         | 182 +++++++++++++++++++++++++++++++----------------
>>  include/hw/net/dp8393x.h |   1 -
>>  3 files changed, 121 insertions(+), 64 deletions(-)
> 
> Thanks for doing this cleanup! "QOM'ify" would be more correct
> (subject). Patch looks good except for one nit: Can you distinguish
> between instance_init and realize function rather than using the old
> SysBusDeviceClass::init?
> 
> Also, can dp83932_init() be inlined in the call site, or is it used in
> multiple places?
> 
> Regards,
> Andreas
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PATCH 0/3] dp8393x update

[Hervé Poussineau]

Hi Laurent,

Le 29/12/2014 01:39, Laurent Vivier a écrit :
> This is a series of patches I wrote to use dp8393x (SONIC) with
> Quadra 800 emulation. I think it is interesting to share them with the
> mainline.
>
> Qdev'ifying allows to remove the annoying warning:
> "requested NIC (anonymous, model dp83932) was not created
>   (not supported by this machine?)"
>
> [PATCH 1/3] dp8393x: add registers offset
> [PATCH 2/3] dp8393x: add PROM to store MAC address
> [PATCH 3/3] qdev'ify dp8393x
>

I also had some patches to QOM'ify dp8393x.
Those are available at http://repo.or.cz/w/qemu/hpoussin.git/shortlog/refs/heads/sonic

Main differences are:
- dp8393x uses an AddressSpace, instead of an offset in a MemoryRegion in yours
- no PROM support, but should be easy to add
- rc4030 (MIPS Jazz chipset) also converted to QOM (but that was not the goal of your patch series)

Minor points are:
- have load/save support
- all functions have the same dp8393x_ prefix
- old_mmio-style functions are not used anymore

What do you think of them?

Regards,

Hervé

Re: [Qemu-devel] [PATCH 0/3] dp8393x update

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 1957 bytes --]

Hi Hervé,

Le 01/01/2015 22:01, Hervé Poussineau a écrit :
> Hi Laurent,
> 
> Le 29/12/2014 01:39, Laurent Vivier a écrit :
>> This is a series of patches I wrote to use dp8393x (SONIC) with
>> Quadra 800 emulation. I think it is interesting to share them with the
>> mainline.
>>
>> Qdev'ifying allows to remove the annoying warning:
>> "requested NIC (anonymous, model dp83932) was not created
>>   (not supported by this machine?)"
>>
>> [PATCH 1/3] dp8393x: add registers offset
>> [PATCH 2/3] dp8393x: add PROM to store MAC address
>> [PATCH 3/3] qdev'ify dp8393x
>>
> 
> I also had some patches to QOM'ify dp8393x.
> Those are available at
> http://repo.or.cz/w/qemu/hpoussin.git/shortlog/refs/heads/sonic
> 
> Main differences are:
> - dp8393x uses an AddressSpace, instead of an offset in a MemoryRegion
> in yours
> - no PROM support, but should be easy to add
> - rc4030 (MIPS Jazz chipset) also converted to QOM (but that was not the
> goal of your patch series)
> 
> Minor points are:
> - have load/save support
> - all functions have the same dp8393x_ prefix
> - old_mmio-style functions are not used anymore
> 
> What do you think of them?

I don't know if it's a good idea to use AddressSpace into device. For
me, AddressSpace must stay in the machine definition. SysBus is there
for that. But it seems to be a good way to do DMA. I have to think about
that...

As I use the dp8393x with a quadra 800 implementation, I don't want to
work on rc4030 part, and I need the PROM and the register offset (but as
you said it is easy to add).

Except for this particular patch I like all the other dp8393x patches.

What I propose is to steal these patches (except the QOM one) and
include them in my series with the modifications done according comments
from Andreas (and perhaps something about the address space, i.e your
patch...). Your comments will be welcome.

Regards,
Laurent






[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PATCH 0/3] dp8393x update

[Laurent Vivier]

[-- Attachment #1.1: Type: text/plain, Size: 1846 bytes --]

Le 02/01/2015 02:34, Laurent Vivier a écrit :
> Hi Hervé,
> 
> Le 01/01/2015 22:01, Hervé Poussineau a écrit :
>> Hi Laurent,
>>
>> Le 29/12/2014 01:39, Laurent Vivier a écrit :
>>> This is a series of patches I wrote to use dp8393x (SONIC) with
>>> Quadra 800 emulation. I think it is interesting to share them with the
>>> mainline.
>>>
>>> Qdev'ifying allows to remove the annoying warning:
>>> "requested NIC (anonymous, model dp83932) was not created
>>>   (not supported by this machine?)"
>>>
>>> [PATCH 1/3] dp8393x: add registers offset
>>> [PATCH 2/3] dp8393x: add PROM to store MAC address
>>> [PATCH 3/3] qdev'ify dp8393x
>>>
>>
>> I also had some patches to QOM'ify dp8393x.
>> Those are available at
>> http://repo.or.cz/w/qemu/hpoussin.git/shortlog/refs/heads/sonic
>>
>> Main differences are:
>> - dp8393x uses an AddressSpace, instead of an offset in a MemoryRegion
>> in yours
>> - no PROM support, but should be easy to add
>> - rc4030 (MIPS Jazz chipset) also converted to QOM (but that was not the
>> goal of your patch series)
>>
>> Minor points are:
>> - have load/save support
>> - all functions have the same dp8393x_ prefix
>> - old_mmio-style functions are not used anymore
>>
>> What do you think of them?
> 
> I don't know if it's a good idea to use AddressSpace into device. For
> me, AddressSpace must stay in the machine definition. SysBus is there
> for that. But it seems to be a good way to do DMA. I have to think about
> that...

Using AddressSpace is really a very good idea, in fact, but I don't like
the way you pass it to the device (a qdev_set_prop()).

I think we should do as it is done in PCI. This must be managed at
sysbus level, not at the device level.

Find attached a (not tested) patch to show what I'm thinking about.

Regards,
Laurent



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: sysbus_dma_rw.patch --]
[-- Type: text/x-patch; name="sysbus_dma_rw.patch", Size: 9358 bytes --]

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 69960ac..8902a4f 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -148,7 +148,6 @@ typedef struct dp8393xState {
 
     /* Hardware */
     uint8_t it_shift;
-    void *as_opaque;
     qemu_irq irq;
 #ifdef DEBUG_SONIC
     int irq_level;
@@ -200,7 +199,7 @@ static void dp8393x_do_load_cam(dp8393xState *s)
 
     while (s->regs[SONIC_CDC] & 0x1f) {
         /* Fill current entry */
-        address_space_rw(s->as,
+        sysbus_dma_rw(SYS_BUS_DEVICE(s),
             (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
             (uint8_t *)data, size, 0);
         s->cam[index][0] = data[1 * width] & 0xff;
@@ -219,7 +218,7 @@ static void dp8393x_do_load_cam(dp8393xState *s)
     }
 
     /* Read CAM enable */
-    address_space_rw(s->as,
+    sysbus_dma_rw(SYS_BUS_DEVICE(s),
         (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
         (uint8_t *)data, size, 0);
     s->regs[SONIC_CE] = data[0 * width];
@@ -239,7 +238,7 @@ static void dp8393x_do_read_rra(dp8393xState *s)
     /* Read memory */
     width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
     size = sizeof(uint16_t) * 4 * width;
-    address_space_rw(s->as,
+    sysbus_dma_rw(SYS_BUS_DEVICE(s),
         (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_RRP],
         (uint8_t *)data, size, 0);
 
@@ -352,7 +351,7 @@ static void dp8393x_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];
-        address_space_rw(s->as,
+        sysbus_dma_rw(SYS_BUS_DEVICE(s),
             ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * width,
             (uint8_t *)data, size, 0);
         tx_len = 0;
@@ -378,7 +377,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
             if (tx_len + len > sizeof(s->tx_buffer)) {
                 len = sizeof(s->tx_buffer) - tx_len;
             }
-            address_space_rw(s->as,
+            sysbus_dma_rw(SYS_BUS_DEVICE(s),
                 (s->regs[SONIC_TSA1] << 16) | s->regs[SONIC_TSA0],
                 &s->tx_buffer[tx_len], len, 0);
             tx_len += len;
@@ -387,7 +386,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
             if (i != s->regs[SONIC_TFC]) {
                 /* Read next fragment details */
                 size = sizeof(uint16_t) * 3 * width;
-                address_space_rw(s->as,
+                sysbus_dma_rw(SYS_BUS_DEVICE(s),
                     ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * i) * width,
                     (uint8_t *)data, size, 0);
                 s->regs[SONIC_TSA0] = data[0 * width];
@@ -421,14 +420,14 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
         /* Write status */
         data[0 * width] = s->regs[SONIC_TCR] & 0x0fff; /* status */
         size = sizeof(uint16_t) * width;
-        address_space_rw(s->as,
+        sysbus_dma_rw(SYS_BUS_DEVICE(s),
             (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA],
             (uint8_t *)data, size, 1);
 
         if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) {
             /* Read footer of packet */
             size = sizeof(uint16_t) * width;
-            address_space_rw(s->as,
+            sysbus_dma_rw(SYS_BUS_DEVICE(s),
                 ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) * width,
                 (uint8_t *)data, size, 0);
             s->regs[SONIC_CTDA] = data[0 * width] & ~0x1;
@@ -695,7 +694,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
         /* 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;
-        address_space_rw(s->as, address, (uint8_t*)data, size, 0);
+        sysbus_dma_rw(SYS_BUS_DEVICE(s), address, (uint8_t*)data, size, 0);
         if (data[0 * width] & 0x1) {
             /* Still EOL ; stop reception */
             return -1;
@@ -714,9 +713,9 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     /* 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];
-    address_space_rw(s->as, address, (uint8_t*)buf, rx_len, 1);
+    sysbus_dma_rw(SYS_BUS_DEVICE(s), address, (uint8_t*)buf, rx_len, 1);
     address += rx_len;
-    address_space_rw(s->as, address, (uint8_t*)&checksum, 4, 1);
+    sysbus_dma_rw(SYS_BUS_DEVICE(s), address, (uint8_t*)&checksum, 4, 1);
     rx_len += 4;
     s->regs[SONIC_CRBA1] = address >> 16;
     s->regs[SONIC_CRBA0] = address & 0xffff;
@@ -744,11 +743,12 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     data[3 * width] = s->regs[SONIC_TRBA1]; /* pkt_ptr1 */
     data[4 * width] = s->regs[SONIC_RSC]; /* seq_no */
     size = sizeof(uint16_t) * 5 * width;
-    address_space_rw(s->as, (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA], (uint8_t *)data, size, 1);
+    sysbus_dma_rw(SYS_BUS_DEVICE(s),
+                  (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA], (uint8_t *)data, size, 1);
 
     /* Move to next descriptor */
     size = sizeof(uint16_t) * width;
-    address_space_rw(s->as,
+    sysbus_dma_rw(SYS_BUS_DEVICE(s),
         ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width,
         (uint8_t *)data, size, 0);
     s->regs[SONIC_LLFA] = data[0 * width];
@@ -757,7 +757,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
         s->regs[SONIC_ISR] |= SONIC_ISR_RDE;
     } else {
         data[0 * width] = 0; /* in_use */
-        address_space_rw(s->as,
+        sysbus_dma_rw(SYS_BUS_DEVICE(s),
             ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 6 * width,
             (uint8_t *)data, size, 1);
         s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA];
@@ -828,12 +828,12 @@ void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,
 
     dev = qdev_create(NULL, "dp83932");
     qdev_prop_set_uint8(dev, "it-shift", it_shift);
-    qdev_prop_set_ptr(dev, "address-space", as);
     qdev_set_nic_properties(dev, nd);
     qdev_init_nofail(dev);
     sysbus = SYS_BUS_DEVICE(dev);
     sysbus_mmio_map(sysbus, 0, base);
     sysbus_connect_irq(sysbus, 0, irq);
+    sysbus_set_address_space(sysbus, as);
 }
 
 static int dp8393x_initfn(SysBusDevice *sysbus)
@@ -841,7 +841,6 @@ static int dp8393x_initfn(SysBusDevice *sysbus)
     DeviceState *dev = DEVICE(sysbus);
     dp8393xState *s = DP8393X(sysbus);
 
-    s->as = s->as_opaque; /* cast address space to right type */
     s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s);
     s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux */
 
@@ -871,7 +870,6 @@ static const VMStateDescription vmstate_dp8393x = {
 
 static Property dp8393x_properties[] = {
     DEFINE_PROP_UINT8("it-shift", dp8393xState, it_shift, 2),
-    DEFINE_PROP_PTR("address-space", dp8393xState, as_opaque),
     DEFINE_NIC_PROPERTIES(dp8393xState, conf),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index d1f3f00..d1e027b 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -3,6 +3,7 @@
 
 /* Devices attached directly to the main system bus.  */
 
+#include "sysemu/dma.h"
 #include "hw/qdev.h"
 #include "exec/memory.h"
 
@@ -53,6 +54,7 @@ struct SysBusDevice {
         hwaddr addr;
         MemoryRegion *memory;
     } mmio[QDEV_MAX_MMIO];
+    AddressSpace *sysbus_as;
     int num_pio;
     pio_addr_t pio[QDEV_MAX_PIO];
 };
@@ -78,6 +80,37 @@ void sysbus_add_io(SysBusDevice *dev, hwaddr addr,
                    MemoryRegion *mem);
 MemoryRegion *sysbus_address_space(SysBusDevice *dev);
 
+static inline AddressSpace *sysbus_get_address_space(SysBusDevice *dev)
+{
+    return dev->sysbus_as;
+}
+
+static inline void sysbus_set_address_space(SysBusDevice *dev,
+                                            AddressSpace *as)
+{
+     dev->sysbus_as = as;
+}
+
+static inline int sysbus_dma_rw(SysBusDevice *dev, dma_addr_t addr,
+                                void *buf, dma_addr_t len, DMADirection dir)
+{
+    dma_memory_rw(sysbus_get_address_space(dev), addr, buf, len, dir);
+    return 0;
+}
+
+static inline int sysbus_dma_read(SysBusDevice *dev, dma_addr_t addr,
+                               void *buf, dma_addr_t len)
+{
+    return sysbus_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE);
+}
+
+static inline int sysbus_dma_write(SysBusDevice *dev, dma_addr_t addr,
+                                const void *buf, dma_addr_t len)
+{
+    return sysbus_dma_rw(dev, addr, (void *) buf, len,
+                         DMA_DIRECTION_FROM_DEVICE);
+}
+
 /* Call func for every dynamically created sysbus device in the system */
 void foreach_dynamic_sysbus_device(FindSysbusDeviceFunc *func, void *opaque);
 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PATCH 0/3] dp8393x update

[Peter Maydell]

On 2 January 2015 at 09:25, Laurent Vivier <Laurent@vivier.eu> wrote:
> Using AddressSpace is really a very good idea, in fact, but I don't like
> the way you pass it to the device (a qdev_set_prop()).
>
> I think we should do as it is done in PCI. This must be managed at
> sysbus level, not at the device level.

Actually I think using properties is the direction we're
headed for this currently. (Consider the case of a sysbus
device which has two DMA master ports, like an ARM trustzone
aware device.) However I have a feeling the plan was to use
MemoryRegion properties rather than AddressSpaces.

Can anybody remember if we have an example of this in-tree?

-- PMM

Re: [Qemu-devel] [PATCH 0/3] dp8393x update

[Laurent Vivier]

[-- Attachment #1: Type: text/plain, Size: 871 bytes --]

Le 02/01/2015 11:19, Peter Maydell a écrit :
> On 2 January 2015 at 09:25, Laurent Vivier <Laurent@vivier.eu> wrote:
>> Using AddressSpace is really a very good idea, in fact, but I don't like
>> the way you pass it to the device (a qdev_set_prop()).
>>
>> I think we should do as it is done in PCI. This must be managed at
>> sysbus level, not at the device level.
> 
> Actually I think using properties is the direction we're
> headed for this currently. (Consider the case of a sysbus
> device which has two DMA master ports, like an ARM trustzone

Does it means that these two DMA master ports can access different
address spaces ?

> aware device.) However I have a feeling the plan was to use
> MemoryRegion properties rather than AddressSpaces.

Is it possible to use something like dma_memory_rw() with MemoryRegion ?

Regards,
Laurent



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PATCH 0/3] dp8393x update

[Peter Maydell]

On 2 January 2015 at 11:33, Laurent Vivier <Laurent@vivier.eu> wrote:
> Le 02/01/2015 11:19, Peter Maydell a écrit :
>> On 2 January 2015 at 09:25, Laurent Vivier <Laurent@vivier.eu> wrote:
>>> Using AddressSpace is really a very good idea, in fact, but I don't like
>>> the way you pass it to the device (a qdev_set_prop()).
>>>
>>> I think we should do as it is done in PCI. This must be managed at
>>> sysbus level, not at the device level.
>>
>> Actually I think using properties is the direction we're
>> headed for this currently. (Consider the case of a sysbus
>> device which has two DMA master ports, like an ARM trustzone
>
> Does it means that these two DMA master ports can access different
> address spaces ?

Yes. (At least, that's how we're planning to model TrustZone:
the secure and non-secure worlds will be different AddressSpaces.)

>> aware device.) However I have a feeling the plan was to use
>> MemoryRegion properties rather than AddressSpaces.
>
> Is it possible to use something like dma_memory_rw() with MemoryRegion ?

The point is that it's always possible to create an AddressSpace
corresponding to a MemoryRegion (which you do if you're a bus
master going to DMA into it). But if you're an SoC model and
you want to add a few of your own devices and pass the whole
thing on to a CPU or other bus master, then you have to do that
using MemoryRegions. So we should use MemoryRegion properties
everywhere, as they're more flexible, and only create the
AddressSpaces where needed locally inside the bus-master
devices.

-- PMM