git: 86bf3fd82f1f - stable/13 - if_bnxt: Convert all doorbell writes into function pointers

From: Warner Losh <imp_at_FreeBSD.org>
Date: Thu, 19 Jan 2023 03:52:11 UTC
The branch stable/13 has been updated by imp:

URL: https://cgit.FreeBSD.org/src/commit/?id=86bf3fd82f1f2c51968df01f483a0524d6168af0

commit 86bf3fd82f1f2c51968df01f483a0524d6168af0
Author:     Sumit Saxena <sumit.saxena@broadcom.com>
AuthorDate: 2022-11-04 22:01:30 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2023-01-19 01:00:05 +0000

    if_bnxt: Convert all doorbell writes into function pointers
    
    This is preparatory patch for making a base for Broadcom's Thor
    controller support. It converts all doorbell writes into function
    pointers.
    
    Reviewed by: imp
    Differential Revision: https://reviews.freebsd.org/D36437
    
    (cherry picked from commit 49a3df78c75f29dfa4991314535c0eac4f5a73fa)
---
 sys/dev/bnxt/bnxt.h      | 95 +++++++-----------------------------------------
 sys/dev/bnxt/bnxt_txrx.c | 14 ++-----
 sys/dev/bnxt/if_bnxt.c   | 87 ++++++++++++++++++++++++++++++++++----------
 3 files changed, 86 insertions(+), 110 deletions(-)

diff --git a/sys/dev/bnxt/bnxt.h b/sys/dev/bnxt/bnxt.h
index f371388a411e..8d806c3a4877 100644
--- a/sys/dev/bnxt/bnxt.h
+++ b/sys/dev/bnxt/bnxt.h
@@ -127,87 +127,6 @@ __FBSDID("$FreeBSD$");
 	    (CACHE_LINE_SIZE / sizeof(struct cmpl_base))) &		    \
 	    ((cpr)->ring.ring_size - 1)])
 
