Re: Re: [PATCH net v6 0/4] net: stmmac: fix RX descriptors and XSK TX on resume

[Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>]

Hi Ding Hui,
Thanks for your comments.

On Fri, Nov 13, 2026 at 10:33:44AM +0800, dinghui1111 wrote:
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> At 2026-11-13 00:38:17, "Tommaso Merciai" <tommaso.merciai.xr@bp.renesas.com> wrote:
> >Hi Ding Hui,
> >Thanks for your series.
> >
> >On Tue, Nov 10, 2026 at 08:38:51PM +0800, Ding Hui wrote:
> >> From: Ding Hui <dinghui@lixiang.com>
> >> 
> >> This series restores RX descriptor addresses overwritten by hardware
> >> writeback between disabling NAPI and stopping RX DMA during suspend. It
> >> also fixes an AF_XDP TX-only resume stall found while testing the series.
> >> 
> >> Patch 1 honors RX allocation flags. Patch 2 keeps unfilled XSK slots
> >> CPU-owned and tracks pending refills explicitly, including an empty ring
> >> and short refills. Patch 3 restores RX addresses from retained buffers,
> >> rebuilds chain pointers and initializes descriptor control fields. It
> >> publishes a contiguous XSK prefix and recomputes the tail and pending
> >> region. Patch 4 restores the bound XSK TX pool after TX buffer cleanup;
> >> otherwise zero-copy transmission can stop after resume.
> >> 
> >> The earlier commit 9c63faaa931e ("net: stmmac: re-init rx buffers when mac
> >> resume back") was reverted by 00423969d806 after a Jetson TX2 suspend/resume
> >> regression. This series retains existing buffers and DMA mappings instead
> >> of recycling and reallocating every buffer. Jetson TX2 has not been tested.
> >> 
> >> Hardware validation used backports on our XGMAC platform with a vendor
> >> kernel based on Linux 6.12.41, not these upstream commits as-is:
> >> - Receiving iperf traffic from a PC while suspending and resuming caused a
> >>   fatal bus error after resume in almost every run without the RX fix.
> >>   The same test no longer showed that failure with the fix.
> >
> >
> >I'm testing this series on the Renesas RZ/G3E.
> >On my setup I'm receiving iperf traffic from a PC while suspending and
> >resuming (s2idle):
> >
> >[ 46.745703] renesas-gbeth 15c30000.ethernet end0: Link is Up - 100Mbps/Full - flow control rx/tx
> >
> >Before this patch I'm seeing (during resume phase):
> >
> >[ 73.919151] rzv2h-icu: BUS error k=1 status=0x00000040
> >
> >Which I don't see anymore applying your series.
> >But after some suspend/resume cycle I'm seeing the following:
> >
> >[  262.878939] PM: suspend entry (s2idle)
> >[  262.883075] Filesystems sync: 0.000 seconds
> >[  262.888229] Freezing user space processes
> >[  262.894080] Freezing user space processes completed (elapsed 0.001 seconds)
> >[  262.901046] OOM killer disabled.
> >[  262.904275] Freezing remaining freezable tasks
> >[  262.910033] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
> >[  262.974153] renesas-gbeth 15c30000.ethernet end0: Link is Down
> >[  263.183095] SError Interrupt on CPU3, code 0x00000000be000011 -- SError
> >[  263.183154] CPU: 3 UID: 0 PID: 0 Comm: swapper/3 Tainted: G   M                7.4.0-rc2-00101-g8c75c70989f1 #6 PREEMPT(full)
> >[  263.183204] Tainted: [M]=MACHINE_CHECK
> >[  263.183217] Hardware name: Renesas SMARC EVK version 2 based on r9a09g047e57 (DT)
> >[  263.183235] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> >[  263.183271] pc : stmmac_tx_timer_arm+0x60/0xa4
> >[  263.183333] lr : stmmac_xmit+0xd4c/0x1650
> >[  263.183383] sp : ffff8000834435b0
> >[  263.183396] x29: ffff8000834435b0 x28: ffff0000c0298ac0 x27: ffff0000c1498ce0
> >[  263.183454] x26: 0000000000000001 x25: 00000000000021c0 x24: 0000000000000001
> >[  263.183501] x23: ffff0000c0298cc0 x22: 0000000000000280 x21: 000000000000010e
> >[  263.183549] x20: 0000000000001388 x19: ffff0000c0299648 x18: ffff0000ff7e01bc
> >[  263.183597] x17: 00000000000a03c8 x16: 003fffffffffffff x15: c15e9f4d40b0ba44
> >[  263.183644] x14: ffff800080c377c4 x13: f39e923bc472b73f x12: fbdb674df0d86e0f
> >[  263.183692] x11: 5a679e689a9836bb x10: 0ca385496fded8a0 x9 : 00000000200005ea
> >[  263.183739] x8 : 0000000000030000 x7 : 000000000000010f x6 : ffff800080c39340
> >[  263.183786] x5 : ffff0000c0298cc0 x4 : 0000000000000000 x3 : 00000000000013c0
> >[  263.183831] x2 : ffff0000c029ad00 x1 : ffff0000c029ab00 x0 : 000000000000203b
> >[  263.183885] Kernel panic - not syncing: Asynchronous SError Interrupt
> >[  263.183912] CPU: 3 UID: 0 PID: 0 Comm: swapper/3 Tainted: G   M                7.4.0-rc2-00101-g8c75c70989f1 #6 PREEMPT(full)
> >[  263.183961] Tainted: [M]=MACHINE_CHECK
> >[  263.183973] Hardware name: Renesas SMARC EVK version 2 based on r9a09g047e57 (DT)
> >[  263.183990] Call trace:
> >[  263.184003]  show_stack+0x18/0x24 (C)
> >[  263.184048]  dump_stack_lvl+0x34/0x8c
> >[  263.184103]  dump_stack+0x18/0x24
> >[  263.184153]  vpanic+0x380/0x3ac
> >[  263.184204]  nmi_panic+0xf0/0x108
> >[  263.184252]  arm64_serror_panic+0x70/0x80
> >[  263.184294]  arm64_is_fatal_ras_serror+0x3c/0xac
> >[  263.184336]  do_serror+0x5c/0x70
> >[  263.184374]  el1h_64_error_handler+0x34/0x54
> >[  263.184433]  el1h_64_error+0x78/0x7c
> >[  263.184472]  stmmac_tx_timer_arm+0x60/0xa4 (P)
> >[  263.184526]  stmmac_xmit+0xd4c/0x1650
> >[  263.184580]  dev_hard_start_xmit+0x8c/0x104
> >[  263.184622]  sch_direct_xmit+0x8c/0x35c
> >[  263.184668]  __dev_queue_xmit+0x808/0xf2c
> >[  263.184706]  ip_finish_output2+0x458/0x5ac
> >[  263.184749]  __ip_finish_output+0x1dc/0x26c
> >[  263.184789]  ip_finish_output+0x34/0x148
> >[  263.184828]  ip_output+0x70/0x114
> >[  263.184866]  __ip_queue_xmit+0x164/0x410
> >[  263.184906]  ip_queue_xmit+0x14/0x20
> >[  263.184945]  __tcp_transmit_skb+0x548/0xe30
> >[  263.184993]  __tcp_retransmit_skb.part.0+0x2a4/0x950
> >[  263.185041]  tcp_retransmit_skb+0x50/0x104
> >[  263.185086]  tcp_retransmit_timer+0x430/0xb68
> >[  263.185135]  tcp_write_timer_handler+0x180/0x1f0
> >[  263.185185]  tcp_write_timer+0x130/0x17c
> >[  263.185232]  call_timer_fn.isra.0+0x24/0x80
> >[  263.185278]  __run_timers+0x228/0x270
> >[  263.185319]  timer_expire_remote+0x50/0x68
> >[  263.185365]  tmigr_handle_remote_up+0x1b4/0x280
> >[  263.185408]  __walk_groups_from+0x3c/0x9c
> >[  263.185449]  tmigr_handle_remote+0xd8/0xf8
> >[  263.185492]  run_timer_softirq+0xec/0xfc
> >[  263.185535]  handle_softirqs+0x128/0x2b4
> >[  263.185577]  __do_softirq+0x14/0x20
> >[  263.185614]  ____do_softirq+0x10/0x1c
> >[  263.185656]  call_on_irq_stack+0x30/0x48
> >[  263.185698]  do_softirq_own_stack+0x1c/0x28
> >[  263.185742]  __irq_exit_rcu+0xf0/0x11c
> >[  263.185782]  irq_exit_rcu+0x10/0x1c
> >[  263.185821]  el1_interrupt+0x40/0x60
> >[  263.185873]  el1h_64_irq_handler+0x18/0x24
> >[  263.185928]  el1h_64_irq+0x78/0x7c
> >[  263.185965]  default_idle_call+0x28/0x44 (P)
> >[  263.186008]  do_idle+0x240/0x2f0
> >[  263.186051]  cpu_startup_entry+0x34/0x3c
> >[  263.186093]  secondary_start_kernel+0x13c/0x15c
> >[  263.186148]  __secondary_switched+0xc0/0xc4
> >[  263.186203] SMP: stopping secondary CPUs
> >[  263.186233] Kernel Offset: disabled
> >[  263.186243] CPU features: 0x0,40000000,00400028,01180a02,0800e21b
> >[  263.186269] Memory Limit: none
> >[  263.544217] ---[ end Kernel panic - not syncing: Asynchronous SError Interrupt ]---
> 
> >
> Yes, I think this is another defect in the `stmmac` driver related to the suspend/resume process.
> I discovered it a few months ago and implemented a fix for it on our platform.
> 
> 
> The race occurs because stmmac_suspend() stops the network TX queues before disabling NAPI,
> while TX completion processing can wake those queues again.
> The current suspend sequence is:
>   netif_device_detach(ndev);
>   stmmac_disable_all_queues(priv);
>   /* Cancel timers, stop DMA, and suspend the hardware. */
> netif_device_detach() clears the device’s __LINK_STATE_PRESENT bit and stops all TX queues by setting __QUEUE_STATE_DRV_XOFF.
> However, TX NAPI may still be running at this point. In stmmac_tx_clean(), if a TX queue is stopped and enough descriptors are
> available, the driver calls netif_tx_wake_queue(). This clears __QUEUE_STATE_DRV_XOFF without checking whether the device has
> been detached or is being suspended。
> The following interleaving is therefore possible:
>   Suspend path                         TX NAPI / networking stack
>   ------------                         --------------------------
>   netif_device_detach()
>     Stop all TX queues
>                                        stmmac_tx_clean()
>                                          Wake a stopped TX queue
>   stmmac_disable_all_queues()
>     Disable NAPI
>   Stop DMA and suspend the hardware
>                                        A timer triggers an outgoing packet
>                                        sch_direct_xmit()
>                                          Observe an unstopped TX queue
>                                          Call stmmac_xmit()
>                                        Access unavailable MAC/DMA registers
>                                        Potential asynchronous SError
> 
> 
> For the normal skb transmit path, both conditions must therefore be established before shutting down the hardware:
> TX NAPI can no longer wake the queues.
> All TX queues are stopped, and any in-flight stmmac_xmit() calls have completed.
> 
> 
> 
> 
> >Which I was able to fix with:
> >
> >--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >@@ -8606,6 +8606,9 @@ int stmmac_suspend(struct device *dev)
> >
> >        stmmac_disable_all_queues(priv);
> >
> >+       /* NAPI may have woken a TX queue after netif_device_detach() */
> >+       netif_tx_disable(ndev);
> >+
> >        for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
> >                hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);
> >
> >This issue IMHO looks similar to the one fixed by
> >commit 7e87508b5c4d ("net: bcmgenet: stop Tx NAPI before disabling the queues").
> >
> >Are you getting the same? What do you think?
> >Thanks in advance.
> >
> I previously resolved this by swapping the order of the calls to `netif_device_detach` and `stmmac_disable_all_queues`.
> However, there is still a separate synchronization gap: stopping a TX queue does not wait for an already-running stmmac_xmit()
> to finish. Reordering the calls so that NAPI is disabled before netif_device_detach() prevents NAPI from reopening the queues,
> but a concurrent transmitter may have already passed the queue-state check and entered the driver. Suspend can then proceed
> toward hardware shutdown while that transmitter is still accessing the device.
> So, I believe the perfect solution would be the modification you’ve proposed. Could you please submit your changes?

