All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] mm: memblock Add some new functions to address the mem limit issue
@ 2016-06-24  3:13 ` Dennis Chen
  0 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-24  3:13 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: nd, Dennis Chen, Catalin Marinas, Steve Capper, Ard Biesheuvel,
	Will Deacon, Mark Rutland, Rafael J . Wysocki, Matt Fleming,
	linux-mm, linux-acpi, linux-efi

In some cases, memblock is queried to determine whether a physical
address corresponds to memory present in a system even if unused by
the OS for the linear mapping, highmem, etc. For example, the ACPI
core needs this information to determine which attributes to use when
mapping ACPI regions. Use of incorrect memory types can result in
faults, data corruption, or other issues.

Removing memory with memblock_enforce_memory_limit throws away this
information, and so a kernel booted with 'mem=' may suffer from the
issues described above. To avoid this, we can mark regions as nomap
rather than removing them, which preserves the information we need
while preventing other use of the regions.

This patch adds new infrastructure to mark all memblock regions in
an address range as nomap, to cater for this. Similarly we add
infrastructure to clear the flag for an address range, which makes
handling some overlap cases simpler.

At last, we add 'size' and 'flag' debug output in the memblock debugfs
for ease of the memblock debug effort.
The '/sys/kernel/debug/memblock/memory' output looks like before:
   0: 0x0000008000000000..0x0000008001e7ffff
   1: 0x0000008001e80000..0x00000083ff184fff
   2: 0x00000083ff185000..0x00000083ff1c2fff
   3: 0x00000083ff1c3000..0x00000083ff222fff
   4: 0x00000083ff223000..0x00000083ffe42fff
   5: 0x00000083ffe43000..0x00000083ffffffff

After applied:
   0: 0x0000008000000000..0x0000008001e7ffff  0x0000000001e80000  0x4
   1: 0x0000008001e80000..0x00000083ff184fff  0x00000003fd305000  0x0
   2: 0x00000083ff185000..0x00000083ff1c2fff  0x000000000003e000  0x4
   3: 0x00000083ff1c3000..0x00000083ff222fff  0x0000000000060000  0x0
   4: 0x00000083ff223000..0x00000083ffe42fff  0x0000000000c20000  0x4
   5: 0x00000083ffe43000..0x00000083ffffffff  0x00000000001bd000  0x0

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Steve Capper <steve.capper@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: linux-mm@kvack.org
Cc: linux-acpi@vger.kernel.org
Cc: linux-efi@vger.kernel.org
---
Changes in v2:
Update the commit message and add __init_memblock prefix to __find_max_addr()
to avoid possible build warnning.


 include/linux/memblock.h |  2 ++
 mm/memblock.c            | 50 ++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 6c14b61..5e069c8 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -92,6 +92,7 @@ int memblock_mark_hotplug(phys_addr_t base, phys_addr_t size);
 int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
 int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
 int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
+int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
 ulong choose_memblock_flags(void);
 
 /* Low level functions */
@@ -332,6 +333,7 @@ phys_addr_t memblock_mem_size(unsigned long limit_pfn);
 phys_addr_t memblock_start_of_DRAM(void);
 phys_addr_t memblock_end_of_DRAM(void);
 void memblock_enforce_memory_limit(phys_addr_t memory_limit);
+void memblock_mem_limit_mark_nomap(phys_addr_t limit);
 bool memblock_is_memory(phys_addr_t addr);
 int memblock_is_map_memory(phys_addr_t addr);
 int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
diff --git a/mm/memblock.c b/mm/memblock.c
index ca09915..795596d 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -814,6 +814,18 @@ int __init_memblock memblock_mark_nomap(phys_addr_t base, phys_addr_t size)
 }
 
 /**
+ * memblock_clear_nomap - Clear flag MEMBLOCK_NOMAP for a specified region.
+ * @base: the base phys addr of the region
+ * @size: the size of the region
+ *
+ * Return 0 on success, -errno on failure.
+ */
+int __init_memblock memblock_clear_nomap(phys_addr_t base, phys_addr_t size)
+{
+	return memblock_setclr_flag(base, size, 0, MEMBLOCK_NOMAP);
+}
+
+/**
  * __next_reserved_mem_region - next function for for_each_reserved_region()
  * @idx: pointer to u64 loop variable
  * @out_start: ptr to phys_addr_t for start address of the region, can be %NULL
@@ -1465,14 +1477,11 @@ phys_addr_t __init_memblock memblock_end_of_DRAM(void)
 	return (memblock.memory.regions[idx].base + memblock.memory.regions[idx].size);
 }
 
-void __init memblock_enforce_memory_limit(phys_addr_t limit)
+static phys_addr_t __init_memblock __find_max_addr(phys_addr_t limit)
 {
 	phys_addr_t max_addr = (phys_addr_t)ULLONG_MAX;
 	struct memblock_region *r;
 
-	if (!limit)
-		return;
-
 	/* find out max address */
 	for_each_memblock(memory, r) {
 		if (limit <= r->size) {
@@ -1482,6 +1491,18 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
 		limit -= r->size;
 	}
 
+	return max_addr;
+}
+
+void __init memblock_enforce_memory_limit(phys_addr_t limit)
+{
+	phys_addr_t max_addr;
+
+	if (!limit)
+		return;
+
+	max_addr = __find_max_addr(limit);
+
 	/* truncate both memory and reserved regions */
 	memblock_remove_range(&memblock.memory, max_addr,
 			      (phys_addr_t)ULLONG_MAX);
@@ -1489,6 +1510,17 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
 			      (phys_addr_t)ULLONG_MAX);
 }
 
+void __init memblock_mem_limit_mark_nomap(phys_addr_t limit)
+{
+	phys_addr_t max_addr;
+
+	if (!limit)
+		return;
+
+	max_addr = __find_max_addr(limit);
+	memblock_mark_nomap(max_addr, (phys_addr_t)ULLONG_MAX);
+}
+
 static int __init_memblock memblock_search(struct memblock_type *type, phys_addr_t addr)
 {
 	unsigned int left = 0, right = type->cnt;
@@ -1677,13 +1709,15 @@ static int memblock_debug_show(struct seq_file *m, void *private)
 		reg = &type->regions[i];
 		seq_printf(m, "%4d: ", i);
 		if (sizeof(phys_addr_t) == 4)
-			seq_printf(m, "0x%08lx..0x%08lx\n",
+			seq_printf(m, "0x%08lx..0x%08lx  0x%08lx  0x%lx\n",
 				   (unsigned long)reg->base,
-				   (unsigned long)(reg->base + reg->size - 1));
+				   (unsigned long)(reg->base + reg->size - 1),
+				   (unsigned long)reg->size, reg->flags);
 		else
-			seq_printf(m, "0x%016llx..0x%016llx\n",
+			seq_printf(m, "0x%016llx..0x%016llx  0x%016llx  0x%lx\n",
 				   (unsigned long long)reg->base,
-				   (unsigned long long)(reg->base + reg->size - 1));
+				   (unsigned long long)(reg->base + reg->size - 1),
+				   (unsigned long long)reg->size, reg->flags);
 
 	}
 	return 0;
-- 
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 1/2] mm: memblock Add some new functions to address the mem limit issue
@ 2016-06-24  3:13 ` Dennis Chen
  0 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-24  3:13 UTC (permalink / raw)
  To: linux-arm-kernel

In some cases, memblock is queried to determine whether a physical
address corresponds to memory present in a system even if unused by
the OS for the linear mapping, highmem, etc. For example, the ACPI
core needs this information to determine which attributes to use when
mapping ACPI regions. Use of incorrect memory types can result in
faults, data corruption, or other issues.

Removing memory with memblock_enforce_memory_limit throws away this
information, and so a kernel booted with 'mem=' may suffer from the
issues described above. To avoid this, we can mark regions as nomap
rather than removing them, which preserves the information we need
while preventing other use of the regions.

This patch adds new infrastructure to mark all memblock regions in
an address range as nomap, to cater for this. Similarly we add
infrastructure to clear the flag for an address range, which makes
handling some overlap cases simpler.

At last, we add 'size' and 'flag' debug output in the memblock debugfs
for ease of the memblock debug effort.
The '/sys/kernel/debug/memblock/memory' output looks like before:
   0: 0x0000008000000000..0x0000008001e7ffff
   1: 0x0000008001e80000..0x00000083ff184fff
   2: 0x00000083ff185000..0x00000083ff1c2fff
   3: 0x00000083ff1c3000..0x00000083ff222fff
   4: 0x00000083ff223000..0x00000083ffe42fff
   5: 0x00000083ffe43000..0x00000083ffffffff

After applied:
   0: 0x0000008000000000..0x0000008001e7ffff  0x0000000001e80000  0x4
   1: 0x0000008001e80000..0x00000083ff184fff  0x00000003fd305000  0x0
   2: 0x00000083ff185000..0x00000083ff1c2fff  0x000000000003e000  0x4
   3: 0x00000083ff1c3000..0x00000083ff222fff  0x0000000000060000  0x0
   4: 0x00000083ff223000..0x00000083ffe42fff  0x0000000000c20000  0x4
   5: 0x00000083ffe43000..0x00000083ffffffff  0x00000000001bd000  0x0

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Steve Capper <steve.capper@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: linux-mm at kvack.org
Cc: linux-acpi at vger.kernel.org
Cc: linux-efi at vger.kernel.org
---
Changes in v2:
Update the commit message and add __init_memblock prefix to __find_max_addr()
to avoid possible build warnning.


 include/linux/memblock.h |  2 ++
 mm/memblock.c            | 50 ++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 6c14b61..5e069c8 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -92,6 +92,7 @@ int memblock_mark_hotplug(phys_addr_t base, phys_addr_t size);
 int memblock_clear_hotplug(phys_addr_t base, phys_addr_t size);
 int memblock_mark_mirror(phys_addr_t base, phys_addr_t size);
 int memblock_mark_nomap(phys_addr_t base, phys_addr_t size);
+int memblock_clear_nomap(phys_addr_t base, phys_addr_t size);
 ulong choose_memblock_flags(void);
 
 /* Low level functions */
@@ -332,6 +333,7 @@ phys_addr_t memblock_mem_size(unsigned long limit_pfn);
 phys_addr_t memblock_start_of_DRAM(void);
 phys_addr_t memblock_end_of_DRAM(void);
 void memblock_enforce_memory_limit(phys_addr_t memory_limit);
+void memblock_mem_limit_mark_nomap(phys_addr_t limit);
 bool memblock_is_memory(phys_addr_t addr);
 int memblock_is_map_memory(phys_addr_t addr);
 int memblock_is_region_memory(phys_addr_t base, phys_addr_t size);
diff --git a/mm/memblock.c b/mm/memblock.c
index ca09915..795596d 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -814,6 +814,18 @@ int __init_memblock memblock_mark_nomap(phys_addr_t base, phys_addr_t size)
 }
 
 /**
+ * memblock_clear_nomap - Clear flag MEMBLOCK_NOMAP for a specified region.
+ * @base: the base phys addr of the region
+ * @size: the size of the region
+ *
+ * Return 0 on success, -errno on failure.
+ */
+int __init_memblock memblock_clear_nomap(phys_addr_t base, phys_addr_t size)
+{
+	return memblock_setclr_flag(base, size, 0, MEMBLOCK_NOMAP);
+}
+
+/**
  * __next_reserved_mem_region - next function for for_each_reserved_region()
  * @idx: pointer to u64 loop variable
  * @out_start: ptr to phys_addr_t for start address of the region, can be %NULL
@@ -1465,14 +1477,11 @@ phys_addr_t __init_memblock memblock_end_of_DRAM(void)
 	return (memblock.memory.regions[idx].base + memblock.memory.regions[idx].size);
 }
 
-void __init memblock_enforce_memory_limit(phys_addr_t limit)
+static phys_addr_t __init_memblock __find_max_addr(phys_addr_t limit)
 {
 	phys_addr_t max_addr = (phys_addr_t)ULLONG_MAX;
 	struct memblock_region *r;
 
-	if (!limit)
-		return;
-
 	/* find out max address */
 	for_each_memblock(memory, r) {
 		if (limit <= r->size) {
@@ -1482,6 +1491,18 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
 		limit -= r->size;
 	}
 
+	return max_addr;
+}
+
+void __init memblock_enforce_memory_limit(phys_addr_t limit)
+{
+	phys_addr_t max_addr;
+
+	if (!limit)
+		return;
+
+	max_addr = __find_max_addr(limit);
+
 	/* truncate both memory and reserved regions */
 	memblock_remove_range(&memblock.memory, max_addr,
 			      (phys_addr_t)ULLONG_MAX);
@@ -1489,6 +1510,17 @@ void __init memblock_enforce_memory_limit(phys_addr_t limit)
 			      (phys_addr_t)ULLONG_MAX);
 }
 
