Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Christophe Leroy]

Hi Maxime,


Le 03/12/2021 à 01:44, Maxime Bizon a écrit :
> 
> Hello Christophe,
> 
> I have a mpc8347 board booting 5.15 fine, but it does not boot with
> CONFIG_DEBUG_PAGEALLOC=y (and enabled) or "nobats".
> 
> Those two options worked fine on my previous kernel (5.4)
> 
> 
> Nothing is output on serial console when I set those options, so I had
> to hack a bit:
> 
> 1) to ease debugging, I used mem=256M, so only a few BATs are used
> 
> 2) I hijacked BAT 7 and the vm space at b000xxxx (modules, unused for
> me) to set a constant mapping so my uart always works (patch attached
> in case)
> 
> 
> I got this:
> 
> mmu_mapin_ram: base:0x0 top:0x10000000 border:0x600000
> mmu_mapin_ram: updated base:0x0 top:0x600000
> setbat index=0 virt=0xc0000000 phys=0x0 size=0x400000
> setbat index=1 virt=0xc0400000 phys=0x400000 size=0x200000
> [    0.000000] kernel tried to execute exec-protected page (c0600944) - exploit attempt? (uid: 0)
> [    0.000000] BUG: Unable to handle kernel instruction fetch
> [    0.000000] Faulting instruction address: 0xc0600944
> [    0.000000] Thread overran stack, or stack corrupted
> [    0.000000] Oops: Kernel access of bad area, sig: 11 [#1]
> [    0.000000] BE PAGE_SIZE=4K DEBUG_PAGEALLOC
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.15.0+ #237
> [    0.000000] NIP:  c0600944 LR: 00003390 CTR: 00000000
> [    0.000000] REGS: c07bbf40 TRAP: 0400   Not tainted  (5.15.0+)
> [    0.000000] MSR:  20001032 <ME,IR,DR,RI>  CR: 48222444  XER: 20000000
> [    0.000000]
> [    0.000000] GPR00: c0003364 c07bbff0 c0707580 c0600944 00001032 00000000 c077dc8c 00000000
> [    0.000000] GPR08: c07d0000 00000001 c07d0000 ffffffff 88222444 00000000 3ff9c5f0 3fffd79c
> [    0.000000] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    0.000000] GPR24: 00000000 00000000 40000000 3ff9c5d8 03ffb000 00000000 3fffe4d4 03ffb000
> [    0.000000] NIP [c0600944] start_kernel+0x0/0x600
> [    0.000000] LR [00003390] 0x3390
> [    0.000000] Call Trace:
> [    0.000000] [c07bbff0] [c0003364] start_here+0x4c/0x90 (unreliable)
> [    0.000000] Instruction dump:
> [    0.000000] 83e1000c 7c0803a6 38210010 4e800020 4e800020 4ba05bb8 4e800020 4e800020
> [    0.000000] 4e800020 4e800020 4e800020 4e800020 <9421ffc0> 3c60c070 7c0802a6 38637580
> [    0.000000] random: get_random_bytes called from oops_exit+0x44/0x84 with crng_init=0
> [    0.000000] ---[ end trace 0000000000000000 ]---
> [    0.000000]
> [    0.000000] Kernel panic - not syncing: Fatal exception
> [    0.000000] Rebooting in 30 seconds..
> 
> 
> 
> It gets a page fault when jumping on start_kernel(), it seems the
> init.text section is not mapped, at least not in execute.
> 
> debug_pagealloc_enabled changes behaviour of mmu_mapin_ram(), it will
> map only up to __init_begin with BATs, and use page mappings for the
> reminder.
> 
> my debug prints confirm that it seems to do a correct job at it, I've
> also verified that __mapin_ram_chunk() was indeed mapping the while
> init.text area with prot=PAGE_KERNEL_TEXT
> 
> but it's as if those page mappings had no effect at all, and I am not
> familiar with PPC MMU to dig further
> 
> Simply changing mmu_mapin_ram() to not to enter the "if
> debug_pagealloc_enabled_or_kfence()" makes the kernel boot fine.
> 
> If you have any guidance that would be appreciated
> 

Thanks for the report.

This problem doesn't happen on powermac on QEMU.
I was able to reproduce this problem on an mpc8321 board.

It happens when CONFIG_MODULES is not defined, in that case the 
Instruction TLB miss exception handler doesn't expect such exception at 
all because all kernel text is expected to be mapped with IBATs. 
However, due to DEBUGPAGE_ALLOC, only main text is mapped with BATs, not 
inittext. That's a mistake. inittext should still be mapped with BATs.

When CONFIG_MODULES is set it works.

One way to fix it is to drop this CONFIG_MODULES #ifdefs in instruction 
TLB miss handler (In kernel/head_book3s_32.S), but that would kill 
performance for just the sake init.

Another way to fix it is to set an IBAT coverring up to _einittext. This 
IBAT should be removed by mark_initmem_nx() at the end of init ... but 
... it looks like we have a problem there as well: as we have not mapped 
_sinittext by DBATs, mmu_mark_initmem_nx() is not called.
Also, as we are setting an IBAT, we shoudn't set the pages executable, 
at least X bit should be cleared. But the way it is done, if we call 
mark_initmem_nx() then mark_initmem_nx() would call set_memory_attr() to 
clear X bit from the pages. At the end it's not a problem because the 
kernel segments are marked NX, but it's not clean.

