[PATCH] wifi: ath12k: flush REO queue extension descriptors before freeing the qdesc

[PATCH] wifi: ath12k: flush REO queue extension descriptors before freeing the qdesc

[Sebastian Salmhofer]

Since commit b706fb4e580b ("wifi: ath12k: Use 1KB Cache Flush Command
for QoS TID Descriptors") a QoS TID RX queue descriptor is retired with
a single FLUSH_CACHE command carrying FLUSH_QUEUE_1K_DESC. On QCN9274
that command does not cover the extension descriptors that follow the
queue descriptor in the same 1536-byte allocation. Those hold the MPDU
link pointers and are written by REO on every enqueue and dequeue, so
they are frequently dirty in the REO cache when the TID is deleted.

After the qdesc is unmapped and freed, the REO cache controller later
evicts the stale extension lines and writes them back to the old DMA
address. On a host with the IOMMU enabled this shows up as a burst of
AMD-Vi IO_PAGE_FAULT write events at 128-byte spacing, e.g.

  AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x0038 address=0xf6e7a100 flags=0x0020]
  AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x0038 address=0xf6e7a180 flags=0x0020]
  ...
  AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x0038 address=0xf6e7a580 flags=0x0020]

The faulting addresses always fall at offsets 0x100..0x580 of a
retired qdesc and never at 0x000 or 0x080, i.e. exactly the ten
extension descriptors and never the queue descriptor or the 1K bitmap
descriptor. The writes are triggered by later REO activity, typically
a new station association, so they can occur minutes or hours after the
memory was freed, and the blocked transactions stall the data path for
several seconds. Without an IOMMU the same writes silently corrupt
whatever now occupies that memory.

Restore the per-segment flush of every 128-byte line above the queue
descriptor, as ath11k still does, before issuing the base flush with
FLUSH_QUEUE_1K_DESC and NEED_STATUS. REO commands execute in order, so
the status of the final base flush also confirms the preceding segment
flushes have completed, and the qdesc is still only freed from that
completion. A send failure in the sequence leaves the descriptor on the
retirement list for retry, as before.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.6-01243-QCAHKSWPL_SILICONZ-1

Fixes: b706fb4e580b ("wifi: ath12k: Use 1KB Cache Flush Command for QoS TID Descriptors")
Signed-off-by: Sebastian Salmhofer <sebastian.salmhofer@salmtek.com>
---
 drivers/net/wireless/ath/ath12k/wifi7/dp_rx.c | 39 ++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 9 deletions(-)

