[PATCH 0/1] net: stmmac: descriptor management issue during PM sequence

[PATCH 0/1] net: stmmac: descriptor management issue during PM sequence

[Thomas Bourgoin]

Hello,

During low power sequence on STM32MP21 we sometimes observe out of range access
to the DDR by the GMAC ethernet. I managed to narrow down the issue to the
resume sequence of the RX descriptors with the function
stmmac_clear_descriptors().

On resume, stmmac_clear_descriptors() returns RX descriptors to the DMA
without restoring their buffer addresses. On DWMAC4, the DMA will
therefore consider a descriptor for the application as one for the DMA.
This can lead to illegal access from the DMA anywhere.
On STM32MP21 the DMA address bus is 40 bits, thus the extended status from the
write-back descriptor is read as an high address by the DMA. DMA will then try
to write in the DDR at 0x100000000 whereas on STM32MP215f-dk DDR is only 2Gb
from 0x8000 0000 to 0xffff ffff and nothing is mapped at 0x100000000.
All the other SoCs of the STM32MP1x or STM32MP2x are using a GMAC with a 32bits
DMA address, so I do not have illegal attemps.

To fix this, I propose to reconstruct the RX descriptors that have been used by
the DMA but not forwarded to the NAPI function stmmac_rx() before suspend.

I'm not used to this driver especially the xsk management and zero copy part.
Since this driver is used by a lot of plateforms I'm open to all suggestions.

This V1 propose to reuse stmmac_init_rx_buffers() but maybe using
stmmac_refill() would better fit the issue. From what I read, I could not call
stmmac_refill() as rx_q->cur_rx is only updated in stmmac_rx(), so
stmmac_rx_dirty() returns 0.

Thomas Bourgoin (1):
  net: stmmac: rebuild RX descriptors before restarting DMA on resume

 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 86 +++++++++++++++----
 1 file changed, 68 insertions(+), 18 deletions(-)

-- 
2.43.0

[PATCH 1/1] net: stmmac: rebuild RX descriptors before restarting DMA on resume

[Thomas Bourgoin]

An RX descriptor can be written back by the DMA before suspend without
being processed by NAPI. Its buffer address fields then contain receive
status rather than DMA addresses.

On resume, stmmac_clear_descriptors() returns RX descriptors to the DMA
without restoring their buffer addresses. On DWMAC4, the DMA will
therefore consider a descriptor for the application as one for the DMA.
This can lead to illegal access from the DMA anywhere.
For example, on STM32MP21 the DMA address bus is 40 bits, thus the
extended status from the write-back descriptor is read as an high
address by the DMA. DMA will then try to write in the DDR at 0x100000000
whereas on STM32MP215f-dk DDR is only 2Gb from 0x8000 0000 to
0xffff ffff and nothing is mapped at 0x100000000.

To fix this, reconstruct the entire RX ring before rearming descriptors
and restarting the DMA. Reuse existing buffers, allocate missing ones,
and restore their DMA addresses independently of the saved RX producer
and consumer indices. Reset the queue indices only after descriptor
reconstruction succeeds.

Update stmmac_init_rx_buffers() to preserve an existing split-header
secondary page and restore its DMA address. Fold TX buffer cleanup into
the descriptor resume helper before clearing the TX descriptors.

Signed-off-by: Thomas Bourgoin <thomas.bourgoin@foss.st.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 86 +++++++++++++++----
 1 file changed, 68 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 9ded31150fbe..198629265c90 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1700,8 +1700,10 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv,
 		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 (stmmac_rx_check_buf2_cap(priv)) {
+		if (!buf->sec_page)
+			buf->sec_page = page_pool_alloc_pages(rx_q->page_pool,
+							      gfp);
 		if (!buf->sec_page)
 			return -ENOMEM;
 
@@ -2149,19 +2151,6 @@ static void dma_free_tx_skbufs(struct stmmac_priv *priv,
 	}
 }
 