-/*
- * If we update the index, a write barrier is needed after the write to ensure
- * the completion ring has space before the RX/TX ring does.  Since we can't
- * make the RX and AG doorbells covered by the same barrier without remapping
- * MSI-X vectors, we create the barrier over the enture doorbell bar.
- * TODO: Remap the MSI-X vectors to allow a barrier to only cover the doorbells
- *       for a single ring group.
- *
- * A barrier of just the size of the write is used to ensure the ordering
- * remains correct and no writes are lost.
- */
-#define BNXT_CP_DISABLE_DB(ring) do {					    \
-	bus_space_barrier((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, (ring)->doorbell, 4,	    \
-	    BUS_SPACE_BARRIER_WRITE);					    \
-	bus_space_barrier((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, 0,			    \
-	    (ring)->softc->doorbell_bar.size, BUS_SPACE_BARRIER_WRITE);	    \
-	bus_space_write_4((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, (ring)->doorbell,	    \
-	    htole32(CMPL_DOORBELL_KEY_CMPL | CMPL_DOORBELL_MASK));	    \
-} while (0)
-
-#define BNXT_CP_ENABLE_DB(ring) do {					    \
-	bus_space_barrier((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, (ring)->doorbell, 4,	    \
-	    BUS_SPACE_BARRIER_WRITE);					    \
-	bus_space_barrier((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, 0,			    \
-	    (ring)->softc->doorbell_bar.size, BUS_SPACE_BARRIER_WRITE);	    \
-	bus_space_write_4((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, (ring)->doorbell,	    \
-	    htole32(CMPL_DOORBELL_KEY_CMPL));				    \
-} while (0)
-
-#define BNXT_CP_IDX_ENABLE_DB(ring, cons) do {				    \
-	bus_space_barrier((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, (ring)->doorbell, 4,	    \
-	    BUS_SPACE_BARRIER_WRITE);					    \
-	bus_space_write_4((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, (ring)->doorbell,	    \
-	    htole32(CMPL_DOORBELL_KEY_CMPL | CMPL_DOORBELL_IDX_VALID |	    \
-	    (cons)));							    \
-	bus_space_barrier((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, 0,			    \
-	    (ring)->softc->doorbell_bar.size, BUS_SPACE_BARRIER_WRITE);	    \
-} while (0)
-
-#define BNXT_CP_IDX_DISABLE_DB(ring, cons) do {				    \
-	bus_space_barrier((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, (ring)->doorbell, 4,	    \
-	    BUS_SPACE_BARRIER_WRITE);					    \
-	bus_space_write_4((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, (ring)->doorbell,	    \
-	    htole32(CMPL_DOORBELL_KEY_CMPL | CMPL_DOORBELL_IDX_VALID |	    \
-	    CMPL_DOORBELL_MASK | (cons)));				    \
-	bus_space_barrier((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, 0,			    \
-	    (ring)->softc->doorbell_bar.size, BUS_SPACE_BARRIER_WRITE);	    \
-} while (0)
-
-#define BNXT_TX_DB(ring, idx) do {					    \
-	bus_space_barrier((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, (ring)->doorbell, 4,	    \
-	    BUS_SPACE_BARRIER_WRITE);					    \
-	bus_space_write_4(						    \
-	    (ring)->softc->doorbell_bar.tag,				    \
-	    (ring)->softc->doorbell_bar.handle,				    \
-	    (ring)->doorbell, htole32(TX_DOORBELL_KEY_TX | (idx)));	    \
-} while (0)
-
-#define BNXT_RX_DB(ring, idx) do {					    \
-	bus_space_barrier((ring)->softc->doorbell_bar.tag,		    \
-	    (ring)->softc->doorbell_bar.handle, (ring)->doorbell, 4,	    \
-	    BUS_SPACE_BARRIER_WRITE);					    \
-	bus_space_write_4(						    \
-	    (ring)->softc->doorbell_bar.tag,				    \
-	    (ring)->softc->doorbell_bar.handle,				    \
-	    (ring)->doorbell, htole32(RX_DOORBELL_KEY_RX | (idx)));	    \
-} while (0)
-
 /* Lock macros */
 #define BNXT_HWRM_LOCK_INIT(_softc, _name) \
     mtx_init(&(_softc)->hwrm_lock, _name, "BNXT HWRM Lock", MTX_DEF)
@@ -243,6 +162,19 @@ __FBSDID("$FreeBSD$");
 
 #define BNXT_MIN_FRAME_SIZE	52	/* Frames must be padded to this size for some A0 chips */
 
+typedef void (*bnxt_doorbell_tx)(void *, uint16_t idx);
+typedef void (*bnxt_doorbell_rx)(void *, uint16_t idx);
+typedef void (*bnxt_doorbell_rx_cq)(void *, bool);
+typedef void (*bnxt_doorbell_tx_cq)(void *, bool);
+typedef void (*bnxt_doorbell_nq)(void *, bool);
+
+typedef struct bnxt_doorbell_ops {
+        bnxt_doorbell_tx bnxt_db_tx;
+        bnxt_doorbell_rx bnxt_db_rx;
+        bnxt_doorbell_rx_cq bnxt_db_rx_cq;
+        bnxt_doorbell_tx_cq bnxt_db_tx_cq;
+        bnxt_doorbell_nq bnxt_db_nq;
+} bnxt_dooorbell_ops_t;
 /* NVRAM access */
 enum bnxt_nvm_directory_type {
 	BNX_DIR_TYPE_UNUSED = 0,
@@ -611,6 +543,7 @@ struct bnxt_softc {
 	struct bnxt_cp_ring	def_cp_ring;
 	struct iflib_dma_info	def_cp_ring_mem;
 	struct grouptask	def_cp_task;
+	struct bnxt_doorbell_ops db_ops;
 
 	struct sysctl_ctx_list	hw_stats;
 	struct sysctl_oid	*hw_stats_oid;
diff --git a/sys/dev/bnxt/bnxt_txrx.c b/sys/dev/bnxt/bnxt_txrx.c
index 9727230b578f..f8b1c7656f2b 100644
--- a/sys/dev/bnxt/bnxt_txrx.c
+++ b/sys/dev/bnxt/bnxt_txrx.c
@@ -180,9 +180,7 @@ bnxt_isc_txd_flush(void *sc, uint16_t txqid, qidx_t pidx)
 	struct bnxt_ring *tx_ring = &softc->tx_rings[txqid];
 
 	/* pidx is what we last set ipi_new_pidx to */
-	BNXT_TX_DB(tx_ring, pidx);
-	/* TODO: Cumulus+ doesn't need the double doorbell */
-	BNXT_TX_DB(tx_ring, pidx);
+	softc->db_ops.bnxt_db_tx(tx_ring, pidx);
 	return;
 }
 
@@ -244,7 +242,7 @@ done:
 	if (clear && avail) {
 		cpr->cons = last_cons;
 		cpr->v_bit = last_v_bit;
-		BNXT_CP_IDX_DISABLE_DB(&cpr->ring, cpr->cons);
+		softc->db_ops.bnxt_db_tx_cq(cpr, 0);
 	}
 
 	return avail;
@@ -313,12 +311,8 @@ bnxt_isc_rxd_flush(void *sc, uint16_t rxqid, uint8_t flid,
 	 * or we will overrun the completion ring and the device will wedge for
 	 * RX.
 	 */
-	if (softc->rx_cp_rings[rxqid].cons != UINT32_MAX)
-		BNXT_CP_IDX_DISABLE_DB(&softc->rx_cp_rings[rxqid].ring,
-		    softc->rx_cp_rings[rxqid].cons);
-	BNXT_RX_DB(rx_ring, pidx);
-	/* TODO: Cumulus+ doesn't need the double doorbell */
-	BNXT_RX_DB(rx_ring, pidx);
+	softc->db_ops.bnxt_db_rx_cq(&softc->rx_cp_rings[rxqid], 0);
+	softc->db_ops.bnxt_db_rx(rx_ring, pidx);
 	return;
 }
 
diff --git a/sys/dev/bnxt/if_bnxt.c b/sys/dev/bnxt/if_bnxt.c
index a2ec25a9ad31..75f01f556e8d 100644
--- a/sys/dev/bnxt/if_bnxt.c
+++ b/sys/dev/bnxt/if_bnxt.c
@@ -661,6 +661,55 @@ static int bnxt_alloc_hwrm_short_cmd_req(struct bnxt_softc *softc)
 	return rc;
 }
 
+/*
+ * If we update the index, a write barrier is needed after the write to ensure
+ * the completion ring has space before the RX/TX ring does.  Since we can't
+ * make the RX and AG doorbells covered by the same barrier without remapping
+ * MSI-X vectors, we create the barrier over the enture doorbell bar.
+ * TODO: Remap the MSI-X vectors to allow a barrier to only cover the doorbells
+ *       for a single ring group.
+ *
+ * A barrier of just the size of the write is used to ensure the ordering
+ * remains correct and no writes are lost.
+ */
+
+static void bnxt_cuw_db_rx(void *db_ptr, uint16_t idx)
+{
+	struct bnxt_ring *ring = (struct bnxt_ring *) db_ptr;
+	struct bnxt_bar_info *db_bar = &ring->softc->doorbell_bar;
+
+	bus_space_barrier(db_bar->tag, db_bar->handle, ring->doorbell, 4,
+			BUS_SPACE_BARRIER_WRITE);
+	bus_space_write_4(db_bar->tag, db_bar->handle, ring->doorbell,
+			htole32(RX_DOORBELL_KEY_RX | idx));
+}
+
+static void bnxt_cuw_db_tx(void *db_ptr, uint16_t idx)
+{
+	struct bnxt_ring *ring = (struct bnxt_ring *) db_ptr;
+	struct bnxt_bar_info *db_bar = &ring->softc->doorbell_bar;
+
+	bus_space_barrier(db_bar->tag, db_bar->handle, ring->doorbell, 4,
+			BUS_SPACE_BARRIER_WRITE);
+	bus_space_write_4(db_bar->tag, db_bar->handle, ring->doorbell,
+			htole32(TX_DOORBELL_KEY_TX | idx));
+}
+
+static void bnxt_cuw_db_cq(void *db_ptr, bool enable_irq)
+{
+	struct bnxt_cp_ring *cpr = (struct bnxt_cp_ring *) db_ptr;
+	struct bnxt_bar_info *db_bar = &cpr->ring.softc->doorbell_bar;
+
+	bus_space_barrier(db_bar->tag, db_bar->handle, cpr->ring.doorbell, 4,
+			BUS_SPACE_BARRIER_WRITE);
+	bus_space_write_4(db_bar->tag, db_bar->handle, cpr->ring.doorbell,
+			htole32(CMPL_DOORBELL_KEY_CMPL |
+				((cpr->cons == UINT32_MAX) ? 0 :
+				 (cpr->cons | CMPL_DOORBELL_IDX_VALID)) |
+				((enable_irq) ? 0 : CMPL_DOORBELL_MASK)));
+	bus_space_barrier(db_bar->tag, db_bar->handle, 0, db_bar->size,
+			BUS_SPACE_BARRIER_WRITE);
+}
 /* Device setup and teardown */
 static int
 bnxt_attach_pre(if_ctx_t ctx)
@@ -754,6 +803,11 @@ bnxt_attach_pre(if_ctx_t ctx)
 		    &softc->nvm_info->available_size);
 	}
 
+	softc->db_ops.bnxt_db_tx = bnxt_cuw_db_tx;
+	softc->db_ops.bnxt_db_rx = bnxt_cuw_db_rx;
+	softc->db_ops.bnxt_db_rx_cq = bnxt_cuw_db_cq;
+	softc->db_ops.bnxt_db_tx_cq = bnxt_cuw_db_cq;
+
 	/* Register the driver with the FW */
 	rc = bnxt_hwrm_func_drv_rgtr(softc);
 	if (rc) {
@@ -1067,9 +1121,7 @@ bnxt_init(if_ctx_t ctx)
 		    HWRM_NA_SIGNATURE, false);
 		if (rc)
 			goto fail;
-		BNXT_RX_DB(&softc->rx_rings[i], 0);
-		/* TODO: Cumulus+ doesn't need the double doorbell */
-		BNXT_RX_DB(&softc->rx_rings[i], 0);
+		softc->db_ops.bnxt_db_rx(&softc->rx_rings[i], 0);
 
 		/* Allocate the AG ring */
 		rc = bnxt_hwrm_ring_alloc(softc,
@@ -1078,9 +1130,7 @@ bnxt_init(if_ctx_t ctx)
 		    HWRM_NA_SIGNATURE, false);
 		if (rc)
 			goto fail;
-		BNXT_RX_DB(&softc->rx_rings[i], 0);
-		/* TODO: Cumulus+ doesn't need the double doorbell */
-		BNXT_RX_DB(&softc->ag_rings[i], 0);
+		softc->db_ops.bnxt_db_rx(&softc->ag_rings[i], 0);
 
 		/* Allocate the ring group */
 		softc->grp_info[i].stats_ctx =
@@ -1156,9 +1206,7 @@ bnxt_init(if_ctx_t ctx)
 		    softc->tx_cp_rings[i].stats_ctx_id, false);
 		if (rc)
 			goto fail;
-		BNXT_TX_DB(&softc->tx_rings[i], 0);
-		/* TODO: Cumulus+ doesn't need the double doorbell */
-		BNXT_TX_DB(&softc->tx_rings[i], 0);
+		softc->db_ops.bnxt_db_tx(&softc->tx_rings[i], 0);
 	}
 
 	bnxt_do_enable_intr(&softc->def_cp_ring);
@@ -1444,20 +1492,20 @@ bnxt_if_timer(if_ctx_t ctx, uint16_t qid)
 static void inline
 bnxt_do_enable_intr(struct bnxt_cp_ring *cpr)
 {
+	struct bnxt_softc *softc = cpr->ring.softc;
+
 	if (cpr->ring.phys_id != (uint16_t)HWRM_NA_SIGNATURE) {
-		/* First time enabling, do not set index */
-		if (cpr->cons == UINT32_MAX)
-			BNXT_CP_ENABLE_DB(&cpr->ring);
-		else
-			BNXT_CP_IDX_ENABLE_DB(&cpr->ring, cpr->cons);
+		softc->db_ops.bnxt_db_rx_cq(cpr, 1);
 	}
 }
 
 static void inline
 bnxt_do_disable_intr(struct bnxt_cp_ring *cpr)
 {
+	struct bnxt_softc *softc = cpr->ring.softc;
+
 	if (cpr->ring.phys_id != (uint16_t)HWRM_NA_SIGNATURE)
-		BNXT_CP_DISABLE_DB(&cpr->ring);
+		softc->db_ops.bnxt_db_rx_cq(cpr, 0);
 }
 
 /* Enable all interrupts */
@@ -1469,7 +1517,7 @@ bnxt_intr_enable(if_ctx_t ctx)
 
 	bnxt_do_enable_intr(&softc->def_cp_ring);
 	for (i = 0; i < softc->nrxqsets; i++)
-		bnxt_do_enable_intr(&softc->rx_cp_rings[i]);
+		softc->db_ops.bnxt_db_rx_cq(&softc->rx_cp_rings[i], 1);
 
 	return;
 }
@@ -2246,9 +2294,10 @@ static int
 bnxt_handle_rx_cp(void *arg)
 {
 	struct bnxt_cp_ring *cpr = arg;
+	struct bnxt_softc *softc = cpr->ring.softc;
 
 	/* Disable further interrupts for this queue */
-	BNXT_CP_DISABLE_DB(&cpr->ring);
+	softc->db_ops.bnxt_db_rx_cq(cpr, 0);
 	return FILTER_SCHEDULE_THREAD;
 }
 
@@ -2257,7 +2306,7 @@ bnxt_handle_def_cp(void *arg)
 {
 	struct bnxt_softc *softc = arg;
 
-	BNXT_CP_DISABLE_DB(&softc->def_cp_ring.ring);
+	softc->db_ops.bnxt_db_rx_cq(&softc->def_cp_ring, 0);
 	GROUPTASK_ENQUEUE(&softc->def_cp_task);
 	return FILTER_HANDLED;
 }
@@ -2391,7 +2440,7 @@ bnxt_def_cp_task(void *context)
 
 	cpr->cons = last_cons;
 	cpr->v_bit = last_v_bit;
-	BNXT_CP_IDX_ENABLE_DB(&cpr->ring, cpr->cons);
+	softc->db_ops.bnxt_db_rx_cq(cpr, 1);
 }
 
 static uint8_t