[RFC PATCH 00/10] Remove skb_dma_map/unmap calls

[RFC PATCH 00/10] Remove skb_dma_map/unmap calls

[Alexander Duyck]

This patch series removes the skb_dma_map and skb_dma_unmap function calls.
The reason for this change is because the use of skb_dma_map/unmap can lead
to serious issues when HW IOMMU is enabled.  This is because each mapping
of the skb with a HW IOMMU enabled results in a new set of DMA mappings.
This in turn leads to issues when skbs are cloned for uses such as
bridging or pktgen because each transmitting device will update the skb
shared info structure resulting in some mappings being overwritten, and others
being freed multiple times.

I am looking for input specifically on the tg3, be2net, and bnx2 driver
patches as I am not very familiar with them and I am not certain if
additional changes are required.

I have included the changes for the Intel wired Ethernet drivers as a
reference.

---

Alexander Duyck (10):
      skbuff: remove skb_dma_map/unmap
      tg3: remove use of skb_dma_map/unmap
      be2net: remove use of skb_dma_map/unmap
      bnx2: remove skb_dma_map/unmap calls from driver
      igbvf: remove skb_dma_map/unmap call from drivers
      igb: remove use of skb_dma_map from driver
      ixgbe: remove skb_dma_map/unmap calls from driver
      ixgb: remove use of skb_dma_map from ixgb
      e1000: remove use of skb_dma_map from e1000 driver
      e1000e: remove use of skb_dma_map from e1000e driver


 drivers/net/benet/be_main.c    |   37 ++++++---
 drivers/net/bnx2.c             |   74 ++++++++++++++---
 drivers/net/bnx2.h             |    1 
 drivers/net/e1000/e1000.h      |    1 
 drivers/net/e1000/e1000_main.c |   56 +++++++++----
 drivers/net/e1000e/e1000.h     |    9 +-
 drivers/net/e1000e/netdev.c    |   59 +++++++++-----
 drivers/net/igb/igb.h          |    5 +
 drivers/net/igb/igb_main.c     |   69 ++++++++++++----
 drivers/net/igbvf/igbvf.h      |    1 
 drivers/net/igbvf/netdev.c     |   65 ++++++++++++---
 drivers/net/ixgb/ixgb.h        |    1 
 drivers/net/ixgb/ixgb_main.c   |   60 +++++++++-----
 drivers/net/ixgbe/ixgbe.h      |    1 
 drivers/net/ixgbe/ixgbe_main.c |   63 +++++++++++----
 drivers/net/tg3.c              |  173 ++++++++++++++++++++++++++++++----------
 drivers/net/tg3.h              |    6 -
 include/linux/skbuff.h         |    8 --
 net/core/Makefile              |    1 
 net/core/skb_dma_map.c         |   65 ---------------
 20 files changed, 508 insertions(+), 247 deletions(-)
 delete mode 100644 net/core/skb_dma_map.c

-- 

[RFC PATCH 01/10] e1000e: remove use of skb_dma_map from e1000e driver

[Alexander Duyck]

In testing we have found that skb_dma_map/unmap is incompatible with HW
IOMMU due to the fact that multiple mappings will return different results.
In order to correct this we need to remove skb_dma_map/unmap calls from the
e1000e driver.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 drivers/net/e1000e/e1000.h  |    9 ++++---
 drivers/net/e1000e/netdev.c |   59 ++++++++++++++++++++++++++++++-------------
 2 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
index c9fcef7..1be8218 100644
--- a/drivers/net/e1000e/e1000.h
+++ b/drivers/net/e1000e/e1000.h
@@ -192,12 +192,15 @@ struct e1000_buffer {
 			unsigned long time_stamp;
 			u16 length;
 			u16 next_to_watch;
+			u16 mapped_as_page;
 		};
 		/* Rx */
-		/* arrays of page information for packet split */
-		struct e1000_ps_page *ps_pages;
+		struct {
+			/* arrays of page information for packet split */
+			struct e1000_ps_page *ps_pages;
+			struct page *page;
+		};
 	};
