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
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 <[email protected]>
---
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
Signed-off-by: Jonathan Neuschäfer <[email protected]>
---
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
Signed-off-by: Jonathan Neuschäfer <[email protected]>
---
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
Instead of open-coding the search in page_is_ram, call memblock_is_memory.
Signed-off-by: Jonathan Neuschäfer <[email protected]>
---
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
This hack, introduced in commit c5df7f775148 ("powerpc: allow ioremap
within reserved memory regions") is now unnecessary.
Signed-off-by: Jonathan Neuschäfer <[email protected]>
---
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
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(-)
>
On Fri, Feb 23, 2018 at 09:01:17AM +0100, Christophe LEROY wrote:
>
>
> 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.
Thanks for testing it!
Jonathan Neuschäfer
Jonathan Neuschäfer <[email protected]> writes:
> Signed-off-by: Jonathan Neuschäfer <[email protected]>
> ---
> 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=24 CoreNet Generic
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.16.0-rc4-gcc-4.6.3-00076-g85319478bdb4 #86
NIP: c0029080 LR: c0029020 CTR: 00000001
REGS: e804bd40 TRAP: 0300 Not tainted (4.16.0-rc4-gcc-4.6.3-00076-g85319478bdb4)
MSR: 00021002 <CE,ME> CR: 24ad4e22 XER: 00000000
DEAR: 00000000 ESR: 00000000
GPR00: c0029020 e804bdf0 e8050000 00000000 00021002 0000004d 00000000 c0aaaeed
GPR08: 00000000 00000000 00000000 2d57d000 22adbe84 00000000 c0002630 00000000
GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 c0a8b4a4
GPR24: 00000001 00000000 00000000 00029002 00000000 00000001 00000001 00000001
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
7f0903a6 60000000 60000000 60000000 <7c0048ac> 39290040 4200fff8 7c0004ac
random: get_random_bytes called from init_oops_id+0x5c/0x70 with crng_init=0
---[ 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=24 CoreNet Generic
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.16.0-rc4-gcc-4.6.3-00076-g85319478bdb4-dirty #90
NIP: c00290a0 LR: c0029040 CTR: 00000001
REGS: e804bd50 TRAP: 0300 Not tainted (4.16.0-rc4-gcc-4.6.3-00076-g85319478bdb4-dirty)
MSR: 00021002 <CE,ME> CR: 24a24e22 XER: 00000000
DEAR: 3fef5140 ESR: 00000000
GPR00: c0029040 e804be00 e8050000 00000023 00021002 0000004e 00000000 c0aaaed3
GPR08: 00000000 3fef5140 00000000 2d57d000 22a2be84 00000000 c0002630 00000000
GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 c0a8b4a4
GPR24: 00000004 00000001 3fef5140 3fef5140 00029002 3fef5140 00000001 00000001
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
4192001c 7f49d378 7f2903a6 60000000 <7c0048ac> 39290040 4200fff8 7c0004ac
random: get_random_bytes called from init_oops_id+0x5c/0x70 with crng_init=0
---[ end trace 950df40ee04f2d5e ]---
So that will require a bit more debugging.
cheers
Michael Ellerman <[email protected]> writes:
> Jonathan Neuschäfer <[email protected]> writes:
>
>> Signed-off-by: Jonathan Neuschäfer <[email protected]>
>> ---
>> 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:
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
Hi,
On Mon, Mar 19, 2018 at 10:19:32PM +1100, Michael Ellerman wrote:
> Michael Ellerman <[email protected]> writes:
> > Jonathan Neuschäfer <[email protected]> writes:
[...]
> >> - 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