+void __init memblock_mem_limit_mark_nomap(phys_addr_t limit)
+{
+	phys_addr_t max_addr;
+
+	if (!limit)
+		return;
+
+	max_addr = __find_max_addr(limit);
+	memblock_mark_nomap(max_addr, (phys_addr_t)ULLONG_MAX);
+}
+
 static int __init_memblock memblock_search(struct memblock_type *type, phys_addr_t addr)
 {
 	unsigned int left = 0, right = type->cnt;
@@ -1677,13 +1709,15 @@ static int memblock_debug_show(struct seq_file *m, void *private)
 		reg = &type->regions[i];
 		seq_printf(m, "%4d: ", i);
 		if (sizeof(phys_addr_t) == 4)
-			seq_printf(m, "0x%08lx..0x%08lx\n",
+			seq_printf(m, "0x%08lx..0x%08lx  0x%08lx  0x%lx\n",
 				   (unsigned long)reg->base,
-				   (unsigned long)(reg->base + reg->size - 1));
+				   (unsigned long)(reg->base + reg->size - 1),
+				   (unsigned long)reg->size, reg->flags);
 		else
-			seq_printf(m, "0x%016llx..0x%016llx\n",
+			seq_printf(m, "0x%016llx..0x%016llx  0x%016llx  0x%lx\n",
 				   (unsigned long long)reg->base,
-				   (unsigned long long)(reg->base + reg->size - 1));
+				   (unsigned long long)(reg->base + reg->size - 1),
+				   (unsigned long long)reg->size, reg->flags);
 
 	}
 	return 0;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
  2016-06-24  3:13 ` Dennis Chen
  (?)
@ 2016-06-24  3:13     ` Dennis Chen
  -1 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-24  3:13 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: nd-5wv7dgnIgG8, Dennis Chen, Catalin Marinas, Steve Capper,
	Ard Biesheuvel, Will Deacon, Mark Rutland, Rafael J . Wysocki,
	Matt Fleming, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA

When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
regions loaded by firmware will beyond the limit of the memory, in this
case we need to nomap the region above the limit while not removing
it from memblock, because once region removed from memblock, the ACPI
will think that region is not a normal memory and map it as device type
memory accordingly. Since the ACPI core will produce non-alignment access
when paring AML data stream, hence result in alignment fault upon the io
mapped memory space.

