[PATCH 0/8] x86 dma_*_coherent rework patchset v2

[PATCH 0/8] x86 dma_*_coherent rework patchset v2

[Joerg Roedel]

Hi,

this patchset reworks the dma_*_coherent functions in the DMA layer for the x86
architecture. The patch series extends the existing DMA backends with missing
*coherent callbacks and simplifies the generic function to basically only call
the registered backend. This allows future optimizations in hardware specific
IOMMU implementations.
The code ist tested on AMD64 with AMD IOMMU, GART, SWIOTLB and NOMMU as well as
on my old 486 box. Muli tested the Calgary specific patch.

Joerg

Changes since v1:

- fixed wrong logic in the pci-nommu alloc_coherent code
- moved dma_*_coherent to include/asm-x86/dma-mapping.h

git diff --stat tip/master.. :

 arch/x86/kernel/amd_iommu.c      |    2 -
 arch/x86/kernel/pci-calgary_64.c |   14 ++++
 arch/x86/kernel/pci-dma.c        |  146 +-------------------------------------
 arch/x86/kernel/pci-gart_64.c    |   35 +++++++++-
 arch/x86/kernel/pci-nommu.c      |   62 ++++++++++++++++
 include/asm-x86/dma-mapping.h    |   47 ++++++++++---
 6 files changed, 149 insertions(+), 157 deletions(-)

[PATCH 1/8] x86: add alloc_coherent dma_ops callback to GART driver

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/pci-gart_64.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index 4d8efb0..44a75a6 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -506,6 +506,26 @@ error:
 	return 0;
 }
 
+/* allocate and map a coherent mapping */
+static void *
+gart_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_addr,
+		    gfp_t flag)
+{
+	void *vaddr;
+
+	vaddr = (void *)__get_free_pages(flag, get_order(size));
+	if (!vaddr)
+		return NULL;
+
+	*dma_addr = gart_map_single(dev, __pa(vaddr), size, DMA_BIDIRECTIONAL);
+	if (*dma_addr != bad_dma_address)
+		return vaddr;
+
+	free_pages((unsigned long)vaddr, get_order(size));
+
+	return NULL;
+}
+
 static int no_agp;
 
 static __init unsigned long check_iommu_size(unsigned long aper, u64 aper_size)
@@ -708,6 +728,7 @@ static struct dma_mapping_ops gart_dma_ops = {
 	.sync_sg_for_device		= NULL,
 	.map_sg				= gart_map_sg,
 	.unmap_sg			= gart_unmap_sg,
+	.alloc_coherent			= gart_alloc_coherent,
 };
 
 void gart_iommu_shutdown(void)
-- 
1.5.3.7

[PATCH 2/8] x86: add free_coherent dma_ops callback to GART driver

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/pci-gart_64.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index 44a75a6..794d026 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -526,6 +526,15 @@ gart_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_addr,
 	return NULL;
 }
 
+/* free a coherent mapping */
+static void
+gart_free_coherent(struct device *dev, size_t size, void *vaddr,
+		   dma_addr_t dma_addr)
+{
+	gart_unmap_single(dev, dma_addr, size, DMA_BIDIRECTIONAL);
+	free_pages((unsigned long)vaddr, get_order(size));
+}
+
 static int no_agp;
 
 static __init unsigned long check_iommu_size(unsigned long aper, u64 aper_size)
@@ -729,6 +738,7 @@ static struct dma_mapping_ops gart_dma_ops = {
 	.map_sg				= gart_map_sg,
 	.unmap_sg			= gart_unmap_sg,
 	.alloc_coherent			= gart_alloc_coherent,
+	.free_coherent			= gart_free_coherent,
 };
 
 void gart_iommu_shutdown(void)
-- 
1.5.3.7

[PATCH 3/8] x86: add free_coherent dma_ops callback to Calgary IOMMU driver

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
Acked-by: Muli Ben-Yehuda <muli@il.ibm.com>
---
 arch/x86/kernel/pci-calgary_64.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/pci-calgary_64.c b/arch/x86/kernel/pci-calgary_64.c
index 8e41d42..ea6a0c8 100644
--- a/arch/x86/kernel/pci-calgary_64.c
+++ b/arch/x86/kernel/pci-calgary_64.c
@@ -510,8 +510,22 @@ error:
 	return ret;
 }
 
+static void calgary_free_coherent(struct device *dev, size_t size,
+				  void *vaddr, dma_addr_t dma_handle)
+{
+	unsigned int npages;
+	struct iommu_table *tbl = find_iommu_table(dev);
+
+	size = PAGE_ALIGN(size);
+	npages = size >> PAGE_SHIFT;
+
+	iommu_free(tbl, dma_handle, npages);
+	free_pages((unsigned long)vaddr, get_order(size));
+}
+
 static struct dma_mapping_ops calgary_dma_ops = {
 	.alloc_coherent = calgary_alloc_coherent,
+	.free_coherent = calgary_free_coherent,
 	.map_single = calgary_map_single,
 	.unmap_single = calgary_unmap_single,
 	.map_sg = calgary_map_sg,
-- 
1.5.3.7

[PATCH 4/8] x86: add alloc_coherent dma_ops callback to NOMMU driver

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/pci-nommu.c |   55 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/pci-nommu.c b/arch/x86/kernel/pci-nommu.c
index 3f91f71..b8ce83c 100644
--- a/arch/x86/kernel/pci-nommu.c
+++ b/arch/x86/kernel/pci-nommu.c
@@ -72,7 +72,62 @@ static int nommu_map_sg(struct device *hwdev, struct scatterlist *sg,
 	return nents;
 }
 
+static void *
+nommu_alloc_coherent(struct device *hwdev, size_t size,
+		     dma_addr_t *dma_addr, gfp_t gfp)
+{
+	unsigned long dma_mask;
+	int node;
+	struct page *page;
+
+	if (hwdev->dma_mask == NULL)
+		return NULL;
+
+	gfp &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32);
+	gfp |= __GFP_ZERO;
+
+	dma_mask = hwdev->coherent_dma_mask;
+	if (!dma_mask)
+		dma_mask = *(hwdev->dma_mask);
+
+	if (dma_mask < DMA_24BIT_MASK)
+		return NULL;
+
+	node = dev_to_node(hwdev);
+
+#ifdef CONFIG_X86_64
+	if (dma_mask <= DMA_32BIT_MASK)
+		gfp |= GFP_DMA32;
+#endif
+
+	/* No alloc-free penalty for ISA devices */
+	if (dma_mask == DMA_24BIT_MASK)
+		gfp |= GFP_DMA;
+
+again:
+	page = alloc_pages_node(node, gfp, get_order(size));
+	if (!page)
+		return NULL;
+
+	if ((page_to_phys(page) + size > dma_mask) && !(gfp & GFP_DMA)) {
+		free_pages((unsigned long)page_address(page), get_order(size));
+		gfp |= GFP_DMA;
+		goto again;
+	}
+
+	*dma_addr = page_to_phys(page);
+	if (check_addr("alloc_coherent", hwdev, *dma_addr, size)) {
+		flush_write_buffers();
+		return page_address(page);
+	}
+
+	free_pages((unsigned long)page_address(page), get_order(size));
+
+	return NULL;
+}
+
 struct dma_mapping_ops nommu_dma_ops = {
+	.alloc_coherent = nommu_alloc_coherent,
 	.map_single = nommu_map_single,
 	.map_sg = nommu_map_sg,
 	.is_phys = 1,
-- 
1.5.3.7

[PATCH 5/8] x86: add free_coherent dma_ops callback to NOMMU driver

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/pci-nommu.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/pci-nommu.c b/arch/x86/kernel/pci-nommu.c
index b8ce83c..73853d3 100644
--- a/arch/x86/kernel/pci-nommu.c
+++ b/arch/x86/kernel/pci-nommu.c
@@ -126,8 +126,15 @@ again:
 	return NULL;
 }
 
