[v2,00/15] powerpc/32s: Use BATs/LTLBs for STRICT_KERNEL_RWX
mbox series

Message ID cover.1547132681.git.christophe.leroy@c-s.fr
Headers show
Series
  • powerpc/32s: Use BATs/LTLBs for STRICT_KERNEL_RWX
Related show

Message

Christophe Leroy Jan. 10, 2019, 3:11 p.m. UTC
The purpose of this serie is to:
- use BATs with STRICT_KERNEL_RWX on book3s (See patch 12 for details.)
- use LTLBs with STRICT_KERNEL_RWX on 8xx (See patch 14 for a few details.)

v2:
- Fix patch 2 (was patch 3 in v1) based on feedback from Jonathan.
- Added support for 8xx with LTLBs.
- Added systematic population of pagetables for Abatron BDI.

Christophe Leroy (15):
  powerpc/mm/32: add base address to mmu_mapin_ram()
  powerpc/mm/32s: rework mmu_mapin_ram()
  powerpc/mm/32s: use generic mmu_mapin_ram() for all blocks.
  powerpc/32: always populate page tables for Abatron BDI.
  powerpc/wii: remove wii_mmu_mapin_mem2()
  powerpc/mm/32s: use _PAGE_EXEC in setbat()
  powerpc/mm/32s: add setibat() clearibat() and update_bats()
  powerpc/32: add helper to write into segment registers
  powerpc/mmu: add is_strict_kernel_rwx() helper
  powerpc/kconfig: define PAGE_SHIFT inside Kconfig
  powerpc/kconfig: define CONFIG_DATA_SHIFT and CONFIG_ETEXT_SHIFT
  powerpc/mm/32s: Use BATs for STRICT_KERNEL_RWX
  powerpc/kconfig: make _etext and data areas alignment configurable on
    Book3s 32
  powerpc/8xx: don't disable large TLBs with CONFIG_STRICT_KERNEL_RWX
  powerpc/kconfig: make _etext and data areas alignment configurable on
    8xx

 arch/powerpc/Kconfig                          |  60 +++++++++
 arch/powerpc/include/asm/book3s/32/mmu-hash.h |   2 +
 arch/powerpc/include/asm/book3s/32/pgtable.h  |  11 ++
 arch/powerpc/include/asm/mmu.h                |  11 ++
 arch/powerpc/include/asm/nohash/32/mmu-8xx.h  |   3 +-
 arch/powerpc/include/asm/page.h               |  13 +-
 arch/powerpc/include/asm/reg.h                |   5 +
 arch/powerpc/kernel/head_32.S                 |  35 +++++
 arch/powerpc/kernel/head_8xx.S                |  33 ++++-
 arch/powerpc/kernel/vmlinux.lds.S             |   9 +-
 arch/powerpc/mm/40x_mmu.c                     |   2 +-
 arch/powerpc/mm/44x_mmu.c                     |   2 +-
 arch/powerpc/mm/8xx_mmu.c                     |  33 ++++-
 arch/powerpc/mm/fsl_booke_mmu.c               |   2 +-
 arch/powerpc/mm/init_32.c                     |   6 +-
 arch/powerpc/mm/mmu_decl.h                    |  10 +-
 arch/powerpc/mm/pgtable_32.c                  |  38 +++---
 arch/powerpc/mm/ppc_mmu_32.c                  | 178 ++++++++++++++++++++++----
 arch/powerpc/platforms/embedded6xx/wii.c      |  24 ----
 19 files changed, 378 insertions(+), 99 deletions(-)

Comments

Jonathan Neuschäfer Jan. 13, 2019, 6:16 p.m. UTC | #1
On Thu, Jan 10, 2019 at 03:11:38PM +0000, Christophe Leroy wrote:
> The purpose of this serie is to:
> - use BATs with STRICT_KERNEL_RWX on book3s (See patch 12 for details.)
> - use LTLBs with STRICT_KERNEL_RWX on 8xx (See patch 14 for a few details.)

Hi,

I just tested the whole series on my Wii (I didn't test any intermediate
steps). Without CONFIG_STRICT_KERNEL_RWX, it seems to work fine, but
with it, I get the following error while booting:

> top of MEM2 @ 13F00000
> 
> zImage starting: loaded at 0x01000000 (sp: 0x0178afa0)
> Allocating 0x166b2c8 bytes for kernel...
> Decompressing (0x00000000 <- 0x01011000:0x01788398)...
> Done! Decompressed 0xf421f4 bytes
> 
> Linux/PowerPC load: root=/dev/mmcblk0p2 rootwait console=usbgecko1
> Finalizing device tree... flat tree at 0x178b7a0
> [    0.000000] printk: bootconsole [udbg0] enabled
> [    0.000000] Kernel panic - not syncing: ERROR: Failed to allocate 0x00100000 bytes below 0x00000000.
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc1-wii-00038-g335347fa417f #1337
> [    0.000000] Call Trace:
> [    0.000000] [c0f1ff30] [c00280f0] panic+0x144/0x324 (unreliable)
> [    0.000000] [c0f1ff90] [c0c18a34] memblock_alloc_base+0x34/0x44
> [    0.000000] [c0f1ffa0] [c0c071e0] MMU_init_hw+0xcc/0x300
> [    0.000000] [c0f1ffd0] [c0c06554] MMU_init+0x12c/0x198
> [    0.000000] [c0f1fff0] [c0003418] start_here+0x40/0x78
> [    0.000000] Rebooting in 180 seconds..


Jonathan
Christophe Leroy Jan. 13, 2019, 7:43 p.m. UTC | #2
Le 13/01/2019 à 19:16, Jonathan Neuschäfer a écrit :
> On Thu, Jan 10, 2019 at 03:11:38PM +0000, Christophe Leroy wrote:
>> The purpose of this serie is to:
>> - use BATs with STRICT_KERNEL_RWX on book3s (See patch 12 for details.)
>> - use LTLBs with STRICT_KERNEL_RWX on 8xx (See patch 14 for a few details.)
> 
> Hi,
> 
> I just tested the whole series on my Wii (I didn't test any intermediate
> steps). Without CONFIG_STRICT_KERNEL_RWX, it seems to work fine, but
> with it, I get the following error while booting:

Argh !

Can you please send the System.map file generated at compile time ?

Thanks
Christophe

> 
>> top of MEM2 @ 13F00000
>>
>> zImage starting: loaded at 0x01000000 (sp: 0x0178afa0)
>> Allocating 0x166b2c8 bytes for kernel...
>> Decompressing (0x00000000 <- 0x01011000:0x01788398)...
>> Done! Decompressed 0xf421f4 bytes
>>
>> Linux/PowerPC load: root=/dev/mmcblk0p2 rootwait console=usbgecko1
>> Finalizing device tree... flat tree at 0x178b7a0
>> [    0.000000] printk: bootconsole [udbg0] enabled
>> [    0.000000] Kernel panic - not syncing: ERROR: Failed to allocate 0x00100000 bytes below 0x00000000.
>> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc1-wii-00038-g335347fa417f #1337
>> [    0.000000] Call Trace:
>> [    0.000000] [c0f1ff30] [c00280f0] panic+0x144/0x324 (unreliable)
>> [    0.000000] [c0f1ff90] [c0c18a34] memblock_alloc_base+0x34/0x44
>> [    0.000000] [c0f1ffa0] [c0c071e0] MMU_init_hw+0xcc/0x300
>> [    0.000000] [c0f1ffd0] [c0c06554] MMU_init+0x12c/0x198
>> [    0.000000] [c0f1fff0] [c0003418] start_here+0x40/0x78
>> [    0.000000] Rebooting in 180 seconds..
> 
> 
> Jonathan
>
Jonathan Neuschäfer Jan. 13, 2019, 9:02 p.m. UTC | #3
On Sun, Jan 13, 2019 at 08:43:07PM +0100, Christophe Leroy wrote:
> Le 13/01/2019 à 19:16, Jonathan Neuschäfer a écrit :
> > I just tested the whole series on my Wii (I didn't test any intermediate
> > steps). Without CONFIG_STRICT_KERNEL_RWX, it seems to work fine, but
> > with it, I get the following error while booting:
> 
> Argh !
> 
> Can you please send the System.map file generated at compile time ?

