From: Stefan Sperling <stsp@stsp.name>
Subject: small set of qwx bug fixes
To: tech@openbsd.org
Date: Sun, 21 Jun 2026 15:00:52 +0200

I found 3 distinct bugs in qwx while looking for causes of qwx instability
across suspend/resume cycles.
Not sure yet if these fixes really make suspend/resume more stable. Across
about a dozen syspend/resume cycles I couldn't trigger problem anymore
with these changes applied. But it might just be down to luck.

1. Ensure that rx_ring->bufs_max and freemap are always initialized correctly.
   If bus_dmamap_create() fails part-way during the loop then we leave
   bufs_max set to a zero value, and the cleanup path won't free any maps.

2. The init task should not wait for the ioctl lock to become available.
   The task could trigger a pointless down/up cycle if the rwlock is
   already contended by ifconfig from userland or by the ACPI thread.

3. Do not ignore errors which occur in qwx_qmi_fw_init_done().
   Any errors were effectively ignored, including DMA allocation failures
   in callees of qwx_qmi_fw_init_done().

ok?

M  sys/dev/ic/qwx.c  |  17+  11-

1 file changed, 17 insertions(+), 11 deletions(-)

commit - 510a3c8701b503ed3d6bbf6e2c2c1d66315cb14c
commit + 8c088bd41d300a603d5464ec5cf71694983b8b78
blob - 4c744b205a09dec658faed98adb3e4e7a7f557be
blob + c2526028507bb5d92bd5d4f6924ac90d01bd51dd
--- sys/dev/ic/qwx.c
+++ sys/dev/ic/qwx.c
@@ -15352,19 +15352,19 @@ qwx_dp_rxdma_ring_buf_setup(struct qwx_softc *sc,
 	    M_DEVBUF, M_NOWAIT | M_ZERO);
 	if (rx_ring->rx_data == NULL)
 		return ENOMEM;
 
+	rx_ring->bufs_max = num_entries;
+	memset(rx_ring->freemap, 0xff, sizeof(rx_ring->freemap));
+
 	for (i = 0; i < num_entries; i++) {
 		struct qwx_rx_data *rx_data = &rx_ring->rx_data[i];
 
 		if (bus_dmamap_create(sc->sc_dmat, DP_RX_BUFFER_SIZE, 1,
 		    DP_RX_BUFFER_SIZE, 0, BUS_DMA_NOWAIT, &rx_data->map))
 			return ENOMEM;
 	}
 
-	rx_ring->bufs_max = num_entries;
-	memset(rx_ring->freemap, 0xff, sizeof(rx_ring->freemap));
-
 	return qwx_dp_rxbufs_replenish(sc, dp->mac_id, rx_ring, num_entries,
 	    sc->hw_params.hal_params->rx_buf_rbm);
 }
 
@@ -20969,9 +20969,9 @@ err_firmware_stop:
 
 	return ret;
 }
 
-void
+int
 qwx_qmi_fw_init_done(struct qwx_softc *sc)
 {
 	int ret = 0;
 
@@ -20982,13 +20982,11 @@ qwx_qmi_fw_init_done(struct qwx_softc *sc)
 	} else {
 		clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags);
 		clear_bit(ATH11K_FLAG_RECOVERY, sc->sc_flags);
 		ret = qwx_core_qmi_firmware_ready(sc);
-		if (ret) {
-			set_bit(ATH11K_FLAG_QMI_FAIL, sc->sc_flags);
-			return;
-		}
 	}
+
+	return ret;
 }
 
 int
 qwx_qmi_event_server_arrive(struct qwx_softc *sc)
@@ -21046,10 +21044,9 @@ qwx_qmi_event_server_arrive(struct qwx_softc *sc)
 			return -1;
 		}
 	}
 
-	qwx_qmi_fw_init_done(sc);
-	return 0;
+	return qwx_qmi_fw_init_done(sc);
 }
 
 int
 qwx_core_init(struct qwx_softc *sc)
@@ -24322,10 +24319,19 @@ qwx_init_task(void *arg)
 {
 	struct qwx_softc *sc = arg;
 	struct ifnet *ifp = &sc->sc_ic.ic_if;
 	int s = splnet();
-	rw_enter_write(&sc->ioctl_rwl);
 
+	/*
+	 * Do not sleep for this lock. The init task is a one-shot
+	 * recovery mechanism. If the ioctl handler is busy then
+	 * we are being reconfigured or reset already.
+	 */
+	if (rw_enter(&sc->ioctl_rwl, RW_WRITE | RW_NOSLEEP) != 0) {
+		splx(s);
+		return;
+	}
+
 	if (ifp->if_flags & IFF_RUNNING)
 		qwx_stop(ifp);
 
 	if ((ifp->if_flags & (IFF_UP | IFF_RUNNING)) == IFF_UP)