+static void nommu_free_coherent(struct device *dev, size_t size, void *vaddr,
+				dma_addr_t dma_addr)
+{
+	free_pages((unsigned long)vaddr, get_order(size));
+}
+
 struct dma_mapping_ops nommu_dma_ops = {
 	.alloc_coherent = nommu_alloc_coherent,
+	.free_coherent = nommu_free_coherent,
 	.map_single = nommu_map_single,
 	.map_sg = nommu_map_sg,
 	.is_phys = 1,
-- 
1.5.3.7

[PATCH 6/8] x86: cleanup dma_*_coherent functions

[Joerg Roedel]

All dma_ops implementations support the alloc_coherent and free_coherent
callbacks now. This allows a big simplification of the dma_alloc_coherent
function which is done with this patch. The dma_free_coherent functions is also
cleaned up and calls now the free_coherent callback of the dma_ops
implementation.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/pci-dma.c |  121 +++++----------------------------------------
 1 files changed, 12 insertions(+), 109 deletions(-)

diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index f704cb5..514f3b8 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -241,33 +241,15 @@ int dma_supported(struct device *dev, u64 mask)
 }
 EXPORT_SYMBOL(dma_supported);
 
-/* Allocate DMA memory on node near device */
-static noinline struct page *
-dma_alloc_pages(struct device *dev, gfp_t gfp, unsigned order)
-{
-	int node;
-
-	node = dev_to_node(dev);
-
-	return alloc_pages_node(node, gfp, order);
-}
-
 /*
  * Allocate memory for a coherent mapping.
  */
-void *
+	void *
 dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
 		   gfp_t gfp)
 {
 	struct dma_mapping_ops *ops = get_dma_ops(dev);
-	void *memory = NULL;
-	struct page *page;
-	unsigned long dma_mask = 0;
-	dma_addr_t bus;
-	int noretry = 0;
-
-	/* ignore region specifiers */
-	gfp &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32);
+	void *memory;
 
 	if (dma_alloc_from_coherent(dev, size, dma_handle, &memory))
 		return memory;
@@ -276,89 +258,10 @@ dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
 		dev = &fallback_dev;
 		gfp |= GFP_DMA;
 	}
-	dma_mask = dev->coherent_dma_mask;
-	if (dma_mask == 0)
-		dma_mask = (gfp & GFP_DMA) ? DMA_24BIT_MASK : DMA_32BIT_MASK;
-
-	/* Device not DMA able */
-	if (dev->dma_mask == NULL)
-		return NULL;
-
-	/* Don't invoke OOM killer or retry in lower 16MB DMA zone */
-	if (gfp & __GFP_DMA)
-		noretry = 1;
-
-#ifdef CONFIG_X86_64
-	/* Why <=? Even when the mask is smaller than 4GB it is often
-	   larger than 16MB and in this case we have a chance of
-	   finding fitting memory in the next higher zone first. If
-	   not retry with true GFP_DMA. -AK */
-	if (dma_mask <= DMA_32BIT_MASK && !(gfp & GFP_DMA)) {
-		gfp |= GFP_DMA32;
-		if (dma_mask < DMA_32BIT_MASK)
-			noretry = 1;
-	}
-#endif
 
- again:
-	page = dma_alloc_pages(dev,
-		noretry ? gfp | __GFP_NORETRY : gfp, get_order(size));
-	if (page == NULL)
-		return NULL;
-
-	{
-		int high, mmu;
-		bus = page_to_phys(page);
-		memory = page_address(page);
-		high = (bus + size) >= dma_mask;
-		mmu = high;
-		if (force_iommu && !(gfp & GFP_DMA))
-			mmu = 1;
-		else if (high) {
-			free_pages((unsigned long)memory,
-				   get_order(size));
-
-			/* Don't use the 16MB ZONE_DMA unless absolutely
-			   needed. It's better to use remapping first. */
-			if (dma_mask < DMA_32BIT_MASK && !(gfp & GFP_DMA)) {
-				gfp = (gfp & ~GFP_DMA32) | GFP_DMA;
-				goto again;
-			}
-
-			/* Let low level make its own zone decisions */
-			gfp &= ~(GFP_DMA32|GFP_DMA);
-
-			if (ops->alloc_coherent)
-				return ops->alloc_coherent(dev, size,
-							   dma_handle, gfp);
-			return NULL;
-		}
-
-		memset(memory, 0, size);
-		if (!mmu) {
-			*dma_handle = bus;
-			return memory;
-		}
-	}
-
-	if (ops->alloc_coherent) {
-		free_pages((unsigned long)memory, get_order(size));
-		gfp &= ~(GFP_DMA|GFP_DMA32);
-		return ops->alloc_coherent(dev, size, dma_handle, gfp);
-	}
-
-	if (ops->map_simple) {
-		*dma_handle = ops->map_simple(dev, virt_to_phys(memory),
-					      size,
-					      PCI_DMA_BIDIRECTIONAL);
-		if (*dma_handle != bad_dma_address)
-			return memory;
-	}
-
-	if (panic_on_overflow)
-		panic("dma_alloc_coherent: IOMMU overflow by %lu bytes\n",
-		      (unsigned long)size);
-	free_pages((unsigned long)memory, get_order(size));
+	if (ops->alloc_coherent)
+		return ops->alloc_coherent(dev, size,
+				dma_handle, gfp);
 	return NULL;
 }
 EXPORT_SYMBOL(dma_alloc_coherent);
@@ -368,17 +271,17 @@ EXPORT_SYMBOL(dma_alloc_coherent);
  * The caller must ensure that the device has finished accessing the mapping.
  */
 void dma_free_coherent(struct device *dev, size_t size,
-			 void *vaddr, dma_addr_t bus)
+		       void *vaddr, dma_addr_t bus)
 {
 	struct dma_mapping_ops *ops = get_dma_ops(dev);
 
-	int order = get_order(size);
-	WARN_ON(irqs_disabled());	/* for portability */
-	if (dma_release_from_coherent(dev, order, vaddr))
+	WARN_ON(irqs_disabled());       /* for portability */
+
+	if (dma_release_from_coherent(dev, get_order(size), vaddr))
 		return;
-	if (ops->unmap_single)
-		ops->unmap_single(dev, bus, size, 0);
-	free_pages((unsigned long)vaddr, order);
+
+	if (ops->free_coherent)
+		ops->free_coherent(dev, size, vaddr, bus);
 }
 EXPORT_SYMBOL(dma_free_coherent);
 
