git: a10151fa662c - main - aq(4): take F/W statistics off the iflib core lock (kick-and-read)

From: Adrian Chadd <adrian_at_FreeBSD.org>
Date: Sat, 20 Jun 2026 19:10:41 UTC
The branch main has been updated by adrian:

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

commit a10151fa662c1d370861154e2f83e88a20be149c
Author:     Nick Price <nick@spun.io>
AuthorDate: 2026-06-20 19:03:18 +0000
Commit:     Adrian Chadd <adrian@FreeBSD.org>
CommitDate: 2026-06-20 19:10:16 +0000

    aq(4): take F/W statistics off the iflib core lock (kick-and-read)
    
    The once-per-second statistics refresh ran the whole F/W-mailbox
    transaction under iflib's CTX (sx) lock: fw2x_get_stats toggled the MPI
    STATISTICS control bit and busy-polled the state register for the
    acknowledgement (up to ~25 ms) before downloading the counters, so a slow
    F/W response blocked datapath reconfigure / ioctls for the duration.
    
    The per-cast and error counters have no direct-register source -- the
    reference Linux atlantic driver and our port both read them out of the
    F/W mailbox, and the MSM registers the chip exposes are never used for the
    periodic counters.  So rather than poll, adopt the kick-and-read shape the
    iflib peer with the same constraint uses (vmxnet3): consume the snapshot
    the F/W produced for the *previous* request, then toggle the bit to
    request the next one -- no wait.  The F/W finished that previous refresh
    ~1 s ago, so the download needs no poll, and the toggle write stays
    serialized against set_mode by the CTX lock exactly as before.  This
    removes the 25 ms poll (and the toggle_mpi_ctrl_and_wait_ helper) from
    under the lock; only the fast 16-dword download remains.
    
    Cost: the counters lag one 1 s cycle, invisible for monitoring, and a torn
    read is already rejected by aq_update_hw_stats' monotonic-delta check.
    
    Validated on AQC107: a fixed 500 MiB RX transfer advances good_octets_rcvd
    by 549.6 MB -- 500 MiB plus the ~4.8% Ethernet framing overhead -- with
    rx_err=0 and traffic at line rate.
    
    Reviewed by:    adrian
    Differential Revision:  https://reviews.freebsd.org/D57439
---
 sys/dev/aq/aq_fw2x.c | 61 ++++++----------------------------------------------
 1 file changed, 7 insertions(+), 54 deletions(-)

diff --git a/sys/dev/aq/aq_fw2x.c b/sys/dev/aq/aq_fw2x.c
index 04935e9368d4..339644d0c8e1 100644
--- a/sys/dev/aq/aq_fw2x.c
+++ b/sys/dev/aq/aq_fw2x.c
@@ -422,69 +422,19 @@ fw2x_stats_to_fw_stats_(struct aq_hw_stats* dst,
 }
 
 
-static bool
-toggle_mpi_ctrl_and_wait_(struct aq_hw* hw, uint64_t mask, uint32_t timeout_ms,
-    uint32_t try_count)
-{
-	uint64_t ctrl = get_mpi_ctrl_(hw);
-	uint64_t state = get_mpi_state_(hw);
-
- //   AQ_DBG_ENTER();
-	// First, check that control and state values are consistent
-	if ((ctrl & mask) != (state & mask)) {
-		trace_warn(dbg_fw,
-		    "fw2x> MPI control (%#llx) and state (%#llx) are not consistent for mask %#llx!",
-		    (unsigned long long)ctrl, (unsigned long long)state,
-		    (unsigned long long)mask);
-		AQ_DBG_EXIT(false);
-		return (false);
-	}
-
-	// Invert bits (toggle) in control register
-	ctrl ^= mask;
-	set_mpi_ctrl_(hw, ctrl);
-
-	// Clear all bits except masked
-	ctrl &= mask;
-
-	// Wait for FW reflecting change in state register
-	while (try_count-- != 0) {
-		if ((get_mpi_state_(hw) & mask) == ctrl)
-		{
-//			AQ_DBG_EXIT(true);
-			return (true);
-		}
-		DELAY((timeout_ms) * 1000);
-	}
-
-	trace_detail(dbg_fw,
-	    "f/w2x> timeout while waiting for response in state register for bit %#llx!",
-	    (unsigned long long)mask);
- //   AQ_DBG_EXIT(false);
-	return (false);
-}
-
-
 int
 fw2x_get_stats(struct aq_hw* hw, struct aq_hw_stats* stats)
 {
-	int err = 0;
 	struct fw2x_msm_statistics fw2x_stats = {0};
-
-//    AQ_DBG_ENTER();
+	uint64_t mpi_ctrl;
+	int err;
 
 	if ((hw->fw_caps & FW2X_CAP_STATISTICS) == 0) {
 		trace_warn(dbg_fw, "fw2x> statistics not supported by F/W");
 		return (ENOTSUP);
 	}
 
-	// Tell F/W to update the statistics.
-	if (!toggle_mpi_ctrl_and_wait_(hw, FW2X_CAP_STATISTICS, 1, 25)) {
-		trace_error(dbg_fw, "fw2x> statistics update timeout");
-		AQ_DBG_EXIT(ETIMEDOUT);
-		return (ETIMEDOUT);
-	}
-
+	/* Kick-and-read: take the F/W's previous snapshot, request the next. */
 	err = aq_hw_fw_downld_dwords(hw,
 	    hw->mbox_addr + offsetof(struct fw2x_mailbox, msm),
 	    (uint32_t*)&fw2x_stats, sizeof fw2x_stats/sizeof(uint32_t));
@@ -495,7 +445,10 @@ fw2x_get_stats(struct aq_hw* hw, struct aq_hw_stats* stats)
 		trace_error(dbg_fw,
 		    "fw2x> download statistics data FAILED, error %d", err);
 
-//    AQ_DBG_EXIT(err);
+	mpi_ctrl = get_mpi_ctrl_(hw);
+	mpi_ctrl ^= FW2X_CAP_STATISTICS;
+	set_mpi_ctrl_(hw, mpi_ctrl);
+
 	return (err);
 }