--- a/drivers/net/wireless/ath/ath12k/wifi7/dp_rx.c
+++ b/drivers/net/wireless/ath/ath12k/wifi7/dp_rx.c
@@ -225,22 +225,43 @@
 				    struct ath12k_dp_rx_tid_rxq *rx_tid)
 {
 	struct ath12k_hal_reo_cmd cmd = {};
+	dma_addr_t paddr = rx_tid->qbuf.paddr_aligned;
+	u32 off = rx_tid->qbuf.size;
 	int ret;
 
-	cmd.addr_lo = lower_32_bits(rx_tid->qbuf.paddr_aligned);
-	cmd.addr_hi = upper_32_bits(rx_tid->qbuf.paddr_aligned);
+	/* The REO cache controller caches the queue descriptor and each
+	 * 128-byte extension descriptor as separate objects, and a
+	 * FLUSH_CACHE command only addresses one of them. FLUSH_QUEUE_1K_DESC
+	 * extends the base flush to the 1K-window descriptor but does not
+	 * cover the extension descriptors, so flush those explicitly first.
+	 * The command ring executes in order, hence the final base flush
+	 * status also confirms the extension flushes have completed.
+	 */
+	while (off > HAL_LINK_DESC_ALIGN) {
+		off -= HAL_LINK_DESC_ALIGN;
+		memset(&cmd, 0, sizeof(cmd));
+		cmd.addr_lo = lower_32_bits(paddr + off);
+		cmd.addr_hi = upper_32_bits(paddr + off);
+		ret = ath12k_wifi7_dp_reo_cmd_send(ab, rx_tid,
+						   HAL_REO_CMD_FLUSH_CACHE,
+						   &cmd, NULL);
+		if (ret) {
+			ath12k_warn(ab,
+				    "failed to send FLUSH_CACHE for tid %d offset 0x%x: %d\n",
+				    rx_tid->tid, off, ret);
+			return ret;
+		}
+	}
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.addr_lo = lower_32_bits(paddr);
+	cmd.addr_hi = upper_32_bits(paddr);
 	/* HAL_REO_CMD_FLG_FLUSH_FWD_ALL_MPDUS - all pending MPDUs
-	 *in the bitmap will be forwarded/flushed to REO output rings
+	 * in the bitmap will be forwarded/flushed to REO output rings
 	 */
 	cmd.flag = HAL_REO_CMD_FLG_NEED_STATUS |
 		   HAL_REO_CMD_FLG_FLUSH_FWD_ALL_MPDUS;
 
-	/* For all QoS TIDs (except NON_QOS), the driver allocates a maximum
-	 * window size of 1024. In such cases, the driver can issue a single
-	 * 1KB descriptor flush command instead of sending multiple 128-byte
-	 * flush commands for each QoS TID, improving efficiency.
-	 */
-
 	if (rx_tid->tid != HAL_DESC_REO_NON_QOS_TID)
 		cmd.flag |= HAL_REO_CMD_FLG_FLUSH_QUEUE_1K_DESC;
 
--
2.47.0

Re: [PATCH] wifi: ath12k: flush REO queue extension descriptors before freeing the qdesc

[Baochen Qiang]

On 8/31/2026 11:10 PM, Sebastian Salmhofer wrote:
> Since commit b706fb4e580b ("wifi: ath12k: Use 1KB Cache Flush Command
> for QoS TID Descriptors") a QoS TID RX queue descriptor is retired with
> a single FLUSH_CACHE command carrying FLUSH_QUEUE_1K_DESC. On QCN9274
> that command does not cover the extension descriptors that follow the
> queue descriptor in the same 1536-byte allocation. Those hold the MPDU
> link pointers and are written by REO on every enqueue and dequeue, so
> they are frequently dirty in the REO cache when the TID is deleted.
> 
> After the qdesc is unmapped and freed, the REO cache controller later
> evicts the stale extension lines and writes them back to the old DMA
> address. On a host with the IOMMU enabled this shows up as a burst of
> AMD-Vi IO_PAGE_FAULT write events at 128-byte spacing, e.g.
> 
>   AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x0038 address=0xf6e7a100 flags=0x0020]
>   AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x0038 address=0xf6e7a180 flags=0x0020]
>   ...
>   AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x0038 address=0xf6e7a580 flags=0x0020]
> 
> The faulting addresses always fall at offsets 0x100..0x580 of a
> retired qdesc and never at 0x000 or 0x080, i.e. exactly the ten
> extension descriptors and never the queue descriptor or the 1K bitmap

the queue desc offset should be 0x000, and the offset of the first extension descriptor
should at 0x080. However the IOMMU warning starts at 0x100, which does not make sense ...

> descriptor. The writes are triggered by later REO activity, typically
> a new station association, so they can occur minutes or hours after the
> memory was freed, and the blocked transactions stall the data path for
> several seconds.

