* [PATCH 0/2] wifi: ath12k: fix dest ring-buffer corruption
@ 2025-05-26 11:51 Johan Hovold
2025-05-26 11:51 ` [PATCH 1/2] " Johan Hovold
2025-05-26 11:51 ` [PATCH 2/2] wifi: ath12k: use plain access for descriptor length Johan Hovold
0 siblings, 2 replies; 4+ messages in thread
From: Johan Hovold @ 2025-05-26 11:51 UTC (permalink / raw)
To: Jeff Johnson
Cc: Miaoqing Pan, Remi Pommarel, linux-wireless, ath12k, linux-kernel,
Johan Hovold
As a follow up to commit:
b67d2cf14ea ("wifi: ath12k: fix ring-buffer corruption")
add the remaining 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.
Also switch back to plain accesses for the descriptor fields which is
sufficient after the memory barrier.
Johan
Johan Hovold (2):
wifi: ath12k: fix dest ring-buffer corruption
wifi: ath12k: use plain access for descriptor length
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 +++
drivers/net/wireless/ath/ath12k/hal.c | 2 +-
4 files changed, 19 insertions(+), 1 deletion(-)
--
2.49.0
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 1/2] wifi: ath12k: fix dest ring-buffer corruption 2025-05-26 11:51 [PATCH 0/2] wifi: ath12k: fix dest ring-buffer corruption Johan Hovold @ 2025-05-26 11:51 ` Johan Hovold 2025-06-04 8:19 ` Miaoqing Pan 2025-05-26 11:51 ` [PATCH 2/2] wifi: ath12k: use plain access for descriptor length Johan Hovold 1 sibling, 1 reply; 4+ messages in thread From: Johan Hovold @ 2025-05-26 11:51 UTC (permalink / raw) To: Jeff Johnson Cc: Miaoqing Pan, Remi Pommarel, linux-wireless, ath12k, linux-kernel, Johan Hovold, stable 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) -- 2.49.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] wifi: ath12k: fix dest ring-buffer corruption 2025-05-26 11:51 ` [PATCH 1/2] " Johan Hovold @ 2025-06-04 8:19 ` Miaoqing Pan 0 siblings, 0 replies; 4+ messages in thread From: Miaoqing Pan @ 2025-06-04 8:19 UTC (permalink / raw) To: Johan Hovold, Jeff Johnson Cc: Remi Pommarel, linux-wireless, ath12k, linux-kernel, stable 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] wifi: ath12k: use plain access for descriptor length 2025-05-26 11:51 [PATCH 0/2] wifi: ath12k: fix dest ring-buffer corruption Johan Hovold 2025-05-26 11:51 ` [PATCH 1/2] " Johan Hovold @ 2025-05-26 11:51 ` Johan Hovold 1 sibling, 0 replies; 4+ messages in thread From: Johan Hovold @ 2025-05-26 11:51 UTC (permalink / raw) To: Jeff Johnson Cc: Miaoqing Pan, Remi Pommarel, linux-wireless, ath12k, linux-kernel, Johan Hovold The read memory barrier added by commit 6b67d2cf14ea ("wifi: ath12k: fix ring-buffer corruption") is enough to guarantee ordering also for plain descriptor accesses if the length helper is ever inlined so drop the unnecessary READ_ONCE(). Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/net/wireless/ath/ath12k/hal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c index 91d5126ca149..962111c66a3e 100644 --- a/drivers/net/wireless/ath/ath12k/hal.c +++ b/drivers/net/wireless/ath/ath12k/hal.c @@ -1962,7 +1962,7 @@ u32 ath12k_hal_ce_dst_status_get_length(struct hal_ce_srng_dst_status_desc *desc { u32 len; - len = le32_get_bits(READ_ONCE(desc->flags), HAL_CE_DST_STATUS_DESC_FLAGS_LEN); + len = le32_get_bits(desc->flags, HAL_CE_DST_STATUS_DESC_FLAGS_LEN); desc->flags &= ~cpu_to_le32(HAL_CE_DST_STATUS_DESC_FLAGS_LEN); return len; -- 2.49.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-06-04 8:19 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-26 11:51 [PATCH 0/2] wifi: ath12k: fix dest ring-buffer corruption Johan Hovold 2025-05-26 11:51 ` [PATCH 1/2] " Johan Hovold 2025-06-04 8:19 ` Miaoqing Pan 2025-05-26 11:51 ` [PATCH 2/2] wifi: ath12k: use plain access for descriptor length Johan Hovold
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox