Subject: Re: port-alpha/30560: i82557.c reports SCB timed out consistently
To: None <port-alpha-maintainer@netbsd.org, gnats-admin@netbsd.org,>
From: Stephen Jones <smj@sdf.lonestar.org>
List: netbsd-bugs
Date: 07/19/2007 19:05:05
The following reply was made to PR port-alpha/30560; it has been noted by GNATS.
From: Stephen Jones <smj@sdf.lonestar.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: port-alpha/30560: i82557.c reports SCB timed out consistently
Date: Thu, 19 Jul 2007 19:04:30 +0000 (UTC)
Hi, sure I will give it a shot! I've got a CS20 with current going at
the moment.
> The following reply was made to PR port-alpha/30560; it has been noted by GNATS.
>
> From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
> To: gnats-bugs@NetBSD.org
> Cc: smj@sdf.lonestar.org, tsutsui@ceres.dti.ne.jp
> Subject: Re: port-alpha/30560: i82557.c reports SCB timed out consistently
> Date: Thu, 19 Jul 2007 22:40:25 +0900
>
> > He also suggests that NetBSD should adopt OpenBSD's 'fix':
> > http://www.openbsd.org/cgi-bin/cvsweb/src/sys/dev/ic/fxpreg.h?sortby=date#rev1.7
>
> Could you try this patch (for -current)?
> It seems working on my AlphaPC164 with i82550.
> ---
>
> Index: i82557.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/ic/i82557.c,v
> retrieving revision 1.102
> diff -u -r1.102 i82557.c
> --- i82557.c 9 Jul 2007 21:00:36 -0000 1.102
> +++ i82557.c 19 Jul 2007 13:34:31 -0000
> @@ -189,7 +189,7 @@
> void fxp_stop(struct ifnet *, int);
>
> void fxp_txintr(struct fxp_softc *);
> -void fxp_rxintr(struct fxp_softc *);
> +int fxp_rxintr(struct fxp_softc *);
>
> int fxp_rx_hwcksum(struct mbuf *, const struct fxp_rfa *);
>
> @@ -916,7 +916,7 @@
> break;
> m = NULL;
>
> - if (sc->sc_txpending == FXP_NTXCB) {
> + if (sc->sc_txpending == FXP_NTXCB - 1) {
> FXP_EVCNT_INCR(&sc->sc_ev_txstall);
> break;
> }
> @@ -1067,7 +1067,7 @@
> #endif
> }
>
> - if (sc->sc_txpending == FXP_NTXCB) {
> + if (sc->sc_txpending == FXP_NTXCB - 1) {
> /* No more slots; notify upper layer. */
> ifp->if_flags |= IFF_OACTIVE;
> }
> @@ -1084,9 +1084,23 @@
> * Cause the chip to interrupt and suspend command
> * processing once the last packet we've enqueued
> * has been transmitted.
> + *
> + * To avoid a race between updating status bits
> + * by the fxp chip and clearing command bits
> + * by this function on machines which don't have
> + * atomic methods to clear/set bits in memory
> + * smaller than 32bits (both cb_status and cb_command
> + * members are uint16_t and in the same 32bit word),
> + * we have to prepare a dummy TX descriptor which has
> + * NOP command and just causes a TX completion interrupt.
> */
> - FXP_CDTX(sc, sc->sc_txlast)->txd_txcb.cb_command |=
> - htole16(FXP_CB_COMMAND_I | FXP_CB_COMMAND_S);
> + sc->sc_txpending++;
> + sc->sc_txlast = FXP_NEXTTX(sc->sc_txlast);
> + txd = FXP_CDTX(sc, sc->sc_txlast);
> + /* BIG_ENDIAN: no need to swap to store 0 */
> + txd->txd_txcb.cb_status = 0;
> + txd->txd_txcb.cb_command = htole16(FXP_CB_COMMAND_NOP |
> + FXP_CB_COMMAND_I | FXP_CB_COMMAND_S);
> FXP_CDTXSYNC(sc, sc->sc_txlast,
> BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE);
>
> @@ -1121,7 +1135,7 @@
> struct fxp_softc *sc = arg;
> struct ifnet *ifp = &sc->sc_ethercom.ec_if;
> bus_dmamap_t rxmap;
> - int claimed = 0;
> + int claimed = 0, rnr;
> u_int8_t statack;
>
> if (!device_is_active(&sc->sc_dev) || sc->sc_enabled == 0)
> @@ -1152,20 +1166,12 @@
> * condition exists, get whatever packets we can and
> * re-start the receiver.
> */
> - if (statack & (FXP_SCB_STATACK_FR | FXP_SCB_STATACK_RNR)) {
> + rnr = (statack & (FXP_SCB_STATACK_RNR | FXP_SCB_STATACK_SWI)) ?
> + 1 : 0;
> + if (statack & (FXP_SCB_STATACK_FR | FXP_SCB_STATACK_RNR |
> + FXP_SCB_STATACK_SWI)) {
> FXP_EVCNT_INCR(&sc->sc_ev_rxintr);
> - fxp_rxintr(sc);
> - }
> -
> - if (statack & FXP_SCB_STATACK_RNR) {
> - fxp_scb_wait(sc);
> - fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_ABORT);
> - rxmap = M_GETCTX(sc->sc_rxq.ifq_head, bus_dmamap_t);
> - fxp_scb_wait(sc);
> - CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL,
> - rxmap->dm_segs[0].ds_addr +
> - RFA_ALIGNMENT_FUDGE);
> - fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_START);
> + rnr |= fxp_rxintr(sc);
> }
>
> /*
> @@ -1188,6 +1194,17 @@
> (void) fxp_init(ifp);
> }
> }
> +
> + if (rnr) {
> + fxp_scb_wait(sc);
> + fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_ABORT);
> + rxmap = M_GETCTX(sc->sc_rxq.ifq_head, bus_dmamap_t);
> + fxp_scb_wait(sc);
> + CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL,
> + rxmap->dm_segs[0].ds_addr +
> + RFA_ALIGNMENT_FUDGE);
> + fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_START);
> + }
> }
>
> #if NRND > 0
> @@ -1218,6 +1235,11 @@
> FXP_CDTXSYNC(sc, i,
> BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);
>
> + /* skip dummy NOP TX descriptor */
> + if ((le16toh(txd->txd_txcb.cb_command) & FXP_CB_COMMAND_CMD)
> + == FXP_CB_COMMAND_NOP)
> + continue;
> +
> txstat = le16toh(txd->txd_txcb.cb_status);
>
> if ((txstat & FXP_CB_STATUS_C) == 0)
> @@ -1301,7 +1323,7 @@
> /*
> * Handle receive interrupts.
> */
> -void
> +int
> fxp_rxintr(struct fxp_softc *sc)
> {
> struct ethercom *ec = &sc->sc_ethercom;
> @@ -1309,8 +1331,11 @@
> struct mbuf *m, *m0;
> bus_dmamap_t rxmap;
> struct fxp_rfa *rfa;
> + int rnr;
> u_int16_t len, rxstat;
>
> + rnr = 0;
> +
> for (;;) {
> m = sc->sc_rxq.ifq_head;
> rfa = FXP_MTORFA(m);
> @@ -1321,13 +1346,16 @@
>
> rxstat = le16toh(rfa->rfa_status);
>
> + if ((rxstat & FXP_RFA_STATUS_RNR) != 0)
> + rnr = 1;
> +
> if ((rxstat & FXP_RFA_STATUS_C) == 0) {
> /*
> * We have processed all of the
> * receive buffers.
> */
> FXP_RFASYNC(sc, m, BUS_DMASYNC_PREREAD);
> - return;
> + return rnr;
> }
>
> IF_DEQUEUE(&sc->sc_rxq, m);
> @@ -1923,11 +1951,13 @@
> /*
> * Initialize receiver buffer area - RFA.
> */
> +#if 0
> rxmap = M_GETCTX(sc->sc_rxq.ifq_head, bus_dmamap_t);
> fxp_scb_wait(sc);
> CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL,
> rxmap->dm_segs[0].ds_addr + RFA_ALIGNMENT_FUDGE);
> fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_START);
> +#endif
>
> if (sc->sc_flags & FXPF_MII) {
> /*
> @@ -1943,6 +1973,16 @@
> ifp->if_flags &= ~IFF_OACTIVE;
>
> /*
> + * Request a software generated interrupt that will be used to
> + * (re)start the RU processing. If we direct the chip to start
> + * receiving from the start of queue now, instead of letting the
> + * interrupt handler first process all received packets, we run
> + * the risk of having it overwrite mbuf clusters while they are
> + * being processed or after they have been returned to the pool.
> + */
> + CSR_WRITE_1(sc, FXP_CSR_SCB_INTRCNTL, FXP_SCB_INTRCNTL_REQUEST_SWI);
> +
> + /*
> * Start the one second timer.
> */
> callout_reset(&sc->sc_callout, hz, fxp_tick, sc);
> Index: i82557reg.h
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/ic/i82557reg.h,v
> retrieving revision 1.18
> diff -u -r1.18 i82557reg.h
> --- i82557reg.h 11 Dec 2005 12:21:26 -0000 1.18
> +++ i82557reg.h 19 Jul 2007 13:34:31 -0000
> @@ -143,6 +143,7 @@
> #define FXP_SCB_COMMAND_RU_BASE 6
> #define FXP_SCB_COMMAND_RU_RBDRESUME 7
>
> +#define FXP_SCB_INTRCNTL_REQUEST_SWI 0x02
> /*
> * Command block definitions
> */
> @@ -368,6 +369,7 @@
> #define FXP_CB_STATUS_C 0x8000
>
> /* commands */
> +#define FXP_CB_COMMAND_CMD 0x0007 /* XXX how about FXPF_IPCB case? */
> #define FXP_CB_COMMAND_NOP 0x0
> #define FXP_CB_COMMAND_IAS 0x1
> #define FXP_CB_COMMAND_CONFIG 0x2
> @@ -409,7 +411,7 @@
> volatile u_int8_t cksum_stat;
> volatile u_int8_t zerocopy_stat;
> volatile u_int8_t unused[8];
> -};
> +}__attribute__((__packed__));
>
> #define RFA_SIZE 16
> #define RFA_EXT_SIZE 32
> Index: i82557var.h
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/ic/i82557var.h,v
> retrieving revision 1.35
> diff -u -r1.35 i82557var.h
> --- i82557var.h 11 Dec 2005 12:21:26 -0000 1.35
> +++ i82557var.h 19 Jul 2007 13:34:31 -0000
> @@ -318,7 +318,8 @@
> __rfa->size = htole16(FXP_RXBUFSIZE((sc), (m))); \
> /* BIG_ENDIAN: no need to swap to store 0 */ \
> __rfa->rfa_status = 0; \
> - __rfa->rfa_control = htole16(FXP_RFA_CONTROL_EL); \
> + __rfa->rfa_control = \
> + htole16(FXP_RFA_CONTROL_EL | FXP_RFA_CONTROL_S); \
> /* BIG_ENDIAN: no need to swap to store 0 */ \
> __rfa->actual_size = 0; \
> \
> @@ -341,7 +342,8 @@
> BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE); \
> memcpy((void *)&__p_rfa->link_addr, &__v, \
> sizeof(__v)); \
> - __p_rfa->rfa_control &= htole16(~FXP_RFA_CONTROL_EL); \
> + __p_rfa->rfa_control &= htole16(~(FXP_RFA_CONTROL_EL| \
> + FXP_RFA_CONTROL_S)); \
> FXP_RFASYNC((sc), __p_m, \
> BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE); \
> } \
>
> ---
> Izumi Tsutsui
>
>