Re: qwx unhappy -> splassert: vm_map_assert_wrlock_ln: want 0 have 4

Stefan Sperling <[email protected]> Thu, 14 May 2026 23:27:39 +0200
Newsgroups gmane.os.openbsd.bugs
Message-ID <[email protected]>
On Thu, May 14, 2026 at 02:12:00PM -0700, Greg Steuck wrote:
> I just installed a new snap and it failed to get qwx working in
> GENERIC.MP upon first boot. I cycled it with ifconfig down/up to no
> avail. At reboot time I got the splassert above. After the second
> reboot it works. I include dmesg from both boots as they carry some qwx
> diagnostics.

Please try the diff below.

It won't fix every problem with this driver but the splasserts will
hopefully go away.


 keep Rx TID DMA memory allocated until interface goes down
 
M  sys/dev/ic/qwx.c     |  46+  63-
M  sys/dev/ic/qwxvar.h  |  12+   1-

2 files changed, 58 insertions(+), 64 deletions(-)

commit - adef1dd91f575dd2c7b99c729694525977f0fd7c
commit + c457050a09cca09d7184ab44e16388b16d46fbb3
blob - 333da499f818b16c992fbff9251463a35b7b7097
blob + bf3de0104bc7c4df8f8f5215172839879578cff7
--- sys/dev/ic/qwx.c
+++ sys/dev/ic/qwx.c
@@ -10994,6 +10994,15 @@ fail_link_desc_cleanup:
 }
 
 void
+qwx_dp_rx_tid_clear(struct qwx_softc *sc, struct dp_rx_tid *rx_tid)
+{
+	rx_tid->mem = NULL;
+	rx_tid->vaddr = NULL;
+	rx_tid->paddr = 0ULL;
+	rx_tid->size = 0;
+}
+
+void
 qwx_dp_reo_cmd_list_cleanup(struct qwx_softc *sc)
 {
 	struct qwx_dp *dp = &sc->dp;
@@ -11006,13 +11015,7 @@ qwx_dp_reo_cmd_list_cleanup(struct qwx_softc *sc)
 	TAILQ_FOREACH_SAFE(cmd, &dp->reo_cmd_list, entry, tmp) {
 		TAILQ_REMOVE(&dp->reo_cmd_list, cmd, entry);
 		rx_tid = &cmd->data;
-		if (rx_tid->mem) {
-			qwx_dmamem_free(sc->sc_dmat, rx_tid->mem);
-			rx_tid->mem = NULL;
-			rx_tid->vaddr = NULL;
-			rx_tid->paddr = 0ULL;
-			rx_tid->size = 0;
-		}
+		qwx_dp_rx_tid_clear(sc, rx_tid);
 		free(cmd, M_DEVBUF, sizeof(*cmd));
 	}
 
@@ -11021,13 +11024,7 @@ qwx_dp_reo_cmd_list_cleanup(struct qwx_softc *sc)
 		TAILQ_REMOVE(&dp->reo_cmd_cache_flush_list, cmd_cache, entry);
 		dp->reo_cmd_cache_flush_count--;
 		rx_tid = &cmd_cache->data;
-		if (rx_tid->mem) {
-			qwx_dmamem_free(sc->sc_dmat, rx_tid->mem);
-			rx_tid->mem = NULL;
-			rx_tid->vaddr = NULL;
-			rx_tid->paddr = 0ULL;
-			rx_tid->size = 0;
-		}
+		qwx_dp_rx_tid_clear(sc, rx_tid);
 		free(cmd_cache, M_DEVBUF, sizeof(*cmd_cache));
 	}
 #ifdef notyet
@@ -11060,6 +11057,13 @@ qwx_dp_free(struct qwx_softc *sc)
 		dp->tx_ring[i].tx_status = NULL;
 	}
 
+	for (i = 0; i <= HAL_DESC_REO_NON_QOS_TID; i++) {
+		if (dp->rx_tid_mem[i]) {
+			qwx_dmamem_free(sc->sc_dmat, dp->rx_tid_mem[i]);
+			dp->rx_tid_mem[i] = NULL;
+		}
+	}
+
 	/* Deinit any SOC level resource */
 }
 