So at the end it seems to be a mess around DEBUGPAGE_ALLOC and 
STRICT_KERNEL_RWX. All this being amplified by those 'nobats' and 
'noltlbs' options that are pointless from a functionnal point of view.

I need to think a bit more about it to find the cleanest solution that 
works for all platforms.

Christophe

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Maxime Bizon]

On Fri, 2021-12-03 at 12:49 +0000, Christophe Leroy wrote:

Hello,

> I need to think a bit more about it to find the cleanest solution
> that works for all platforms.

Maybe related, when enabling KASAN on that same platform, it oopses early.

I have picked the patch "powerpc/32s: Fix shift-out-of-bounds in KASAN
init", and that does not fix it


For some mem= values like 769M, all BATs are used for kernel linear
mapping, and there are none left to map the KASAN shadow area in
kasan/book3s_32.c => no oops

If I don't compile kasan/book3s_32.c and use weak implementation => no
oops


But for mem=768M, it oopses

I added some debugs in kasan init and dumped BATs content (BAT 7 is my
debug BAT for uart)

[    0.000000] kasan_init_region: start=0xc0000000 size:0x30000000
[    0.000000] kasan_init_region: k_start:0xf8000000 k_end:0xfe000000 k_size:0x6000000 k_size_base=0x2000000
[    0.000000] kasan_init_region: IF{} k_size_more:0x4000000
[    0.000000] setbat index=3 virt:0xf8000000 phys:0x2a000000 size:0x2000000
[    0.000000] setbat index=4 virt:0xfa000000 phys:0x2c000000 size:0x4000000
[    0.000000] kasan_init_region: final k_cur=0xfe000000
[    0.000000] 
[    0.000000] ---[ Data Block Address Translation ]---
[    0.000000] 0: 0xc0000000-0xcfffffff 0x00000000       256M Kernel rw      m  
[    0.000000] 1: 0xd0000000-0xdfffffff 0x10000000       256M Kernel rw      m  
[    0.000000] 2: 0xe0000000-0xefffffff 0x20000000       256M Kernel rw      m  
[    0.000000] 3: 0xf8000000-0xf9ffffff 0x2a000000        32M Kernel rw      m  
[    0.000000] 4: 0xfa000000-0xfdffffff 0x2c000000        64M Kernel rw      m  
[    0.000000] 5:         -
[    0.000000] 6:         -
[    0.000000] 7: 0xb0000000-0xb00fffff 0xe0000000         1M Kernel rw    i   g
[    0.000000] BUG: Unable to handle kernel data access on read at 0xfd3fce00
[    0.000000] Faulting instruction address: 0xc013ed84
[    0.000000] Oops: Kernel access of bad area, sig: 11 [#1]
[    0.000000] BE PAGE_SIZE=4K 
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.15.0+ #379
[    0.000000] NIP:  c013ed84 LR: c0140264 CTR: 00000020
[    0.000000] REGS: c0b07dd0 TRAP: 0300   Not tainted  (5.15.0+)
[    0.000000] MSR:  00001032 <ME,IR,DR,RI>  CR: 28222448  XER: 00000000
[    0.000000] DAR: fd3fce00 DSISR: 20000000 
[    0.000000] GPR00: fd3fd000 c0b07e80 c09c8a20 0000003f 00001000 00000001 c08c67a8 e9fe7fff 
[    0.000000] GPR08: e9fe7000 fd3fce00 00000020 fd3fcfff 00000000 00000000 3ff9c5f0 3fffd79c 
[    0.000000] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 
[    0.000000] GPR24: 00000000 feffffff 00000591 c0b22000 ffffffff 00000000 e9fe7000 00001000 
[    0.000000] NIP [c013ed84] kasan_check_range+0x98/0x2c0
[    0.000000] LR [c0140264] memset+0x34/0x80
[    0.000000] Call Trace:
[    0.000000] [c0b07e80] [c08c630c] memblock_alloc_internal+0x9c/0x108 (unreliable)
[    0.000000] [c0b07e90] [feffffff] 0xfeffffff
[    0.000000] [c0b07eb0] [c08c67a8] memblock_alloc_try_nid+0xf4/0x128
[    0.000000] [c0b07f30] [c08bb7ac] kasan_init_shadow_page_tables+0x84/0x1cc
[    0.000000] [c0b07f60] [c08bba40] kasan_init+0xdc/0x184
[    0.000000] [c0b07f90] [c08b8108] setup_arch+0x18/0x1c4
[    0.000000] [c0b07fc0] [c08b3fd4] start_kernel+0x5c/0x2d4
[    0.000000] [c0b07ff0] [000033c0] 0x33c0
[    0.000000] Instruction dump:
[    0.000000] 93e1000c 83c90000 83e90004 7fdffb79 83c10008 83e1000c 408201cc 2c030000 
[    0.000000] 39290008 41820034 554af87e 7d4903a6 <80690000> 81490004 7c6a5379 408201a8 


It makes no sense to me that we get that fault with a valid BAT
covering that area, BAT are not supposed to be checked first ?

-- 
Maxime

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Christophe Leroy]

