Thread (7 messages) 7 messages, 3 authors, 2010-04-02

Re: [PATCH RFC] fix problems with NETIF_F_HIGHDMA in networking drivers v2

From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date: 2010-03-31 08:36:01
Also in: lkml
Subsystem: networking [general], the rest · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

On Thu, 25 Mar 2010 20:35:20 -0700 (PDT)
David Miller [off-list ref] wrote:
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date: Fri, 26 Mar 2010 12:33:12 +0900
quoted
On Thu, 25 Mar 2010 19:03:37 -0600
Robert Hancock [off-list ref] wrote:
quoted
This seems like it could be a reasonable approach. The only thing is
that in this code you're returning 1 if the parent device has no DMA
mask set. Wouldn't it make more sense to return 0 in this case? I'm
assuming that in that situation it's a virtual device not backed by
any hardware and there should be no DMA mask restriction...
I chose the safer option because I don't know enough how net_device
structure is used. If returning zero in such case is always safe, it's
fine by me. any example of such virtual device driver?
Like Fujita I'd rather play it safe here.

Even for virtual devices, DMA information up to the root bus
ought to be sane.
Agreed. 

Here's the patch in the proper format.

Probably, PCI_DMA_BUS_IS_PHYS should be renamed to something like
DMA_BUS_IS_PHYS and moved to the better place. Then we can avoid
including linux/pci.h.

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH] net: change illegal_highdma to use dma_mask

Robert Hancock pointed out two problems about NETIF_F_HIGHDMA:

-Many drivers only set the flag when they detect they can use 64-bit DMA,
since otherwise they could receive DMA addresses that they can't handle
(which on platforms without IOMMU/SWIOTLB support is fatal). This means that if
64-bit support isn't available, even buffers located below 4GB will get copied
unnecessarily.

-Some drivers set the flag even though they can't actually handle 64-bit DMA,
which would mean that on platforms without IOMMU/SWIOTLB they would get a DMA
mapping error if the memory they received happened to be located above 4GB.

http://lkml.org/lkml/2010/3/3/530


We can use the dma_mask if we need bouncing or not here. Then we can
safely fix drivers that misuse NETIF_F_HIGHDMA.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 net/core/dev.c |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 5e3dc28..3a09f76 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -129,6 +129,7 @@
 #include <linux/jhash.h>
 #include <linux/random.h>
 #include <trace/events/napi.h>
+#include <linux/pci.h>
 
 #include "net-sysfs.h"
 
@@ -1790,14 +1791,21 @@ static inline int illegal_highdma(struct net_device *dev, struct sk_buff *skb)
 {
 #ifdef CONFIG_HIGHMEM
 	int i;
+	if (!(dev->features & NETIF_F_HIGHDMA)) {
+		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
+			if (PageHighMem(skb_shinfo(skb)->frags[i].page))
+				return 1;
+	}
 
-	if (dev->features & NETIF_F_HIGHDMA)
-		return 0;
-
-	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
-		if (PageHighMem(skb_shinfo(skb)->frags[i].page))
-			return 1;
+	if (PCI_DMA_BUS_IS_PHYS) {
+		struct device *pdev = dev->dev.parent;
 
+		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+			dma_addr_t addr = page_to_phys(skb_shinfo(skb)->frags[i].page);
+			if (!pdev->dma_mask || addr + PAGE_SIZE - 1 > *pdev->dma_mask)
+				return 1;
+		}
+	}
 #endif
 	return 0;
 }
-- 
1.7.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help