[PATCH net 0/3] dpaa2-eth: software TSO fixes

[PATCH net 0/3] dpaa2-eth: software TSO fixes

[Ioana Ciornei]

This patch fixes the software TSO feature in dpaa2-eth.

There are multiple errors that I made in the initial submission of the
code, which I didn't caught since I was always running with passthough
IOMMU.

The bug report came in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=215886

The bugs are in the Tx confirmation path, where I was trying to retrieve
a virtual address after DMA unmapping the area. Besides that, another
dma_unmap call was made with the wrong size.

Ioana Ciornei (3):
  dpaa2-eth: retrieve the virtual address before dma_unmap
  dpaa2-eth: use the correct software annotation field
  dpaa2-eth: unmap the SGT buffer before accessing its contents

 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

-- 
2.33.1

[PATCH net 1/3] dpaa2-eth: retrieve the virtual address before dma_unmap

[Ioana Ciornei]

The TSO header was DMA unmapped before the virtual address was retrieved
and then used to free the buffer. This meant that we were actually
removing the DMA map and then trying to search for it to help in
retrieving the virtual address. This lead to a invalid virtual address
being used in the kfree call.

Fix this by calling dpaa2_iova_to_virt() prior to the dma_unmap call.

[  487.231819] Unable to handle kernel paging request at virtual address fffffd9807000008

(...)

[  487.354061] Hardware name: SolidRun LX2160A Honeycomb (DT)
[  487.359535] pstate: a0400005 (NzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  487.366485] pc : kfree+0xac/0x304
[  487.369799] lr : kfree+0x204/0x304
[  487.373191] sp : ffff80000c4eb120
[  487.376493] x29: ffff80000c4eb120 x28: ffff662240c46400 x27: 0000000000000001
[  487.383621] x26: 0000000000000001 x25: ffff662246da0cc0 x24: ffff66224af78000
[  487.390748] x23: ffffad184f4ce008 x22: ffffad1850185000 x21: ffffad1838d13cec
[  487.397874] x20: ffff6601c0000000 x19: fffffd9807000000 x18: 0000000000000000
[  487.405000] x17: ffffb910cdc49000 x16: ffffad184d7d9080 x15: 0000000000004000
[  487.412126] x14: 0000000000000008 x13: 000000000000ffff x12: 0000000000000000
[  487.419252] x11: 0000000000000004 x10: 0000000000000001 x9 : ffffad184d7d927c
[  487.426379] x8 : 0000000000000000 x7 : 0000000ffffffd1d x6 : ffff662240a94900
[  487.433505] x5 : 0000000000000003 x4 : 0000000000000009 x3 : ffffad184f4ce008
[  487.440632] x2 : ffff662243eec000 x1 : 0000000100000100 x0 : fffffc0000000000
[  487.447758] Call trace:
[  487.450194]  kfree+0xac/0x304
[  487.453151]  dpaa2_eth_free_tx_fd.isra.0+0x33c/0x3e0 [fsl_dpaa2_eth]
[  487.459507]  dpaa2_eth_tx_conf+0x100/0x2e0 [fsl_dpaa2_eth]
[  487.464989]  dpaa2_eth_poll+0xdc/0x380 [fsl_dpaa2_eth]