Le 03/12/2021 à 19:43, Maxime Bizon a écrit :
> 
> On Fri, 2021-12-03 at 12:49 +0000, Christophe Leroy wrote:
> 
> Hello,
> 
>> I need to think a bit more about it to find the cleanest solution
>> that works for all platforms.
> 
> Maybe related, when enabling KASAN on that same platform, it oopses early.

Unrelated I think, but thanks for the report.

> 
> I have picked the patch "powerpc/32s: Fix shift-out-of-bounds in KASAN
> init", and that does not fix it
> 
> 
> For some mem= values like 769M, all BATs are used for kernel linear
> mapping, and there are none left to map the KASAN shadow area in
> kasan/book3s_32.c => no oops
> 
> If I don't compile kasan/book3s_32.c and use weak implementation => no
> oops
> 
> 
> But for mem=768M, it oopses
> 
> I added some debugs in kasan init and dumped BATs content (BAT 7 is my
> debug BAT for uart)
> 
> [    0.000000] kasan_init_region: start=0xc0000000 size:0x30000000
> [    0.000000] kasan_init_region: k_start:0xf8000000 k_end:0xfe000000 k_size:0x6000000 k_size_base=0x2000000
> [    0.000000] kasan_init_region: IF{} k_size_more:0x4000000
> [    0.000000] setbat index=3 virt:0xf8000000 phys:0x2a000000 size:0x2000000
> [    0.000000] setbat index=4 virt:0xfa000000 phys:0x2c000000 size:0x4000000
> [    0.000000] kasan_init_region: final k_cur=0xfe000000
> [    0.000000]
> [    0.000000] ---[ Data Block Address Translation ]---
> [    0.000000] 0: 0xc0000000-0xcfffffff 0x00000000       256M Kernel rw      m
> [    0.000000] 1: 0xd0000000-0xdfffffff 0x10000000       256M Kernel rw      m
> [    0.000000] 2: 0xe0000000-0xefffffff 0x20000000       256M Kernel rw      m
> [    0.000000] 3: 0xf8000000-0xf9ffffff 0x2a000000        32M Kernel rw      m
> [    0.000000] 4: 0xfa000000-0xfdffffff 0x2c000000        64M Kernel rw      m
> [    0.000000] 5:         -
> [    0.000000] 6:         -
> [    0.000000] 7: 0xb0000000-0xb00fffff 0xe0000000         1M Kernel rw    i   g
> [    0.000000] BUG: Unable to handle kernel data access on read at 0xfd3fce00
> [    0.000000] Faulting instruction address: 0xc013ed84
> [    0.000000] Oops: Kernel access of bad area, sig: 11 [#1]
> [    0.000000] BE PAGE_SIZE=4K
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.15.0+ #379
> [    0.000000] NIP:  c013ed84 LR: c0140264 CTR: 00000020
> [    0.000000] REGS: c0b07dd0 TRAP: 0300   Not tainted  (5.15.0+)
> [    0.000000] MSR:  00001032 <ME,IR,DR,RI>  CR: 28222448  XER: 00000000
> [    0.000000] DAR: fd3fce00 DSISR: 20000000
> [    0.000000] GPR00: fd3fd000 c0b07e80 c09c8a20 0000003f 00001000 00000001 c08c67a8 e9fe7fff
> [    0.000000] GPR08: e9fe7000 fd3fce00 00000020 fd3fcfff 00000000 00000000 3ff9c5f0 3fffd79c
> [    0.000000] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    0.000000] GPR24: 00000000 feffffff 00000591 c0b22000 ffffffff 00000000 e9fe7000 00001000
> [    0.000000] NIP [c013ed84] kasan_check_range+0x98/0x2c0
> [    0.000000] LR [c0140264] memset+0x34/0x80
> [    0.000000] Call Trace:
> [    0.000000] [c0b07e80] [c08c630c] memblock_alloc_internal+0x9c/0x108 (unreliable)
> [    0.000000] [c0b07e90] [feffffff] 0xfeffffff
> [    0.000000] [c0b07eb0] [c08c67a8] memblock_alloc_try_nid+0xf4/0x128
> [    0.000000] [c0b07f30] [c08bb7ac] kasan_init_shadow_page_tables+0x84/0x1cc
> [    0.000000] [c0b07f60] [c08bba40] kasan_init+0xdc/0x184
> [    0.000000] [c0b07f90] [c08b8108] setup_arch+0x18/0x1c4
> [    0.000000] [c0b07fc0] [c08b3fd4] start_kernel+0x5c/0x2d4
> [    0.000000] [c0b07ff0] [000033c0] 0x33c0
> [    0.000000] Instruction dump:
> [    0.000000] 93e1000c 83c90000 83e90004 7fdffb79 83c10008 83e1000c 408201cc 2c030000
> [    0.000000] 39290008 41820034 554af87e 7d4903a6 <80690000> 81490004 7c6a5379 408201a8
> 
> 
> It makes no sense to me that we get that fault with a valid BAT
> covering that area, BAT are not supposed to be checked first ?
> 


In fact BAT4 is wrong. Both virtual and physical address of a 64M BAT 
must be 64M aligned. I think the display is wrong as well (You took it 
from ptdump ?), BEPI and BRPN must be anded with complement of BL.

So here your 64M BAT maps 0xf8000000-0xfbffffff, therefore the address 
0xfd3fce00 is not mapped by any BAT hence the OOPS.

Christophe

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Maxime Bizon]

On Saturday 04 Dec 2021 à 10:01:07 (+0000), Christophe Leroy wrote:

> In fact BAT4 is wrong. Both virtual and physical address of a 64M BAT 
> must be 64M aligned. I think the display is wrong as well (You took it

oh so hardware does simple bitmask after all

I got fooled by the lack of guard in the bat setup code, so I assumed
magical hardware

> from ptdump ?), BEPI and BRPN must be anded with complement of BL.

yes that was ptdump code with seq_printf replaced by printk

ptdump code is correct but iif the bat addresses are correctly
aligned, maybe add a safeguard like this ?

index 85062ce2d849..f7c5cf62ef41 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -275,6 +279,10 @@ void __init setbat(int index, unsigned long virt, phys_addr_t phys,
                       (unsigned long long)phys);
                return;
        }
+
+       WARN_ON(!is_power_of_2(size));
+       WARN_ON((phys & (size - 1)));
+       WARN_ON((virt & (size - 1)));
        bat = BATS[index];


> So here your 64M BAT maps 0xf8000000-0xfbffffff, therefore the address 
> 0xfd3fce00 is not mapped by any BAT hence the OOPS.

ok I think I found the issue:

diff --git a/arch/powerpc/mm/kasan/book3s_32.c b/arch/powerpc/mm/kasan/book3s_32.c
index 35b287b0a8da..fcbb9a136c1a 100644
--- a/arch/powerpc/mm/kasan/book3s_32.c
+++ b/arch/powerpc/mm/kasan/book3s_32.c
@@ -12,14 +12,14 @@ int __init kasan_init_region(void *start, size_t size)
        unsigned long k_end = (unsigned long)kasan_mem_to_shadow(start + size);
        unsigned long k_cur = k_start;
        int k_size = k_end - k_start;
-       int k_size_base = 1 << (ffs(k_size) - 1);
+       int k_size_base = 1 << (fls(k_size) - 1);
        int ret;
        void *block;
 
        block = memblock_alloc(k_size, k_size_base);
 
        if (block && k_size_base >= SZ_128K && k_start == ALIGN(k_start, k_size_base)) {
-               int shift = ffs(k_size - k_size_base);
+               int shift = fls(k_size - k_size_base);
                int k_size_more = shift ? 1 << (shift - 1) : 0;
 
                setbat(-1, k_start, __pa(block), k_size_base, PAGE_KERNEL);



-- 
Maxime

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Christophe Leroy]

Le 04/12/2021 à 15:10, Maxime Bizon a écrit :
> 
> On Saturday 04 Dec 2021 à 10:01:07 (+0000), Christophe Leroy wrote:
> 
>> In fact BAT4 is wrong. Both virtual and physical address of a 64M BAT
>> must be 64M aligned. I think the display is wrong as well (You took it
> 
> oh so hardware does simple bitmask after all
> 
> I got fooled by the lack of guard in the bat setup code, so I assumed
> magical hardware

I guess all the guard is in the comment ...

/*
  * Set up one of the I/D BAT (block address translation) register pairs.
  * The parameters are not checked; in particular size must be a power
  * of 2 between 128k and 256M.
  */
void __init setbat(int index, unsigned long virt, phys_addr_t phys,
		   unsigned int size, pgprot_t prot)


> 
>> from ptdump ?), BEPI and BRPN must be anded with complement of BL.
> 
> yes that was ptdump code with seq_printf replaced by printk
> 
> ptdump code is correct but iif the bat addresses are correctly
> aligned, maybe add a safeguard like this ?
> 
> index 85062ce2d849..f7c5cf62ef41 100644
> --- a/arch/powerpc/mm/book3s32/mmu.c
> +++ b/arch/powerpc/mm/book3s32/mmu.c
> @@ -275,6 +279,10 @@ void __init setbat(int index, unsigned long virt, phys_addr_t phys,
>                         (unsigned long long)phys);
>                  return;
>          }
> +
> +       WARN_ON(!is_power_of_2(size));
> +       WARN_ON((phys & (size - 1)));
> +       WARN_ON((virt & (size - 1)));
>          bat = BATS[index];
> 

Yes we could add some check allthough I'd go for a 'pr_err()' like when 
no BAT is available.

