RESEND: Re: Problem booting a PowerBook G4 Aluminum after commit cd08f109 with CONFIG_VMAP_STACK=y

On 2/11/20 1:23 PM, Christophe Leroy wrote:
> Can you send me a picture of that BUG Unable to handle kernel data access with 
> all the registers values etc..., together with the matching vmlinux ?
> 
> First thing is to identify where we are when that happens. That mean see what is 
> at 0xc0013674. Can be done with 'ppc-linux-objdump -d vmlinux' (Or whatever your 
> PPC objdump is named) and get the function code.
> 
> Then we need to understand how we reach that function and why it tries to access 
> a physical address.
> 
> 
> Another thing I'm thinking about, not necessarily related to that problem: Some 
> buggy drivers do DMA from stack. This doesn't work anymore with 
> CONFIG_VMAP_STACK. Most of them can be detected with CONFIG_DEBUG_VIRTUAL so you 
> should activate it.

Christophe,

The previous send of this message failed because the attached vmlinux was too large.

I have gone about as far as I can in debugging the problem. Setting 
CONFIG_DEBUG_VIRTUAL made no difference.

Attached are the final screenshot, and the patches that I have applied. You 
already have the gzipped vmlinux.

My kernel is built with commit cd08f1089 with patches applied as in the attached 
file. These include the change to arch/powerpc/kernel/entry_32.S, line 217. The 
next patch was to arch/powerpc/mm/mem.c where I added a call to 
btext_clearscreen() to stop the bootconsole from overwriting lines. If does not 
scroll, but jumps from the bottom to the top.

The other change I made to the program was in kernel/printk/printk.c where I 
returned early from unregister_console() to keep logging going. As you noted, 
the console goes silent when the bootconsole is disabled.

The rest of the changes were made to follow the flow of the program to see where 
if failed. The following change showed where the problem happens:

index 8bf5f2f..c69d91d9 100644
The logged output shows that the problem happens in schedule(). At that point, 
we are back in assembler and out of my realm.

Thanks for your help, and I will supply any additional info and testing that you 
need.

Larry

Christophe,

With this patch, it gets further. Sometime after the boot process tries to start 
process init, it crashes with the unable to read data at 0x000157a0 with a 
faulting address of 0xc001683c. The screenshot is attached and the gzipped 
vmlinux is at http://www.lwfinger.com/download/vmlinux2.gz. The patches that 
were applied for this kernel are also attached,

Larry

Larry,

Le 14/02/2020 à 00:09, Larry Finger a écrit :
> Christophe,
> 
> With this patch, it gets further. Sometime after the boot process tries 
> to start process init, it crashes with the unable to read data at 
> 0x000157a0 with a faulting address of 0xc001683c. The screenshot is 
> attached and the gzipped vmlinux is at 
> http://www.lwfinger.com/download/vmlinux2.gz. The patches that were 
> applied for this kernel are also attached,
> 


Did you try with the patch at https://patchwork.ozlabs.org/patch/1237387/ ?

I see the problem happens in kprobe_handler(). Can you try without 
CONFIG_KPROBE ?

Christophe

Le 14/02/2020 à 07:24, Christophe Leroy a écrit :
> Larry,
> 
> Le 14/02/2020 à 00:09, Larry Finger a écrit :
>> Christophe,
>>
>> With this patch, it gets further. Sometime after the boot process 
>> tries to start process init, it crashes with the unable to read data 
>> at 0x000157a0 with a faulting address of 0xc001683c. The screenshot is 
>> attached and the gzipped vmlinux is at 
>> http://www.lwfinger.com/download/vmlinux2.gz. The patches that were 
>> applied for this kernel are also attached,
>>
> 
> 
> Did you try with the patch at https://patchwork.ozlabs.org/patch/1237387/ ?
> 
> I see the problem happens in kprobe_handler(). Can you try without 
> CONFIG_KPROBE ?
> 