then what happens? the new sta association succeeds?
> Without an IOMMU the same writes silently corrupt
> whatever now occupies that memory.
> 
> Restore the per-segment flush of every 128-byte line above the queue
> descriptor, as ath11k still does, before issuing the base flush with
> FLUSH_QUEUE_1K_DESC and NEED_STATUS. REO commands execute in order, so
> the status of the final base flush also confirms the preceding segment
> flushes have completed, and the qdesc is still only freed from that
> completion. A send failure in the sequence leaves the descriptor on the
> retirement list for retry, as before.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.6-01243-QCAHKSWPL_SILICONZ-1
> 
> Fixes: b706fb4e580b ("wifi: ath12k: Use 1KB Cache Flush Command for QoS TID Descriptors")
> Signed-off-by: Sebastian Salmhofer <sebastian.salmhofer@salmtek.com>
> ---
>  drivers/net/wireless/ath/ath12k/wifi7/dp_rx.c | 39 ++++++++++++++++++++-------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> --- a/drivers/net/wireless/ath/ath12k/wifi7/dp_rx.c
> +++ b/drivers/net/wireless/ath/ath12k/wifi7/dp_rx.c
> @@ -225,22 +225,43 @@
>  				    struct ath12k_dp_rx_tid_rxq *rx_tid)
>  {
>  	struct ath12k_hal_reo_cmd cmd = {};
> +	dma_addr_t paddr = rx_tid->qbuf.paddr_aligned;
> +	u32 off = rx_tid->qbuf.size;
>  	int ret;
>  
> -	cmd.addr_lo = lower_32_bits(rx_tid->qbuf.paddr_aligned);
> -	cmd.addr_hi = upper_32_bits(rx_tid->qbuf.paddr_aligned);
> +	/* The REO cache controller caches the queue descriptor and each
> +	 * 128-byte extension descriptor as separate objects, and a
> +	 * FLUSH_CACHE command only addresses one of them. FLUSH_QUEUE_1K_DESC
> +	 * extends the base flush to the 1K-window descriptor but does not
> +	 * cover the extension descriptors, so flush those explicitly first.
> +	 * The command ring executes in order, hence the final base flush
> +	 * status also confirms the extension flushes have completed.
> +	 */
> +	while (off > HAL_LINK_DESC_ALIGN) {
> +		off -= HAL_LINK_DESC_ALIGN;
> +		memset(&cmd, 0, sizeof(cmd));

unnecessary cleanup since all required fields are refilled in each iteration.

> +		cmd.addr_lo = lower_32_bits(paddr + off);
> +		cmd.addr_hi = upper_32_bits(paddr + off);
> +		ret = ath12k_wifi7_dp_reo_cmd_send(ab, rx_tid,
> +						   HAL_REO_CMD_FLUSH_CACHE,
> +						   &cmd, NULL);
> +		if (ret) {
> +			ath12k_warn(ab,
> +				    "failed to send FLUSH_CACHE for tid %d offset 0x%x: %d\n",
> +				    rx_tid->tid, off, ret);
> +			return ret;
> +		}
> +	}
> +
> +	memset(&cmd, 0, sizeof(cmd));

also unnecessary

> +	cmd.addr_lo = lower_32_bits(paddr);
> +	cmd.addr_hi = upper_32_bits(paddr);
>  	/* HAL_REO_CMD_FLG_FLUSH_FWD_ALL_MPDUS - all pending MPDUs
> -	 *in the bitmap will be forwarded/flushed to REO output rings
> +	 * in the bitmap will be forwarded/flushed to REO output rings
>  	 */
>  	cmd.flag = HAL_REO_CMD_FLG_NEED_STATUS |
>  		   HAL_REO_CMD_FLG_FLUSH_FWD_ALL_MPDUS;
>  
> -	/* For all QoS TIDs (except NON_QOS), the driver allocates a maximum
> -	 * window size of 1024. In such cases, the driver can issue a single
> -	 * 1KB descriptor flush command instead of sending multiple 128-byte
> -	 * flush commands for each QoS TID, improving efficiency.
> -	 */
> -
>  	if (rx_tid->tid != HAL_DESC_REO_NON_QOS_TID)
>  		cmd.flag |= HAL_REO_CMD_FLG_FLUSH_QUEUE_1K_DESC;

HAL_REO_CMD_FLG_FLUSH_QUEUE_1K_DESC is used to flush all in a single cmd. since we switch
back to the per segment flush, do we still need it?

>  
> --
> 2.47.0
> 
> 

Re: [PATCH] wifi: ath12k: flush REO queue extension descriptors before freeing the qdesc

[Sebastian Salmhofer]

On 9/3/2026 5:07 AM, Baochen Qiang wrote:
 >> The faulting addresses always fall at offsets 0x100..0x580 of a
 >> retired qdesc and never at 0x000 or 0x080, i.e. exactly the ten
 >> extension descriptors and never the queue descriptor or the 1K bitmap
 >
 > the queue desc offset should be 0x000, and the offset of the first 
extension descriptor
 > should at 0x080. However the IOMMU warning starts at 0x100, which 
does not make sense ...

You are right that under the driver's struct layout ext_desc[0] sits at
0x080, so that sentence of the commit message was an interpretation on
my part, not an observed fact. The observed facts, across about ten
days of logs and more than a dozen retired qdescs, are:

- every fault falls at an offset in 0x100..0x580 of a retired
   1536-byte qdesc (bases are 2KB-aligned from kmalloc-2k, so the
   offsets are unambiguous),
- no fault was ever seen at 0x000 or 0x080,
- within one qdesc the faulting lines are 0x80-strided runs, sometimes
   the full 0x100..0x580 sweep, sometimes a subset, and lines of one
   qdesc can be written back in separate bursts minutes apart.

Two explanations would fit that pattern: either the hardware expects
the 1K bitmap descriptor directly after the base queue structure (so
extension descriptors effectively start at 0x100 from the hardware's
point of view), or FLUSH_QUEUE_1K_DESC covers the first 256 bytes of
the queue as one object. I don't have documentation for the REO cache
controller, so I can only report that on QCN9274 hw2.0 with
WLAN.WBE.1.6-01243 the single 1K flush demonstrably leaves everything
from 0x100 upward dirty. Could you check internally what the 1K flush
actually covers on this target, and where the hardware expects the 1K
bitmap? I will reword the commit message to stay purely with the
observed offsets in v2.

 >> The writes are triggered by later REO activity, typically
 >> a new station association, so they can occur minutes or hours after the
 >> memory was freed, and the blocked transactions stall the data path for
 >> several seconds.
 >
 > then what happens? the new sta association succeeds?

Yes, the association succeeds. The IOMMU blocks the writes, so nothing
is corrupted; the user-visible symptom is that the data path stalls
for several seconds around each fault burst, for all connected
stations, which is how this was noticed (periodic freezes of
latency-sensitive traffic). With the fix applied the stalls are gone
along with the faults. Without an IOMMU the same writes would land in
freed memory. I will make that explicit in v2.

 >> +    while (off > HAL_LINK_DESC_ALIGN) {
 >> +        off -= HAL_LINK_DESC_ALIGN;
 >> +        memset(&cmd, 0, sizeof(cmd));
 >
 > unnecessary cleanup since all required fields are refilled in each 
iteration.

Right, and the same for the one before the base flush - cmd is
zero-initialized and both sites set every field the flush-cache
command reads. Will drop both in v2.

 >> +    if (rx_tid->tid != HAL_DESC_REO_NON_QOS_TID)
 >> +        cmd.flag |= HAL_REO_CMD_FLG_FLUSH_QUEUE_1K_DESC;
 >
 > HAL_REO_CMD_FLG_FLUSH_QUEUE_1K_DESC is used to flush all in a single 
cmd. since we switch
 > back to the per segment flush, do we still need it?

I kept it deliberately. The per-segment loop covers every 128-byte
line from 0x080 upward, and the flag makes the final base flush cover
whatever the "1K descriptor" object is, so the sequence is correct
under either interpretation above, at no extra cost. Dropping the flag
would restore the exact pre-b706fb4e580b sequence; that may well be
sufficient, but if the 1K-window queue is cached as an object larger
than 128 bytes on this target, a plain flush of the base address might
not cover its tail, and I cannot rule that out from here. If you can
confirm that plain 128-byte flushes fully cover a 1K-window queue
descriptor, I am happy to drop the flag in v2.

I will send v2 with the memsets removed and the commit message
clarified once the 1K flush semantics are settled.

Thanks for the review,
Sebastian