Patchwork ibmveth: Fix alignment of rx queue bug

login
register
mail settings
Submitter Santiago Leon
Date Sept. 5, 2012, 12:41 a.m.
Message ID <50469FC1.2000202@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/181712/
State Accepted
Delegated to: David Miller
Headers show

Comments

Santiago Leon - Sept. 5, 2012, 12:41 a.m.
This patch fixes a bug found by Nish Aravamudan 
(https://lkml.org/lkml/2012/5/15/220) where the driver is not following
the spec (it is not aligning the rx buffer on a 16-byte boundary) and the
hypervisor aborts the registration, making the device unusable. 

The fix follows BenH's recommendation (https://lkml.org/lkml/2012/7/20/461)
to replace the kmalloc+map for a single call to dma_alloc_coherent()
because that function always aligns to a 16-byte boundary.

The stable trees will run into this bug whenever the rx buffer kmalloc call
returns something not aligned on a 16-byte boundary.

Cc: <stable@vger.kernel.org>
Signed-off-by: Santiago Leon <santil@linux.vnet.ibm.com>
---
 ibmveth.c |   26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Sept. 5, 2012, 9:45 p.m.
From: Santiago Leon <santil@linux.vnet.ibm.com>
Date: Tue, 04 Sep 2012 19:41:37 -0500

> This patch fixes a bug found by Nish Aravamudan 
> (https://lkml.org/lkml/2012/5/15/220) where the driver is not following
> the spec (it is not aligning the rx buffer on a 16-byte boundary) and the
> hypervisor aborts the registration, making the device unusable. 
> 
> The fix follows BenH's recommendation (https://lkml.org/lkml/2012/7/20/461)
> to replace the kmalloc+map for a single call to dma_alloc_coherent()
> because that function always aligns to a 16-byte boundary.
> 
> The stable trees will run into this bug whenever the rx buffer kmalloc call
> returns something not aligned on a 16-byte boundary.
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Santiago Leon <santil@linux.vnet.ibm.com>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

--- a/drivers/net/ethernet/ibm/ibmveth.c	2012-07-09 16:00:53.000000000 -0400
+++ b/drivers/net/ethernet/ibm/ibmveth.c	2012-08-17 19:51:02.840000188 -0400
@@ -472,14 +472,9 @@  static void ibmveth_cleanup(struct ibmve
 	}
 
 	if (adapter->rx_queue.queue_addr != NULL) {
-		if (!dma_mapping_error(dev, adapter->rx_queue.queue_dma)) {
-			dma_unmap_single(dev,
-					adapter->rx_queue.queue_dma,
-					adapter->rx_queue.queue_len,
-					DMA_BIDIRECTIONAL);
-			adapter->rx_queue.queue_dma = DMA_ERROR_CODE;
-		}
-		kfree(adapter->rx_queue.queue_addr);
+		dma_free_coherent(dev, adapter->rx_queue.queue_len,
+				  adapter->rx_queue.queue_addr,
+				  adapter->rx_queue.queue_dma);
 		adapter->rx_queue.queue_addr = NULL;
 	}
 
@@ -556,10 +551,13 @@  static int ibmveth_open(struct net_devic
 		goto err_out;
 	}
 
+	dev = &adapter->vdev->dev;
+
 	adapter->rx_queue.queue_len = sizeof(struct ibmveth_rx_q_entry) *
 						rxq_entries;
-	adapter->rx_queue.queue_addr = kmalloc(adapter->rx_queue.queue_len,
-						GFP_KERNEL);
+	adapter->rx_queue.queue_addr =
+	    dma_alloc_coherent(dev, adapter->rx_queue.queue_len,
+			       &adapter->rx_queue.queue_dma, GFP_KERNEL);
 
 	if (!adapter->rx_queue.queue_addr) {
 		netdev_err(netdev, "unable to allocate rx queue pages\n");
@@ -567,19 +565,13 @@  static int ibmveth_open(struct net_devic
 		goto err_out;
 	}
 
-	dev = &adapter->vdev->dev;
-
 	adapter->buffer_list_dma = dma_map_single(dev,
 			adapter->buffer_list_addr, 4096, DMA_BIDIRECTIONAL);
 	adapter->filter_list_dma = dma_map_single(dev,
 			adapter->filter_list_addr, 4096, DMA_BIDIRECTIONAL);
-	adapter->rx_queue.queue_dma = dma_map_single(dev,
-			adapter->rx_queue.queue_addr,
-			adapter->rx_queue.queue_len, DMA_BIDIRECTIONAL);
 
 	if ((dma_mapping_error(dev, adapter->buffer_list_dma)) ||
-	    (dma_mapping_error(dev, adapter->filter_list_dma)) ||
-	    (dma_mapping_error(dev, adapter->rx_queue.queue_dma))) {
+	    (dma_mapping_error(dev, adapter->filter_list_dma))) {
 		netdev_err(netdev, "unable to map filter or buffer list "
 			   "pages\n");
 		rc = -ENOMEM;