In fact, you hit two bugs. The first one is due to CONFIG_VMAP_STACK. 
The second one has always existed (at least since kernel source tree has 
been in git).

First bug is in function enter_rtas() which tries to read data on stack 
by using the linear physical address translation. This cannot be used 
with VM stack, it must re-enable data MMU translation to access data on 
the stack.

Second bug is in kprobe_handler() function, which does:

	if (*addr != BREAKPOINT_INSTRUCTION)

addr is the address where the 'trap' happened. When a trap happens with 
MMU disabled, addr contains the physical address of the trap. 
kprobe_handler() tries to read the instruction using physical address 
whereas MMU is enabled, so you get a bad access either because the said 
address is not mapped, or because access to userspace is not allowed.


Due to the first bug, you get a 'machine check', and as 
current->thread.rtas_sp has not been cleared yet, the machine check 
handler jumps to 'machine_check_in_rtas'.

machine_check_in_rtas does a trap, which in turn triggers the second bug.


Once the first bug is fixed, the second one should not popup.

Can you test patch https://patchwork.ozlabs.org/patch/1237929/ that 
fixes the first bug ?

Christophe

On 2/14/20 12:24 AM, Christophe Leroy wrote:
> 
> Did you try with the patch at https://patchwork.ozlabs.org/patch/1237387/ ?

Christophe,

When I apply that patch, there is an error at

--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -301,6 +301,39 @@  MachineCheck:
  	. = 0x300
  	DO_KVM  0x300
  DataAccess:

It complains about "an attempt to move .org backwards".

Larry

On 2/14/20 12:24 AM, Christophe Leroy wrote:
> 
> Did you try with the patch at https://patchwork.ozlabs.org/patch/1237387/ ?

Christophe,

When I apply that patch, there is an error at

--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -301,6 +301,39 @@  MachineCheck:
  	. = 0x300
  	DO_KVM  0x300
  DataAccess:

It complains about "an attempt to move .org backwards".

When I change the 0x300 to 0x310 in two places, it builds OK. Is that OK?

Larry

On 02/14/2020 06:24 PM, Larry Finger wrote:
> On 2/14/20 12:24 AM, Christophe Leroy wrote:
>>
>> Did you try with the patch at 
>> https://patchwork.ozlabs.org/patch/1237387/ ?
> 
> Christophe,
> 
> When I apply that patch, there is an error at
> 
> --- a/arch/powerpc/kernel/head_32.S
> +++ b/arch/powerpc/kernel/head_32.S
> @@ -301,6 +301,39 @@  MachineCheck:
>       . = 0x300
>       DO_KVM  0x300
>   DataAccess:
> 
> It complains about "an attempt to move .org backwards".
> 

Argh !

> When I change the 0x300 to 0x310 in two places, it builds OK. Is that OK?

No you can't do that.

The following should solve it for your case.

---
diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index 32875afb3319..f9941b766f63 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -270,6 +270,9 @@ __secondary_hold_acknowledge:
   * pointer when we take an exception from supervisor mode.)
   *	-- paulus.
   */
+#ifdef CONFIG_PPC_CHRP
+1:	b	machine_check_in_rtas
+#endif
  	. = 0x200
  	DO_KVM  0x200
  MachineCheck:
@@ -290,12 +293,9 @@ MachineCheck:
  7:	EXCEPTION_PROLOG_2
  	addi	r3,r1,STACK_FRAME_OVERHEAD
  #ifdef CONFIG_PPC_CHRP
-	bne	cr1,1f
+	bne	cr1,1b
  #endif
  	EXC_XFER_STD(0x200, machine_check_exception)
-#ifdef CONFIG_PPC_CHRP
-1:	b	machine_check_in_rtas
-#endif

  /* Data access exception. */
  	. = 0x300
---

Christophe

Christophe,

