[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 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