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.

New in v2 are two patches that add the missing barriers also for source
rings and when updating the tail pointer for destination rings.

To avoid leaking ring details from the "hal" (lmac or non-lmac), the
barriers are added to the ath12k_hal_srng_access_end() helper. For
symmetry I therefore moved also the dest ring barriers into
ath12k_hal_srng_access_begin() and made the barrier conditional.

[ Due to this change I did not add Miaoqing's reviewed-by tag. ]

Johan


Changes in v2:
 - add tested-on tags to plain access patch
 - move destination barriers into begin helper
 - fix source ring corruption (new patch)
 - fix dest ring corruption when ring is full (new patch)


Johan Hovold (4):
  wifi: ath12k: fix dest ring-buffer corruption
  wifi: ath12k: use plain access for descriptor length
  wifi: ath12k: fix source ring-buffer corruption
  wifi: ath12k: fix dest ring-buffer corruption when ring is full

 drivers/net/wireless/ath/ath12k/ce.c  |  3 --
 drivers/net/wireless/ath/ath12k/hal.c | 40 ++++++++++++++++++++++-----
 2 files changed, 33 insertions(+), 10 deletions(-)

-- 
2.49.0

On 6/4/2025 10:45 PM, Johan Hovold wrote:
> 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.
> 
> New in v2 are two patches that add the missing barriers also for source
> rings and when updating the tail pointer for destination rings.
> 
> To avoid leaking ring details from the "hal" (lmac or non-lmac), the
> barriers are added to the ath12k_hal_srng_access_end() helper. For

Could you elaborate? what do you mean by "leaking ring details from the 'hal'"?

> symmetry I therefore moved also the dest ring barriers into
> ath12k_hal_srng_access_begin() and made the barrier conditional.
> 
> [ Due to this change I did not add Miaoqing's reviewed-by tag. ]
> 
> Johan
> 
> 
> Changes in v2:
>  - add tested-on tags to plain access patch
>  - move destination barriers into begin helper
>  - fix source ring corruption (new patch)
>  - fix dest ring corruption when ring is full (new patch)
> 
> 
> Johan Hovold (4):
>   wifi: ath12k: fix dest ring-buffer corruption
>   wifi: ath12k: use plain access for descriptor length
>   wifi: ath12k: fix source ring-buffer corruption
>   wifi: ath12k: fix dest ring-buffer corruption when ring is full
> 
>  drivers/net/wireless/ath/ath12k/ce.c  |  3 --
>  drivers/net/wireless/ath/ath12k/hal.c | 40 ++++++++++++++++++++++-----
>  2 files changed, 33 insertions(+), 10 deletions(-)
>

On Thu, Jun 05, 2025 at 04:37:13PM +0800, Baochen Qiang wrote:
> On 6/4/2025 10:45 PM, Johan Hovold wrote:
> > 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.
> > 
> > New in v2 are two patches that add the missing barriers also for source
> > rings and when updating the tail pointer for destination rings.
> > 
> > To avoid leaking ring details from the "hal" (lmac or non-lmac), the
> > barriers are added to the ath12k_hal_srng_access_end() helper. For
> 
> Could you elaborate? what do you mean by "leaking ring details from the 'hal'"?

The type of barrier needed depends on the type of the ring. If we add
the barrier directly in the caller, the caller would need to know what
kind of ring (lmac or non-lmac) it is operating on, something which is
currently abstracted away in the hal helpers.

> > symmetry I therefore moved also the dest ring barriers into
> > ath12k_hal_srng_access_begin() and made the barrier conditional.

Johan

On 6/5/2025 4:44 PM, Johan Hovold wrote:
> On Thu, Jun 05, 2025 at 04:37:13PM +0800, Baochen Qiang wrote:
>> On 6/4/2025 10:45 PM, Johan Hovold wrote:
>>> 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.
>>>
>>> New in v2 are two patches that add the missing barriers also for source
>>> rings and when updating the tail pointer for destination rings.
>>>
>>> To avoid leaking ring details from the "hal" (lmac or non-lmac), the
>>> barriers are added to the ath12k_hal_srng_access_end() helper. For
>>
>> Could you elaborate? what do you mean by "leaking ring details from the 'hal'"?
> 
> The type of barrier needed depends on the type of the ring. If we add
> the barrier directly in the caller, the caller would need to know what
> kind of ring (lmac or non-lmac) it is operating on, something which is
> currently abstracted away in the hal helpers.
> 

Thanks, I get your point. I can see the difference in patch [3/4]

>>> symmetry I therefore moved also the dest ring barriers into
>>> ath12k_hal_srng_access_begin() and made the barrier conditional.
> 
> Johan