On 2/14/20 1:35 PM, Christophe Leroy wrote:
> --- a/arch/powerpc/kernel/head_32.S
> +++ b/arch/powerpc/kernel/head_32.S
> @@ -270,6 +270,9 @@ __secondary_hold_acknowledge:
>    * pointer when we take an exception from supervisor mode.)
>    *    -- paulus.
>    */
> +#ifdef CONFIG_PPC_CHRP
> +1:    b    machine_check_in_rtas
> +#endif
>       . = 0x200
>       DO_KVM  0x200
>   MachineCheck:
> @@ -290,12 +293,9 @@ MachineCheck:
>   7:    EXCEPTION_PROLOG_2
>       addi    r3,r1,STACK_FRAME_OVERHEAD
>   #ifdef CONFIG_PPC_CHRP
> -    bne    cr1,1f
> +    bne    cr1,1b
>   #endif
>       EXC_XFER_STD(0x200, machine_check_exception)
> -#ifdef CONFIG_PPC_CHRP
> -1:    b    machine_check_in_rtas
> -#endif
> 
>   /* Data access exception. */
>       . = 0x300

With the above changes and all the other patches applied, the machine finally 
boots. It is so bloody slow that it takes a long time to do anything, but you 
finally got all the places that needed patches. I really lost track of how many 
bugs were fixed in the process, but I can now put that old box aside until time 
for v5.7.0-rc1. As you can tell, it only gets used to verify that PPC32 is 
working on real G4 hardware. It has no real value for any other function.

Thanks for the help,

Larry

Le 15/02/2020 à 03:42, Larry Finger a écrit :
> Christophe,
> 
> On 2/14/20 1:35 PM, Christophe Leroy wrote:
>> --- a/arch/powerpc/kernel/head_32.S
>> +++ b/arch/powerpc/kernel/head_32.S
>> @@ -270,6 +270,9 @@ __secondary_hold_acknowledge:
>>    * pointer when we take an exception from supervisor mode.)
>>    *    -- paulus.
>>    */
>> +#ifdef CONFIG_PPC_CHRP
>> +1:    b    machine_check_in_rtas
>> +#endif
>>       . = 0x200
>>       DO_KVM  0x200
>>   MachineCheck:
>> @@ -290,12 +293,9 @@ MachineCheck:
>>   7:    EXCEPTION_PROLOG_2
>>       addi    r3,r1,STACK_FRAME_OVERHEAD
>>   #ifdef CONFIG_PPC_CHRP
>> -    bne    cr1,1f
>> +    bne    cr1,1b
>>   #endif
>>       EXC_XFER_STD(0x200, machine_check_exception)
>> -#ifdef CONFIG_PPC_CHRP
>> -1:    b    machine_check_in_rtas
>> -#endif

I'll need to make it a bit different because it shoehorns into your 
config but won't fit if CONFIG_KVM_BOOK3S_32 is added.

>>
>>   /* Data access exception. */
>>       . = 0x300
> 
> With the above changes and all the other patches applied, the machine 
> finally boots. It is so bloody slow that it takes a long time to do 
> anything, but you finally got all the places that needed patches. I 
> really lost track of how many bugs were fixed in the process, but I can 
> now put that old box aside until time for v5.7.0-rc1. As you can tell, 
> it only gets used to verify that PPC32 is working on real G4 hardware. 
> It has no real value for any other function.

Yes, I don't have a G4 myself but this is so much nested with other 
stuff for the powerpc 83xx than we can't avoid the changes impacting the 
G4 and other hash-MMU based PPC32 allthough the changes I'm doing are 
not targetted at those platform at first. And as the 83xx is a 603 core, 
it is non-hash so all hash related things can't be verified. Plus all 
those small parts like power saving, RTAS, etc... which are more specific.
And checking with all possible options is also not easy.

VMAP-STACK was really a challenging functionnality, I'm happy it made 
its way to mainline though.

> 
> Thanks for the help,

Thanks to you for testing and for your patience.

Christophe