From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42810) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y6yUC-0001JD-68 for qemu-devel@nongnu.org; Fri, 02 Jan 2015 04:25:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y6yU8-00013r-TW for qemu-devel@nongnu.org; Fri, 02 Jan 2015 04:25:36 -0500 Received: from mout.kundenserver.de ([212.227.17.24]:62167) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y6yU8-00013k-HP for qemu-devel@nongnu.org; Fri, 02 Jan 2015 04:25:32 -0500 Message-ID: <54A66408.3040406@Vivier.EU> Date: Fri, 02 Jan 2015 10:25:28 +0100 From: Laurent Vivier MIME-Version: 1.0 References: <1419813550-26182-1-git-send-email-laurent@vivier.eu> <54A5B5A0.7000600@reactos.org> <54A5F5C0.4030402@Vivier.EU> In-Reply-To: <54A5F5C0.4030402@Vivier.EU> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="GdXQTm3UnjKtqvmLtUo2uxg0sJFIe877x" Subject: Re: [Qemu-devel] [PATCH 0/3] dp8393x update List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?windows-1252?Q?Herv=E9_Poussineau?= , qemu-devel@nongnu.org Cc: Leon Alrae , Aurelien Jarno This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --GdXQTm3UnjKtqvmLtUo2uxg0sJFIe877x Content-Type: multipart/mixed; boundary="------------050205010404030406000304" This is a multi-part message in MIME format. --------------050205010404030406000304 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Le 02/01/2015 02:34, Laurent Vivier a =E9crit : > Hi Herv=E9, >=20 > Le 01/01/2015 22:01, Herv=E9 Poussineau a =E9crit : >> Hi Laurent, >> >> Le 29/12/2014 01:39, Laurent Vivier a =E9crit : >>> 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 th= e >>> 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 t= he >> 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? >=20 > 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 abou= t > 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 --------------050205010404030406000304 Content-Type: text/x-patch; name="sysbus_dma_rw.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="sysbus_dma_rw.patch" 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 { =20 /* 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) =20 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] =3D data[1 * width] & 0xff; @@ -219,7 +218,7 @@ static void dp8393x_do_load_cam(dp8393xState *s) } =20 /* 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] =3D data[0 * width]; @@ -239,7 +238,7 @@ static void dp8393x_do_read_rra(dp8393xState *s) /* Read memory */ width =3D (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; size =3D 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); =20 @@ -352,7 +351,7 @@ static void dp8393x_do_transmit_packets(dp8393xState = *s) (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_CTDA]); size =3D sizeof(uint16_t) * 6 * width; s->regs[SONIC_TTDA] =3D 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 =3D 0; @@ -378,7 +377,7 @@ static void dp8393x_do_transmit_packets(dp8393xState = *s) if (tx_len + len > sizeof(s->tx_buffer)) { len =3D 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 +=3D len; @@ -387,7 +386,7 @@ static void dp8393x_do_transmit_packets(dp8393xState = *s) if (i !=3D s->regs[SONIC_TFC]) { /* Read next fragment details */ size =3D 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] =3D data[0 * width]; @@ -421,14 +420,14 @@ static void dp8393x_do_transmit_packets(dp8393xStat= e *s) /* Write status */ data[0 * width] =3D s->regs[SONIC_TCR] & 0x0fff; /* status */ size =3D 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); =20 if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { /* Read footer of packet */ size =3D 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]) + si= zeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) * width, (uint8_t *)data, size, 0); s->regs[SONIC_CTDA] =3D data[0 * width] & ~0x1; @@ -695,7 +694,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, co= nst uint8_t * buf, /* Are we still in resource exhaustion? */ size =3D sizeof(uint16_t) * 1 * width; address =3D ((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, co= nst uint8_t * buf, /* Put packet into RBA */ DPRINTF("Receive packet at %08x\n", (s->regs[SONIC_CRBA1] << 16) | s= ->regs[SONIC_CRBA0]); address =3D (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 +=3D 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 +=3D 4; s->regs[SONIC_CRBA1] =3D address >> 16; s->regs[SONIC_CRBA0] =3D address & 0xffff; @@ -744,11 +743,12 @@ static ssize_t dp8393x_receive(NetClientState *nc, = const uint8_t * buf, data[3 * width] =3D s->regs[SONIC_TRBA1]; /* pkt_ptr1 */ data[4 * width] =3D s->regs[SONIC_RSC]; /* seq_no */ size =3D 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], (ui= nt8_t *)data, size, 1); =20 /* Move to next descriptor */ size =3D 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(uin= t16_t) * 5 * width, (uint8_t *)data, size, 0); s->regs[SONIC_LLFA] =3D data[0 * width]; @@ -757,7 +757,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, co= nst uint8_t * buf, s->regs[SONIC_ISR] |=3D SONIC_ISR_RDE; } else { data[0 * width] =3D 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] =3D s->regs[SONIC_LLFA]; @@ -828,12 +828,12 @@ void dp83932_init(NICInfo *nd, hwaddr base, int it_= shift, =20 dev =3D 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 =3D SYS_BUS_DEVICE(dev); sysbus_mmio_map(sysbus, 0, base); sysbus_connect_irq(sysbus, 0, irq); + sysbus_set_address_space(sysbus, as); } =20 static int dp8393x_initfn(SysBusDevice *sysbus) @@ -841,7 +841,6 @@ static int dp8393x_initfn(SysBusDevice *sysbus) DeviceState *dev =3D DEVICE(sysbus); dp8393xState *s =3D DP8393X(sysbus); =20 - s->as =3D s->as_opaque; /* cast address space to right type */ s->watchdog =3D timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s= ); s->regs[SONIC_SR] =3D 0x0004; /* only revision recognized by Linux *= / =20 @@ -871,7 +870,6 @@ static const VMStateDescription vmstate_dp8393x =3D {= =20 static Property dp8393x_properties[] =3D { 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 @@ =20 /* Devices attached directly to the main system bus. */ =20 +#include "sysemu/dma.h" #include "hw/qdev.h" #include "exec/memory.h" =20 @@ -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); =20 +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 =3D 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 *opa= que); =20 --------------050205010404030406000304-- --GdXQTm3UnjKtqvmLtUo2uxg0sJFIe877x Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iEYEARECAAYFAlSmZAgACgkQNKT2yavzbFPofgCdGv2qNRTEFireZnwkykoMYaqX +XUAnjzrmO1IE8iT3dY354xgSW5iQjl4 =U02e -----END PGP SIGNATURE----- --GdXQTm3UnjKtqvmLtUo2uxg0sJFIe877x--