-	struct page *page;
 };
 
 struct e1000_ring {
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 11a5274..406d51b 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -534,10 +534,17 @@ next_desc:
 static void e1000_put_txbuf(struct e1000_adapter *adapter,
 			     struct e1000_buffer *buffer_info)
 {
-	buffer_info->dma = 0;
+	if (buffer_info->dma) {
+		if (buffer_info->mapped_as_page)
+			pci_unmap_page(adapter->pdev, buffer_info->dma,
+				       buffer_info->length, PCI_DMA_TODEVICE);
+		else
+			pci_unmap_single(adapter->pdev,	buffer_info->dma,
+					 buffer_info->length,
+					 PCI_DMA_TODEVICE);
+		buffer_info->dma = 0;
+	}
 	if (buffer_info->skb) {
-		skb_dma_unmap(&adapter->pdev->dev, buffer_info->skb,
-		              DMA_TO_DEVICE);
 		dev_kfree_skb_any(buffer_info->skb);
 		buffer_info->skb = NULL;
 	}
@@ -3890,23 +3897,14 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 			unsigned int mss)
 {
 	struct e1000_ring *tx_ring = adapter->tx_ring;
+	struct pci_dev *pdev = adapter->pdev;
 	struct e1000_buffer *buffer_info;
 	unsigned int len = skb_headlen(skb);
-	unsigned int offset, size, count = 0, i;
+	unsigned int offset = 0, size, count = 0, i;
 	unsigned int f;
-	dma_addr_t *map;
 
 	i = tx_ring->next_to_use;
 
-	if (skb_dma_map(&adapter->pdev->dev, skb, DMA_TO_DEVICE)) {
-		dev_err(&adapter->pdev->dev, "TX DMA map failed\n");
-		adapter->tx_dma_failed++;
-		return 0;
-	}
-
-	map = skb_shinfo(skb)->dma_maps;
-	offset = 0;
-
 	while (len) {
 		buffer_info = &tx_ring->buffer_info[i];
 		size = min(len, max_per_txd);
@@ -3914,11 +3912,15 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 		buffer_info->length = size;
 		buffer_info->time_stamp = jiffies;
 		buffer_info->next_to_watch = i;
-		buffer_info->dma = skb_shinfo(skb)->dma_head + offset;
-		count++;
+		buffer_info->dma = pci_map_single(pdev,	skb->data + offset,
+						  size,	PCI_DMA_TODEVICE);
+		buffer_info->mapped_as_page = false;
+		if (pci_dma_mapping_error(pdev, buffer_info->dma))
+			goto dma_error;
 
 		len -= size;
 		offset += size;
+		count++;
 
 		if (len) {
 			i++;
@@ -3932,7 +3934,7 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 
 		frag = &skb_shinfo(skb)->frags[f];
 		len = frag->size;
-		offset = 0;
+		offset = frag->page_offset;
 
 		while (len) {
 			i++;
@@ -3945,7 +3947,12 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 			buffer_info->length = size;
 			buffer_info->time_stamp = jiffies;
 			buffer_info->next_to_watch = i;
-			buffer_info->dma = map[f] + offset;
+			buffer_info->dma = pci_map_page(pdev, frag->page,
+							offset, size,
+							PCI_DMA_TODEVICE);
+			buffer_info->mapped_as_page = true;
+			if (pci_dma_mapping_error(pdev, buffer_info->dma))
+				goto dma_error;
 
 			len -= size;
 			offset += size;
@@ -3957,6 +3964,22 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 	tx_ring->buffer_info[first].next_to_watch = i;
 
 	return count;
+
+dma_error:
+	dev_err(&pdev->dev, "TX DMA map failed\n");
+	buffer_info->dma = 0;
+	count--;
+
+	while (count >= 0) {
+		count--;
+		i--;
+		if (i < 0)
+			i += tx_ring->count;
+		buffer_info = &tx_ring->buffer_info[i];
+		e1000_put_txbuf(adapter, buffer_info);;
+	}
+
+	return 0;
 }
 
 static void e1000_tx_queue(struct e1000_adapter *adapter,

[RFC PATCH 02/10] e1000: remove use of skb_dma_map from e1000 driver

[Alexander Duyck]

Remove the use of skb_dma_map from the e1000 driver in order to avoid
issues when HW iommu are in use.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 drivers/net/e1000/e1000.h      |    1 +
 drivers/net/e1000/e1000_main.c |   56 +++++++++++++++++++++++++++++-----------
 2 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index a566528..2a567df 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -167,6 +167,7 @@ struct e1000_buffer {
 	unsigned long time_stamp;
 	u16 length;
 	u16 next_to_watch;
+	u16 mapped_as_page;
 };
 
 struct e1000_tx_ring {
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index c938114..b9d86f8 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -1839,10 +1839,17 @@ void e1000_free_all_tx_resources(struct e1000_adapter *adapter)
 static void e1000_unmap_and_free_tx_resource(struct e1000_adapter *adapter,
 					     struct e1000_buffer *buffer_info)
 {
-	buffer_info->dma = 0;
+	if (buffer_info->dma) {
+		if (buffer_info->mapped_as_page)
+			pci_unmap_page(adapter->pdev, buffer_info->dma,
+				       buffer_info->length, PCI_DMA_TODEVICE);
+		else
+			pci_unmap_single(adapter->pdev,	buffer_info->dma,
+					 buffer_info->length,
+					 PCI_DMA_TODEVICE);
+		buffer_info->dma = 0;
+	}
 	if (buffer_info->skb) {
-		skb_dma_unmap(&adapter->pdev->dev, buffer_info->skb,
-		              DMA_TO_DEVICE);
 		dev_kfree_skb_any(buffer_info->skb);
 		buffer_info->skb = NULL;
 	}
@@ -2683,22 +2690,14 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 			unsigned int mss)
 {
 	struct e1000_hw *hw = &adapter->hw;
+	struct pci_dev *pdev = adapter->pdev;
 	struct e1000_buffer *buffer_info;
 	unsigned int len = skb_headlen(skb);
-	unsigned int offset, size, count = 0, i;
+	unsigned int offset = 0, size, count = 0, i;
 	unsigned int f;
-	dma_addr_t *map;
 
 	i = tx_ring->next_to_use;
 
-	if (skb_dma_map(&adapter->pdev->dev, skb, DMA_TO_DEVICE)) {
-		dev_err(&adapter->pdev->dev, "TX DMA map failed\n");
-		return 0;
-	}
-
-	map = skb_shinfo(skb)->dma_maps;
-	offset = 0;
-
 	while (len) {
 		buffer_info = &tx_ring->buffer_info[i];
 		size = min(len, max_per_txd);
@@ -2735,7 +2734,11 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 		buffer_info->length = size;
 		/* set time_stamp *before* dma to help avoid a possible race */
 		buffer_info->time_stamp = jiffies;
-		buffer_info->dma = skb_shinfo(skb)->dma_head + offset;
+		buffer_info->mapped_as_page = false;
+		buffer_info->dma = pci_map_single(pdev,	skb->data + offset,
+						  size,	PCI_DMA_TODEVICE);
+		if (pci_dma_mapping_error(pdev, buffer_info->dma))
+			goto dma_error;
 		buffer_info->next_to_watch = i;
 
 		len -= size;
@@ -2753,7 +2756,7 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 
 		frag = &skb_shinfo(skb)->frags[f];
 		len = frag->size;
-		offset = 0;
+		offset = frag->page_offset;
 
 		while (len) {
 			i++;
@@ -2777,7 +2780,12 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 
 			buffer_info->length = size;
 			buffer_info->time_stamp = jiffies;
-			buffer_info->dma = map[f] + offset;
+			buffer_info->mapped_as_page = true;
+			buffer_info->dma = pci_map_page(pdev, frag->page,
+							offset,	size,
+							PCI_DMA_TODEVICE);
+			if (pci_dma_mapping_error(pdev, buffer_info->dma))
+				goto dma_error;
 			buffer_info->next_to_watch = i;
 
 			len -= size;
@@ -2790,6 +2798,22 @@ static int e1000_tx_map(struct e1000_adapter *adapter,
 	tx_ring->buffer_info[first].next_to_watch = i;
 
 	return count;
+
+dma_error:
+	dev_err(&pdev->dev, "TX DMA map failed\n");
+	buffer_info->dma = 0;
+	count--;
+
+	while (count >= 0) {
+		count--;
+		i--;
+		if (i < 0)
+			i += tx_ring->count;
+		buffer_info = &tx_ring->buffer_info[i];
+		e1000_unmap_and_free_tx_resource(adapter, buffer_info);
+	}
+
+	return 0;
 }
 
 static void e1000_tx_queue(struct e1000_adapter *adapter,

[RFC PATCH 03/10] ixgb: remove use of skb_dma_map from ixgb

[Alexander Duyck]

skb_dma_map is incompatible with HW iommu due to the fact that multiple
mappings can result in different results each time.  For this reason it is
best to just remove use of these function calls.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 drivers/net/ixgb/ixgb.h      |    1 +
 drivers/net/ixgb/ixgb_main.c |   60 +++++++++++++++++++++++++++++-------------
 2 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ixgb/ixgb.h b/drivers/net/ixgb/ixgb.h
index e95d9b6..5257ae0 100644
--- a/drivers/net/ixgb/ixgb.h
+++ b/drivers/net/ixgb/ixgb.h
@@ -117,6 +117,7 @@ struct ixgb_buffer {
 	unsigned long time_stamp;
 	u16 length;
 	u16 next_to_watch;
+	u16 mapped_as_page;
 };
 
 struct ixgb_desc_ring {
diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
index 7364606..b955ce8 100644
--- a/drivers/net/ixgb/ixgb_main.c
+++ b/drivers/net/ixgb/ixgb_main.c
@@ -892,10 +892,18 @@ static void
 ixgb_unmap_and_free_tx_resource(struct ixgb_adapter *adapter,
                                 struct ixgb_buffer *buffer_info)
 {
-	buffer_info->dma = 0;
+	if (buffer_info->dma) {
+		if (buffer_info->mapped_as_page)
+			pci_unmap_page(adapter->pdev, buffer_info->dma,
+				       buffer_info->length, PCI_DMA_TODEVICE);
+		else
+			pci_unmap_single(adapter->pdev, buffer_info->dma,
+					 buffer_info->length,
+					 PCI_DMA_TODEVICE);
+		buffer_info->dma = 0;
+	}
+
 	if (buffer_info->skb) {
-		skb_dma_unmap(&adapter->pdev->dev, buffer_info->skb,
-		              DMA_TO_DEVICE);
 		dev_kfree_skb_any(buffer_info->skb);
 		buffer_info->skb = NULL;
 	}
@@ -1272,24 +1280,16 @@ ixgb_tx_map(struct ixgb_adapter *adapter, struct sk_buff *skb,
 	    unsigned int first)
 {
 	struct ixgb_desc_ring *tx_ring = &adapter->tx_ring;
+	struct pci_dev *pdev = adapter->pdev;
 	struct ixgb_buffer *buffer_info;
 	int len = skb_headlen(skb);
 	unsigned int offset = 0, size, count = 0, i;
 	unsigned int mss = skb_shinfo(skb)->gso_size;
-
 	unsigned int nr_frags = skb_shinfo(skb)->nr_frags;
 	unsigned int f;
-	dma_addr_t *map;
 
 	i = tx_ring->next_to_use;
 
-	if (skb_dma_map(&adapter->pdev->dev, skb, DMA_TO_DEVICE)) {
-		dev_err(&adapter->pdev->dev, "TX DMA map failed\n");
-		return 0;
-	}
-
-	map = skb_shinfo(skb)->dma_maps;
-
 	while (len) {
 		buffer_info = &tx_ring->buffer_info[i];
 		size = min(len, IXGB_MAX_DATA_PER_TXD);
@@ -1301,11 +1301,11 @@ ixgb_tx_map(struct ixgb_adapter *adapter, struct sk_buff *skb,
 		buffer_info->length = size;
 		WARN_ON(buffer_info->dma != 0);
 		buffer_info->time_stamp = jiffies;
-		buffer_info->dma = skb_shinfo(skb)->dma_head + offset;
-			pci_map_single(adapter->pdev,
-				skb->data + offset,
-				size,
-				PCI_DMA_TODEVICE);
+		buffer_info->mapped_as_page = false;
+		buffer_info->dma = pci_map_single(pdev, skb->data + offset,
+						  size, PCI_DMA_TODEVICE);
+		if (pci_dma_mapping_error(pdev, buffer_info->dma))
+			goto dma_error;
 		buffer_info->next_to_watch = 0;
 
 		len -= size;
@@ -1323,7 +1323,7 @@ ixgb_tx_map(struct ixgb_adapter *adapter, struct sk_buff *skb,
 
 		frag = &skb_shinfo(skb)->frags[f];
 		len = frag->size;
-		offset = 0;
+		offset = frag->page_offset;
 
 		while (len) {
 			i++;
@@ -1341,7 +1341,13 @@ ixgb_tx_map(struct ixgb_adapter *adapter, struct sk_buff *skb,
 
 			buffer_info->length = size;
 			buffer_info->time_stamp = jiffies;
-			buffer_info->dma = map[f] + offset;
+			buffer_info->mapped_as_page = true;
+			buffer_info->dma =
+				pci_map_page(pdev, frag->page,
+					     frag->page_offset + offset, size,
+					     PCI_DMA_TODEVICE);
+			if (pci_dma_mapping_error(pdev, buffer_info->dma))
+				goto dma_error;
 			buffer_info->next_to_watch = 0;
 
 			len -= size;
@@ -1353,6 +1359,22 @@ ixgb_tx_map(struct ixgb_adapter *adapter, struct sk_buff *skb,
 	tx_ring->buffer_info[first].next_to_watch = i;
 
 	return count;
+
+dma_error:
+	dev_err(&pdev->dev, "TX DMA map failed\n");
+	buffer_info->dma = 0;
+	count--;
+
+	while (count >= 0) {
+		count--;
+		i--;
+		if (i < 0)
+			i += tx_ring->count;
+		buffer_info = &tx_ring->buffer_info[i];
+		ixgb_unmap_and_free_tx_resource(adapter, buffer_info);
+	}
+
+	return 0;
 }
 
 static void

[RFC PATCH 04/10] ixgbe: remove skb_dma_map/unmap calls from driver

[Alexander Duyck]

This patch removes skb_dma_map/unmap calls from the ixgbe driver due to the
fact that the calls don't work with HW IOMMU enabled systems.  The problem
is that multiple mappings will give different results when HW IOMMU is
enabled and the skb_dma_map/unmap calls only have one location to store
mappings.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 drivers/net/ixgbe/ixgbe.h      |    1 +
 drivers/net/ixgbe/ixgbe_main.c |   63 +++++++++++++++++++++++++++++++---------
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe.h b/drivers/net/ixgbe/ixgbe.h
index 6ebcc2c..4e7d1fc 100644
--- a/drivers/net/ixgbe/ixgbe.h
+++ b/drivers/net/ixgbe/ixgbe.h
@@ -123,6 +123,7 @@ struct ixgbe_tx_buffer {
 	unsigned long time_stamp;
 	u16 length;
 	u16 next_to_watch;
+	u16 mapped_as_page;
 };
 
 struct ixgbe_rx_buffer {
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 9335352..d274d64 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -262,10 +262,20 @@ static void ixgbe_unmap_and_free_tx_resource(struct ixgbe_adapter *adapter,
                                              struct ixgbe_tx_buffer
                                              *tx_buffer_info)
 {
-	tx_buffer_info->dma = 0;
+	if (tx_buffer_info->dma) {
+		if (tx_buffer_info->mapped_as_page)
+			pci_unmap_page(adapter->pdev,
+				       tx_buffer_info->dma,
+				       tx_buffer_info->length,
+				       PCI_DMA_TODEVICE);
+		else
+			pci_unmap_single(adapter->pdev,
+					 tx_buffer_info->dma,
+					 tx_buffer_info->length,
+					 PCI_DMA_TODEVICE);
+		tx_buffer_info->dma = 0;
+	}
 	if (tx_buffer_info->skb) {
-		skb_dma_unmap(&adapter->pdev->dev, tx_buffer_info->skb,
-		              DMA_TO_DEVICE);
 		dev_kfree_skb_any(tx_buffer_info->skb);
 		tx_buffer_info->skb = NULL;
 	}
@@ -5210,23 +5220,16 @@ static int ixgbe_tx_map(struct ixgbe_adapter *adapter,
                         struct sk_buff *skb, u32 tx_flags,
                         unsigned int first)
 {
+	struct pci_dev *pdev = adapter->pdev;
 	struct ixgbe_tx_buffer *tx_buffer_info;
 	unsigned int len;
 	unsigned int total = skb->len;
 	unsigned int offset = 0, size, count = 0, i;
 	unsigned int nr_frags = skb_shinfo(skb)->nr_frags;
 	unsigned int f;
-	dma_addr_t *map;
 
 	i = tx_ring->next_to_use;
 
-	if (skb_dma_map(&adapter->pdev->dev, skb, DMA_TO_DEVICE)) {
-		dev_err(&adapter->pdev->dev, "TX DMA map failed\n");
-		return 0;
-	}
-
-	map = skb_shinfo(skb)->dma_maps;
-
 	if (tx_flags & IXGBE_TX_FLAGS_FCOE)
 		/* excluding fcoe_crc_eof for FCoE */
 		total -= sizeof(struct fcoe_crc_eof);
@@ -5237,7 +5240,12 @@ static int ixgbe_tx_map(struct ixgbe_adapter *adapter,
 		size = min(len, (uint)IXGBE_MAX_DATA_PER_TXD);
 
 		tx_buffer_info->length = size;
-		tx_buffer_info->dma = skb_shinfo(skb)->dma_head + offset;
+		tx_buffer_info->mapped_as_page = false;
+		tx_buffer_info->dma = pci_map_single(pdev,
+						     skb->data + offset,
+						     size, PCI_DMA_TODEVICE);
+		if (pci_dma_mapping_error(pdev, tx_buffer_info->dma))
+			goto dma_error;
 		tx_buffer_info->time_stamp = jiffies;
 		tx_buffer_info->next_to_watch = i;
 
@@ -5258,7 +5266,7 @@ static int ixgbe_tx_map(struct ixgbe_adapter *adapter,
 
 		frag = &skb_shinfo(skb)->frags[f];
 		len = min((unsigned int)frag->size, total);
-		offset = 0;
+		offset = frag->page_offset;
 
 		while (len) {
 			i++;
@@ -5269,7 +5277,13 @@ static int ixgbe_tx_map(struct ixgbe_adapter *adapter,
 			size = min(len, (uint)IXGBE_MAX_DATA_PER_TXD);
 
 			tx_buffer_info->length = size;
-			tx_buffer_info->dma = map[f] + offset;
+			tx_buffer_info->dma = pci_map_page(adapter->pdev,
+							   frag->page,
+							   offset, size,
+							   PCI_DMA_TODEVICE);
+			tx_buffer_info->mapped_as_page = true;
+			if (pci_dma_mapping_error(pdev, tx_buffer_info->dma))
+				goto dma_error;
 			tx_buffer_info->time_stamp = jiffies;
 			tx_buffer_info->next_to_watch = i;
 
@@ -5286,6 +5300,27 @@ static int ixgbe_tx_map(struct ixgbe_adapter *adapter,
 	tx_ring->tx_buffer_info[first].next_to_watch = i;
 
 	return count;
+
+dma_error:
+	dev_err(&pdev->dev, "TX DMA map failed\n");
+
+	/* clear timestamp and dma mappings for failed tx_buffer_info map */
+	tx_buffer_info->dma = 0;
+	tx_buffer_info->time_stamp = 0;
+	tx_buffer_info->next_to_watch = 0;
+	count--;
+
+	/* clear timestamp and dma mappings for remaining portion of packet */
+	while (count >= 0) {
+		count--;
+		i--;
+		if (i < 0)
+			i += tx_ring->count;
+		tx_buffer_info = &tx_ring->tx_buffer_info[i];
+		ixgbe_unmap_and_free_tx_resource(adapter, tx_buffer_info);
+	}
+
+	return count;
 }
 
 static void ixgbe_tx_queue(struct ixgbe_adapter *adapter,

[RFC PATCH 05/10] igb: remove use of skb_dma_map from driver

[Alexander Duyck]

This change removes skb_dma_map/unmap calls from the igb driver due to the
fact that the call is incompatible with iommu enabled kernels.  In order to
prevent warnings about using the wrong unmap call I have added a
mapped_as_page value to the buffer_info structure to track if the mapped
region is a page or a buffer.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 drivers/net/igb/igb.h      |    5 ++-
 drivers/net/igb/igb_main.c |   69 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 56 insertions(+), 18 deletions(-)

diff --git a/drivers/net/igb/igb.h b/drivers/net/igb/igb.h
index c458d9b..b1c1eb8 100644
--- a/drivers/net/igb/igb.h
+++ b/drivers/net/igb/igb.h
@@ -137,12 +137,13 @@ struct igb_buffer {
 			unsigned long time_stamp;
 			u16 length;
 			u16 next_to_watch;
+			u16 mapped_as_page;
 		};
 		/* RX */
 		struct {
 			struct page *page;
-			u64 page_dma;
-			unsigned int page_offset;
+			dma_addr_t page_dma;
+			u16 page_offset;
 		};
 	};
 };
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index bb1a6ee..e57b32d 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -2654,16 +2654,27 @@ static void igb_free_all_tx_resources(struct igb_adapter *adapter)
 void igb_unmap_and_free_tx_resource(struct igb_ring *tx_ring,
 				    struct igb_buffer *buffer_info)
 {
-	buffer_info->dma = 0;
+	if (buffer_info->dma) {
+		if (buffer_info->mapped_as_page)
+			pci_unmap_page(tx_ring->pdev,
+					buffer_info->dma,
+					buffer_info->length,
+					PCI_DMA_TODEVICE);
+		else
+			pci_unmap_single(tx_ring->pdev,
+					buffer_info->dma,
+					buffer_info->length,
+					PCI_DMA_TODEVICE);
+		buffer_info->dma = 0;
+	}
 	if (buffer_info->skb) {
-		skb_dma_unmap(&tx_ring->pdev->dev,
-		              buffer_info->skb,
-		              DMA_TO_DEVICE);
 		dev_kfree_skb_any(buffer_info->skb);
 		buffer_info->skb = NULL;
 	}
 	buffer_info->time_stamp = 0;
-	/* buffer_info must be completely set up in the transmit path */
+	buffer_info->length = 0;
+	buffer_info->next_to_watch = 0;
+	buffer_info->mapped_as_page = false;
 }
 
 /**
@@ -3561,24 +3572,19 @@ static inline int igb_tx_map_adv(struct igb_ring *tx_ring, struct sk_buff *skb,
 	unsigned int len = skb_headlen(skb);
 	unsigned int count = 0, i;
 	unsigned int f;
-	dma_addr_t *map;
 
 	i = tx_ring->next_to_use;
 
-	if (skb_dma_map(&pdev->dev, skb, DMA_TO_DEVICE)) {
-		dev_err(&pdev->dev, "TX DMA map failed\n");
-		return 0;
-	}
-
-	map = skb_shinfo(skb)->dma_maps;
-
 	buffer_info = &tx_ring->buffer_info[i];
 	BUG_ON(len >= IGB_MAX_DATA_PER_TXD);
 	buffer_info->length = len;
 	/* set time_stamp *before* dma to help avoid a possible race */
 	buffer_info->time_stamp = jiffies;
 	buffer_info->next_to_watch = i;
-	buffer_info->dma = skb_shinfo(skb)->dma_head;
+	buffer_info->dma = pci_map_single(pdev, skb->data, len,
+					  PCI_DMA_TODEVICE);
+	if (pci_dma_mapping_error(pdev, buffer_info->dma))
+		goto dma_error;
 
 	for (f = 0; f < skb_shinfo(skb)->nr_frags; f++) {
 		struct skb_frag_struct *frag;
@@ -3595,7 +3601,15 @@ static inline int igb_tx_map_adv(struct igb_ring *tx_ring, struct sk_buff *skb,
 		buffer_info->length = len;
 		buffer_info->time_stamp = jiffies;
 		buffer_info->next_to_watch = i;
-		buffer_info->dma = map[count];
+		buffer_info->mapped_as_page = true;
+		buffer_info->dma = pci_map_page(pdev,
+						frag->page,
+						frag->page_offset,
+						len,
+						PCI_DMA_TODEVICE);
+		if (pci_dma_mapping_error(pdev, buffer_info->dma))
+			goto dma_error;
+
 		count++;
 	}
 
@@ -3603,6 +3617,29 @@ static inline int igb_tx_map_adv(struct igb_ring *tx_ring, struct sk_buff *skb,
 	tx_ring->buffer_info[first].next_to_watch = i;
 
 	return ++count;
+
+dma_error:
+	dev_err(&pdev->dev, "TX DMA map failed\n");
+
+	/* clear timestamp and dma mappings for failed buffer_info mapping */
+	buffer_info->dma = 0;
+	buffer_info->time_stamp = 0;
+	buffer_info->length = 0;
+	buffer_info->next_to_watch = 0;
+	buffer_info->mapped_as_page = false;
+	count--;
+
+	/* clear timestamp and dma mappings for remaining portion of packet */
+	while (count >= 0) {
+		count--;
+		i--;
+		if (i < 0)
+			i += tx_ring->count;
+		buffer_info = &tx_ring->buffer_info[i];
+		igb_unmap_and_free_tx_resource(tx_ring, buffer_info);
+	}
+
+	return 0;
 }
 
 static inline void igb_tx_queue_adv(struct igb_ring *tx_ring,
@@ -3755,7 +3792,7 @@ netdev_tx_t igb_xmit_frame_ring_adv(struct sk_buff *skb,
 	 * has occured and we need to rewind the descriptor queue
 	 */
 	count = igb_tx_map_adv(tx_ring, skb, first);
-	if (count <= 0) {
+	if (!count) {
 		dev_kfree_skb_any(skb);
 		tx_ring->buffer_info[first].time_stamp = 0;
 		tx_ring->next_to_use = first;

[RFC PATCH 06/10] igbvf: remove skb_dma_map/unmap call from drivers

[Alexander Duyck]

This patch removes the skb_dma_map/unmap calls from the igbvf driver due to
the fact that it does not play well with HW IOMMU when combined with
transmitting cloned skbs.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 drivers/net/igbvf/igbvf.h  |    1 +
 drivers/net/igbvf/netdev.c |   65 +++++++++++++++++++++++++++++++++++---------
 2 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/drivers/net/igbvf/igbvf.h b/drivers/net/igbvf/igbvf.h
index 8e9b67e..3d1ee7a 100644
--- a/drivers/net/igbvf/igbvf.h
+++ b/drivers/net/igbvf/igbvf.h
@@ -117,6 +117,7 @@ struct igbvf_buffer {
 			unsigned long time_stamp;
 			u16 length;
 			u16 next_to_watch;
+			u16 mapped_as_page;
 		};
 		/* Rx */
 		struct {
diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
index e01f445..cf26f92 100644
--- a/drivers/net/igbvf/netdev.c
+++ b/drivers/net/igbvf/netdev.c
@@ -366,10 +366,20 @@ next_desc:
 static void igbvf_put_txbuf(struct igbvf_adapter *adapter,
                             struct igbvf_buffer *buffer_info)
 {
-	buffer_info->dma = 0;
+	if (buffer_info->dma) {
+		if (buffer_info->mapped_as_page)
+			pci_unmap_page(adapter->pdev,
+				       buffer_info->dma,
+				       buffer_info->length,
+				       PCI_DMA_TODEVICE);
+		else
+			pci_unmap_single(adapter->pdev,
+					 buffer_info->dma,
+					 buffer_info->length,
+					 PCI_DMA_TODEVICE);
+		buffer_info->dma = 0;
+	}
 	if (buffer_info->skb) {
-		skb_dma_unmap(&adapter->pdev->dev, buffer_info->skb,
-		              DMA_TO_DEVICE);
 		dev_kfree_skb_any(buffer_info->skb);
 		buffer_info->skb = NULL;
 	}
@@ -2088,27 +2098,24 @@ static inline int igbvf_tx_map_adv(struct igbvf_adapter *adapter,
                                    unsigned int first)
 {
 	struct igbvf_buffer *buffer_info;
+	struct pci_dev *pdev = adapter->pdev;
 	unsigned int len = skb_headlen(skb);
 	unsigned int count = 0, i;
 	unsigned int f;
-	dma_addr_t *map;
 
 	i = tx_ring->next_to_use;
 
-	if (skb_dma_map(&adapter->pdev->dev, skb, DMA_TO_DEVICE)) {
-		dev_err(&adapter->pdev->dev, "TX DMA map failed\n");
-		return 0;
-	}
-
-	map = skb_shinfo(skb)->dma_maps;
-
 	buffer_info = &tx_ring->buffer_info[i];
 	BUG_ON(len >= IGBVF_MAX_DATA_PER_TXD);
 	buffer_info->length = len;
 	/* set time_stamp *before* dma to help avoid a possible race */
 	buffer_info->time_stamp = jiffies;
 	buffer_info->next_to_watch = i;
-	buffer_info->dma = skb_shinfo(skb)->dma_head;
+	buffer_info->dma = pci_map_single(pdev, skb->data, len,
+					  PCI_DMA_TODEVICE);
+	if (pci_dma_mapping_error(pdev, buffer_info->dma))
+		goto dma_error;
+
 
 	for (f = 0; f < skb_shinfo(skb)->nr_frags; f++) {
 		struct skb_frag_struct *frag;
@@ -2125,14 +2132,44 @@ static inline int igbvf_tx_map_adv(struct igbvf_adapter *adapter,
 		buffer_info->length = len;
 		buffer_info->time_stamp = jiffies;
 		buffer_info->next_to_watch = i;
-		buffer_info->dma = map[count];
+		buffer_info->mapped_as_page = true;
+		buffer_info->dma = pci_map_page(pdev,
+						frag->page,
+						frag->page_offset,
+						len,
+						PCI_DMA_TODEVICE);
+		if (pci_dma_mapping_error(pdev, buffer_info->dma))
+			goto dma_error;
 		count++;
 	}
 
 	tx_ring->buffer_info[i].skb = skb;
 	tx_ring->buffer_info[first].next_to_watch = i;
 
-	return count + 1;
+	return ++count;
+
+dma_error:
+	dev_err(&pdev->dev, "TX DMA map failed\n");
+
+	/* clear timestamp and dma mappings for failed buffer_info mapping */
+	buffer_info->dma = 0;
+	buffer_info->time_stamp = 0;
+	buffer_info->length = 0;
+	buffer_info->next_to_watch = 0;
+	buffer_info->mapped_as_page = false;
+	count--;
+
+	/* clear timestamp and dma mappings for remaining portion of packet */
+	while (count >= 0) {
+		count--;
+		i--;
+		if (i < 0)
+			i += tx_ring->count;
+		buffer_info = &tx_ring->buffer_info[i];
+		igbvf_put_txbuf(adapter, buffer_info);
+	}
+
+	return 0;
 }
 
 static inline void igbvf_tx_queue_adv(struct igbvf_adapter *adapter,

[RFC PATCH 07/10] bnx2: remove skb_dma_map/unmap calls from driver

[Alexander Duyck]

Due to the fact that skb_dma_map/unmap do not work correctly when a HW
IOMMU is enabled it has been recommended to go about removing the calls
from the network device drivers.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 drivers/net/bnx2.c |   74 +++++++++++++++++++++++++++++++++++++++++++---------
 drivers/net/bnx2.h |    1 +
 2 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 539d23b..6daddb5 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -2815,13 +2815,21 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
 			}
 		}
 
-		skb_dma_unmap(&bp->pdev->dev, skb, DMA_TO_DEVICE);
+		pci_unmap_single(bp->pdev, pci_unmap_addr(tx_buf, mapping),
+			skb_headlen(skb), PCI_DMA_TODEVICE);
 
 		tx_buf->skb = NULL;
 		last = tx_buf->nr_frags;
 
 		for (i = 0; i < last; i++) {
 			sw_cons = NEXT_TX_BD(sw_cons);
+
+			pci_unmap_page(bp->pdev,
+				pci_unmap_addr(
+					&txr->tx_buf_ring[TX_RING_IDX(sw_cons)],
+					mapping),
+				skb_shinfo(skb)->frags[i].size,
+				PCI_DMA_TODEVICE);
 		}
 
 		sw_cons = NEXT_TX_BD(sw_cons);
@@ -5295,18 +5303,30 @@ bnx2_free_tx_skbs(struct bnx2 *bp)
 		for (j = 0; j < TX_DESC_CNT; ) {
 			struct sw_tx_bd *tx_buf = &txr->tx_buf_ring[j];
 			struct sk_buff *skb = tx_buf->skb;
+			int k, last;
 
 			if (skb == NULL) {
 				j++;
 				continue;
 			}
 
-			skb_dma_unmap(&bp->pdev->dev, skb, DMA_TO_DEVICE);
+			pci_unmap_single(bp->pdev,
+					 pci_unmap_addr(tx_buf, mapping),
+					 skb_headlen(skb),
+					 PCI_DMA_TODEVICE);
 
 			tx_buf->skb = NULL;
 
-			j += skb_shinfo(skb)->nr_frags + 1;
+			last = skb_shinfo(skb)->nr_frags;
+			for (k = 0; k < last; k++) {
+				tx_buf = &txr->tx_buf_ring[j + k + 1];
+				pci_unmap_page(bp->pdev,
+					pci_unmap_addr(tx_buf, mapping),
+					skb_shinfo(skb)->frags[j].size,
+					PCI_DMA_TODEVICE);
+			}
 			dev_kfree_skb(skb);
+			j += k + 1;
 		}
 	}
 }
@@ -5684,11 +5704,12 @@ bnx2_run_loopback(struct bnx2 *bp, int loopback_mode)
 	for (i = 14; i < pkt_size; i++)
 		packet[i] = (unsigned char) (i & 0xff);
 
-	if (skb_dma_map(&bp->pdev->dev, skb, DMA_TO_DEVICE)) {
+	map = pci_map_single(bp->pdev, skb->data, pkt_size,
+		PCI_DMA_TODEVICE);
+	if (pci_dma_mapping_error(bp->pdev, map)) {
 		dev_kfree_skb(skb);
 		return -EIO;
 	}
-	map = skb_shinfo(skb)->dma_head;
 
 	REG_WR(bp, BNX2_HC_COMMAND,
 	       bp->hc_cmd | BNX2_HC_COMMAND_COAL_NOW_WO_INT);
@@ -5723,7 +5744,7 @@ bnx2_run_loopback(struct bnx2 *bp, int loopback_mode)
 
 	udelay(5);
 
-	skb_dma_unmap(&bp->pdev->dev, skb, DMA_TO_DEVICE);
+	pci_unmap_single(bp->pdev, map, pkt_size, PCI_DMA_TODEVICE);
 	dev_kfree_skb(skb);
 
 	if (bnx2_get_hw_tx_cons(tx_napi) != txr->tx_prod)
@@ -6302,7 +6323,6 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct bnx2_napi *bnapi;
 	struct bnx2_tx_ring_info *txr;
 	struct netdev_queue *txq;
-	struct skb_shared_info *sp;
 
 	/*  Determine which tx ring we will be placed on */
 	i = skb_get_queue_mapping(skb);
@@ -6367,16 +6387,15 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	} else
 		mss = 0;
 
-	if (skb_dma_map(&bp->pdev->dev, skb, DMA_TO_DEVICE)) {
+	mapping = pci_map_single(bp->pdev, skb->data, len, PCI_DMA_TODEVICE);
+	if (pci_dma_mapping_error(bp->pdev, mapping)) {
 		dev_kfree_skb(skb);
-		return NETDEV_TX_OK;
+		return -EIO;
 	}
 
-	sp = skb_shinfo(skb);
-	mapping = sp->dma_head;
-
 	tx_buf = &txr->tx_buf_ring[ring_prod];
 	tx_buf->skb = skb;
+	pci_unmap_addr_set(tx_buf, mapping, mapping);
 
 	txbd = &txr->tx_desc_ring[ring_prod];
 
@@ -6397,7 +6416,12 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		txbd = &txr->tx_desc_ring[ring_prod];
 
 		len = frag->size;
-		mapping = sp->dma_maps[i];
+		mapping = pci_map_page(bp->pdev, frag->page, frag->page_offset,
+			len, PCI_DMA_TODEVICE);
+		if (pci_dma_mapping_error(bp->pdev, mapping))
+			goto dma_error;
+		pci_unmap_addr_set(&txr->tx_buf_ring[ring_prod], mapping,
+				   mapping);
 
 		txbd->tx_bd_haddr_hi = (u64) mapping >> 32;
 		txbd->tx_bd_haddr_lo = (u64) mapping & 0xffffffff;
@@ -6424,6 +6448,30 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	return NETDEV_TX_OK;
+dma_error:
+	/* save value of frag that failed */
+	last_frag = i;
+
+	/* start back at beginning and unmap skb */
+	prod = txr->tx_prod;
+	ring_prod = TX_RING_IDX(prod);
+	tx_buf = &txr->tx_buf_ring[ring_prod];
+	tx_buf->skb = NULL;
+	pci_unmap_single(bp->pdev, pci_unmap_addr(tx_buf, mapping),
+			 skb_headlen(skb), PCI_DMA_TODEVICE);
+
+	/* unmap remaining mapped pages */
+	for (i = 0; i < last_frag; i++) {
+		prod = NEXT_TX_BD(prod);
+		ring_prod = TX_RING_IDX(prod);
+		tx_buf = &txr->tx_buf_ring[ring_prod];
+		pci_unmap_page(bp->pdev, pci_unmap_addr(tx_buf, mapping),
+			       skb_shinfo(skb)->frags[i].size,
+			       PCI_DMA_TODEVICE);
+	}
+
+	dev_kfree_skb(skb);
+	return -EIO;
 }
 
 /* Called with rtnl_lock */
diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h
index a4d8340..4908b9f 100644
--- a/drivers/net/bnx2.h
+++ b/drivers/net/bnx2.h
@@ -6559,6 +6559,7 @@ struct sw_pg {
 
 struct sw_tx_bd {
 	struct sk_buff		*skb;
+	DECLARE_PCI_UNMAP_ADDR(mapping)
 	unsigned short		is_gso;
 	unsigned short		nr_frags;
 };

[RFC PATCH 08/10] be2net: remove use of skb_dma_map/unmap

[Alexander Duyck]

Due to the fact that skb_dma_map/unmap do not work correctly when a HW
IOMMU is enabled it has been recommended to go about removing the calls
from the network device drivers.
    
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 drivers/net/benet/be_main.c |   37 ++++++++++++++++++++++++++-----------
 1 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 7959364..f6d8a88 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -390,15 +390,11 @@ static int make_tx_wrbs(struct be_adapter *adapter,
 	atomic_add(wrb_cnt, &txq->used);
 	queue_head_inc(txq);
 
-	if (skb_dma_map(&pdev->dev, skb, DMA_TO_DEVICE)) {
-		dev_err(&pdev->dev, "TX DMA mapping failed\n");
-		return 0;
-	}
-
 	if (skb->len > skb->data_len) {
 		int len = skb->len - skb->data_len;
+		busaddr = pci_map_single(pdev, skb->data, len,
+					 PCI_DMA_TODEVICE);
 		wrb = queue_head_node(txq);
-		busaddr = skb_shinfo(skb)->dma_head;
 		wrb_fill(wrb, busaddr, len);
 		be_dws_cpu_to_le(wrb, sizeof(*wrb));
 		queue_head_inc(txq);
@@ -408,8 +404,9 @@ static int make_tx_wrbs(struct be_adapter *adapter,
 	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
 		struct skb_frag_struct *frag =
 			&skb_shinfo(skb)->frags[i];
-
-		busaddr = skb_shinfo(skb)->dma_maps[i];
+		busaddr = pci_map_page(pdev, frag->page,
+				       frag->page_offset,
+				       frag->size, PCI_DMA_TODEVICE);
 		wrb = queue_head_node(txq);
 		wrb_fill(wrb, busaddr, frag->size);
 		be_dws_cpu_to_le(wrb, sizeof(*wrb));
@@ -982,23 +979,41 @@ static struct be_eth_tx_compl *be_tx_compl_get(struct be_queue_info *tx_cq)
 static void be_tx_compl_process(struct be_adapter *adapter, u16 last_index)
 {
 	struct be_queue_info *txq = &adapter->tx_obj.q;
+	struct be_eth_wrb *wrb;
 	struct sk_buff **sent_skbs = adapter->tx_obj.sent_skb_list;
 	struct sk_buff *sent_skb;
+	u64 busaddr;
 	u16 cur_index, num_wrbs = 0;
 
 	cur_index = txq->tail;
 	sent_skb = sent_skbs[cur_index];
 	BUG_ON(!sent_skb);
 	sent_skbs[cur_index] = NULL;
+	wrb = queue_tail_node(txq);
+	be_dws_le_to_cpu(wrb, sizeof(*wrb));
+	busaddr = ((u64)wrb->frag_pa_hi << 32) | (u64)wrb->frag_pa_lo;
+	if (busaddr != 0) {
+		pci_unmap_single(adapter->pdev, busaddr,
+				 wrb->frag_len, PCI_DMA_TODEVICE);
+	}
+	num_wrbs++;
+	queue_tail_inc(txq);
 
-	do {
+	while (cur_index != last_index) {
 		cur_index = txq->tail;
+		wrb = queue_tail_node(txq);
+		be_dws_le_to_cpu(wrb, sizeof(*wrb));
+		busaddr = ((u64)wrb->frag_pa_hi << 32) | (u64)wrb->frag_pa_lo;
+		if (busaddr != 0) {
+			pci_unmap_page(adapter->pdev, busaddr,
+				       wrb->frag_len, PCI_DMA_TODEVICE);
+		}
 		num_wrbs++;
 		queue_tail_inc(txq);
-	} while (cur_index != last_index);
+	}
 
 	atomic_sub(num_wrbs, &txq->used);
-	skb_dma_unmap(&adapter->pdev->dev, sent_skb, DMA_TO_DEVICE);
+
 	kfree_skb(sent_skb);
 }
 

[RFC PATCH 09/10] tg3: remove use of skb_dma_map/unmap

[Alexander Duyck]

Due to the fact that skb_dma_map/unmap do not work correctly when a HW
IOMMU is enabled it has been recommended to go about removing the calls
from the network device drivers.
    
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 drivers/net/tg3.c |  173 ++++++++++++++++++++++++++++++++++++++++-------------
 drivers/net/tg3.h |    6 --
 2 files changed, 132 insertions(+), 47 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 6e6db95..302ea0b 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -4357,7 +4357,7 @@ static void tg3_tx(struct tg3_napi *tnapi)
 	txq = netdev_get_tx_queue(tp->dev, index);
 
 	while (sw_idx != hw_idx) {
-		struct tx_ring_info *ri = &tnapi->tx_buffers[sw_idx];
+		struct ring_info *ri = &tnapi->tx_buffers[sw_idx];
 		struct sk_buff *skb = ri->skb;
 		int i, tx_bug = 0;
 
@@ -4366,7 +4366,10 @@ static void tg3_tx(struct tg3_napi *tnapi)
 			return;
 		}
 
-		skb_dma_unmap(&tp->pdev->dev, skb, DMA_TO_DEVICE);
+		pci_unmap_single(tp->pdev,
+				 pci_unmap_addr(ri, mapping),
+				 skb_headlen(skb),
+				 PCI_DMA_TODEVICE);
 
 		ri->skb = NULL;
 
@@ -4376,6 +4379,11 @@ static void tg3_tx(struct tg3_napi *tnapi)
 			ri = &tnapi->tx_buffers[sw_idx];
 			if (unlikely(ri->skb != NULL || sw_idx == hw_idx))
 				tx_bug = 1;
+
+			pci_unmap_page(tp->pdev,
+				       pci_unmap_addr(ri, mapping),
+				       skb_shinfo(skb)->frags[i].size,
+				       PCI_DMA_TODEVICE);
 			sw_idx = NEXT_TX(sw_idx);
 		}
 
@@ -5334,17 +5342,21 @@ static int tigon3_dma_hwbug_workaround(struct tg3_napi *tnapi,
 	} else {
 		/* New SKB is guaranteed to be linear. */
 		entry = *start;
-		ret = skb_dma_map(&tp->pdev->dev, new_skb, DMA_TO_DEVICE);
-		new_addr = skb_shinfo(new_skb)->dma_head;
+		new_addr = pci_map_single(tp->pdev, new_skb->data, new_skb->len,
+					  PCI_DMA_TODEVICE);
+		/* Make sure the mapping succeeded */
+		if (pci_dma_mapping_error(tp->pdev, new_addr)) {
+			ret = -1;
+			dev_kfree_skb(new_skb);
+			new_skb = NULL;
 
 		/* Make sure new skb does not cross any 4G boundaries.
 		 * Drop the packet if it does.
 		 */
-		if (ret || ((tp->tg3_flags3 & TG3_FLG3_4G_DMA_BNDRY_BUG) &&
-			    tg3_4g_overflow_test(new_addr, new_skb->len))) {
-			if (!ret)
-				skb_dma_unmap(&tp->pdev->dev, new_skb,
-					      DMA_TO_DEVICE);
+		} else if ((tp->tg3_flags3 & TG3_FLG3_4G_DMA_BNDRY_BUG) &&
+			    tg3_4g_overflow_test(new_addr, new_skb->len)) {
+			pci_unmap_single(tp->pdev, new_addr, new_skb->len,
+					 PCI_DMA_TODEVICE);
 			ret = -1;
 			dev_kfree_skb(new_skb);
 			new_skb = NULL;
@@ -5358,15 +5370,28 @@ static int tigon3_dma_hwbug_workaround(struct tg3_napi *tnapi,
 	/* Now clean up the sw ring entries. */
 	i = 0;
 	while (entry != last_plus_one) {
+		int len;
+
 		if (i == 0)
-			tnapi->tx_buffers[entry].skb = new_skb;
+			len = skb_headlen(skb);
 		else
+			len = skb_shinfo(skb)->frags[i-1].size;
+
+		pci_unmap_single(tp->pdev,
+				 pci_unmap_addr(&tnapi->tx_buffers[entry],
+						mapping),
+				 len, PCI_DMA_TODEVICE);
+		if (i == 0) {
+			tnapi->tx_buffers[entry].skb = new_skb;
+			pci_unmap_addr_set(&tnapi->tx_buffers[entry], mapping,
+					   new_addr);
+		} else {
 			tnapi->tx_buffers[entry].skb = NULL;
+		}
 		entry = NEXT_TX(entry);
 		i++;
 	}
 
-	skb_dma_unmap(&tp->pdev->dev, skb, DMA_TO_DEVICE);
 	dev_kfree_skb(skb);
 
 	return ret;
@@ -5403,10 +5428,11 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
 {
 	struct tg3 *tp = netdev_priv(dev);
 	u32 len, entry, base_flags, mss;
-	struct skb_shared_info *sp;
 	dma_addr_t mapping;
 	struct tg3_napi *tnapi;
 	struct netdev_queue *txq;
+	unsigned int i, last;
+
 
 	txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
 	tnapi = &tp->napi[skb_get_queue_mapping(skb)];
@@ -5477,18 +5503,17 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
 			       (vlan_tx_tag_get(skb) << 16));
 #endif
 
-	if (skb_dma_map(&tp->pdev->dev, skb, DMA_TO_DEVICE)) {
+	len = skb_headlen(skb);
+
+	/* Queue skb data, a.k.a. the main skb fragment. */
+	mapping = pci_map_single(tp->pdev, skb->data, len, PCI_DMA_TODEVICE);
+	if (pci_dma_mapping_error(tp->pdev, mapping)) {
 		dev_kfree_skb(skb);
 		goto out_unlock;
 	}
 
-	sp = skb_shinfo(skb);
-
-	mapping = sp->dma_head;
-
 	tnapi->tx_buffers[entry].skb = skb;
-
-	len = skb_headlen(skb);
+	pci_unmap_addr_set(&tnapi->tx_buffers[entry], mapping, mapping);
 
 	if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5717 &&
 	    !mss && skb->len > ETH_DATA_LEN)
@@ -5501,15 +5526,21 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
 
 	/* Now loop through additional data fragments, and queue them. */
 	if (skb_shinfo(skb)->nr_frags > 0) {
-		unsigned int i, last;
-
 		last = skb_shinfo(skb)->nr_frags - 1;
 		for (i = 0; i <= last; i++) {
 			skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
 
 			len = frag->size;
-			mapping = sp->dma_maps[i];
+			mapping = pci_map_page(tp->pdev,
+					       frag->page,
+					       frag->page_offset,
+					       len, PCI_DMA_TODEVICE);
+			if (pci_dma_mapping_error(tp->pdev, mapping))
+				goto dma_error;
+
 			tnapi->tx_buffers[entry].skb = NULL;
+			pci_unmap_addr_set(&tnapi->tx_buffers[entry], mapping,
+					   mapping);
 
 			tg3_set_txd(tnapi, entry, mapping, len,
 				    base_flags, (i == last) | (mss << 1));
@@ -5532,6 +5563,27 @@ out_unlock:
 	mmiowb();
 
 	return NETDEV_TX_OK;
+
+dma_error:
+	last = i;
+	entry = tnapi->tx_prod;
+	tnapi->tx_buffers[entry].skb = NULL;
+	pci_unmap_single(tp->pdev,
+			 pci_unmap_addr(&tnapi->tx_buffers[entry], mapping),
+			 skb_headlen(skb),
+			 PCI_DMA_TODEVICE);
+	for (i = 0; i <= last; i++) {
+		skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+		entry = NEXT_TX(entry);
+
+		pci_unmap_page(tp->pdev,
+			       pci_unmap_addr(&tnapi->tx_buffers[entry],
+					      mapping),
+			       frag->size, PCI_DMA_TODEVICE);
+	}
+
+	dev_kfree_skb(skb);
+	return NETDEV_TX_OK;
 }
 
 static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *,
@@ -5579,11 +5631,12 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
 {
 	struct tg3 *tp = netdev_priv(dev);
 	u32 len, entry, base_flags, mss;
-	struct skb_shared_info *sp;
 	int would_hit_hwbug;
 	dma_addr_t mapping;
 	struct tg3_napi *tnapi;
 	struct netdev_queue *txq;
+	unsigned int i, last;
+
 
 	txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
 	tnapi = &tp->napi[skb_get_queue_mapping(skb)];
@@ -5678,21 +5731,19 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
 	    !mss && skb->len > ETH_DATA_LEN)
 		base_flags |= TXD_FLAG_JMB_PKT;
 
-	if (skb_dma_map(&tp->pdev->dev, skb, DMA_TO_DEVICE)) {
+	len = skb_headlen(skb);
+
+	mapping = pci_map_single(tp->pdev, skb->data, len, PCI_DMA_TODEVICE);
+	if (pci_dma_mapping_error(tp->pdev, mapping)) {
 		dev_kfree_skb(skb);
 		goto out_unlock;
 	}
 
-	sp = skb_shinfo(skb);
-
-	mapping = sp->dma_head;
-
 	tnapi->tx_buffers[entry].skb = skb;
+	pci_unmap_addr_set(&tnapi->tx_buffers[entry], mapping, mapping);
 
 	would_hit_hwbug = 0;
 
-	len = skb_headlen(skb);
-
 	if ((tp->tg3_flags3 & TG3_FLG3_SHORT_DMA_BUG) && len <= 8)
 		would_hit_hwbug = 1;
 
@@ -5714,16 +5765,21 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
 
 	/* Now loop through additional data fragments, and queue them. */
 	if (skb_shinfo(skb)->nr_frags > 0) {
-		unsigned int i, last;
-
 		last = skb_shinfo(skb)->nr_frags - 1;
 		for (i = 0; i <= last; i++) {
 			skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
 
 			len = frag->size;
-			mapping = sp->dma_maps[i];
+			mapping = pci_map_page(tp->pdev,
+					       frag->page,
+					       frag->page_offset,
+					       len, PCI_DMA_TODEVICE);
 
 			tnapi->tx_buffers[entry].skb = NULL;
+			pci_unmap_addr_set(&tnapi->tx_buffers[entry], mapping,
+					   mapping);
+			if (pci_dma_mapping_error(tp->pdev, mapping))
+				goto dma_error;
 
 			if ((tp->tg3_flags3 & TG3_FLG3_SHORT_DMA_BUG) &&
 			    len <= 8)
@@ -5779,6 +5835,27 @@ out_unlock:
 	mmiowb();
 
 	return NETDEV_TX_OK;
+
+dma_error:
+	last = i;
+	entry = tnapi->tx_prod;
+	tnapi->tx_buffers[entry].skb = NULL;
+	pci_unmap_single(tp->pdev,
+			 pci_unmap_addr(&tnapi->tx_buffers[entry], mapping),
+			 skb_headlen(skb),
+			 PCI_DMA_TODEVICE);
+	for (i = 0; i <= last; i++) {
+		skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+		entry = NEXT_TX(entry);
+
+		pci_unmap_page(tp->pdev,
+			       pci_unmap_addr(&tnapi->tx_buffers[entry],
+					      mapping),
+			       frag->size, PCI_DMA_TODEVICE);
+	}
+
+	dev_kfree_skb(skb);
+	return NETDEV_TX_OK;
 }
 
 static inline void tg3_set_mtu(struct net_device *dev, struct tg3 *tp,
@@ -6046,8 +6123,9 @@ static void tg3_free_rings(struct tg3 *tp)
 			continue;
 
 		for (i = 0; i < TG3_TX_RING_SIZE; ) {
-			struct tx_ring_info *txp;
+			struct ring_info *txp;
 			struct sk_buff *skb;
+			unsigned int k;
 
 			txp = &tnapi->tx_buffers[i];
 			skb = txp->skb;
@@ -6057,11 +6135,22 @@ static void tg3_free_rings(struct tg3 *tp)
 				continue;
 			}
 
-			skb_dma_unmap(&tp->pdev->dev, skb, DMA_TO_DEVICE);
-
+			pci_unmap_single(tp->pdev,
+					 pci_unmap_addr(txp, mapping),
+					 skb_headlen(skb),
+					 PCI_DMA_TODEVICE);
 			txp->skb = NULL;
 
-			i += skb_shinfo(skb)->nr_frags + 1;
+			i++;
+
+			for (k = 0; k < skb_shinfo(skb)->nr_frags; k++) {
+				txp = &tnapi->tx_buffers[i & (TG3_TX_RING_SIZE - 1)];
+				pci_unmap_page(tp->pdev,
+					       pci_unmap_addr(txp, mapping),
+					       skb_shinfo(skb)->frags[k].size,
+					       PCI_DMA_TODEVICE);
+				i++;
+			}
 
 			dev_kfree_skb_any(skb);
 		}
@@ -6231,7 +6320,7 @@ static int tg3_alloc_consistent(struct tg3 *tp)
 
 		memset(tnapi->rx_rcb, 0, TG3_RX_RCB_RING_BYTES(tp));
 
-		tnapi->tx_buffers = kzalloc(sizeof(struct tx_ring_info) *
+		tnapi->tx_buffers = kzalloc(sizeof(struct ring_info) *
 					    TG3_TX_RING_SIZE, GFP_KERNEL);
 		if (!tnapi->tx_buffers)
 			goto err_out;
@@ -10637,7 +10726,8 @@ static int tg3_run_loopback(struct tg3 *tp, int loopback_mode)
 	for (i = 14; i < tx_len; i++)
 		tx_data[i] = (u8) (i & 0xff);
 
-	if (skb_dma_map(&tp->pdev->dev, skb, DMA_TO_DEVICE)) {
+	map = pci_map_single(tp->pdev, skb->data, tx_len, PCI_DMA_TODEVICE);
+	if (pci_dma_mapping_error(tp->pdev, map)) {
 		dev_kfree_skb(skb);
 		return -EIO;
 	}
@@ -10651,8 +10741,7 @@ static int tg3_run_loopback(struct tg3 *tp, int loopback_mode)
 
 	num_pkts = 0;
 
-	tg3_set_txd(tnapi, tnapi->tx_prod,
-		    skb_shinfo(skb)->dma_head, tx_len, 0, 1);
+	tg3_set_txd(tnapi, tnapi->tx_prod, map, tx_len, 0, 1);
 
 	tnapi->tx_prod++;
 	num_pkts++;
@@ -10676,7 +10765,7 @@ static int tg3_run_loopback(struct tg3 *tp, int loopback_mode)
 			break;
 	}
 
-	skb_dma_unmap(&tp->pdev->dev, skb, DMA_TO_DEVICE);
+	pci_unmap_single(tp->pdev, map, tx_len, PCI_DMA_TODEVICE);
 	dev_kfree_skb(skb);
 
 	if (tx_idx != tnapi->tx_prod)
diff --git a/drivers/net/tg3.h b/drivers/net/tg3.h
index 453a34f..8972523 100644
--- a/drivers/net/tg3.h
+++ b/drivers/net/tg3.h
@@ -2441,10 +2441,6 @@ struct ring_info {
 	DECLARE_PCI_UNMAP_ADDR(mapping)
 };
 
-struct tx_ring_info {
-	struct sk_buff			*skb;
-};
-
 struct tg3_config_info {
 	u32				flags;
 };
@@ -2608,7 +2604,7 @@ struct tg3_napi {
 
 	struct tg3_rx_buffer_desc	*rx_rcb;
 	struct tg3_tx_buffer_desc	*tx_ring;
-	struct tx_ring_info		*tx_buffers;
+	struct ring_info		*tx_buffers;
 
 	dma_addr_t			status_mapping;
 	dma_addr_t			rx_rcb_mapping;

[RFC PATCH 10/10] skbuff: remove skb_dma_map/unmap

[Alexander Duyck]

The two functions skb_dma_map/unmap are unsafe to use as they cause
problems when packets are cloned and sent to multiple devices while a HW
IOMMU is enabled.  Due to this it is best to remove the code so it is not
used by any other network driver maintainters.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 include/linux/skbuff.h |    8 ------
 net/core/Makefile      |    1 -
 net/core/skb_dma_map.c |   65 ------------------------------------------------
 3 files changed, 0 insertions(+), 74 deletions(-)
 delete mode 100644 net/core/skb_dma_map.c

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 89eed8c..ae836fd 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -416,14 +416,6 @@ struct sk_buff {
 
 #include <asm/system.h>
 
-#ifdef CONFIG_HAS_DMA
-#include <linux/dma-mapping.h>
-extern int skb_dma_map(struct device *dev, struct sk_buff *skb,
-		       enum dma_data_direction dir);
-extern void skb_dma_unmap(struct device *dev, struct sk_buff *skb,
-			  enum dma_data_direction dir);
-#endif
-
 static inline struct dst_entry *skb_dst(const struct sk_buff *skb)
 {
 	return (struct dst_entry *)skb->_skb_dst;
diff --git a/net/core/Makefile b/net/core/Makefile
index 796f46e..08791ac 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -6,7 +6,6 @@ obj-y := sock.o request_sock.o skbuff.o iovec.o datagram.o stream.o scm.o \
 	 gen_stats.o gen_estimator.o net_namespace.o
 
 obj-$(CONFIG_SYSCTL) += sysctl_net_core.o
-obj-$(CONFIG_HAS_DMA) += skb_dma_map.o
 
 obj-y		     += dev.o ethtool.o dev_mcast.o dst.o netevent.o \
 			neighbour.o rtnetlink.o utils.o link_watch.o filter.o
diff --git a/net/core/skb_dma_map.c b/net/core/skb_dma_map.c
deleted file mode 100644
index 79687df..0000000
--- a/net/core/skb_dma_map.c
+++ /dev/null
@@ -1,65 +0,0 @@
-/* skb_dma_map.c: DMA mapping helpers for socket buffers.
- *
- * Copyright (C) David S. Miller <davem@davemloft.net>
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/dma-mapping.h>
-#include <linux/skbuff.h>
-
-int skb_dma_map(struct device *dev, struct sk_buff *skb,
-		enum dma_data_direction dir)
-{
-	struct skb_shared_info *sp = skb_shinfo(skb);
-	dma_addr_t map;
-	int i;
-
-	map = dma_map_single(dev, skb->data,
-			     skb_headlen(skb), dir);
-	if (dma_mapping_error(dev, map))
-		goto out_err;
-
-	sp->dma_head = map;
-	for (i = 0; i < sp->nr_frags; i++) {
-		skb_frag_t *fp = &sp->frags[i];
-
-		map = dma_map_page(dev, fp->page, fp->page_offset,
-				   fp->size, dir);
-		if (dma_mapping_error(dev, map))
-			goto unwind;
-		sp->dma_maps[i] = map;
-	}
-
-	return 0;
-
-unwind:
-	while (--i >= 0) {
-		skb_frag_t *fp = &sp->frags[i];
-
-		dma_unmap_page(dev, sp->dma_maps[i],
-			       fp->size, dir);
-	}
-	dma_unmap_single(dev, sp->dma_head,
-			 skb_headlen(skb), dir);
-out_err:
-	return -ENOMEM;
-}
-EXPORT_SYMBOL(skb_dma_map);
-
-void skb_dma_unmap(struct device *dev, struct sk_buff *skb,
-		   enum dma_data_direction dir)
-{
-	struct skb_shared_info *sp = skb_shinfo(skb);
-	int i;
-
-	dma_unmap_single(dev, sp->dma_head,
-			 skb_headlen(skb), dir);
-	for (i = 0; i < sp->nr_frags; i++) {
-		skb_frag_t *fp = &sp->frags[i];
-
-		dma_unmap_page(dev, sp->dma_maps[i],
-			       fp->size, dir);
-	}
-}
-EXPORT_SYMBOL(skb_dma_unmap);

Re: [RFC PATCH 00/10] Remove skb_dma_map/unmap calls

[Ajit Khaparde]

On 24/11/09 17:20 -0800, Alexander Duyck wrote:
> This patch series removes the skb_dma_map and skb_dma_unmap function calls.
> The reason for this change is because the use of skb_dma_map/unmap can lead
> to serious issues when HW IOMMU is enabled.  This is because each mapping
> of the skb with a HW IOMMU enabled results in a new set of DMA mappings.
> This in turn leads to issues when skbs are cloned for uses such as
> bridging or pktgen because each transmitting device will update the skb
> shared info structure resulting in some mappings being overwritten, and others
> being freed multiple times.

If this is the case can the members related to the dma mapping stuff
(skb_shinfo(skb)->dma_maps) be moved out of this shared info structure and
we retain this "good" abstraction provided by this skb_dma_map/unmap api?
> 
> I am looking for input specifically on the tg3, be2net, and bnx2 driver
> patches as I am not very familiar with them and I am not certain if
> additional changes are required.

>       be2net: remove use of skb_dma_map/unmap
> 
>  drivers/net/benet/be_main.c    |   37 ++++++---

We have pulled the be2net specific patch for testing and review.
We will send an update once done with it.

-Ajit

Re: [RFC PATCH 00/10] Remove skb_dma_map/unmap calls

[Alexander Duyck]

Ajit Khaparde wrote:
> On 24/11/09 17:20 -0800, Alexander Duyck wrote:
>> This patch series removes the skb_dma_map and skb_dma_unmap function calls.
>> The reason for this change is because the use of skb_dma_map/unmap can lead
>> to serious issues when HW IOMMU is enabled.  This is because each mapping
>> of the skb with a HW IOMMU enabled results in a new set of DMA mappings.
>> This in turn leads to issues when skbs are cloned for uses such as
>> bridging or pktgen because each transmitting device will update the skb
>> shared info structure resulting in some mappings being overwritten, and others
>> being freed multiple times.
> 
> If this is the case can the members related to the dma mapping stuff
> (skb_shinfo(skb)->dma_maps) be moved out of this shared info structure and
> we retain this "good" abstraction provided by this skb_dma_map/unmap api?

I had submitted a patch on November 5th that tried to move the maps out 
of the shared info structure and into the sk_buff itself.  That was 
rejected and it was suggested to go back to what was there before since 
not enough drivers were using the skb_dma_map/unmap calls to justify 
increasing the size of an sk_buff.

The biggest issue is that you end up needing to retain a copy of the DMA 
mapping output each time it is called and so the easiest place to retain 
the information is within the transmit buffer driver specific data 
structure.

>> I am looking for input specifically on the tg3, be2net, and bnx2 driver
>> patches as I am not very familiar with them and I am not certain if
>> additional changes are required.
> 
>>       be2net: remove use of skb_dma_map/unmap
>>
>>  drivers/net/benet/be_main.c    |   37 ++++++---
> 
> We have pulled the be2net specific patch for testing and review.
> We will send an update once done with it.
> 
> -Ajit

Thanks for taking a look at this.

Alex

Re: [RFC PATCH 08/10] be2net: remove use of skb_dma_map/unmap

[Ajit Khaparde]

On 24/11/09 17:20 -0800, Alexander Duyck wrote:
> Due to the fact that skb_dma_map/unmap do not work correctly when a HW
> IOMMU is enabled it has been recommended to go about removing the calls
> from the network device drivers.
>     
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> 
>  drivers/net/benet/be_main.c |   37 ++++++++++++++++++++++++++-----------
>  1 files changed, 26 insertions(+), 11 deletions(-)
> 
Thanks for the changes. They are good to go.

Acked-by: Ajit Khaparde <ajitk@serverengines.com>

Re: [RFC PATCH 00/10] Remove skb_dma_map/unmap calls

[David Miller]

From: Alexander Duyck <alexander.h.duyck@intel.com>
Date: Tue, 24 Nov 2009 17:20:12 -0800

> This patch series removes the skb_dma_map and skb_dma_unmap function calls.
> The reason for this change is because the use of skb_dma_map/unmap can lead
> to serious issues when HW IOMMU is enabled.  This is because each mapping
> of the skb with a HW IOMMU enabled results in a new set of DMA mappings.
> This in turn leads to issues when skbs are cloned for uses such as
> bridging or pktgen because each transmitting device will update the skb
> shared info structure resulting in some mappings being overwritten, and others
> being freed multiple times.
> 
> I am looking for input specifically on the tg3, be2net, and bnx2 driver
> patches as I am not very familiar with them and I am not certain if
> additional changes are required.
> 
> I have included the changes for the Intel wired Ethernet drivers as a
> reference.

These changes look good, but at least one of them does not apply
to current net-2.6

Can you respin this against current sources?

Thanks a lot for doing this work Alexander.

Re: [RFC PATCH 07/10] bnx2: remove skb_dma_map/unmap calls from driver

[Michael Chan]

Alexander Duyck wrote:

> Due to the fact that skb_dma_map/unmap do not work correctly when a HW
> IOMMU is enabled it has been recommended to go about removing
> the calls
> from the network device drivers.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>

Thanks Alexander.  Sorry for the late response, I just got back
from vacation.  It looks ok except in bnx2_free_tx_skbs():

> @@ -5295,18 +5303,30 @@ bnx2_free_tx_skbs(struct bnx2 *bp)
>               for (j = 0; j < TX_DESC_CNT; ) {
>                       struct sw_tx_bd *tx_buf = &txr->tx_buf_ring[j];
>                       struct sk_buff *skb = tx_buf->skb;
> +                     int k, last;
>
>                       if (skb == NULL) {
>                               j++;
>                               continue;
>                       }
>
> -                     skb_dma_unmap(&bp->pdev->dev, skb,
> DMA_TO_DEVICE);
> +                     pci_unmap_single(bp->pdev,
> +                                      pci_unmap_addr(tx_buf,
> mapping),
> +                                      skb_headlen(skb),
> +                                      PCI_DMA_TODEVICE);
>
>                       tx_buf->skb = NULL;
>
> -                     j += skb_shinfo(skb)->nr_frags + 1;
> +                     last = skb_shinfo(skb)->nr_frags;
> +                     for (k = 0; k < last; k++) {
> +                             tx_buf = &txr->tx_buf_ring[j + k + 1];

j + k + 1 can go beyond the ring when the ring wraps around.
I'll send an updated patch tomorrow.

> +                             pci_unmap_page(bp->pdev,
> +                                     pci_unmap_addr(tx_buf, mapping),
> +                                     skb_shinfo(skb)->frags[j].size,
> +                                     PCI_DMA_TODEVICE);
> +                     }
>                       dev_kfree_skb(skb);
> +                     j += k + 1;
>               }
>       }
>  }

Re: [RFC PATCH 07/10] bnx2: remove skb_dma_map/unmap calls from driver

[Michael Chan]

On Mon, 2009-11-30 at 02:26 -0800, Michael Chan wrote:
> It looks ok except in bnx2_free_tx_skbs()

Here's the slightly revised patch with some bug fixes.  Thanks again to
Alexander.

From: Alexander Duyck <alexander.h.duyck@intel.com>

Due to the fact that skb_dma_map/unmap do not work correctly when a HW
IOMMU is enabled it has been recommended to go about removing the calls
from the network device drivers.

[ Fix bnx2_free_tx_skbs() ring indexing and use NETDEV_TX_OK return
  code in bnx2_start_xmit() after cleaning up DMA mapping errors. -Mchan ]

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/bnx2.c |   72 +++++++++++++++++++++++++++++++++++++++++++--------
 drivers/net/bnx2.h |    1 +
 2 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 539d23b..f47bf50 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -2815,13 +2815,21 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
 			}
 		}
 
-		skb_dma_unmap(&bp->pdev->dev, skb, DMA_TO_DEVICE);
+		pci_unmap_single(bp->pdev, pci_unmap_addr(tx_buf, mapping),
+			skb_headlen(skb), PCI_DMA_TODEVICE);
 
 		tx_buf->skb = NULL;
 		last = tx_buf->nr_frags;
 
 		for (i = 0; i < last; i++) {
 			sw_cons = NEXT_TX_BD(sw_cons);
+
+			pci_unmap_page(bp->pdev,
+				pci_unmap_addr(
+					&txr->tx_buf_ring[TX_RING_IDX(sw_cons)],
+					mapping),
+				skb_shinfo(skb)->frags[i].size,
+				PCI_DMA_TODEVICE);
 		}
 
 		sw_cons = NEXT_TX_BD(sw_cons);
@@ -5295,17 +5303,29 @@ bnx2_free_tx_skbs(struct bnx2 *bp)
 		for (j = 0; j < TX_DESC_CNT; ) {
 			struct sw_tx_bd *tx_buf = &txr->tx_buf_ring[j];
 			struct sk_buff *skb = tx_buf->skb;
+			int k, last;
 
 			if (skb == NULL) {
 				j++;
 				continue;
 			}
 
-			skb_dma_unmap(&bp->pdev->dev, skb, DMA_TO_DEVICE);
+			pci_unmap_single(bp->pdev,
+					 pci_unmap_addr(tx_buf, mapping),
+					 skb_headlen(skb),
+					 PCI_DMA_TODEVICE);
 
 			tx_buf->skb = NULL;
 
-			j += skb_shinfo(skb)->nr_frags + 1;
+			last = tx_buf->nr_frags;
+			j++;
+			for (k = 0; k < last; k++, j++) {
+				tx_buf = &txr->tx_buf_ring[TX_RING_IDX(j)];
+				pci_unmap_page(bp->pdev,
+					pci_unmap_addr(tx_buf, mapping),
+					skb_shinfo(skb)->frags[k].size,
+					PCI_DMA_TODEVICE);
+			}
 			dev_kfree_skb(skb);
 		}
 	}
@@ -5684,11 +5704,12 @@ bnx2_run_loopback(struct bnx2 *bp, int loopback_mode)
 	for (i = 14; i < pkt_size; i++)
 		packet[i] = (unsigned char) (i & 0xff);
 
-	if (skb_dma_map(&bp->pdev->dev, skb, DMA_TO_DEVICE)) {
+	map = pci_map_single(bp->pdev, skb->data, pkt_size,
+		PCI_DMA_TODEVICE);
+	if (pci_dma_mapping_error(bp->pdev, map)) {
 		dev_kfree_skb(skb);
 		return -EIO;
 	}
-	map = skb_shinfo(skb)->dma_head;
 
 	REG_WR(bp, BNX2_HC_COMMAND,
 	       bp->hc_cmd | BNX2_HC_COMMAND_COAL_NOW_WO_INT);
@@ -5723,7 +5744,7 @@ bnx2_run_loopback(struct bnx2 *bp, int loopback_mode)
 
 	udelay(5);
 
-	skb_dma_unmap(&bp->pdev->dev, skb, DMA_TO_DEVICE);
+	pci_unmap_single(bp->pdev, map, pkt_size, PCI_DMA_TODEVICE);
 	dev_kfree_skb(skb);
 
 	if (bnx2_get_hw_tx_cons(tx_napi) != txr->tx_prod)
@@ -6302,7 +6323,6 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct bnx2_napi *bnapi;
 	struct bnx2_tx_ring_info *txr;
 	struct netdev_queue *txq;
-	struct skb_shared_info *sp;
 
 	/*  Determine which tx ring we will be placed on */
 	i = skb_get_queue_mapping(skb);
@@ -6367,16 +6387,15 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	} else
 		mss = 0;
 
-	if (skb_dma_map(&bp->pdev->dev, skb, DMA_TO_DEVICE)) {
+	mapping = pci_map_single(bp->pdev, skb->data, len, PCI_DMA_TODEVICE);
+	if (pci_dma_mapping_error(bp->pdev, mapping)) {
 		dev_kfree_skb(skb);
 		return NETDEV_TX_OK;
 	}
 
-	sp = skb_shinfo(skb);
-	mapping = sp->dma_head;
-
 	tx_buf = &txr->tx_buf_ring[ring_prod];
 	tx_buf->skb = skb;
+	pci_unmap_addr_set(tx_buf, mapping, mapping);
 
 	txbd = &txr->tx_desc_ring[ring_prod];
 
@@ -6397,7 +6416,12 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		txbd = &txr->tx_desc_ring[ring_prod];
 
 		len = frag->size;
-		mapping = sp->dma_maps[i];
+		mapping = pci_map_page(bp->pdev, frag->page, frag->page_offset,
+			len, PCI_DMA_TODEVICE);
+		if (pci_dma_mapping_error(bp->pdev, mapping))
+			goto dma_error;
+		pci_unmap_addr_set(&txr->tx_buf_ring[ring_prod], mapping,
+				   mapping);
 
 		txbd->tx_bd_haddr_hi = (u64) mapping >> 32;
 		txbd->tx_bd_haddr_lo = (u64) mapping & 0xffffffff;
@@ -6424,6 +6448,30 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	return NETDEV_TX_OK;
+dma_error:
+	/* save value of frag that failed */
+	last_frag = i;
+
+	/* start back at beginning and unmap skb */
+	prod = txr->tx_prod;
+	ring_prod = TX_RING_IDX(prod);
+	tx_buf = &txr->tx_buf_ring[ring_prod];
+	tx_buf->skb = NULL;
+	pci_unmap_single(bp->pdev, pci_unmap_addr(tx_buf, mapping),
+			 skb_headlen(skb), PCI_DMA_TODEVICE);
+
+	/* unmap remaining mapped pages */
+	for (i = 0; i < last_frag; i++) {
+		prod = NEXT_TX_BD(prod);
+		ring_prod = TX_RING_IDX(prod);
+		tx_buf = &txr->tx_buf_ring[ring_prod];
+		pci_unmap_page(bp->pdev, pci_unmap_addr(tx_buf, mapping),
+			       skb_shinfo(skb)->frags[i].size,
+			       PCI_DMA_TODEVICE);
+	}
+
+	dev_kfree_skb(skb);
+	return NETDEV_TX_OK;
 }
 
 /* Called with rtnl_lock */
diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h
index a4d8340..4908b9f 100644
--- a/drivers/net/bnx2.h
+++ b/drivers/net/bnx2.h
@@ -6559,6 +6559,7 @@ struct sw_pg {
 
 struct sw_tx_bd {
 	struct sk_buff		*skb;
+	DECLARE_PCI_UNMAP_ADDR(mapping)
 	unsigned short		is_gso;
 	unsigned short		nr_frags;
 };
-- 
1.6.4.GIT

RE: [RFC PATCH 00/10] Remove skb_dma_map/unmap calls

[Duyck, Alexander H]

David Miller wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> Date: Tue, 24 Nov 2009 17:20:12 -0800
> 
>> This patch series removes the skb_dma_map and skb_dma_unmap function
>> calls. The reason for this change is because the use of
>> skb_dma_map/unmap can lead to serious issues when HW IOMMU is
>> enabled.  This is because each mapping 
>> of the skb with a HW IOMMU enabled results in a new set of DMA
>> mappings. 
>> This in turn leads to issues when skbs are cloned for uses such as
>> bridging or pktgen because each transmitting device will update the
>> skb shared info structure resulting in some mappings being
>> overwritten, and others being freed multiple times. 
>> 
>> I am looking for input specifically on the tg3, be2net, and bnx2
>> driver patches as I am not very familiar with them and I am not
>> certain if additional changes are required.
>> 
>> I have included the changes for the Intel wired Ethernet drivers as a
>> reference.
> 
> These changes look good, but at least one of them does not apply
> to current net-2.6
> 
> Can you respin this against current sources?
> 
> Thanks a lot for doing this work Alexander.

Due to the fact that nobody else had reported the issue and it existed in previous kernel versions I figured it would be easier to just drop it into net-next-2.6.

If you want I can change the patches to net-2.6 though if that is what you would prefer.

Thanks,

Alex

Re: [RFC PATCH 00/10] Remove skb_dma_map/unmap calls

[David Miller]

From: "Duyck, Alexander H" <alexander.h.duyck@intel.com>
Date: Mon, 30 Nov 2009 17:03:25 -0800

> David Miller wrote:
>> These changes look good, but at least one of them does not apply
>> to current net-2.6
>> 
>> Can you respin this against current sources?
>> 
>> Thanks a lot for doing this work Alexander.
>
> Due to the fact that nobody else had reported the issue and it
> existed in previous kernel versions I figured it would be easier to
> just drop it into net-next-2.6.
>
> If you want I can change the patches to net-2.6 though if that is
> what you would prefer.

Ok, net-next-2.6 is fine with me too.

Re: [RFC PATCH 00/10] Remove skb_dma_map/unmap calls

[Eric Dumazet]

David Miller a écrit :
> From: "Duyck, Alexander H" <alexander.h.duyck@intel.com>
> Date: Mon, 30 Nov 2009 17:03:25 -0800
> 
>> David Miller wrote:
>>> These changes look good, but at least one of them does not apply
>>> to current net-2.6
>>>
>>> Can you respin this against current sources?
>>>
>>> Thanks a lot for doing this work Alexander.
>> Due to the fact that nobody else had reported the issue and it
>> existed in previous kernel versions I figured it would be easier to
>> just drop it into net-next-2.6.
>>
>> If you want I can change the patches to net-2.6 though if that is
>> what you would prefer.
> 
> Ok, net-next-2.6 is fine with me too.

I am not sure if I already mentioned these patches speedup
TX completion path, since we avoid a cache line miss, if
driver caches nr_frags/mapping in its tx descriptor (like bnx2)

(no need to access to skb_shinfo(skb)->dma_head & ->nr_frags)

(This is not an argument vs ns-next-2.6, or net-2.6, just a note
to say that these patches are welcomed performance wise)