* [PATCH net v5 0/2] net: stmmac: restore RX descriptors on resume
@ 2026-10-25 10:06 Ding Hui
2026-10-25 10:06 ` [PATCH net v5 1/2] net: stmmac: honor allocation flags when initializing RX buffers Ding Hui
2026-10-25 10:06 ` [PATCH net v5 2/2] net: stmmac: restore RX descriptor buffer addresses on resume Ding Hui
0 siblings, 2 replies; 4+ messages in thread
From: Ding Hui @ 2026-10-25 10:06 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
open list:STMMAC ETHERNET DRIVER,
moderated list:ARM/STM32 ARCHITECTURE,
moderated list:ARM/STM32 ARCHITECTURE, open list
Cc: dinghui, xiasanbo, yangchen11, liuxuanjun, zhoujing3, fujiahui
From: Ding Hui <dinghui@lixiang.com>
This series fixes RX descriptor buffer addresses that hardware writeback
may overwrite between disabling NAPI and stopping RX DMA during suspend.
Restoring these addresses before restarting DMA on resume prevents a
fatal bus error.
Patch 1 fixes the ignored allocation flags independently. Patch 2 shares
buffer preparation with the normal and zero-copy refill paths and restores
RX descriptor addresses on resume. Missing primary and secondary pages
are allocated with GFP_KERNEL; failure aborts resume. Unavailable XSK slots
remain CPU-owned, and all address writes precede the final OWN update.
The earlier fix, commit 9c63faaa931e ("net: stmmac: re-init rx buffers when
mac resume back"), was reverted by commit 00423969d806 after a Jetson TX2
suspend/resume regression. This series retains attached buffers and their
DMA mappings and only allocates missing buffers. It does not repeat the
recycle-and-reallocate sequence of that fix. Jetson TX2 has not been tested
with this series.
Reproduction and testing:
On our hardware platform, run iperf traffic from a PC to the platform
(the platform is the receiver), and suspend/resume the platform while
traffic is running. Without the fix, a fatal bus error occurs after
resume in almost every run. With the fix applied, the fatal bus error
was no longer observed under the same test conditions.
This comparison was performed using a backport of this series to a kernel
based on Linux 6.12.41, running on our hardware platform that integrates
a Synopsys XGMAC.
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 (2):
net: stmmac: honor allocation flags when initializing RX buffers
net: stmmac: restore RX descriptor buffer addresses on resume
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 190 +++++++++++++-----
1 file changed, 138 insertions(+), 52 deletions(-)
base-commit: 7076f684d5132e6a6145e1da2fa74771ecd052e5
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH net v5 1/2] net: stmmac: honor allocation flags when initializing RX buffers 2026-10-25 10:06 [PATCH net v5 0/2] net: stmmac: restore RX descriptors on resume Ding Hui @ 2026-10-25 10:06 ` Ding Hui 2026-10-25 10:06 ` [PATCH net v5 2/2] net: stmmac: restore RX descriptor buffer addresses on resume Ding Hui 1 sibling, 0 replies; 4+ messages in thread From: Ding Hui @ 2026-10-25 10:06 UTC (permalink / raw) To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, open list:STMMAC ETHERNET DRIVER, moderated list:ARM/STM32 ARCHITECTURE, moderated list:ARM/STM32 ARCHITECTURE, open list Cc: dinghui, xiasanbo, yangchen11, liuxuanjun, zhoujing3, fujiahui From: Ding Hui <dinghui@lixiang.com> stmmac_init_rx_buffers() accepts allocation flags but stopped using them when RX buffers were converted to page_pool. As a result, callers passing GFP_KERNEL still allocate with GFP_ATOMIC | __GFP_NOWARN and cannot use reclaim to satisfy an allocation. Use the supplied flags while retaining GFP_DMA32 for hardware without 64-bit DMA addressing. This also allows the resume path to allocate missing RX pages with GFP_KERNEL. Fixes: 2af6106ae949 ("net: stmmac: Introducing support for Page Pool") Signed-off-by: Ding Hui <dinghui@lixiang.com> --- Changes in v5: - Split this fix from the resume fix as requested. - Add a Fixes tag for the page_pool conversion. drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index b20e5038eead..f5ae12208e85 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1688,7 +1688,7 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, { struct stmmac_rx_queue *rx_q = &dma_conf->rx_queue[queue]; struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i]; - gfp_t gfp = (GFP_ATOMIC | __GFP_NOWARN); + gfp_t gfp = flags; if (priv->dma_cap.host_dma_width <= 32) gfp |= GFP_DMA32; -- 2.34.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH net v5 2/2] net: stmmac: restore RX descriptor buffer addresses on resume 2026-10-25 10:06 [PATCH net v5 0/2] net: stmmac: restore RX descriptors on resume Ding Hui 2026-10-25 10:06 ` [PATCH net v5 1/2] net: stmmac: honor allocation flags when initializing RX buffers Ding Hui @ 2026-10-25 10:06 ` Ding Hui 2026-10-26 11:10 ` netdev-bot+sashiko 1 sibling, 1 reply; 4+ messages in thread From: Ding Hui @ 2026-10-25 10:06 UTC (permalink / raw) To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, open list:STMMAC ETHERNET DRIVER, moderated list:ARM/STM32 ARCHITECTURE, moderated list:ARM/STM32 ARCHITECTURE, open list Cc: dinghui, xiasanbo, yangchen11, liuxuanjun, zhoujing3, fujiahui From: Ding Hui <dinghui@lixiang.com> stmmac_suspend() disables NAPI before stopping RX DMA. Frames received in that interval can leave completed descriptors that have not been refilled. On GMAC4 and XGMAC, hardware writeback overwrites the descriptor buffer addresses with status and timestamp data. stmmac_resume() calls stmmac_clear_descriptors(), which returns those descriptors to DMA without restoring their buffer addresses. Restarting RX DMA can therefore cause a fatal bus error. Restore the addresses from the attached buffers before restarting DMA. Factor buffer allocation and address programming into helpers shared with the normal and zero-copy refill paths. Allocate missing primary and secondary pages with GFP_KERNEL, including a missing secondary page when the primary page was allocated before a refill failure. Abort resume if a required page cannot be allocated. For XSK queues, try to allocate missing buffers but leave unavailable slots CPU-owned so an empty fill ring does not prevent TX-only use. DMA may stop at such a slot with an RBU event; it must not receive an invalid buffer address. Rebuild chain pointers that timestamp writeback may have overwritten. Publish OWN only after all address fields have been restored, since programming the secondary address overwrites OWN on GMAC4 and XGMAC. Keep existing buffers and their DMA mappings rather than recycling them as the earlier, reverted resume fix did. Fixes: f748be531d70 ("stmmac: support new GMAC4") Signed-off-by: Ding Hui <dinghui@lixiang.com> --- Changes in v5: - Target net and add a Fixes tag for GMAC4 support. - Share buffer preparation helpers with both RX refill paths. - Allocate missing secondary pages independently of primary pages. - Condense the kernel-doc and clarify CPU-owned XSK slots/RBU handling. - Rebase onto net/main and retain SerDes cleanup on resume failure. The allocation-flags fix is now patch 1/2 of this series. .../net/ethernet/stmicro/stmmac/stmmac_main.c | 190 +++++++++++++----- 1 file changed, 138 insertions(+), 52 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index f5ae12208e85..e56f5d044fc8 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1670,6 +1670,40 @@ static bool stmmac_rx_check_buf2_cap(struct stmmac_priv *priv) return priv->hw->desc && priv->hw->desc->set_sec_addr; } +static int stmmac_prepare_rx_buffer(struct stmmac_priv *priv, + struct stmmac_rx_queue *rx_q, + struct stmmac_rx_buffer *buf, + struct dma_desc *p, gfp_t flags) +{ + gfp_t gfp = flags; + + if (priv->dma_cap.host_dma_width <= 32) + gfp |= GFP_DMA32; + + if (!buf->page) { + buf->page = page_pool_alloc_pages(rx_q->page_pool, gfp); + if (!buf->page) + return -ENOMEM; + buf->page_offset = stmmac_rx_offset(priv); + } + + if (stmmac_rx_check_buf2_cap(priv) && !buf->sec_page) { + buf->sec_page = page_pool_alloc_pages(rx_q->page_pool, gfp); + if (!buf->sec_page) + return -ENOMEM; + + buf->sec_addr = page_pool_get_dma_addr(buf->sec_page); + } + + buf->addr = page_pool_get_dma_addr(buf->page) + buf->page_offset; + + stmmac_set_desc_addr(priv, p, buf->addr); + stmmac_set_desc_sec_addr(priv, p, buf->sec_addr, + stmmac_rx_check_buf2_cap(priv)); + + return 0; +} + /** * stmmac_init_rx_buffers - init the RX descriptor buffer. * @priv: driver private structure @@ -1688,32 +1722,98 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, { struct stmmac_rx_queue *rx_q = &dma_conf->rx_queue[queue]; struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i]; - gfp_t gfp = flags; + int ret; - if (priv->dma_cap.host_dma_width <= 32) - gfp |= GFP_DMA32; + ret = stmmac_prepare_rx_buffer(priv, rx_q, buf, p, flags); + if (ret) + return ret; - if (!buf->page) { - buf->page = page_pool_alloc_pages(rx_q->page_pool, gfp); - if (!buf->page) + if (dma_conf->dma_buf_sz == BUF_SIZE_16KiB) + stmmac_init_desc3(priv, p); + + return 0; +} + +static int stmmac_prepare_rx_buffer_zc(struct stmmac_priv *priv, + struct stmmac_rx_queue *rx_q, + struct stmmac_rx_buffer *buf, + struct dma_desc *p) +{ + if (!buf->xdp) { + buf->xdp = xsk_buff_alloc(rx_q->xsk_pool); + if (!buf->xdp) return -ENOMEM; - buf->page_offset = stmmac_rx_offset(priv); } - if (stmmac_rx_check_buf2_cap(priv) && !buf->sec_page) { - buf->sec_page = page_pool_alloc_pages(rx_q->page_pool, gfp); - if (!buf->sec_page) - return -ENOMEM; + stmmac_set_desc_addr(priv, p, xsk_buff_xdp_get_dma(buf->xdp)); + stmmac_set_desc_sec_addr(priv, p, 0, false); - buf->sec_addr = page_pool_get_dma_addr(buf->sec_page); - stmmac_set_desc_sec_addr(priv, p, buf->sec_addr, true); + return 0; +} + +/** + * stmmac_reinit_rx_descriptors - restore RX descriptors before restarting DMA + * @priv: driver private structure + * @dma_conf: DMA configuration + * @queue: RX queue index + * + * Restore buffer addresses overwritten by hardware writeback after + * stmmac_clear_descriptors(). Reuse attached buffers and allocate missing + * primary and secondary pages with GFP_KERNEL before restarting DMA. + * + * For XSK queues, try to allocate missing buffers and leave unavailable + * descriptors CPU-owned. Rebuild chain pointers when needed, then return + * valid descriptors to DMA after restoring all address fields. + * + * Return: 0 on success, or -ENOMEM if a required page cannot be allocated. + */ +static int stmmac_reinit_rx_descriptors(struct stmmac_priv *priv, + struct stmmac_dma_conf *dma_conf, + u32 queue) +{ + struct stmmac_rx_queue *rx_q = &dma_conf->rx_queue[queue]; + struct stmmac_rx_buffer *buf; + struct dma_desc *p; + int i, ret; + + for (i = 0; i < dma_conf->dma_rx_size; i++) { + buf = &rx_q->buf_pool[i]; + p = stmmac_get_rx_desc(priv, rx_q, i); + + if (rx_q->xsk_pool) { + ret = stmmac_prepare_rx_buffer_zc(priv, rx_q, buf, p); + if (ret) { + /* An empty fill ring must not prevent TX-only use. */ + stmmac_clear_desc(priv, p); + continue; + } + } else { + ret = stmmac_init_rx_buffers(priv, dma_conf, p, i, + GFP_KERNEL, queue); + if (ret) + return ret; + } } - buf->addr = page_pool_get_dma_addr(buf->page) + buf->page_offset; + /* Hardware timestamp writeback may have overwritten chain pointers. */ + if (priv->descriptor_mode == STMMAC_CHAIN_MODE) { + void *des = priv->extend_desc ? (void *)rx_q->dma_erx : + (void *)rx_q->dma_rx; - stmmac_set_desc_addr(priv, p, buf->addr); - if (dma_conf->dma_buf_sz == BUF_SIZE_16KiB) - stmmac_init_desc3(priv, p); + stmmac_mode_init(priv, des, rx_q->dma_rx_phy, + dma_conf->dma_rx_size, priv->extend_desc); + } + + /* GMAC4/XGMAC secondary address writes overwrite the OWN bit. */ + dma_wmb(); + for (i = 0; i < dma_conf->dma_rx_size; i++) { + buf = &rx_q->buf_pool[i]; + if (rx_q->xsk_pool && !buf->xdp) + continue; + + p = stmmac_get_rx_desc(priv, rx_q, i); + stmmac_set_rx_owner(priv, p, false); + } return 0; } @@ -5145,10 +5245,6 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue) struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue]; int dirty = stmmac_rx_dirty(priv, queue); unsigned int entry = rx_q->dirty_rx; - gfp_t gfp = (GFP_ATOMIC | __GFP_NOWARN); - - if (priv->dma_cap.host_dma_width <= 32) - gfp |= GFP_DMA32; while (dirty-- > 0) { struct stmmac_rx_buffer *buf = &rx_q->buf_pool[entry]; @@ -5157,25 +5253,9 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue) p = stmmac_get_rx_desc(priv, rx_q, entry); - if (!buf->page) { - buf->page = page_pool_alloc_pages(rx_q->page_pool, gfp); - if (!buf->page) - break; - } - - if (stmmac_rx_check_buf2_cap(priv) && !buf->sec_page) { - buf->sec_page = page_pool_alloc_pages(rx_q->page_pool, gfp); - if (!buf->sec_page) - break; - - buf->sec_addr = page_pool_get_dma_addr(buf->sec_page); - } - - buf->addr = page_pool_get_dma_addr(buf->page) + buf->page_offset; - - stmmac_set_desc_addr(priv, p, buf->addr); - stmmac_set_desc_sec_addr(priv, p, buf->sec_addr, - stmmac_rx_check_buf2_cap(priv)); + if (stmmac_prepare_rx_buffer(priv, rx_q, buf, p, + GFP_ATOMIC | __GFP_NOWARN)) + break; stmmac_refill_desc3(priv, rx_q, p); use_rx_wd = !stmmac_rx_need_ic(priv, rx_q, queue); @@ -5578,22 +5658,15 @@ static bool stmmac_rx_refill_zc(struct stmmac_priv *priv, u32 queue, u32 budget) while (budget-- > 0 && entry != rx_q->cur_rx) { struct stmmac_rx_buffer *buf = &rx_q->buf_pool[entry]; - dma_addr_t dma_addr; + struct dma_desc *p = stmmac_get_rx_desc(priv, rx_q, entry); bool use_rx_wd; - if (!buf->xdp) { - buf->xdp = xsk_buff_alloc(rx_q->xsk_pool); - if (!buf->xdp) { - ret = false; - break; - } + if (stmmac_prepare_rx_buffer_zc(priv, rx_q, buf, p)) { + ret = false; + break; } - rx_desc = stmmac_get_rx_desc(priv, rx_q, entry); - - dma_addr = xsk_buff_xdp_get_dma(buf->xdp); - stmmac_set_desc_addr(priv, rx_desc, dma_addr); - stmmac_set_desc_sec_addr(priv, rx_desc, 0, false); + rx_desc = p; stmmac_refill_desc3(priv, rx_q, rx_desc); use_rx_wd = !stmmac_rx_need_ic(priv, rx_q, queue); @@ -8454,6 +8527,7 @@ int stmmac_resume(struct device *dev) { struct net_device *ndev = dev_get_drvdata(dev); struct stmmac_priv *priv = netdev_priv(ndev); + u32 queue; int ret; if (priv->plat->resume) { @@ -8503,6 +8577,18 @@ int stmmac_resume(struct device *dev) stmmac_free_tx_skbufs(priv); stmmac_clear_descriptors(priv, &priv->dma_conf); + for (queue = 0; queue < priv->plat->rx_queues_to_use; queue++) { + ret = stmmac_reinit_rx_descriptors(priv, &priv->dma_conf, queue); + if (ret) { + netdev_err(ndev, "Failed to restore RX queue %u: %d\n", + queue, ret); + stmmac_legacy_serdes_power_down(priv); + mutex_unlock(&priv->lock); + rtnl_unlock(); + return ret; + } + } + ret = stmmac_hw_setup(ndev); if (ret < 0) { netdev_err(priv->dev, "%s: Hw setup failed\n", __func__); -- 2.34.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v5 2/2] net: stmmac: restore RX descriptor buffer addresses on resume 2026-10-25 10:06 ` [PATCH net v5 2/2] net: stmmac: restore RX descriptor buffer addresses on resume Ding Hui @ 2026-10-26 11:10 ` netdev-bot+sashiko 0 siblings, 0 replies; 4+ messages in thread From: netdev-bot+sashiko @ 2026-10-26 11:10 UTC (permalink / raw) To: dinghui1111 Cc: maxime.chevallier, andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32, alexandre.torgue, netdev, linux-stm32, linux-arm-kernel, linux-kernel, dinghui, xiasanbo, yangchen11, liuxuanjun, zhoujing3, fujiahui Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider. Critical: 0 · High: 2 · Medium: 1 · Low: 1 (1) If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [High] On cores with legacy enhanced/normal descriptors (DWMAC100, DWMAC1000, DWMAC 3.x: enh_desc_ops and ndesc_ops), an XSK RX slot with… - [High] On GMAC4/XGMAC, the XSK 'leave CPU-owned holes' approach puts unfilled slots in the middle of the ring, inside the region the RX… - [Medium] The patch adds an allocation-dependent way for stmmac_resume() to fail, and that failure leaves the netdev unrecoverable. - [Low] stmmac_reinit_rx_descriptors() publishes every restored descriptor with stmmac_set_rx_owner(priv, p, false), so… Pre-existing issues: - [Low] stmmac_reinit_rx_descriptors() can attach XSK buffers to slots that had none at open or pool-enable time, but it never updates… -- net: stmmac: restore RX descriptor buffer addresses on resume This patch restores RX descriptor buffer addresses in stmmac_resume() after stmmac_clear_descriptors(). On GMAC4/XGMAC, hardware writeback can overwrite those addresses for descriptors that completed but were not refilled before suspend. It moves buffer setup into stmmac_prepare_rx_buffer() and stmmac_prepare_rx_buffer_zc(), which the refill paths also use, allocates missing pages with GFP_KERNEL, and leaves XSK slots that have no buffer CPU-owned. > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index f5ae12208e85b..e56f5d044fc8a 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c [ ... ] > @@ -1688,32 +1722,98 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, [ ... ] > +static int stmmac_reinit_rx_descriptors(struct stmmac_priv *priv, > + struct stmmac_dma_conf *dma_conf, > + u32 queue) > +{ > + struct stmmac_rx_queue *rx_q = &dma_conf->rx_queue[queue]; > + struct stmmac_rx_buffer *buf; > + struct dma_desc *p; > + int i, ret; > + > + for (i = 0; i < dma_conf->dma_rx_size; i++) { > + buf = &rx_q->buf_pool[i]; > + p = stmmac_get_rx_desc(priv, rx_q, i); > + > + if (rx_q->xsk_pool) { > + ret = stmmac_prepare_rx_buffer_zc(priv, rx_q, buf, p); [Severity: Low] This is a pre-existing issue, not one introduced by this patch, but rx_q->buf_alloc_num is not updated when this loop attaches XSK buffers to slots that had none at open time. stmmac_hw_setup()->stmmac_init_dma_engine() programs the RX tail from it: stmmac_set_queue_rx_tail_ptr(priv, rx_q, chan, rx_q->buf_alloc_num); Also, stmmac_set_queue_rx_buf_size() uses the XSK frame size only when buf_alloc_num is non-zero. If the fill ring was short at open, slots filled here at index buf_alloc_num or above may lie beyond the programmed tail. If buf_alloc_num is 0, the buffer size falls back to dma_buf_sz. The open path uses the same stale value, and so did the code before this patch, so this is only a bookkeeping mismatch. > + if (ret) { > + /* An empty fill ring must not prevent TX-only use. */ > + stmmac_clear_desc(priv, p); > + continue; > + } [Severity: High] On cores with the legacy enh_desc_ops or ndesc_ops, does this leave the slot DMA-owned with a buffer address of 0? stmmac_clear_descriptors() has just run init_rx_desc on every RX descriptor, and that sets OWN in des0: enh_desc_init_rx_desc() { ... p->des0 |= cpu_to_le32(RDES0_OWN); ... } On these cores, the ->clear op only clears des2: static void enh_desc_clear(struct dma_desc *p) { p->des2 = 0; } static void ndesc_clear(struct dma_desc *p) { p->des2 = 0; } The final loop below skips this slot when !buf->xdp, but nothing clears OWN. These DMA engines have no RX tail pointer, so the DMA keeps walking the ring while OWN is set. The next frame that lands in this slot would be written to bus address 0. XSK zero-copy is advertised on every core (NETDEV_XDP_ACT_XSK_ZEROCOPY in xdp_features), and stmmac_xdp_enable_pool() has no descriptor-format check, so these cores can reach this path. Before this patch, des2 here still held the previous, still-mapped UMEM chunk address. The commit message says such a slot "must not receive an invalid buffer address". Could OWN be cleared explicitly here, for example RDES0_OWN in des0 on enh/ndesc? Only dwmac4 and xgmac zero the whole descriptor in ->clear. [Severity: High] On GMAC4/XGMAC, can a CPU-owned hole in the middle of the ring stall RX on this queue permanently? stmmac_reset_queues_param() has set cur_rx = dirty_rx = 0, so a hole at slot D sits in the region the RX path treats as hardware-owned. The DMA fills slots 0 to D-1 and stops at D with an RBU event. In stmmac_rx_zc(), dwmac4_wrback_get_rx_status() does not report a fully zeroed descriptor as dma_own. So cur_rx moves to D+1 before the missing buffer is noticed: stmmac_rx_zc() { ... rx_q->cur_rx = next_entry; ... if (!buf->xdp) break; ... } stmmac_rx_zc() is the only caller of the refill, and only once the dirty count reaches STMMAC_RX_FILL_BATCH: if (dirty >= STMMAC_RX_FILL_BATCH) { failure = failure || !stmmac_rx_refill_zc(priv, queue, dirty); If D < 15, dirty stays at D+1, the hole is never refilled, and the DMA stays stopped at D. stmmac_xsk_wakeup() only schedules NAPI, so refilling the fill ring from userspace does not recover the queue. If D >= 15, a later refill fills D and moves the tail. Software has already moved past D, though, so that frame would be consumed a full ring later and out of order. Could the holes be placed in the dirty region instead, for example by setting dirty_rx and the tail to the first unfilled slot? > + } else { > + ret = stmmac_init_rx_buffers(priv, dma_conf, p, i, > + GFP_KERNEL, queue); > + if (ret) > + return ret; > + } > } [ ... ] > + /* GMAC4/XGMAC secondary address writes overwrite the OWN bit. */ > + dma_wmb(); > + for (i = 0; i < dma_conf->dma_rx_size; i++) { > + buf = &rx_q->buf_pool[i]; > + if (rx_q->xsk_pool && !buf->xdp) > + continue; > + > + p = stmmac_get_rx_desc(priv, rx_q, i); > + stmmac_set_rx_owner(priv, p, false); [Severity: Low] Should the disable_rx_ic argument here follow priv->use_riwt instead of always being false? stmmac_clear_rx_descriptors() passes priv->use_riwt as disable_rx_ic. On GMAC4/XGMAC that choice is lost, because dwmac4_set_sec_addr() assigns des3 outright: p->des2 = cpu_to_le32(lower_32_bits(addr)); p->des3 = cpu_to_le32(upper_32_bits(addr)); This loop then always sets IOC through dwmac4_set_rx_owner(): if (!disable_rx_ic) flags |= RDES3_INT_ON_COMPLETION_EN; With RIWT coalescing enabled, every descriptor in the first ring pass after resume would raise a completion interrupt. That continues until stmmac_rx_refill() or stmmac_rx_refill_zc() re-arm the descriptors using stmmac_rx_need_ic(). > + } > > return 0; > } [ ... ] > @@ -8503,6 +8577,18 @@ int stmmac_resume(struct device *dev) > stmmac_free_tx_skbufs(priv); > stmmac_clear_descriptors(priv, &priv->dma_conf); > > + for (queue = 0; queue < priv->plat->rx_queues_to_use; queue++) { > + ret = stmmac_reinit_rx_descriptors(priv, &priv->dma_conf, queue); > + if (ret) { > + netdev_err(ndev, "Failed to restore RX queue %u: %d\n", > + queue, ret); > + stmmac_legacy_serdes_power_down(priv); > + mutex_unlock(&priv->lock); > + rtnl_unlock(); > + return ret; > + } > + } [Severity: Medium] If stmmac_reinit_rx_descriptors() returns -ENOMEM here, what state is the netdev left in? This early return skips stmmac_enable_all_queues(), phylink_resume() and netif_device_attach(). NAPI stays disabled from stmmac_suspend(), the device stays detached, and phylink_prepare_resume() is never matched by phylink_resume(). netif_running() is still true. A later close then runs: dev_close() stmmac_release() __stmmac_release() stmmac_disable_all_queues() napi_disable() on NAPI instances that are already disabled. napi_disable_locked() waits for NAPIF_STATE_SCHED to clear, but that bit is still set from the first disable, so the close would hang with RTNL held. The existing stmmac_hw_setup() failure path has the same problem. This patch adds a new trigger that depends on a GFP_KERNEL allocation succeeding during system resume. The XSK branch already tolerates missing buffers. Could the page_pool branch do the same, for example by leaving the slot CPU-owned for the refill path to retry, instead of failing the whole resume? > ret = stmmac_hw_setup(ndev); > if (ret < 0) { > netdev_err(priv->dev, "%s: Hw setup failed\n", __func__); -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261025100636.613670-1-dinghui1111%40163.com ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-26 11:10 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-25 10:06 [PATCH net v5 0/2] net: stmmac: restore RX descriptors on resume Ding Hui 2026-10-25 10:06 ` [PATCH net v5 1/2] net: stmmac: honor allocation flags when initializing RX buffers Ding Hui 2026-10-25 10:06 ` [PATCH net v5 2/2] net: stmmac: restore RX descriptor buffer addresses on resume Ding Hui 2026-10-26 11:10 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®