From: Stefan Sperling Subject: Re: qwx(4) memory allocations To: Mark Kettenis Cc: mglocker@openbsd.org, tech@openbsd.org Date: Tue, 7 Jul 2026 09:37:48 +0200 On Mon, Jul 06, 2026 at 09:02:39PM +0200, Mark Kettenis wrote: > >From time to time, my AMD x13g4 has trouble bringing qwx(4) up: > > qwx0: failed to init DP: 12 > qwx0: fatal firmware error > qwx0: tx credits timeout > qwx0: failed to send WMI_PDEV_SET_PARAM cmd > qwx0: failed to enable dynamic bw for pdev 0: 35 > > Error code 12 is ENOMEM, so somehow it can't allocate the memory it > needs. Looking at the driver, and especially the codepath that > generates this error, I see a few places that use M_NOWAIT or > BUS_DMA_NOWAIT in code that only gets called from process context. As > far as I can tell at least. So here is a diff that changes some of > those in M_WAITOK or BUS_DMA_WAITOK. > > I also sprinkled a few BUS_DMA_ALLOCNOW in there. These should make > sure that when the time comes to load a dma map, the resources are > already there. So having BUS_DMA_NOWAIT in those calls shouldn't be > an issue. > > Not 100% sure this fixes the issue, but I haven't seen the error yet > with this diff despite a dozen or so reboots. > > ok? ok stsp@ I still intend to move most of the DMA allocations this driver performs to qwx_attach(). But that is not a trivial change, and I lack time for working on it, for now. Since sleeping is allowed in autoconf context I don't think your diff would get in the way of such refactoring in the future. If your diff already avoids the problem in some (or even in most) cases then I am very happy with this change. I also see these errors sometimes. > Index: dev/ic/qwx.c > =================================================================== > RCS file: /cvs/src/sys/dev/ic/qwx.c,v > diff -u -p -r1.126 qwx.c > --- dev/ic/qwx.c 3 Jun 2026 06:59:51 -0000 1.126 > +++ dev/ic/qwx.c 6 Jul 2026 18:54:02 -0000 > @@ -10917,15 +10917,13 @@ qwx_dp_tx_ring_alloc_tx_data(struct qwx_ > int i, ret; > > tx_ring->data = mallocarray(sc->hw_params.tx_ring_size, > - sizeof(struct qwx_tx_data), M_DEVBUF, M_NOWAIT | M_ZERO); > - if (tx_ring->data == NULL) > - return ENOMEM; > + sizeof(struct qwx_tx_data), M_DEVBUF, M_WAITOK | M_ZERO); > > for (i = 0; i < sc->hw_params.tx_ring_size; i++) { > struct qwx_tx_data *tx_data = &tx_ring->data[i]; > > ret = bus_dmamap_create(sc->sc_dmat, MCLBYTES, 1, MCLBYTES, 0, > - BUS_DMA_NOWAIT, &tx_data->map); > + BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW, &tx_data->map); > if (ret) > return ret; > } > @@ -10992,11 +10990,7 @@ qwx_dp_alloc(struct qwx_softc *sc) > dp->tx_ring[i].tx_status_head = 0; > dp->tx_ring[i].tx_status_tail = DP_TX_COMP_RING_SIZE - 1; > dp->tx_ring[i].tx_status = malloc(size, M_DEVBUF, > - M_NOWAIT | M_ZERO); > - if (!dp->tx_ring[i].tx_status) { > - ret = ENOMEM; > - goto fail_cmn_srng_cleanup; > - } > + M_WAITOK | M_ZERO); > } > > for (i = 0; i < HAL_DSCP_TID_MAP_TBL_NUM_ENTRIES_MAX; i++) > @@ -11137,10 +11131,7 @@ qwx_qmi_wlanfw_wlan_cfg_send(struct qwx_ > ce_cfg = sc->hw_params.target_ce_config; > svc_cfg = sc->hw_params.svc_to_ce_map; > > - req = malloc(sizeof(*req), M_DEVBUF, M_NOWAIT | M_ZERO); > - if (!req) > - return ENOMEM; > - > + req = malloc(sizeof(*req), M_DEVBUF, M_WAITOK | M_ZERO); > req->host_version_valid = 1; > strlcpy(req->host_version, ATH11K_HOST_VERSION_STRING, > sizeof(req->host_version)); > @@ -15343,22 +15334,23 @@ qwx_dp_rxdma_ring_buf_setup(struct qwx_s > { > struct qwx_pdev_dp *dp = &sc->pdev_dp; > int num_entries, i; > + int ret; > > num_entries = rx_ring->refill_buf_ring.size / > qwx_hal_srng_get_entrysize(sc, ringtype); > > KASSERT(rx_ring->rx_data == NULL); > rx_ring->rx_data = mallocarray(num_entries, sizeof(rx_ring->rx_data[0]), > - M_DEVBUF, M_NOWAIT | M_ZERO); > - if (rx_ring->rx_data == NULL) > - return ENOMEM; > + M_DEVBUF, M_WAITOK | M_ZERO); > > 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; > + ret = bus_dmamap_create(sc->sc_dmat, DP_RX_BUFFER_SIZE, 1, > + DP_RX_BUFFER_SIZE, 0, BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW, > + &rx_data->map); > + if (ret) > + return ret; > } > > rx_ring->bufs_max = num_entries; > @@ -22428,9 +22420,7 @@ qwx_ce_alloc_src_ring_transfer_contexts( > > /* Allocate an array of qwx_tx_data structures. */ > txdata = mallocarray(pipe->src_ring->nentries, sizeof(*txdata), > - M_DEVBUF, M_NOWAIT | M_ZERO); > - if (txdata == NULL) > - return ENOMEM; > + M_DEVBUF, M_WAITOK | M_ZERO); > > size = sizeof(*txdata) * pipe->src_ring->nentries; > > @@ -22438,7 +22428,8 @@ qwx_ce_alloc_src_ring_transfer_contexts( > for (i = 0; i < pipe->src_ring->nentries; i++) { > struct qwx_tx_data *ctx = &txdata[i]; > ret = bus_dmamap_create(sc->sc_dmat, attr->src_sz_max, 1, > - attr->src_sz_max, 0, BUS_DMA_NOWAIT, &ctx->map); > + attr->src_sz_max, 0, BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW, > + &ctx->map); > if (ret) { > int j; > for (j = 0; j < i; j++) { > @@ -22465,9 +22456,7 @@ qwx_ce_alloc_dest_ring_transfer_contexts > > /* Allocate an array of qwx_rx_data structures. */ > rxdata = mallocarray(pipe->dest_ring->nentries, sizeof(*rxdata), > - M_DEVBUF, M_NOWAIT | M_ZERO); > - if (rxdata == NULL) > - return ENOMEM; > + M_DEVBUF, M_WAITOK | M_ZERO); > > size = sizeof(*rxdata) * pipe->dest_ring->nentries; > > @@ -22475,7 +22464,8 @@ qwx_ce_alloc_dest_ring_transfer_contexts > for (i = 0; i < pipe->dest_ring->nentries; i++) { > struct qwx_rx_data *ctx = &rxdata[i]; > ret = bus_dmamap_create(sc->sc_dmat, attr->src_sz_max, 1, > - attr->src_sz_max, 0, BUS_DMA_NOWAIT, &ctx->map); > + attr->src_sz_max, 0, BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW, > + &ctx->map); > if (ret) { > int j; > for (j = 0; j < i; j++) { > @@ -22499,36 +22489,33 @@ qwx_ce_alloc_ring(struct qwx_softc *sc, > (nentries * sizeof(ce_ring->per_transfer_context[0])); > bus_size_t dsize; > > - ce_ring = malloc(size, M_DEVBUF, M_NOWAIT | M_ZERO); > - if (ce_ring == NULL) > - return NULL; > - > + ce_ring = malloc(size, M_DEVBUF, M_WAITOK | M_ZERO); > ce_ring->nentries = nentries; > ce_ring->nentries_mask = nentries - 1; > ce_ring->desc_sz = desc_sz; > > dsize = nentries * desc_sz; > - if (bus_dmamap_create(sc->sc_dmat, dsize, 1, dsize, 0, BUS_DMA_NOWAIT, > - &ce_ring->dmap)) { > + if (bus_dmamap_create(sc->sc_dmat, dsize, 1, dsize, 0, > + BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW, &ce_ring->dmap)) { > free(ce_ring, M_DEVBUF, size); > return NULL; > } > > if (bus_dmamem_alloc(sc->sc_dmat, dsize, CE_DESC_RING_ALIGN, 0, > &ce_ring->dsegs, 1, &ce_ring->nsegs, > - BUS_DMA_NOWAIT | BUS_DMA_ZERO)) { > + BUS_DMA_WAITOK | BUS_DMA_ZERO)) { > qwx_ce_free_ring(sc, ce_ring); > return NULL; > } > > if (bus_dmamem_map(sc->sc_dmat, &ce_ring->dsegs, 1, dsize, > - &ce_ring->base_addr, BUS_DMA_NOWAIT | BUS_DMA_COHERENT)) { > + &ce_ring->base_addr, BUS_DMA_WAITOK | BUS_DMA_COHERENT)) { > qwx_ce_free_ring(sc, ce_ring); > return NULL; > } > > if (bus_dmamap_load_raw(sc->sc_dmat, ce_ring->dmap, &ce_ring->dsegs, > - ce_ring->nsegs, dsize, BUS_DMA_NOWAIT)) { > + ce_ring->nsegs, dsize, BUS_DMA_WAITOK)) { > qwx_ce_free_ring(sc, ce_ring); > return NULL; > } > > > > > >