| Message ID | 20230518150202.1920375-1-shenwei.wang@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v3,net] net: fec: add dma_wmb to ensure correct descriptor values | expand |
> -----Original Message----- > From: Shenwei Wang <shenwei.wang@nxp.com> > Sent: 2023年5月18日 23:02 > To: Wei Fang <wei.fang@nxp.com>; David S. Miller <davem@davemloft.net>; > Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; > Paolo Abeni <pabeni@redhat.com>; Simon Horman <horms@kernel.org>; > Horatiu Vultur <horatiu.vultur@microchip.com>; Heiner Kallweit > <hkallweit1@gmail.com> > Cc: Shenwei Wang <shenwei.wang@nxp.com>; Clark Wang > <xiaoning.wang@nxp.com>; dl-linux-imx <linux-imx@nxp.com>; > netdev@vger.kernel.org; imx@lists.linux.dev > Subject: [PATCH v3 net] net: fec: add dma_wmb to ensure correct descriptor > values > > Two dma_wmb() are added in the XDP TX path to ensure proper ordering of > descriptor and buffer updates: > 1. A dma_wmb() is added after updating the last BD to make sure > the updates to rest of the descriptor are visible before > transferring ownership to FEC. > 2. A dma_wmb() is also added after updating the bdp to ensure these > updates are visible before updating txq->bd.cur. > 3. Start the xmit of the frame immediately right after configuring the > tx descriptor. > > Fixes: 6d6b39f180b8 ("net: fec: add initial XDP support") > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com> > --- > v3: > - use the lightweight memory barrier dma_wmb. > > v2: > - update the inline comments for 2nd wmb per Wei Fang's review. > > drivers/net/ethernet/freescale/fec_main.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > b/drivers/net/ethernet/freescale/fec_main.c > index 6d0b46c76924..a5096c3cac01 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -3834,6 +3834,11 @@ static int fec_enet_txq_xmit_frame(struct > fec_enet_private *fep, > index = fec_enet_get_bd_index(last_bdp, &txq->bd); > txq->tx_skbuff[index] = NULL; > > + /* Make sure the updates to rest of the descriptor are performed before > + * transferring ownership. > + */ > + dma_wmb(); > + > /* Send it on its way. Tell FEC it's ready, interrupt when done, > * it's the last BD of the frame, and to put the CRC on the end. > */ > @@ -3843,8 +3848,14 @@ static int fec_enet_txq_xmit_frame(struct > fec_enet_private *fep, > /* If this was the last BD in the ring, start at the beginning again. */ > bdp = fec_enet_get_nextdesc(last_bdp, &txq->bd); > > + /* Make sure the update to bdp are performed before txq->bd.cur. */ > + dma_wmb(); > + > txq->bd.cur = bdp; > > + /* Trigger transmission start */ > + writel(0, txq->bd.reg_desc_active); > + > return 0; > } > > @@ -3873,12 +3884,6 @@ static int fec_enet_xdp_xmit(struct net_device > *dev, > sent_frames++; > } > > - /* Make sure the update to bdp and tx_skbuff are performed. */ > - wmb(); > - > - /* Trigger transmission start */ > - writel(0, txq->bd.reg_desc_active); > - > __netif_tx_unlock(nq); > > return sent_frames; > -- > 2.34.1 Reviewed-by: Wei Fang <wei.fang@nxp.com>
On Thu, 18 May 2023 10:02:02 -0500 Shenwei Wang wrote: > Two dma_wmb() are added in the XDP TX path to ensure proper ordering of > descriptor and buffer updates: > 1. A dma_wmb() is added after updating the last BD to make sure > the updates to rest of the descriptor are visible before > transferring ownership to FEC. > 2. A dma_wmb() is also added after updating the bdp to ensure these > updates are visible before updating txq->bd.cur. > 3. Start the xmit of the frame immediately right after configuring the > tx descriptor. > > Fixes: 6d6b39f180b8 ("net: fec: add initial XDP support") > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com> Applied, thanks (commit 9025944fddfed).
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 6d0b46c76924..a5096c3cac01 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -3834,6 +3834,11 @@ static int fec_enet_txq_xmit_frame(struct fec_enet_private *fep, index = fec_enet_get_bd_index(last_bdp, &txq->bd); txq->tx_skbuff[index] = NULL; + /* Make sure the updates to rest of the descriptor are performed before + * transferring ownership. + */ + dma_wmb(); + /* Send it on its way. Tell FEC it's ready, interrupt when done, * it's the last BD of the frame, and to put the CRC on the end. */ @@ -3843,8 +3848,14 @@ static int fec_enet_txq_xmit_frame(struct fec_enet_private *fep, /* If this was the last BD in the ring, start at the beginning again. */ bdp = fec_enet_get_nextdesc(last_bdp, &txq->bd); + /* Make sure the update to bdp are performed before txq->bd.cur. */ + dma_wmb(); + txq->bd.cur = bdp; + /* Trigger transmission start */ + writel(0, txq->bd.reg_desc_active); + return 0; } @@ -3873,12 +3884,6 @@ static int fec_enet_xdp_xmit(struct net_device *dev, sent_frames++; } - /* Make sure the update to bdp and tx_skbuff are performed. */ - wmb(); - - /* Trigger transmission start */ - writel(0, txq->bd.reg_desc_active); - __netif_tx_unlock(nq); return sent_frames;
Two dma_wmb() are added in the XDP TX path to ensure proper ordering of descriptor and buffer updates: 1. A dma_wmb() is added after updating the last BD to make sure the updates to rest of the descriptor are visible before transferring ownership to FEC. 2. A dma_wmb() is also added after updating the bdp to ensure these updates are visible before updating txq->bd.cur. 3. Start the xmit of the frame immediately right after configuring the tx descriptor. Fixes: 6d6b39f180b8 ("net: fec: add initial XDP support") Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com> --- v3: - use the lightweight memory barrier dma_wmb. v2: - update the inline comments for 2nd wmb per Wei Fang's review. drivers/net/ethernet/freescale/fec_main.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) -- 2.34.1