-- 
1.5.3.7

[PATCH 7/8] x86: move dma_*_coherent functions to include file

[Joerg Roedel]

All the x86 DMA-API functions are defined in asm/dma-mapping.h. This patch
moves the dma_*_coherent functions also to this header file because they are
now small enough to do so.
This is done as a separate patch because it also includes some renaming and
restructuring of the dma-mapping.h file.

Signed-off-by: Joerg Roedel <joerg.roede@amd.com>
---
 arch/x86/kernel/pci-dma.c     |   49 ++--------------------------------------
 arch/x86/kernel/pci-gart_64.c |    4 +-
 include/asm-x86/dma-mapping.h |   47 +++++++++++++++++++++++++++++++--------
 3 files changed, 42 insertions(+), 58 deletions(-)

diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 514f3b8..23882c4 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -41,11 +41,12 @@ EXPORT_SYMBOL(bad_dma_address);
 /* Dummy device used for NULL arguments (normally ISA). Better would
    be probably a smaller DMA mask, but this is bug-to-bug compatible
    to older i386. */
-struct device fallback_dev = {
+struct device x86_dma_fallback_dev = {
 	.bus_id = "fallback device",
 	.coherent_dma_mask = DMA_32BIT_MASK,
-	.dma_mask = &fallback_dev.coherent_dma_mask,
+	.dma_mask = &x86_dma_fallback_dev.coherent_dma_mask,
 };
+EXPORT_SYMBOL(x86_dma_fallback_dev);
 
 int dma_set_mask(struct device *dev, u64 mask)
 {
@@ -241,50 +242,6 @@ int dma_supported(struct device *dev, u64 mask)
 }
 EXPORT_SYMBOL(dma_supported);
 
-/*
- * Allocate memory for a coherent mapping.
- */
-	void *
-dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
-		   gfp_t gfp)
-{
-	struct dma_mapping_ops *ops = get_dma_ops(dev);
-	void *memory;
-
-	if (dma_alloc_from_coherent(dev, size, dma_handle, &memory))
-		return memory;
-
-	if (!dev) {
-		dev = &fallback_dev;
-		gfp |= GFP_DMA;
-	}
-
-	if (ops->alloc_coherent)
-		return ops->alloc_coherent(dev, size,
-				dma_handle, gfp);
-	return NULL;
-}
-EXPORT_SYMBOL(dma_alloc_coherent);
-
-/*
- * Unmap coherent memory.
- * The caller must ensure that the device has finished accessing the mapping.
- */
-void dma_free_coherent(struct device *dev, size_t size,
-		       void *vaddr, dma_addr_t bus)
-{
-	struct dma_mapping_ops *ops = get_dma_ops(dev);
-
-	WARN_ON(irqs_disabled());       /* for portability */
-
-	if (dma_release_from_coherent(dev, get_order(size), vaddr))
-		return;
-
-	if (ops->free_coherent)
-		ops->free_coherent(dev, size, vaddr, bus);
-}
-EXPORT_SYMBOL(dma_free_coherent);
-
 static int __init pci_iommu_init(void)
 {
 	calgary_iommu_init();
diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index 794d026..92f5c67 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -282,7 +282,7 @@ gart_map_single(struct device *dev, phys_addr_t paddr, size_t size, int dir)
 	unsigned long bus;
 
 	if (!dev)
-		dev = &fallback_dev;
+		dev = &x86_dma_fallback_dev;
 
 	if (!need_iommu(dev, paddr, size))
 		return paddr;
@@ -434,7 +434,7 @@ gart_map_sg(struct device *dev, struct scatterlist *sg, int nents, int dir)
 		return 0;
 
 	if (!dev)
-		dev = &fallback_dev;
+		dev = &x86_dma_fallback_dev;
 
 	out = 0;
 	start = 0;
diff --git a/include/asm-x86/dma-mapping.h b/include/asm-x86/dma-mapping.h
index 40688b2..b732c36 100644
--- a/include/asm-x86/dma-mapping.h
+++ b/include/asm-x86/dma-mapping.h
@@ -10,10 +10,11 @@
 #include <linux/scatterlist.h>
 #include <asm/io.h>
 #include <asm/swiotlb.h>
+#include <asm-generic/dma-coherent.h>
 
 extern dma_addr_t bad_dma_address;
 extern int iommu_merge;
-extern struct device fallback_dev;
+extern struct device x86_dma_fallback_dev;
 extern int panic_on_overflow;
 extern int force_iommu;
 
@@ -88,13 +89,7 @@ static inline int dma_mapping_error(struct device *dev, dma_addr_t dma_addr)
 
 #define dma_alloc_noncoherent(d, s, h, f) dma_alloc_coherent(d, s, h, f)
 #define dma_free_noncoherent(d, s, v, h) dma_free_coherent(d, s, v, h)
-
-void *dma_alloc_coherent(struct device *dev, size_t size,
-			   dma_addr_t *dma_handle, gfp_t flag);
-
-void dma_free_coherent(struct device *dev, size_t size,
-			 void *vaddr, dma_addr_t dma_handle);
-
+#define dma_is_consistent(d, h)	(1)
 
 extern int dma_supported(struct device *hwdev, u64 mask);
 extern int dma_set_mask(struct device *dev, u64 mask);
@@ -249,7 +244,39 @@ static inline int dma_get_cache_alignment(void)
 	return boot_cpu_data.x86_clflush_size;
 }
 
-#define dma_is_consistent(d, h)	(1)
+static inline void *
+dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
+		gfp_t gfp)
+{
+	struct dma_mapping_ops *ops = get_dma_ops(dev);
+	void *memory;
+
+	if (dma_alloc_from_coherent(dev, size, dma_handle, &memory))
+		return memory;
+
+	if (!dev) {
+		dev = &x86_dma_fallback_dev;
+		gfp |= GFP_DMA;
+	}
+
+	if (ops->alloc_coherent)
+		return ops->alloc_coherent(dev, size,
+				dma_handle, gfp);
+	return NULL;
+}
+
+static inline void dma_free_coherent(struct device *dev, size_t size,
+				     void *vaddr, dma_addr_t bus)
+{
+	struct dma_mapping_ops *ops = get_dma_ops(dev);
+
+	WARN_ON(irqs_disabled());       /* for portability */
+
+	if (dma_release_from_coherent(dev, get_order(size), vaddr))
+		return;
+
+	if (ops->free_coherent)
+		ops->free_coherent(dev, size, vaddr, bus);
+}
 
