Date: Tue, 18 Jul 2000 22:15:45 +0200 (CEST) From: =?ISO-8859-1?Q?G=E9rard_Roudier?= <groudier@club-internet.fr> To: Andrew Gallatin <gallatin@cs.duke.edu> Cc: Bernd Walter <ticso@cicely8.cicely.de>, dg@root.com, mjacob@feral.com, Mike Smith <msmith@FreeBSD.ORG>, freebsd-alpha@FreeBSD.ORG Subject: Re: fxp0 hangs on a PC164 using STABLE Message-ID: <Pine.LNX.4.10.10007182135430.1638-100000@linux.local> In-Reply-To: <14707.48450.371071.494543@grasshopper.cs.duke.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 17 Jul 2000, Andrew Gallatin wrote:
> 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 but
> > > the recv. side is at least partially OK. And I'm still getting stats
> > > updates.
> >
> > Looks like you are on the right way.
> > I can download a big file to the host without any problems but cvsup gets
> > the interface stoped quite soon.
>
> 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:
>
> /*
> * Advance the end of list forward.
> */
> sc->cbl_last->cb_command &= ~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
assembly is to be used. The assembly will force compiler barrier, but if
C can be used, may-be a volatile cast should be used to force such a
barrier.
You may also want to add a memory barrier in places where some strong
ordering regarding how LOADs and STOREs are carried out to the system
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.
>
> 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?
>
> 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.
>
> 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érard.
> Index: pci/if_fxp.c
> ===================================================================
> 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 2000/07/13 07:53:06 1.77.2.4
> +++ pci/if_fxp.c 2000/07/18 02:07:04
> @@ -527,6 +527,8 @@
>
> callout_handle_init(&sc->stat_ch);
>
> + sc->dev = dev;
> +
> s = splimp();
>
> /*
> @@ -535,6 +537,7 @@
> val = pci_read_config(dev, PCIR_COMMAND, 2);
> val |= (PCIM_CMD_MEMEN|PCIM_CMD_BUSMASTEREN);
> pci_write_config(dev, PCIR_COMMAND, val, 2);
> + sc->cmd = val;
>
> /*
> * Map control/status registers.
> @@ -960,6 +963,7 @@
> {
> struct fxp_softc *sc = ifp->if_softc;
> struct fxp_cb_tx *txp;
> + int s;
>
> /*
> * See if we need to suspend xmit until the multicast filter
> @@ -1052,10 +1056,24 @@
> }
> txp->tx_threshold = tx_threshold;
>
> +#ifdef __alpha__
> + /*
> + * Prevent the card from DMA'ing up the status while
> + * we update the command field. This could cause
> + * us to overwrite the completion status.
> + */
> + s = splhigh();
> + pci_write_config(sc->dev, PCIR_COMMAND,
> + (sc->cmd & ~PCIM_CMD_BUSMASTEREN), 2);
> +#endif /*alpha*/
> /*
> * Advance the end of list forward.
> */
> sc->cbl_last->cb_command &= ~FXP_CB_COMMAND_S;
> +#ifdef __alpha__
> + pci_write_config(sc->dev, PCIR_COMMAND, sc->cmd, 2);
> + splx(s);
> +#endif /*alpha*/
> sc->cbl_last = txp;
>
> /*
> Index: pci/if_fxpvar.h
> ===================================================================
> 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 2000/03/29 02:02:39 1.9.2.1
> +++ pci/if_fxpvar.h 2000/07/18 02:03:50
> @@ -48,6 +48,7 @@
> struct resource *mem; /* resource descriptor for registers */
> struct resource *irq; /* resource descriptor for interrupt */
> void *ih; /* interrupt handler cookie */
> + device_t dev; /* newbus device pointer */
> #endif /* __NetBSD__ */
> bus_space_tag_t sc_st; /* bus space tag */
> bus_space_handle_t sc_sh; /* bus space handle */
> @@ -68,6 +69,7 @@
> int phy_primary_device; /* device type of primary PHY */
> int phy_10Mbps_only; /* PHY is 10Mbps-only device */
> int eeprom_size; /* size of serial EEPROM */
> + u_long cmd; /* contents of pci CMD register */
> };
>
> /* Macros to ease CSR access. */
>
>
> Cheers,
>
> Drew
> ------------------------------------------------------------------------------
> Andrew Gallatin, Sr Systems Programmer http://www.cs.duke.edu/~gallatin
> Duke University Email: gallatin@cs.duke.edu
> Department of Computer Science Phone: (919) 660-6590
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-alpha" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.LNX.4.10.10007182135430.1638-100000>