From: stsp <stsp@openbsd.org>
Date: Wed, 30 Jun 2021 09:46:46 +0000 (+0000)
Subject: Make Tx-done interrupt processing in iwx(4) more similar to iwlwifi.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=9b075b3f4b0d0fe1f04574592e3b2439629c3040;p=openbsd

Make Tx-done interrupt processing in iwx(4) more similar to iwlwifi.

When iwx(4) takes a frame off the queue we used the ring index reported
by firmware to figure out which frame can be taken off the ring.
This logic was inherited from iwn(4).

We have sometimes seen an index get skipped and had a workaround in place
where we took all pending frames up to this index off the ring.

Apart from the ring index the firmware reports another value which is its
starting sequence number (SSN). An SSN is usually associated with a Tx
aggregation queue that uses block ack. On non-aggregation queues the SSN
matches the ring index most of the time and hence seems redundant at first
sight. But the values are not always the same.

We now always use the SSN as the upper bound which matches the Linux driver.

This seems to fix fatal firmware errors during Tx commands seen by jcs@
which suggests that we were sometimes taking frames off the ring too early.
---

diff --git a/sys/dev/pci/if_iwx.c b/sys/dev/pci/if_iwx.c
index 21bc8be3450..8761374945f 100644
--- a/sys/dev/pci/if_iwx.c
+++ b/sys/dev/pci/if_iwx.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_iwx.c,v 1.59 2021/06/21 10:19:21 stsp Exp $	*/
+/*	$OpenBSD: if_iwx.c,v 1.60 2021/06/30 09:46:46 stsp Exp $	*/
 
 /*
  * Copyright (c) 2014, 2016 genua gmbh <info@genua.de>
@@ -4245,6 +4245,9 @@ iwx_rx_tx_cmd(struct iwx_softc *sc, struct iwx_rx_packet *pkt,
 	int qid = cmd_hdr->qid;
 	struct iwx_tx_ring *ring = &sc->txq[qid];
 	struct iwx_tx_data *txd;
+	struct iwx_tx_resp *tx_resp = (void *)pkt->data;
+	uint32_t ssn;
+	uint32_t len = iwx_rx_packet_len(pkt);
 
 	bus_dmamap_sync(sc->sc_dmat, data->map, 0, IWX_RBUF_SIZE,
 	    BUS_DMASYNC_POSTREAD);
@@ -4255,20 +4258,21 @@ iwx_rx_tx_cmd(struct iwx_softc *sc, struct iwx_rx_packet *pkt,
 	if (txd->m == NULL)
 		return;
 
+	if (sizeof(*tx_resp) + sizeof(ssn) +
+	    tx_resp->frame_count * sizeof(tx_resp->status) > len)
+		return;
+
 	iwx_rx_tx_cmd_single(sc, pkt, txd->in);
-	iwx_txd_done(sc, txd);
-	iwx_tx_update_byte_tbl(ring, idx, 0, 0);
 
 	/*
-	 * XXX Sometimes we miss Tx completion interrupts.
-	 * We cannot check Tx success/failure for affected frames; just free
-	 * the associated mbuf and release the associated node reference.
+	 * Even though this is not an agg queue, we must only free
+	 * frames before the firmware's starting sequence number.
 	 */
-	while (ring->tail != idx) {
+	memcpy(&ssn, &tx_resp->status + tx_resp->frame_count, sizeof(ssn));
+	ssn = le32toh(ssn) & 0xfff;
+	while (ring->tail != IWX_AGG_SSN_TO_TXQ_IDX(ssn)) {
 		txd = &ring->data[ring->tail];
 		if (txd->m != NULL) {
-			DPRINTF(("%s: missed Tx completion: tail=%d idx=%d\n",
-			    __func__, ring->tail, idx));
 			iwx_txd_done(sc, txd);
 			iwx_tx_update_byte_tbl(ring, idx, 0, 0);
 			ring->queued--;
diff --git a/sys/dev/pci/if_iwxvar.h b/sys/dev/pci/if_iwxvar.h
index 390b2fedc5c..928b4d9ec79 100644
--- a/sys/dev/pci/if_iwxvar.h
+++ b/sys/dev/pci/if_iwxvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_iwxvar.h,v 1.16 2021/05/06 09:19:28 stsp Exp $	*/
+/*	$OpenBSD: if_iwxvar.h,v 1.17 2021/06/30 09:46:46 stsp Exp $	*/
 
 /*
  * Copyright (c) 2014 genua mbh <info@genua.de>
@@ -228,6 +228,9 @@ struct iwx_dma_info {
 #define IWX_TX_RING_LOMARK	192
 #define IWX_TX_RING_HIMARK	224
 
+/* For aggregation queues, index must be aligned to frame sequence number. */
+#define IWX_AGG_SSN_TO_TXQ_IDX(x)	((x) & (IWX_TX_RING_COUNT - 1))
+
 struct iwx_tx_data {
 	bus_dmamap_t	map;
 	bus_addr_t	cmd_paddr;