Re: qwx(4) memory allocations

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;
>  	}
> 
> 
> 
> 
> 
>