Subject: Re: port-alpha/30560: i82557.c reports SCB timed out consistently
To: None <port-alpha-maintainer@netbsd.org, gnats-admin@netbsd.org,>
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
List: netbsd-bugs
Date: 07/19/2007 13:45:02
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