[PATCH 0/5] PPC32/ioremap: Use memblock API to check for RAM

STALE3054d

Revision v1 of 2 in this series.

11 messages, 3 authors, 2018-03-27 · open the first message on its own page

[PATCH 0/5] PPC32/ioremap: Use memblock API to check for RAM

From: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Date: 2018-02-22 12:15:30

This patchset solves the same problem as my previous one[1] but follows
a rather different approach. Instead of implementing DISCONTIGMEM for
PowerPC32, I simply switched the "is this RAM" check in __ioremap_caller
to the existing page_is_ram function, and unified page_is_ram to search
memblock.memory on PPC64 and PPC32.

The intended result is, as before, that my Wii can allocate the MMIO
range of its GPIO controller, which was previously not possible, because
the reserved memory hack (__allow_ioremap_reserved) didn't affect the
API in kernel/resource.c.

Thanks to Christophe Leroy for reviewing the previous patchset.

[1]: https://www.spinics.net/lists/kernel/msg2726786.html

Jonathan Neuschäfer (5):
  powerpc: mm: Simplify page_is_ram by using memblock_is_memory
  powerpc: mm: Use memblock API for PPC32 page_is_ram
  powerpc/mm/32: Use page_is_ram to check for RAM
  powerpc: wii: Don't rely on the reserved memory hack
  powerpc/mm/32: Remove the reserved memory hack

 arch/powerpc/mm/init_32.c                |  5 -----
 arch/powerpc/mm/mem.c                    | 12 +-----------
 arch/powerpc/mm/mmu_decl.h               |  1 -
 arch/powerpc/mm/pgtable_32.c             |  4 +---
 arch/powerpc/platforms/embedded6xx/wii.c | 14 +-------------
 5 files changed, 3 insertions(+), 33 deletions(-)

-- 
2.16.1

[PATCH 1/5] powerpc: mm: Simplify page_is_ram by using memblock_is_memory

From: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Date: 2018-02-22 12:16:02

Instead of open-coding the search in page_is_ram, call memblock_is_memory.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 arch/powerpc/mm/mem.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index fe8c61149fb8..da4e1555d61d 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c

@@ -85,13 +85,7 @@ int page_is_ram(unsigned long pfn)
 #ifndef CONFIG_PPC64	/* XXX for now */
 	return pfn < max_pfn;
 #else
-	unsigned long paddr = (pfn << PAGE_SHIFT);
-	struct memblock_region *reg;
-
-	for_each_memblock(memory, reg)
-		if (paddr >= reg->base && paddr < (reg->base + reg->size))
-			return 1;
-	return 0;
+	return memblock_is_memory(__pfn_to_phys(pfn));
 #endif
 }

-- 
2.16.1

[PATCH 2/5] powerpc: mm: Use memblock API for PPC32 page_is_ram

From: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Date: 2018-02-22 12:16:07

To support accurate checking for different blocks of memory on PPC32,
use the same memblock-based approach that's already used on PPC64 also
on PPC32.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 arch/powerpc/mm/mem.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index da4e1555d61d..a42b86e2a34c 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c

@@ -82,11 +82,7 @@ static inline pte_t *virt_to_kpte(unsigned long vaddr)
 
 int page_is_ram(unsigned long pfn)
 {
-#ifndef CONFIG_PPC64	/* XXX for now */
-	return pfn < max_pfn;
-#else
 	return memblock_is_memory(__pfn_to_phys(pfn));
-#endif
 }
 
 pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,

-- 
2.16.1

[PATCH 4/5] powerpc: wii: Don't rely on the reserved memory hack

From: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Date: 2018-02-22 12:16:32

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 arch/powerpc/platforms/embedded6xx/wii.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/arch/powerpc/platforms/embedded6xx/wii.c b/arch/powerpc/platforms/embedded6xx/wii.c
index 4682327f76a9..fc00d82691e1 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c

@@ -81,21 +81,9 @@ void __init wii_memory_fixups(void)
 	BUG_ON(memblock.memory.cnt != 2);
 	BUG_ON(!page_aligned(p[0].base) || !page_aligned(p[1].base));
 