-#include <asm-generic/dma-coherent.h>
 #endif /* ASM_X86__DMA_MAPPING_H */
-- 
1.5.3.7

[PATCH 8/8] x86, AMD IOMMU: remove obsolete FIXME comment

[Joerg Roedel]

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 69b4d06..01c68c3 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -1038,8 +1038,6 @@ out:
 
 /*
  * The exported free_coherent function for dma_ops.
- * FIXME: fix the generic x86 DMA layer so that it actually calls that
- *        function.
  */
 static void free_coherent(struct device *dev, size_t size,
 			  void *virt_addr, dma_addr_t dma_addr)
-- 
1.5.3.7

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[Ingo Molnar]

* Joerg Roedel <joerg.roedel@amd.com> wrote:

> Hi,
> 
> this patchset reworks the dma_*_coherent functions in the DMA layer 
> for the x86 architecture. The patch series extends the existing DMA 
> backends with missing *coherent callbacks and simplifies the generic 
> function to basically only call the registered backend. This allows 
> future optimizations in hardware specific IOMMU implementations. The 
> code ist tested on AMD64 with AMD IOMMU, GART, SWIOTLB and NOMMU as 
> well as on my old 486 box. Muli tested the Calgary specific patch.
> 
> Joerg
> 
> Changes since v1:
> 
> - fixed wrong logic in the pci-nommu alloc_coherent code
> - moved dma_*_coherent to include/asm-x86/dma-mapping.h
> 
> git diff --stat tip/master.. :
> 
>  arch/x86/kernel/amd_iommu.c      |    2 -
>  arch/x86/kernel/pci-calgary_64.c |   14 ++++
>  arch/x86/kernel/pci-dma.c        |  146 +-------------------------------------
>  arch/x86/kernel/pci-gart_64.c    |   35 +++++++++-
>  arch/x86/kernel/pci-nommu.c      |   62 ++++++++++++++++
>  include/asm-x86/dma-mapping.h    |   47 ++++++++++---
>  6 files changed, 149 insertions(+), 157 deletions(-)

applied to tip/x86/iommu - thanks Joerg!

Jesse, Fujita-san, do these changes look fine to you?

	Ingo

[PATCH] dma-coherent: export dma_[alloc|release]_from_coherent methods

[Ingo Molnar]

>From ed0a6445788eed1fa836442730d8fe8ab1086629 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Wed, 20 Aug 2008 12:16:09 +0200
Subject: [PATCH] dma-coherent: export dma_[alloc|release]_from_coherent methods

fixes modular builds:

  ERROR: "dma_alloc_from_coherent" [sound/core/snd-page-alloc.ko] undefined!
  ERROR: "dma_release_from_coherent" [sound/core/snd-page-alloc.ko] undefined!

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/dma-coherent.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kernel/dma-coherent.c b/kernel/dma-coherent.c
index c1d4d5b..f013a0c 100644
--- a/kernel/dma-coherent.c
+++ b/kernel/dma-coherent.c
@@ -124,6 +124,7 @@ int dma_alloc_from_coherent(struct device *dev, ssize_t size,
 	}
 	return (mem != NULL);
 }
