[PATCH v2 0/5] enhance DMA CMA on x86

[PATCH v2 0/5] enhance DMA CMA on x86

[Akinobu Mita]

This patch set enhances the DMA Contiguous Memory Allocator on x86.

Currently the DMA CMA is only supported with pci-nommu dma_map_ops
and furthermore it can't be enabled on x86_64.  But I would like to
allocate big contiguous memory with dma_alloc_coherent() and tell it
to the device that requires it, regardless of which dma mapping
implementation is actually used in the system.

So this makes it work with swiotlb and intel-iommu dma_map_ops, too.
And this also extends "cma=" kernel parameter to specify placement
constraint by the physical address range of memory allocations.  For
example, CMA allocates memory below 4GB by "cma=64M@0-4G", it is
required for the devices only supporting 32-bit addressing on 64-bit
systems without iommu.

* Changes from v1
- fix dma_alloc_coherent() with __GFP_ZERO
- add placement specifier for "cma=" kernel parameter

Akinobu Mita (5):
  x86: make dma_alloc_coherent() return zeroed memory if CMA is enabled
  x86: enable DMA CMA with swiotlb
  intel-iommu: integrate DMA CMA
  memblock: introduce memblock_alloc_range()
  cma: add placement specifier for "cma=" kernel parameter

 Documentation/kernel-parameters.txt |  7 +++++--
 arch/x86/Kconfig                    |  2 +-
 arch/x86/include/asm/swiotlb.h      |  7 +++++++
 arch/x86/kernel/amd_gart_64.c       |  2 +-
 arch/x86/kernel/pci-dma.c           |  3 +--
 arch/x86/kernel/pci-swiotlb.c       |  9 +++++---
 arch/x86/pci/sta2x11-fixup.c        |  6 ++----
 drivers/base/dma-contiguous.c       | 42 ++++++++++++++++++++++++++++---------
 drivers/iommu/intel-iommu.c         | 32 +++++++++++++++++++++-------
 include/linux/dma-contiguous.h      |  9 +++++---
 include/linux/memblock.h            |  2 ++
 include/linux/swiotlb.h             |  2 ++
 lib/swiotlb.c                       |  2 +-
 mm/memblock.c                       | 27 +++++++++++++++++-------
 14 files changed, 110 insertions(+), 42 deletions(-)

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Don Dutile <ddutile@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: x86@kernel.org
Cc: iommu@lists.linux-foundation.org
-- 
1.8.3.2

[PATCH v2 1/5] x86: make dma_alloc_coherent() return zeroed memory if CMA is enabled

[Akinobu Mita]

Calling dma_alloc_coherent() with __GFP_ZERO must return zeroed memory.

But when the contiguous memory allocator (CMA) is enabled on x86 and
the memory region is allocated by dma_alloc_from_contiguous(), it
doesn't return zeroed memory.  Because dma_generic_alloc_coherent()
forgot to fill the memory region with zero if it was allocated by
dma_alloc_from_contiguous()

Most implementations of dma_alloc_coherent() return zeroed memory
regardless of whether __GFP_ZERO is specified.  So this fixes it by
unconditionally zeroing the allocated memory region.

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Don Dutile <ddutile@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: x86@kernel.org
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
New patch from this version

 arch/x86/kernel/pci-dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 872079a..9644405 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -97,7 +97,6 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t size,
 
 	dma_mask = dma_alloc_coherent_mask(dev, flag);
 
-	flag |= __GFP_ZERO;
 again:
 	page = NULL;
 	if (!(flag & GFP_ATOMIC))
@@ -118,7 +117,7 @@ again:
 
 		return NULL;
 	}
-
+	memset(page_address(page), 0, size);
 	*dma_addr = addr;
 	return page_address(page);
 }
-- 
1.8.3.2

[PATCH v2 2/5] x86: enable DMA CMA with swiotlb

[Akinobu Mita]

The DMA Contiguous Memory Allocator support on x86 is disabled when
swiotlb config option is enabled.  So DMA CMA is always disabled on
x86_64 because swiotlb is always enabled.  This attempts to support
for DMA CMA with enabling swiotlb config option.

The contiguous memory allocator on x86 is integrated in the function
dma_generic_alloc_coherent() which is .alloc callback in nommu_dma_ops
for dma_alloc_coherent().

x86_swiotlb_alloc_coherent() which is .alloc callback in swiotlb_dma_ops
tries to allocate with dma_generic_alloc_coherent() firstly and then
swiotlb_alloc_coherent() is called as a fallback.

The main part of supporting DMA CMA with swiotlb is that changing
x86_swiotlb_free_coherent() which is .free callback in swiotlb_dma_ops
for dma_free_coherent() so that it can distinguish memory allocated by
dma_generic_alloc_coherent() from one allocated by swiotlb_alloc_coherent()
and release it with dma_generic_free_coherent() which can handle contiguous
memory.  This change requires making is_swiotlb_buffer() global function.

This also needs to change .free callback in the dma_map_ops for amd_gart
and sta2x11, because these dma_ops are also using
dma_generic_alloc_coherent().

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Don Dutile <ddutile@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: x86@kernel.org
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
No change from the previous version

 arch/x86/Kconfig               | 2 +-
 arch/x86/include/asm/swiotlb.h | 7 +++++++
 arch/x86/kernel/amd_gart_64.c  | 2 +-
 arch/x86/kernel/pci-swiotlb.c  | 9 ++++++---
 arch/x86/pci/sta2x11-fixup.c   | 6 ++----
 include/linux/swiotlb.h        | 2 ++
 lib/swiotlb.c                  | 2 +-
 7 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0952ecd..1b6275d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -40,7 +40,7 @@ config X86
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARCH_WANT_FRAME_POINTERS
 	select HAVE_DMA_ATTRS
-	select HAVE_DMA_CONTIGUOUS if !SWIOTLB
+	select HAVE_DMA_CONTIGUOUS
 	select HAVE_KRETPROBES
 	select HAVE_OPTPROBES
 	select HAVE_KPROBES_ON_FTRACE
diff --git a/arch/x86/include/asm/swiotlb.h b/arch/x86/include/asm/swiotlb.h
index 977f176..ab05d73 100644
--- a/arch/x86/include/asm/swiotlb.h
+++ b/arch/x86/include/asm/swiotlb.h
@@ -29,4 +29,11 @@ static inline void pci_swiotlb_late_init(void)
 
 static inline void dma_mark_clean(void *addr, size_t size) {}
 
+extern void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
+					dma_addr_t *dma_handle, gfp_t flags,
+					struct dma_attrs *attrs);
+extern void x86_swiotlb_free_coherent(struct device *dev, size_t size,
+					void *vaddr, dma_addr_t dma_addr,
+					struct dma_attrs *attrs);
+
 #endif /* _ASM_X86_SWIOTLB_H */
diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
index b574b29..8e3842f 100644
--- a/arch/x86/kernel/amd_gart_64.c
+++ b/arch/x86/kernel/amd_gart_64.c
@@ -512,7 +512,7 @@ gart_free_coherent(struct device *dev, size_t size, void *vaddr,
 		   dma_addr_t dma_addr, struct dma_attrs *attrs)
 {
 	gart_unmap_page(dev, dma_addr, size, DMA_BIDIRECTIONAL, NULL);
-	free_pages((unsigned long)vaddr, get_order(size));
+	dma_generic_free_coherent(dev, size, vaddr, dma_addr, attrs);
 }
 
 static int gart_mapping_error(struct device *dev, dma_addr_t dma_addr)
diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
index 6c483ba..77dd0ad 100644
--- a/arch/x86/kernel/pci-swiotlb.c
+++ b/arch/x86/kernel/pci-swiotlb.c
@@ -14,7 +14,7 @@
 #include <asm/iommu_table.h>
 int swiotlb __read_mostly;
 
-static void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
+void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 					dma_addr_t *dma_handle, gfp_t flags,
 					struct dma_attrs *attrs)
 {
@@ -28,11 +28,14 @@ static void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 	return swiotlb_alloc_coherent(hwdev, size, dma_handle, flags);
 }
 
