diff mbox

arm64: make CONFIG_ZONE_DMA user settable

Message ID 1403499924-11214-1-git-send-email-msalter@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mark Salter June 23, 2014, 5:05 a.m. UTC
Commit 19e7640d1f (arm64: Replace ZONE_DMA32 with ZONE_DMA)
moves support for 32-bit DMA addressing into ZONE_DMA and renames
CONFIG_ZONE_DMA32 to CONFIG_ZONE_DMA.

Commit 2d5a5612bc (arm64: Limit the CMA buffer to 32-bit if ZONE_DMA)
forces the CMA buffer to be 32-bit addressable if CONFIG_ZONE_DMA is
defined.

These two patches pose a problem for platforms which have no 32-bit
addressable DRAM. If CONFIG_ZONE_DMA is turned on for such platforms,
CMA is unable to reserve a buffer for allocations. CONFIG_ZONE_DMA is
not user settable however, so there is no way to turn it off without
editing arch/arm64/Kconfig. Even if one edits Kconfig to turn off
CONFIG_ZONE_DMA, the kernel fails to build with such a config:

  arch/arm64/mm/init.c: In function ‘zone_sizes_init’:
  arch/arm64/mm/init.c:76:13: error: ‘ZONE_DMA’ undeclared (first use in this function)
     zone_size[ZONE_DMA] = max_dma - min;
               ^

This patch makes CONFIG_ZONE_DMA user settable and fixes the kernel
build when it is turned off.

Signed-off-by: Mark Salter <msalter@redhat.com>
---
 arch/arm64/Kconfig   | 11 ++++++++---
 arch/arm64/mm/init.c |  8 ++++++--
 2 files changed, 14 insertions(+), 5 deletions(-)

Comments

Russell King - ARM Linux June 23, 2014, 9:01 a.m. UTC | #1
On Mon, Jun 23, 2014 at 01:05:24AM -0400, Mark Salter wrote:
> +config ZONE_DMA
> +	bool "32-bit DMA memory allocation"
> +	def_bool y

This should be either bool + default or def_bool.  Not bool + def_bool.
Catalin Marinas June 23, 2014, 11:09 a.m. UTC | #2
On Mon, Jun 23, 2014 at 06:05:24AM +0100, Mark Salter wrote:
> Commit 19e7640d1f (arm64: Replace ZONE_DMA32 with ZONE_DMA)
> moves support for 32-bit DMA addressing into ZONE_DMA and renames
> CONFIG_ZONE_DMA32 to CONFIG_ZONE_DMA.
> 
> Commit 2d5a5612bc (arm64: Limit the CMA buffer to 32-bit if ZONE_DMA)
> forces the CMA buffer to be 32-bit addressable if CONFIG_ZONE_DMA is
> defined.
> 
> These two patches pose a problem for platforms which have no 32-bit
> addressable DRAM.

It's actually the bus/dma address that needs to be 32-bit rather than
the DRAM as seen by the CPU (which can be beyond 32-bit like the
Keystone platform).

> If CONFIG_ZONE_DMA is turned on for such platforms,
> CMA is unable to reserve a buffer for allocations. CONFIG_ZONE_DMA is
> not user settable however, so there is no way to turn it off without
> editing arch/arm64/Kconfig. Even if one edits Kconfig to turn off
> CONFIG_ZONE_DMA, the kernel fails to build with such a config:
> 
>   arch/arm64/mm/init.c: In function ‘zone_sizes_init’:
>   arch/arm64/mm/init.c:76:13: error: ‘ZONE_DMA’ undeclared (first use in this function)
>      zone_size[ZONE_DMA] = max_dma - min;
>                ^
> 
> This patch makes CONFIG_ZONE_DMA user settable and fixes the kernel
> build when it is turned off.

The reason I left CONFIG_ZONE_DMA not user settable is because on arm64
we aim for single Image by default. So ZONE_DMA would most likely be
needed on at least one platform. Even with ZONE_DMA it's just a guess
without proper system topology description. dma_to_phys() just takes a
NULL argument for device when trying to guess the physical address for a
32-bit DMA mask (and 32-bit devices may have some physical offset wired
already).

With the CMA fix, does the kernel work properly with a zero sized DMA
zone? Any GFP_DMA allocations will fail, including the swiotlb bounce
buffer. It may be fine if no devices require 32-bit DMA memory but I
wonder whether on such platforms it would be better to simply add
all the memory to ZONE_DMA.

Which gets us back to finding a way for describing such system topology
in hardware. We may be able to find a way with DT but unlikely for ACPI.