+EXPORT_SYMBOL(dma_alloc_from_coherent);
 
 /**
  * dma_release_from_coherent() - try to free the memory allocated from per-device coherent memory pool
@@ -151,3 +152,4 @@ int dma_release_from_coherent(struct device *dev, int order, void *vaddr)
 	}
 	return 0;
 }
+EXPORT_SYMBOL(dma_release_from_coherent);

Re: [PATCH] dma-coherent: export dma_[alloc|release]_from_coherent methods

[Joerg Roedel]

On Wed, Aug 20, 2008 at 12:18:11PM +0200, Ingo Molnar wrote:
> 
> From ed0a6445788eed1fa836442730d8fe8ab1086629 Mon Sep 17 00:00:00 2001
> From: Ingo Molnar <mingo@elte.hu>
> Date: Wed, 20 Aug 2008 12:16:09 +0200
> Subject: [PATCH] dma-coherent: export dma_[alloc|release]_from_coherent methods
> 
> fixes modular builds:
> 
>   ERROR: "dma_alloc_from_coherent" [sound/core/snd-page-alloc.ko] undefined!
>   ERROR: "dma_release_from_coherent" [sound/core/snd-page-alloc.ko] undefined!
> 
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  kernel/dma-coherent.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/dma-coherent.c b/kernel/dma-coherent.c
> index c1d4d5b..f013a0c 100644
> --- a/kernel/dma-coherent.c
> +++ b/kernel/dma-coherent.c
> @@ -124,6 +124,7 @@ int dma_alloc_from_coherent(struct device *dev, ssize_t size,
>  	}
>  	return (mem != NULL);
>  }
> +EXPORT_SYMBOL(dma_alloc_from_coherent);
>  
>  /**
>   * dma_release_from_coherent() - try to free the memory allocated from per-device coherent memory pool
> @@ -151,3 +152,4 @@ int dma_release_from_coherent(struct device *dev, int order, void *vaddr)
>  	}
>  	return 0;
>  }
> +EXPORT_SYMBOL(dma_release_from_coherent);

Hmm, weird. I should have found this with the allmodconfig build test.
Anyway, thanks for the fix.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[Jesse Barnes]

On Wednesday, August 20, 2008 2:46 am Ingo Molnar wrote:
> * Joerg Roedel <joerg.roedel@amd.com> wrote:
> > Hi,
> >
> > this patchset reworks the dma_*_coherent functions in the DMA layer
> > for the x86 architecture. The patch series extends the existing DMA
> > backends with missing *coherent callbacks and simplifies the generic
> > function to basically only call the registered backend. This allows
> > future optimizations in hardware specific IOMMU implementations. The
> > code ist tested on AMD64 with AMD IOMMU, GART, SWIOTLB and NOMMU as
> > well as on my old 486 box. Muli tested the Calgary specific patch.
> >
> > Joerg
> >
> > Changes since v1:
> >
> > - fixed wrong logic in the pci-nommu alloc_coherent code
> > - moved dma_*_coherent to include/asm-x86/dma-mapping.h
> >
> > git diff --stat tip/master.. :
> >
> >  arch/x86/kernel/amd_iommu.c      |    2 -
> >  arch/x86/kernel/pci-calgary_64.c |   14 ++++
> >  arch/x86/kernel/pci-dma.c        |  146
> > +------------------------------------- arch/x86/kernel/pci-gart_64.c    |
> >   35 +++++++++-
> >  arch/x86/kernel/pci-nommu.c      |   62 ++++++++++++++++
> >  include/asm-x86/dma-mapping.h    |   47 ++++++++++---
> >  6 files changed, 149 insertions(+), 157 deletions(-)
>
> applied to tip/x86/iommu - thanks Joerg!
>
> Jesse, Fujita-san, do these changes look fine to you?

Yeah, I'll let you push this time. :)

Signed-off-by:  Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[Ingo Molnar]

* Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Wednesday, August 20, 2008 2:46 am Ingo Molnar wrote:
> > * Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > Hi,
> > >
> > > this patchset reworks the dma_*_coherent functions in the DMA layer
> > > for the x86 architecture. The patch series extends the existing DMA
> > > backends with missing *coherent callbacks and simplifies the generic
> > > function to basically only call the registered backend. This allows
> > > future optimizations in hardware specific IOMMU implementations. The
> > > code ist tested on AMD64 with AMD IOMMU, GART, SWIOTLB and NOMMU as
> > > well as on my old 486 box. Muli tested the Calgary specific patch.
> > >
> > > Joerg
> > >
> > > Changes since v1:
> > >
> > > - fixed wrong logic in the pci-nommu alloc_coherent code
> > > - moved dma_*_coherent to include/asm-x86/dma-mapping.h
> > >
> > > git diff --stat tip/master.. :
> > >
> > >  arch/x86/kernel/amd_iommu.c      |    2 -
> > >  arch/x86/kernel/pci-calgary_64.c |   14 ++++
> > >  arch/x86/kernel/pci-dma.c        |  146
> > > +------------------------------------- arch/x86/kernel/pci-gart_64.c    |
> > >   35 +++++++++-
> > >  arch/x86/kernel/pci-nommu.c      |   62 ++++++++++++++++
> > >  include/asm-x86/dma-mapping.h    |   47 ++++++++++---
> > >  6 files changed, 149 insertions(+), 157 deletions(-)
> >
> > applied to tip/x86/iommu - thanks Joerg!
> >
> > Jesse, Fujita-san, do these changes look fine to you?
> 
> Yeah, I'll let you push this time. :)
> 
> Signed-off-by:  Jesse Barnes <jbarnes@virtuousgeek.org>

with a v2.6.28 ETA, right?

	Ingo

Re: [PATCH 1/8] x86: add alloc_coherent dma_ops callback to GART driver

[FUJITA Tomonori]

On Tue, 19 Aug 2008 16:32:39 +0200
Joerg Roedel <joerg.roedel@amd.com> wrote:

> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  arch/x86/kernel/pci-gart_64.c |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
> index 4d8efb0..44a75a6 100644
> --- a/arch/x86/kernel/pci-gart_64.c
> +++ b/arch/x86/kernel/pci-gart_64.c
> @@ -506,6 +506,26 @@ error:
>  	return 0;
>  }
>  
> +/* allocate and map a coherent mapping */
> +static void *
> +gart_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_addr,
> +		    gfp_t flag)
> +{
> +	void *vaddr;
> +
> +	vaddr = (void *)__get_free_pages(flag, get_order(size));
> +	if (!vaddr)
> +		return NULL;
> +
> +	*dma_addr = gart_map_single(dev, __pa(vaddr), size, DMA_BIDIRECTIONAL);
> +	if (*dma_addr != bad_dma_address)
> +		return vaddr;

I'm not sure a rule is documented or not, but I think that IOMMUs
return zeroed memory wrt dma_alloc_coherent. The current pci-dma.c
does, so I think that it would be better to keep the current behavior.

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[FUJITA Tomonori]

On Wed, 20 Aug 2008 11:46:12 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > Hi,
> > 
> > this patchset reworks the dma_*_coherent functions in the DMA layer 
> > for the x86 architecture. The patch series extends the existing DMA 
> > backends with missing *coherent callbacks and simplifies the generic 
> > function to basically only call the registered backend. This allows 
> > future optimizations in hardware specific IOMMU implementations. The 
> > code ist tested on AMD64 with AMD IOMMU, GART, SWIOTLB and NOMMU as 
> > well as on my old 486 box. Muli tested the Calgary specific patch.
> > 
> > Joerg
> > 
> > Changes since v1:
> > 
> > - fixed wrong logic in the pci-nommu alloc_coherent code
> > - moved dma_*_coherent to include/asm-x86/dma-mapping.h
> > 
> > git diff --stat tip/master.. :
> > 
> >  arch/x86/kernel/amd_iommu.c      |    2 -
> >  arch/x86/kernel/pci-calgary_64.c |   14 ++++
> >  arch/x86/kernel/pci-dma.c        |  146 +-------------------------------------
> >  arch/x86/kernel/pci-gart_64.c    |   35 +++++++++-
> >  arch/x86/kernel/pci-nommu.c      |   62 ++++++++++++++++
> >  include/asm-x86/dma-mapping.h    |   47 ++++++++++---
> >  6 files changed, 149 insertions(+), 157 deletions(-)
> 
> applied to tip/x86/iommu - thanks Joerg!
> 
> Jesse, Fujita-san, do these changes look fine to you?

As I wrote in another mail, GART should return zeroed memory (keeping
the current behavior is better). Except for it, the patchset looks
fine.

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[FUJITA Tomonori]

On Thu, 21 Aug 2008 14:00:11 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> 
> > On Wednesday, August 20, 2008 2:46 am Ingo Molnar wrote:
> > > * Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > > Hi,
> > > >
> > > > this patchset reworks the dma_*_coherent functions in the DMA layer
> > > > for the x86 architecture. The patch series extends the existing DMA
> > > > backends with missing *coherent callbacks and simplifies the generic
> > > > function to basically only call the registered backend. This allows
> > > > future optimizations in hardware specific IOMMU implementations. The
> > > > code ist tested on AMD64 with AMD IOMMU, GART, SWIOTLB and NOMMU as
> > > > well as on my old 486 box. Muli tested the Calgary specific patch.
> > > >
> > > > Joerg
> > > >
> > > > Changes since v1:
> > > >
> > > > - fixed wrong logic in the pci-nommu alloc_coherent code
> > > > - moved dma_*_coherent to include/asm-x86/dma-mapping.h
> > > >
> > > > git diff --stat tip/master.. :
> > > >
> > > >  arch/x86/kernel/amd_iommu.c      |    2 -
> > > >  arch/x86/kernel/pci-calgary_64.c |   14 ++++
> > > >  arch/x86/kernel/pci-dma.c        |  146
> > > > +------------------------------------- arch/x86/kernel/pci-gart_64.c    |
> > > >   35 +++++++++-
> > > >  arch/x86/kernel/pci-nommu.c      |   62 ++++++++++++++++
> > > >  include/asm-x86/dma-mapping.h    |   47 ++++++++++---
> > > >  6 files changed, 149 insertions(+), 157 deletions(-)
> > >
> > > applied to tip/x86/iommu - thanks Joerg!
> > >
> > > Jesse, Fujita-san, do these changes look fine to you?
> > 
> > Yeah, I'll let you push this time. :)
> > 
> > Signed-off-by:  Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> with a v2.6.28 ETA, right?

Surely, this patchset should be for 2.6.28.

Can you send this via the x86 tree instead of pci?

- IOMMU code is arch stuff rather than pci.
- We can avoid a mistake such as the previous one.

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[Jesse Barnes]

On Thursday, August 21, 2008 7:16 am FUJITA Tomonori wrote:
> > > > Jesse, Fujita-san, do these changes look fine to you?
> > >
> > > Yeah, I'll let you push this time. :)
> > >
> > > Signed-off-by:  Jesse Barnes <jbarnes@virtuousgeek.org>
> >
> > with a v2.6.28 ETA, right?
>
> Surely, this patchset should be for 2.6.28.
>
> Can you send this via the x86 tree instead of pci?
>
> - IOMMU code is arch stuff rather than pci.
> - We can avoid a mistake such as the previous one.

Yeah I was thinking 2.6.28 too, via the x86 tree.  The problem last time was 
that I sent it too soon (I misunderstood Ingo when he said it was ready) so 
we broke the build on some non-x86 platforms.  I don't think that'll be an 
issue this time, but we may as well push through x86 anyway; I agree that 
IOMMU is really a platform feature more than a PCI one.

Jesse

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[FUJITA Tomonori]

On Thu, 21 Aug 2008 23:16:50 +0900
FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> On Wed, 20 Aug 2008 11:46:12 +0200
> Ingo Molnar <mingo@elte.hu> wrote:
> 
> > 
> > * Joerg Roedel <joerg.roedel@amd.com> wrote:
> > 
> > > Hi,
> > > 
> > > this patchset reworks the dma_*_coherent functions in the DMA layer 
> > > for the x86 architecture. The patch series extends the existing DMA 
> > > backends with missing *coherent callbacks and simplifies the generic 
> > > function to basically only call the registered backend. This allows 
> > > future optimizations in hardware specific IOMMU implementations. The 
> > > code ist tested on AMD64 with AMD IOMMU, GART, SWIOTLB and NOMMU as 
> > > well as on my old 486 box. Muli tested the Calgary specific patch.
> > > 
> > > Joerg
> > > 
> > > Changes since v1:
> > > 
> > > - fixed wrong logic in the pci-nommu alloc_coherent code
> > > - moved dma_*_coherent to include/asm-x86/dma-mapping.h
> > > 
> > > git diff --stat tip/master.. :
> > > 
> > >  arch/x86/kernel/amd_iommu.c      |    2 -
> > >  arch/x86/kernel/pci-calgary_64.c |   14 ++++
> > >  arch/x86/kernel/pci-dma.c        |  146 +-------------------------------------
> > >  arch/x86/kernel/pci-gart_64.c    |   35 +++++++++-
> > >  arch/x86/kernel/pci-nommu.c      |   62 ++++++++++++++++
> > >  include/asm-x86/dma-mapping.h    |   47 ++++++++++---
> > >  6 files changed, 149 insertions(+), 157 deletions(-)
> > 
> > applied to tip/x86/iommu - thanks Joerg!
> > 
> > Jesse, Fujita-san, do these changes look fine to you?
> 
> As I wrote in another mail, GART should return zeroed memory (keeping
> the current behavior is better). Except for it, the patchset looks
> fine.

I think that you can apply the following patch after Joerg's patchset.

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH] x86: remove map_simple hook in struct dma_mapping_ops