Here:
  https://gist.githubusercontent.com/neuschaefer/12ccc87ff8aeff543fad558e8742cd2b/raw/d49d321709cac364779e6893bbd91ff5a80bcb03/System.map

And here's the config, reduced with 'make savedefconfig':
  https://gist.githubusercontent.com/neuschaefer/12ccc87ff8aeff543fad558e8742cd2b/raw/d49d321709cac364779e6893bbd91ff5a80bcb03/config

The exact code I used is here:
  https://github.com/neuschaefer/linux/tree/ppcbat-test


Thanks,
Jonathan
Christophe Leroy Jan. 14, 2019, 6:23 p.m. UTC | #4
Le 13/01/2019 à 22:02, Jonathan Neuschäfer a écrit :
> On Sun, Jan 13, 2019 at 08:43:07PM +0100, Christophe Leroy wrote:
>> Le 13/01/2019 à 19:16, Jonathan Neuschäfer a écrit :
>>> I just tested the whole series on my Wii (I didn't test any intermediate
>>> steps). Without CONFIG_STRICT_KERNEL_RWX, it seems to work fine, but
>>> with it, I get the following error while booting:
>>
>> Argh !
>>
>> Can you please send the System.map file generated at compile time ?
> 
> Here:
>    https://gist.githubusercontent.com/neuschaefer/12ccc87ff8aeff543fad558e8742cd2b/raw/d49d321709cac364779e6893bbd91ff5a80bcb03/System.map
> 
> And here's the config, reduced with 'make savedefconfig':
>    https://gist.githubusercontent.com/neuschaefer/12ccc87ff8aeff543fad558e8742cd2b/raw/d49d321709cac364779e6893bbd91ff5a80bcb03/config
> 
> The exact code I used is here:
>    https://github.com/neuschaefer/linux/tree/ppcbat-test

Thanks

I can't see anything special in your setup, and this failure looks 
rather unexpected because I can't see anything done that early when 
CONFIG_STRICT_KERNEL_RWX is selected.

Does CONFIG_STRICT_KERNEL_RWX works properly without my serie ?

Christophe

> 
> 
> Thanks,
> Jonathan
>
Jonathan Neuschäfer Jan. 15, 2019, 12:33 a.m. UTC | #5
On Mon, Jan 14, 2019 at 07:23:07PM +0100, Christophe Leroy wrote:
> 
> 
> Le 13/01/2019 à 22:02, Jonathan Neuschäfer a écrit :
> > On Sun, Jan 13, 2019 at 08:43:07PM +0100, Christophe Leroy wrote:
> > > Le 13/01/2019 à 19:16, Jonathan Neuschäfer a écrit :
> > > > I just tested the whole series on my Wii (I didn't test any intermediate
> > > > steps). Without CONFIG_STRICT_KERNEL_RWX, it seems to work fine, but
> > > > with it, I get the following error while booting:
[...]
> I can't see anything special in your setup, and this failure looks rather
> unexpected because I can't see anything done that early when
> CONFIG_STRICT_KERNEL_RWX is selected.
> 
> Does CONFIG_STRICT_KERNEL_RWX works properly without my serie ?

I hadn't tried this before, but yes, without this series (on v5.0-rc2),
a kernel with CONFIG_STRICT_KERNEL_RWX boots.

I've checked it patch-by-patch now (with STRICT_KERNEL_RWX):