-/**
- * stmmac_free_tx_skbufs - free TX skb buffers
- * @priv: private structure
- */
-static void stmmac_free_tx_skbufs(struct stmmac_priv *priv)
-{
-	u8 tx_queue_cnt = priv->plat->tx_queues_to_use;
-	u8 queue;
-
-	for (queue = 0; queue < tx_queue_cnt; queue++)
-		dma_free_tx_skbufs(priv, &priv->dma_conf, queue);
-}
-
 /**
  * __free_dma_rx_desc_resources - free RX dma desc resources (per queue)
  * @priv: private structure
@@ -8425,6 +8414,63 @@ static void stmmac_reset_rx_queue(struct stmmac_priv *priv, u32 queue)
 	rx_q->dirty_rx = 0;
 }
 
+static int stmmac_resume_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];
+	int i;
+	int ret;
+
+	for (i = 0; i < dma_conf->dma_rx_size; i++) {
+		struct dma_desc *p = stmmac_get_rx_desc(priv, rx_q, i);
+		struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i];
+		dma_addr_t dma_addr;
+
+		if (rx_q->xsk_pool) {
+			if (!buf->xdp) {
+				buf->xdp = xsk_buff_alloc(rx_q->xsk_pool);
+				if (!buf->xdp)
+					return -ENOMEM;
+				}
+			dma_addr = xsk_buff_xdp_get_dma(buf->xdp);
+			stmmac_set_desc_addr(priv, p, dma_addr);
+			stmmac_set_desc_sec_addr(priv, p, 0, false);
+		} else {
+			ret = stmmac_init_rx_buffers(priv, dma_conf, p, i,
+						     GFP_KERNEL, queue);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int stmmac_resume_descriptors(struct stmmac_priv *priv,
+				     struct stmmac_dma_conf *dma_conf)
+{
+	u32 rx_queue_cnt = priv->plat->rx_queues_to_use;
+	u32 tx_queue_cnt = priv->plat->tx_queues_to_use;
+	u32 queue;
+	int ret;
+
+	for (queue = 0; queue < rx_queue_cnt; queue++) {
+		ret = stmmac_resume_rx_descriptors(priv, dma_conf, queue);
+		if (ret)
+			return ret;
+
+		stmmac_clear_rx_descriptors(priv, dma_conf, queue);
+	}
+
+	for (queue = 0; queue < tx_queue_cnt; queue++) {
+		dma_free_tx_skbufs(priv, &priv->dma_conf, queue);
+		stmmac_clear_tx_descriptors(priv, dma_conf, queue);
+	}
+
+	return 0;
+}
+
 static void stmmac_reset_tx_queue(struct stmmac_priv *priv, u32 queue)
 {
 	struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue];
@@ -8507,10 +8553,14 @@ int stmmac_resume(struct device *dev)
 
 	mutex_lock(&priv->lock);
 
-	stmmac_reset_queues_param(priv);
+	ret = stmmac_resume_descriptors(priv, &priv->dma_conf);
+	if (ret < 0) {
+		netdev_err(priv->dev, "%s: Resume descriptor failed\n",
+			   __func__);
+		goto error_unlock;
+	}
 
-	stmmac_free_tx_skbufs(priv);
-	stmmac_clear_descriptors(priv, &priv->dma_conf);
+	stmmac_reset_queues_param(priv);
 
 	ret = stmmac_hw_setup(ndev);
 	if (ret < 0) {
-- 
2.43.0

Re: [PATCH 1/1] net: stmmac: rebuild RX descriptors before restarting DMA on resume

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] net: stmmac: stmmac_resume_descriptors() fails to restore chained des3 links
- [High] net: stmmac: stmmac_resume_rx_descriptors() fails on empty or partial XSK pool
- [Medium] net: stmmac: stmmac_init_rx_buffers() ignores flags and uses GFP_ATOMIC
--

commit 8d3c0459b1ad22247b62f5f4182ad5e15b2eab5d
Author: Thomas Bourgoin <thomas.bourgoin@foss.st.com>

net: stmmac: rebuild RX descriptors before restarting DMA on resume

This commit reconstructs RX descriptors and restores their DMA buffer
addresses during system resume before clearing descriptors and restarting
DMA, preventing the DMA engine from treating write-back status fields as
buffer addresses.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 9ded31150fbe6..198629265c904 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

[ ... ]

> @@ -8425,6 +8414,63 @@ static void stmmac_reset_rx_queue(struct stmmac_priv *priv, u32 queue)
>  	rx_q->dirty_rx = 0;
>  }
>  
> +static int stmmac_resume_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];
> +	int i;
> +	int ret;
> +
> +	for (i = 0; i < dma_conf->dma_rx_size; i++) {
> +		struct dma_desc *p = stmmac_get_rx_desc(priv, rx_q, i);
> +		struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i];
> +		dma_addr_t dma_addr;
> +
> +		if (rx_q->xsk_pool) {
> +			if (!buf->xdp) {
> +				buf->xdp = xsk_buff_alloc(rx_q->xsk_pool);
> +				if (!buf->xdp)
> +					return -ENOMEM;
> +				}
> +			dma_addr = xsk_buff_xdp_get_dma(buf->xdp);
> +			stmmac_set_desc_addr(priv, p, dma_addr);
> +			stmmac_set_desc_sec_addr(priv, p, 0, false);

[Severity: High]
Can this break resume when an AF_XDP zero-copy pool is attached with an
empty or partially populated fill ring?

In __init_dma_rx_desc_rings(), an unpopulated XSK pool (such as TX-only
xdpsock) is explicitly allowed:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:__init_dma_rx_desc_rings() {
	if (rx_q->xsk_pool) {
		/* RX XDP ZC buffer pool may not be populated, e.g.
		 * xdpsock TX-only.
		 */
		stmmac_alloc_rx_buffers_zc(priv, dma_conf, queue);
	}
}

