From owner-freebsd-alpha Tue Jul 18 13:36:50 2000 Delivered-To: freebsd-alpha@freebsd.org Received: from front2.grolier.fr (front2.grolier.fr [194.158.96.52]) by hub.freebsd.org (Postfix) with ESMTP id 8DE8937B8EB; Tue, 18 Jul 2000 13:36:44 -0700 (PDT) (envelope-from groudier@club-internet.fr) Received: from Guyancourt-1-99.club-internet.fr (Guyancourt-1-99.club-internet.fr [195.36.205.99]) by front2.grolier.fr (8.9.3/No_Relay+No_Spam_MGC990224) with ESMTP id WAA19657; Tue, 18 Jul 2000 22:36:15 +0200 (MET DST) Date: Tue, 18 Jul 2000 22:15:45 +0200 (CEST) From: =?ISO-8859-1?Q?G=E9rard_Roudier?= X-Sender: groudier@linux.local To: Andrew Gallatin Cc: Bernd Walter , dg@root.com, mjacob@feral.com, Mike Smith , freebsd-alpha@FreeBSD.ORG Subject: Re: fxp0 hangs on a PC164 using STABLE In-Reply-To: <14707.48450.371071.494543@grasshopper.cs.duke.edu> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Sender: owner-freebsd-alpha@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org On Mon, 17 Jul 2000, Andrew Gallatin wrote: =20 > Bernd Walter writes: > > On Fri, Jul 14, 2000 at 05:23:21PM -0400, Andrew Gallatin wrote: > > > As for the bug. I can duplicate it I run both the sender & receiver > > > fairly hard, like 2 100Mb/tcp connections running in opposite > > > directions. When I do this, it looks like the xmit side locks up b= ut=20 > > > the recv. side is at least partially OK. And I'm still getting stat= s > > > updates.=20 > >=20 > > Looks like you are on the right way. > > I can download a big file to the host without any problems but cvsup g= ets > > the interface stoped quite soon. >=20 > The problem is that the status & command field share a 32-bit word. > The programming API of the eepro apparently requires that you update > the status field of a transmit slot that you've given to the card. > Since alphas can only operate on 32-bit chunks of memory, both the > status & command fields are loaded into memory & operated on in > registers when the following line of C is executed: >=20 > =09=09/* > =09=09 * Advance the end of list forward. > =09=09 */ > =09=09sc->cbl_last->cb_command &=3D ~FXP_CB_COMMAND_S; I am not an Alpha expert, but my reading of the Alpha Architecture Handbook rev. 4 let me think that not too old Alpha Systems should allow atomic WORD and BYTE memory access from agents on the system BUS. CPUs with AMASK bit 0 set support proper intructions for doing such kind of access. If the C compiler does not want to generate the corresponding code, then=20 assembly is to be used. The assembly will force compiler barrier, but if=20 C can be used, may-be a volatile cast should be used to force such a=20 barrier. You may also want to add a memory barrier in places where some strong=20 ordering regarding how LOADs and STOREs are carried out to the system=20 BUS is expected. > The race is caused by the card DMA'ing up the status at just the wrong > time -- after it has been loaded into a register & before it has been > written back. The old value of the status is written back, clobbering > the status the card just DMA'ed up. The fact that the card has sent > this frame is missed & the transmit engine appears to hang. >=20 > I think I have a fix. It feels like overkill, but if the programming > interface of the chip does not allow for something better why not just > turn off DMA while we update cb_command? >=20 > Can anybody having problems with fxp's please try the appended patch? > Without this patch, I have a test case that can lock the xmitter in < > 5 seconds 100% of the time. With the patch, I've been unable to get > it to lock up using this test case. >=20 > Can a PCI expert comment on its safety? Mike? As a PCI non-expert, I may comment on its apparent brokenness. ;-) Clearing the PCI BUS MASTER ENABLE bit is generally not a documented way to tell a `running' PCI device to suspend DMA and wait for the bit to be set again prior to continuing operations. This is likely to confuse most PCI devices, in my opinion. I hope I haven't been too wrong. :-) G=E9rard. > Index: pci/if_fxp.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /home/ncvs/src/sys/pci/if_fxp.c,v > retrieving revision 1.77.2.4 > diff -u -r1.77.2.4 if_fxp.c > --- pci/if_fxp.c=092000/07/13 07:53:06=091.77.2.4 > +++ pci/if_fxp.c=092000/07/18 02:07:04 > @@ -527,6 +527,8 @@ > =20 > =09callout_handle_init(&sc->stat_ch); > =20 > +=09sc->dev =3D dev; > + > =09s =3D splimp(); > =20 > =09/* > @@ -535,6 +537,7 @@ > =09val =3D pci_read_config(dev, PCIR_COMMAND, 2); > =09val |=3D (PCIM_CMD_MEMEN|PCIM_CMD_BUSMASTEREN); > =09pci_write_config(dev, PCIR_COMMAND, val, 2); > +=09sc->cmd =3D val; > =20 > =09/* > =09 * Map control/status registers. > @@ -960,6 +963,7 @@ > { > =09struct fxp_softc *sc =3D ifp->if_softc; > =09struct fxp_cb_tx *txp; > +=09int s; > =20 > =09/* > =09 * See if we need to suspend xmit until the multicast filter > @@ -1052,10 +1056,24 @@ > =09=09} > =09=09txp->tx_threshold =3D tx_threshold; > =09 > +#ifdef __alpha__ > +=09=09/* > +=09=09 * Prevent the card from DMA'ing up the status while > +=09=09 * we update the command field. This could cause > +=09=09 * us to overwrite the completion status. > +=09=09 */ > +=09=09s =3D splhigh(); > +=09=09pci_write_config(sc->dev, PCIR_COMMAND,=20 > +=09=09 (sc->cmd & ~PCIM_CMD_BUSMASTEREN), 2); > +#endif /*alpha*/ > =09=09/* > =09=09 * Advance the end of list forward. > =09=09 */ > =09=09sc->cbl_last->cb_command &=3D ~FXP_CB_COMMAND_S; > +#ifdef __alpha__ > +=09=09pci_write_config(sc->dev, PCIR_COMMAND, sc->cmd, 2); > +=09=09splx(s); > +#endif /*alpha*/ > =09=09sc->cbl_last =3D txp; > =20 > =09=09/* > Index: pci/if_fxpvar.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /home/ncvs/src/sys/pci/if_fxpvar.h,v > retrieving revision 1.9.2.1 > diff -u -r1.9.2.1 if_fxpvar.h > --- pci/if_fxpvar.h=092000/03/29 02:02:39=091.9.2.1 > +++ pci/if_fxpvar.h=092000/07/18 02:03:50 > @@ -48,6 +48,7 @@ > =09struct resource *mem;=09=09/* resource descriptor for registers */ > =09struct resource *irq;=09=09/* resource descriptor for interrupt */ > =09void *ih;=09=09=09/* interrupt handler cookie */ > +=09device_t dev;=09=09=09/* newbus device pointer */ > #endif /* __NetBSD__ */ > =09bus_space_tag_t sc_st;=09=09/* bus space tag */ > =09bus_space_handle_t sc_sh;=09/* bus space handle */ > @@ -68,6 +69,7 @@ > =09int phy_primary_device;=09=09/* device type of primary PHY */ > =09int phy_10Mbps_only;=09=09/* PHY is 10Mbps-only device */ > =09int eeprom_size;=09=09/* size of serial EEPROM */ > +=09u_long cmd;=09=09=09/* contents of pci CMD register */ > }; > =20 > /* Macros to ease CSR access. */ >=20 >=20 > Cheers, >=20 > Drew > -------------------------------------------------------------------------= ----- > Andrew Gallatin, Sr Systems Programmer=09http://www.cs.duke.edu/~gallatin > Duke University=09=09=09=09Email: gallatin@cs.duke.edu > Department of Computer Science=09=09Phone: (919) 660-6590 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-alpha" in the body of the message