- patches 1 and 2 build and boot fine
- patches 3 to 6 build, but fail to boot with this error:

	top of MEM2 @ 13F00000

	zImage starting: loaded at 0x00e00000 (sp: 0x01588fa0)
	Allocating 0x14e92c8 bytes for kernel...
	Decompressing (0x00000000 <- 0x00e11000:0x01586ba7)...
	Done! Decompressed 0xdc01f4 bytes

	Linux/PowerPC load: root=/dev/mmcblk0p2 rootwait console=usbgecko1
	Finalizing device tree... flat tree at 0x15897a0
	[    0.000000] printk: bootconsole [udbg0] enabled
	[    0.000000] Total memory = 319MB; using 1024kB for hash table (at (ptrval))
	[    0.000000] RAM mapped without BATs
	[    0.000000] RAM mapped without BATs
	[    0.000000] ------------[ cut here ]------------
	[    0.000000] kernel BUG at arch/powerpc/mm/pgtable_32.c:223!
	[    0.000000] Oops: Exception in kernel mode, sig: 5 [#1]
	[    0.000000] BE PREEMPT
	[    0.000000] Modules linked in:
	[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc1-wii-00024-g596f9fe23c13 #1337
	[    0.000000] NIP:  c0017c4c LR: c0a836a0 CTR: c001edc4
	[    0.000000] REGS: c0d9deb0 TRAP: 0700   Not tainted  (5.0.0-rc1-wii-00024-g596f9fe23c13)
	[    0.000000] MSR:  00020030 <IR,DR>  CR: 42000888  XER: 20000000
	[    0.000000]
	[    0.000000] GPR00: c0a836a0 c0d9df60 c0d2a4a0 c0d29c00 00000000 c16ff000 c0d9de28 c0dc0000
	[    0.000000] GPR08: c0d9c000 00000001 00000001 00000000 28000824 00000000 00000000 00000000
	[    0.000000] GPR16: 00000000 00000000 00000020 00000000 c0860000 c0da0000 c0000000 c0a7d000
	[    0.000000] GPR24: c0acd55c c0d487c8 13f00000 c0d29000 00000c00 00000311 c0000000 c0d487c8
	[    0.000000] NIP [c0017c4c] map_kernel_page+0x78/0xf0
	[    0.000000] LR [c0a836a0] mapin_ram+0xe0/0x14c
	[    0.000000] Call Trace:
	[    0.000000] [c0d9df60] [c0a83f54] mmu_mapin_ram+0x54/0x1a4 (unreliable)
	[    0.000000] [c0d9df90] [c0a836a0] mapin_ram+0xe0/0x14c
	[    0.000000] [c0d9dfd0] [c0a83578] MMU_init+0x158/0x1a0
	[    0.000000] [c0d9dff0] [c0003418] start_here+0x40/0x78
	[    0.000000] Instruction dump:
	[    0.000000] 55290026 57c5b53a 7ca54a14 3d204000 7f854800 3ca5c000 419e0088 81250000
	[    0.000000] 552afffe 552907fe 7d4a4b79 4082004c <0f0a0000> 54840026 7c84eb78 9081000c
	[    0.000000] random: get_random_bytes called from print_oops_end_marker+0x34/0x6c with crng_init=0
	[    0.000000] ---[ end trace 0000000000000000 ]---
	[    0.000000]
	[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
	[    0.000000] Rebooting in 180 seconds..

- patches 7 to 11 fail to build with this error (really a warning, but
  arch/powerpc doesn't allow warnings by default):

	  CC      arch/powerpc/mm/ppc_mmu_32.o
	../arch/powerpc/mm/ppc_mmu_32.c:133:13: error: ‘clearibat’ defined but not used [-Werror=unused-function]
	 static void clearibat(int index)
		     ^~~~~~~~~
	../arch/powerpc/mm/ppc_mmu_32.c:115:13: error: ‘setibat’ defined but not used [-Werror=unused-function]
	 static void setibat(int index, unsigned long virt, phys_addr_t phys,
		     ^~~~~~~
	cc1: all warnings being treated as errors

- patches 12 to 15 build but fail to boot with this error:

	top of MEM2 @ 13F00000

	zImage starting: loaded at 0x01000000 (sp: 0x0178afa0)
	Allocating 0x166b2c8 bytes for kernel...
	Decompressing (0x00000000 <- 0x01011000:0x017880ce)...
	Done! Decompressed 0xf421f4 bytes

	Linux/PowerPC load: root=/dev/mmcblk0p2 rootwait console=usbgecko1
	Finalizing device tree... flat tree at 0x178b7a0
	[    0.000000] printk: bootconsole [udbg0] enabled
	[    0.000000] Kernel panic - not syncing: ERROR: Failed to allocate 0x00100000 bytes below 0x00000000.
	[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc1-wii-00033-gc263f8162122 #1337
	[    0.000000] Call Trace:
	[    0.000000] [c0f1ff30] [c00280f0] panic+0x144/0x324 (unreliable)
	[    0.000000] [c0f1ff90] [c0c18a34] memblock_alloc_base+0x34/0x44
	[    0.000000] [c0f1ffa0] [c0c071e0] MMU_init_hw+0xcc/0x300
	[    0.000000] [c0f1ffd0] [c0c06554] MMU_init+0x12c/0x198
	[    0.000000] [c0f1fff0] [c0003418] start_here+0x40/0x78
	[    0.000000] Rebooting in 180 seconds..


I'll investigate some more tomorrow.

Jonathan
Christophe Leroy Jan. 15, 2019, 6:51 a.m. UTC | #6
Le 15/01/2019 à 01:33, Jonathan Neuschäfer a écrit :
> On Mon, Jan 14, 2019 at 07:23:07PM +0100, Christophe Leroy wrote:
>>
>>
>> Le 13/01/2019 à 22:02, Jonathan Neuschäfer a écrit :
>>> On Sun, Jan 13, 2019 at 08:43:07PM +0100, Christophe Leroy wrote:
>>>> Le 13/01/2019 à 19:16, Jonathan Neuschäfer a écrit :
>>>>> I just tested the whole series on my Wii (I didn't test any intermediate
>>>>> steps). Without CONFIG_STRICT_KERNEL_RWX, it seems to work fine, but
>>>>> with it, I get the following error while booting:
> [...]
>> I can't see anything special in your setup, and this failure looks rather
>> unexpected because I can't see anything done that early when
>> CONFIG_STRICT_KERNEL_RWX is selected.
>>
>> Does CONFIG_STRICT_KERNEL_RWX works properly without my serie ?
> 
> I hadn't tried this before, but yes, without this series (on v5.0-rc2),
> a kernel with CONFIG_STRICT_KERNEL_RWX boots.
> 
> I've checked it patch-by-patch now (with STRICT_KERNEL_RWX):
> 
> - patches 1 and 2 build and boot fine
> - patches 3 to 6 build, but fail to boot with this error:

The bug is in patch 2, mmu_mapin_ram() should return base instead of 
returning 0 when __map_without_bats is set.

> 
> 	top of MEM2 @ 13F00000
> 
> 	zImage starting: loaded at 0x00e00000 (sp: 0x01588fa0)
> 	Allocating 0x14e92c8 bytes for kernel...
> 	Decompressing (0x00000000 <- 0x00e11000:0x01586ba7)...
> 	Done! Decompressed 0xdc01f4 bytes
> 
> 	Linux/PowerPC load: root=/dev/mmcblk0p2 rootwait console=usbgecko1
> 	Finalizing device tree... flat tree at 0x15897a0
> 	[    0.000000] printk: bootconsole [udbg0] enabled
> 	[    0.000000] Total memory = 319MB; using 1024kB for hash table (at (ptrval))
> 	[    0.000000] RAM mapped without BATs
> 	[    0.000000] RAM mapped without BATs
> 	[    0.000000] ------------[ cut here ]------------
> 	[    0.000000] kernel BUG at arch/powerpc/mm/pgtable_32.c:223!
> 	[    0.000000] Oops: Exception in kernel mode, sig: 5 [#1]
> 	[    0.000000] BE PREEMPT
> 	[    0.000000] Modules linked in:
> 	[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc1-wii-00024-g596f9fe23c13 #1337
> 	[    0.000000] NIP:  c0017c4c LR: c0a836a0 CTR: c001edc4
> 	[    0.000000] REGS: c0d9deb0 TRAP: 0700   Not tainted  (5.0.0-rc1-wii-00024-g596f9fe23c13)
> 	[    0.000000] MSR:  00020030 <IR,DR>  CR: 42000888  XER: 20000000
> 	[    0.000000]
> 	[    0.000000] GPR00: c0a836a0 c0d9df60 c0d2a4a0 c0d29c00 00000000 c16ff000 c0d9de28 c0dc0000
> 	[    0.000000] GPR08: c0d9c000 00000001 00000001 00000000 28000824 00000000 00000000 00000000
> 	[    0.000000] GPR16: 00000000 00000000 00000020 00000000 c0860000 c0da0000 c0000000 c0a7d000
> 	[    0.000000] GPR24: c0acd55c c0d487c8 13f00000 c0d29000 00000c00 00000311 c0000000 c0d487c8
> 	[    0.000000] NIP [c0017c4c] map_kernel_page+0x78/0xf0
> 	[    0.000000] LR [c0a836a0] mapin_ram+0xe0/0x14c
> 	[    0.000000] Call Trace:
> 	[    0.000000] [c0d9df60] [c0a83f54] mmu_mapin_ram+0x54/0x1a4 (unreliable)
> 	[    0.000000] [c0d9df90] [c0a836a0] mapin_ram+0xe0/0x14c
> 	[    0.000000] [c0d9dfd0] [c0a83578] MMU_init+0x158/0x1a0
> 	[    0.000000] [c0d9dff0] [c0003418] start_here+0x40/0x78
> 	[    0.000000] Instruction dump:
> 	[    0.000000] 55290026 57c5b53a 7ca54a14 3d204000 7f854800 3ca5c000 419e0088 81250000
> 	[    0.000000] 552afffe 552907fe 7d4a4b79 4082004c <0f0a0000> 54840026 7c84eb78 9081000c
> 	[    0.000000] random: get_random_bytes called from print_oops_end_marker+0x34/0x6c with crng_init=0
> 	[    0.000000] ---[ end trace 0000000000000000 ]---
> 	[    0.000000]
> 	[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
> 	[    0.000000] Rebooting in 180 seconds..
> 
> - patches 7 to 11 fail to build with this error (really a warning, but
>    arch/powerpc doesn't allow warnings by default):
> 
> 	  CC      arch/powerpc/mm/ppc_mmu_32.o
> 	../arch/powerpc/mm/ppc_mmu_32.c:133:13: error: ‘clearibat’ defined but not used [-Werror=unused-function]
> 	 static void clearibat(int index)
> 		     ^~~~~~~~~
> 	../arch/powerpc/mm/ppc_mmu_32.c:115:13: error: ‘setibat’ defined but not used [-Werror=unused-function]
> 	 static void setibat(int index, unsigned long virt, phys_addr_t phys,
> 		     ^~~~~~~
> 	cc1: all warnings being treated as errors

Argh ! I have to squash the patch bringing the new functions with the 
one using them (patch 12). The result is a big messy patch which is more 
difficult to review but that's life.

> 
> - patches 12 to 15 build but fail to boot with this error:

Thats the one we need to really understand.

Do you have modules ? If so, can you try without ?

> 
> 	top of MEM2 @ 13F00000
> 
> 	zImage starting: loaded at 0x01000000 (sp: 0x0178afa0)
> 	Allocating 0x166b2c8 bytes for kernel...
> 	Decompressing (0x00000000 <- 0x01011000:0x017880ce)...
> 	Done! Decompressed 0xf421f4 bytes
> 
> 	Linux/PowerPC load: root=/dev/mmcblk0p2 rootwait console=usbgecko1
> 	Finalizing device tree... flat tree at 0x178b7a0
> 	[    0.000000] printk: bootconsole [udbg0] enabled
> 	[    0.000000] Kernel panic - not syncing: ERROR: Failed to allocate 0x00100000 bytes below 0x00000000.
> 	[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc1-wii-00033-gc263f8162122 #1337
> 	[    0.000000] Call Trace:
> 	[    0.000000] [c0f1ff30] [c00280f0] panic+0x144/0x324 (unreliable)
> 	[    0.000000] [c0f1ff90] [c0c18a34] memblock_alloc_base+0x34/0x44
> 	[    0.000000] [c0f1ffa0] [c0c071e0] MMU_init_hw+0xcc/0x300
> 	[    0.000000] [c0f1ffd0] [c0c06554] MMU_init+0x12c/0x198
> 	[    0.000000] [c0f1fff0] [c0003418] start_here+0x40/0x78
> 	[    0.000000] Rebooting in 180 seconds..
> 
> 
> I'll investigate some more tomorrow.

Thanks a lot for your help.

> 
> Jonathan
> 

Christophe
Michael Ellerman Jan. 15, 2019, 10:22 a.m. UTC | #7
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 15/01/2019 à 01:33, Jonathan Neuschäfer a écrit :
...
>> 
>> - patches 7 to 11 fail to build with this error (really a warning, but
>>    arch/powerpc doesn't allow warnings by default):
>> 
>> 	  CC      arch/powerpc/mm/ppc_mmu_32.o
>> 	../arch/powerpc/mm/ppc_mmu_32.c:133:13: error: ‘clearibat’ defined but not used [-Werror=unused-function]
>> 	 static void clearibat(int index)
>> 		     ^~~~~~~~~
>> 	../arch/powerpc/mm/ppc_mmu_32.c:115:13: error: ‘setibat’ defined but not used [-Werror=unused-function]
>> 	 static void setibat(int index, unsigned long virt, phys_addr_t phys,
>> 		     ^~~~~~~
>> 	cc1: all warnings being treated as errors
>
> Argh ! I have to squash the patch bringing the new functions with the 
> one using them (patch 12). The result is a big messy patch which is more 
> difficult to review but that's life.

You don't *have* to squash them.

We like to preserve bisectability, but it's not a 100% hard requirement.

Someone trying to bisect through those patches can always turn off
-Werror with PPC_DISABLE_WERROR. But they probably can just skip them
because they just add new code that's not called yet.

So I won't object if you send them as-is.

cheers
Christophe Leroy Jan. 15, 2019, 10:57 a.m. UTC | #8
Le 15/01/2019 à 11:22, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>> Le 15/01/2019 à 01:33, Jonathan Neuschäfer a écrit :
> ...
>>>
>>> - patches 7 to 11 fail to build with this error (really a warning, but
>>>     arch/powerpc doesn't allow warnings by default):
>>>
>>> 	  CC      arch/powerpc/mm/ppc_mmu_32.o
>>> 	../arch/powerpc/mm/ppc_mmu_32.c:133:13: error: ‘clearibat’ defined but not used [-Werror=unused-function]
>>> 	 static void clearibat(int index)
>>> 		     ^~~~~~~~~
>>> 	../arch/powerpc/mm/ppc_mmu_32.c:115:13: error: ‘setibat’ defined but not used [-Werror=unused-function]
>>> 	 static void setibat(int index, unsigned long virt, phys_addr_t phys,
>>> 		     ^~~~~~~
>>> 	cc1: all warnings being treated as errors
>>
>> Argh ! I have to squash the patch bringing the new functions with the
>> one using them (patch 12). The result is a big messy patch which is more
>> difficult to review but that's life.
> 
> You don't *have* to squash them.
> 
> We like to preserve bisectability, but it's not a 100% hard requirement.
> 
> Someone trying to bisect through those patches can always turn off
> -Werror with PPC_DISABLE_WERROR. But they probably can just skip them
> because they just add new code that's not called yet.

Ok thanks for the note.

> 
> So I won't object if you send them as-is.

Good to know. Anyway I think I will at least re-order so that the patch 
using the new functions immediatly follows the one adding the functions.

Christophe

> 
> cheers
>
Jonathan Neuschäfer Jan. 16, 2019, 12:35 a.m. UTC | #9
On Tue, Jan 15, 2019 at 07:51:01AM +0100, Christophe Leroy wrote:
> Le 15/01/2019 à 01:33, Jonathan Neuschäfer a écrit :
[...]
> > I've checked it patch-by-patch now (with STRICT_KERNEL_RWX):
> > 
> > - patches 1 and 2 build and boot fine
> > - patches 3 to 6 build, but fail to boot with this error:
> 
> The bug is in patch 2, mmu_mapin_ram() should return base instead of
> returning 0 when __map_without_bats is set.

Indeed, with this change, I can boot up to patch 11.

> > - patches 12 to 15 build but fail to boot with this error:
> 
> Thats the one we need to really understand.
> 
> Do you have modules ? If so, can you try without ?

I don't use any modules in my test setup, but I have module support
enabled. Disabling CONFIG_MODULES makes no difference, as far as I can
see (I get the same backtrace with memblock_alloc_base+0x34/0x44).

> > 	[    0.000000] [c0f1ff30] [c00280f0] panic+0x144/0x324 (unreliable)
> > 	[    0.000000] [c0f1ff90] [c0c18a34] memblock_alloc_base+0x34/0x44
> > 	[    0.000000] [c0f1ffa0] [c0c071e0] MMU_init_hw+0xcc/0x300
> > 	[    0.000000] [c0f1ffd0] [c0c06554] MMU_init+0x12c/0x198
> > 	[    0.000000] [c0f1fff0] [c0003418] start_here+0x40/0x78

With a few printks[1], I traced this error, and got the following
result:

[    0.000000] __memblock_find_range_top_down(1000:1800000, 100000:100000, ffffffff, 0)
[    0.000000] __memblock_find_range_top_down: in loop, 10000000:13f00000
[    0.000000] __memblock_find_range_top_down: in loop, 179962d:1800000
[    0.000000] __memblock_find_range_top_down: in loop, 1676000:17987a0
[    0.000000] __memblock_find_range_top_down: nothing found :(

The limit of 0x1800000 comes from setup_initial_memory_limit, which only
considers the first memblock, but the second memblock starts at 256MiB,
so it wouldn't be usable anyway, according to the comment in
setup_initial_memory_limit.

Thinning the kernel down a bit actually makes it boot again. Ooops...!
Maybe enabling CONFIG_STRICT_KERNEL_RWX has made it just large enough to
fail the hash table allocation, but there may have been other factors
involved (I'm not sure exactly).  Sorry for the confusion!


Jonathan

[1]:
diff --git a/mm/memblock.c b/mm/memblock.c
index 022d4cbb3618..66d588e08487 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -215,8 +215,11 @@ __memblock_find_range_top_down(phys_addr_t start, phys_addr_t end,
 	phys_addr_t this_start, this_end, cand;
 	u64 i;
 
+	printk("%s(%x:%x, %x:%x, %x, %x)\n", __func__, start, end, size, align, nid, flags);
+
 	for_each_free_mem_range_reverse(i, nid, flags, &this_start, &this_end,
 					NULL) {
+		printk("%s: in loop, %x:%x\n", __func__, this_start, this_end);
 		this_start = clamp(this_start, start, end);
 		this_end = clamp(this_end, start, end);
 
@@ -228,6 +231,7 @@ __memblock_find_range_top_down(phys_addr_t start, phys_addr_t end,
 			return cand;
 	}
 
+	printk("%s: nothing found :(\n", __func__);
 	return 0;
 }
Christophe Leroy Jan. 16, 2019, 6:55 a.m. UTC | #10
Le 16/01/2019 à 01:35, Jonathan Neuschäfer a écrit :
> On Tue, Jan 15, 2019 at 07:51:01AM +0100, Christophe Leroy wrote:
>> Le 15/01/2019 à 01:33, Jonathan Neuschäfer a écrit :
> [...]
>>> I've checked it patch-by-patch now (with STRICT_KERNEL_RWX):
>>>
>>> - patches 1 and 2 build and boot fine
>>> - patches 3 to 6 build, but fail to boot with this error:
>>
>> The bug is in patch 2, mmu_mapin_ram() should return base instead of
>> returning 0 when __map_without_bats is set.
> 
> Indeed, with this change, I can boot up to patch 11.
> 
>>> - patches 12 to 15 build but fail to boot with this error:
>>
>> Thats the one we need to really understand.
>>
>> Do you have modules ? If so, can you try without ?
> 
> I don't use any modules in my test setup, but I have module support
> enabled. Disabling CONFIG_MODULES makes no difference, as far as I can
> see (I get the same backtrace with memblock_alloc_base+0x34/0x44).
> 
>>> 	[    0.000000] [c0f1ff30] [c00280f0] panic+0x144/0x324 (unreliable)
>>> 	[    0.000000] [c0f1ff90] [c0c18a34] memblock_alloc_base+0x34/0x44
>>> 	[    0.000000] [c0f1ffa0] [c0c071e0] MMU_init_hw+0xcc/0x300
>>> 	[    0.000000] [c0f1ffd0] [c0c06554] MMU_init+0x12c/0x198
>>> 	[    0.000000] [c0f1fff0] [c0003418] start_here+0x40/0x78
> 
> With a few printks[1], I traced this error, and got the following
> result:
> 
> [    0.000000] __memblock_find_range_top_down(1000:1800000, 100000:100000, ffffffff, 0)
> [    0.000000] __memblock_find_range_top_down: in loop, 10000000:13f00000
> [    0.000000] __memblock_find_range_top_down: in loop, 179962d:1800000
> [    0.000000] __memblock_find_range_top_down: in loop, 1676000:17987a0
> [    0.000000] __memblock_find_range_top_down: nothing found :(
> 
> The limit of 0x1800000 comes from setup_initial_memory_limit, which only
> considers the first memblock, but the second memblock starts at 256MiB,
> so it wouldn't be usable anyway, according to the comment in
> setup_initial_memory_limit.

Yes, initial_bats() in head_32.S sets one 256Mb BAT for initial booting:

/*
  * On 601, we use 3 BATs to map up to 24M of RAM at _PAGE_OFFSET
  * (we keep one for debugging) and on others, we use one 256M BAT.
  */
initial_bats:
	lis	r11,PAGE_OFFSET@h
	mfspr	r9,SPRN_PVR
	rlwinm	r9,r9,16,16,31		/* r9 = 1 for 601, 4 for 604 */
	cmpwi	0,r9,1
	bne	4f
...
4:	tophys(r8,r11)
#ifdef CONFIG_SMP
	ori	r8,r8,0x12		/* R/W access, M=1 */
#else
	ori	r8,r8,2			/* R/W access */
#endif /* CONFIG_SMP */
	ori	r11,r11,BL_256M<<2|0x2	/* set up BAT registers for 604 */

	mtspr	SPRN_DBAT0L,r8		/* N.B. 6xx (not 601) have valid */
	mtspr	SPRN_DBAT0U,r11		/* bit in upper BAT register */
	mtspr	SPRN_IBAT0L,r8
	mtspr	SPRN_IBAT0U,r11
	isync
	blr


> 
> Thinning the kernel down a bit actually makes it boot again. Ooops...!
> Maybe enabling CONFIG_STRICT_KERNEL_RWX has made it just large enough to
> fail the hash table allocation, but there may have been other factors
> involved (I'm not sure exactly).  Sorry for the confusion!

Ok, that must be the reason. Thanks for testing.

What about the following modification which maps a second 256Mb BAT, 
does it helps ?



diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index c2f564690778..ea574596de37 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -1160,6 +1160,14 @@ initial_bats:
  	mtspr	SPRN_DBAT0U,r11		/* bit in upper BAT register */
  	mtspr	SPRN_IBAT0L,r8
  	mtspr	SPRN_IBAT0U,r11
+#ifdef CONFIG_WII
+	addis	r11,r11,0x10000000@h
+	addis	r8,r8,0x10000000@h
+	mtspr	SPRN_DBAT2L,r8
+	mtspr	SPRN_DBAT2U,r11
+	mtspr	SPRN_IBAT2L,r8
+	mtspr	SPRN_IBAT2U,r11
+#endif
  	isync
  	blr

diff --git a/arch/powerpc/mm/ppc_mmu_32.c b/arch/powerpc/mm/ppc_mmu_32.c
index 3f4193201ee7..a334fd5210a8 100644
--- a/arch/powerpc/mm/ppc_mmu_32.c
+++ b/arch/powerpc/mm/ppc_mmu_32.c
@@ -259,6 +259,8 @@ void setup_initial_memory_limit(phys_addr_t 
first_memblock_base,
  	/* 601 can only access 16MB at the moment */
  	if (PVR_VER(mfspr(SPRN_PVR)) == 1)
  		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x01000000));
+	else if (IS_ENABLED(CONFIG_WII))
+		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x20000000));
  	else /* Anything else has 256M mapped */
  		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x10000000));
  }


Christophe

> 
> 
> Jonathan
> 
> [1]:
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 022d4cbb3618..66d588e08487 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -215,8 +215,11 @@ __memblock_find_range_top_down(phys_addr_t start, phys_addr_t end,
>   	phys_addr_t this_start, this_end, cand;
>   	u64 i;
>   
> +	printk("%s(%x:%x, %x:%x, %x, %x)\n", __func__, start, end, size, align, nid, flags);
> +
>   	for_each_free_mem_range_reverse(i, nid, flags, &this_start, &this_end,
>   					NULL) {
> +		printk("%s: in loop, %x:%x\n", __func__, this_start, this_end);
>   		this_start = clamp(this_start, start, end);
>   		this_end = clamp(this_end, start, end);
>   
> @@ -228,6 +231,7 @@ __memblock_find_range_top_down(phys_addr_t start, phys_addr_t end,
>   			return cand;
>   	}
>   
> +	printk("%s: nothing found :(\n", __func__);
>   	return 0;
>   }
>   
>
Jonathan Neuschäfer Jan. 16, 2019, 1:16 p.m. UTC | #11
On Wed, Jan 16, 2019 at 07:55:29AM +0100, Christophe Leroy wrote:
> Le 16/01/2019 à 01:35, Jonathan Neuschäfer a écrit :
> > Thinning the kernel down a bit actually makes it boot again. Ooops...!
> > Maybe enabling CONFIG_STRICT_KERNEL_RWX has made it just large enough to
> > fail the hash table allocation, but there may have been other factors
> > involved (I'm not sure exactly).  Sorry for the confusion!
> 
> Ok, that must be the reason. Thanks for testing.
> 
> What about the following modification which maps a second 256Mb BAT, does it
> helps ?
> 
> 
> 
> diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
> index c2f564690778..ea574596de37 100644
> --- a/arch/powerpc/kernel/head_32.S
> +++ b/arch/powerpc/kernel/head_32.S
> @@ -1160,6 +1160,14 @@ initial_bats:
>  	mtspr	SPRN_DBAT0U,r11		/* bit in upper BAT register */
>  	mtspr	SPRN_IBAT0L,r8
>  	mtspr	SPRN_IBAT0U,r11
> +#ifdef CONFIG_WII
> +	addis	r11,r11,0x10000000@h
> +	addis	r8,r8,0x10000000@h
> +	mtspr	SPRN_DBAT2L,r8
> +	mtspr	SPRN_DBAT2U,r11
> +	mtspr	SPRN_IBAT2L,r8
> +	mtspr	SPRN_IBAT2U,r11
> +#endif
>  	isync
>  	blr
> 
> diff --git a/arch/powerpc/mm/ppc_mmu_32.c b/arch/powerpc/mm/ppc_mmu_32.c
> index 3f4193201ee7..a334fd5210a8 100644
> --- a/arch/powerpc/mm/ppc_mmu_32.c
> +++ b/arch/powerpc/mm/ppc_mmu_32.c
> @@ -259,6 +259,8 @@ void setup_initial_memory_limit(phys_addr_t
> first_memblock_base,
>  	/* 601 can only access 16MB at the moment */
>  	if (PVR_VER(mfspr(SPRN_PVR)) == 1)
>  		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x01000000));
> +	else if (IS_ENABLED(CONFIG_WII))
> +		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x20000000));
>  	else /* Anything else has 256M mapped */
>  		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x10000000));
>  }

I haven't tested it, but this patch won't be enough, because we're only
looking at the first memblock, and the additional memory in the Wii
(MEM2) is the second memblock.


Jonathan
Christophe Leroy Jan. 16, 2019, 1:34 p.m. UTC | #12
Le 16/01/2019 à 14:16, Jonathan Neuschäfer a écrit :
> On Wed, Jan 16, 2019 at 07:55:29AM +0100, Christophe Leroy wrote:
>> Le 16/01/2019 à 01:35, Jonathan Neuschäfer a écrit :
>>> Thinning the kernel down a bit actually makes it boot again. Ooops...!
>>> Maybe enabling CONFIG_STRICT_KERNEL_RWX has made it just large enough to
>>> fail the hash table allocation, but there may have been other factors
>>> involved (I'm not sure exactly).  Sorry for the confusion!
>>
>> Ok, that must be the reason. Thanks for testing.
>>
>> What about the following modification which maps a second 256Mb BAT, does it
>> helps ?
>>
>>
>>
>> diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
>> index c2f564690778..ea574596de37 100644
>> --- a/arch/powerpc/kernel/head_32.S
>> +++ b/arch/powerpc/kernel/head_32.S
>> @@ -1160,6 +1160,14 @@ initial_bats:
>>   	mtspr	SPRN_DBAT0U,r11		/* bit in upper BAT register */
>>   	mtspr	SPRN_IBAT0L,r8
>>   	mtspr	SPRN_IBAT0U,r11
>> +#ifdef CONFIG_WII
>> +	addis	r11,r11,0x10000000@h
>> +	addis	r8,r8,0x10000000@h
>> +	mtspr	SPRN_DBAT2L,r8
>> +	mtspr	SPRN_DBAT2U,r11
>> +	mtspr	SPRN_IBAT2L,r8
>> +	mtspr	SPRN_IBAT2U,r11
>> +#endif
>>   	isync
>>   	blr
>>
>> diff --git a/arch/powerpc/mm/ppc_mmu_32.c b/arch/powerpc/mm/ppc_mmu_32.c
>> index 3f4193201ee7..a334fd5210a8 100644
>> --- a/arch/powerpc/mm/ppc_mmu_32.c
>> +++ b/arch/powerpc/mm/ppc_mmu_32.c
>> @@ -259,6 +259,8 @@ void setup_initial_memory_limit(phys_addr_t
>> first_memblock_base,
>>   	/* 601 can only access 16MB at the moment */
>>   	if (PVR_VER(mfspr(SPRN_PVR)) == 1)
>>   		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x01000000));
>> +	else if (IS_ENABLED(CONFIG_WII))
>> +		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x20000000));
>>   	else /* Anything else has 256M mapped */
>>   		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x10000000));
>>   }
> 
> I haven't tested it, but this patch won't be enough, because we're only
> looking at the first memblock, and the additional memory in the Wii
> (MEM2) is the second memblock.
> 

Yes right.


Would the following work instead ?

memblock_set_current_limit(0x20000000);


Christophe
Jonathan Neuschäfer Jan. 16, 2019, 11:48 p.m. UTC | #13
On Wed, Jan 16, 2019 at 02:34:53PM +0100, Christophe Leroy wrote:
> Le 16/01/2019 à 14:16, Jonathan Neuschäfer a écrit :
> > On Wed, Jan 16, 2019 at 07:55:29AM +0100, Christophe Leroy wrote:
> > > Le 16/01/2019 à 01:35, Jonathan Neuschäfer a écrit :
> > > > Thinning the kernel down a bit actually makes it boot again. Ooops...!
> > > > Maybe enabling CONFIG_STRICT_KERNEL_RWX has made it just large enough to
> > > > fail the hash table allocation, but there may have been other factors
> > > > involved (I'm not sure exactly).  Sorry for the confusion!
> > > 
> > > Ok, that must be the reason. Thanks for testing.
> > > 
> > > What about the following modification which maps a second 256Mb BAT, does it
> > > helps ?
> > > 
> > > 
> > > 
> > > diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
> > > index c2f564690778..ea574596de37 100644
> > > --- a/arch/powerpc/kernel/head_32.S
> > > +++ b/arch/powerpc/kernel/head_32.S
> > > @@ -1160,6 +1160,14 @@ initial_bats:
> > >   	mtspr	SPRN_DBAT0U,r11		/* bit in upper BAT register */
> > >   	mtspr	SPRN_IBAT0L,r8
> > >   	mtspr	SPRN_IBAT0U,r11
> > > +#ifdef CONFIG_WII
> > > +	addis	r11,r11,0x10000000@h
> > > +	addis	r8,r8,0x10000000@h
> > > +	mtspr	SPRN_DBAT2L,r8
> > > +	mtspr	SPRN_DBAT2U,r11
> > > +	mtspr	SPRN_IBAT2L,r8
> > > +	mtspr	SPRN_IBAT2U,r11
> > > +#endif
> > >   	isync
> > >   	blr
> > > 
> > > diff --git a/arch/powerpc/mm/ppc_mmu_32.c b/arch/powerpc/mm/ppc_mmu_32.c
> > > index 3f4193201ee7..a334fd5210a8 100644
> > > --- a/arch/powerpc/mm/ppc_mmu_32.c
> > > +++ b/arch/powerpc/mm/ppc_mmu_32.c
> > > @@ -259,6 +259,8 @@ void setup_initial_memory_limit(phys_addr_t
> > > first_memblock_base,
> > >   	/* 601 can only access 16MB at the moment */
> > >   	if (PVR_VER(mfspr(SPRN_PVR)) == 1)
> > >   		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x01000000));
> > > +	else if (IS_ENABLED(CONFIG_WII))
> > > +		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x20000000));
> > >   	else /* Anything else has 256M mapped */
> > >   		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x10000000));
> > >   }
> > 
> > I haven't tested it, but this patch won't be enough, because we're only
> > looking at the first memblock, and the additional memory in the Wii
> > (MEM2) is the second memblock.
> > 
> 
> Yes right.
> 
> 
> Would the following work instead ?
> 
> memblock_set_current_limit(0x20000000);

With the config at https://gist.githubusercontent.com/neuschaefer/12ccc87ff8aeff543fad558e8742cd2b/raw/d49d321709cac364779e6893bbd91ff5a80bcb03/config
it still doesn't boot, but with a different error:

top of MEM2 @ 13F00000

zImage starting: loaded at 0x01000000 (sp: 0x0178afa0)
Allocating 0x166b2c8 bytes for kernel...
Decompressing (0x00000000 <- 0x01011000:0x01788799)...
Done! Decompressed 0xf421f4 bytes

Linux/PowerPC load: root=/dev/mmcblk0p2 rootwait console=usbgecko1
Finalizing device tree... flat tree at 0x178b7a0
[    0.000000] printk: bootconsole [udbg0] enabled
[    0.000000] __memblock_find_range_top_down(1000:20000000, 100000:100000, ffffffff, 0)
[    0.000000] __memblock_find_range_top_down: in loop, 10000000:13f00000
[    0.000000] Total memory = 319MB; using 1024kB for hash table (at d3e00000)
[    0.000000] __memblock_find_range_top_down(1000:20000000, 1000:1000, ffffffff, 0)
[    0.000000] __memblock_find_range_top_down: in loop, 10000000:13e00000
[    0.000000] __memblock_find_range_top_down(1000:20000000, 1000:1000, ffffffff, 0)
[    0.000000] __memblock_find_range_top_down: in loop, 10000000:13dff000
[    0.000000] BUG: Unable to handle kernel data access at 0xc106a434
[    0.000000] Faulting instruction address: 0xc0071bf4
[    0.000000] Oops: Kernel access of bad area, sig: 11 [#1]
[    0.000000] BE PREEMPT
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc1-wii-00038-gc34b70d591b6-dirty #1337
[    0.000000] NIP:  c0071bf4 LR: c00727d8 CTR: 00000000
[    0.000000] REGS: c0f1fd30 TRAP: 0300   Not tainted  (5.0.0-rc1-wii-00038-gc34b70d591b6-dirty)
[    0.000000] MSR:  00001032 <ME,IR,DR,RI>  CR: 44002842  XER: 00000000
[    0.000000] DAR: c106a434 DSISR: 40000000
[    0.000000] GPR00: c0074a98 c0f1fde0 c0ead4a0 c0ead4a0 c0ead9c8 00000008 00000000 00000000
[    0.000000] GPR08: 00000003 c106a418 00000258 00000001 24000444 fb43ef5b c0f30000 7561f327
[    0.000000] GPR16: c0f40000 691cfd11 38afe359 f161e513 00000000 c0ead9c8 00000001 00000000
[    0.000000] GPR24: 00000000 c0f46288 00000000 c0ead4a0 c0ead9c8 00000008 c0ead4a0 00000100
[    0.000000] NIP [c0071bf4] mark_lock+0x64/0x858
[    0.000000] LR [c00727d8] __lock_acquire+0x334/0x1a40
[    0.000000] Call Trace:
[    0.000000] [c0f1fe20] [00000006] 0x6
[    0.000000] [c0f1fed0] [c0074a98] lock_acquire+0x110/0x20c
[    0.000000] [c0f1ff10] [c085e8cc] _raw_spin_lock+0x44/0x60
[    0.000000] [c0f1ff30] [c007f220] vprintk_emit+0xa0/0x328
[    0.000000] [c0f1ff70] [c007fc48] printk+0x5c/0x84
[    0.000000] [c0f1ffb0] [c0c00854] start_kernel+0x64/0x460
[    0.000000] [c0f1fff0] [00003438] 0x3438
[    0.000000] Instruction dump:
[    0.000000] 41820170 55086cfe 550a083c 7d4a4214 554a1838 3d20c0f4 7d4a4214 39296288
[    0.000000] 554a1838 3d290012 7d295214 39293f38 <8129001c> 7fe94839 41820028 3bc00001
[    0.000000] random: get_random_bytes called from print_oops_end_marker+0x34/0x6c with crng_init=0
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000]
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!

Not sure what's wrong. It does work with wii_defconfig, though.
I think for now I'm happy without this patch, as it doesn't seem to be
really necessary.


Jonathan
Christophe Leroy Jan. 17, 2019, 10:14 a.m. UTC | #14
Le 17/01/2019 à 00:48, Jonathan Neuschäfer a écrit :
> On Wed, Jan 16, 2019 at 02:34:53PM +0100, Christophe Leroy wrote:
>> Le 16/01/2019 à 14:16, Jonathan Neuschäfer a écrit :
>>> On Wed, Jan 16, 2019 at 07:55:29AM +0100, Christophe Leroy wrote:
>>>> Le 16/01/2019 à 01:35, Jonathan Neuschäfer a écrit :
>>>>> Thinning the kernel down a bit actually makes it boot again. Ooops...!
>>>>> Maybe enabling CONFIG_STRICT_KERNEL_RWX has made it just large enough to
>>>>> fail the hash table allocation, but there may have been other factors
>>>>> involved (I'm not sure exactly).  Sorry for the confusion!
>>>>
>>>> Ok, that must be the reason. Thanks for testing.
>>>>
>>>> What about the following modification which maps a second 256Mb BAT, does it
>>>> helps ?
>>>>
>>>>
>>>>
>>>> diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
>>>> index c2f564690778..ea574596de37 100644
>>>> --- a/arch/powerpc/kernel/head_32.S
>>>> +++ b/arch/powerpc/kernel/head_32.S
>>>> @@ -1160,6 +1160,14 @@ initial_bats:
>>>>    	mtspr	SPRN_DBAT0U,r11		/* bit in upper BAT register */
>>>>    	mtspr	SPRN_IBAT0L,r8
>>>>    	mtspr	SPRN_IBAT0U,r11
>>>> +#ifdef CONFIG_WII
>>>> +	addis	r11,r11,0x10000000@h
>>>> +	addis	r8,r8,0x10000000@h
>>>> +	mtspr	SPRN_DBAT2L,r8
>>>> +	mtspr	SPRN_DBAT2U,r11
>>>> +	mtspr	SPRN_IBAT2L,r8
>>>> +	mtspr	SPRN_IBAT2U,r11
>>>> +#endif
>>>>    	isync
>>>>    	blr
>>>>
>>>> diff --git a/arch/powerpc/mm/ppc_mmu_32.c b/arch/powerpc/mm/ppc_mmu_32.c
>>>> index 3f4193201ee7..a334fd5210a8 100644
>>>> --- a/arch/powerpc/mm/ppc_mmu_32.c
>>>> +++ b/arch/powerpc/mm/ppc_mmu_32.c
>>>> @@ -259,6 +259,8 @@ void setup_initial_memory_limit(phys_addr_t
>>>> first_memblock_base,
>>>>    	/* 601 can only access 16MB at the moment */
>>>>    	if (PVR_VER(mfspr(SPRN_PVR)) == 1)
>>>>    		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x01000000));
>>>> +	else if (IS_ENABLED(CONFIG_WII))
>>>> +		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x20000000));
>>>>    	else /* Anything else has 256M mapped */
>>>>    		memblock_set_current_limit(min_t(u64, first_memblock_size, 0x10000000));
>>>>    }
>>>
>>> I haven't tested it, but this patch won't be enough, because we're only
>>> looking at the first memblock, and the additional memory in the Wii
>>> (MEM2) is the second memblock.
>>>
>>
>> Yes right.
>>
>>
>> Would the following work instead ?
>>
>> memblock_set_current_limit(0x20000000);
> 
> With the config at https://gist.githubusercontent.com/neuschaefer/12ccc87ff8aeff543fad558e8742cd2b/raw/d49d321709cac364779e6893bbd91ff5a80bcb03/config
> it still doesn't boot, but with a different error:
> 
> top of MEM2 @ 13F00000
> 
> zImage starting: loaded at 0x01000000 (sp: 0x0178afa0)
> Allocating 0x166b2c8 bytes for kernel...
> Decompressing (0x00000000 <- 0x01011000:0x01788799)...
> Done! Decompressed 0xf421f4 bytes
> 
> Linux/PowerPC load: root=/dev/mmcblk0p2 rootwait console=usbgecko1
> Finalizing device tree... flat tree at 0x178b7a0
> [    0.000000] printk: bootconsole [udbg0] enabled
> [    0.000000] __memblock_find_range_top_down(1000:20000000, 100000:100000, ffffffff, 0)
> [    0.000000] __memblock_find_range_top_down: in loop, 10000000:13f00000
> [    0.000000] Total memory = 319MB; using 1024kB for hash table (at d3e00000)
> [    0.000000] __memblock_find_range_top_down(1000:20000000, 1000:1000, ffffffff, 0)
> [    0.000000] __memblock_find_range_top_down: in loop, 10000000:13e00000
> [    0.000000] __memblock_find_range_top_down(1000:20000000, 1000:1000, ffffffff, 0)
> [    0.000000] __memblock_find_range_top_down: in loop, 10000000:13dff000
> [    0.000000] BUG: Unable to handle kernel data access at 0xc106a434
> [    0.000000] Faulting instruction address: 0xc0071bf4
> [    0.000000] Oops: Kernel access of bad area, sig: 11 [#1]
> [    0.000000] BE PREEMPT
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc1-wii-00038-gc34b70d591b6-dirty #1337
> [    0.000000] NIP:  c0071bf4 LR: c00727d8 CTR: 00000000
> [    0.000000] REGS: c0f1fd30 TRAP: 0300   Not tainted  (5.0.0-rc1-wii-00038-gc34b70d591b6-dirty)
> [    0.000000] MSR:  00001032 <ME,IR,DR,RI>  CR: 44002842  XER: 00000000
> [    0.000000] DAR: c106a434 DSISR: 40000000
> [    0.000000] GPR00: c0074a98 c0f1fde0 c0ead4a0 c0ead4a0 c0ead9c8 00000008 00000000 00000000
> [    0.000000] GPR08: 00000003 c106a418 00000258 00000001 24000444 fb43ef5b c0f30000 7561f327
> [    0.000000] GPR16: c0f40000 691cfd11 38afe359 f161e513 00000000 c0ead9c8 00000001 00000000
> [    0.000000] GPR24: 00000000 c0f46288 00000000 c0ead4a0 c0ead9c8 00000008 c0ead4a0 00000100
> [    0.000000] NIP [c0071bf4] mark_lock+0x64/0x858
> [    0.000000] LR [c00727d8] __lock_acquire+0x334/0x1a40
> [    0.000000] Call Trace:
> [    0.000000] [c0f1fe20] [00000006] 0x6
> [    0.000000] [c0f1fed0] [c0074a98] lock_acquire+0x110/0x20c
> [    0.000000] [c0f1ff10] [c085e8cc] _raw_spin_lock+0x44/0x60
> [    0.000000] [c0f1ff30] [c007f220] vprintk_emit+0xa0/0x328
> [    0.000000] [c0f1ff70] [c007fc48] printk+0x5c/0x84
> [    0.000000] [c0f1ffb0] [c0c00854] start_kernel+0x64/0x460
> [    0.000000] [c0f1fff0] [00003438] 0x3438
> [    0.000000] Instruction dump:
> [    0.000000] 41820170 55086cfe 550a083c 7d4a4214 554a1838 3d20c0f4 7d4a4214 39296288
> [    0.000000] 554a1838 3d290012 7d295214 39293f38 <8129001c> 7fe94839 41820028 3bc00001
> [    0.000000] random: get_random_bytes called from print_oops_end_marker+0x34/0x6c with crng_init=0
> [    0.000000] ---[ end trace 0000000000000000 ]---
> [    0.000000]
> [    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
> 
> Not sure what's wrong. It does work with wii_defconfig, though.
> I think for now I'm happy without this patch, as it doesn't seem to be
> really necessary.

That's strange, 0xc106a434 is within the first block, should not be a 
problem, should it ?

According to DSISR, the fault is due to: Set if the translation of an 
attempted access is not found in the primary or secondary hash table entry
group (HTEG), or in the range of a DBAT register (page fault condition)

Ok, won't spend more time on that for now, 24Mbytes should be OK for 
early init in most cases.

Christophe

> 
> 
> Jonathan
>
Michael Ellerman Feb. 20, 2019, 1:23 p.m. UTC | #15
Christophe Leroy <christophe.leroy@c-s.fr> writes:

> Le 15/01/2019 à 11:22, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>> Le 15/01/2019 à 01:33, Jonathan Neuschäfer a écrit :
>> ...
>>>>
>>>> - patches 7 to 11 fail to build with this error (really a warning, but
>>>>     arch/powerpc doesn't allow warnings by default):
>>>>
>>>> 	  CC      arch/powerpc/mm/ppc_mmu_32.o
>>>> 	../arch/powerpc/mm/ppc_mmu_32.c:133:13: error: ‘clearibat’ defined but not used [-Werror=unused-function]
>>>> 	 static void clearibat(int index)
>>>> 		     ^~~~~~~~~
>>>> 	../arch/powerpc/mm/ppc_mmu_32.c:115:13: error: ‘setibat’ defined but not used [-Werror=unused-function]
>>>> 	 static void setibat(int index, unsigned long virt, phys_addr_t phys,
>>>> 		     ^~~~~~~
>>>> 	cc1: all warnings being treated as errors
>>>
>>> Argh ! I have to squash the patch bringing the new functions with the
>>> one using them (patch 12). The result is a big messy patch which is more
>>> difficult to review but that's life.
>> 
>> You don't *have* to squash them.
>> 
>> We like to preserve bisectability, but it's not a 100% hard requirement.
>> 
>> Someone trying to bisect through those patches can always turn off
>> -Werror with PPC_DISABLE_WERROR. But they probably can just skip them
>> because they just add new code that's not called yet.
>
> Ok thanks for the note.
>
>> 
>> So I won't object if you send them as-is.
>
> Good to know. Anyway I think I will at least re-order so that the patch 
> using the new functions immediatly follows the one adding the functions.
 
Based on that I'm expecting a v3 of this series, right?

cheers
Christophe Leroy Feb. 20, 2019, 3:30 p.m. UTC | #16
Le 20/02/2019 à 14:23, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
> 
>> Le 15/01/2019 à 11:22, Michael Ellerman a écrit :
>>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>>> Le 15/01/2019 à 01:33, Jonathan Neuschäfer a écrit :
>>> ...
>>>>>
>>>>> - patches 7 to 11 fail to build with this error (really a warning, but
>>>>>      arch/powerpc doesn't allow warnings by default):
>>>>>
>>>>> 	  CC      arch/powerpc/mm/ppc_mmu_32.o
>>>>> 	../arch/powerpc/mm/ppc_mmu_32.c:133:13: error: ‘clearibat’ defined but not used [-Werror=unused-function]
>>>>> 	 static void clearibat(int index)
>>>>> 		     ^~~~~~~~~
>>>>> 	../arch/powerpc/mm/ppc_mmu_32.c:115:13: error: ‘setibat’ defined but not used [-Werror=unused-function]
>>>>> 	 static void setibat(int index, unsigned long virt, phys_addr_t phys,
>>>>> 		     ^~~~~~~
>>>>> 	cc1: all warnings being treated as errors
>>>>
>>>> Argh ! I have to squash the patch bringing the new functions with the
>>>> one using them (patch 12). The result is a big messy patch which is more
>>>> difficult to review but that's life.
>>>
>>> You don't *have* to squash them.
>>>
>>> We like to preserve bisectability, but it's not a 100% hard requirement.
>>>
>>> Someone trying to bisect through those patches can always turn off
>>> -Werror with PPC_DISABLE_WERROR. But they probably can just skip them
>>> because they just add new code that's not called yet.
>>
>> Ok thanks for the note.
>>
>>>
>>> So I won't object if you send them as-is.
>>
>> Good to know. Anyway I think I will at least re-order so that the patch
>> using the new functions immediatly follows the one adding the functions.
>   
> Based on that I'm expecting a v3 of this series, right?

Right. Coming soon.

Christophe