For example, below is an alignment exception observed on softIron board
when booting the kernel with 'acpi=force mem=8G':
...
[    0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
[    0.550457] pgd = ffff000008aa0000
[    0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
[    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
[    0.570553] Modules linked in:
[    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
[    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
[    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
[    0.597571] PC is at acpi_ns_lookup+0x520/0x734
[    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
[    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
[    0.614145] sp : ffff800001efb8b0
[    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
[    0.622829] x27: 0000000000000001 x26: 0000000000000000
[    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
[    0.633531] x23: 0000000000000001 x22: 0000000000000001
[    0.638881] x21: ffff000008724000 x20: 000000000000001b
[    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
[    0.649580] x17: 00000000000038ff x16: 0000000000000002
[    0.654929] x15: 0000000000000007 x14: 0000000000007fff
[    0.660278] x13: ffffff0000000000 x12: 0000000000000018
[    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
[    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
[    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
[    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
[    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
[    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
...
[    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
[    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
[    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
[    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
[    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
[    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
[    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
[    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
[    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
[    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
[    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
[    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
[    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
[    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
[    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
[    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
[    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
[    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
[    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

With 'efi=debug', we can see those ACPI regions loaded by firmware on
that board as:
[    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
[    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*

This patch is trying to address the above issues by nomaping the region
instead of removing it.

Signed-off-by: Dennis Chen <dennis.chen-5wv7dgnIgG8@public.gmane.org>
Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
Cc: Steve Capper <steve.capper-5wv7dgnIgG8@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org
Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
Changes in v2:
Update the commit message and remove the memblock_is_map_memory() check
according to the suggestion from Mark Rutland.

 arch/arm64/mm/init.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index d45f862..6af2456 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
 
 	/*
 	 * Apply the memory limit if it was set. Since the kernel may be loaded
-	 * high up in memory, add back the kernel region that must be accessible
-	 * via the linear mapping.
+	 * in the memory regions above the limit, so we need to clear the
+	 * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
+	 * the linear mapping.
 	 */
 	if (memory_limit != (phys_addr_t)ULLONG_MAX) {
-		memblock_enforce_memory_limit(memory_limit);
-		memblock_add(__pa(_text), (u64)(_end - _text));
+		memblock_mem_limit_mark_nomap(memory_limit);
+		memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
 	}
 
 	if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && initrd_start) {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-24  3:13     ` Dennis Chen
  0 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-24  3:13 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: nd, Dennis Chen, Catalin Marinas, Steve Capper, Ard Biesheuvel,
	Will Deacon, Mark Rutland, Rafael J . Wysocki, Matt Fleming,
	linux-mm, linux-acpi, linux-efi

When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
regions loaded by firmware will beyond the limit of the memory, in this
case we need to nomap the region above the limit while not removing
it from memblock, because once region removed from memblock, the ACPI
will think that region is not a normal memory and map it as device type
memory accordingly. Since the ACPI core will produce non-alignment access
when paring AML data stream, hence result in alignment fault upon the io
mapped memory space.

For example, below is an alignment exception observed on softIron board
when booting the kernel with 'acpi=force mem=8G':
...
[    0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
[    0.550457] pgd = ffff000008aa0000
[    0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
[    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
[    0.570553] Modules linked in:
[    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
[    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
[    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
[    0.597571] PC is at acpi_ns_lookup+0x520/0x734
[    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
[    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
[    0.614145] sp : ffff800001efb8b0
[    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
[    0.622829] x27: 0000000000000001 x26: 0000000000000000
[    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
[    0.633531] x23: 0000000000000001 x22: 0000000000000001
[    0.638881] x21: ffff000008724000 x20: 000000000000001b
[    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
[    0.649580] x17: 00000000000038ff x16: 0000000000000002
[    0.654929] x15: 0000000000000007 x14: 0000000000007fff
[    0.660278] x13: ffffff0000000000 x12: 0000000000000018
[    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
[    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
[    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
[    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
[    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
[    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
...
[    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
[    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
[    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
[    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
[    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
[    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
[    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
[    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
[    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
[    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
[    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
[    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
[    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
[    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
[    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
[    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
[    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
[    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
[    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

With 'efi=debug', we can see those ACPI regions loaded by firmware on
that board as:
[    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
[    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*

This patch is trying to address the above issues by nomaping the region
instead of removing it.

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Steve Capper <steve.capper@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: linux-mm@kvack.org
Cc: linux-acpi@vger.kernel.org
Cc: linux-efi@vger.kernel.org
---
Changes in v2:
Update the commit message and remove the memblock_is_map_memory() check
according to the suggestion from Mark Rutland.

 arch/arm64/mm/init.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index d45f862..6af2456 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
 
 	/*
 	 * Apply the memory limit if it was set. Since the kernel may be loaded
-	 * high up in memory, add back the kernel region that must be accessible
-	 * via the linear mapping.
+	 * in the memory regions above the limit, so we need to clear the
+	 * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
+	 * the linear mapping.
 	 */
 	if (memory_limit != (phys_addr_t)ULLONG_MAX) {
-		memblock_enforce_memory_limit(memory_limit);
-		memblock_add(__pa(_text), (u64)(_end - _text));
+		memblock_mem_limit_mark_nomap(memory_limit);
+		memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
 	}
 
 	if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && initrd_start) {
-- 
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-24  3:13     ` Dennis Chen
  0 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-24  3:13 UTC (permalink / raw)
  To: linux-arm-kernel

When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
regions loaded by firmware will beyond the limit of the memory, in this
case we need to nomap the region above the limit while not removing
it from memblock, because once region removed from memblock, the ACPI
will think that region is not a normal memory and map it as device type
memory accordingly. Since the ACPI core will produce non-alignment access
when paring AML data stream, hence result in alignment fault upon the io
mapped memory space.

For example, below is an alignment exception observed on softIron board
when booting the kernel with 'acpi=force mem=8G':
...
[    0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
[    0.550457] pgd = ffff000008aa0000
[    0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
[    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
[    0.570553] Modules linked in:
[    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
[    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
[    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
[    0.597571] PC is at acpi_ns_lookup+0x520/0x734
[    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
[    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
[    0.614145] sp : ffff800001efb8b0
[    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
[    0.622829] x27: 0000000000000001 x26: 0000000000000000
[    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
[    0.633531] x23: 0000000000000001 x22: 0000000000000001
[    0.638881] x21: ffff000008724000 x20: 000000000000001b
[    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
[    0.649580] x17: 00000000000038ff x16: 0000000000000002
[    0.654929] x15: 0000000000000007 x14: 0000000000007fff
[    0.660278] x13: ffffff0000000000 x12: 0000000000000018
[    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
[    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
[    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
[    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
[    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
[    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
...
[    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
[    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
[    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
[    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
[    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
[    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
[    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
[    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
[    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
[    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
[    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
[    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
[    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
[    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
[    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
[    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
[    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
[    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
[    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

With 'efi=debug', we can see those ACPI regions loaded by firmware on
that board as:
[    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
[    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*

This patch is trying to address the above issues by nomaping the region
instead of removing it.

Signed-off-by: Dennis Chen <dennis.chen@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Steve Capper <steve.capper@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: linux-mm at kvack.org
Cc: linux-acpi at vger.kernel.org
Cc: linux-efi at vger.kernel.org
---
Changes in v2:
Update the commit message and remove the memblock_is_map_memory() check
according to the suggestion from Mark Rutland.

 arch/arm64/mm/init.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index d45f862..6af2456 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
 
 	/*
 	 * Apply the memory limit if it was set. Since the kernel may be loaded
-	 * high up in memory, add back the kernel region that must be accessible
-	 * via the linear mapping.
+	 * in the memory regions above the limit, so we need to clear the
+	 * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
+	 * the linear mapping.
 	 */
 	if (memory_limit != (phys_addr_t)ULLONG_MAX) {
-		memblock_enforce_memory_limit(memory_limit);
-		memblock_add(__pa(_text), (u64)(_end - _text));
+		memblock_mem_limit_mark_nomap(memory_limit);
+		memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
 	}
 
 	if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && initrd_start) {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
  2016-06-24  3:13     ` Dennis Chen
@ 2016-06-24 10:43       ` Ard Biesheuvel
  -1 siblings, 0 replies; 21+ messages in thread
From: Ard Biesheuvel @ 2016-06-24 10:43 UTC (permalink / raw)
  To: Dennis Chen
  Cc: linux-arm-kernel, nd, Catalin Marinas, Steve Capper, Will Deacon,
	Mark Rutland, Rafael J . Wysocki, Matt Fleming, linux-mm,
	linux-acpi, linux-efi

On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
> When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
> regions loaded by firmware will beyond the limit of the memory, in this
> case we need to nomap the region above the limit while not removing
> it from memblock, because once region removed from memblock, the ACPI
> will think that region is not a normal memory and map it as device type
> memory accordingly. Since the ACPI core will produce non-alignment access
> when paring AML data stream, hence result in alignment fault upon the io
> mapped memory space.
>
> For example, below is an alignment exception observed on softIron board
> when booting the kernel with 'acpi=force mem=8G':
> ...
> [ 0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
> [ 0.550457] pgd = ffff000008aa0000
> [ 0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
> [    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
> [    0.570553] Modules linked in:
> [    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
> [    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
> [    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
> [    0.597571] PC is at acpi_ns_lookup+0x520/0x734
> [    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
> [    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
> [    0.614145] sp : ffff800001efb8b0
> [    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
> [    0.622829] x27: 0000000000000001 x26: 0000000000000000
> [    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
> [    0.633531] x23: 0000000000000001 x22: 0000000000000001
> [    0.638881] x21: ffff000008724000 x20: 000000000000001b
> [    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
> [    0.649580] x17: 00000000000038ff x16: 0000000000000002
> [    0.654929] x15: 0000000000000007 x14: 0000000000007fff
> [    0.660278] x13: ffffff0000000000 x12: 0000000000000018
> [    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
> [    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
> [    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
> [    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
> [    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
> [    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
> ...
> [    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
> [    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
> [    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
> [    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
> [    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
> [    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
> [    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
> [    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
> [    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
> [    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
> [    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
> [    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
> [    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
> [    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
> [    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
> [    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
> [    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
> [    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
> [    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
> With 'efi=debug', we can see those ACPI regions loaded by firmware on
> that board as:
> [    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> [    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
>
> This patch is trying to address the above issues by nomaping the region
> instead of removing it.
>
> Signed-off-by: Dennis Chen <dennis.chen@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Steve Capper <steve.capper@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: linux-mm@kvack.org
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-efi@vger.kernel.org
> ---
> Changes in v2:
> Update the commit message and remove the memblock_is_map_memory() check
> according to the suggestion from Mark Rutland.
>
>  arch/arm64/mm/init.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index d45f862..6af2456 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
>
>         /*
>          * Apply the memory limit if it was set. Since the kernel may be loaded
> -        * high up in memory, add back the kernel region that must be accessible
> -        * via the linear mapping.
> +        * in the memory regions above the limit, so we need to clear the
> +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> +        * the linear mapping.
>          */
>         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> -               memblock_enforce_memory_limit(memory_limit);
> -               memblock_add(__pa(_text), (u64)(_end - _text));
> +               memblock_mem_limit_mark_nomap(memory_limit);
> +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));

Up until now, we have ignored the effect of having NOMAP memblocks on
the return values of functions like memblock_phys_mem_size() and
memblock_mem_size(), since they could reasonably be expected to cover
only a small slice of all available memory. However, after applying
this patch, it may well be the case that most of memory is marked
NOMAP, and these functions will cease to work as expected.

This means NOMAP is really only suited to punch some holes into the
kernel direct mapping, and so implementing the memory limit by marking
everything NOMAP is not the way to go. Instead, we should probably
reorder the init sequence so that the regions that are reserved in the
UEFI memory map are declared and marked NOMAP [again] after applying
the memory limit in the old way.

-- 
Ard.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-24 10:43       ` Ard Biesheuvel
  0 siblings, 0 replies; 21+ messages in thread
From: Ard Biesheuvel @ 2016-06-24 10:43 UTC (permalink / raw)
  To: linux-arm-kernel

On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
> When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
> regions loaded by firmware will beyond the limit of the memory, in this
> case we need to nomap the region above the limit while not removing
> it from memblock, because once region removed from memblock, the ACPI
> will think that region is not a normal memory and map it as device type
> memory accordingly. Since the ACPI core will produce non-alignment access
> when paring AML data stream, hence result in alignment fault upon the io
> mapped memory space.
>
> For example, below is an alignment exception observed on softIron board
> when booting the kernel with 'acpi=force mem=8G':
> ...
> [ 0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
> [ 0.550457] pgd = ffff000008aa0000
> [ 0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
> [    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
> [    0.570553] Modules linked in:
> [    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
> [    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
> [    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
> [    0.597571] PC is at acpi_ns_lookup+0x520/0x734
> [    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
> [    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
> [    0.614145] sp : ffff800001efb8b0
> [    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
> [    0.622829] x27: 0000000000000001 x26: 0000000000000000
> [    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
> [    0.633531] x23: 0000000000000001 x22: 0000000000000001
> [    0.638881] x21: ffff000008724000 x20: 000000000000001b
> [    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
> [    0.649580] x17: 00000000000038ff x16: 0000000000000002
> [    0.654929] x15: 0000000000000007 x14: 0000000000007fff
> [    0.660278] x13: ffffff0000000000 x12: 0000000000000018
> [    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
> [    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
> [    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
> [    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
> [    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
> [    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
> ...
> [    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
> [    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
> [    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
> [    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
> [    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
> [    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
> [    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
> [    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
> [    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
> [    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
> [    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
> [    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
> [    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
> [    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
> [    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
> [    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
> [    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
> [    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
> [    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
> With 'efi=debug', we can see those ACPI regions loaded by firmware on
> that board as:
> [    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> [    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
>
> This patch is trying to address the above issues by nomaping the region
> instead of removing it.
>
> Signed-off-by: Dennis Chen <dennis.chen@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Steve Capper <steve.capper@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: linux-mm at kvack.org
> Cc: linux-acpi at vger.kernel.org
> Cc: linux-efi at vger.kernel.org
> ---
> Changes in v2:
> Update the commit message and remove the memblock_is_map_memory() check
> according to the suggestion from Mark Rutland.
>
>  arch/arm64/mm/init.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index d45f862..6af2456 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
>
>         /*
>          * Apply the memory limit if it was set. Since the kernel may be loaded
> -        * high up in memory, add back the kernel region that must be accessible
> -        * via the linear mapping.
> +        * in the memory regions above the limit, so we need to clear the
> +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> +        * the linear mapping.
>          */
>         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> -               memblock_enforce_memory_limit(memory_limit);
> -               memblock_add(__pa(_text), (u64)(_end - _text));
> +               memblock_mem_limit_mark_nomap(memory_limit);
> +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));

Up until now, we have ignored the effect of having NOMAP memblocks on
the return values of functions like memblock_phys_mem_size() and
memblock_mem_size(), since they could reasonably be expected to cover
only a small slice of all available memory. However, after applying
this patch, it may well be the case that most of memory is marked
NOMAP, and these functions will cease to work as expected.

This means NOMAP is really only suited to punch some holes into the
kernel direct mapping, and so implementing the memory limit by marking
everything NOMAP is not the way to go. Instead, we should probably
reorder the init sequence so that the regions that are reserved in the
UEFI memory map are declared and marked NOMAP [again] after applying
the memory limit in the old way.

-- 
Ard.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
  2016-06-24 10:43       ` Ard Biesheuvel
  (?)
@ 2016-06-24 12:01           ` Dennis Chen
  -1 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-24 12:01 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	nd-5wv7dgnIgG8, Catalin Marinas, Steve Capper, Will Deacon,
	Mark Rutland, Rafael J . Wysocki, Matt Fleming,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA

On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen-5wv7dgnIgG8@public.gmane.org> wrote:
> > When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
> > regions loaded by firmware will beyond the limit of the memory, in this
> > case we need to nomap the region above the limit while not removing
> > it from memblock, because once region removed from memblock, the ACPI
> > will think that region is not a normal memory and map it as device type
> > memory accordingly. Since the ACPI core will produce non-alignment access
> > when paring AML data stream, hence result in alignment fault upon the io
> > mapped memory space.
> >
> > For example, below is an alignment exception observed on softIron board
> > when booting the kernel with 'acpi=force mem=8G':
> > ...
> > [ 0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
> > [ 0.550457] pgd = ffff000008aa0000
> > [ 0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
> > [    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
> > [    0.570553] Modules linked in:
> > [    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
> > [    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
> > [    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
> > [    0.597571] PC is at acpi_ns_lookup+0x520/0x734
> > [    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
> > [    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
> > [    0.614145] sp : ffff800001efb8b0
> > [    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
> > [    0.622829] x27: 0000000000000001 x26: 0000000000000000
> > [    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
> > [    0.633531] x23: 0000000000000001 x22: 0000000000000001
> > [    0.638881] x21: ffff000008724000 x20: 000000000000001b
> > [    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
> > [    0.649580] x17: 00000000000038ff x16: 0000000000000002
> > [    0.654929] x15: 0000000000000007 x14: 0000000000007fff
> > [    0.660278] x13: ffffff0000000000 x12: 0000000000000018
> > [    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
> > [    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
> > [    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
> > [    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
> > [    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
> > [    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
> > ...
> > [    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
> > [    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
> > [    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
> > [    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
> > [    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
> > [    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
> > [    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
> > [    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
> > [    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
> > [    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
> > [    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
> > [    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
> > [    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
> > [    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
> > [    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
> > [    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
> > [    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
> > [    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
> > [    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >
> > With 'efi=debug', we can see those ACPI regions loaded by firmware on
> > that board as:
> > [    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> > [    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> >
> > This patch is trying to address the above issues by nomaping the region
> > instead of removing it.
> >
> > Signed-off-by: Dennis Chen <dennis.chen-5wv7dgnIgG8@public.gmane.org>
> > Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> > Cc: Steve Capper <steve.capper-5wv7dgnIgG8@public.gmane.org>
> > Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> > Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org
> > Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > ---
> > Changes in v2:
> > Update the commit message and remove the memblock_is_map_memory() check
> > according to the suggestion from Mark Rutland.
> >
> >  arch/arm64/mm/init.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index d45f862..6af2456 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
> >
> >         /*
> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> > -        * high up in memory, add back the kernel region that must be accessible
> > -        * via the linear mapping.
> > +        * in the memory regions above the limit, so we need to clear the
> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> > +        * the linear mapping.
> >          */
> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> > -               memblock_enforce_memory_limit(memory_limit);
> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> > +               memblock_mem_limit_mark_nomap(memory_limit);
> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> 
> Up until now, we have ignored the effect of having NOMAP memblocks on
> the return values of functions like memblock_phys_mem_size() and
> memblock_mem_size(), since they could reasonably be expected to cover
> only a small slice of all available memory. However, after applying
> this patch, it may well be the case that most of memory is marked
> NOMAP, and these functions will cease to work as expected.
>
Hi Ard, I noticed these inconsistences as you mentioned, but seems the
available memory is limited correctly. For this case('mem='), will it bring
some substantive side effects except that some log messages maybe confusing?  
> 
> This means NOMAP is really only suited to punch some holes into the
> kernel direct mapping, and so implementing the memory limit by marking
> everything NOMAP is not the way to go. Instead, we should probably
> reorder the init sequence so that the regions that are reserved in the
> UEFI memory map are declared and marked NOMAP [again] after applying
> the memory limit in the old way.
>
Before this patch, I have another one addressing the same issue [1], with
that patch we'll not have these inconsistences, but it looks like a little
bit complicated, so it becomes current one. Any comments about that?

[1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html

Thanks,
Dennis
> 
> -- 
> Ard.
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-24 12:01           ` Dennis Chen
  0 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-24 12:01 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, nd, Catalin Marinas, Steve Capper, Will Deacon,
	Mark Rutland, Rafael J . Wysocki, Matt Fleming, linux-mm,
	linux-acpi, linux-efi

On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
> > When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
> > regions loaded by firmware will beyond the limit of the memory, in this
> > case we need to nomap the region above the limit while not removing
> > it from memblock, because once region removed from memblock, the ACPI
> > will think that region is not a normal memory and map it as device type
> > memory accordingly. Since the ACPI core will produce non-alignment access
> > when paring AML data stream, hence result in alignment fault upon the io
> > mapped memory space.
> >
> > For example, below is an alignment exception observed on softIron board
> > when booting the kernel with 'acpi=force mem=8G':
> > ...
> > [ 0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
> > [ 0.550457] pgd = ffff000008aa0000
> > [ 0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
> > [    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
> > [    0.570553] Modules linked in:
> > [    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
> > [    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
> > [    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
> > [    0.597571] PC is at acpi_ns_lookup+0x520/0x734
> > [    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
> > [    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
> > [    0.614145] sp : ffff800001efb8b0
> > [    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
> > [    0.622829] x27: 0000000000000001 x26: 0000000000000000
> > [    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
> > [    0.633531] x23: 0000000000000001 x22: 0000000000000001
> > [    0.638881] x21: ffff000008724000 x20: 000000000000001b
> > [    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
> > [    0.649580] x17: 00000000000038ff x16: 0000000000000002
> > [    0.654929] x15: 0000000000000007 x14: 0000000000007fff
> > [    0.660278] x13: ffffff0000000000 x12: 0000000000000018
> > [    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
> > [    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
> > [    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
> > [    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
> > [    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
> > [    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
> > ...
> > [    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
> > [    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
> > [    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
> > [    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
> > [    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
> > [    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
> > [    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
> > [    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
> > [    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
> > [    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
> > [    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
> > [    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
> > [    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
> > [    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
> > [    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
> > [    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
> > [    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
> > [    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
> > [    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >
> > With 'efi=debug', we can see those ACPI regions loaded by firmware on
> > that board as:
> > [    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> > [    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> >
> > This patch is trying to address the above issues by nomaping the region
> > instead of removing it.
> >
> > Signed-off-by: Dennis Chen <dennis.chen@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Steve Capper <steve.capper@arm.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> > Cc: linux-mm@kvack.org
> > Cc: linux-acpi@vger.kernel.org
> > Cc: linux-efi@vger.kernel.org
> > ---
> > Changes in v2:
> > Update the commit message and remove the memblock_is_map_memory() check
> > according to the suggestion from Mark Rutland.
> >
> >  arch/arm64/mm/init.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index d45f862..6af2456 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
> >
> >         /*
> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> > -        * high up in memory, add back the kernel region that must be accessible
> > -        * via the linear mapping.
> > +        * in the memory regions above the limit, so we need to clear the
> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> > +        * the linear mapping.
> >          */
> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> > -               memblock_enforce_memory_limit(memory_limit);
> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> > +               memblock_mem_limit_mark_nomap(memory_limit);
> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> 
> Up until now, we have ignored the effect of having NOMAP memblocks on
> the return values of functions like memblock_phys_mem_size() and
> memblock_mem_size(), since they could reasonably be expected to cover
> only a small slice of all available memory. However, after applying
> this patch, it may well be the case that most of memory is marked
> NOMAP, and these functions will cease to work as expected.
>
Hi Ard, I noticed these inconsistences as you mentioned, but seems the
available memory is limited correctly. For this case('mem='), will it bring
some substantive side effects except that some log messages maybe confusing?  
> 
> This means NOMAP is really only suited to punch some holes into the
> kernel direct mapping, and so implementing the memory limit by marking
> everything NOMAP is not the way to go. Instead, we should probably
> reorder the init sequence so that the regions that are reserved in the
> UEFI memory map are declared and marked NOMAP [again] after applying
> the memory limit in the old way.
>
Before this patch, I have another one addressing the same issue [1], with
that patch we'll not have these inconsistences, but it looks like a little
bit complicated, so it becomes current one. Any comments about that?

[1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html

Thanks,
Dennis
> 
> -- 
> Ard.
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-24 12:01           ` Dennis Chen
  0 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-24 12:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
> > When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
> > regions loaded by firmware will beyond the limit of the memory, in this
> > case we need to nomap the region above the limit while not removing
> > it from memblock, because once region removed from memblock, the ACPI
> > will think that region is not a normal memory and map it as device type
> > memory accordingly. Since the ACPI core will produce non-alignment access
> > when paring AML data stream, hence result in alignment fault upon the io
> > mapped memory space.
> >
> > For example, below is an alignment exception observed on softIron board
> > when booting the kernel with 'acpi=force mem=8G':
> > ...
> > [ 0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
> > [ 0.550457] pgd = ffff000008aa0000
> > [ 0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
> > [    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
> > [    0.570553] Modules linked in:
> > [    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
> > [    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
> > [    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
> > [    0.597571] PC is at acpi_ns_lookup+0x520/0x734
> > [    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
> > [    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
> > [    0.614145] sp : ffff800001efb8b0
> > [    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
> > [    0.622829] x27: 0000000000000001 x26: 0000000000000000
> > [    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
> > [    0.633531] x23: 0000000000000001 x22: 0000000000000001
> > [    0.638881] x21: ffff000008724000 x20: 000000000000001b
> > [    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
> > [    0.649580] x17: 00000000000038ff x16: 0000000000000002
> > [    0.654929] x15: 0000000000000007 x14: 0000000000007fff
> > [    0.660278] x13: ffffff0000000000 x12: 0000000000000018
> > [    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
> > [    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
> > [    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
> > [    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
> > [    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
> > [    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
> > ...
> > [    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
> > [    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
> > [    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
> > [    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
> > [    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
> > [    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
> > [    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
> > [    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
> > [    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
> > [    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
> > [    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
> > [    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
> > [    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
> > [    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
> > [    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
> > [    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
> > [    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
> > [    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
> > [    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >
> > With 'efi=debug', we can see those ACPI regions loaded by firmware on
> > that board as:
> > [    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> > [    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> >
> > This patch is trying to address the above issues by nomaping the region
> > instead of removing it.
> >
> > Signed-off-by: Dennis Chen <dennis.chen@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Steve Capper <steve.capper@arm.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> > Cc: linux-mm at kvack.org
> > Cc: linux-acpi at vger.kernel.org
> > Cc: linux-efi at vger.kernel.org
> > ---
> > Changes in v2:
> > Update the commit message and remove the memblock_is_map_memory() check
> > according to the suggestion from Mark Rutland.
> >
> >  arch/arm64/mm/init.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index d45f862..6af2456 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
> >
> >         /*
> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> > -        * high up in memory, add back the kernel region that must be accessible
> > -        * via the linear mapping.
> > +        * in the memory regions above the limit, so we need to clear the
> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> > +        * the linear mapping.
> >          */
> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> > -               memblock_enforce_memory_limit(memory_limit);
> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> > +               memblock_mem_limit_mark_nomap(memory_limit);
> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> 
> Up until now, we have ignored the effect of having NOMAP memblocks on
> the return values of functions like memblock_phys_mem_size() and
> memblock_mem_size(), since they could reasonably be expected to cover
> only a small slice of all available memory. However, after applying
> this patch, it may well be the case that most of memory is marked
> NOMAP, and these functions will cease to work as expected.
>
Hi Ard, I noticed these inconsistences as you mentioned, but seems the
available memory is limited correctly. For this case('mem='), will it bring
some substantive side effects except that some log messages maybe confusing?  
> 
> This means NOMAP is really only suited to punch some holes into the
> kernel direct mapping, and so implementing the memory limit by marking
> everything NOMAP is not the way to go. Instead, we should probably
> reorder the init sequence so that the regions that are reserved in the
> UEFI memory map are declared and marked NOMAP [again] after applying
> the memory limit in the old way.
>
Before this patch, I have another one addressing the same issue [1], with
that patch we'll not have these inconsistences, but it looks like a little
bit complicated, so it becomes current one. Any comments about that?

[1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html

Thanks,
Dennis
> 
> -- 
> Ard.
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
  2016-06-24 12:01           ` Dennis Chen
@ 2016-06-24 14:12             ` Ard Biesheuvel
  -1 siblings, 0 replies; 21+ messages in thread
From: Ard Biesheuvel @ 2016-06-24 14:12 UTC (permalink / raw)
  To: Dennis Chen
  Cc: linux-arm-kernel, nd, Catalin Marinas, Steve Capper, Will Deacon,
	Mark Rutland, Rafael J . Wysocki, Matt Fleming, linux-mm,
	linux-acpi, linux-efi

On 24 June 2016 at 14:01, Dennis Chen <dennis.chen@arm.com> wrote:
> On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
>> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
>> > When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
>> > regions loaded by firmware will beyond the limit of the memory, in this
>> > case we need to nomap the region above the limit while not removing
>> > it from memblock, because once region removed from memblock, the ACPI
>> > will think that region is not a normal memory and map it as device type
>> > memory accordingly. Since the ACPI core will produce non-alignment access
>> > when paring AML data stream, hence result in alignment fault upon the io
>> > mapped memory space.
>> >
>> > For example, below is an alignment exception observed on softIron board
>> > when booting the kernel with 'acpi=force mem=8G':
>> > ...
>> > [ 0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
>> > [ 0.550457] pgd = ffff000008aa0000
>> > [ 0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
>> > [    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
>> > [    0.570553] Modules linked in:
>> > [    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
>> > [    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
>> > [    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
>> > [    0.597571] PC is at acpi_ns_lookup+0x520/0x734
>> > [    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
>> > [    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
>> > [    0.614145] sp : ffff800001efb8b0
>> > [    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
>> > [    0.622829] x27: 0000000000000001 x26: 0000000000000000
>> > [    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
>> > [    0.633531] x23: 0000000000000001 x22: 0000000000000001
>> > [    0.638881] x21: ffff000008724000 x20: 000000000000001b
>> > [    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
>> > [    0.649580] x17: 00000000000038ff x16: 0000000000000002
>> > [    0.654929] x15: 0000000000000007 x14: 0000000000007fff
>> > [    0.660278] x13: ffffff0000000000 x12: 0000000000000018
>> > [    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
>> > [    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
>> > [    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
>> > [    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
>> > [    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
>> > [    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
>> > ...
>> > [    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
>> > [    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
>> > [    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
>> > [    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
>> > [    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
>> > [    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
>> > [    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
>> > [    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
>> > [    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
>> > [    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
>> > [    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
>> > [    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
>> > [    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
>> > [    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
>> > [    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
>> > [    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
>> > [    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
>> > [    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
>> > [    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>> >
>> > With 'efi=debug', we can see those ACPI regions loaded by firmware on
>> > that board as:
>> > [    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
>> > [    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
>> >
>> > This patch is trying to address the above issues by nomaping the region
>> > instead of removing it.
>> >
>> > Signed-off-by: Dennis Chen <dennis.chen@arm.com>
>> > Cc: Catalin Marinas <catalin.marinas@arm.com>
>> > Cc: Steve Capper <steve.capper@arm.com>
>> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > Cc: Will Deacon <will.deacon@arm.com>
>> > Cc: Mark Rutland <mark.rutland@arm.com>
>> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
>> > Cc: linux-mm@kvack.org
>> > Cc: linux-acpi@vger.kernel.org
>> > Cc: linux-efi@vger.kernel.org
>> > ---
>> > Changes in v2:
>> > Update the commit message and remove the memblock_is_map_memory() check
>> > according to the suggestion from Mark Rutland.
>> >
>> >  arch/arm64/mm/init.c | 9 +++++----
>> >  1 file changed, 5 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> > index d45f862..6af2456 100644
>> > --- a/arch/arm64/mm/init.c
>> > +++ b/arch/arm64/mm/init.c
>> > @@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
>> >
>> >         /*
>> >          * Apply the memory limit if it was set. Since the kernel may be loaded
>> > -        * high up in memory, add back the kernel region that must be accessible
>> > -        * via the linear mapping.
>> > +        * in the memory regions above the limit, so we need to clear the
>> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
>> > +        * the linear mapping.
>> >          */
>> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
>> > -               memblock_enforce_memory_limit(memory_limit);
>> > -               memblock_add(__pa(_text), (u64)(_end - _text));
>> > +               memblock_mem_limit_mark_nomap(memory_limit);
>> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
>>
>> Up until now, we have ignored the effect of having NOMAP memblocks on
>> the return values of functions like memblock_phys_mem_size() and
>> memblock_mem_size(), since they could reasonably be expected to cover
>> only a small slice of all available memory. However, after applying
>> this patch, it may well be the case that most of memory is marked
>> NOMAP, and these functions will cease to work as expected.
>>
> Hi Ard, I noticed these inconsistences as you mentioned, but seems the
> available memory is limited correctly. For this case('mem='), will it bring
> some substantive side effects except that some log messages maybe confusing?

That is exactly the question that needs answering before we can merge
these patches. I know we consider mem= a development hack, but the
intent is to make it appear to the kernel as if only a smaller amount
of memory is available to the kernel, and this is signficantly
different from having memblock_mem_size() et al return much larger
values than what is actually available. Perhaps this doesn't matter at
all, but it is something we must discuss before proceeding with these
changes.

>>
>> This means NOMAP is really only suited to punch some holes into the
>> kernel direct mapping, and so implementing the memory limit by marking
>> everything NOMAP is not the way to go. Instead, we should probably
>> reorder the init sequence so that the regions that are reserved in the
>> UEFI memory map are declared and marked NOMAP [again] after applying
>> the memory limit in the old way.
>>
> Before this patch, I have another one addressing the same issue [1], with
> that patch we'll not have these inconsistences, but it looks like a little
> bit complicated, so it becomes current one. Any comments about that?
>
> [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html
>

The problem caused by mem= is that it removes regions that are marked
NOMAP. So instead of marking everything above the limit NOMAP, I would
much rather see an alternative implementation of
memblock_enforce_memory_limit() that enforces the mem= limit by only
removing memblocks that have to NOMAP flag cleared, and leaving the
NOMAP ones where they are.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-24 14:12             ` Ard Biesheuvel
  0 siblings, 0 replies; 21+ messages in thread
From: Ard Biesheuvel @ 2016-06-24 14:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 24 June 2016 at 14:01, Dennis Chen <dennis.chen@arm.com> wrote:
> On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
>> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
>> > When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
>> > regions loaded by firmware will beyond the limit of the memory, in this
>> > case we need to nomap the region above the limit while not removing
>> > it from memblock, because once region removed from memblock, the ACPI
>> > will think that region is not a normal memory and map it as device type
>> > memory accordingly. Since the ACPI core will produce non-alignment access
>> > when paring AML data stream, hence result in alignment fault upon the io
>> > mapped memory space.
>> >
>> > For example, below is an alignment exception observed on softIron board
>> > when booting the kernel with 'acpi=force mem=8G':
>> > ...
>> > [ 0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
>> > [ 0.550457] pgd = ffff000008aa0000
>> > [ 0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
>> > [    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
>> > [    0.570553] Modules linked in:
>> > [    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
>> > [    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
>> > [    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
>> > [    0.597571] PC is at acpi_ns_lookup+0x520/0x734
>> > [    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
>> > [    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
>> > [    0.614145] sp : ffff800001efb8b0
>> > [    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
>> > [    0.622829] x27: 0000000000000001 x26: 0000000000000000
>> > [    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
>> > [    0.633531] x23: 0000000000000001 x22: 0000000000000001
>> > [    0.638881] x21: ffff000008724000 x20: 000000000000001b
>> > [    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
>> > [    0.649580] x17: 00000000000038ff x16: 0000000000000002
>> > [    0.654929] x15: 0000000000000007 x14: 0000000000007fff
>> > [    0.660278] x13: ffffff0000000000 x12: 0000000000000018
>> > [    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
>> > [    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
>> > [    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
>> > [    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
>> > [    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
>> > [    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
>> > ...
>> > [    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
>> > [    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
>> > [    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
>> > [    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
>> > [    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
>> > [    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
>> > [    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
>> > [    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
>> > [    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
>> > [    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
>> > [    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
>> > [    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
>> > [    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
>> > [    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
>> > [    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
>> > [    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
>> > [    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
>> > [    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
>> > [    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>> >
>> > With 'efi=debug', we can see those ACPI regions loaded by firmware on
>> > that board as:
>> > [    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
>> > [    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
>> >
>> > This patch is trying to address the above issues by nomaping the region
>> > instead of removing it.
>> >
>> > Signed-off-by: Dennis Chen <dennis.chen@arm.com>
>> > Cc: Catalin Marinas <catalin.marinas@arm.com>
>> > Cc: Steve Capper <steve.capper@arm.com>
>> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > Cc: Will Deacon <will.deacon@arm.com>
>> > Cc: Mark Rutland <mark.rutland@arm.com>
>> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
>> > Cc: linux-mm at kvack.org
>> > Cc: linux-acpi at vger.kernel.org
>> > Cc: linux-efi at vger.kernel.org
>> > ---
>> > Changes in v2:
>> > Update the commit message and remove the memblock_is_map_memory() check
>> > according to the suggestion from Mark Rutland.
>> >
>> >  arch/arm64/mm/init.c | 9 +++++----
>> >  1 file changed, 5 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> > index d45f862..6af2456 100644
>> > --- a/arch/arm64/mm/init.c
>> > +++ b/arch/arm64/mm/init.c
>> > @@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
>> >
>> >         /*
>> >          * Apply the memory limit if it was set. Since the kernel may be loaded
>> > -        * high up in memory, add back the kernel region that must be accessible
>> > -        * via the linear mapping.
>> > +        * in the memory regions above the limit, so we need to clear the
>> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
>> > +        * the linear mapping.
>> >          */
>> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
>> > -               memblock_enforce_memory_limit(memory_limit);
>> > -               memblock_add(__pa(_text), (u64)(_end - _text));
>> > +               memblock_mem_limit_mark_nomap(memory_limit);
>> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
>>
>> Up until now, we have ignored the effect of having NOMAP memblocks on
>> the return values of functions like memblock_phys_mem_size() and
>> memblock_mem_size(), since they could reasonably be expected to cover
>> only a small slice of all available memory. However, after applying
>> this patch, it may well be the case that most of memory is marked
>> NOMAP, and these functions will cease to work as expected.
>>
> Hi Ard, I noticed these inconsistences as you mentioned, but seems the
> available memory is limited correctly. For this case('mem='), will it bring
> some substantive side effects except that some log messages maybe confusing?

That is exactly the question that needs answering before we can merge
these patches. I know we consider mem= a development hack, but the
intent is to make it appear to the kernel as if only a smaller amount
of memory is available to the kernel, and this is signficantly
different from having memblock_mem_size() et al return much larger
values than what is actually available. Perhaps this doesn't matter at
all, but it is something we must discuss before proceeding with these
changes.

>>
>> This means NOMAP is really only suited to punch some holes into the
>> kernel direct mapping, and so implementing the memory limit by marking
>> everything NOMAP is not the way to go. Instead, we should probably
>> reorder the init sequence so that the regions that are reserved in the
>> UEFI memory map are declared and marked NOMAP [again] after applying
>> the memory limit in the old way.
>>
> Before this patch, I have another one addressing the same issue [1], with
> that patch we'll not have these inconsistences, but it looks like a little
> bit complicated, so it becomes current one. Any comments about that?
>
> [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html
>

The problem caused by mem= is that it removes regions that are marked
NOMAP. So instead of marking everything above the limit NOMAP, I would
much rather see an alternative implementation of
memblock_enforce_memory_limit() that enforces the mem= limit by only
removing memblocks that have to NOMAP flag cleared, and leaving the
NOMAP ones where they are.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
  2016-06-24 14:12             ` Ard Biesheuvel
  (?)
@ 2016-06-27  1:20                 ` Dennis Chen
  -1 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-27  1:20 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	nd-5wv7dgnIgG8, Catalin Marinas, Steve Capper, Will Deacon,
	Mark Rutland, Rafael J . Wysocki, Matt Fleming,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA

On Fri, Jun 24, 2016 at 04:12:02PM +0200, Ard Biesheuvel wrote:
> On 24 June 2016 at 14:01, Dennis Chen <dennis.chen-5wv7dgnIgG8@public.gmane.org> wrote:
> > On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> >> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen-5wv7dgnIgG8@public.gmane.org> wrote:
> >> > When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
> >> > regions loaded by firmware will beyond the limit of the memory, in this
> >> > case we need to nomap the region above the limit while not removing
> >> > it from memblock, because once region removed from memblock, the ACPI
> >> > will think that region is not a normal memory and map it as device type
> >> > memory accordingly. Since the ACPI core will produce non-alignment access
> >> > when paring AML data stream, hence result in alignment fault upon the io
> >> > mapped memory space.
> >> >
> >> > For example, below is an alignment exception observed on softIron board
> >> > when booting the kernel with 'acpi=force mem=8G':
> >> > ...
> >> > [ 0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
> >> > [ 0.550457] pgd = ffff000008aa0000
> >> > [ 0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
> >> > [    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
> >> > [    0.570553] Modules linked in:
> >> > [    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
> >> > [    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
> >> > [    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
> >> > [    0.597571] PC is at acpi_ns_lookup+0x520/0x734
> >> > [    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
> >> > [    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
> >> > [    0.614145] sp : ffff800001efb8b0
> >> > [    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
> >> > [    0.622829] x27: 0000000000000001 x26: 0000000000000000
> >> > [    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
> >> > [    0.633531] x23: 0000000000000001 x22: 0000000000000001
> >> > [    0.638881] x21: ffff000008724000 x20: 000000000000001b
> >> > [    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
> >> > [    0.649580] x17: 00000000000038ff x16: 0000000000000002
> >> > [    0.654929] x15: 0000000000000007 x14: 0000000000007fff
> >> > [    0.660278] x13: ffffff0000000000 x12: 0000000000000018
> >> > [    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
> >> > [    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
> >> > [    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
> >> > [    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
> >> > [    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
> >> > [    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
> >> > ...
> >> > [    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
> >> > [    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
> >> > [    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
> >> > [    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
> >> > [    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
> >> > [    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
> >> > [    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
> >> > [    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
> >> > [    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
> >> > [    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
> >> > [    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
> >> > [    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
> >> > [    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
> >> > [    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
> >> > [    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
> >> > [    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
> >> > [    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
> >> > [    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
> >> > [    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >> >
> >> > With 'efi=debug', we can see those ACPI regions loaded by firmware on
> >> > that board as:
> >> > [    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> >> > [    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> >> >
> >> > This patch is trying to address the above issues by nomaping the region
> >> > instead of removing it.
> >> >
> >> > Signed-off-by: Dennis Chen <dennis.chen-5wv7dgnIgG8@public.gmane.org>
> >> > Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> >> > Cc: Steve Capper <steve.capper-5wv7dgnIgG8@public.gmane.org>
> >> > Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> > Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> >> > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> >> > Cc: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >> > Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> >> > Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org
> >> > Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> > Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> > ---
> >> > Changes in v2:
> >> > Update the commit message and remove the memblock_is_map_memory() check
> >> > according to the suggestion from Mark Rutland.
> >> >
> >> >  arch/arm64/mm/init.c | 9 +++++----
> >> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> >> > index d45f862..6af2456 100644
> >> > --- a/arch/arm64/mm/init.c
> >> > +++ b/arch/arm64/mm/init.c
> >> > @@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
> >> >
> >> >         /*
> >> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> >> > -        * high up in memory, add back the kernel region that must be accessible
> >> > -        * via the linear mapping.
> >> > +        * in the memory regions above the limit, so we need to clear the
> >> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> >> > +        * the linear mapping.
> >> >          */
> >> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> >> > -               memblock_enforce_memory_limit(memory_limit);
> >> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> >> > +               memblock_mem_limit_mark_nomap(memory_limit);
> >> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> >>
> >> Up until now, we have ignored the effect of having NOMAP memblocks on
> >> the return values of functions like memblock_phys_mem_size() and
> >> memblock_mem_size(), since they could reasonably be expected to cover
> >> only a small slice of all available memory. However, after applying
> >> this patch, it may well be the case that most of memory is marked
> >> NOMAP, and these functions will cease to work as expected.
> >>
> > Hi Ard, I noticed these inconsistences as you mentioned, but seems the
> > available memory is limited correctly. For this case('mem='), will it bring
> > some substantive side effects except that some log messages maybe confusing?
> 
> That is exactly the question that needs answering before we can merge
> these patches. I know we consider mem= a development hack, but the
> intent is to make it appear to the kernel as if only a smaller amount
> of memory is available to the kernel, and this is signficantly
> different from having memblock_mem_size() et al return much larger
> values than what is actually available. Perhaps this doesn't matter at
> all, but it is something we must discuss before proceeding with these
> changes.
>
Indeed. So let's go back to the method below... 
>
> >>
> >> This means NOMAP is really only suited to punch some holes into the
> >> kernel direct mapping, and so implementing the memory limit by marking
> >> everything NOMAP is not the way to go. Instead, we should probably
> >> reorder the init sequence so that the regions that are reserved in the
> >> UEFI memory map are declared and marked NOMAP [again] after applying
> >> the memory limit in the old way.
> >>
> > Before this patch, I have another one addressing the same issue [1], with
> > that patch we'll not have these inconsistences, but it looks like a little
> > bit complicated, so it becomes current one. Any comments about that?
> >
> > [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html
> >
> 
> The problem caused by mem= is that it removes regions that are marked
> NOMAP. So instead of marking everything above the limit NOMAP, I would
> much rather see an alternative implementation of
> memblock_enforce_memory_limit() that enforces the mem= limit by only
> removing memblocks that have to NOMAP flag cleared, and leaving the
> NOMAP ones where they are.
>
At least for me, this approach will mitigate the inconsistence in some degree while
keeping the similar logic as it was, so I will post an updated version patch soon.

Thanks,
Dennis
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-27  1:20                 ` Dennis Chen
  0 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-27  1:20 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, nd, Catalin Marinas, Steve Capper, Will Deacon,
	Mark Rutland, Rafael J . Wysocki, Matt Fleming, linux-mm,
	linux-acpi, linux-efi

On Fri, Jun 24, 2016 at 04:12:02PM +0200, Ard Biesheuvel wrote:
> On 24 June 2016 at 14:01, Dennis Chen <dennis.chen@arm.com> wrote:
> > On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> >> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
> >> > When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
> >> > regions loaded by firmware will beyond the limit of the memory, in this
> >> > case we need to nomap the region above the limit while not removing
> >> > it from memblock, because once region removed from memblock, the ACPI
> >> > will think that region is not a normal memory and map it as device type
> >> > memory accordingly. Since the ACPI core will produce non-alignment access
> >> > when paring AML data stream, hence result in alignment fault upon the io
> >> > mapped memory space.
> >> >
> >> > For example, below is an alignment exception observed on softIron board
> >> > when booting the kernel with 'acpi=force mem=8G':
> >> > ...
> >> > [ 0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
> >> > [ 0.550457] pgd = ffff000008aa0000
> >> > [ 0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
> >> > [    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
> >> > [    0.570553] Modules linked in:
> >> > [    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
> >> > [    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
> >> > [    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
> >> > [    0.597571] PC is at acpi_ns_lookup+0x520/0x734
> >> > [    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
> >> > [    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
> >> > [    0.614145] sp : ffff800001efb8b0
> >> > [    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
> >> > [    0.622829] x27: 0000000000000001 x26: 0000000000000000
> >> > [    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
> >> > [    0.633531] x23: 0000000000000001 x22: 0000000000000001
> >> > [    0.638881] x21: ffff000008724000 x20: 000000000000001b
> >> > [    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
> >> > [    0.649580] x17: 00000000000038ff x16: 0000000000000002
> >> > [    0.654929] x15: 0000000000000007 x14: 0000000000007fff
> >> > [    0.660278] x13: ffffff0000000000 x12: 0000000000000018
> >> > [    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
> >> > [    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
> >> > [    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
> >> > [    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
> >> > [    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
> >> > [    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
> >> > ...
> >> > [    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
> >> > [    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
> >> > [    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
> >> > [    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
> >> > [    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
> >> > [    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
> >> > [    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
> >> > [    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
> >> > [    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
> >> > [    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
> >> > [    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
> >> > [    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
> >> > [    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
> >> > [    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
> >> > [    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
> >> > [    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
> >> > [    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
> >> > [    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
> >> > [    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >> >
> >> > With 'efi=debug', we can see those ACPI regions loaded by firmware on
> >> > that board as:
> >> > [    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> >> > [    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> >> >
> >> > This patch is trying to address the above issues by nomaping the region
> >> > instead of removing it.
> >> >
> >> > Signed-off-by: Dennis Chen <dennis.chen@arm.com>
> >> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> > Cc: Steve Capper <steve.capper@arm.com>
> >> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> > Cc: Will Deacon <will.deacon@arm.com>
> >> > Cc: Mark Rutland <mark.rutland@arm.com>
> >> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> >> > Cc: linux-mm@kvack.org
> >> > Cc: linux-acpi@vger.kernel.org
> >> > Cc: linux-efi@vger.kernel.org
> >> > ---
> >> > Changes in v2:
> >> > Update the commit message and remove the memblock_is_map_memory() check
> >> > according to the suggestion from Mark Rutland.
> >> >
> >> >  arch/arm64/mm/init.c | 9 +++++----
> >> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> >> > index d45f862..6af2456 100644
> >> > --- a/arch/arm64/mm/init.c
> >> > +++ b/arch/arm64/mm/init.c
> >> > @@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
> >> >
> >> >         /*
> >> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> >> > -        * high up in memory, add back the kernel region that must be accessible
> >> > -        * via the linear mapping.
> >> > +        * in the memory regions above the limit, so we need to clear the
> >> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> >> > +        * the linear mapping.
> >> >          */
> >> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> >> > -               memblock_enforce_memory_limit(memory_limit);
> >> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> >> > +               memblock_mem_limit_mark_nomap(memory_limit);
> >> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> >>
> >> Up until now, we have ignored the effect of having NOMAP memblocks on
> >> the return values of functions like memblock_phys_mem_size() and
> >> memblock_mem_size(), since they could reasonably be expected to cover
> >> only a small slice of all available memory. However, after applying
> >> this patch, it may well be the case that most of memory is marked
> >> NOMAP, and these functions will cease to work as expected.
> >>
> > Hi Ard, I noticed these inconsistences as you mentioned, but seems the
> > available memory is limited correctly. For this case('mem='), will it bring
> > some substantive side effects except that some log messages maybe confusing?
> 
> That is exactly the question that needs answering before we can merge
> these patches. I know we consider mem= a development hack, but the
> intent is to make it appear to the kernel as if only a smaller amount
> of memory is available to the kernel, and this is signficantly
> different from having memblock_mem_size() et al return much larger
> values than what is actually available. Perhaps this doesn't matter at
> all, but it is something we must discuss before proceeding with these
> changes.
>
Indeed. So let's go back to the method below... 
>
> >>
> >> This means NOMAP is really only suited to punch some holes into the
> >> kernel direct mapping, and so implementing the memory limit by marking
> >> everything NOMAP is not the way to go. Instead, we should probably
> >> reorder the init sequence so that the regions that are reserved in the
> >> UEFI memory map are declared and marked NOMAP [again] after applying
> >> the memory limit in the old way.
> >>
> > Before this patch, I have another one addressing the same issue [1], with
> > that patch we'll not have these inconsistences, but it looks like a little
> > bit complicated, so it becomes current one. Any comments about that?
> >
> > [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html
> >
> 
> The problem caused by mem= is that it removes regions that are marked
> NOMAP. So instead of marking everything above the limit NOMAP, I would
> much rather see an alternative implementation of
> memblock_enforce_memory_limit() that enforces the mem= limit by only
> removing memblocks that have to NOMAP flag cleared, and leaving the
> NOMAP ones where they are.
>
At least for me, this approach will mitigate the inconsistence in some degree while
keeping the similar logic as it was, so I will post an updated version patch soon.

Thanks,
Dennis
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-27  1:20                 ` Dennis Chen
  0 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-27  1:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jun 24, 2016 at 04:12:02PM +0200, Ard Biesheuvel wrote:
> On 24 June 2016 at 14:01, Dennis Chen <dennis.chen@arm.com> wrote:
> > On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> >> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
> >> > When booting an ACPI enabled kernel with 'mem=', probably the ACPI data
> >> > regions loaded by firmware will beyond the limit of the memory, in this
> >> > case we need to nomap the region above the limit while not removing
> >> > it from memblock, because once region removed from memblock, the ACPI
> >> > will think that region is not a normal memory and map it as device type
> >> > memory accordingly. Since the ACPI core will produce non-alignment access
> >> > when paring AML data stream, hence result in alignment fault upon the io
> >> > mapped memory space.
> >> >
> >> > For example, below is an alignment exception observed on softIron board
> >> > when booting the kernel with 'acpi=force mem=8G':
> >> > ...
> >> > [ 0.542475] Unable to handle kernel paging request at virtual address ffff0000080521e7
> >> > [ 0.550457] pgd = ffff000008aa0000
> >> > [ 0.553880] [ffff0000080521e7] *pgd=000000801fffe003, *pud=000000801fffd003, *pmd=000000801fffc003, *pte=00e80083ff1c1707
> >> > [    0.564939] Internal error: Oops: 96000021 [#1] PREEMPT SMP
> >> > [    0.570553] Modules linked in:
> >> > [    0.573626] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.7.0-rc3-next-20160616+ #172
> >> > [    0.581344] Hardware name: AMD Overdrive/Supercharger/Default string, BIOS ROD1001A 02/09/2016
> >> > [    0.590025] task: ffff800001ef0000 ti: ffff800001ef8000 task.ti: ffff800001ef8000
> >> > [    0.597571] PC is at acpi_ns_lookup+0x520/0x734
> >> > [    0.602134] LR is at acpi_ns_lookup+0x4a4/0x734
> >> > [    0.606693] pc : [<ffff0000083b8b10>] lr : [<ffff0000083b8a94>] pstate: 60000045
> >> > [    0.614145] sp : ffff800001efb8b0
> >> > [    0.617478] x29: ffff800001efb8c0 x28: 000000000000001b
> >> > [    0.622829] x27: 0000000000000001 x26: 0000000000000000
> >> > [    0.628181] x25: ffff800001efb9e8 x24: ffff000008a10000
> >> > [    0.633531] x23: 0000000000000001 x22: 0000000000000001
> >> > [    0.638881] x21: ffff000008724000 x20: 000000000000001b
> >> > [    0.644230] x19: ffff0000080521e7 x18: 000000000000000d
> >> > [    0.649580] x17: 00000000000038ff x16: 0000000000000002
> >> > [    0.654929] x15: 0000000000000007 x14: 0000000000007fff
> >> > [    0.660278] x13: ffffff0000000000 x12: 0000000000000018
> >> > [    0.665627] x11: 000000001fffd200 x10: 00000000ffffff76
> >> > [    0.670978] x9 : 000000000000005f x8 : ffff000008725fa8
> >> > [    0.676328] x7 : ffff000008a8df70 x6 : ffff000008a8df70
> >> > [    0.681679] x5 : ffff000008a8d000 x4 : 0000000000000010
> >> > [    0.687027] x3 : 0000000000000010 x2 : 000000000000000c
> >> > [    0.692378] x1 : 0000000000000006 x0 : 0000000000000000
> >> > ...
> >> > [    1.262235] [<ffff0000083b8b10>] acpi_ns_lookup+0x520/0x734
> >> > [    1.267845] [<ffff0000083a7160>] acpi_ds_load1_begin_op+0x174/0x4fc
> >> > [    1.274156] [<ffff0000083c1f4c>] acpi_ps_build_named_op+0xf8/0x220
> >> > [    1.280380] [<ffff0000083c227c>] acpi_ps_create_op+0x208/0x33c
> >> > [    1.286254] [<ffff0000083c1820>] acpi_ps_parse_loop+0x204/0x838
> >> > [    1.292215] [<ffff0000083c2fd4>] acpi_ps_parse_aml+0x1bc/0x42c
> >> > [    1.298090] [<ffff0000083bc6e8>] acpi_ns_one_complete_parse+0x1e8/0x22c
> >> > [    1.304753] [<ffff0000083bc7b8>] acpi_ns_parse_table+0x8c/0x128
> >> > [    1.310716] [<ffff0000083bb8fc>] acpi_ns_load_table+0xc0/0x1e8
> >> > [    1.316591] [<ffff0000083c9068>] acpi_tb_load_namespace+0xf8/0x2e8
> >> > [    1.322818] [<ffff000008984128>] acpi_load_tables+0x7c/0x110
> >> > [    1.328516] [<ffff000008982ea4>] acpi_init+0x90/0x2c0
> >> > [    1.333603] [<ffff0000080819fc>] do_one_initcall+0x38/0x12c
> >> > [    1.339215] [<ffff000008960cd4>] kernel_init_freeable+0x148/0x1ec
> >> > [    1.345353] [<ffff0000086b7d30>] kernel_init+0x10/0xec
> >> > [    1.350529] [<ffff000008084e10>] ret_from_fork+0x10/0x40
> >> > [    1.355878] Code: b9009fbc 2a00037b 36380057 3219037b (b9400260)
> >> > [    1.362035] ---[ end trace 03381e5eb0a24de4 ]---
> >> > [    1.366691] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >> >
> >> > With 'efi=debug', we can see those ACPI regions loaded by firmware on
> >> > that board as:
> >> > [    0.000000] efi:   0x0083ff1b5000-0x0083ff1c2fff [ACPI Reclaim Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> >> > [    0.000000] efi:   0x0083ff223000-0x0083ff224fff [ACPI Memory NVS    |   |  |  |  |  |  |  |   |WB|WT|WC|UC]*
> >> >
> >> > This patch is trying to address the above issues by nomaping the region
> >> > instead of removing it.
> >> >
> >> > Signed-off-by: Dennis Chen <dennis.chen@arm.com>
> >> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> > Cc: Steve Capper <steve.capper@arm.com>
> >> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> > Cc: Will Deacon <will.deacon@arm.com>
> >> > Cc: Mark Rutland <mark.rutland@arm.com>
> >> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> >> > Cc: linux-mm at kvack.org
> >> > Cc: linux-acpi at vger.kernel.org
> >> > Cc: linux-efi at vger.kernel.org
> >> > ---
> >> > Changes in v2:
> >> > Update the commit message and remove the memblock_is_map_memory() check
> >> > according to the suggestion from Mark Rutland.
> >> >
> >> >  arch/arm64/mm/init.c | 9 +++++----
> >> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> >> > index d45f862..6af2456 100644
> >> > --- a/arch/arm64/mm/init.c
> >> > +++ b/arch/arm64/mm/init.c
> >> > @@ -222,12 +222,13 @@ void __init arm64_memblock_init(void)
> >> >
> >> >         /*
> >> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> >> > -        * high up in memory, add back the kernel region that must be accessible
> >> > -        * via the linear mapping.
> >> > +        * in the memory regions above the limit, so we need to clear the
> >> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> >> > +        * the linear mapping.
> >> >          */
> >> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> >> > -               memblock_enforce_memory_limit(memory_limit);
> >> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> >> > +               memblock_mem_limit_mark_nomap(memory_limit);
> >> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> >>
> >> Up until now, we have ignored the effect of having NOMAP memblocks on
> >> the return values of functions like memblock_phys_mem_size() and
> >> memblock_mem_size(), since they could reasonably be expected to cover
> >> only a small slice of all available memory. However, after applying
> >> this patch, it may well be the case that most of memory is marked
> >> NOMAP, and these functions will cease to work as expected.
> >>
> > Hi Ard, I noticed these inconsistences as you mentioned, but seems the
> > available memory is limited correctly. For this case('mem='), will it bring
> > some substantive side effects except that some log messages maybe confusing?
> 
> That is exactly the question that needs answering before we can merge
> these patches. I know we consider mem= a development hack, but the
> intent is to make it appear to the kernel as if only a smaller amount
> of memory is available to the kernel, and this is signficantly
> different from having memblock_mem_size() et al return much larger
> values than what is actually available. Perhaps this doesn't matter at
> all, but it is something we must discuss before proceeding with these
> changes.
>
Indeed. So let's go back to the method below... 
>
> >>
> >> This means NOMAP is really only suited to punch some holes into the
> >> kernel direct mapping, and so implementing the memory limit by marking
> >> everything NOMAP is not the way to go. Instead, we should probably
> >> reorder the init sequence so that the regions that are reserved in the
> >> UEFI memory map are declared and marked NOMAP [again] after applying
> >> the memory limit in the old way.
> >>
> > Before this patch, I have another one addressing the same issue [1], with
> > that patch we'll not have these inconsistences, but it looks like a little
> > bit complicated, so it becomes current one. Any comments about that?
> >
> > [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html
> >
> 
> The problem caused by mem= is that it removes regions that are marked
> NOMAP. So instead of marking everything above the limit NOMAP, I would
> much rather see an alternative implementation of
> memblock_enforce_memory_limit() that enforces the mem= limit by only
> removing memblocks that have to NOMAP flag cleared, and leaving the
> NOMAP ones where they are.
>
At least for me, this approach will mitigate the inconsistence in some degree while
keeping the similar logic as it was, so I will post an updated version patch soon.

Thanks,
Dennis
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
  2016-06-24 14:12             ` Ard Biesheuvel
  (?)
@ 2016-06-27  9:53                 ` Mark Rutland
  -1 siblings, 0 replies; 21+ messages in thread
From: Mark Rutland @ 2016-06-27  9:53 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Dennis Chen, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	nd-5wv7dgnIgG8, Catalin Marinas, Steve Capper, Will Deacon,
	Rafael J . Wysocki, Matt Fleming,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA

On Fri, Jun 24, 2016 at 04:12:02PM +0200, Ard Biesheuvel wrote:
> On 24 June 2016 at 14:01, Dennis Chen <dennis.chen-5wv7dgnIgG8@public.gmane.org> wrote:
> > On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> >> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen-5wv7dgnIgG8@public.gmane.org> wrote:
> >> >         /*
> >> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> >> > -        * high up in memory, add back the kernel region that must be accessible
> >> > -        * via the linear mapping.
> >> > +        * in the memory regions above the limit, so we need to clear the
> >> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> >> > +        * the linear mapping.
> >> >          */
> >> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> >> > -               memblock_enforce_memory_limit(memory_limit);
> >> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> >> > +               memblock_mem_limit_mark_nomap(memory_limit);
> >> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> >>
> >> Up until now, we have ignored the effect of having NOMAP memblocks on
> >> the return values of functions like memblock_phys_mem_size() and
> >> memblock_mem_size(), since they could reasonably be expected to cover
> >> only a small slice of all available memory. However, after applying
> >> this patch, it may well be the case that most of memory is marked
> >> NOMAP, and these functions will cease to work as expected.
> >>
> > Hi Ard, I noticed these inconsistences as you mentioned, but seems the
> > available memory is limited correctly. For this case('mem='), will it bring
> > some substantive side effects except that some log messages maybe confusing?
> 
> That is exactly the question that needs answering before we can merge
> these patches. I know we consider mem= a development hack, but the
> intent is to make it appear to the kernel as if only a smaller amount
> of memory is available to the kernel, and this is signficantly
> different from having memblock_mem_size() et al return much larger
> values than what is actually available. Perhaps this doesn't matter at
> all, but it is something we must discuss before proceeding with these
> changes.

Yeah, I think we need to figure out precisely what the expected
semantics are.

>From taking a look, memblock_mem_size() is only used by arch/x86. In
reserve_initrd, it's used to determine the amount of *free* memory, but
it counts reserved (and nomap) regions, so that doesn't feel right
regardless. For reserve_crashkernel_low it's not immediately clear to me
what it should do, as I've not gone digging.

There are many memblock_end_of_DRAM() users, mostly in arch code. We
(arm64) use it to determine the size of the linear map, and effectively
need it to be the limit for what should be mapped, which could/should
exclude nomap. I've not yet dug into the rest, so I don't know whether
that holds.

> >> This means NOMAP is really only suited to punch some holes into the
> >> kernel direct mapping, and so implementing the memory limit by marking
> >> everything NOMAP is not the way to go. Instead, we should probably
> >> reorder the init sequence so that the regions that are reserved in the
> >> UEFI memory map are declared and marked NOMAP [again] after applying
> >> the memory limit in the old way.
> >>
> > Before this patch, I have another one addressing the same issue [1], with
> > that patch we'll not have these inconsistences, but it looks like a little
> > bit complicated, so it becomes current one. Any comments about that?
> >
> > [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html
> 
> The problem caused by mem= is that it removes regions that are marked
> NOMAP. So instead of marking everything above the limit NOMAP, I would
> much rather see an alternative implementation of
> memblock_enforce_memory_limit() that enforces the mem= limit by only
> removing memblocks that have to NOMAP flag cleared, and leaving the
> NOMAP ones where they are.

That would work for me.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-27  9:53                 ` Mark Rutland
  0 siblings, 0 replies; 21+ messages in thread
From: Mark Rutland @ 2016-06-27  9:53 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Dennis Chen, linux-arm-kernel, nd, Catalin Marinas, Steve Capper,
	Will Deacon, Rafael J . Wysocki, Matt Fleming, linux-mm,
	linux-acpi, linux-efi

On Fri, Jun 24, 2016 at 04:12:02PM +0200, Ard Biesheuvel wrote:
> On 24 June 2016 at 14:01, Dennis Chen <dennis.chen@arm.com> wrote:
> > On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> >> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
> >> >         /*
> >> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> >> > -        * high up in memory, add back the kernel region that must be accessible
> >> > -        * via the linear mapping.
> >> > +        * in the memory regions above the limit, so we need to clear the
> >> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> >> > +        * the linear mapping.
> >> >          */
> >> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> >> > -               memblock_enforce_memory_limit(memory_limit);
> >> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> >> > +               memblock_mem_limit_mark_nomap(memory_limit);
> >> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> >>
> >> Up until now, we have ignored the effect of having NOMAP memblocks on
> >> the return values of functions like memblock_phys_mem_size() and
> >> memblock_mem_size(), since they could reasonably be expected to cover
> >> only a small slice of all available memory. However, after applying
> >> this patch, it may well be the case that most of memory is marked
> >> NOMAP, and these functions will cease to work as expected.
> >>
> > Hi Ard, I noticed these inconsistences as you mentioned, but seems the
> > available memory is limited correctly. For this case('mem='), will it bring
> > some substantive side effects except that some log messages maybe confusing?
> 
> That is exactly the question that needs answering before we can merge
> these patches. I know we consider mem= a development hack, but the
> intent is to make it appear to the kernel as if only a smaller amount
> of memory is available to the kernel, and this is signficantly
> different from having memblock_mem_size() et al return much larger
> values than what is actually available. Perhaps this doesn't matter at
> all, but it is something we must discuss before proceeding with these
> changes.

Yeah, I think we need to figure out precisely what the expected
semantics are.

>From taking a look, memblock_mem_size() is only used by arch/x86. In
reserve_initrd, it's used to determine the amount of *free* memory, but
it counts reserved (and nomap) regions, so that doesn't feel right
regardless. For reserve_crashkernel_low it's not immediately clear to me
what it should do, as I've not gone digging.

There are many memblock_end_of_DRAM() users, mostly in arch code. We
(arm64) use it to determine the size of the linear map, and effectively
need it to be the limit for what should be mapped, which could/should
exclude nomap. I've not yet dug into the rest, so I don't know whether
that holds.

> >> This means NOMAP is really only suited to punch some holes into the
> >> kernel direct mapping, and so implementing the memory limit by marking
> >> everything NOMAP is not the way to go. Instead, we should probably
> >> reorder the init sequence so that the regions that are reserved in the
> >> UEFI memory map are declared and marked NOMAP [again] after applying
> >> the memory limit in the old way.
> >>
> > Before this patch, I have another one addressing the same issue [1], with
> > that patch we'll not have these inconsistences, but it looks like a little
> > bit complicated, so it becomes current one. Any comments about that?
> >
> > [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html
> 
> The problem caused by mem= is that it removes regions that are marked
> NOMAP. So instead of marking everything above the limit NOMAP, I would
> much rather see an alternative implementation of
> memblock_enforce_memory_limit() that enforces the mem= limit by only
> removing memblocks that have to NOMAP flag cleared, and leaving the
> NOMAP ones where they are.

That would work for me.

Thanks,
Mark.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-27  9:53                 ` Mark Rutland
  0 siblings, 0 replies; 21+ messages in thread
From: Mark Rutland @ 2016-06-27  9:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jun 24, 2016 at 04:12:02PM +0200, Ard Biesheuvel wrote:
> On 24 June 2016 at 14:01, Dennis Chen <dennis.chen@arm.com> wrote:
> > On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> >> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
> >> >         /*
> >> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> >> > -        * high up in memory, add back the kernel region that must be accessible
> >> > -        * via the linear mapping.
> >> > +        * in the memory regions above the limit, so we need to clear the
> >> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> >> > +        * the linear mapping.
> >> >          */
> >> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> >> > -               memblock_enforce_memory_limit(memory_limit);
> >> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> >> > +               memblock_mem_limit_mark_nomap(memory_limit);
> >> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> >>
> >> Up until now, we have ignored the effect of having NOMAP memblocks on
> >> the return values of functions like memblock_phys_mem_size() and
> >> memblock_mem_size(), since they could reasonably be expected to cover
> >> only a small slice of all available memory. However, after applying
> >> this patch, it may well be the case that most of memory is marked
> >> NOMAP, and these functions will cease to work as expected.
> >>
> > Hi Ard, I noticed these inconsistences as you mentioned, but seems the
> > available memory is limited correctly. For this case('mem='), will it bring
> > some substantive side effects except that some log messages maybe confusing?
> 
> That is exactly the question that needs answering before we can merge
> these patches. I know we consider mem= a development hack, but the
> intent is to make it appear to the kernel as if only a smaller amount
> of memory is available to the kernel, and this is signficantly
> different from having memblock_mem_size() et al return much larger
> values than what is actually available. Perhaps this doesn't matter at
> all, but it is something we must discuss before proceeding with these
> changes.

Yeah, I think we need to figure out precisely what the expected
semantics are.

>From taking a look, memblock_mem_size() is only used by arch/x86. In
reserve_initrd, it's used to determine the amount of *free* memory, but
it counts reserved (and nomap) regions, so that doesn't feel right
regardless. For reserve_crashkernel_low it's not immediately clear to me
what it should do, as I've not gone digging.

There are many memblock_end_of_DRAM() users, mostly in arch code. We
(arm64) use it to determine the size of the linear map, and effectively
need it to be the limit for what should be mapped, which could/should
exclude nomap. I've not yet dug into the rest, so I don't know whether
that holds.

> >> This means NOMAP is really only suited to punch some holes into the
> >> kernel direct mapping, and so implementing the memory limit by marking
> >> everything NOMAP is not the way to go. Instead, we should probably
> >> reorder the init sequence so that the regions that are reserved in the
> >> UEFI memory map are declared and marked NOMAP [again] after applying
> >> the memory limit in the old way.
> >>
> > Before this patch, I have another one addressing the same issue [1], with
> > that patch we'll not have these inconsistences, but it looks like a little
> > bit complicated, so it becomes current one. Any comments about that?
> >
> > [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html
> 
> The problem caused by mem= is that it removes regions that are marked
> NOMAP. So instead of marking everything above the limit NOMAP, I would
> much rather see an alternative implementation of
> memblock_enforce_memory_limit() that enforces the mem= limit by only
> removing memblocks that have to NOMAP flag cleared, and leaving the
> NOMAP ones where they are.

That would work for me.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
  2016-06-27  9:53                 ` Mark Rutland
  (?)
@ 2016-06-28  2:20                   ` Dennis Chen
  -1 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-28  2:20 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Ard Biesheuvel,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	nd-5wv7dgnIgG8, Catalin Marinas, Steve Capper, Will Deacon,
	Rafael J . Wysocki, Matt Fleming,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA

On Mon, Jun 27, 2016 at 10:53:18AM +0100, Mark Rutland wrote:
> On Fri, Jun 24, 2016 at 04:12:02PM +0200, Ard Biesheuvel wrote:
> > On 24 June 2016 at 14:01, Dennis Chen <dennis.chen-5wv7dgnIgG8@public.gmane.org> wrote:
> > > On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> > >> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen-5wv7dgnIgG8@public.gmane.org> wrote:
> > >> >         /*
> > >> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> > >> > -        * high up in memory, add back the kernel region that must be accessible
> > >> > -        * via the linear mapping.
> > >> > +        * in the memory regions above the limit, so we need to clear the
> > >> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> > >> > +        * the linear mapping.
> > >> >          */
> > >> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> > >> > -               memblock_enforce_memory_limit(memory_limit);
> > >> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> > >> > +               memblock_mem_limit_mark_nomap(memory_limit);
> > >> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> > >>
> > >> Up until now, we have ignored the effect of having NOMAP memblocks on
> > >> the return values of functions like memblock_phys_mem_size() and
> > >> memblock_mem_size(), since they could reasonably be expected to cover
> > >> only a small slice of all available memory. However, after applying
> > >> this patch, it may well be the case that most of memory is marked
> > >> NOMAP, and these functions will cease to work as expected.
> > >>
> > > Hi Ard, I noticed these inconsistences as you mentioned, but seems the
> > > available memory is limited correctly. For this case('mem='), will it bring
> > > some substantive side effects except that some log messages maybe confusing?
> > 
> > That is exactly the question that needs answering before we can merge
> > these patches. I know we consider mem= a development hack, but the
> > intent is to make it appear to the kernel as if only a smaller amount
> > of memory is available to the kernel, and this is signficantly
> > different from having memblock_mem_size() et al return much larger
> > values than what is actually available. Perhaps this doesn't matter at
> > all, but it is something we must discuss before proceeding with these
> > changes.
> 
> Yeah, I think we need to figure out precisely what the expected
> semantics are.
> 
> From taking a look, memblock_mem_size() is only used by arch/x86. In
> reserve_initrd, it's used to determine the amount of *free* memory, but
> it counts reserved (and nomap) regions, so that doesn't feel right
> regardless. For reserve_crashkernel_low it's not immediately clear to me
> what it should do, as I've not gone digging.
>
After rough digging go, memblock_mem_size() used by arch/x86 to calculate
the size of a segment of direct mapping physical memory, it only counts on 
memory memblock region regardless of the flag of that region, so from this
point, if we have a segment of memory marked as NOMAP, memblock_mem_size()
will still take its size into the total size and have it a direct mapped.
IMO memblock_mem_size() is not used to determine the amount of *free*, it
just to determine the amount of mem that can be mapped directly, so it's
reasonable to count reserved regions.
    
> 
> There are many memblock_end_of_DRAM() users, mostly in arch code. We
> (arm64) use it to determine the size of the linear map, and effectively
> need it to be the limit for what should be mapped, which could/should
> exclude nomap. I've not yet dug into the rest, so I don't know whether
> that holds.
>
we will use memblock_end_of_DRAM() to get the top boundary of the linear mapping,
given some memblock region is NOMAP, so some holes will be punched into the
linear mapping zone just as you mentioned those NOMAP should be excluded.
As my understanding, NOMAP regions only have possible potential side effect to count
the mem size such as memblock_mem_size 
> 
> > >> This means NOMAP is really only suited to punch some holes into the
> > >> kernel direct mapping, and so implementing the memory limit by marking
> > >> everything NOMAP is not the way to go. Instead, we should probably
> > >> reorder the init sequence so that the regions that are reserved in the
> > >> UEFI memory map are declared and marked NOMAP [again] after applying
> > >> the memory limit in the old way.
> > >>
> > > Before this patch, I have another one addressing the same issue [1], with
> > > that patch we'll not have these inconsistences, but it looks like a little
> > > bit complicated, so it becomes current one. Any comments about that?
> > >
> > > [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html
> > 
> > The problem caused by mem= is that it removes regions that are marked
> > NOMAP. So instead of marking everything above the limit NOMAP, I would
> > much rather see an alternative implementation of
> > memblock_enforce_memory_limit() that enforces the mem= limit by only
> > removing memblocks that have to NOMAP flag cleared, and leaving the
> > NOMAP ones where they are.
> 
> That would work for me.
> 
> Thanks,
> Mark.
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-28  2:20                   ` Dennis Chen
  0 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-28  2:20 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Ard Biesheuvel, linux-arm-kernel, Catalin Marinas, Steve Capper,
	Will Deacon, Rafael J . Wysocki, Matt Fleming, linux-mm,
	linux-acpi, linux-efi

On Mon, Jun 27, 2016 at 10:53:18AM +0100, Mark Rutland wrote:
> On Fri, Jun 24, 2016 at 04:12:02PM +0200, Ard Biesheuvel wrote:
> > On 24 June 2016 at 14:01, Dennis Chen <dennis.chen@arm.com> wrote:
> > > On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> > >> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
> > >> >         /*
> > >> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> > >> > -        * high up in memory, add back the kernel region that must be accessible
> > >> > -        * via the linear mapping.
> > >> > +        * in the memory regions above the limit, so we need to clear the
> > >> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> > >> > +        * the linear mapping.
> > >> >          */
> > >> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> > >> > -               memblock_enforce_memory_limit(memory_limit);
> > >> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> > >> > +               memblock_mem_limit_mark_nomap(memory_limit);
> > >> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> > >>
> > >> Up until now, we have ignored the effect of having NOMAP memblocks on
> > >> the return values of functions like memblock_phys_mem_size() and
> > >> memblock_mem_size(), since they could reasonably be expected to cover
> > >> only a small slice of all available memory. However, after applying
> > >> this patch, it may well be the case that most of memory is marked
> > >> NOMAP, and these functions will cease to work as expected.
> > >>
> > > Hi Ard, I noticed these inconsistences as you mentioned, but seems the
> > > available memory is limited correctly. For this case('mem='), will it bring
> > > some substantive side effects except that some log messages maybe confusing?
> > 
> > That is exactly the question that needs answering before we can merge
> > these patches. I know we consider mem= a development hack, but the
> > intent is to make it appear to the kernel as if only a smaller amount
> > of memory is available to the kernel, and this is signficantly
> > different from having memblock_mem_size() et al return much larger
> > values than what is actually available. Perhaps this doesn't matter at
> > all, but it is something we must discuss before proceeding with these
> > changes.
> 
> Yeah, I think we need to figure out precisely what the expected
> semantics are.
> 
> From taking a look, memblock_mem_size() is only used by arch/x86. In
> reserve_initrd, it's used to determine the amount of *free* memory, but
> it counts reserved (and nomap) regions, so that doesn't feel right
> regardless. For reserve_crashkernel_low it's not immediately clear to me
> what it should do, as I've not gone digging.
>
After rough digging go, memblock_mem_size() used by arch/x86 to calculate
the size of a segment of direct mapping physical memory, it only counts on 
memory memblock region regardless of the flag of that region, so from this
point, if we have a segment of memory marked as NOMAP, memblock_mem_size()
will still take its size into the total size and have it a direct mapped.
IMO memblock_mem_size() is not used to determine the amount of *free*, it
just to determine the amount of mem that can be mapped directly, so it's
reasonable to count reserved regions.
    
> 
> There are many memblock_end_of_DRAM() users, mostly in arch code. We
> (arm64) use it to determine the size of the linear map, and effectively
> need it to be the limit for what should be mapped, which could/should
> exclude nomap. I've not yet dug into the rest, so I don't know whether
> that holds.
>
we will use memblock_end_of_DRAM() to get the top boundary of the linear mapping,
given some memblock region is NOMAP, so some holes will be punched into the
linear mapping zone just as you mentioned those NOMAP should be excluded.
As my understanding, NOMAP regions only have possible potential side effect to count
the mem size such as memblock_mem_size 
> 
> > >> This means NOMAP is really only suited to punch some holes into the
> > >> kernel direct mapping, and so implementing the memory limit by marking
> > >> everything NOMAP is not the way to go. Instead, we should probably
> > >> reorder the init sequence so that the regions that are reserved in the
> > >> UEFI memory map are declared and marked NOMAP [again] after applying
> > >> the memory limit in the old way.
> > >>
> > > Before this patch, I have another one addressing the same issue [1], with
> > > that patch we'll not have these inconsistences, but it looks like a little
> > > bit complicated, so it becomes current one. Any comments about that?
> > >
> > > [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html
> > 
> > The problem caused by mem= is that it removes regions that are marked
> > NOMAP. So instead of marking everything above the limit NOMAP, I would
> > much rather see an alternative implementation of
> > memblock_enforce_memory_limit() that enforces the mem= limit by only
> > removing memblocks that have to NOMAP flag cleared, and leaving the
> > NOMAP ones where they are.
> 
> That would work for me.
> 
> Thanks,
> Mark.
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified
@ 2016-06-28  2:20                   ` Dennis Chen
  0 siblings, 0 replies; 21+ messages in thread
From: Dennis Chen @ 2016-06-28  2:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jun 27, 2016 at 10:53:18AM +0100, Mark Rutland wrote:
> On Fri, Jun 24, 2016 at 04:12:02PM +0200, Ard Biesheuvel wrote:
> > On 24 June 2016 at 14:01, Dennis Chen <dennis.chen@arm.com> wrote:
> > > On Fri, Jun 24, 2016 at 12:43:52PM +0200, Ard Biesheuvel wrote:
> > >> On 24 June 2016 at 05:13, Dennis Chen <dennis.chen@arm.com> wrote:
> > >> >         /*
> > >> >          * Apply the memory limit if it was set. Since the kernel may be loaded
> > >> > -        * high up in memory, add back the kernel region that must be accessible
> > >> > -        * via the linear mapping.
> > >> > +        * in the memory regions above the limit, so we need to clear the
> > >> > +        * MEMBLOCK_NOMAP flag of this region to make it can be accessible via
> > >> > +        * the linear mapping.
> > >> >          */
> > >> >         if (memory_limit != (phys_addr_t)ULLONG_MAX) {
> > >> > -               memblock_enforce_memory_limit(memory_limit);
> > >> > -               memblock_add(__pa(_text), (u64)(_end - _text));
> > >> > +               memblock_mem_limit_mark_nomap(memory_limit);
> > >> > +               memblock_clear_nomap(__pa(_text), (u64)(_end - _text));
> > >>
> > >> Up until now, we have ignored the effect of having NOMAP memblocks on
> > >> the return values of functions like memblock_phys_mem_size() and
> > >> memblock_mem_size(), since they could reasonably be expected to cover
> > >> only a small slice of all available memory. However, after applying
> > >> this patch, it may well be the case that most of memory is marked
> > >> NOMAP, and these functions will cease to work as expected.
> > >>
> > > Hi Ard, I noticed these inconsistences as you mentioned, but seems the
> > > available memory is limited correctly. For this case('mem='), will it bring
> > > some substantive side effects except that some log messages maybe confusing?
> > 
> > That is exactly the question that needs answering before we can merge
> > these patches. I know we consider mem= a development hack, but the
> > intent is to make it appear to the kernel as if only a smaller amount
> > of memory is available to the kernel, and this is signficantly
> > different from having memblock_mem_size() et al return much larger
> > values than what is actually available. Perhaps this doesn't matter at
> > all, but it is something we must discuss before proceeding with these
> > changes.
> 
> Yeah, I think we need to figure out precisely what the expected
> semantics are.
> 
> From taking a look, memblock_mem_size() is only used by arch/x86. In
> reserve_initrd, it's used to determine the amount of *free* memory, but
> it counts reserved (and nomap) regions, so that doesn't feel right
> regardless. For reserve_crashkernel_low it's not immediately clear to me
> what it should do, as I've not gone digging.
>
After rough digging go, memblock_mem_size() used by arch/x86 to calculate
the size of a segment of direct mapping physical memory, it only counts on 
memory memblock region regardless of the flag of that region, so from this
point, if we have a segment of memory marked as NOMAP, memblock_mem_size()
will still take its size into the total size and have it a direct mapped.
IMO memblock_mem_size() is not used to determine the amount of *free*, it
just to determine the amount of mem that can be mapped directly, so it's
reasonable to count reserved regions.
    
> 
> There are many memblock_end_of_DRAM() users, mostly in arch code. We
> (arm64) use it to determine the size of the linear map, and effectively
> need it to be the limit for what should be mapped, which could/should
> exclude nomap. I've not yet dug into the rest, so I don't know whether
> that holds.
>
we will use memblock_end_of_DRAM() to get the top boundary of the linear mapping,
given some memblock region is NOMAP, so some holes will be punched into the
linear mapping zone just as you mentioned those NOMAP should be excluded.
As my understanding, NOMAP regions only have possible potential side effect to count
the mem size such as memblock_mem_size 
> 
> > >> This means NOMAP is really only suited to punch some holes into the
> > >> kernel direct mapping, and so implementing the memory limit by marking
> > >> everything NOMAP is not the way to go. Instead, we should probably
> > >> reorder the init sequence so that the regions that are reserved in the
> > >> UEFI memory map are declared and marked NOMAP [again] after applying
> > >> the memory limit in the old way.
> > >>
> > > Before this patch, I have another one addressing the same issue [1], with
> > > that patch we'll not have these inconsistences, but it looks like a little
> > > bit complicated, so it becomes current one. Any comments about that?
> > >
> > > [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2016-June/438443.html
> > 
> > The problem caused by mem= is that it removes regions that are marked
> > NOMAP. So instead of marking everything above the limit NOMAP, I would
> > much rather see an alternative implementation of
> > memblock_enforce_memory_limit() that enforces the mem= limit by only
> > removing memblocks that have to NOMAP flag cleared, and leaving the
> > NOMAP ones where they are.
> 
> That would work for me.
> 
> Thanks,
> Mark.
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2016-06-28  2:20 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-24  3:13 [PATCH v2 1/2] mm: memblock Add some new functions to address the mem limit issue Dennis Chen
2016-06-24  3:13 ` Dennis Chen
     [not found] ` <1466738027-15066-1-git-send-email-dennis.chen-5wv7dgnIgG8@public.gmane.org>
2016-06-24  3:13   ` [PATCH v2 2/2] arm64:acpi Fix the acpi alignment exeception when 'mem=' specified Dennis Chen
2016-06-24  3:13     ` Dennis Chen
2016-06-24  3:13     ` Dennis Chen
2016-06-24 10:43     ` Ard Biesheuvel
2016-06-24 10:43       ` Ard Biesheuvel
     [not found]       ` <CAKv+Gu8ZyWG-OZ8=2u9jrdS-0j+qL1sstPQ0uX=j7wyj+ETo-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-06-24 12:01         ` Dennis Chen
2016-06-24 12:01           ` Dennis Chen
2016-06-24 12:01           ` Dennis Chen
2016-06-24 14:12           ` Ard Biesheuvel
2016-06-24 14:12             ` Ard Biesheuvel
     [not found]             ` <CAKv+Gu9XHYVEoL846WBx6PZqSnbBCjwup0CPkZ1JexJVkvds9A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-06-27  1:20               ` Dennis Chen
2016-06-27  1:20                 ` Dennis Chen
2016-06-27  1:20                 ` Dennis Chen
2016-06-27  9:53               ` Mark Rutland
2016-06-27  9:53                 ` Mark Rutland
2016-06-27  9:53                 ` Mark Rutland
2016-06-28  2:20                 ` Dennis Chen
2016-06-28  2:20                   ` Dennis Chen
2016-06-28  2:20                   ` Dennis Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.