Re: [PATCH 1/2] wifi: ath12k: fix dest ring-buffer corruption

[Miaoqing Pan <quic_miaoqing@quicinc.com>]

On 5/26/2025 7:51 PM, Johan Hovold wrote:
> Add the missing memory barriers to make sure that destination ring
> descriptors are read after the head pointers to avoid using stale data
> on weakly ordered architectures like aarch64.
> 
> Note that this may fix the empty descriptor issue recently worked around
> by commit 51ad34a47e9f ("wifi: ath12k: Add drop descriptor handling for
> monitor ring").
> 
> Tested-on: WCN7850 hw2.0 WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Cc: stable@vger.kernel.org	# 6.3
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/net/wireless/ath/ath12k/dp_mon.c |  3 +++
>   drivers/net/wireless/ath/ath12k/dp_rx.c  | 12 ++++++++++++
>   drivers/net/wireless/ath/ath12k/dp_tx.c  |  3 +++
>   3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/dp_mon.c b/drivers/net/wireless/ath/ath12k/dp_mon.c
> index d22800e89485..90a7763502c8 100644
> --- a/drivers/net/wireless/ath/ath12k/dp_mon.c
> +++ b/drivers/net/wireless/ath/ath12k/dp_mon.c
> @@ -3258,6 +3258,9 @@ int ath12k_dp_mon_srng_process(struct ath12k *ar, int *budget,
>   	spin_lock_bh(&srng->lock);
>   	ath12k_hal_srng_access_begin(ab, srng);
>   
> +	/* Make sure descriptor is read after the head pointer. */
> +	dma_rmb();
> +
>   	while (likely(*budget)) {
>   		*budget -= 1;
>   		mon_dst_desc = ath12k_hal_srng_dst_peek(ab, srng);
> diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c
> index 75bf4211ad42..68fceb4201d7 100644
> --- a/drivers/net/wireless/ath/ath12k/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath12k/dp_rx.c
> @@ -2753,6 +2753,9 @@ int ath12k_dp_rx_process(struct ath12k_base *ab, int ring_id,
>   try_again:
>   	ath12k_hal_srng_access_begin(ab, srng);
>   
> +	/* Make sure descriptor is read after the head pointer. */
> +	dma_rmb();
> +
>   	while ((desc = ath12k_hal_srng_dst_get_next_entry(ab, srng))) {
>   		struct rx_mpdu_desc *mpdu_info;
>   		struct rx_msdu_desc *msdu_info;
> @@ -3599,6 +3602,9 @@ int ath12k_dp_rx_process_err(struct ath12k_base *ab, struct napi_struct *napi,
>   
>   	ath12k_hal_srng_access_begin(ab, srng);
>   
> +	/* Make sure descriptor is read after the head pointer. */
> +	dma_rmb();
> +
>   	while (budget &&
>   	       (reo_desc = ath12k_hal_srng_dst_get_next_entry(ab, srng))) {
>   		drop = false;
> @@ -3941,6 +3947,9 @@ int ath12k_dp_rx_process_wbm_err(struct ath12k_base *ab,
>   
>   	ath12k_hal_srng_access_begin(ab, srng);
>   
> +	/* Make sure descriptor is read after the head pointer. */
> +	dma_rmb();
> +
>   	while (budget) {
>   		rx_desc = ath12k_hal_srng_dst_get_next_entry(ab, srng);
>   		if (!rx_desc)
> @@ -4122,6 +4131,9 @@ void ath12k_dp_rx_process_reo_status(struct ath12k_base *ab)
>   
>   	ath12k_hal_srng_access_begin(ab, srng);
>   
> +	/* Make sure descriptor is read after the head pointer. */
> +	dma_rmb();
> +
>   	while ((hdr = ath12k_hal_srng_dst_get_next_entry(ab, srng))) {
>   		tag = le64_get_bits(hdr->tl, HAL_SRNG_TLV_HDR_TAG);
>   
> diff --git a/drivers/net/wireless/ath/ath12k/dp_tx.c b/drivers/net/wireless/ath/ath12k/dp_tx.c
> index ced232bf4aed..3124eafa0201 100644
> --- a/drivers/net/wireless/ath/ath12k/dp_tx.c
> +++ b/drivers/net/wireless/ath/ath12k/dp_tx.c
> @@ -853,6 +853,9 @@ void ath12k_dp_tx_completion_handler(struct ath12k_base *ab, int ring_id)
>   
>   	ath12k_hal_srng_access_begin(ab, status_ring);
>   
> +	/* Make sure descriptor is read after the head pointer. */
> +	dma_rmb();
> +
>   	while (ATH12K_TX_COMPL_NEXT(tx_ring->tx_status_head) != tx_ring->tx_status_tail) {
>   		desc = ath12k_hal_srng_dst_get_next_entry(ab, status_ring);
>   		if (!desc)

Reviewed-by: Miaoqing Pan <quic_miaoqing@quicinc.com>