Fix a couple of bad races that can occur when a cxgbe interface is taken

down.  The ingress queue lock was unused and has been removed as part of
these changes.

- An in-flight egress update from the SGE must be handled before the
  queue that requested it is destroyed.  Wait for the update to arrive.

- Interrupt handlers must stop processing rx events for a queue before
  the queue is destroyed.  Events that have not yet been processed
  should be ignored once the queue disappears.

MFC after:	1 week

5 files changed, 97 insertions, 31 deletions

diff --git a/sys/dev/cxgbe/adapter.h b/sys/dev/cxgbe/adapter.h
index e38ffb4..59609f2 100644
--- a/sys/dev/cxgbe/adapter.h
+++ b/sys/dev/cxgbe/adapter.h
@@ -227,6 +227,11 @@ enum {
 	/* iq flags */
 	IQ_ALLOCATED	= (1 << 1),	/* firmware resources allocated */
 	IQ_STARTED	= (1 << 2),	/* started */
+
+	/* iq state */
+	IQS_DISABLED	= 0,
+	IQS_BUSY	= 1,
+	IQS_IDLE	= 2,
 };
 
 /*
@@ -244,7 +249,7 @@ struct sge_iq {
 	iq_intr_handler_t *handler;
 	__be64  *desc;		/* KVA of descriptor ring */
 
-	struct mtx iq_lock;
+	volatile uint32_t state;
 	struct adapter *adapter;
 	const __be64 *cdesc;	/* current descriptor */
 	uint8_t  gen;		/* generation bit */
@@ -445,22 +450,12 @@ struct adapter {
 #define PORT_LOCK_ASSERT_OWNED(pi)	mtx_assert(&(pi)->pi_lock, MA_OWNED)
 #define PORT_LOCK_ASSERT_NOTOWNED(pi)	mtx_assert(&(pi)->pi_lock, MA_NOTOWNED)
 
-#define IQ_LOCK(iq)			mtx_lock(&(iq)->iq_lock)
-#define IQ_UNLOCK(iq)			mtx_unlock(&(iq)->iq_lock)
-#define IQ_LOCK_ASSERT_OWNED(iq)	mtx_assert(&(iq)->iq_lock, MA_OWNED)
-#define IQ_LOCK_ASSERT_NOTOWNED(iq)	mtx_assert(&(iq)->iq_lock, MA_NOTOWNED)
-
 #define FL_LOCK(fl)			mtx_lock(&(fl)->fl_lock)
 #define FL_TRYLOCK(fl)			mtx_trylock(&(fl)->fl_lock)
 #define FL_UNLOCK(fl)			mtx_unlock(&(fl)->fl_lock)
 #define FL_LOCK_ASSERT_OWNED(fl)	mtx_assert(&(fl)->fl_lock, MA_OWNED)
 #define FL_LOCK_ASSERT_NOTOWNED(fl)	mtx_assert(&(fl)->fl_lock, MA_NOTOWNED)
 
-#define RXQ_LOCK(rxq)			IQ_LOCK(&(rxq)->iq)
-#define RXQ_UNLOCK(rxq)			IQ_UNLOCK(&(rxq)->iq)
-#define RXQ_LOCK_ASSERT_OWNED(rxq)	IQ_LOCK_ASSERT_OWNED(&(rxq)->iq)
-#define RXQ_LOCK_ASSERT_NOTOWNED(rxq)	IQ_LOCK_ASSERT_NOTOWNED(&(rxq)->iq)
-
 #define RXQ_FL_LOCK(rxq)		FL_LOCK(&(rxq)->fl)
 #define RXQ_FL_UNLOCK(rxq)		FL_UNLOCK(&(rxq)->fl)
 #define RXQ_FL_LOCK_ASSERT_OWNED(rxq)	FL_LOCK_ASSERT_OWNED(&(rxq)->fl)
@@ -586,6 +581,8 @@ void t4_intr_fwd(void *);
 void t4_intr_err(void *);
 void t4_intr_evt(void *);
 void t4_intr_data(void *);
+void t4_evt_rx(void *);
+void t4_eth_rx(void *);
 int t4_eth_tx(struct ifnet *, struct sge_txq *, struct mbuf *);
 void t4_update_fl_bufsize(struct ifnet *);
 
diff --git a/sys/dev/cxgbe/common/t4_hw.c b/sys/dev/cxgbe/common/t4_hw.c
index ce96cb0..dd8d0fc 100644
--- a/sys/dev/cxgbe/common/t4_hw.c
+++ b/sys/dev/cxgbe/common/t4_hw.c
@@ -32,6 +32,8 @@ __FBSDID("$FreeBSD$");
 #include "t4_regs_values.h"
 #include "t4fw_interface.h"
 
+#undef msleep
+#define msleep(x) DELAY((x) * 1000)
 
 /**
  *	t4_wait_op_done_val - wait until an operation is completed
diff --git a/sys/dev/cxgbe/osdep.h b/sys/dev/cxgbe/osdep.h
index 693cac2..438a434 100644
--- a/sys/dev/cxgbe/osdep.h
+++ b/sys/dev/cxgbe/osdep.h
@@ -74,8 +74,6 @@ typedef boolean_t bool;
 #define false FALSE
 #define true TRUE
 
-#undef msleep
-#define msleep(x) DELAY((x) * 1000)
 #define mdelay(x) DELAY((x) * 1000)
 #define udelay(x) DELAY(x)
 
diff --git a/sys/dev/cxgbe/t4_main.c b/sys/dev/cxgbe/t4_main.c
index f22f5da..b31e52c 100644
--- a/sys/dev/cxgbe/t4_main.c
+++ b/sys/dev/cxgbe/t4_main.c
@@ -1038,8 +1038,21 @@ static void
 cxgbe_qflush(struct ifnet *ifp)
 {
 	struct port_info *pi = ifp->if_softc;
+	struct sge_txq *txq;
+	int i;
+	struct mbuf *m;
 
-	device_printf(pi->dev, "%s unimplemented.\n", __func__);
+	/* queues do not exist if !IFF_DRV_RUNNING. */
+	if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
+		for_each_txq(pi, i, txq) {
+			TXQ_LOCK(txq);
+			m_freem(txq->m);
+			while ((m = buf_ring_dequeue_sc(txq->eq.br)) != NULL)
+				m_freem(m);
+			TXQ_UNLOCK(txq);
+		}
+	}
+	if_qflush(ifp);
 }
 
 static int