Fixes: 3dc709e0cd47 ("dpaa2-eth: add support for software TSO")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215886
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 4b047255d928..766391310d1b 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -1097,6 +1097,7 @@ static void dpaa2_eth_free_tx_fd(struct dpaa2_eth_priv *priv,
 	u32 fd_len = dpaa2_fd_get_len(fd);
 	struct dpaa2_sg_entry *sgt;
 	int should_free_skb = 1;
+	void *tso_hdr;
 	int i;
 
 	fd_addr = dpaa2_fd_get_addr(fd);
@@ -1136,9 +1137,10 @@ static void dpaa2_eth_free_tx_fd(struct dpaa2_eth_priv *priv,
 							priv->tx_data_offset);
 
 			/* Unmap and free the header */
+			tso_hdr = dpaa2_iova_to_virt(priv->iommu_domain, dpaa2_sg_get_addr(sgt));
 			dma_unmap_single(dev, dpaa2_sg_get_addr(sgt), TSO_HEADER_SIZE,
 					 DMA_TO_DEVICE);
-			kfree(dpaa2_iova_to_virt(priv->iommu_domain, dpaa2_sg_get_addr(sgt)));
+			kfree(tso_hdr);
 
 			/* Unmap the other SG entries for the data */
 			for (i = 1; i < swa->tso.num_sg; i++)
-- 
2.33.1

[PATCH net 2/3] dpaa2-eth: use the correct software annotation field

[Ioana Ciornei]

The incorrect software annotation field was being used, swa->sg.sgt_size
instead of swa->tso.sgt_size, which meant that the SGT buffer was
unmapped with a wrong size.
This is also confirmed by the DMA API debug prints which showed the
following:

[   38.962434] DMA-API: fsl_dpaa2_eth dpni.2: device driver frees DMA memory with different size [device address=0x0000fffffafba740] [map size=224 bytes] [unmap size=0 bytes]
[   38.980496] WARNING: CPU: 11 PID: 1131 at kernel/dma/debug.c:973 check_unmap+0x58c/0x9b0
[   38.988586] Modules linked in:
[   38.991631] CPU: 11 PID: 1131 Comm: iperf3 Not tainted 5.18.0-rc7-00117-g59130eeb2b8f #1972
[   38.999970] Hardware name: NXP Layerscape LX2160ARDB (DT)

Fixes: 3dc709e0cd47 ("dpaa2-eth: add support for software TSO")
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index 766391310d1b..f1f140277184 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -1148,7 +1148,7 @@ static void dpaa2_eth_free_tx_fd(struct dpaa2_eth_priv *priv,
 						 dpaa2_sg_get_len(&sgt[i]), DMA_TO_DEVICE);
 
 			/* Unmap the SGT buffer */
-			dma_unmap_single(dev, fd_addr, swa->sg.sgt_size,
+			dma_unmap_single(dev, fd_addr, swa->tso.sgt_size,
 					 DMA_BIDIRECTIONAL);
 
 			if (!swa->tso.is_last_fd)
-- 
2.33.1

[PATCH net 3/3] dpaa2-eth: unmap the SGT buffer before accessing its contents

[Ioana Ciornei]

DMA unmap the Scatter/Gather table before going through the array to
unmap and free each of the header and data chunks. This is so we do not
touch the data between the dma_map and dma_unmap calls.

Fixes: 3dc709e0cd47 ("dpaa2-eth: add support for software TSO")
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
index f1f140277184..cd9ec80522e7 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
@@ -1136,6 +1136,10 @@ static void dpaa2_eth_free_tx_fd(struct dpaa2_eth_priv *priv,
 			sgt = (struct dpaa2_sg_entry *)(buffer_start +
 							priv->tx_data_offset);
 
+			/* Unmap the SGT buffer */
+			dma_unmap_single(dev, fd_addr, swa->tso.sgt_size,
+					 DMA_BIDIRECTIONAL);
+
 			/* Unmap and free the header */
 			tso_hdr = dpaa2_iova_to_virt(priv->iommu_domain, dpaa2_sg_get_addr(sgt));
 			dma_unmap_single(dev, dpaa2_sg_get_addr(sgt), TSO_HEADER_SIZE,
@@ -1147,10 +1151,6 @@ static void dpaa2_eth_free_tx_fd(struct dpaa2_eth_priv *priv,
 				dma_unmap_single(dev, dpaa2_sg_get_addr(&sgt[i]),
 						 dpaa2_sg_get_len(&sgt[i]), DMA_TO_DEVICE);
 
-			/* Unmap the SGT buffer */
-			dma_unmap_single(dev, fd_addr, swa->tso.sgt_size,
-					 DMA_BIDIRECTIONAL);
-
 			if (!swa->tso.is_last_fd)
 				should_free_skb = 0;
 		} else {
-- 
2.33.1

Re: [PATCH net 0/3] dpaa2-eth: software TSO fixes

[Jakub Kicinski]

On Sun, 22 May 2022 15:52:48 +0300 Ioana Ciornei wrote:
> This patch fixes the software TSO feature in dpaa2-eth.
> 
> There are multiple errors that I made in the initial submission of the
> code, which I didn't caught since I was always running with passthough
> IOMMU.
> 
> The bug report came in bugzilla:
> https://bugzilla.kernel.org/show_bug.cgi?id=215886
> 
> The bugs are in the Tx confirmation path, where I was trying to retrieve
> a virtual address after DMA unmapping the area. Besides that, another
> dma_unmap call was made with the wrong size.

Thanks for the fixes! FTR it's been applied as commit 7e4d1c237592
("Merge branch 'dpaa2-swtso-fixes'") to net on Sunday.

Re: [PATCH net 0/3] dpaa2-eth: software TSO fixes

[David Lamparter]

On Sun, May 22, 2022 at 03:52:48PM +0300, Ioana Ciornei wrote:
> This patch fixes the software TSO feature in dpaa2-eth.

Could I bug you to get these thrown into -stable?  5.18.1 is currently
unusable on LX2k (with IOMMU enabled).

Thanks,
David

(Apologies if it's already underway.)