Thread (13 messages) 13 messages, 5 authors, 1d ago

Re: [PATCH net 1/2] net: macb: reprogram TBQP after shuffling the TX ring on link-up

From: Kevin Hao <hidden>
Date: 2026-07-07 09:13:47
Also in: linux-rt-devel, lkml, stable

On Mon, Jul 06, 2026 at 04:02:14PM +0200, Christian Taedcke via B4 Relay wrote:
quoted hunk ↗ jump to hunk
From: Christian Taedcke <redacted>

gem_shuffle_tx_one_ring() rotates the software TX ring so that the
tail sits at index 0 and resets queue->tx_tail to 0, but it never
reprograms the hardware transmit buffer queue pointer (TBQP). Other
paths that reset tx_tail to the ring base (macb_init_buffers() and
macb_tx_error_task()) also reprogram TBQP to queue->tx_ring_dma; this
path does not, leaving TBQP pointing at a stale descriptor.

gem_shuffle_tx_rings() runs on every link-up from
macb_mac_link_up(). After a few link up/down flaps that leave
un-completed descriptors in the ring, the stale TBQP keeps pointing at
a descriptor whose used bit is set. When TX is re-enabled on link-up,
the GEM reads that used descriptor and raises TXUBR. macb_interrupt()
schedules the TX NAPI, macb_tx_poll() makes no progress (work_done ==
0) and macb_tx_restart() re-issues TSTART, which makes the controller
read the same used descriptor again and re-assert TXUBR. As the MAC
interrupt is level-triggered, it never deasserts and one CPU is pegged
at 100% in the threaded handler, eventually triggering "sched: RT
throttling activated" and a dead network interface.

Fix it by reprogramming TBQP to the ring base on every path of
gem_shuffle_tx_one_ring() that resets tx_tail to 0, mirroring
macb_tx_error_task(). The early return for an already-aligned tail is
left untouched as TBQP is already consistent there. This is safe
because the shuffle runs from macb_mac_link_up() while TE is still
disabled, so the transmitter is halted.

Fixes: 881a0263d502 ("net: macb: Shuffle the tx ring before enabling tx")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Christian Taedcke <redacted>
---
 drivers/net/ethernet/cadence/macb_main.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index fd282a1700fb..b11cb8f068b7 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -820,7 +820,7 @@ static void gem_shuffle_tx_one_ring(struct macb_queue *queue)
 	if (!count) {
 		queue->tx_head = 0;
 		queue->tx_tail = 0;
-		goto unlock;
+		goto reset_hw_ptr;
 	}
 
 	shift = tail % ring_size;
@@ -869,6 +869,13 @@ static void gem_shuffle_tx_one_ring(struct macb_queue *queue)
 	/* Make descriptor updates visible to hardware */
 	wmb();
 
+reset_hw_ptr:
+	/* tx_tail was reset to the ring base, so TBQP must be reprogrammed
+	 * to match; otherwise it keeps pointing at a stale descriptor. Safe
+	 * to write directly here as TX is still disabled (called from
+	 * macb_mac_link_up() before TE is set).
+	 */
Could you elaborate on why we need to reprogram the TBQP here? Based on my
understanding, the transmit-buffer queue pointer automatically resets to the
value of TBQP when TX is disabled. The following is quoted from the Zynq
UltraScale TRM [1]:
  While transmit is disabled, bit [3] of the network control is
  set Low, the transmit-buffer queue pointer resets to point to the address indicated by the
  transmit-buffer queue base address register. Disabling receive does not have the same
  effect on the receive-buffer queue pointer.

[1] https://docs.amd.com/v/u/en-US/ug1085-zynq-ultrascale-trm

Thanks,
Kevin
+	queue_writel(queue, TBQP, lower_32_bits(queue->tx_ring_dma));
 unlock:
 	spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
 }

-- 
2.54.0

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help