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

[Johan Hovold <johan@kernel.org>]

On Mon, Jun 16, 2025 at 02:59:24PM +0530, Praneesh P wrote:
> On 6/5/2025 4:19 PM, Baochen Qiang wrote:
> > On 6/5/2025 6:00 PM, Johan Hovold wrote:
> >> On Thu, Jun 05, 2025 at 04:41:32PM +0800, Baochen Qiang wrote:
> >>> On 6/4/2025 10:45 PM, Johan Hovold wrote:

> >>>> Add the missing memory barrier to make sure that destination ring
> >>>> descriptors are read after the head pointers to avoid using stale data
> >>>> on weakly ordered architectures like aarch64.
> >>>>
> >>>> The barrier is added to the ath12k_hal_srng_access_begin() helper for
> >>>> symmetry with follow-on fixes for source ring buffer corruption which
> >>>> will add barriers to ath12k_hal_srng_access_end().
> >>>>
> >>>> Note that this may fix the empty descriptor issue recently worked around
> >>>> by commit 51ad34a47e9f ("wifi: ath12k: Add drop descriptor handling for
> >>>> monitor ring").

> >>> why? I would expect drunk cookies are valid in case of HAL_MON_DEST_INFO0_EMPTY_DESC,
> >>> rather than anything caused by reordering.

> >> Based on a quick look it seemed like this could possibly fall in the
> >> same category as some of the other workarounds I've spotted while
> >> looking into these ordering issues (e.g. f9fff67d2d7c ("wifi: ath11k:
> >> Fix SKB corruption in REO destination ring")).
> >>
> >> If you say this one is clearly unrelated, I'll drop the comment.

> > Praneesh, could you comment here since you made that change?

> Empty/Drop descriptor is intentionally issued by the hardware during 
> backpressure scenario
> and is unrelated to the issue discussed in this patch series.

Thanks for confirming. I've dropped this comment in v3:

	https://lore.kernel.org/lkml/20250617084402.14475-1-johan+linaro@kernel.org/

Johan