My proposal (in the absence of any kind of description) is to still
create a ZONE_DMA if we have DMA memory below 32-bit, otherwise just add
everything (>32-bit) to ZONE_DMA. Basically an extension from your CMA
patch, make dma_phys_limit static in that file and set it to
memblock_end_of_DRAM() if no 32-bit DMA. Re-use it in the
zone_sizes_init() function for ZONE_DMA (maybe with a pr_info for no
32-bit only DMA zone).
Mark Salter June 23, 2014, 1:17 p.m. UTC | #3
On Mon, 2014-06-23 at 12:09 +0100, Catalin Marinas wrote:
> On Mon, Jun 23, 2014 at 06:05:24AM +0100, Mark Salter wrote:
> > Commit 19e7640d1f (arm64: Replace ZONE_DMA32 with ZONE_DMA)
> > moves support for 32-bit DMA addressing into ZONE_DMA and renames
> > CONFIG_ZONE_DMA32 to CONFIG_ZONE_DMA.
> > 
> > Commit 2d5a5612bc (arm64: Limit the CMA buffer to 32-bit if ZONE_DMA)
> > forces the CMA buffer to be 32-bit addressable if CONFIG_ZONE_DMA is
> > defined.
> > 
> > These two patches pose a problem for platforms which have no 32-bit
> > addressable DRAM.
> 
> It's actually the bus/dma address that needs to be 32-bit rather than
> the DRAM as seen by the CPU (which can be beyond 32-bit like the
> Keystone platform).

Ah, right.

> 
> > If CONFIG_ZONE_DMA is turned on for such platforms,
> > CMA is unable to reserve a buffer for allocations. CONFIG_ZONE_DMA is
> > not user settable however, so there is no way to turn it off without
> > editing arch/arm64/Kconfig. Even if one edits Kconfig to turn off
> > CONFIG_ZONE_DMA, the kernel fails to build with such a config:
> > 
> >   arch/arm64/mm/init.c: In function ‘zone_sizes_init’:
> >   arch/arm64/mm/init.c:76:13: error: ‘ZONE_DMA’ undeclared (first use in this function)
> >      zone_size[ZONE_DMA] = max_dma - min;
> >                ^
> > 
> > This patch makes CONFIG_ZONE_DMA user settable and fixes the kernel
> > build when it is turned off.
> 
> The reason I left CONFIG_ZONE_DMA not user settable is because on arm64
> we aim for single Image by default. So ZONE_DMA would most likely be
> needed on at least one platform. Even with ZONE_DMA it's just a guess
> without proper system topology description. dma_to_phys() just takes a
> NULL argument for device when trying to guess the physical address for a
> 32-bit DMA mask (and 32-bit devices may have some physical offset wired
> already).
> 
> With the CMA fix, does the kernel work properly with a zero sized DMA
> zone? Any GFP_DMA allocations will fail, including the swiotlb bounce
> buffer. It may be fine if no devices require 32-bit DMA memory but I
> wonder whether on such platforms it would be better to simply add
> all the memory to ZONE_DMA.

The kernel works, but there are no devices needing GFP_DMA. And yes,
the swiotlb init fails because of no GFP_DMA. But that has always
been the case on platforms with no 32-bit memory.

> 
> Which gets us back to finding a way for describing such system topology
> in hardware. We may be able to find a way with DT but unlikely for ACPI.
> 
> My proposal (in the absence of any kind of description) is to still
> create a ZONE_DMA if we have DMA memory below 32-bit, otherwise just add
> everything (>32-bit) to ZONE_DMA. Basically an extension from your CMA
> patch, make dma_phys_limit static in that file and set it to
> memblock_end_of_DRAM() if no 32-bit DMA. Re-use it in the
> zone_sizes_init() function for ZONE_DMA (maybe with a pr_info for no
> 32-bit only DMA zone).

There's a performance issue with all memory being in ZONE_DMA. It means
all normal allocations will fail on ZONE_NORMAL and then have to fall
back to ZONE_DMA. It would be better to put some percentage of memory
in ZONE_DMA.
Catalin Marinas June 24, 2014, 2:14 p.m. UTC | #4
On Mon, Jun 23, 2014 at 02:17:03PM +0100, Mark Salter wrote:
> On Mon, 2014-06-23 at 12:09 +0100, Catalin Marinas wrote:
> > My proposal (in the absence of any kind of description) is to still
> > create a ZONE_DMA if we have DMA memory below 32-bit, otherwise just add
> > everything (>32-bit) to ZONE_DMA. Basically an extension from your CMA
> > patch, make dma_phys_limit static in that file and set it to
> > memblock_end_of_DRAM() if no 32-bit DMA. Re-use it in the
> > zone_sizes_init() function for ZONE_DMA (maybe with a pr_info for no
> > 32-bit only DMA zone).
> 
> There's a performance issue with all memory being in ZONE_DMA. It means
> all normal allocations will fail on ZONE_NORMAL and then have to fall
> back to ZONE_DMA. It would be better to put some percentage of memory
> in ZONE_DMA.