Similarly, if RX frames were consumed in stmmac_rx_zc() before suspend
(setting buf->xdp = NULL) and the frozen userspace fill ring has fewer than
dma_conf->dma_rx_size buffers available, xsk_buff_alloc() returns NULL.

Returning -ENOMEM here causes stmmac_resume() to abort at error_unlock,
skipping stmmac_hw_setup(), stmmac_enable_all_queues(), and
netif_device_attach() while netif_running(ndev) remains true. Because
stmmac_suspend() already called stmmac_disable_all_queues(), leaving NAPI
disabled on a running interface causes a subsequent stmmac_xdp_disable_pool()
or stmmac_release() to call napi_disable() a second time and hang under
rtnl_lock().

Additionally, stmmac_resume_rx_descriptors() does not update
rx_q->buf_alloc_num, which stmmac_hw_setup() -> stmmac_init_dma_engine()
uses to program the hardware RX tail pointer and
stmmac_set_queue_rx_buf_size() uses to select
xsk_pool_get_rx_frame_size(rx_q->xsk_pool).

> +		} else {
> +			ret = stmmac_init_rx_buffers(priv, dma_conf, p, i,
> +						     GFP_KERNEL, queue);
> +			if (ret)
> +				return ret;
> +		}
> +	}

[Severity: Medium]
Here GFP_KERNEL is passed as the flags argument to stmmac_init_rx_buffers(),
but does stmmac_init_rx_buffers() actually honour the caller's flags?

Looking at stmmac_init_rx_buffers() in
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:

static int stmmac_init_rx_buffers(struct stmmac_priv *priv,
				  struct stmmac_dma_conf *dma_conf,
				  struct dma_desc *p,
				  int i, gfp_t flags, u32 queue)
{
	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);