-	/* trim unaligned tail */
-	memblock_remove(ALIGN(p[1].base + p[1].size, PAGE_SIZE),
-			(phys_addr_t)ULLONG_MAX);
-
-	/* determine hole, add & reserve them */
+	/* determine hole */
 	wii_hole_start = ALIGN(p[0].base + p[0].size, PAGE_SIZE);
 	wii_hole_size = p[1].base - wii_hole_start;
-	memblock_add(wii_hole_start, wii_hole_size);
-	memblock_reserve(wii_hole_start, wii_hole_size);
-
-	BUG_ON(memblock.memory.cnt != 1);
-	__memblock_dump_all();
-
-	/* allow ioremapping the address space in the hole */
-	__allow_ioremap_reserved = 1;
 }
 
 unsigned long __init wii_mmu_mapin_mem2(unsigned long top)

-- 
2.16.1

[PATCH 3/5] powerpc/mm/32: Use page_is_ram to check for RAM

From: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Date: 2018-02-22 12:16:54

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 arch/powerpc/mm/pgtable_32.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index d35d9ad3c1cd..d54e1a9c1c99 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c

@@ -145,9 +145,8 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, unsigned long flags,
 #ifndef CONFIG_CRASH_DUMP
 	/*
 	 * Don't allow anybody to remap normal RAM that we're using.
-	 * mem_init() sets high_memory so only do the check after that.
 	 */
-	if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
+	if (page_is_ram(__phys_to_pfn(p)) &&
 	    !(__allow_ioremap_reserved && memblock_is_region_reserved(p, size))) {
 		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
 		       (unsigned long long)p, __builtin_return_address(0));

-- 
2.16.1

[PATCH 5/5] powerpc/mm/32: Remove the reserved memory hack

From: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Date: 2018-02-22 12:21:57