pci-dma.c doesn't use map_simple hook any more so we can remove it
from struct dma_mapping_ops now.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 arch/x86/kernel/pci-gart_64.c |    1 -
 include/asm-x86/dma-mapping.h |    3 ---
 2 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index 10afe97..b3bbe2b 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -720,7 +720,6 @@ extern int agp_amd64_init(void);
 
 static struct dma_mapping_ops gart_dma_ops = {
 	.map_single			= gart_map_single,
-	.map_simple			= gart_map_simple,
 	.unmap_single			= gart_unmap_single,
 	.sync_single_for_cpu		= NULL,
 	.sync_single_for_device		= NULL,
diff --git a/include/asm-x86/dma-mapping.h b/include/asm-x86/dma-mapping.h
index 8e16095..3a9a6f5 100644
--- a/include/asm-x86/dma-mapping.h
+++ b/include/asm-x86/dma-mapping.h
@@ -26,9 +26,6 @@ struct dma_mapping_ops {
 				void *vaddr, dma_addr_t dma_handle);
 	dma_addr_t      (*map_single)(struct device *hwdev, phys_addr_t ptr,
 				size_t size, int direction);
-	/* like map_single, but doesn't check the device mask */
-	dma_addr_t      (*map_simple)(struct device *hwdev, phys_addr_t ptr,
-				size_t size, int direction);
 	void            (*unmap_single)(struct device *dev, dma_addr_t addr,
 				size_t size, int direction);
 	void            (*sync_single_for_cpu)(struct device *hwdev,
-- 
1.5.5.GIT

Re: [PATCH 1/8] x86: add alloc_coherent dma_ops callback to GART driver

[Joerg Roedel]

On Thu, Aug 21, 2008 at 11:16:49PM +0900, FUJITA Tomonori wrote:
> On Tue, 19 Aug 2008 16:32:39 +0200
> Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > ---
> >  arch/x86/kernel/pci-gart_64.c |   21 +++++++++++++++++++++
> >  1 files changed, 21 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
> > index 4d8efb0..44a75a6 100644
> > --- a/arch/x86/kernel/pci-gart_64.c
> > +++ b/arch/x86/kernel/pci-gart_64.c
> > @@ -506,6 +506,26 @@ error:
> >  	return 0;
> >  }
> >  
> > +/* allocate and map a coherent mapping */
> > +static void *
> > +gart_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_addr,
> > +		    gfp_t flag)
> > +{
> > +	void *vaddr;
> > +
> > +	vaddr = (void *)__get_free_pages(flag, get_order(size));
> > +	if (!vaddr)
> > +		return NULL;
> > +
> > +	*dma_addr = gart_map_single(dev, __pa(vaddr), size, DMA_BIDIRECTIONAL);
> > +	if (*dma_addr != bad_dma_address)
> > +		return vaddr;
> 
> I'm not sure a rule is documented or not, but I think that IOMMUs
> return zeroed memory wrt dma_alloc_coherent. The current pci-dma.c
> does, so I think that it would be better to keep the current behavior.

Ok, true. I will send and update patch to add this. Thanks.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[Joerg Roedel]

On Thu, Aug 21, 2008 at 11:16:50PM +0900, FUJITA Tomonori wrote:
> On Thu, 21 Aug 2008 14:00:11 +0200
> Ingo Molnar <mingo@elte.hu> wrote:
> 
> > 
> > * Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > 
> > > On Wednesday, August 20, 2008 2:46 am Ingo Molnar wrote:
> > > > * Joerg Roedel <joerg.roedel@amd.com> wrote:
> > > > > Hi,
> > > > >
> > > > > this patchset reworks the dma_*_coherent functions in the DMA layer
> > > > > for the x86 architecture. The patch series extends the existing DMA
> > > > > backends with missing *coherent callbacks and simplifies the generic
> > > > > function to basically only call the registered backend. This allows
> > > > > future optimizations in hardware specific IOMMU implementations. The
> > > > > code ist tested on AMD64 with AMD IOMMU, GART, SWIOTLB and NOMMU as
> > > > > well as on my old 486 box. Muli tested the Calgary specific patch.
> > > > >
> > > > > Joerg
> > > > >
> > > > > Changes since v1:
> > > > >
> > > > > - fixed wrong logic in the pci-nommu alloc_coherent code
> > > > > - moved dma_*_coherent to include/asm-x86/dma-mapping.h
> > > > >
> > > > > git diff --stat tip/master.. :
> > > > >
> > > > >  arch/x86/kernel/amd_iommu.c      |    2 -
> > > > >  arch/x86/kernel/pci-calgary_64.c |   14 ++++
> > > > >  arch/x86/kernel/pci-dma.c        |  146
> > > > > +------------------------------------- arch/x86/kernel/pci-gart_64.c    |
> > > > >   35 +++++++++-
> > > > >  arch/x86/kernel/pci-nommu.c      |   62 ++++++++++++++++
> > > > >  include/asm-x86/dma-mapping.h    |   47 ++++++++++---
> > > > >  6 files changed, 149 insertions(+), 157 deletions(-)
> > > >
> > > > applied to tip/x86/iommu - thanks Joerg!
> > > >
> > > > Jesse, Fujita-san, do these changes look fine to you?
> > > 
> > > Yeah, I'll let you push this time. :)
> > > 
> > > Signed-off-by:  Jesse Barnes <jbarnes@virtuousgeek.org>
> > 
> > with a v2.6.28 ETA, right?
> 
> Surely, this patchset should be for 2.6.28.
> 
> Can you send this via the x86 tree instead of pci?
> 
> - IOMMU code is arch stuff rather than pci.
> - We can avoid a mistake such as the previous one.

Yes, this is definitly 2.6.28 stuff.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

[PATCH] x86: make gart_alloc_coherent return zeroed memory

[Joerg Roedel]

The dma_alloc_coherent function should return memory set to zero. This patch
adds this to the GART implementation.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/pci-gart_64.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index 92f5c67..3b5e9e8 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -513,7 +513,7 @@ gart_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_addr,
 {
 	void *vaddr;
 
-	vaddr = (void *)__get_free_pages(flag, get_order(size));
+	vaddr = (void *)__get_free_pages(flag | __GFP_ZERO, get_order(size));
 	if (!vaddr)
 		return NULL;
 
-- 
1.5.3.7

Re: [PATCH] x86: make gart_alloc_coherent return zeroed memory

[FUJITA Tomonori]

On Thu, 21 Aug 2008 19:28:27 +0200
Joerg Roedel <joerg.roedel@amd.com> wrote:

> The dma_alloc_coherent function should return memory set to zero. This patch
> adds this to the GART implementation.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  arch/x86/kernel/pci-gart_64.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

This should be foiled to the first patch in the patchset. No good
reason to push a patch that might cause a problem.

Re: [PATCH] x86: make gart_alloc_coherent return zeroed memory

[Joerg Roedel]

On Fri, Aug 22, 2008 at 08:19:35AM +0900, FUJITA Tomonori wrote:
> On Thu, 21 Aug 2008 19:28:27 +0200
> Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > The dma_alloc_coherent function should return memory set to zero. This patch
> > adds this to the GART implementation.
> > 
> > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > ---
> >  arch/x86/kernel/pci-gart_64.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> This should be foiled to the first patch in the patchset. No good
> reason to push a patch that might cause a problem.

The reason is, that in my experience, Ingo prefers update patches
instead replacements once he has merged something.

Joerg

Re: [PATCH] x86: make gart_alloc_coherent return zeroed memory

[Ingo Molnar]

* FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> On Thu, 21 Aug 2008 19:28:27 +0200
> Joerg Roedel <joerg.roedel@amd.com> wrote:
> 
> > The dma_alloc_coherent function should return memory set to zero. This patch
> > adds this to the GART implementation.
> > 
> > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > ---
> >  arch/x86/kernel/pci-gart_64.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> This should be foiled to the first patch in the patchset. No good
> reason to push a patch that might cause a problem.

agreed - i folded the fix back and respun this portion of tip/x86/iommu.

	Ingo

Re: [PATCH] x86: make gart_alloc_coherent return zeroed memory

[Ingo Molnar]

* Joerg Roedel <joro@8bytes.org> wrote:

> > This should be foiled to the first patch in the patchset. No good 
> > reason to push a patch that might cause a problem.
> 
> The reason is, that in my experience, Ingo prefers update patches 
> instead replacements once he has merged something.

yeah, that's very true - but i can also fold back things if it's done in 
a reasonably short amount of time, when there's no other commits mixed 
in inbetween. (as in this case)

	Ingo

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[Ingo Molnar]

* FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> > As I wrote in another mail, GART should return zeroed memory 
> > (keeping the current behavior is better). Except for it, the 
> > patchset looks fine.
> 
> I think that you can apply the following patch after Joerg's patchset.
> 
> =
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Subject: [PATCH] x86: remove map_simple hook in struct dma_mapping_ops
> 
> pci-dma.c doesn't use map_simple hook any more so we can remove it 
> from struct dma_mapping_ops now.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

applied to tip/x86/iommu - thanks!

i also grepped the full tree and this is indeed the last user of this. 
Nice cleanup.

	Ingo

Re: [PATCH] x86: make gart_alloc_coherent return zeroed memory

[Joerg Roedel]

On Fri, Aug 22, 2008 at 08:41:07AM +0200, Ingo Molnar wrote:
> 
> * Joerg Roedel <joro@8bytes.org> wrote:
> 
> > > This should be foiled to the first patch in the patchset. No good 
> > > reason to push a patch that might cause a problem.
> > 
> > The reason is, that in my experience, Ingo prefers update patches 
> > instead replacements once he has merged something.
> 
> yeah, that's very true - but i can also fold back things if it's done in 
> a reasonably short amount of time, when there's no other commits mixed 
> in inbetween. (as in this case)

Ah ok, fine for me. Good to know :-)

Joerg

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[FUJITA Tomonori]

On Fri, 22 Aug 2008 08:44:14 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> 
> > > As I wrote in another mail, GART should return zeroed memory 
> > > (keeping the current behavior is better). Except for it, the 
> > > patchset looks fine.
> > 
> > I think that you can apply the following patch after Joerg's patchset.
> > 
> > =
> > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Subject: [PATCH] x86: remove map_simple hook in struct dma_mapping_ops
> > 
> > pci-dma.c doesn't use map_simple hook any more so we can remove it 
> > from struct dma_mapping_ops now.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> 
> applied to tip/x86/iommu - thanks!

Can you drop the following patch in tip/x86/gart?

commit 0bc65ffd52a78a65f582b42503f727b3f3773cc4
Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date:   Mon Aug 18 00:36:18 2008 +0900

    x86 gart: allocate size-aligned address for alloc_coherent, v2


Joerg's patchset and this patch slightly conflict with the above
patch. I'll send a new patch against tip/x86/iommu shortly.

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[Ingo Molnar]

* FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> Can you drop the following patch in tip/x86/gart?
> 
> commit 0bc65ffd52a78a65f582b42503f727b3f3773cc4
> Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Date:   Mon Aug 18 00:36:18 2008 +0900
> 
>     x86 gart: allocate size-aligned address for alloc_coherent, v2
> 
> Joerg's patchset and this patch slightly conflict with the above
> patch. I'll send a new patch against tip/x86/iommu shortly.

such conflicts are no problem usually, check out the resolution in 
tip/master - so there's no need to resend. Also, i just merged 
tip/x86/gart into tip/x86/iommu, to make the integration even more 
apparent.

	Ingo

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[Ingo Molnar]

* Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> > Surely, this patchset should be for 2.6.28.
> >
> > Can you send this via the x86 tree instead of pci?
> >
> > - IOMMU code is arch stuff rather than pci.
> > - We can avoid a mistake such as the previous one.
> 
> Yeah I was thinking 2.6.28 too, via the x86 tree.  The problem last 
> time was that I sent it too soon (I misunderstood Ingo when he said it 
> was ready) so we broke the build on some non-x86 platforms.  I don't 
> think that'll be an issue this time, but we may as well push through 
> x86 anyway; I agree that IOMMU is really a platform feature more than 
> a PCI one.

yeah, GART/IOMMU has been historically a more platform specific thing 
although there's certainly no hard boundaries. This code will show up in 
linux-next as well soon, so there's plenty of time to discover and fix 
any cross-arch breakages. (if any)

and if the PCI tree becomes very active in this area that would make the 
movement of these commits over there more practical we can do that too 
at any point of time, it's all kept separate in tip/x86/iommu, purely 
based on Linus's tree. The blessings of 160+ topic branches ;-)

	Ingo

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[FUJITA Tomonori]

On Fri, 22 Aug 2008 09:04:50 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> 
> > Can you drop the following patch in tip/x86/gart?
> > 
> > commit 0bc65ffd52a78a65f582b42503f727b3f3773cc4
> > Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Date:   Mon Aug 18 00:36:18 2008 +0900
> > 
> >     x86 gart: allocate size-aligned address for alloc_coherent, v2
> > 
> > Joerg's patchset and this patch slightly conflict with the above
> > patch. I'll send a new patch against tip/x86/iommu shortly.
> 
> such conflicts are no problem usually, check out the resolution in 
> tip/master - so there's no need to resend. Also, i just merged 
> tip/x86/gart into tip/x86/iommu, to make the integration even more 
> apparent.

Ok, here's a patch against tip/x86/iommu:

commit 0d8136ea509132c66155ca8a1e7cf60699f95c37
Merge: 766af9f... 7b22ff5...
Author: Ingo Molnar <mingo@elte.hu>
Date:   Fri Aug 22 09:03:43 2008 +0200

    Merge branch 'x86/gart' into x86/iommu

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH] gart: allocate size-aligned address for alloc_coherent