Is the performance penalty real or just theoretical? I haven't run any
benchmarks myself.
Mark Salter June 24, 2014, 2:38 p.m. UTC | #5
On Tue, 2014-06-24 at 15:14 +0100, Catalin Marinas wrote:
> On Mon, Jun 23, 2014 at 02:17:03PM +0100, Mark Salter wrote:
> > On Mon, 2014-06-23 at 12:09 +0100, Catalin Marinas wrote:
> > > My proposal (in the absence of any kind of description) is to still
> > > create a ZONE_DMA if we have DMA memory below 32-bit, otherwise just add
> > > everything (>32-bit) to ZONE_DMA. Basically an extension from your CMA
> > > patch, make dma_phys_limit static in that file and set it to
> > > memblock_end_of_DRAM() if no 32-bit DMA. Re-use it in the
> > > zone_sizes_init() function for ZONE_DMA (maybe with a pr_info for no
> > > 32-bit only DMA zone).
> > 
> > There's a performance issue with all memory being in ZONE_DMA. It means
> > all normal allocations will fail on ZONE_NORMAL and then have to fall
> > back to ZONE_DMA. It would be better to put some percentage of memory
> > in ZONE_DMA.
> 
> Is the performance penalty real or just theoretical? I haven't run any
> benchmarks myself.
> 
It is real insofar as you must eat cycles eliminating ZONE_NORMAL from
consideration in the page allocation hot path. How much that really
costs, I don't know. But it seems like it could be easily avoided by
limiting ZONE_DMA size. Is there any reason it needs to be larger than
4GiB?
diff mbox

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c8b1d0b..f669597 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -99,9 +99,6 @@  config GENERIC_CSUM
 config GENERIC_CALIBRATE_DELAY
 	def_bool y
 
-config ZONE_DMA
-	def_bool y
-
 config ARCH_DMA_ADDR_T_64BIT
 	def_bool y
 
@@ -211,6 +208,14 @@  config HOTPLUG_CPU
 	  Say Y here to experiment with turning CPUs off and on.  CPUs
 	  can be controlled through /sys/devices/system/cpu.
 
+config ZONE_DMA
+	bool "32-bit DMA memory allocation"
+	def_bool y
+	help
+	  32-bit DMA memory allocation supports devices limited to 32-bit
+	  DMA addressing to allocate within the first 4GiB of address space.
+	  Disable if no such devices will be used.
+
 source kernel/Kconfig.preempt
 
 config HZ
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index f43db8a..c5415e2 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -69,12 +69,14 @@  static void __init zone_sizes_init(unsigned long min, unsigned long max)
 	memset(zone_size, 0, sizeof(zone_size));
 
 	/* 4GB maximum for 32-bit only capable devices */
-	if (IS_ENABLED(CONFIG_ZONE_DMA)) {
+#ifdef CONFIG_ZONE_DMA
+	{
 		unsigned long max_dma_phys =
 			(unsigned long)(dma_to_phys(NULL, DMA_BIT_MASK(32)) + 1);
 		max_dma = max(min, min(max, max_dma_phys >> PAGE_SHIFT));
 		zone_size[ZONE_DMA] = max_dma - min;
 	}
+#endif
 	zone_size[ZONE_NORMAL] = max - max_dma;
 
 	memcpy(zhole_size, zone_size, sizeof(zhole_size));
@@ -86,10 +88,12 @@  static void __init zone_sizes_init(unsigned long min, unsigned long max)
 		if (start >= max)
 			continue;
 
-		if (IS_ENABLED(CONFIG_ZONE_DMA) && start < max_dma) {
+#ifdef CONFIG_ZONE_DMA
+		if (start < max_dma) {
 			unsigned long dma_end = min(end, max_dma);
 			zhole_size[ZONE_DMA] -= dma_end - start;
 		}
+#endif
 
 		if (end > max_dma) {
 			unsigned long normal_end = min(end, max);