@@ -2673,8 +2686,11 @@ cxgbe_txq_start(void *arg, int count)
 	struct sge_txq *txq = arg;
 
 	TXQ_LOCK(txq);
-	txq->eq.flags &= ~EQ_CRFLUSHED;
-	txq_start(txq->ifp, txq);
+	if (txq->eq.flags & EQ_CRFLUSHED) {
+		txq->eq.flags &= ~EQ_CRFLUSHED;
+		txq_start(txq->ifp, txq);
+	} else
+		wakeup_one(txq);	/* txq is going away, wakeup free_txq */
 	TXQ_UNLOCK(txq);
 }
 
diff --git a/sys/dev/cxgbe/t4_sge.c b/sys/dev/cxgbe/t4_sge.c
index 5764f4c..0be6a0f 100644
--- a/sys/dev/cxgbe/t4_sge.c
+++ b/sys/dev/cxgbe/t4_sge.c
@@ -279,7 +279,7 @@ t4_setup_adapter_iqs(struct adapter *sc)
 			}
 		}
 
-		handler = t4_intr_evt;
+		handler = t4_evt_rx;
 		i = 0;	/* forward fwq's interrupt to the first fiq */
 	} else {
 		handler = NULL;
@@ -345,7 +345,7 @@ t4_setup_eth_queues(struct port_info *pi)
 		    device_get_nameunit(pi->dev), i);
 		init_iq(&rxq->iq, sc, pi->tmr_idx, pi->pktc_idx,
 		    pi->qsize_rxq, RX_IQ_ESIZE,
-		    sc->flags & INTR_FWD ? t4_intr_data: NULL, name);
+		    sc->flags & INTR_FWD ? t4_eth_rx : NULL, name);
 
 		snprintf(name, sizeof(name), "%s rxq%d-fl",
 		    device_get_nameunit(pi->dev), i);
@@ -428,6 +428,9 @@ t4_intr_fwd(void *arg)
 	int ndesc_pending = 0, ndesc_total = 0;
 	int qid;
 
+	if (!atomic_cmpset_32(&iq->state, IQS_IDLE, IQS_BUSY))
+		return;
+
 	while (is_new_response(iq, &ctrl)) {
 
 		rmb();
@@ -460,6 +463,8 @@ t4_intr_fwd(void *arg)
 		    V_CIDXINC(ndesc_pending) | V_INGRESSQID((u32)iq->cntxt_id) |
 		    V_SEINTARM(iq->intr_params));
 	}
+
+	atomic_cmpset_32(&iq->state, IQS_BUSY, IQS_IDLE);
 }
 
 /* Deals with error interrupts */
@@ -479,6 +484,32 @@ void
 t4_intr_evt(void *arg)
 {
 	struct sge_iq *iq = arg;
+
+	if (!atomic_cmpset_32(&iq->state, IQS_IDLE, IQS_BUSY))
+		return;
+
+	t4_evt_rx(arg);
+
+	atomic_cmpset_32(&iq->state, IQS_BUSY, IQS_IDLE);
+}
+
+void
+t4_intr_data(void *arg)
+{
+	struct sge_iq *iq = arg;
+
+	if (!atomic_cmpset_32(&iq->state, IQS_IDLE, IQS_BUSY))
+		return;
+
+	t4_eth_rx(arg);
+
+	atomic_cmpset_32(&iq->state, IQS_BUSY, IQS_IDLE);
+}
+
+void
+t4_evt_rx(void *arg)
+{
+	struct sge_iq *iq = arg;
 	struct adapter *sc = iq->adapter;
 	struct rsp_ctrl *ctrl;
 	const struct rss_header *rss;
@@ -537,7 +568,7 @@ t4_intr_evt(void *arg)
 }
 
 void