alloc_coherent dma_ops callback was added to GART, however, it doesn't
return a size aligned address wrt dma_alloc_coherent, as
DMA-mapping.txt defines. This patch fixes it.

This patch also removes unused gart_map_simple
(dma_mapping_ops->map_simple has gone).

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 arch/x86/kernel/pci-gart_64.c |   25 ++++++++++---------------
 1 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index 338c4f2..4d08649 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -261,20 +261,6 @@ static dma_addr_t dma_map_area(struct device *dev, dma_addr_t phys_mem,
 	return iommu_bus_base + iommu_page*PAGE_SIZE + (phys_mem & ~PAGE_MASK);
 }
 
-static dma_addr_t
-gart_map_simple(struct device *dev, phys_addr_t paddr, size_t size, int dir)
-{
-	dma_addr_t map;
-	unsigned long align_mask;
-
-	align_mask = (1UL << get_order(size)) - 1;
-	map = dma_map_area(dev, paddr, size, dir, align_mask);
-
-	flush_gart();
-
-	return map;
-}
-
 /* Map a single area into the IOMMU */
 static dma_addr_t
 gart_map_single(struct device *dev, phys_addr_t paddr, size_t size, int dir)
@@ -512,12 +498,21 @@ gart_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_addr,
 		    gfp_t flag)
 {
 	void *vaddr;
+	unsigned long align_mask;
 
 	vaddr = (void *)__get_free_pages(flag | __GFP_ZERO, get_order(size));
 	if (!vaddr)
 		return NULL;
 
-	*dma_addr = gart_map_single(dev, __pa(vaddr), size, DMA_BIDIRECTIONAL);
+	align_mask = (1UL << get_order(size)) - 1;
+
+	if (!dev)
+		dev = &x86_dma_fallback_dev;
+
+	*dma_addr = dma_map_area(dev, __pa(vaddr), size, DMA_BIDIRECTIONAL,
+				 align_mask);
+	flush_gart();
+
 	if (*dma_addr != bad_dma_address)
 		return vaddr;
 
-- 
1.5.5.GIT

Re: [PATCH 0/8] x86 dma_*_coherent rework patchset v2

[Ingo Molnar]

* FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> On Fri, 22 Aug 2008 09:04:50 +0200
> Ingo Molnar <mingo@elte.hu> wrote:
> 
> > 
> > * FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
> > 
> > > Can you drop the following patch in tip/x86/gart?
> > > 
> > > commit 0bc65ffd52a78a65f582b42503f727b3f3773cc4
> > > Author: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > > Date:   Mon Aug 18 00:36:18 2008 +0900
> > > 
> > >     x86 gart: allocate size-aligned address for alloc_coherent, v2
> > > 
> > > Joerg's patchset and this patch slightly conflict with the above
> > > patch. I'll send a new patch against tip/x86/iommu shortly.
> > 
> > such conflicts are no problem usually, check out the resolution in 
> > tip/master - so there's no need to resend. Also, i just merged 
> > tip/x86/gart into tip/x86/iommu, to make the integration even more 
> > apparent.
> 
> Ok, here's a patch against tip/x86/iommu:
> 
> commit 0d8136ea509132c66155ca8a1e7cf60699f95c37
> Merge: 766af9f... 7b22ff5...
> Author: Ingo Molnar <mingo@elte.hu>
> Date:   Fri Aug 22 09:03:43 2008 +0200
> 
>     Merge branch 'x86/gart' into x86/iommu
> 
> =
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Subject: [PATCH] gart: allocate size-aligned address for alloc_coherent
> 
> alloc_coherent dma_ops callback was added to GART, however, it doesn't
> return a size aligned address wrt dma_alloc_coherent, as
> DMA-mapping.txt defines. This patch fixes it.
> 
> This patch also removes unused gart_map_simple
> (dma_mapping_ops->map_simple has gone).

applied, thanks!

	Ingo