> 
>> So here your 64M BAT maps 0xf8000000-0xfbffffff, therefore the address
>> 0xfd3fce00 is not mapped by any BAT hence the OOPS.
> 
> ok I think I found the issue:
> 
> diff --git a/arch/powerpc/mm/kasan/book3s_32.c b/arch/powerpc/mm/kasan/book3s_32.c
> index 35b287b0a8da..fcbb9a136c1a 100644
> --- a/arch/powerpc/mm/kasan/book3s_32.c
> +++ b/arch/powerpc/mm/kasan/book3s_32.c
> @@ -12,14 +12,14 @@ int __init kasan_init_region(void *start, size_t size)
>          unsigned long k_end = (unsigned long)kasan_mem_to_shadow(start + size);
>          unsigned long k_cur = k_start;
>          int k_size = k_end - k_start;
> -       int k_size_base = 1 << (ffs(k_size) - 1);
> +       int k_size_base = 1 << (fls(k_size) - 1);
>          int ret;
>          void *block;
>   
>          block = memblock_alloc(k_size, k_size_base);
>   
>          if (block && k_size_base >= SZ_128K && k_start == ALIGN(k_start, k_size_base)) {
> -               int shift = ffs(k_size - k_size_base);
> +               int shift = fls(k_size - k_size_base);
>                  int k_size_more = shift ? 1 << (shift - 1) : 0;
>   
>                  setbat(-1, k_start, __pa(block), k_size_base, PAGE_KERNEL);
> 
> 
> 

Not sure it is that simple.

I'm cooking a patch reusing the block_size() function in mm/book3s32/mmu.c

Christophe

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Maxime Bizon]

On Saturday 04 Dec 2021 à 17:42:44 (+0000), Christophe Leroy wrote:

> I guess all the guard is in the comment ...
> 
> /*
>   * Set up one of the I/D BAT (block address translation) register pairs.
>   * The parameters are not checked; in particular size must be a power
>   * of 2 between 128k and 256M.
>   */

It's missing the aligment rule you just taught me, but it's arguably
not the right place to teach ppc 101.

> Not sure it is that simple.
> 
> I'm cooking a patch reusing the block_size() function in mm/book3s32/mmu.c

Indeed it will handle cases that need more than 2 BATs.

Also when mem=2G, I have physical memory mapped twice:

CONFIG_PAGE_OFFSET=0x80000000
CONFIG_LOWMEM_SIZE=0x60000000

0: 0x80000000-0x8fffffff 0x00000000       256M Kernel rw      m   
1: 0x90000000-0x9fffffff 0x10000000       256M Kernel rw      m   
2: 0xa0000000-0xafffffff 0x20000000       256M Kernel rw      m   
3: 0xb0000000-0xbfffffff 0x30000000       256M Kernel rw      m   
4: 0xc0000000-0xcfffffff 0x40000000       256M Kernel rw      m   
5: 0xd0000000-0xdfffffff 0x50000000       256M Kernel rw      m   
6: 0xf0000000-0xf7ffffff 0x50000000       128M Kernel rw      m   

BAT5 comes from __mmu_mapin_ram(), BAT6 from kasan init

Is BAT5 needed here ?


Last one, with KASAN and the following layout, I have an non working
kernel with VMALLOC_START > VMALLOC_END:

mem=2G
CONFIG_PAGE_OFFSET=0x80000000
CONFIG_LOWMEM_SIZE=0x70000000

[    0.000000]   * 0xf0000000..0xfe000000  : kasan shadow mem
[    0.000000]   * 0xef7ff000..0xeffff000  : fixmap
[    0.000000]   * 0xf1000000..0xef7ff000  : vmalloc & ioremap


IIUC the safeguard is here:

arch/powerpc/mm/init_32.c:
/* The amount of lowmem must be within 0xF0000000 - KERNELBASE. */
#if (CONFIG_LOWMEM_SIZE > (0xF0000000 - PAGE_OFFSET))
#error "You must adjust CONFIG_LOWMEM_SIZE or CONFIG_KERNEL_START"


but the definition needs to be adapted for KASAN=y and require 256
more MB.


-- 
Maxime

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Christophe Leroy]

Le 05/12/2021 à 17:42, Maxime Bizon a écrit :
> 
> 
> On Saturday 04 Dec 2021 à 17:42:44 (+0000), Christophe Leroy wrote:
> 
>> I guess all the guard is in the comment ...
>>
>> /*
>>    * Set up one of the I/D BAT (block address translation) register pairs.
>>    * The parameters are not checked; in particular size must be a power
>>    * of 2 between 128k and 256M.
>>    */
> 
> It's missing the aligment rule you just taught me, but it's arguably
> not the right place to teach ppc 101.

Indeed

> 
>> Not sure it is that simple.
>>
>> I'm cooking a patch reusing the block_size() function in mm/book3s32/mmu.c
> 
> Indeed it will handle cases that need more than 2 BATs.

Yes, that's the idea.

> 
> Also when mem=2G, I have physical memory mapped twice:
> 
> CONFIG_PAGE_OFFSET=0x80000000
> CONFIG_LOWMEM_SIZE=0x60000000
> 
> 0: 0x80000000-0x8fffffff 0x00000000       256M Kernel rw      m
> 1: 0x90000000-0x9fffffff 0x10000000       256M Kernel rw      m
> 2: 0xa0000000-0xafffffff 0x20000000       256M Kernel rw      m
> 3: 0xb0000000-0xbfffffff 0x30000000       256M Kernel rw      m
> 4: 0xc0000000-0xcfffffff 0x40000000       256M Kernel rw      m
> 5: 0xd0000000-0xdfffffff 0x50000000       256M Kernel rw      m
> 6: 0xf0000000-0xf7ffffff 0x50000000       128M Kernel rw      m
> 
> BAT5 comes from __mmu_mapin_ram(), BAT6 from kasan init
> 
> Is BAT5 needed here ?