It ignores the flags parameter and forces GFP_ATOMIC | __GFP_NOWARN for both
buf->page and buf->sec_page allocations, preventing direct reclaim during
resume and risking avoidable -ENOMEM failures under memory pressure.

> +
> +	return 0;
> +}
> +
> +static int stmmac_resume_descriptors(struct stmmac_priv *priv,
> +				     struct stmmac_dma_conf *dma_conf)
> +{
> +	u32 rx_queue_cnt = priv->plat->rx_queues_to_use;
> +	u32 tx_queue_cnt = priv->plat->tx_queues_to_use;
> +	u32 queue;
> +	int ret;
> +
> +	for (queue = 0; queue < rx_queue_cnt; queue++) {
> +		ret = stmmac_resume_rx_descriptors(priv, dma_conf, queue);
> +		if (ret)
> +			return ret;
> +
> +		stmmac_clear_rx_descriptors(priv, dma_conf, queue);
> +	}
> +
> +	for (queue = 0; queue < tx_queue_cnt; queue++) {
> +		dma_free_tx_skbufs(priv, &priv->dma_conf, queue);
> +		stmmac_clear_tx_descriptors(priv, dma_conf, queue);
> +	}
> +
> +	return 0;
> +}

[Severity: Critical]
When priv->descriptor_mode == STMMAC_CHAIN_MODE and IEEE 1588-2002
timestamping is enabled without extended descriptors (!priv->extend_desc
with priv->hwts_rx_en or priv->hwts_tx_en), doesn't the hardware overwrite
p->des3 with the upper 32 bits of the timestamp on descriptor write-back?

In drivers/net/ethernet/stmicro/stmmac/chain_mode.c, refill_desc3() and
clean_desc3() note that the device overwrites des3 with a timestamp value:

static void refill_desc3(struct stmmac_rx_queue *rx_q, struct dma_desc *p)
{
	struct stmmac_priv *priv = rx_q->priv_data;

	if (priv->hwts_rx_en && !priv->extend_desc)
		/* NOTE: Device will overwrite des3 with timestamp value if
		 * 1588-2002 time stamping is enabled, hence reinitialize it
		 * to keep explicit chaining in the descriptor.
		 */
		p->des3 = cpu_to_le32((unsigned int)(rx_q->dma_rx_phy +
				      (((rx_q->dirty_rx) + 1) %
				       priv->dma_conf.dma_rx_size) *
				      sizeof(struct dma_desc)));
}

If an RX or TX descriptor was written back before suspend without being
cleaned by NAPI, p->des3 still holds the timestamp rather than the next
descriptor's physical DMA address. Neither stmmac_resume_rx_descriptors(),
stmmac_clear_rx_descriptors(), nor stmmac_clear_tx_descriptors() calls
stmmac_mode_init() (as __init_dma_rx_desc_rings() and
__init_dma_tx_desc_rings() do) to restore the chained des3 pointers when
priv->descriptor_mode == STMMAC_CHAIN_MODE. Could the DMA engine then
fetch the next descriptor from the timestamp address after resume?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027150556.51209-1-thomas.bourgoin@foss.st.com?part=1

Re: [PATCH 0/1] net: stmmac: descriptor management issue during PM sequence

[Maxime Chevallier]

Hi Thomas,

