Re: implement qwx_dp_rx_h_null_q_desc()

Kevin Lo <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
On Sat, May 30, 2026 at 03:01:32PM +0200, Stefan Sperling wrote:
> 
> This function prints "not implemented" in dmesg for some people.
> Implement it to avoid the noise and actually handle the error condition.

Tested on the X13s:

qwx0 at pci1 dev 0 function 0 "Qualcomm QCNFA765" rev 0x01: msi
qwx0: wcn6855 hw2.1 fw 0x11088c35 address xx:xx:xx:xx:xx:xx

No regressions observed in my testing, thanks.

> ok?

ok kevlo@

> M  sys/dev/ic/qwx.c     |  124+  44-
> M  sys/dev/ic/qwxvar.h  |    1+   5-
> 
> 2 files changed, 125 insertions(+), 49 deletions(-)
> 
> commit - df013094d758e1986e1a9698533920fe7c095993
> commit + 6fc05dd3ca79c618525bd4157809ef937ee7fa59
> blob - 2dc4ad99a17b4ea9165e041fb26d3ec5bc7bde3e
> blob + 60f91e880e7572f76c49b2bb26aac0c3449a1fc5
> --- sys/dev/ic/qwx.c
> +++ sys/dev/ic/qwx.c
> @@ -173,6 +173,8 @@ void qwx_dp_stop_shadow_timers(struct qwx_softc *);
>  void qwx_ce_stop_shadow_timers(struct qwx_softc *);
>  int qwx_wmi_vdev_set_param_cmd(struct qwx_softc *, uint32_t, uint8_t,
>      uint32_t, uint32_t);
> +int qwx_dp_rx_h_null_q_desc(struct qwx_softc *, struct qwx_rx_msdu *,
> +    struct qwx_rx_msdu_list *);
>  
>  int qwx_scan(struct qwx_softc *, int);
>  void qwx_scan_abort(struct qwx_softc *);
> @@ -1500,6 +1502,13 @@ qwx_hw_ipq8074_rx_desc_get_first_msdu(struct hal_rx_de
>  	    le32toh(desc->u.ipq8074.msdu_end.info2));
>  }
>  
> +int
> +qwx_hw_ipq8074_rx_desc_get_last_msdu(struct hal_rx_desc *desc)
> +{
> +	return !!FIELD_GET(RX_MSDU_END_INFO2_LAST_MSDU,
> +			   le32toh(desc->u.ipq8074.msdu_end.info2));
> +}
> +
>  uint8_t
>  qwx_hw_ipq8074_rx_desc_get_l3_pad_bytes(struct hal_rx_desc *desc)
>  {
> @@ -1673,6 +1682,12 @@ qwx_dp_rx_h_msdu_end_first_msdu(struct qwx_softc *sc, 
>  }
>  
>  int
> +qwx_dp_rx_h_msdu_end_last_msdu(struct qwx_softc *sc, struct hal_rx_desc *desc)
> +{
> +	return sc->hw_params.hw_ops->rx_desc_get_last_msdu(desc);
> +}
> +
> +int
>  qwx_hw_ipq8074_rx_desc_mac_addr2_valid(struct hal_rx_desc *desc)
>  {
>  	return le32toh(desc->u.ipq8074.mpdu_start.info1) &
> @@ -2140,9 +2155,7 @@ const struct ath11k_hw_ops ipq8074_ops = {
>  	.tx_mesh_enable = ath11k_hw_ipq8074_tx_mesh_enable,
>  #endif
>  	.rx_desc_get_first_msdu = qwx_hw_ipq8074_rx_desc_get_first_msdu,
> -#if notyet
> -	.rx_desc_get_last_msdu = ath11k_hw_ipq8074_rx_desc_get_last_msdu,
> -#endif
> +	.rx_desc_get_last_msdu = qwx_hw_ipq8074_rx_desc_get_last_msdu,
>  	.rx_desc_get_l3_pad_bytes = qwx_hw_ipq8074_rx_desc_get_l3_pad_bytes,
>  	.rx_desc_get_hdr_status = qwx_hw_ipq8074_rx_desc_get_hdr_status,
>  	.rx_desc_encrypt_valid = qwx_hw_ipq8074_rx_desc_encrypt_valid,
> @@ -2164,9 +2177,7 @@ const struct ath11k_hw_ops ipq8074_ops = {
>  	.rx_desc_get_msdu_pkt_type = ath11k_hw_ipq8074_rx_desc_get_msdu_pkt_type,
>  #endif
>  	.rx_desc_get_msdu_nss = qwx_hw_ipq8074_rx_desc_get_msdu_nss,
> -#ifdef notyet
> -	.rx_desc_get_mpdu_tid = ath11k_hw_ipq8074_rx_desc_get_mpdu_tid,
> -#endif
> +	.rx_desc_get_mpdu_tid = qwx_hw_ipq8074_rx_desc_get_mpdu_tid,
>  	.rx_desc_get_mpdu_peer_id = qwx_hw_ipq8074_rx_desc_get_mpdu_peer_id,
>  #if 0
>  	.rx_desc_copy_attn_end_tlv = ath11k_hw_ipq8074_rx_desc_copy_attn_end,
> @@ -2196,9 +2207,7 @@ const struct ath11k_hw_ops ipq6018_ops = {
>  	.tx_mesh_enable = ath11k_hw_ipq8074_tx_mesh_enable,
>  #endif
>  	.rx_desc_get_first_msdu = qwx_hw_ipq8074_rx_desc_get_first_msdu,
> -#if notyet
> -	.rx_desc_get_last_msdu = ath11k_hw_ipq8074_rx_desc_get_last_msdu,
> -#endif
> +	.rx_desc_get_last_msdu = qwx_hw_ipq8074_rx_desc_get_last_msdu,
>  	.rx_desc_get_l3_pad_bytes = qwx_hw_ipq8074_rx_desc_get_l3_pad_bytes,
>  	.rx_desc_get_hdr_status = qwx_hw_ipq8074_rx_desc_get_hdr_status,
>  	.rx_desc_encrypt_valid = qwx_hw_ipq8074_rx_desc_encrypt_valid,
> @@ -2218,9 +2227,7 @@ const struct ath11k_hw_ops ipq6018_ops = {
>  	.rx_desc_get_msdu_freq = qwx_hw_ipq8074_rx_desc_get_msdu_freq,
>  	.rx_desc_get_msdu_pkt_type = qwx_hw_ipq8074_rx_desc_get_msdu_pkt_type,
>  	.rx_desc_get_msdu_nss = qwx_hw_ipq8074_rx_desc_get_msdu_nss,
> -#ifdef notyet
> -	.rx_desc_get_mpdu_tid = ath11k_hw_ipq8074_rx_desc_get_mpdu_tid,
> -#endif
> +	.rx_desc_get_mpdu_tid = qwx_hw_ipq8074_rx_desc_get_mpdu_tid,
>  	.rx_desc_get_mpdu_peer_id = qwx_hw_ipq8074_rx_desc_get_mpdu_peer_id,
>  #if 0
>  	.rx_desc_copy_attn_end_tlv = ath11k_hw_ipq8074_rx_desc_copy_attn_end,
> @@ -2250,9 +2257,7 @@ const struct ath11k_hw_ops qca6390_ops = {
>  	.tx_mesh_enable = ath11k_hw_ipq8074_tx_mesh_enable,
>  #endif
>  	.rx_desc_get_first_msdu = qwx_hw_ipq8074_rx_desc_get_first_msdu,
> -#if notyet
> -	.rx_desc_get_last_msdu = ath11k_hw_ipq8074_rx_desc_get_last_msdu,
> -#endif
> +	.rx_desc_get_last_msdu = qwx_hw_ipq8074_rx_desc_get_last_msdu,
>  	.rx_desc_get_l3_pad_bytes = qwx_hw_ipq8074_rx_desc_get_l3_pad_bytes,
>  	.rx_desc_get_hdr_status = qwx_hw_ipq8074_rx_desc_get_hdr_status,
>  	.rx_desc_encrypt_valid = qwx_hw_ipq8074_rx_desc_encrypt_valid,
> @@ -2272,9 +2277,7 @@ const struct ath11k_hw_ops qca6390_ops = {
>  	.rx_desc_get_msdu_freq = qwx_hw_ipq8074_rx_desc_get_msdu_freq,
>  	.rx_desc_get_msdu_pkt_type = qwx_hw_ipq8074_rx_desc_get_msdu_pkt_type,
>  	.rx_desc_get_msdu_nss = qwx_hw_ipq8074_rx_desc_get_msdu_nss,
> -#ifdef notyet
> -	.rx_desc_get_mpdu_tid = ath11k_hw_ipq8074_rx_desc_get_mpdu_tid,
> -#endif
> +	.rx_desc_get_mpdu_tid = qwx_hw_ipq8074_rx_desc_get_mpdu_tid,
>  	.rx_desc_get_mpdu_peer_id = qwx_hw_ipq8074_rx_desc_get_mpdu_peer_id,
>  #if 0
>  	.rx_desc_copy_attn_end_tlv = ath11k_hw_ipq8074_rx_desc_copy_attn_end,
> @@ -2304,9 +2307,7 @@ const struct ath11k_hw_ops qcn9074_ops = {
>  	.tx_mesh_enable = ath11k_hw_qcn9074_tx_mesh_enable,
>  #endif
>  	.rx_desc_get_first_msdu = qwx_hw_qcn9074_rx_desc_get_first_msdu,
> -#if notyet
> -	.rx_desc_get_last_msdu = ath11k_hw_qcn9074_rx_desc_get_last_msdu,
> -#endif
> +	.rx_desc_get_last_msdu = qwx_hw_qcn9074_rx_desc_get_last_msdu,
>  	.rx_desc_get_l3_pad_bytes = qwx_hw_qcn9074_rx_desc_get_l3_pad_bytes,
>  	.rx_desc_get_hdr_status = qwx_hw_qcn9074_rx_desc_get_hdr_status,
>  	.rx_desc_encrypt_valid = qwx_hw_qcn9074_rx_desc_encrypt_valid,
> @@ -2326,9 +2327,7 @@ const struct ath11k_hw_ops qcn9074_ops = {
>  	.rx_desc_get_msdu_freq = qwx_hw_qcn9074_rx_desc_get_msdu_freq,
>  	.rx_desc_get_msdu_pkt_type = qwx_hw_qcn9074_rx_desc_get_msdu_pkt_type,
>  	.rx_desc_get_msdu_nss = qwx_hw_qcn9074_rx_desc_get_msdu_nss,
> -#ifdef notyet
> -	.rx_desc_get_mpdu_tid = ath11k_hw_qcn9074_rx_desc_get_mpdu_tid,
> -#endif
> +	.rx_desc_get_mpdu_tid = qwx_hw_qcn9074_rx_desc_get_mpdu_tid,
>  	.rx_desc_get_mpdu_peer_id = qwx_hw_qcn9074_rx_desc_get_mpdu_peer_id,
>  #if 0
>  	.rx_desc_copy_attn_end_tlv = ath11k_hw_qcn9074_rx_desc_copy_attn_end,
> @@ -2358,9 +2357,7 @@ const struct ath11k_hw_ops wcn6855_ops = {
>  	.tx_mesh_enable = ath11k_hw_wcn6855_tx_mesh_enable,
>  #endif
>  	.rx_desc_get_first_msdu = qwx_hw_wcn6855_rx_desc_get_first_msdu,
> -#if notyet
> -	.rx_desc_get_last_msdu = ath11k_hw_wcn6855_rx_desc_get_last_msdu,
> -#endif
> +	.rx_desc_get_last_msdu = qwx_hw_wcn6855_rx_desc_get_last_msdu,
>  	.rx_desc_get_l3_pad_bytes = qwx_hw_wcn6855_rx_desc_get_l3_pad_bytes,
>  	.rx_desc_get_hdr_status = qwx_hw_wcn6855_rx_desc_get_hdr_status,
>  	.rx_desc_encrypt_valid = qwx_hw_wcn6855_rx_desc_encrypt_valid,
> @@ -2380,9 +2377,7 @@ const struct ath11k_hw_ops wcn6855_ops = {
>  	.rx_desc_get_msdu_freq = qwx_hw_wcn6855_rx_desc_get_msdu_freq,
>  	.rx_desc_get_msdu_pkt_type = qwx_hw_wcn6855_rx_desc_get_msdu_pkt_type,
>  	.rx_desc_get_msdu_nss = qwx_hw_wcn6855_rx_desc_get_msdu_nss,
> -#ifdef notyet
> -	.rx_desc_get_mpdu_tid = ath11k_hw_wcn6855_rx_desc_get_mpdu_tid,
> -#endif
> +	.rx_desc_get_mpdu_tid = qwx_hw_wcn6855_rx_desc_get_mpdu_tid,
>  	.rx_desc_get_mpdu_peer_id = qwx_hw_wcn6855_rx_desc_get_mpdu_peer_id,
>  #if 0
>  	.rx_desc_copy_attn_end_tlv = ath11k_hw_wcn6855_rx_desc_copy_attn_end,
> @@ -2412,9 +2407,7 @@ const struct ath11k_hw_ops wcn6750_ops = {
>  	.tx_mesh_enable = ath11k_hw_qcn9074_tx_mesh_enable,
>  #endif
>  	.rx_desc_get_first_msdu = qwx_hw_qcn9074_rx_desc_get_first_msdu,
> -#if notyet
> -	.rx_desc_get_last_msdu = ath11k_hw_qcn9074_rx_desc_get_last_msdu,
> -#endif
> +	.rx_desc_get_last_msdu = qwx_hw_qcn9074_rx_desc_get_last_msdu,
>  	.rx_desc_get_l3_pad_bytes = qwx_hw_qcn9074_rx_desc_get_l3_pad_bytes,
>  	.rx_desc_get_hdr_status = qwx_hw_qcn9074_rx_desc_get_hdr_status,
>  	.rx_desc_encrypt_valid = qwx_hw_qcn9074_rx_desc_encrypt_valid,
> @@ -2434,9 +2427,7 @@ const struct ath11k_hw_ops wcn6750_ops = {
>  	.rx_desc_get_msdu_freq = qwx_hw_qcn9074_rx_desc_get_msdu_freq,
>  	.rx_desc_get_msdu_pkt_type = qwx_hw_qcn9074_rx_desc_get_msdu_pkt_type,
>  	.rx_desc_get_msdu_nss = qwx_hw_qcn9074_rx_desc_get_msdu_nss,
> -#ifdef notyet
> -	.rx_desc_get_mpdu_tid = ath11k_hw_qcn9074_rx_desc_get_mpdu_tid,
> -#endif
> +	.rx_desc_get_mpdu_tid = qwx_hw_qcn9074_rx_desc_get_mpdu_tid,
>  	.rx_desc_get_mpdu_peer_id = qwx_hw_qcn9074_rx_desc_get_mpdu_peer_id,
>  #if 0
>  	.rx_desc_copy_attn_end_tlv = ath11k_hw_qcn9074_rx_desc_copy_attn_end,
> @@ -16636,14 +16627,6 @@ qwx_hal_wbm_desc_parse_err(void *desc, struct hal_rx_w
>  }
>  
>  int
> -qwx_dp_rx_h_null_q_desc(struct qwx_softc *sc, struct qwx_rx_msdu *msdu,
> -    struct qwx_rx_msdu_list *msdu_list)
> -{
> -	printf("%s: not implemented\n", __func__);
> -	return ENOTSUP;
> -}
> -
> -int
>  qwx_dp_rx_h_reo_err(struct qwx_softc *sc, struct qwx_rx_msdu *msdu,
>      struct qwx_rx_msdu_list *msdu_list)
>  {
> @@ -16923,6 +16906,12 @@ qwx_dp_rx_h_msdu_start_nss(struct qwx_softc *sc, struc
>  	return hweight8(sc->hw_params.hw_ops->rx_desc_get_msdu_nss(desc));
>  }
>  
> +static inline uint8_t
> +qwx_dp_rx_h_mpdu_start_tid(struct qwx_softc *sc, struct hal_rx_desc *desc)
> +{
> +	return sc->hw_params.hw_ops->rx_desc_get_mpdu_tid(desc);
> +}
> +
>  uint32_t
>  qwx_dp_rx_h_attn_mpdu_err(struct rx_attention *attn)
>  {
> @@ -17360,7 +17349,98 @@ qwx_dp_rx_h_mpdu(struct qwx_softc *sc, struct qwx_rx_m
>  	return err_bitmap ? EIO : 0;
>  }
>  
> +void
> +qwx_dp_rx_null_q_desc_sg_drop(struct qwx_softc *sc, int msdu_len,
> +    struct qwx_rx_msdu_list *msdu_list)
> +{
> +	struct qwx_rx_msdu *msdu, *tmp;
> +	int n_buffs;
> +
> +	n_buffs = howmany(msdu_len,
> +	    (DP_RX_BUFFER_SIZE - sc->hw_params.hal_desc_sz));
> +
> +	TAILQ_FOREACH_SAFE(msdu, msdu_list, entry, tmp) {
> +		if (msdu->err_rel_src == HAL_WBM_REL_SRC_MODULE_REO &&
> +		    msdu->err_code ==
> +		    HAL_REO_DEST_RING_ERROR_CODE_DESC_ADDR_ZERO) {
> +			if (!n_buffs)
> +				break;
> +			TAILQ_REMOVE(msdu_list, msdu, entry);
> +			m_freem(msdu->m);
> +			msdu->m = NULL;
> +			n_buffs--;
> +		}
> +	}
> +}
> +
>  int
> +qwx_dp_rx_h_null_q_desc(struct qwx_softc *sc, struct qwx_rx_msdu *msdu,
> +    struct qwx_rx_msdu_list *msdu_list)
> +{
> +	uint16_t msdu_len;
> +	struct rx_attention *rx_attention;
> +	uint8_t l3pad_bytes;
> +	uint32_t hal_rx_desc_sz = sc->hw_params.hal_desc_sz;
> +
> +	msdu_len = qwx_dp_rx_h_msdu_start_msdu_len(sc, msdu->rx_desc);
> +
> +	if (!msdu->is_frag && (msdu_len + hal_rx_desc_sz) > DP_RX_BUFFER_SIZE) {
> +		/*
> +		 * First buffer will be freed by the caller, so deduct its
> +		 * length.
> +		 */
> +		msdu_len = msdu_len - (DP_RX_BUFFER_SIZE - hal_rx_desc_sz);
> +		qwx_dp_rx_null_q_desc_sg_drop(sc, msdu_len, msdu_list);
> +		return EINVAL;
> +	}
> +
> +	rx_attention = qwx_dp_rx_get_attention(sc, msdu->rx_desc);
> +	if (!qwx_dp_rx_h_attn_msdu_done(rx_attention)) {
> +		while ((msdu = TAILQ_FIRST(msdu_list))) {
> +			TAILQ_REMOVE(msdu_list, msdu, entry);
> +			m_freem(msdu->m);
> +			msdu->m = NULL;
> +		}
> +		return EIO;
> +	}
> +
> +	/* Handle NULL queue descriptor violations arising out a missing
> +	 * REO queue for a given peer or a given TID. This typically
> +	 * may happen if a packet is received on a QOS enabled TID before the
> +	 * ADDBA negotiation for that TID, when the TID queue is setup. Or
> +	 * it may also happen for MC/BC frames if they are not routed to the
> +	 * non-QOS TID queue, in the absence of any other default TID queue.
> +	 * This error can show up both in a REO destination or WBM release ring.
> +	 */
> +
> +	msdu->is_first_msdu = qwx_dp_rx_h_msdu_end_first_msdu(sc,
> +	    msdu->rx_desc);
> +	msdu->is_last_msdu = qwx_dp_rx_h_msdu_end_last_msdu(sc, msdu->rx_desc);
> +
> +	if (msdu->is_frag) {
> +		m_adj(msdu->m, hal_rx_desc_sz);
> +		msdu->m->m_len = msdu->m->m_pkthdr.len = msdu_len;
> +	} else {
> +		l3pad_bytes = qwx_dp_rx_h_msdu_end_l3pad(sc, msdu->rx_desc);
> +
> +		if ((hal_rx_desc_sz + l3pad_bytes + msdu_len) >
> +		    DP_RX_BUFFER_SIZE)
> +			return EINVAL;
> +
> +		m_adj(msdu->m, hal_rx_desc_sz + l3pad_bytes);
> +		msdu->m->m_len = msdu->m->m_pkthdr.len = msdu_len;
> +	}
> +
> +	qwx_dp_rx_h_ppdu(sc, msdu->rx_desc, &msdu->rxi);
> +
> +	qwx_dp_rx_h_mpdu(sc, msdu, msdu->rx_desc);
> +
> +	msdu->tid = qwx_dp_rx_h_mpdu_start_tid(sc, msdu->rx_desc);
> +
> +	return 0;
> +}
> +
> +int
>  qwx_dp_rx_process_msdu(struct qwx_softc *sc, struct qwx_rx_msdu *msdu,
>      struct qwx_rx_msdu_list *msdu_list)
>  {
> blob - 159ad6a2e1c77ee97a784419764221e1be23da3d
> blob + 2779251163185e64db61b1bc2f31f074b12153e8
> --- sys/dev/ic/qwxvar.h
> +++ sys/dev/ic/qwxvar.h
> @@ -266,9 +266,7 @@ struct ath11k_hw_ops {
>  			       struct hal_tcl_data_cmd *tcl_cmd);
>  #endif
>  	int (*rx_desc_get_first_msdu)(struct hal_rx_desc *desc);
> -#if notyet
> -	bool (*rx_desc_get_last_msdu)(struct hal_rx_desc *desc);
> -#endif
> +	int (*rx_desc_get_last_msdu)(struct hal_rx_desc *desc);
>  	uint8_t (*rx_desc_get_l3_pad_bytes)(struct hal_rx_desc *desc);
>  	uint8_t *(*rx_desc_get_hdr_status)(struct hal_rx_desc *desc);
>  	int (*rx_desc_encrypt_valid)(struct hal_rx_desc *desc);
> @@ -288,9 +286,7 @@ struct ath11k_hw_ops {
>  	uint32_t (*rx_desc_get_msdu_freq)(struct hal_rx_desc *desc);
>  	uint8_t (*rx_desc_get_msdu_pkt_type)(struct hal_rx_desc *desc);
>  	uint8_t (*rx_desc_get_msdu_nss)(struct hal_rx_desc *desc);
> -#ifdef notyet
>  	uint8_t (*rx_desc_get_mpdu_tid)(struct hal_rx_desc *desc);
> -#endif
>  	uint16_t (*rx_desc_get_mpdu_peer_id)(struct hal_rx_desc *desc);
>  #if 0
>  	void (*rx_desc_copy_attn_end_tlv)(struct hal_rx_desc *fdesc,
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.