Sure it is, because that's were kernel expects lowmem to be mapped. 
Allthough the kernel will unlikely access the 128M reserved for KASAN 
directly, the other 128M are still needed.

> 
> 
> Last one, with KASAN and the following layout, I have an non working
> kernel with VMALLOC_START > VMALLOC_END:
> 
> mem=2G
> CONFIG_PAGE_OFFSET=0x80000000
> CONFIG_LOWMEM_SIZE=0x70000000
> 
> [    0.000000]   * 0xf0000000..0xfe000000  : kasan shadow mem
> [    0.000000]   * 0xef7ff000..0xeffff000  : fixmap
> [    0.000000]   * 0xf1000000..0xef7ff000  : vmalloc & ioremap
> 
> 
> IIUC the safeguard is here:
> 
> arch/powerpc/mm/init_32.c:
> /* The amount of lowmem must be within 0xF0000000 - KERNELBASE. */
> #if (CONFIG_LOWMEM_SIZE > (0xF0000000 - PAGE_OFFSET))
> #error "You must adjust CONFIG_LOWMEM_SIZE or CONFIG_KERNEL_START"
> 
> 
> but the definition needs to be adapted for KASAN=y and require 256
> more MB.
> 
> 

Hum.. Good point.

Christophe

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Maxime Bizon]

On Sunday 05 Dec 2021 à 18:11:59 (+0000), Christophe Leroy wrote:

> > Is BAT5 needed here ?
> 
> Sure it is, because that's were kernel expects lowmem to be mapped. 
> Allthough the kernel will unlikely access the 128M reserved for KASAN 
> directly, the other 128M are still needed.
> 

Yes that was my point

I'm wondering if for specific PAGE_OFFSET values, __mmu_mapin_ram()
ends using a BAT to map exactly the KASAN area, thus wasting it
because the kernel would never/rarely access it.

Or worse, it could consume the latest BAT available, and there would
be none left for the actual KASAN vm area

Maybe mmu_mapin_ram() could clamp top to KASAN phys start.

-- 
Maxime

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Christophe Leroy]

Le 05/12/2021 à 22:44, Maxime Bizon a écrit :
> 
> 
> On Sunday 05 Dec 2021 à 18:11:59 (+0000), Christophe Leroy wrote:
> 
>>> Is BAT5 needed here ?
>>
>> Sure it is, because that's were kernel expects lowmem to be mapped.
>> Allthough the kernel will unlikely access the 128M reserved for KASAN
>> directly, the other 128M are still needed.
>>
> 
> Yes that was my point
> 
> I'm wondering if for specific PAGE_OFFSET values, __mmu_mapin_ram()
> ends using a BAT to map exactly the KASAN area, thus wasting it
> because the kernel would never/rarely access it.

Is it worth it ? I have the feeling that's more a corner case.


> 
> Or worse, it could consume the latest BAT available, and there would
> be none left for the actual KASAN vm area

Even more.

> 
> Maybe mmu_mapin_ram() could clamp top to KASAN phys start.
> 


An alternative could be to try and use for KASAN some memory which is 
not available in the linear mapping.
For instance, in your case, you have move than 2G bytes mem I guess and 
you are mapping 0x00000000 to 0x5fffffff. Maybe could use 0x6000000 up 
for KASAN.

But I don't know how to proceed to do it that way. Today we use 
memblock_alloc() which is the only memory allocator available at the 
time we allocate KASAN shadow area. But memblock_alloc() allocates 
memory from the directly accessible area.

Christophe

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Maxime Bizon]

On Mon, 2021-12-06 at 07:03 +0000, Christophe Leroy wrote:

> Is it worth it ? I have the feeling that's more a corner case.

probably not since it's not an easy fix

I'm running on the edge wrt BAT usage on my 2GB board, it's not that
common I guess.

-- 
Maxime

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Christophe Leroy]

Le 06/12/2021 à 09:47, Maxime Bizon a écrit :
> 
> On Mon, 2021-12-06 at 07:03 +0000, Christophe Leroy wrote:
> 
>> Is it worth it ? I have the feeling that's more a corner case.
> 
> probably not since it's not an easy fix
> 
> I'm running on the edge wrt BAT usage on my 2GB board, it's not that
> common I guess.
> 

Looks like you can win something if you take the patch I just sent and 
replace the memblock_phys_alloc(k_size, k_size) by 
memblock_phys_alloc_range(k_size, k_size, 0, MEMBLOCK_ALLOC_ANYWHERE)

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Maxime Bizon]

On Mon, 2021-12-06 at 09:07 +0000, Christophe Leroy wrote:

Hello,

> Looks like you can win something if you take the patch I just sent
> and replace the memblock_phys_alloc(k_size, k_size) by 
> memblock_phys_alloc_range(k_size, k_size, 0, MEMBLOCK_ALLOC_ANYWHERE)

I tried your patch without your proposed modification and got something new:

