[PATCH v4 10/10] net/ps3_gelic: Fix DMA mapping problems

Geoff Levand geoff at infradead.org
Sat Jul 24 06:31:48 AEST 2021


Fixes several DMA mapping problems with the PS3's gelic network driver:

 * Change from checking the return value of dma_map_single to using the
   dma_mapping_error routine.
 * Use the correct buffer length when mapping the RX skb.
 * Improved error checking and debug logging.

Fixes runtime errors like these, and also other randomly occurring errors:

  IP-Config: Complete:
  DMA-API: ps3_gelic_driver sb_05: device driver failed to check map error
  WARNING: CPU: 0 PID: 0 at kernel/dma/debug.c:1027 .check_unmap+0x888/0x8dc

Signed-off-by: Geoff Levand <geoff at infradead.org>
---
 drivers/net/ethernet/toshiba/ps3_gelic_net.c | 183 +++++++++++--------
 1 file changed, 108 insertions(+), 75 deletions(-)

diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index 42f4de9ad5fe..11ddeacb1159 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -336,22 +336,31 @@ static int gelic_card_init_chain(struct gelic_card *card,
 	struct gelic_descr_chain *chain, struct gelic_descr *start_descr,
 	int descr_count)
 {
-	int i;
-	struct gelic_descr *descr;
+	struct gelic_descr *descr = start_descr;
 	struct device *dev = ctodev(card);
+	unsigned int index;
 
-	descr = start_descr;
-	memset(descr, 0, sizeof(*descr) *descr_count);
+	memset(start_descr, 0, descr_count * sizeof(*start_descr));
 
-	for (i = 0; i < descr_count; i++, descr++) {
-		descr->link.size = sizeof(struct gelic_hw_regs);
+	for (index = 0, descr = start_descr; index < descr_count;
+		index++, descr++) {
 		gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
-		descr->link.cpu_addr =
-			dma_map_single(dev, descr, descr->link.size,
-				DMA_BIDIRECTIONAL);
 
-		if (!descr->link.cpu_addr)
-			goto iommu_error;
+		descr->link.size = sizeof(struct gelic_hw_regs);
+		descr->link.cpu_addr = dma_map_single(dev, descr,
+			descr->link.size, DMA_BIDIRECTIONAL);
+
+		if (unlikely(dma_mapping_error(dev, descr->link.cpu_addr))) {
+			dev_err(dev, "%s:%d: dma_mapping_error\n", __func__,
+				__LINE__);
+
+			for (index--, descr--; index > 0; index--, descr--) {
+				if (descr->link.cpu_addr) {
+					gelic_unmap_link(dev, descr);
+				}
+			}
+			return -ENOMEM;
+		}
 
 		descr->next = descr + 1;
 		descr->prev = descr - 1;
@@ -360,8 +369,9 @@ static int gelic_card_init_chain(struct gelic_card *card,
 	(descr - 1)->next = start_descr;
 	start_descr->prev = (descr - 1);
 
-	descr = start_descr;
-	for (i = 0; i < descr_count; i++, descr++) {
+	/* chain bus addr of hw descriptor */
+	for (index = 0, descr = start_descr; index < descr_count;
+		index++, descr++) {
 		descr->hw_regs.next_descr_addr =
 			cpu_to_be32(descr->next->link.cpu_addr);
 	}
@@ -373,12 +383,6 @@ static int gelic_card_init_chain(struct gelic_card *card,
 	(descr - 1)->hw_regs.next_descr_addr = 0;
 
 	return 0;
-
-iommu_error:
-	for (i--, descr--; 0 <= i; i--, descr--)
-		if (descr->link.cpu_addr)
-			gelic_unmap_link(dev, descr);
-	return -ENOMEM;
 }
 
 /**
@@ -395,49 +399,63 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
 	struct gelic_descr *descr)
 {
 	struct device *dev = ctodev(card);
-	int offset;
-	unsigned int bufsize;
+	struct aligned_buff {
+		unsigned int total_bytes;
+		unsigned int offset;
+	};
+	struct aligned_buff a_buf;
+	dma_addr_t cpu_addr;
 
 	if (gelic_descr_get_status(descr) !=  GELIC_DESCR_DMA_NOT_IN_USE) {
 		dev_err(dev, "%s:%d: ERROR status\n", __func__, __LINE__);
 	}
 
-	/* we need to round up the buffer size to a multiple of 128 */
-	bufsize = ALIGN(GELIC_NET_MAX_MTU, GELIC_NET_RXBUF_ALIGN);
+	a_buf.total_bytes = ALIGN(GELIC_NET_MAX_MTU, GELIC_NET_RXBUF_ALIGN)
+		+ GELIC_NET_RXBUF_ALIGN;
+
+	descr->skb = dev_alloc_skb(a_buf.total_bytes);
 
-	/* and we need to have it 128 byte aligned, therefore we allocate a
-	 * bit more */
-	descr->skb = dev_alloc_skb(bufsize + GELIC_NET_RXBUF_ALIGN - 1);
 	if (!descr->skb) {
-		descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */
+		descr->hw_regs.payload.dev_addr = 0;
+		descr->hw_regs.payload.size = 0;
 		return -ENOMEM;
 	}
-	descr->hw_regs.payload.size = cpu_to_be32(bufsize);
+
+	a_buf.offset = PTR_ALIGN(descr->skb->data, GELIC_NET_RXBUF_ALIGN)
+		- descr->skb->data;
+
+	if (a_buf.offset) {
+		dev_dbg(dev, "%s:%d: offset=%u\n", __func__, __LINE__,
+			a_buf.offset);
+		skb_reserve(descr->skb, a_buf.offset);
+	}
+
 	descr->hw_regs.dmac_cmd_status = 0;
 	descr->hw_regs.result_size = 0;
 	descr->hw_regs.valid_size = 0;
 	descr->hw_regs.data_error = 0;
 
-	offset = ((unsigned long)descr->skb->data) &
-		(GELIC_NET_RXBUF_ALIGN - 1);
-	if (offset)
-		skb_reserve(descr->skb, GELIC_NET_RXBUF_ALIGN - offset);
-	/* io-mmu-map the skb */
-	descr->hw_regs.payload.dev_addr = cpu_to_be32(dma_map_single(dev,
-						     descr->skb->data,
-						     GELIC_NET_MAX_MTU,
-						     DMA_FROM_DEVICE));
-	if (!descr->hw_regs.payload.dev_addr) {
+	descr->hw_regs.payload.size = a_buf.total_bytes - a_buf.offset;
+	cpu_addr = dma_map_single(dev, descr->skb->data,
+		descr->hw_regs.payload.size, DMA_FROM_DEVICE);
+	descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
+
+	if (unlikely(dma_mapping_error(dev, cpu_addr))) {
+		dev_err(dev, "%s:%d: dma_mapping_error\n", __func__, __LINE__);
+
+		descr->hw_regs.payload.dev_addr = 0;
+		descr->hw_regs.payload.size = 0;
+
 		dev_kfree_skb_any(descr->skb);
 		descr->skb = NULL;
-		dev_info(dev,
-			 "%s:Could not iommu-map rx buffer\n", __func__);
+
 		gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
+
 		return -ENOMEM;
-	} else {
-		gelic_descr_set_status(descr, GELIC_DESCR_DMA_CARDOWNED);
-		return 0;
 	}
+
+	gelic_descr_set_status(descr, GELIC_DESCR_DMA_CARDOWNED);
+	return 0;
 }
 
 /**
@@ -454,13 +472,18 @@ static void gelic_card_release_rx_chain(struct gelic_card *card)
 		if (descr->skb) {
 			dma_unmap_single(dev,
 				be32_to_cpu(descr->hw_regs.payload.dev_addr),
-				descr->skb->len, DMA_FROM_DEVICE);
-			descr->hw_regs.payload.dev_addr = 0;
+				descr->hw_regs.payload.size, DMA_FROM_DEVICE);
+
 			dev_kfree_skb_any(descr->skb);
 			descr->skb = NULL;
+
 			gelic_descr_set_status(descr,
 				GELIC_DESCR_DMA_NOT_IN_USE);
 		}
+
+		descr->hw_regs.payload.dev_addr = 0;
+		descr->hw_regs.payload.size = 0;
+
 		descr = descr->next;
 	} while (descr != card->rx_chain.head);
 }
@@ -526,17 +549,19 @@ static void gelic_descr_release_tx(struct gelic_card *card,
 		GELIC_DESCR_TX_TAIL));
 
 	dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr),
-		skb->len, DMA_TO_DEVICE);
-	dev_kfree_skb_any(skb);
+		descr->hw_regs.payload.size, DMA_TO_DEVICE);
 
 	descr->hw_regs.payload.dev_addr = 0;
 	descr->hw_regs.payload.size = 0;
+
+	dev_kfree_skb_any(skb);
+	descr->skb = NULL;
+
 	descr->hw_regs.next_descr_addr = 0;
 	descr->hw_regs.result_size = 0;
 	descr->hw_regs.valid_size = 0;
 	descr->hw_regs.data_status = 0;
 	descr->hw_regs.data_error = 0;
-	descr->skb = NULL;
 
 	gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
 }
@@ -565,31 +590,34 @@ static void gelic_card_wake_queues(struct gelic_card *card)
 static void gelic_card_release_tx_chain(struct gelic_card *card, int stop)
 {
 	struct gelic_descr_chain *tx_chain;
-	enum gelic_descr_dma_status status;
 	struct device *dev = ctodev(card);
-	struct net_device *netdev;
-	int release = 0;
+	int release;
+
+	for (release = 0, tx_chain = &card->tx_chain;
+		tx_chain->head != tx_chain->tail && tx_chain->tail;
+		tx_chain->tail = tx_chain->tail->next) {
+		enum gelic_descr_dma_status status;
+		struct gelic_descr *descr;
+		struct net_device *netdev;
+
+		descr = tx_chain->tail;
+		status = gelic_descr_get_status(descr);
+		netdev = descr->skb->dev;
 
-	for (tx_chain = &card->tx_chain;
-	     tx_chain->head != tx_chain->tail && tx_chain->tail;
-	     tx_chain->tail = tx_chain->tail->next) {
-		status = gelic_descr_get_status(tx_chain->tail);
-		netdev = tx_chain->tail->skb->dev;
 		switch (status) {
 		case GELIC_DESCR_DMA_RESPONSE_ERROR:
 		case GELIC_DESCR_DMA_PROTECTION_ERROR:
 		case GELIC_DESCR_DMA_FORCE_END:
-			 dev_info_ratelimited(dev,
-					 "%s:%d: forcing end of tx descriptor with status %x\n",
-					 __func__, __LINE__, status);
+			dev_info_ratelimited(dev,
+				"%s:%d: forcing end of tx descriptor with status %x\n",
+				__func__, __LINE__, status);
 			netdev->stats.tx_dropped++;
 			break;
 
 		case GELIC_DESCR_DMA_COMPLETE:
-			if (tx_chain->tail->skb) {
+			if (descr->skb) {
 				netdev->stats.tx_packets++;
-				netdev->stats.tx_bytes +=
-					tx_chain->tail->skb->len;
+				netdev->stats.tx_bytes += descr->skb->len;
 			}
 			break;
 
@@ -599,7 +627,7 @@ static void gelic_card_release_tx_chain(struct gelic_card *card, int stop)
 			}
 		}
 
-		gelic_descr_release_tx(card, tx_chain->tail);
+		gelic_descr_release_tx(card, descr);
 		release++;
 	}
 out:
@@ -703,19 +731,19 @@ int gelic_net_stop(struct net_device *netdev)
  *
  * returns the address of the next descriptor, or NULL if not available.
  */
-static struct gelic_descr *
-gelic_card_get_next_tx_descr(struct gelic_card *card)
+static struct gelic_descr *gelic_card_get_next_tx_descr(struct gelic_card *card)
 {
 	if (!card->tx_chain.head)
 		return NULL;
+
 	/*  see if the next descriptor is free */
 	if (card->tx_chain.tail != card->tx_chain.head->next &&
-		gelic_descr_get_status(card->tx_chain.head) ==
-			GELIC_DESCR_DMA_NOT_IN_USE)
+		(gelic_descr_get_status(card->tx_chain.head) ==
+			GELIC_DESCR_DMA_NOT_IN_USE)) {
 		return card->tx_chain.head;
-	else
-		return NULL;
+	}
 
+	return NULL;
 }
 
 /**
@@ -809,18 +837,23 @@ static int gelic_descr_prepare_tx(struct gelic_card *card,
 		if (!skb_tmp) {
 			return -ENOMEM;
 		}
+
 		skb = skb_tmp;
 	}
 
-	cpu_addr = dma_map_single(dev, skb->data, skb->len, DMA_TO_DEVICE);
+	descr->hw_regs.payload.size = skb->len;
+	cpu_addr = dma_map_single(dev, skb->data, descr->hw_regs.payload.size,
+		DMA_TO_DEVICE);
+	descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
 
-	if (!cpu_addr) {
+	if (unlikely(dma_mapping_error(dev, cpu_addr))) {
 		dev_err(dev, "%s:%d: dma_mapping_error\n", __func__, __LINE__);
+
+		descr->hw_regs.payload.dev_addr = 0;
+		descr->hw_regs.payload.size = 0;
 		return -ENOMEM;
 	}
 
-	descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
-	descr->hw_regs.payload.size = cpu_to_be32(skb->len);
 	descr->skb = skb;
 	descr->hw_regs.data_status = 0;
 	descr->hw_regs.next_descr_addr = 0; /* terminate hw descr */
@@ -948,9 +981,9 @@ static void gelic_net_pass_skb_up(struct gelic_descr *descr,
 
 	data_status = be32_to_cpu(descr->hw_regs.data_status);
 	data_error = be32_to_cpu(descr->hw_regs.data_error);
-	/* unmap skb buffer */
+
 	dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr),
-			 GELIC_NET_MAX_MTU, DMA_FROM_DEVICE);
+			 descr->hw_regs.payload.size, DMA_FROM_DEVICE);
 
 	skb_put(skb, be32_to_cpu(descr->hw_regs.valid_size) ?
 		be32_to_cpu(descr->hw_regs.valid_size) :
-- 
2.25.1



More information about the Linuxppc-dev mailing list