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
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);
signature.asc
Description: OpenPGP digital signature