On 10/27/26 16:05, Thomas Bourgoin wrote:
> Hello,
> 
> During low power sequence on STM32MP21 we sometimes observe out of range access
> to the DDR by the GMAC ethernet. I managed to narrow down the issue to the
> resume sequence of the RX descriptors with the function
> stmmac_clear_descriptors().
> 
> On resume, stmmac_clear_descriptors() returns RX descriptors to the DMA
> without restoring their buffer addresses. On DWMAC4, the DMA will
> therefore consider a descriptor for the application as one for the DMA.
> This can lead to illegal access from the DMA anywhere.
> On STM32MP21 the DMA address bus is 40 bits, thus the extended status from the
> write-back descriptor is read as an high address by the DMA. DMA will then try
> to write in the DDR at 0x100000000 whereas on STM32MP215f-dk DDR is only 2Gb
> from 0x8000 0000 to 0xffff ffff and nothing is mapped at 0x100000000.
> All the other SoCs of the STM32MP1x or STM32MP2x are using a GMAC with a 32bits
> DMA address, so I do not have illegal attemps.
> 
> To fix this, I propose to reconstruct the RX descriptors that have been used by
> the DMA but not forwarded to the NAPI function stmmac_rx() before suspend.
> 
> I'm not used to this driver especially the xsk management and zero copy part.
> Since this driver is used by a lot of plateforms I'm open to all suggestions.
> 
> This V1 propose to reuse stmmac_init_rx_buffers() but maybe using
> stmmac_refill() would better fit the issue. From what I read, I could not call
> stmmac_refill() as rx_q->cur_rx is only updated in stmmac_rx(), so
> stmmac_rx_dirty() returns 0.

Thanks for the series :) However it looks like this is already being addressed by
Ding Hui's series (currently in V5) :

https://lore.kernel.org/r/20261025100636.613670-1-dinghui1111@163.com

Can you take a look and maybe provide some reviews/tested-by tags ?

Thanks,

Maxime

Re: [PATCH 0/1] net: stmmac: descriptor management issue during PM sequence

[Thomas BOURGOIN]

> Hi Thomas,
> 
> On 10/27/26 16:05, Thomas Bourgoin wrote:
>> Hello,
>>
>> During low power sequence on STM32MP21 we sometimes observe out of range access
>> to the DDR by the GMAC ethernet. I managed to narrow down the issue to the
>> resume sequence of the RX descriptors with the function
>> stmmac_clear_descriptors().
>>
>> On resume, stmmac_clear_descriptors() returns RX descriptors to the DMA
>> without restoring their buffer addresses. On DWMAC4, the DMA will
>> therefore consider a descriptor for the application as one for the DMA.
>> This can lead to illegal access from the DMA anywhere.
>> On STM32MP21 the DMA address bus is 40 bits, thus the extended status from the
>> write-back descriptor is read as an high address by the DMA. DMA will then try
>> to write in the DDR at 0x100000000 whereas on STM32MP215f-dk DDR is only 2Gb
>> from 0x8000 0000 to 0xffff ffff and nothing is mapped at 0x100000000.
>> All the other SoCs of the STM32MP1x or STM32MP2x are using a GMAC with a 32bits
>> DMA address, so I do not have illegal attemps.
>>
>> To fix this, I propose to reconstruct the RX descriptors that have been used by
>> the DMA but not forwarded to the NAPI function stmmac_rx() before suspend.
>>
>> I'm not used to this driver especially the xsk management and zero copy part.
>> Since this driver is used by a lot of plateforms I'm open to all suggestions.
>>
>> This V1 propose to reuse stmmac_init_rx_buffers() but maybe using
>> stmmac_refill() would better fit the issue. From what I read, I could not call
>> stmmac_refill() as rx_q->cur_rx is only updated in stmmac_rx(), so
>> stmmac_rx_dirty() returns 0.
> 
> 
> 

Hi Maxime

> Thanks for the series :) However it looks like this is already being 
 > addressed by
> Ding Hui's series (currently in V5) :
>
> https://lore.kernel.org/r/20261025100636.613670-1-dinghui1111@163.com
>
> Can you take a look and maybe provide some reviews/tested-by tags ?

Thanks, for the sharing. I guess I'm not the first to have the issue :)

At first glance, the v5 seems really close to the solution I proposed 
(update init descriptor function to factorize some code so it can be 
used during resume to reconstruct the descriptors that have been used by 
the GMAC DMA).

I'll take a closer look at the series, I'll try to test on 
STM32MP215F-DK but I'm not sure of what I can do.

Do you think, this patch is a candidate for LTS ?

Regards,

Thomas