[    0.000000] ==================================================================
[    0.000000] BUG: KASAN: unknown-crash in vprintk+0x30/0xe8
[    0.000000] Read of size 4 at addr 80ad0740 by task swapper/0
[    0.000000] 
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.15.0+ #415
[    0.000000] Call Trace:
[    0.000000] [80cdfe50] [8017711c] print_address_description.constprop.0+0x78/0x338 (unreliable)
[    0.000000] [80cdfe80] [80176e48] kasan_report+0x1c0/0x1d4
[    0.000000] [80cdfec0] [80099a34] vprintk+0x30/0xe8
[    0.000000] [80cdfee0] [80099600] _printk+0x9c/0xbc
[    0.000000] [80cdff60] [80999628] kasan_init+0x14c/0x164
[    0.000000] [80cdff90] [80995440] setup_arch+0x18/0x1c4
[    0.000000] [80cdffc0] [809910c8] start_kernel+0x60/0x2fc
[    0.000000] [80cdfff0] [000033c0] 0x33c0
[    0.000000] 
[    0.000000] The buggy address belongs to the variable:
[    0.000000]  init_task+0x0/0xce0
[    0.000000] 
[    0.000000] Memory state around the buggy address:
[    0.000000]  80ad0600: de ad be ef de ad be ef de ad be ef de ad be ef
[    0.000000]  80ad0680: de ad be ef de ad be ef de ad be ef de ad be ef
[    0.000000] >80ad0700: de ad be ef de ad be ef de ad be ef de ad be ef
[    0.000000]                                    ^
[    0.000000]  80ad0780: de ad be ef de ad be ef de ad be ef de ad be ef
[    0.000000]  80ad0800: de ad be ef de ad be ef de ad be ef de ad be ef
[    0.000000] ==================================================================
[    0.000000] Disabling lock debugging due to kernel taint
[    0.000000] KASAN init done


Looking at the archive when you introduced KASAN, you had this kind of
bug, and the conclusion of the thread was:

> Indeed the problem is in kasan_init() : memblock_phys_alloc()
> doesn't zeroize the allocated memory. I changed it to
> memblock_alloc() and now it works.	

since your patch uses memblock_phys_alloc() again, maybe that's the
same issue

-- 
Maxime

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Christophe Leroy]

Le 06/12/2021 à 11:32, Maxime Bizon a écrit :
> 
> 
> On Mon, 2021-12-06 at 09:07 +0000, Christophe Leroy wrote:
> 
> Hello,
> 
>> Looks like you can win something if you take the patch I just sent
>> and replace the memblock_phys_alloc(k_size, k_size) by
>> memblock_phys_alloc_range(k_size, k_size, 0, MEMBLOCK_ALLOC_ANYWHERE)
> 
> I tried your patch without your proposed modification and got something new:
> 
> [    0.000000] ==================================================================
> [    0.000000] BUG: KASAN: unknown-crash in vprintk+0x30/0xe8
> [    0.000000] Read of size 4 at addr 80ad0740 by task swapper/0
> [    0.000000]
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.15.0+ #415
> [    0.000000] Call Trace:
> [    0.000000] [80cdfe50] [8017711c] print_address_description.constprop.0+0x78/0x338 (unreliable)
> [    0.000000] [80cdfe80] [80176e48] kasan_report+0x1c0/0x1d4
> [    0.000000] [80cdfec0] [80099a34] vprintk+0x30/0xe8
> [    0.000000] [80cdfee0] [80099600] _printk+0x9c/0xbc
> [    0.000000] [80cdff60] [80999628] kasan_init+0x14c/0x164
> [    0.000000] [80cdff90] [80995440] setup_arch+0x18/0x1c4
> [    0.000000] [80cdffc0] [809910c8] start_kernel+0x60/0x2fc
> [    0.000000] [80cdfff0] [000033c0] 0x33c0
> [    0.000000]
> [    0.000000] The buggy address belongs to the variable:
> [    0.000000]  init_task+0x0/0xce0
> [    0.000000]
> [    0.000000] Memory state around the buggy address:
> [    0.000000]  80ad0600: de ad be ef de ad be ef de ad be ef de ad be ef
> [    0.000000]  80ad0680: de ad be ef de ad be ef de ad be ef de ad be ef
> [    0.000000] >80ad0700: de ad be ef de ad be ef de ad be ef de ad be ef
> [    0.000000]                                    ^
> [    0.000000]  80ad0780: de ad be ef de ad be ef de ad be ef de ad be ef
> [    0.000000]  80ad0800: de ad be ef de ad be ef de ad be ef de ad be ef
> [    0.000000] ==================================================================
> [    0.000000] Disabling lock debugging due to kernel taint
> [    0.000000] KASAN init done
> 
> 
> Looking at the archive when you introduced KASAN, you had this kind of
> bug, and the conclusion of the thread was:
> 
>> Indeed the problem is in kasan_init() : memblock_phys_alloc()
>> doesn't zeroize the allocated memory. I changed it to
>> memblock_alloc() and now it works.	
> 
> since your patch uses memblock_phys_alloc() again, maybe that's the
> same issue
> 

There was two issues:
- The one you describe
- The fallback part was allocating only the remaining memory required 
but was still using the total size for offset calculation so we were 
mapping memory outside of the allocated block.

Fixed both in v2.

Christophe

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Maxime Bizon]

On Mon, 2021-12-06 at 14:22 +0000, Christophe Leroy wrote:

> Fixed both in v2.

Works fine, many thanks 

-- 
Maxime

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Christophe Leroy]

