[PATCH stable 4.9] net: stmmac: Set dma ring length before enabling the DMA

[PATCH stable 4.9] net: stmmac: Set dma ring length before enabling the DMA

[Lars Persson]

This was fixed in upstream by commit 7d9e6c5afab6 ("net: stmmac: Integrate
XGMAC into main driver flow") that is a new feature commit.

We found a race condition in the DMA init sequence that hits if the
PHY already has link up during stmmac_hw_setup. Since the ring length
was programmed after enabling the RX path, we might receive a packet
before the correct ring length is programmed. When that happened we
could not get reliable interrupts for DMA RX and the MTL complained
about RX FIFO overrun.

The correct init sequence according to the data book for DWC Ethernet
QoS 4.10 is:
1. Write Ring length
2. Write Descriptor list base address
3. Start the DMA.

Signed-off-by: Lars Persson <larper@axis.com>
Cc: stable@vger.kernel.org # 4.9.x
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fc437d75ac76..561e095511d6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1618,6 +1618,15 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
 		return ret;
 	}
 
+	/*  set TX ring length */
+	if (priv->hw->dma->set_tx_ring_len)
+		priv->hw->dma->set_tx_ring_len(priv->ioaddr,
+					       (DMA_TX_SIZE - 1));
+	/*  set RX ring length */
+	if (priv->hw->dma->set_rx_ring_len)
+		priv->hw->dma->set_rx_ring_len(priv->ioaddr,
+					       (DMA_RX_SIZE - 1));
+
 	priv->hw->dma->init(priv->ioaddr, pbl, fixed_burst, mixed_burst,
 			    aal, priv->dma_tx_phy, priv->dma_rx_phy, atds);
 
@@ -1767,14 +1776,6 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
 	if (priv->hw->pcs && priv->hw->mac->pcs_ctrl_ane)
 		priv->hw->mac->pcs_ctrl_ane(priv->hw, 1, priv->hw->ps, 0);
 
-	/*  set TX ring length */
-	if (priv->hw->dma->set_tx_ring_len)
-		priv->hw->dma->set_tx_ring_len(priv->ioaddr,
-					       (DMA_TX_SIZE - 1));
-	/*  set RX ring length */
-	if (priv->hw->dma->set_rx_ring_len)
-		priv->hw->dma->set_rx_ring_len(priv->ioaddr,
-					       (DMA_RX_SIZE - 1));
 	/* Enable TSO */
 	if (priv->tso)
 		priv->hw->dma->enable_tso(priv->ioaddr, 1, STMMAC_CHAN0);
-- 
2.11.0

Re: [PATCH stable 4.9] net: stmmac: Set dma ring length before enabling the DMA

[Lars Persson]

On 4/12/19 4:16 PM, Sasha Levin wrote:
> On Fri, Apr 12, 2019 at 01:17:05PM +0200, Lars Persson wrote:
>> This was fixed in upstream by commit 7d9e6c5afab6 ("net: stmmac: Integrate
>> XGMAC into main driver flow") that is a new feature commit.
>>
>> We found a race condition in the DMA init sequence that hits if the
>> PHY already has link up during stmmac_hw_setup. Since the ring length
>> was programmed after enabling the RX path, we might receive a packet
>> before the correct ring length is programmed. When that happened we
>> could not get reliable interrupts for DMA RX and the MTL complained
>> about RX FIFO overrun.
>>
>> The correct init sequence according to the data book for DWC Ethernet
>> QoS 4.10 is:
>> 1. Write Ring length
>> 2. Write Descriptor list base address
>> 3. Start the DMA.
>>
>> Signed-off-by: Lars Persson <larper@axis.com>
>> Cc: stable@vger.kernel.org # 4.9.x
>> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> Cc: Alexandre Torgue <alexandre.torgue@st.com>
>> Cc: Jose Abreu <joabreu@synopsys.com>
> 
> What about 4.14? 7d9e6c5afab6 isn't in it either.
> 

I will send a separate patch for 4.14 in some days.

-Lars

Re: [PATCH stable 4.9] net: stmmac: Set dma ring length before enabling the DMA

[Sasha Levin]

On Fri, Apr 12, 2019 at 04:21:31PM +0200, Lars Persson wrote:
>
>
>On 4/12/19 4:16 PM, Sasha Levin wrote:
>>On Fri, Apr 12, 2019 at 01:17:05PM +0200, Lars Persson wrote:
>>>This was fixed in upstream by commit 7d9e6c5afab6 ("net: stmmac: Integrate
>>>XGMAC into main driver flow") that is a new feature commit.
>>>
>>>We found a race condition in the DMA init sequence that hits if the
>>>PHY already has link up during stmmac_hw_setup. Since the ring length
>>>was programmed after enabling the RX path, we might receive a packet
>>>before the correct ring length is programmed. When that happened we
>>>could not get reliable interrupts for DMA RX and the MTL complained
>>>about RX FIFO overrun.
>>>
>>>The correct init sequence according to the data book for DWC Ethernet
>>>QoS 4.10 is:
>>>1. Write Ring length
>>>2. Write Descriptor list base address
>>>3. Start the DMA.
>>>
>>>Signed-off-by: Lars Persson <larper@axis.com>
>>>Cc: stable@vger.kernel.org # 4.9.x
>>>Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>>>Cc: Alexandre Torgue <alexandre.torgue@st.com>
>>>Cc: Jose Abreu <joabreu@synopsys.com>
>>
>>What about 4.14? 7d9e6c5afab6 isn't in it either.
>>
>
>I will send a separate patch for 4.14 in some days.

Okay. I can't merge this one until we have the 4.14 as well (otherwise,
people who upgrade from 4.9 to 4.14 will see a regression), so when it's
posted I can do both.

--
Thanks,
Sasha