Thanks for sharing.
Can you please rebase your series on top of latest net-next/main and send
another version? In this way I will send my patch on top of your work.

This could be a valid plan for you?
Thanks in advance.

Kind regards,
Tommaso

> 
> >Kind regards,
> >Tommaso
> >
> >> - Native XDP/AF_XDP zero-copy RX continued after two suspend/resume cycles
> >>   with the receive socket kept open.
> >> - Zero-copy TX-only, with no RX fill buffers, is tested ok after one suspend/resume
> >>   cycle, with no fatal bus error.
> >> - Pausing the receive application with SIGSTOP and resuming it with SIGCONT
> >>   recovered RX without recreating the socket.
> >> - xdpsock is used for native XDP testing.
> >> 
> >> Additional testing would be welcome for RX sockets initially supplied with
> >> 0/1/8/15 fill buffers, replenishment in batches below 16, and those states
> >> across suspend/resume. These differ from TX-only with no RX fill buffers.
> >> Legacy descriptor cores, GMAC4, chain mode, timestamp-specific descriptor
> >> paths, multiple XSK queues and long suspend/resume runs remain untested.
> >> 
> >> Missing page_pool buffers are still allocated during resume. Allocation
> >> failure returns -ENOMEM without restarting DMA and leaves the device
> >> detached. This adds a trigger for existing resume error-recovery problems;
> >> full close/open recovery, including NAPI and platform power handling, is
> >> separate work. The Medium-severity AI review on failure recovery is not
> >> fully resolved by this revision.
> >> 
> >> Changes in v6:
> >> - Add a separate fix to restore the bound XSK TX pool after resume cleanup,
> >>   addressing TX-only stalls found during XGMAC validation.
> >> - Factor resume descriptor work into one TX queue loop and one RX loop,
> >>   following Thomas's review, and use common error cleanup.
> >> - Restore addresses and chain pointers before initializing RX control fields,
> >>   preserving legacy fields and RIWT/IOC policy without a separate OWN loop.
> >> - Add format-specific RX ownership clearing for unfilled XSK descriptors.
> >> - Track pending XSK refills independently of circular index equality; retry
> >>   short refills and prevent consumption of unfilled descriptors.
> >> - Restore a populated XSK prefix and retain later attached buffers for refill;
> >>   recompute the allocation count, initial tail and pending region.
> >> - Select XSK buffer size independently of the initial allocation count.
> >> - Discard any partially assembled RX frame before resetting the RX ring after resume.
> >> 
> >> Link to v5:
> >> https://lore.kernel.org/netdev/20261025100636.613670-1-dinghui1111@163.com/
> >> 
> >> Changes in v5:
> >> - Target net and add Fixes tags to both patches.
> >> - Split the allocation-flags fix into a separate patch, with a Fixes tag
> >>   identifying the page_pool conversion that stopped honoring the flags.
> >> - Share page_pool and XSK buffer preparation with the RX refill paths.
> >> - Allocate missing secondary pages even when the primary page exists,
> >>   using the current baseline's buffer2 capability check.
> >> - Condense the reinitialization kernel-doc to two paragraphs.
> >> - Restore descriptor addresses before the final dma_wmb()/OWN pass.
> >> - Clarify that unavailable XSK slots remain CPU-owned and may stop DMA
> >>   with RBU, rather than claiming that hardware skips them.
> >> - Power down legacy SerDes if RX descriptor restoration fails.
> >> 
> >> Link to v4:
> >> https://lore.kernel.org/netdev/20260627122533.1165324-1-dinghui1111@163.com/
> >> 
> >> Changes in v4:
> >> - Document the return value of stmmac_reinit_rx_descriptors().
> >> 
> >> Link to v3:
> >> https://lore.kernel.org/netdev/20260604144557.3175399-1-dinghui1111@163.com/
> >> 
> >> Changes in v3:
> >> - Allocate missing page_pool buffers with GFP_KERNEL and abort resume
> >>   on allocation failure.
> >> - Try to allocate missing XSK buffers and clear descriptors when the
> >>   fill ring cannot supply a buffer.
> >> - Restore OWN after programming addresses and rebuilding chain pointers.
> >> - Honor the flags argument of stmmac_init_rx_buffers().
> >> - Restore RX descriptors after stmmac_clear_descriptors().
> >> 
> >> Link to v2:
> >> https://lore.kernel.org/netdev/20260526022620.501229-1-dinghui1111@163.com/
> >> 
> >> Changes in v2:
> >> - Introduce a helper to restore addresses from existing RX buffers.
> >> - Update the commit message.
> >> 
> >> Link to v1:
> >> https://lore.kernel.org/netdev/20260515053856.2310369-1-dinghui1111@163.com/
> >> 
> >> Ding Hui (4):
> >>   net: stmmac: honor allocation flags when initializing RX buffers
> >>   net: stmmac: track unfilled XSK RX descriptors explicitly
> >>   net: stmmac: restore RX descriptor buffer addresses on resume
> >>   net: stmmac: restore the XSK TX pool on resume
> >> 
> >>  .../ethernet/stmicro/stmmac/dwmac4_descs.c    |   6 +
> >>  .../ethernet/stmicro/stmmac/dwxgmac2_descs.c  |   6 +
> >>  .../net/ethernet/stmicro/stmmac/enh_desc.c    |   6 +
> >>  drivers/net/ethernet/stmicro/stmmac/hwif.h    |   3 +
> >>  .../net/ethernet/stmicro/stmmac/norm_desc.c   |   6 +
> >>  drivers/net/ethernet/stmicro/stmmac/stmmac.h  |   2 +
> >>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 277 +++++++++++++-----
> >>  7 files changed, 228 insertions(+), 78 deletions(-)
> >> 
> >> 
> >> base-commit: 7076f684d5132e6a6145e1da2fa74771ecd052e5
> >> -- 
> >> 2.34.1
> >>