-t4_intr_data(void *arg)
+t4_eth_rx(void *arg)
 {
 	struct sge_rxq *rxq = arg;
 	struct sge_iq *iq = arg;
@@ -1017,8 +1048,6 @@ alloc_iq_fl(struct port_info *pi, struct sge_iq *iq, struct sge_fl *fl,
 	if (pi == NULL)
 		pi = sc->port[0];
 
-	mtx_init(&iq->iq_lock, iq->lockname, NULL, MTX_DEF);
-
 	len = iq->qsize * iq->esize;
 	rc = alloc_ring(sc, len, &iq->desc_tag, &iq->desc_map, &iq->ba,
 	    (void **)&iq->desc);
@@ -1148,6 +1177,7 @@ alloc_iq_fl(struct port_info *pi, struct sge_iq *iq, struct sge_fl *fl,
 	}
 
 	/* Enable IQ interrupts */
+	atomic_store_rel_32(&iq->state, IQS_IDLE);
 	t4_write_reg(sc, MYPF_REG(A_SGE_PF_GTS), V_SEINTARM(iq->intr_params) |
 	    V_INGRESSQID(iq->cntxt_id));
 
@@ -1179,6 +1209,10 @@ free_iq_fl(struct port_info *pi, struct sge_iq *iq, struct sge_fl *fl)
 			return (rc);
 		}
 		iq->flags &= ~IQ_STARTED;
+
+		/* Synchronize with the interrupt handler */
+		while (!atomic_cmpset_32(&iq->state, IQS_IDLE, IQS_DISABLED))
+			pause("iqfree", hz / 1000);
 	}
 
 	if (iq->flags & IQ_ALLOCATED) {
@@ -1196,9 +1230,6 @@ free_iq_fl(struct port_info *pi, struct sge_iq *iq, struct sge_fl *fl)
 
 	free_ring(sc, iq->desc_tag, iq->desc_map, iq->ba, iq->desc);
 
-	if (mtx_initialized(&iq->iq_lock))
-		mtx_destroy(&iq->iq_lock);
-
 	bzero(iq, sizeof(*iq));
 
 	if (fl) {
@@ -1425,6 +1456,27 @@ free_txq(struct port_info *pi, struct sge_txq *txq)
 	struct sge_eq *eq = &txq->eq;
 
 	if (eq->flags & (EQ_ALLOCATED | EQ_STARTED)) {
+
+		/*
+		 * Wait for the response to a credit flush if there's one
+		 * pending.  Clearing the flag tells handle_sge_egr_update or
+		 * cxgbe_txq_start (depending on how far the response has made
+		 * it) that they should ignore the response and wake up free_txq
+		 * instead.
+		 *
+		 * The interface has been marked down by the time we get here
+		 * (both IFF_UP and IFF_DRV_RUNNING cleared).  qflush has
+		 * emptied the tx buf_rings and we know nothing new is being
+		 * queued for tx so we don't have to worry about a new credit
+		 * flush request.
+		 */
+		TXQ_LOCK(txq);
+		if (eq->flags & EQ_CRFLUSHED) {
+			eq->flags &= ~EQ_CRFLUSHED;
+			msleep(txq, &eq->eq_lock, 0, "crflush", 0);
+		}
+		TXQ_UNLOCK(txq);
+
 		rc = -t4_eth_eq_free(sc, sc->mbox, sc->pf, 0, eq->cntxt_id);
 		if (rc != 0) {
 			device_printf(pi->dev,
@@ -2444,13 +2496,14 @@ handle_sge_egr_update(struct adapter *sc, const struct cpl_sge_egr_update *cpl)
 	struct port_info *pi;
 
 	txq = (void *)s->eqmap[qid - s->eq_start];
-
-	KASSERT(txq->eq.flags & EQ_CRFLUSHED,
-	    ("%s: tx queue %p not expecting an update.", __func__, txq));
-
-	pi = txq->ifp->if_softc;
-	taskqueue_enqueue(pi->tq, &txq->resume_tx);
-	txq->egr_update++;
+	TXQ_LOCK(txq);
+	if (txq->eq.flags & EQ_CRFLUSHED) {
+		pi = txq->ifp->if_softc;
+		taskqueue_enqueue(pi->tq, &txq->resume_tx);
+		txq->egr_update++;
+	} else
+		wakeup_one(txq);	/* txq is going away, wakeup free_txq */
+	TXQ_UNLOCK(txq);
 
 	return (0);
 }