This hack, introduced in commit c5df7f775148 ("powerpc: allow ioremap
within reserved memory regions") is now unnecessary.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 arch/powerpc/mm/init_32.c    | 5 -----
 arch/powerpc/mm/mmu_decl.h   | 1 -
 arch/powerpc/mm/pgtable_32.c | 3 +--
 3 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/powerpc/mm/init_32.c b/arch/powerpc/mm/init_32.c
index 6419b33ca309..326240177fa6 100644
--- a/arch/powerpc/mm/init_32.c
+++ b/arch/powerpc/mm/init_32.c

@@ -88,11 +88,6 @@ void MMU_init(void);
 int __map_without_bats;
 int __map_without_ltlbs;
 
-/*
- * This tells the system to allow ioremapping memory marked as reserved.
- */
-int __allow_ioremap_reserved;
-
 /* max amount of low RAM to map in */
 unsigned long __max_low_memory = MAX_LOW_MEM;

diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 57fbc554c785..c4c0a09a7775 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h

@@ -98,7 +98,6 @@ extern void setbat(int index, unsigned long virt, phys_addr_t phys,
 		   unsigned int size, pgprot_t prot);
 
 extern int __map_without_bats;
-extern int __allow_ioremap_reserved;
 extern unsigned int rtas_data, rtas_size;
 
 struct hash_pte;

diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index d54e1a9c1c99..a89eb3b898cd 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c

@@ -146,8 +146,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, unsigned long flags,
 	/*
 	 * Don't allow anybody to remap normal RAM that we're using.
 	 */
-	if (page_is_ram(__phys_to_pfn(p)) &&
-	    !(__allow_ioremap_reserved && memblock_is_region_reserved(p, size))) {
+	if (page_is_ram(__phys_to_pfn(p))) {
 		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
 		       (unsigned long long)p, __builtin_return_address(0));
 		return NULL;

-- 
2.16.1

Re: [PATCH 0/5] PPC32/ioremap: Use memblock API to check for RAM

From: Christophe LEROY <hidden>
Date: 2018-02-23 08:01:05


Le 22/02/2018 à 13:15, Jonathan Neuschäfer a écrit :

This patchset solves the same problem as my previous one[1] but follows
a rather different approach. Instead of implementing DISCONTIGMEM for
PowerPC32, I simply switched the "is this RAM" check in __ioremap_caller
to the existing page_is_ram function, and unified page_is_ram to search
memblock.memory on PPC64 and PPC32.

The intended result is, as before, that my Wii can allocate the MMIO
range of its GPIO controller, which was previously not possible, because
the reserved memory hack (__allow_ioremap_reserved) didn't affect the
API in kernel/resource.c.

Thanks to Christophe Leroy for reviewing the previous patchset.

I tested your new serie, it doesn't break my 8xx so it is OK for me.

Christophe

[1]: https://www.spinics.net/lists/kernel/msg2726786.html

Jonathan Neuschäfer (5):
   powerpc: mm: Simplify page_is_ram by using memblock_is_memory
   powerpc: mm: Use memblock API for PPC32 page_is_ram
   powerpc/mm/32: Use page_is_ram to check for RAM
   powerpc: wii: Don't rely on the reserved memory hack
   powerpc/mm/32: Remove the reserved memory hack

  arch/powerpc/mm/init_32.c                |  5 -----
  arch/powerpc/mm/mem.c                    | 12 +-----------
  arch/powerpc/mm/mmu_decl.h               |  1 -
  arch/powerpc/mm/pgtable_32.c             |  4 +---
  arch/powerpc/platforms/embedded6xx/wii.c | 14 +-------------
  5 files changed, 3 insertions(+), 33 deletions(-)

Re: [PATCH 0/5] PPC32/ioremap: Use memblock API to check for RAM

From: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Date: 2018-02-24 13:48:42

On Fri, Feb 23, 2018 at 09:01:17AM +0100, Christophe LEROY wrote:


Le 22/02/2018 à 13:15, Jonathan Neuschäfer a écrit :

quoted

This patchset solves the same problem as my previous one[1] but follows
a rather different approach. Instead of implementing DISCONTIGMEM for
PowerPC32, I simply switched the "is this RAM" check in __ioremap_caller
to the existing page_is_ram function, and unified page_is_ram to search
memblock.memory on PPC64 and PPC32.

The intended result is, as before, that my Wii can allocate the MMIO
range of its GPIO controller, which was previously not possible, because
the reserved memory hack (__allow_ioremap_reserved) didn't affect the
API in kernel/resource.c.

Thanks to Christophe Leroy for reviewing the previous patchset.

I tested your new serie, it doesn't break my 8xx so it is OK for me.

Thanks for testing it!


Jonathan Neuschäfer

Re: [PATCH 3/5] powerpc/mm/32: Use page_is_ram to check for RAM

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2018-03-19 06:05:40

Jonathan Neusch=C3=A4fer [off-list ref] writes:

quoted hunk

Signed-off-by: Jonathan Neusch=C3=A4fer <j.neuschaefer@gmx.net>
---
 arch/powerpc/mm/pgtable_32.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index d35d9ad3c1cd..d54e1a9c1c99 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c

@@ -145,9 +145,8 @@ __ioremap_caller(phys_addr_t addr, unsigned long size=

, unsigned long flags,

 #ifndef CONFIG_CRASH_DUMP
 	/*
 	 * Don't allow anybody to remap normal RAM that we're using.
-	 * mem_init() sets high_memory so only do the check after that.
 	 */
-	if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
+	if (page_is_ram(__phys_to_pfn(p)) &&
 	    !(__allow_ioremap_reserved && memblock_is_region_reserved(p, size))=

) {

 		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
 		       (unsigned long long)p, __builtin_return_address(0));


This is killing my p5020ds (Freescale e5500) unfortunately:

  smp: Bringing up secondary CPUs ...
  __ioremap(): phys addr 0x7fef5000 is RAM lr smp_85xx_kick_cpu
  Unable to handle kernel paging request for data at address 0x00000000
  Faulting instruction address: 0xc0029080
  Oops: Kernel access of bad area, sig: 11 [#1]
  BE SMP NR_CPUS=3D24 CoreNet Generic
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.16.0-rc4-gcc-4.6.3-00076-g853=
19478bdb4 #86
  NIP:  c0029080 LR: c0029020 CTR: 00000001
  REGS: e804bd40 TRAP: 0300   Not tainted  (4.16.0-rc4-gcc-4.6.3-00076-g853=
19478bdb4)
  MSR:  00021002 <CE,ME>  CR: 24ad4e22  XER: 00000000
  DEAR: 00000000 ESR: 00000000=20
  GPR00: c0029020 e804bdf0 e8050000 00000000 00021002 0000004d 00000000 c0a=
aaeed=20
  GPR08: 00000000 00000000 00000000 2d57d000 22adbe84 00000000 c0002630 000=
00000=20
  GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 c0a=
8b4a4=20
  GPR24: 00000001 00000000 00000000 00029002 00000000 00000001 00000001 000=
00001=20
  NIP [c0029080] smp_85xx_kick_cpu+0x100/0x2c0
  LR [c0029020] smp_85xx_kick_cpu+0xa0/0x2c0
  Call Trace:
  [e804bdf0] [c0029020] smp_85xx_kick_cpu+0xa0/0x2c0 (unreliable)
  [e804be30] [c0011194] __cpu_up+0xb4/0x1c0
  [e804be60] [c002f16c] bringup_cpu+0x2c/0xf0
  [e804be80] [c002ec9c] cpuhp_invoke_callback+0x12c/0x310
  [e804beb0] [c002fdd8] cpu_up+0x108/0x230
  [e804bee0] [c09f7438] smp_init+0x84/0x104
  [e804bf00] [c09e9acc] kernel_init_freeable+0xc4/0x228
  [e804bf30] [c0002644] kernel_init+0x14/0x110
  [e804bf40] [c000f3b0] ret_from_kernel_thread+0x5c/0x64
  Instruction dump:
  57990032 3b1c0057 7f19c050 7f3acb78 5718d1be 2e180000 41920024 7f29cb78=20
  7f0903a6 60000000 60000000 60000000 <7c0048ac> 39290040 4200fff8 7c0004ac=
=20
  random: get_random_bytes called from init_oops_id+0x5c/0x70 with crng_ini=
t=3D0
  ---[ end trace c3807aa91cf16cd8 ]---


The obvious fix of changing the test in smp_85xx_start_cpu() didn't
work, I get a different oops:

  Unable to handle kernel paging request for data at address 0x3fef5140
  Faulting instruction address: 0xc00290a0
  Oops: Kernel access of bad area, sig: 11 [#1]
  BE SMP NR_CPUS=3D24 CoreNet Generic
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.16.0-rc4-gcc-4.6.3-00076-g853=
19478bdb4-dirty #90
  NIP:  c00290a0 LR: c0029040 CTR: 00000001
  REGS: e804bd50 TRAP: 0300   Not tainted  (4.16.0-rc4-gcc-4.6.3-00076-g853=
19478bdb4-dirty)
  MSR:  00021002 <CE,ME>  CR: 24a24e22  XER: 00000000
  DEAR: 3fef5140 ESR: 00000000=20
  GPR00: c0029040 e804be00 e8050000 00000023 00021002 0000004e 00000000 c0a=
aaed3=20
  GPR08: 00000000 3fef5140 00000000 2d57d000 22a2be84 00000000 c0002630 000=
00000=20
  GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 c0a=
8b4a4=20
  GPR24: 00000004 00000001 3fef5140 3fef5140 00029002 3fef5140 00000001 000=
00001=20
  NIP [c00290a0] smp_85xx_kick_cpu+0x120/0x2e0
  LR [c0029040] smp_85xx_kick_cpu+0xc0/0x2e0
  Call Trace:
  [e804be00] [c0029040] smp_85xx_kick_cpu+0xc0/0x2e0 (unreliable)
  [e804be30] [c0011194] __cpu_up+0xb4/0x1c0
  [e804be60] [c002f18c] bringup_cpu+0x2c/0xf0
  [e804be80] [c002ecbc] cpuhp_invoke_callback+0x12c/0x310
  [e804beb0] [c002fdf8] cpu_up+0x108/0x230
  [e804bee0] [c09f7438] smp_init+0x84/0x104
  [e804bf00] [c09e9acc] kernel_init_freeable+0xc4/0x228
  [e804bf30] [c0002644] kernel_init+0x14/0x110
  [e804bf40] [c000f3b0] ret_from_kernel_thread+0x5c/0x64
  Instruction dump:
  7c0903a6 4e800421 57ba0032 3b3d0057 7f3ac850 7f5bd378 5739d1be 2e190000=20
  4192001c 7f49d378 7f2903a6 60000000 <7c0048ac> 39290040 4200fff8 7c0004ac=
=20
  random: get_random_bytes called from init_oops_id+0x5c/0x70 with crng_ini=
t=3D0
  ---[ end trace 950df40ee04f2d5e ]---


So that will require a bit more debugging.

cheers

Re: [PATCH 3/5] powerpc/mm/32: Use page_is_ram to check for RAM

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2018-03-19 11:19:42

Michael Ellerman [off-list ref] writes:

Jonathan Neusch=C3=A4fer [off-list ref] writes:

quoted

Signed-off-by: Jonathan Neusch=C3=A4fer <j.neuschaefer@gmx.net>
---
 arch/powerpc/mm/pgtable_32.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index d35d9ad3c1cd..d54e1a9c1c99 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c

@@ -145,9 +145,8 @@ __ioremap_caller(phys_addr_t addr, unsigned long siz=

e, unsigned long flags,

quoted

 #ifndef CONFIG_CRASH_DUMP
 	/*
 	 * Don't allow anybody to remap normal RAM that we're using.
-	 * mem_init() sets high_memory so only do the check after that.
 	 */
-	if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
+	if (page_is_ram(__phys_to_pfn(p)) &&
 	    !(__allow_ioremap_reserved && memblock_is_region_reserved(p, size)=

)) {

quoted

 		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
 		       (unsigned long long)p, __builtin_return_address(0));


This is killing my p5020ds (Freescale e5500) unfortunately:

Duh, I should actually read the patch :)

This is a 32-bit system with 4G of RAM, so not all of RAM is mapped,
some of it is highem which is why removing the test against high_memory
above breaks it.

So I need the high_memory test on this system.

I'm not clear why it was a problem for you on the Wii, do you even build
the Wii kernel with HIGHMEM enabled?

cheers

Re: [PATCH 3/5] powerpc/mm/32: Use page_is_ram to check for RAM

From: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Date: 2018-03-27 19:25:07

Hi,

On Mon, Mar 19, 2018 at 10:19:32PM +1100, Michael Ellerman wrote:

Michael Ellerman [off-list ref] writes:

quoted

Jonathan Neuschäfer [off-list ref] writes:

[...]

quoted

-	if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
+	if (page_is_ram(__phys_to_pfn(p)) &&
 	    !(__allow_ioremap_reserved && memblock_is_region_reserved(p, size))) {
 		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
 		       (unsigned long long)p, __builtin_return_address(0));


This is killing my p5020ds (Freescale e5500) unfortunately:

Duh, I should actually read the patch :)

This is a 32-bit system with 4G of RAM, so not all of RAM is mapped,
some of it is highem which is why removing the test against high_memory
above breaks it.

So I need the high_memory test on this system.

This is an oversight on my part. I thought I wouldn't need this test
because the memblock-based test is more accurate, but I didn't think
through how high memory actually works.

I'm not clear why it was a problem for you on the Wii, do you even build
the Wii kernel with HIGHMEM enabled?

No. The Wii works fine with the p < virt_to_phys(high_memory) test, and
doesn't use CONFIG_HIGHMEM.  I'll send a version two of this patchset.


Thanks for testing,
Jonathan Neuschäfer

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help