-static void x86_swiotlb_free_coherent(struct device *dev, size_t size,
+void x86_swiotlb_free_coherent(struct device *dev, size_t size,
 				      void *vaddr, dma_addr_t dma_addr,
 				      struct dma_attrs *attrs)
 {
-	swiotlb_free_coherent(dev, size, vaddr, dma_addr);
+	if (is_swiotlb_buffer(dma_to_phys(dev, dma_addr)))
+		swiotlb_free_coherent(dev, size, vaddr, dma_addr);
+	else
+		dma_generic_free_coherent(dev, size, vaddr, dma_addr, attrs);
 }
 
 static struct dma_map_ops swiotlb_dma_ops = {
diff --git a/arch/x86/pci/sta2x11-fixup.c b/arch/x86/pci/sta2x11-fixup.c
index 9d8a509..5ceda85 100644
--- a/arch/x86/pci/sta2x11-fixup.c
+++ b/arch/x86/pci/sta2x11-fixup.c
@@ -173,9 +173,7 @@ static void *sta2x11_swiotlb_alloc_coherent(struct device *dev,
 {
 	void *vaddr;
 
-	vaddr = dma_generic_alloc_coherent(dev, size, dma_handle, flags, attrs);
-	if (!vaddr)
-		vaddr = swiotlb_alloc_coherent(dev, size, dma_handle, flags);
+	vaddr = x86_swiotlb_alloc_coherent(dev, size, dma_handle, flags, attrs);
 	*dma_handle = p2a(*dma_handle, to_pci_dev(dev));
 	return vaddr;
 }
@@ -183,7 +181,7 @@ static void *sta2x11_swiotlb_alloc_coherent(struct device *dev,
 /* We have our own dma_ops: the same as swiotlb but from alloc (above) */
 static struct dma_map_ops sta2x11_dma_ops = {
 	.alloc = sta2x11_swiotlb_alloc_coherent,
-	.free = swiotlb_free_coherent,
+	.free = x86_swiotlb_free_coherent,
 	.map_page = swiotlb_map_page,
 	.unmap_page = swiotlb_unmap_page,
 	.map_sg = swiotlb_map_sg_attrs,
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index a5ffd32..e7a018e 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -116,4 +116,6 @@ static inline void swiotlb_free(void) { }
 #endif
 
 extern void swiotlb_print_info(void);
+extern int is_swiotlb_buffer(phys_addr_t paddr);
+
 #endif /* __LINUX_SWIOTLB_H */
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index fe978e0..6e4a798 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -369,7 +369,7 @@ void __init swiotlb_free(void)
 	io_tlb_nslabs = 0;
 }
 
-static int is_swiotlb_buffer(phys_addr_t paddr)
+int is_swiotlb_buffer(phys_addr_t paddr)
 {
 	return paddr >= io_tlb_start && paddr < io_tlb_end;
 }
-- 
1.8.3.2

[PATCH v2 3/5] intel-iommu: integrate DMA CMA

[Akinobu Mita]

This adds support for the DMA Contiguous Memory Allocator for intel-iommu.
This change enables dma_alloc_coherent() to allocate big contiguous
memory.

It is achieved in the same way as nommu_dma_ops currently does, i.e.
trying to allocate memory by dma_alloc_from_contiguous() and alloc_pages()
is used as a fallback.

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Don Dutile <ddutile@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: x86@kernel.org
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
No change from the previous version

 drivers/iommu/intel-iommu.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index fd426ca..172c2b0 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3004,7 +3004,7 @@ static void *intel_alloc_coherent(struct device *hwdev, size_t size,
 				  dma_addr_t *dma_handle, gfp_t flags,
 				  struct dma_attrs *attrs)
 {
-	void *vaddr;
+	struct page *page = NULL;
 	int order;
 
 	size = PAGE_ALIGN(size);
@@ -3019,17 +3019,31 @@ static void *intel_alloc_coherent(struct device *hwdev, size_t size,
 			flags |= GFP_DMA32;
 	}
 
-	vaddr = (void *)__get_free_pages(flags, order);
-	if (!vaddr)
+	if (!(flags & GFP_ATOMIC)) {
+		unsigned int count = size >> PAGE_SHIFT;
+
+		page = dma_alloc_from_contiguous(hwdev, count, order);
+		if (page && iommu_no_mapping(hwdev) &&
+		    page_to_phys(page) + size > hwdev->coherent_dma_mask) {
+			dma_release_from_contiguous(hwdev, page, count);
+			page = NULL;
+		}
+	}
+
+	if (!page)
+		page = alloc_pages(flags, order);
+	if (!page)
 		return NULL;
-	memset(vaddr, 0, size);
+	memset(page_address(page), 0, size);
 
-	*dma_handle = __intel_map_single(hwdev, virt_to_bus(vaddr), size,
+	*dma_handle = __intel_map_single(hwdev, page_to_phys(page), size,
 					 DMA_BIDIRECTIONAL,
 					 hwdev->coherent_dma_mask);
 	if (*dma_handle)
-		return vaddr;
-	free_pages((unsigned long)vaddr, order);
+		return page_address(page);
+	if (!dma_release_from_contiguous(hwdev, page, size >> PAGE_SHIFT))
+		__free_pages(page, order);
+
 	return NULL;
 }
 
@@ -3037,12 +3051,14 @@ static void intel_free_coherent(struct device *hwdev, size_t size, void *vaddr,
 				dma_addr_t dma_handle, struct dma_attrs *attrs)
 {
 	int order;
+	struct page *page = virt_to_page(vaddr);
 
 	size = PAGE_ALIGN(size);
 	order = get_order(size);
 
 	intel_unmap_page(hwdev, dma_handle, size, DMA_BIDIRECTIONAL, NULL);
-	free_pages((unsigned long)vaddr, order);
+	if (!dma_release_from_contiguous(hwdev, page, size >> PAGE_SHIFT))
+		__free_pages(page, order);
 }
 
 static void intel_unmap_sg(struct device *hwdev, struct scatterlist *sglist,
-- 
1.8.3.2

[PATCH v2 4/5] memblock: introduce memblock_alloc_range()

[Akinobu Mita]

This introduces memblock_alloc_range() which allocates memblock from
the specified range of physical address.  I would like to use this
function to specify the location of CMA.

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Don Dutile <ddutile@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: x86@kernel.org
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
New patch from this version

 include/linux/memblock.h |  2 ++
 mm/memblock.c            | 27 ++++++++++++++++++++-------
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 77c60e5..95e0cfa 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -176,6 +176,8 @@ static inline bool memblock_bottom_up(void) { return false; }
 #define MEMBLOCK_ALLOC_ANYWHERE	(~(phys_addr_t)0)
 #define MEMBLOCK_ALLOC_ACCESSIBLE	0
 
+phys_addr_t __init memblock_alloc_range(phys_addr_t size, phys_addr_t align,
+					phys_addr_t start, phys_addr_t end);
 phys_addr_t memblock_alloc_base(phys_addr_t size, phys_addr_t align,
 				phys_addr_t max_addr);
 phys_addr_t __memblock_alloc_base(phys_addr_t size, phys_addr_t align,
diff --git a/mm/memblock.c b/mm/memblock.c
index 53e477b..83b0542 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -864,25 +864,38 @@ int __init_memblock memblock_set_node(phys_addr_t base, phys_addr_t size,
 }
 #endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
 
-static phys_addr_t __init memblock_alloc_base_nid(phys_addr_t size,
-					phys_addr_t align, phys_addr_t max_addr,
-					int nid)
+static phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size,
+					phys_addr_t align, phys_addr_t start,
+					phys_addr_t end, int nid)
 {
 	phys_addr_t found;
 
 	if (WARN_ON(!align))
 		align = __alignof__(long long);
 
-	/* align @size to avoid excessive fragmentation on reserved array */
-	size = round_up(size, align);
-
-	found = memblock_find_in_range_node(0, max_addr, size, align, nid);
+	found = memblock_find_in_range_node(start, end, size, align, nid);
 	if (found && !memblock_reserve(found, size))
 		return found;
 
 	return 0;
 }
 
+phys_addr_t __init memblock_alloc_range(phys_addr_t size, phys_addr_t align,
+					phys_addr_t start, phys_addr_t end)
+{
+	return memblock_alloc_range_nid(size, align, start, end, MAX_NUMNODES);
+}
+
+static phys_addr_t __init memblock_alloc_base_nid(phys_addr_t size,
+					phys_addr_t align, phys_addr_t max_addr,
+					int nid)
+{
+	/* align @size to avoid excessive fragmentation on reserved array */
+	size = round_up(size, align);
+
+	return memblock_alloc_range_nid(size, align, 0, max_addr, nid);
+}
+
 phys_addr_t __init memblock_alloc_nid(phys_addr_t size, phys_addr_t align, int nid)
 {
 	return memblock_alloc_base_nid(size, align, MEMBLOCK_ALLOC_ACCESSIBLE, nid);
-- 
1.8.3.2

[PATCH v2 5/5] cma: add placement specifier for "cma=" kernel parameter

[Akinobu Mita]

Currently, "cma=" kernel parameter is used to specify the size of CMA,
but we can't specify where it is located.  We want to locate CMA below
4GB for devices only supporting 32-bit addressing on 64-bit systems
without iommu.

This enables to specify the placement of CMA by extending "cma=" kernel
parameter.

Examples:
1. locate 64MB CMA below 4GB by "cma=64M@0-4G"
2. locate 64MB CMA exact at 512MB by "cma=64M@512M"

Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Don Dutile <ddutile@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: x86@kernel.org
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
New patch from this version

 Documentation/kernel-parameters.txt |  7 +++++--
 drivers/base/dma-contiguous.c       | 42 ++++++++++++++++++++++++++++---------
 include/linux/dma-contiguous.h      |  9 +++++---
 3 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index b9e9bd8..fdfd1ff 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -577,8 +577,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Also note the kernel might malfunction if you disable
 			some critical bits.
 
-	cma=nn[MG]	[ARM,KNL]
-			Sets the size of kernel global memory area for contiguous
+	cma=nn[MG]@[start[MG][-end[MG]]]
+			[ARM,X86,KNL]
+			Sets the size of kernel global memory area for
+			contiguous memory allocations and optionally the
+			placement constraint by the physical address range of
 			memory allocations. For more information, see
 			include/linux/dma-contiguous.h
 
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 165c2c2..b056661 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -59,11 +59,22 @@ struct cma *dma_contiguous_default_area;
  */
 static const phys_addr_t size_bytes = CMA_SIZE_MBYTES * SZ_1M;
 static phys_addr_t size_cmdline = -1;
+static phys_addr_t base_cmdline;
+static phys_addr_t limit_cmdline;
 
 static int __init early_cma(char *p)
 {
 	pr_debug("%s(%s)\n", __func__, p);
 	size_cmdline = memparse(p, &p);
+	if (*p != '@')
+		return 0;
+	base_cmdline = memparse(p + 1, &p);
+	if (*p != '-') {
+		limit_cmdline = base_cmdline + size_cmdline;
+		return 0;
+	}
+	limit_cmdline = memparse(p + 1, &p);
+
 	return 0;
 }
 early_param("cma", early_cma);
@@ -107,11 +118,18 @@ static inline __maybe_unused phys_addr_t cma_early_percent_memory(void)
 void __init dma_contiguous_reserve(phys_addr_t limit)
 {
 	phys_addr_t selected_size = 0;
+	phys_addr_t selected_base = 0;
+	phys_addr_t selected_limit = limit;
+	bool fixed = false;
 
 	pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
 
 	if (size_cmdline != -1) {
 		selected_size = size_cmdline;
+		selected_base = base_cmdline;
+		selected_limit = min_not_zero(limit_cmdline, limit);
+		if (base_cmdline + size_cmdline == limit_cmdline)
+			fixed = true;
 	} else {
 #ifdef CONFIG_CMA_SIZE_SEL_MBYTES
 		selected_size = size_bytes;
@@ -128,10 +146,12 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
 		pr_debug("%s: reserving %ld MiB for global area\n", __func__,
 			 (unsigned long)selected_size / SZ_1M);
 
-		dma_contiguous_reserve_area(selected_size, 0, limit,
-					    &dma_contiguous_default_area);
+		dma_contiguous_reserve_area(selected_size, selected_base,
+					    selected_limit,
+					    &dma_contiguous_default_area,
+					    fixed);
 	}
-};
+}
 
 static DEFINE_MUTEX(cma_mutex);
 
@@ -187,15 +207,20 @@ core_initcall(cma_init_reserved_areas);
  * @base: Base address of the reserved area optional, use 0 for any
  * @limit: End address of the reserved memory (optional, 0 for any).
  * @res_cma: Pointer to store the created cma region.
+ * @fixed: hint about where to place the reserved area
  *
  * This function reserves memory from early allocator. It should be
  * called by arch specific code once the early allocator (memblock or bootmem)
  * has been activated and all other subsystems have already allocated/reserved
  * memory. This function allows to create custom reserved areas for specific
  * devices.
+ *
+ * If @fixed is true, reserve contiguous area at exactly @base.  If false,
+ * reserve in range from @base to @limit.
  */
 int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
-				       phys_addr_t limit, struct cma **res_cma)
+				       phys_addr_t limit, struct cma **res_cma,
+				       bool fixed)
 {
 	struct cma *cma = &cma_areas[cma_area_count];
 	phys_addr_t alignment;
@@ -221,18 +246,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
 	limit &= ~(alignment - 1);
 
 	/* Reserve memory */
-	if (base) {
+	if (base && fixed) {
 		if (memblock_is_region_reserved(base, size) ||
 		    memblock_reserve(base, size) < 0) {
 			ret = -EBUSY;
 			goto err;
 		}
 	} else {
-		/*
-		 * Use __memblock_alloc_base() since
-		 * memblock_alloc_base() panic()s.
-		 */
-		phys_addr_t addr = __memblock_alloc_base(size, alignment, limit);
+		phys_addr_t addr = memblock_alloc_range(size, alignment, base,
+							limit);
 		if (!addr) {
 			ret = -ENOMEM;
 			goto err;
diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
index 3b28f93..772eab5 100644
--- a/include/linux/dma-contiguous.h
+++ b/include/linux/dma-contiguous.h
@@ -88,7 +88,8 @@ static inline void dma_contiguous_set_default(struct cma *cma)
 void dma_contiguous_reserve(phys_addr_t addr_limit);
 
 int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
-				       phys_addr_t limit, struct cma **res_cma);
+				       phys_addr_t limit, struct cma **res_cma,
+				       bool fixed);
 
 /**
  * dma_declare_contiguous() - reserve area for contiguous memory handling
@@ -108,7 +109,7 @@ static inline int dma_declare_contiguous(struct device *dev, phys_addr_t size,
 {
 	struct cma *cma;
 	int ret;
-	ret = dma_contiguous_reserve_area(size, base, limit, &cma);
+	ret = dma_contiguous_reserve_area(size, base, limit, &cma, true);
 	if (ret == 0)
 		dev_set_cma_area(dev, cma);
 
@@ -136,7 +137,9 @@ static inline void dma_contiguous_set_default(struct cma *cma) { }
 static inline void dma_contiguous_reserve(phys_addr_t limit) { }
 
 static inline int dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
-				       phys_addr_t limit, struct cma **res_cma) {
+				       phys_addr_t limit, struct cma **res_cma,
+				       bool fixed)
+{
 	return -ENOSYS;
 }
 
-- 
1.8.3.2

Re: [PATCH v2 1/5] x86: make dma_alloc_coherent() return zeroed memory if CMA is enabled

[Konrad Rzeszutek Wilk]

On Tue, Jan 14, 2014 at 11:13:46PM +0900, Akinobu Mita wrote:
> Calling dma_alloc_coherent() with __GFP_ZERO must return zeroed memory.
> 
> But when the contiguous memory allocator (CMA) is enabled on x86 and
> the memory region is allocated by dma_alloc_from_contiguous(), it
> doesn't return zeroed memory.  Because dma_generic_alloc_coherent()

So why not fix it there to return zeroed out memory?

> forgot to fill the memory region with zero if it was allocated by
> dma_alloc_from_contiguous()
> 
> Most implementations of dma_alloc_coherent() return zeroed memory
> regardless of whether __GFP_ZERO is specified.  So this fixes it by
> unconditionally zeroing the allocated memory region.
> 
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Don Dutile <ddutile@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: x86@kernel.org
> Cc: iommu@lists.linux-foundation.org
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
> New patch from this version
> 
>  arch/x86/kernel/pci-dma.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> index 872079a..9644405 100644
> --- a/arch/x86/kernel/pci-dma.c
> +++ b/arch/x86/kernel/pci-dma.c
> @@ -97,7 +97,6 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t size,
>  
>  	dma_mask = dma_alloc_coherent_mask(dev, flag);
>  
> -	flag |= __GFP_ZERO;
>  again:
>  	page = NULL;
>  	if (!(flag & GFP_ATOMIC))
> @@ -118,7 +117,7 @@ again:
>  
>  		return NULL;
>  	}
> -
> +	memset(page_address(page), 0, size);
>  	*dma_addr = addr;
>  	return page_address(page);
>  }
> -- 
> 1.8.3.2
> 

Re: [PATCH v2 2/5] x86: enable DMA CMA with swiotlb

[Konrad Rzeszutek Wilk]

On Tue, Jan 14, 2014 at 11:13:47PM +0900, Akinobu Mita wrote:
> The DMA Contiguous Memory Allocator support on x86 is disabled when
> swiotlb config option is enabled.  So DMA CMA is always disabled on
> x86_64 because swiotlb is always enabled.  This attempts to support
> for DMA CMA with enabling swiotlb config option.
> 
> The contiguous memory allocator on x86 is integrated in the function
> dma_generic_alloc_coherent() which is .alloc callback in nommu_dma_ops
> for dma_alloc_coherent().
> 
> x86_swiotlb_alloc_coherent() which is .alloc callback in swiotlb_dma_ops
> tries to allocate with dma_generic_alloc_coherent() firstly and then
> swiotlb_alloc_coherent() is called as a fallback.
> 
> The main part of supporting DMA CMA with swiotlb is that changing
> x86_swiotlb_free_coherent() which is .free callback in swiotlb_dma_ops
> for dma_free_coherent() so that it can distinguish memory allocated by
> dma_generic_alloc_coherent() from one allocated by swiotlb_alloc_coherent()
> and release it with dma_generic_free_coherent() which can handle contiguous
> memory.  This change requires making is_swiotlb_buffer() global function.
> 
> This also needs to change .free callback in the dma_map_ops for amd_gart
> and sta2x11, because these dma_ops are also using
> dma_generic_alloc_coherent().
> 
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Don Dutile <ddutile@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: x86@kernel.org
> Cc: iommu@lists.linux-foundation.org
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> No change from the previous version
> 
>  arch/x86/Kconfig               | 2 +-
>  arch/x86/include/asm/swiotlb.h | 7 +++++++
>  arch/x86/kernel/amd_gart_64.c  | 2 +-
>  arch/x86/kernel/pci-swiotlb.c  | 9 ++++++---
>  arch/x86/pci/sta2x11-fixup.c   | 6 ++----
>  include/linux/swiotlb.h        | 2 ++
>  lib/swiotlb.c                  | 2 +-

It looks reasonable from my perspective (as swiotlb maintainer).

Not too thrilled about the  'is_swiotlb_buffer' but that code is
quite small so it should be fast enough.

>  7 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 0952ecd..1b6275d 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -40,7 +40,7 @@ config X86
>  	select ARCH_WANT_OPTIONAL_GPIOLIB
>  	select ARCH_WANT_FRAME_POINTERS
>  	select HAVE_DMA_ATTRS
> -	select HAVE_DMA_CONTIGUOUS if !SWIOTLB
> +	select HAVE_DMA_CONTIGUOUS
>  	select HAVE_KRETPROBES
>  	select HAVE_OPTPROBES
>  	select HAVE_KPROBES_ON_FTRACE
> diff --git a/arch/x86/include/asm/swiotlb.h b/arch/x86/include/asm/swiotlb.h
> index 977f176..ab05d73 100644
> --- a/arch/x86/include/asm/swiotlb.h
> +++ b/arch/x86/include/asm/swiotlb.h
> @@ -29,4 +29,11 @@ static inline void pci_swiotlb_late_init(void)
>  
>  static inline void dma_mark_clean(void *addr, size_t size) {}
>  
> +extern void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> +					dma_addr_t *dma_handle, gfp_t flags,
> +					struct dma_attrs *attrs);
> +extern void x86_swiotlb_free_coherent(struct device *dev, size_t size,
> +					void *vaddr, dma_addr_t dma_addr,
> +					struct dma_attrs *attrs);
> +
>  #endif /* _ASM_X86_SWIOTLB_H */
> diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
> index b574b29..8e3842f 100644
> --- a/arch/x86/kernel/amd_gart_64.c
> +++ b/arch/x86/kernel/amd_gart_64.c
> @@ -512,7 +512,7 @@ gart_free_coherent(struct device *dev, size_t size, void *vaddr,
>  		   dma_addr_t dma_addr, struct dma_attrs *attrs)
>  {
>  	gart_unmap_page(dev, dma_addr, size, DMA_BIDIRECTIONAL, NULL);
> -	free_pages((unsigned long)vaddr, get_order(size));
> +	dma_generic_free_coherent(dev, size, vaddr, dma_addr, attrs);
>  }
>  
>  static int gart_mapping_error(struct device *dev, dma_addr_t dma_addr)
> diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
> index 6c483ba..77dd0ad 100644
> --- a/arch/x86/kernel/pci-swiotlb.c
> +++ b/arch/x86/kernel/pci-swiotlb.c
> @@ -14,7 +14,7 @@
>  #include <asm/iommu_table.h>
>  int swiotlb __read_mostly;
>  
> -static void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
> +void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
>  					dma_addr_t *dma_handle, gfp_t flags,
>  					struct dma_attrs *attrs)
>  {
> @@ -28,11 +28,14 @@ static void *x86_swiotlb_alloc_coherent(struct device *hwdev, size_t size,
>  	return swiotlb_alloc_coherent(hwdev, size, dma_handle, flags);
>  }
>  
> -static void x86_swiotlb_free_coherent(struct device *dev, size_t size,
> +void x86_swiotlb_free_coherent(struct device *dev, size_t size,
>  				      void *vaddr, dma_addr_t dma_addr,
>  				      struct dma_attrs *attrs)
>  {
> -	swiotlb_free_coherent(dev, size, vaddr, dma_addr);
> +	if (is_swiotlb_buffer(dma_to_phys(dev, dma_addr)))
> +		swiotlb_free_coherent(dev, size, vaddr, dma_addr);
> +	else
> +		dma_generic_free_coherent(dev, size, vaddr, dma_addr, attrs);
>  }
>  
>  static struct dma_map_ops swiotlb_dma_ops = {
> diff --git a/arch/x86/pci/sta2x11-fixup.c b/arch/x86/pci/sta2x11-fixup.c
> index 9d8a509..5ceda85 100644
> --- a/arch/x86/pci/sta2x11-fixup.c
> +++ b/arch/x86/pci/sta2x11-fixup.c
> @@ -173,9 +173,7 @@ static void *sta2x11_swiotlb_alloc_coherent(struct device *dev,
>  {
>  	void *vaddr;
>  
> -	vaddr = dma_generic_alloc_coherent(dev, size, dma_handle, flags, attrs);
> -	if (!vaddr)
> -		vaddr = swiotlb_alloc_coherent(dev, size, dma_handle, flags);
> +	vaddr = x86_swiotlb_alloc_coherent(dev, size, dma_handle, flags, attrs);
>  	*dma_handle = p2a(*dma_handle, to_pci_dev(dev));
>  	return vaddr;
>  }
> @@ -183,7 +181,7 @@ static void *sta2x11_swiotlb_alloc_coherent(struct device *dev,
>  /* We have our own dma_ops: the same as swiotlb but from alloc (above) */
>  static struct dma_map_ops sta2x11_dma_ops = {
>  	.alloc = sta2x11_swiotlb_alloc_coherent,
> -	.free = swiotlb_free_coherent,
> +	.free = x86_swiotlb_free_coherent,
>  	.map_page = swiotlb_map_page,
>  	.unmap_page = swiotlb_unmap_page,
>  	.map_sg = swiotlb_map_sg_attrs,
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index a5ffd32..e7a018e 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -116,4 +116,6 @@ static inline void swiotlb_free(void) { }
>  #endif
>  
>  extern void swiotlb_print_info(void);
> +extern int is_swiotlb_buffer(phys_addr_t paddr);
> +
>  #endif /* __LINUX_SWIOTLB_H */
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index fe978e0..6e4a798 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -369,7 +369,7 @@ void __init swiotlb_free(void)
>  	io_tlb_nslabs = 0;
>  }
>  
> -static int is_swiotlb_buffer(phys_addr_t paddr)
> +int is_swiotlb_buffer(phys_addr_t paddr)
>  {
>  	return paddr >= io_tlb_start && paddr < io_tlb_end;
>  }
> -- 
> 1.8.3.2
> 

Re: [PATCH v2 3/5] intel-iommu: integrate DMA CMA

[Marek Szyprowski]

Hello,

On 2014-01-14 15:13, Akinobu Mita wrote:
> This adds support for the DMA Contiguous Memory Allocator for intel-iommu.
> This change enables dma_alloc_coherent() to allocate big contiguous
> memory.
>
> It is achieved in the same way as nommu_dma_ops currently does, i.e.
> trying to allocate memory by dma_alloc_from_contiguous() and alloc_pages()
> is used as a fallback.
>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Don Dutile <ddutile@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: x86@kernel.org
> Cc: iommu@lists.linux-foundation.org
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
> No change from the previous version
>
>   drivers/iommu/intel-iommu.c | 32 ++++++++++++++++++++++++--------
>   1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index fd426ca..172c2b0 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -3004,7 +3004,7 @@ static void *intel_alloc_coherent(struct device *hwdev, size_t size,
>   				  dma_addr_t *dma_handle, gfp_t flags,
>   				  struct dma_attrs *attrs)
>   {
> -	void *vaddr;
> +	struct page *page = NULL;
>   	int order;
>   
>   	size = PAGE_ALIGN(size);
> @@ -3019,17 +3019,31 @@ static void *intel_alloc_coherent(struct device *hwdev, size_t size,
>   			flags |= GFP_DMA32;
>   	}
>   
> -	vaddr = (void *)__get_free_pages(flags, order);
> -	if (!vaddr)
> +	if (!(flags & GFP_ATOMIC)) {

GFP_ATOMIC is not a flag, so please change the above check to:
            if (flags & __GFP_WAIT)

I will also fix the similar issue in arch/x86/kernel/pci-dma.c

> +		unsigned int count = size >> PAGE_SHIFT;
> +
> +		page = dma_alloc_from_contiguous(hwdev, count, order);
> +		if (page && iommu_no_mapping(hwdev) &&
> +		    page_to_phys(page) + size > hwdev->coherent_dma_mask) {
> +			dma_release_from_contiguous(hwdev, page, count);
> +			page = NULL;
> +		}
> +	}
> +
> +	if (!page)
> +		page = alloc_pages(flags, order);
> +	if (!page)
>   		return NULL;
> -	memset(vaddr, 0, size);
> +	memset(page_address(page), 0, size);
>   
> -	*dma_handle = __intel_map_single(hwdev, virt_to_bus(vaddr), size,
> +	*dma_handle = __intel_map_single(hwdev, page_to_phys(page), size,
>   					 DMA_BIDIRECTIONAL,
>   					 hwdev->coherent_dma_mask);
>   	if (*dma_handle)
> -		return vaddr;
> -	free_pages((unsigned long)vaddr, order);
> +		return page_address(page);
> +	if (!dma_release_from_contiguous(hwdev, page, size >> PAGE_SHIFT))
> +		__free_pages(page, order);
> +
>   	return NULL;
>   }
>   
> @@ -3037,12 +3051,14 @@ static void intel_free_coherent(struct device *hwdev, size_t size, void *vaddr,
>   				dma_addr_t dma_handle, struct dma_attrs *attrs)
>   {
>   	int order;
> +	struct page *page = virt_to_page(vaddr);
>   
>   	size = PAGE_ALIGN(size);
>   	order = get_order(size);
>   
>   	intel_unmap_page(hwdev, dma_handle, size, DMA_BIDIRECTIONAL, NULL);
> -	free_pages((unsigned long)vaddr, order);
> +	if (!dma_release_from_contiguous(hwdev, page, size >> PAGE_SHIFT))
> +		__free_pages(page, order);
>   }
>   
>   static void intel_unmap_sg(struct device *hwdev, struct scatterlist *sglist,

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v2 3/5] intel-iommu: integrate DMA CMA

[Akinobu Mita]

2014/1/16 Marek Szyprowski <m.szyprowski@samsung.com>:
>> @@ -3019,17 +3019,31 @@ static void *intel_alloc_coherent(struct device
>> *hwdev, size_t size,
>>                         flags |= GFP_DMA32;
>>         }
>>   -     vaddr = (void *)__get_free_pages(flags, order);
>> -       if (!vaddr)
>> +       if (!(flags & GFP_ATOMIC)) {
>
>
> GFP_ATOMIC is not a flag, so please change the above check to:
>            if (flags & __GFP_WAIT)

Thanks for your review.  I'll fix it in the next version.

Re: [PATCH v2 2/5] x86: enable DMA CMA with swiotlb

[Akinobu Mita]

2014/1/16 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> On Tue, Jan 14, 2014 at 11:13:47PM +0900, Akinobu Mita wrote:
>> The DMA Contiguous Memory Allocator support on x86 is disabled when
>> swiotlb config option is enabled.  So DMA CMA is always disabled on
>> x86_64 because swiotlb is always enabled.  This attempts to support
>> for DMA CMA with enabling swiotlb config option.
>>
>> The contiguous memory allocator on x86 is integrated in the function
>> dma_generic_alloc_coherent() which is .alloc callback in nommu_dma_ops
>> for dma_alloc_coherent().
>>
>> x86_swiotlb_alloc_coherent() which is .alloc callback in swiotlb_dma_ops
>> tries to allocate with dma_generic_alloc_coherent() firstly and then
>> swiotlb_alloc_coherent() is called as a fallback.
>>
>> The main part of supporting DMA CMA with swiotlb is that changing
>> x86_swiotlb_free_coherent() which is .free callback in swiotlb_dma_ops
>> for dma_free_coherent() so that it can distinguish memory allocated by
>> dma_generic_alloc_coherent() from one allocated by swiotlb_alloc_coherent()
>> and release it with dma_generic_free_coherent() which can handle contiguous
>> memory.  This change requires making is_swiotlb_buffer() global function.
>>
>> This also needs to change .free callback in the dma_map_ops for amd_gart
>> and sta2x11, because these dma_ops are also using
>> dma_generic_alloc_coherent().
>>
>> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: David Woodhouse <dwmw2@infradead.org>
>> Cc: Don Dutile <ddutile@redhat.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: Andi Kleen <andi@firstfloor.org>
>> Cc: x86@kernel.org
>> Cc: iommu@lists.linux-foundation.org
>> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
>> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> No change from the previous version
>>
>>  arch/x86/Kconfig               | 2 +-
>>  arch/x86/include/asm/swiotlb.h | 7 +++++++
>>  arch/x86/kernel/amd_gart_64.c  | 2 +-
>>  arch/x86/kernel/pci-swiotlb.c  | 9 ++++++---
>>  arch/x86/pci/sta2x11-fixup.c   | 6 ++----
>>  include/linux/swiotlb.h        | 2 ++
>>  lib/swiotlb.c                  | 2 +-
>
> It looks reasonable from my perspective (as swiotlb maintainer).
>
> Not too thrilled about the  'is_swiotlb_buffer' but that code is
> quite small so it should be fast enough.

Thanks.  Can I get your Acked-by for this patch?

Re: [PATCH v2 1/5] x86: make dma_alloc_coherent() return zeroed memory if CMA is enabled

[Akinobu Mita]

2014/1/16 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> On Tue, Jan 14, 2014 at 11:13:46PM +0900, Akinobu Mita wrote:
>> Calling dma_alloc_coherent() with __GFP_ZERO must return zeroed memory.
>>
>> But when the contiguous memory allocator (CMA) is enabled on x86 and
>> the memory region is allocated by dma_alloc_from_contiguous(), it
>> doesn't return zeroed memory.  Because dma_generic_alloc_coherent()
>
> So why not fix it there to return zeroed out memory?

I thought it looked nicer than this patch as we can remove memset
from all caller of dma_alloc_from_contiguous().  But if I look at
the caller on arm, we can't simply remove the memset because
__dma_clear_buffer() is used there for ensuring cache flushing and
it is used in many places.

Of course we can do redundant memset in dma_alloc_from_contiguous(),
but now I think this patch is less impact for fixing this problem.

Re: [PATCH v2 2/5] x86: enable DMA CMA with swiotlb

[Konrad Rzeszutek Wilk]

On Fri, Jan 17, 2014 at 08:32:09AM +0900, Akinobu Mita wrote:
> 2014/1/16 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> > On Tue, Jan 14, 2014 at 11:13:47PM +0900, Akinobu Mita wrote:
> >> The DMA Contiguous Memory Allocator support on x86 is disabled when
> >> swiotlb config option is enabled.  So DMA CMA is always disabled on
> >> x86_64 because swiotlb is always enabled.  This attempts to support
> >> for DMA CMA with enabling swiotlb config option.
> >>
> >> The contiguous memory allocator on x86 is integrated in the function
> >> dma_generic_alloc_coherent() which is .alloc callback in nommu_dma_ops
> >> for dma_alloc_coherent().
> >>
> >> x86_swiotlb_alloc_coherent() which is .alloc callback in swiotlb_dma_ops
> >> tries to allocate with dma_generic_alloc_coherent() firstly and then
> >> swiotlb_alloc_coherent() is called as a fallback.
> >>
> >> The main part of supporting DMA CMA with swiotlb is that changing
> >> x86_swiotlb_free_coherent() which is .free callback in swiotlb_dma_ops
> >> for dma_free_coherent() so that it can distinguish memory allocated by
> >> dma_generic_alloc_coherent() from one allocated by swiotlb_alloc_coherent()
> >> and release it with dma_generic_free_coherent() which can handle contiguous
> >> memory.  This change requires making is_swiotlb_buffer() global function.
> >>
> >> This also needs to change .free callback in the dma_map_ops for amd_gart
> >> and sta2x11, because these dma_ops are also using
> >> dma_generic_alloc_coherent().
> >>
> >> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> Cc: David Woodhouse <dwmw2@infradead.org>
> >> Cc: Don Dutile <ddutile@redhat.com>
> >> Cc: Thomas Gleixner <tglx@linutronix.de>
> >> Cc: Ingo Molnar <mingo@redhat.com>
> >> Cc: "H. Peter Anvin" <hpa@zytor.com>
> >> Cc: Andi Kleen <andi@firstfloor.org>
> >> Cc: x86@kernel.org
> >> Cc: iommu@lists.linux-foundation.org
> >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> >> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> ---
> >> No change from the previous version
> >>
> >>  arch/x86/Kconfig               | 2 +-
> >>  arch/x86/include/asm/swiotlb.h | 7 +++++++
> >>  arch/x86/kernel/amd_gart_64.c  | 2 +-
> >>  arch/x86/kernel/pci-swiotlb.c  | 9 ++++++---
> >>  arch/x86/pci/sta2x11-fixup.c   | 6 ++----
> >>  include/linux/swiotlb.h        | 2 ++
> >>  lib/swiotlb.c                  | 2 +-
> >
> > It looks reasonable from my perspective (as swiotlb maintainer).
> >
> > Not too thrilled about the  'is_swiotlb_buffer' but that code is
> > quite small so it should be fast enough.
> 
> Thanks.  Can I get your Acked-by for this patch?

Yes. Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Re: [PATCH v2 1/5] x86: make dma_alloc_coherent() return zeroed memory if CMA is enabled

[Konrad Rzeszutek Wilk]

On Fri, Jan 17, 2014 at 08:34:38AM +0900, Akinobu Mita wrote:
> 2014/1/16 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> > On Tue, Jan 14, 2014 at 11:13:46PM +0900, Akinobu Mita wrote:
> >> Calling dma_alloc_coherent() with __GFP_ZERO must return zeroed memory.
> >>
> >> But when the contiguous memory allocator (CMA) is enabled on x86 and
> >> the memory region is allocated by dma_alloc_from_contiguous(), it
> >> doesn't return zeroed memory.  Because dma_generic_alloc_coherent()
> >
> > So why not fix it there to return zeroed out memory?
> 
> I thought it looked nicer than this patch as we can remove memset
> from all caller of dma_alloc_from_contiguous().  But if I look at
> the caller on arm, we can't simply remove the memset because
> __dma_clear_buffer() is used there for ensuring cache flushing and
> it is used in many places.

OK, that should be part of the commit description.
> 
> Of course we can do redundant memset in dma_alloc_from_contiguous(),
> but now I think this patch is less impact for fixing this problem.

OK.

Re: [PATCH v2 1/5] x86: make dma_alloc_coherent() return zeroed memory if CMA is enabled

[Marek Szyprowski]

Hello,

On 2014-01-14 15:13, Akinobu Mita wrote:
> Calling dma_alloc_coherent() with __GFP_ZERO must return zeroed memory.
>
> But when the contiguous memory allocator (CMA) is enabled on x86 and
> the memory region is allocated by dma_alloc_from_contiguous(), it
> doesn't return zeroed memory.  Because dma_generic_alloc_coherent()
> forgot to fill the memory region with zero if it was allocated by
> dma_alloc_from_contiguous()

I just wonder how it will work with high mem? I've didn't check the x86
dma mapping code yet, but page_address() works only for pages, which comes
from low memory. In other patches you have added an option to place CMA
area anywhere in the memory. Is the x86 pci dma code ready for the case
when cma area is put into high mem and direct mappings are not available?

> Most implementations of dma_alloc_coherent() return zeroed memory
> regardless of whether __GFP_ZERO is specified.  So this fixes it by
> unconditionally zeroing the allocated memory region.
>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Don Dutile <ddutile@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: x86@kernel.org
> Cc: iommu@lists.linux-foundation.org
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
> New patch from this version
>
>   arch/x86/kernel/pci-dma.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> index 872079a..9644405 100644
> --- a/arch/x86/kernel/pci-dma.c
> +++ b/arch/x86/kernel/pci-dma.c
> @@ -97,7 +97,6 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t size,
>   
>   	dma_mask = dma_alloc_coherent_mask(dev, flag);
>   
> -	flag |= __GFP_ZERO;
>   again:
>   	page = NULL;
>   	if (!(flag & GFP_ATOMIC))
> @@ -118,7 +117,7 @@ again:
>   
>   		return NULL;
>   	}
> -
> +	memset(page_address(page), 0, size);
>   	*dma_addr = addr;
>   	return page_address(page);
>   }

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v2 1/5] x86: make dma_alloc_coherent() return zeroed memory if CMA is enabled

[Konrad Rzeszutek Wilk]

On Mon, Jan 27, 2014 at 02:54:47PM +0100, Marek Szyprowski wrote:
> Hello,
> 
> On 2014-01-14 15:13, Akinobu Mita wrote:
> >Calling dma_alloc_coherent() with __GFP_ZERO must return zeroed memory.
> >
> >But when the contiguous memory allocator (CMA) is enabled on x86 and
> >the memory region is allocated by dma_alloc_from_contiguous(), it
> >doesn't return zeroed memory.  Because dma_generic_alloc_coherent()
> >forgot to fill the memory region with zero if it was allocated by
> >dma_alloc_from_contiguous()
> 
> I just wonder how it will work with high mem? I've didn't check the x86
> dma mapping code yet, but page_address() works only for pages, which comes
> from low memory. In other patches you have added an option to place CMA
> area anywhere in the memory. Is the x86 pci dma code ready for the case
> when cma area is put into high mem and direct mappings are not available?

Yes and no. The swiotbl_bounce does have the code to take that into account.
But that is it - nothing else does - so I think you would run in the 
possiblity of 'page_address' not providing an correct virtual address.

> 
> >Most implementations of dma_alloc_coherent() return zeroed memory
> >regardless of whether __GFP_ZERO is specified.  So this fixes it by
> >unconditionally zeroing the allocated memory region.
> >
> >Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> >Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >Cc: David Woodhouse <dwmw2@infradead.org>
> >Cc: Don Dutile <ddutile@redhat.com>
> >Cc: Thomas Gleixner <tglx@linutronix.de>
> >Cc: Ingo Molnar <mingo@redhat.com>
> >Cc: "H. Peter Anvin" <hpa@zytor.com>
> >Cc: Andi Kleen <andi@firstfloor.org>
> >Cc: x86@kernel.org
> >Cc: iommu@lists.linux-foundation.org
> >Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> >---
> >New patch from this version
> >
> >  arch/x86/kernel/pci-dma.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> >diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> >index 872079a..9644405 100644
> >--- a/arch/x86/kernel/pci-dma.c
> >+++ b/arch/x86/kernel/pci-dma.c
> >@@ -97,7 +97,6 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t size,
> >  	dma_mask = dma_alloc_coherent_mask(dev, flag);
> >-	flag |= __GFP_ZERO;
> >  again:
> >  	page = NULL;
> >  	if (!(flag & GFP_ATOMIC))
> >@@ -118,7 +117,7 @@ again:
> >  		return NULL;
> >  	}
> >-
> >+	memset(page_address(page), 0, size);
> >  	*dma_addr = addr;
> >  	return page_address(page);
> >  }
> 
> Best regards
> -- 
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
> 

Re: [PATCH v2 1/5] x86: make dma_alloc_coherent() return zeroed memory if CMA is enabled

[Akinobu Mita]

2014-01-28 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
> On Mon, Jan 27, 2014 at 02:54:47PM +0100, Marek Szyprowski wrote:
>> Hello,
>>
>> On 2014-01-14 15:13, Akinobu Mita wrote:
>> >Calling dma_alloc_coherent() with __GFP_ZERO must return zeroed memory.
>> >
>> >But when the contiguous memory allocator (CMA) is enabled on x86 and
>> >the memory region is allocated by dma_alloc_from_contiguous(), it
>> >doesn't return zeroed memory.  Because dma_generic_alloc_coherent()
>> >forgot to fill the memory region with zero if it was allocated by
>> >dma_alloc_from_contiguous()
>>
>> I just wonder how it will work with high mem? I've didn't check the x86
>> dma mapping code yet, but page_address() works only for pages, which comes
>> from low memory. In other patches you have added an option to place CMA
>> area anywhere in the memory. Is the x86 pci dma code ready for the case
>> when cma area is put into high mem and direct mappings are not available?
>
> Yes and no. The swiotbl_bounce does have the code to take that into account.
> But that is it - nothing else does - so I think you would run in the
> possiblity of 'page_address' not providing an correct virtual address.

Thanks for spotting the issue.  I haven't much tested on x86_32.
I'll go through it and try to find the solution.

>>
>> >Most implementations of dma_alloc_coherent() return zeroed memory
>> >regardless of whether __GFP_ZERO is specified.  So this fixes it by
>> >unconditionally zeroing the allocated memory region.
>> >
>> >Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>> >Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> >Cc: David Woodhouse <dwmw2@infradead.org>
>> >Cc: Don Dutile <ddutile@redhat.com>
>> >Cc: Thomas Gleixner <tglx@linutronix.de>
>> >Cc: Ingo Molnar <mingo@redhat.com>
>> >Cc: "H. Peter Anvin" <hpa@zytor.com>
>> >Cc: Andi Kleen <andi@firstfloor.org>
>> >Cc: x86@kernel.org
>> >Cc: iommu@lists.linux-foundation.org
>> >Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
>> >---
>> >New patch from this version
>> >
>> >  arch/x86/kernel/pci-dma.c | 3 +--
>> >  1 file changed, 1 insertion(+), 2 deletions(-)
>> >
>> >diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
>> >index 872079a..9644405 100644
>> >--- a/arch/x86/kernel/pci-dma.c
>> >+++ b/arch/x86/kernel/pci-dma.c
>> >@@ -97,7 +97,6 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t size,
>> >     dma_mask = dma_alloc_coherent_mask(dev, flag);
>> >-    flag |= __GFP_ZERO;
>> >  again:
>> >     page = NULL;
>> >     if (!(flag & GFP_ATOMIC))
>> >@@ -118,7 +117,7 @@ again:
>> >             return NULL;
>> >     }
>> >-
>> >+    memset(page_address(page), 0, size);
>> >     *dma_addr = addr;
>> >     return page_address(page);
>> >  }
>>
>> Best regards
>> --
>> Marek Szyprowski, PhD
>> Samsung R&D Institute Poland
>>

Re: [PATCH v2 1/5] x86: make dma_alloc_coherent() return zeroed memory if CMA is enabled

[Akinobu Mita]

2014-01-29 Akinobu Mita <akinobu.mita@gmail.com>:
> 2014-01-28 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>:
>> On Mon, Jan 27, 2014 at 02:54:47PM +0100, Marek Szyprowski wrote:
>>> Hello,
>>>
>>> On 2014-01-14 15:13, Akinobu Mita wrote:
>>> >Calling dma_alloc_coherent() with __GFP_ZERO must return zeroed memory.
>>> >
>>> >But when the contiguous memory allocator (CMA) is enabled on x86 and
>>> >the memory region is allocated by dma_alloc_from_contiguous(), it
>>> >doesn't return zeroed memory.  Because dma_generic_alloc_coherent()
>>> >forgot to fill the memory region with zero if it was allocated by
>>> >dma_alloc_from_contiguous()
>>>
>>> I just wonder how it will work with high mem? I've didn't check the x86
>>> dma mapping code yet, but page_address() works only for pages, which comes
>>> from low memory. In other patches you have added an option to place CMA
>>> area anywhere in the memory. Is the x86 pci dma code ready for the case
>>> when cma area is put into high mem and direct mappings are not available?
>>
>> Yes and no. The swiotbl_bounce does have the code to take that into account.
>> But that is it - nothing else does - so I think you would run in the
>> possiblity of 'page_address' not providing an correct virtual address.
>
> Thanks for spotting the issue.  I haven't much tested on x86_32.
> I'll go through it and try to find the solution.

I have confirmed that locating CMA on highmem range with a
'cma=size@start-end' kernel parameter by this patch set caused the
issue on x86_32.

This can be fixed by limiting CMA area upto max_low_pfn to prevent
from locating it on highmem at arch/x86/kernel/setup.c:setup_arch()

-       dma_contiguous_reserve(0);
+       dma_contiguous_reserve(max_low_pfn << PAGE_SHIFT);

I'm going to inlcude this change in this patch set.