@@ -24516,13 +24520,7 @@ qwx_dp_reo_cmd_free(struct qwx_dp *dp, void *ctx,
 		printf("%s: failed to flush rx tid hw desc, tid %d status %d\n",
 		    sc->sc_dev.dv_xname, rx_tid->tid, status);
 
-	if (rx_tid->mem) {
-		qwx_dmamem_free(sc->sc_dmat, rx_tid->mem);
-		rx_tid->mem = NULL;
-		rx_tid->vaddr = NULL;
-		rx_tid->paddr = 0ULL;
-		rx_tid->size = 0;
-	}
+	qwx_dp_rx_tid_clear(sc, rx_tid);
 }
 
 void
@@ -24557,13 +24555,7 @@ qwx_dp_reo_cache_flush(struct qwx_softc *sc, struct dp
 	if (ret) {
 		printf("%s: failed to send HAL_REO_CMD_FLUSH_CACHE cmd, "
 		    "tid %d (%d)\n", sc->sc_dev.dv_xname, rx_tid->tid, ret);
-		if (rx_tid->mem) {
-			qwx_dmamem_free(sc->sc_dmat, rx_tid->mem);
-			rx_tid->mem = NULL;
-			rx_tid->vaddr = NULL;
-			rx_tid->paddr = 0ULL;
-			rx_tid->size = 0;
-		}
+		qwx_dp_rx_tid_clear(sc, rx_tid);
 	}
 }
 
@@ -24577,7 +24569,8 @@ qwx_dp_rx_tid_del_func(struct qwx_dp *dp, void *ctx,
 	uint64_t now;
 
 	if (status == HAL_REO_CMD_DRAIN) {
-		goto free_desc;
+		qwx_dp_rx_tid_clear(sc, rx_tid);
+		return;
 	} else if (status != HAL_REO_CMD_SUCCESS) {
 		/* Shouldn't happen! Cleanup in case of other failure? */
 		printf("%s: failed to delete rx tid %d hw descriptor %d\n",
@@ -24586,17 +24579,16 @@ qwx_dp_rx_tid_del_func(struct qwx_dp *dp, void *ctx,
 	}
 
 	elem = malloc(sizeof(*elem), M_DEVBUF, M_ZERO | M_NOWAIT);
-	if (!elem)
-		goto free_desc;
+	if (!elem) {
+		qwx_dp_rx_tid_clear(sc, rx_tid);
+		return;
+	}
 
 	now = getnsecuptime();
 	elem->ts = now;
 	memcpy(&elem->data, rx_tid, sizeof(*rx_tid));
 
-	rx_tid->mem = NULL;
-	rx_tid->vaddr = NULL;
-	rx_tid->paddr = 0ULL;
-	rx_tid->size = 0;
+	qwx_dp_rx_tid_clear(sc, rx_tid);
 
 #ifdef notyet
 	spin_lock_bh(&dp->reo_cmd_lock);
@@ -24623,15 +24615,6 @@ qwx_dp_rx_tid_del_func(struct qwx_dp *dp, void *ctx,
 #ifdef notyet
 	spin_unlock_bh(&dp->reo_cmd_lock);
 #endif
-	return;
-free_desc:
-	if (rx_tid->mem) {
-		qwx_dmamem_free(sc->sc_dmat, rx_tid->mem);
-		rx_tid->mem = NULL;
-		rx_tid->vaddr = NULL;
-		rx_tid->paddr = 0ULL;
-		rx_tid->size = 0;
-	}
 }
 
 void
@@ -24660,13 +24643,7 @@ qwx_peer_rx_tid_delete(struct qwx_softc *sc, struct at
 			    sc->sc_dev.dv_xname, tid, ret);
 		}
 
-		if (rx_tid->mem) {
-			qwx_dmamem_free(sc->sc_dmat, rx_tid->mem);
-			rx_tid->mem = NULL;
-			rx_tid->vaddr = NULL;
-			rx_tid->paddr = 0ULL;
-			rx_tid->size = 0;
-		}
+		qwx_dp_rx_tid_clear(sc, rx_tid);
 	}
 }
 