Le 06/12/2021 à 16:48, Maxime Bizon a écrit :
> 
> On Mon, 2021-12-06 at 14:22 +0000, Christophe Leroy wrote:
> 
>> Fixed both in v2.
> 
> Works fine, many thanks
> 

Great.

Could you then reply to the patch with the following line ?

Reported-by: Maxime Bizon <mbizon@freebox.fr>

That way it will be taken into account in patchwork at 
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?submitter=79086

Thanks
Christophe

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Christophe Leroy]

Le 07/12/2021 à 06:50, Christophe Leroy a écrit :
> 
> 
> Le 06/12/2021 à 16:48, Maxime Bizon a écrit :
>>
>> On Mon, 2021-12-06 at 14:22 +0000, Christophe Leroy wrote:
>>
>>> Fixed both in v2.
>>
>> Works fine, many thanks
>>
> 
> Great.
> 
> Could you then reply to the patch with the following line ?
> 
> Reported-by: Maxime Bizon <mbizon@freebox.fr>

I meant

Tested-by: Maxime Bizon <mbizon@freebox.fr>


The Reported-by: is already in the patch.

> 
> That way it will be taken into account in patchwork at
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?submitter=79086
> 
> Thanks
> Christophe
> 

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Maxime Bizon]

On Tuesday 07 Dec 2021 à 07:11:40 (+0100), Christophe Leroy wrote:

Hello Christophe,

With all your recent patches, I was able to boot a kernel with every
CONFIG_DEBUG enabled.

After modprobing an empty module (probe just return 0), I get this new
one:

[   15.351649] BUG: spinlock recursion on CPU#0, kworker/0:2/217
[   15.357540]  lock: init_mm+0x3c/0x420, .magic: dead4ead, .owner: kworker/0:2/217, .owner_cpu: 0
[   15.366563] CPU: 0 PID: 217 Comm: kworker/0:2 Not tainted 5.15.0+ #523
[   15.373350] Workqueue: events do_free_init
[   15.377615] Call Trace:
[   15.380232] [e4105ac0] [800946a4] do_raw_spin_lock+0xf8/0x120 (unreliable)
[   15.387340] [e4105ae0] [8001f4ec] change_page_attr+0x40/0x1d4
[   15.393413] [e4105b10] [801424e0] __apply_to_page_range+0x164/0x310
[   15.400009] [e4105b60] [80169620] free_pcp_prepare+0x1e4/0x4a0
[   15.406045] [e4105ba0] [8016c5a0] free_unref_page+0x40/0x2b8
[   15.411979] [e4105be0] [8018724c] kasan_depopulate_vmalloc_pte+0x6c/0x94
[   15.418989] [e4105c00] [801424e0] __apply_to_page_range+0x164/0x310
[   15.425451] [e4105c50] [80187834] kasan_release_vmalloc+0xbc/0x134
[   15.431898] [e4105c70] [8015f7a8] __purge_vmap_area_lazy+0x4e4/0xdd8
[   15.438560] [e4105d30] [80160d10] _vm_unmap_aliases.part.0+0x17c/0x24c
[   15.445283] [e4105d60] [801642d0] __vunmap+0x2f0/0x5c8
[   15.450684] [e4105db0] [800e32d0] do_free_init+0x68/0x94
[   15.456181] [e4105dd0] [8005d094] process_one_work+0x4bc/0x7b8
[   15.462283] [e4105e90] [8005d614] worker_thread+0x284/0x6e8
[   15.468227] [e4105f00] [8006aaec] kthread+0x1f0/0x210
[   15.473489] [e4105f40] [80017148] ret_from_kernel_thread+0x14/0x1c


-- 
Maxime

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Christophe Leroy]

Le 05/12/2021 à 17:42, Maxime Bizon a écrit :
> 
> 
> On Saturday 04 Dec 2021 à 17:42:44 (+0000), Christophe Leroy wrote:
> 
> Also when mem=2G, I have physical memory mapped twice:
> 
> CONFIG_PAGE_OFFSET=0x80000000
> CONFIG_LOWMEM_SIZE=0x60000000
> 
> 0: 0x80000000-0x8fffffff 0x00000000       256M Kernel rw      m
> 1: 0x90000000-0x9fffffff 0x10000000       256M Kernel rw      m
> 2: 0xa0000000-0xafffffff 0x20000000       256M Kernel rw      m
> 3: 0xb0000000-0xbfffffff 0x30000000       256M Kernel rw      m
> 4: 0xc0000000-0xcfffffff 0x40000000       256M Kernel rw      m
> 5: 0xd0000000-0xdfffffff 0x50000000       256M Kernel rw      m
> 6: 0xf0000000-0xf7ffffff 0x50000000       128M Kernel rw      m
> 
> BAT5 comes from __mmu_mapin_ram(), BAT6 from kasan init
> 
> Is BAT5 needed here ?

Did you check with my latest patch (v2) ?

It should now allocate a block outside lowmem, most likely 0x78000000.

Christophe

Re: Fail to boot 5.15 on mpc8347 with either debug_pagealloc or nobats

[Maxime Bizon]

On Tue, 2021-12-07 at 05:54 +0000, Christophe Leroy wrote:

> 
> Did you check with my latest patch (v2) ?
> 

yes I did, works perfectly, I just sent the Tested-By

-- 
Maxime