@@ -24781,13 +24758,11 @@ qwx_dp_rx_tid_mem_free(struct qwx_softc *sc, struct ie
 	if (peer) {
 		rx_tid = &peer->rx_tid[tid];
 
-		if (rx_tid->mem) {
-			qwx_dmamem_free(sc->sc_dmat, rx_tid->mem);
-			rx_tid->mem = NULL;
-			rx_tid->vaddr = NULL;
-			rx_tid->paddr = 0ULL;
-			rx_tid->size = 0;
-		}
+		/*
+		 * Nothing to free here since DMA memory pointers are
+		 * managed as part of qwx_softc.
+		 */
+		qwx_dp_rx_tid_clear(sc, rx_tid);
 
 		rx_tid->active = 0;
 	}
@@ -24801,6 +24776,7 @@ qwx_peer_rx_tid_setup(struct qwx_softc *sc, struct iee
     int vdev_id, int pdev_id, uint8_t tid, uint32_t ba_win_sz, uint16_t ssn,
     enum hal_pn_type pn_type)
 {
+	struct qwx_dp *dp = &sc->dp;
 	struct qwx_node *nq = (struct qwx_node *)ni;
 	struct ath11k_peer *peer;
 	struct dp_rx_tid *rx_tid;
@@ -24856,19 +24832,26 @@ qwx_peer_rx_tid_setup(struct qwx_softc *sc, struct iee
 	else
 		hw_desc_sz = qwx_hal_reo_qdesc_size(DP_BA_WIN_SZ_MAX, tid);
 
-	rx_tid->mem = qwx_dmamem_alloc(sc->sc_dmat, hw_desc_sz,
-	    HAL_LINK_DESC_ALIGN);
-	if (rx_tid->mem == NULL) {
+	if (dp->rx_tid_mem[tid] == NULL) {
+		dp->rx_tid_mem[tid] = qwx_dmamem_alloc(sc->sc_dmat, hw_desc_sz,
+		    HAL_LINK_DESC_ALIGN);
+		if (dp->rx_tid_mem[tid] == NULL) {
 #ifdef notyet
-		spin_unlock_bh(&ab->base_lock);
+			spin_unlock_bh(&ab->base_lock);
 #endif
-		return ENOMEM;
+			return ENOMEM;
+		}
 	}
 
+	rx_tid->mem = dp->rx_tid_mem[tid];
+
 	vaddr = QWX_DMA_KVA(rx_tid->mem);
 
 	qwx_hal_reo_qdesc_setup(vaddr, tid, ba_win_sz, ssn, pn_type);
 
+	bus_dmamap_sync(sc->sc_dmat, rx_tid->mem->map,
+	    0, rx_tid->mem->size, BUS_DMASYNC_PREREAD);
+
 	paddr = QWX_DMA_DVA(rx_tid->mem);
 
 	rx_tid->vaddr = vaddr;
blob - 1d0699c9aec0fe9f6ed45a564cadda2a93de6937
blob + 636c2086885099148cf7db56fe361d313d05e517
--- sys/dev/ic/qwxvar.h
+++ sys/dev/ic/qwxvar.h
@@ -1012,7 +1012,7 @@ struct qwx_hp_update_timer {
 
 struct dp_rx_tid {
 	uint8_t tid;
-	struct qwx_dmamem *mem;
+	const struct qwx_dmamem *mem;
 	uint32_t *vaddr;
 	uint64_t paddr;
 	uint32_t size;
@@ -1131,6 +1131,17 @@ struct qwx_dp {
 #endif
 	struct qwx_hp_update_timer reo_cmd_timer;
 	struct qwx_hp_update_timer tx_ring_timer[DP_TCL_NUM_RING_MAX];
+
+	/*
+	 * Cache of DMA memory regions used for Rx aggregation.
+	 * We used to free these DMA allocations in interrupt context but
+	 * destroying DMA memory in interrupt context is not allowed.
+	 *
+	 * This array contains enough entries for client station mode.
+	 * It will need to grow in order to support multiple clients if
+	 * support for HostAP mode gets added to the driver.
+	 */
+	struct qwx_dmamem *rx_tid_mem[HAL_DESC_REO_NON_QOS_TID + 1];
 };
 
 #define ATH11K_SHADOW_DP_TIMER_INTERVAL 20