Opened 10 years ago

Closed 9 years ago

#6032 closed enhancement (fixed)

[PATCH] Implement arch_debug_call_with_fault_handler for ppc

Reported by: andreasf Owned by: axeld
Priority: normal Milestone: R1
Component: System/Kernel Version: R1/Development
Keywords: Cc: stevenh, kallisti5
Blocked By: Blocking:
Has a Patch: yes Platform: PowerPC

Description

Following up on #5956, here's my try at implementing the missing pieces for a KDL stack trace.

This is what I get so far:

Welcome to Kernel Debugging Land...
Running on CPU 0
Current thread pointer is 0x801c9280, which is an address we can't read from.
stack trace for thread 0x0 ""
    kernel stack: 0x00000000 to 0x00000000
frame            caller     <image>:function + offset

DEFAULT CATCH!, code=900 at   %SRR0: ff818d70   %SRR1: 0000b030

Attachments (5)

ppc_asm_offsets.diff (1.5 KB ) - added by andreasf 10 years ago.
proposed patch: add asm_offsets.cpp
ppc_arch_debug_call_with_fault_handler_2010-05-18.diff (2.5 KB ) - added by andreasf 10 years ago.
draft patch: Implement arch_debug_call_with_fault_handler in assembly
0002-Add-support-for-per-CPU-fault_handler.patch (1.2 KB ) - added by andreasf 9 years ago.
proposed patch: add support for CPU fault handler
0003-Implement-arch_debug_call_with_fault_handler-in-asse.patch (3.0 KB ) - added by andreasf 9 years ago.
proposed patch: implement arch_debug_call_with_fault_handler
0004-Insert-panic-for-testing.patch (720 bytes ) - added by andreasf 9 years ago.
test case: panic

Download all attachments as: .zip

Change History (10)

by andreasf, 10 years ago

Attachment: ppc_asm_offsets.diff added

proposed patch: add asm_offsets.cpp

comment:1 by andreasf, 10 years ago

Has a Patch: set

by andreasf, 10 years ago

draft patch: Implement arch_debug_call_with_fault_handler in assembly

comment:3 by andreasf, 10 years ago

Attached are two patches. I think the asm_offsets.cpp patch is ready to be committed. The other one needs careful review. Seeing how complicated it got, maybe it's better to use inline assembly in the original C function?

I tested it by adding a panic("..."); in main.cpp just after TRACE("init VM").

in reply to:  3 comment:4 by bonefish, 9 years ago

Replying to andreasf:

Attached are two patches. I think the asm_offsets.cpp patch is ready to be committed. The other one needs careful review. Seeing how complicated it got, maybe it's better to use inline assembly in the original C function?

Unfortunately that is not possible. As mentioned in the other ticket, full control over the registers (at least the stack frame pointer) is needed, which isn't the case with inline assembly (the C++ compiler controls the registers and the stack).

353	FUNCTION(arch_debug_call_with_fault_handler):
354	    // store jump buffer pointer on stack
355	    stwu    %r4, -4(%r1)

A stack frame needs to be set up for the function. Otherwise stack traces won't work. All stwu and lwzu instructions must be replaced.

356	    // set CPU's fault handler
357	    lis     %r13, 1b@ha
358	    ori     %r13, %r13, 1b@l
359	    stw     %r13, CPU_ENT_fault_handler(%r3)

r13 is not a scratch register, so it would have to be saved and restored. In fact it is the small data area pointer register and shouldn't be changed at all. Please have a look at the "SYSTEM V APPLICATION BINARY INTERFACE, PowerPC Processor Supplement". There are lots of scratch registers which can be used.

The label reference is incorrect. Being a forward reference it should be 1f, not 1b.

360	    // set CPU's fault_handler_stack_pointer
361	    stw     %r1, CPU_ENT_fault_handler_stack_pointer(%r3)
362	    // call the given function
363	    stwu    %r3, -4(%r1)

r3 doesn't need to be preserved.

377	    // fault -- return via longjmp(jumpBuffer, 1)
378	1:
379	    lwzu    %r3, 4(%r1)
380	    li      %r4, 1
381	    mflr    %r13
382	    stwu    %r13, -4(%r1)
383	    lis     %r13, longjmp@ha
384	    ori     %r13, %r13, longjmp@l
385	    mtlr    %r13
386	    blr

Again, r13 should not be used. LR does not need to be preserved -- longjmp() never returns.

387	    lwzu    %r13, 4(%r1)
388	    mtlr    %r13
389	    blr

Superfluous.

Before the fault handling can work at all cpu_ent::fault_handler* must be supported in the page fault handler.

I tested it by adding a panic("..."); in main.cpp just after TRACE("init VM").

The "DEFAULT CATCH!" output probably stems from the OF exception handler. While the stack trace should work as long as it doesn't encounter an invalid stack frame (which it does with the above code), the PPC kernel doesn't set up its exception handling until arch_int_init_post_vm(). Could probably be done earlier.

by andreasf, 9 years ago

proposed patch: add support for CPU fault handler

by andreasf, 9 years ago

proposed patch: implement arch_debug_call_with_fault_handler

by andreasf, 9 years ago

test case: panic

comment:5 by andreasf, 9 years ago

Summary: Implement arch_debug_call_with_fault_handler for ppc[PATCH] Implement arch_debug_call_with_fault_handler for ppc

Next try, appears to be working better!

exception handlers at 0x80881000
PANIC: Test!
Welcome to Kernel Debugging Land...
Running on CPU 0
Current thread pointer is 0x801c80c0, which is an address we can't read from.
stack trace for thread 0x0 ""
    kernel stack: 0x00000000 to 0x00000000
frame            caller     <image>:function + offset
80003d30 (+ 240) 80164074   <kernel_ppc>:_ZL11stack_traceiPPc + 0x0350
80003e20 (+  16) 801641ec   <kernel_ppc>:arch_debug_stack_trace + 0x0018
80003e30 (+  16) 800b7e64   <kernel_ppc>:_ZL22stack_trace_trampolinePv + 0x0010
80003e40 (+  16) 8016a14c   <kernel_ppc>:arch_debug_call_with_fault_handler + 0x002c (nearest)
80003e50 (+  48) 800b8638   <kernel_ppc>:debug_call_with_fault_handler + 0x0078
80003e80 (+ 112) 800b9b98   <kernel_ppc>:_ZL20kernel_debugger_loopPKcS0_P13__va_list_tagl + 0x01c8
80003ef0 (+  80) 800b9f5c   <kernel_ppc>:_ZL24kernel_debugger_internalPKcS0_P13__va_list_tagl + 0x01c8
80003f40 (+ 144) 800ba2e8   <kernel_ppc>:panic + 0x00ac
80003fd0 (+  32) 8008a3d0   <kernel_ppc>:_start + 0x01c4

comment:6 by bonefish, 9 years ago

Resolution: fixed
Status: newclosed

Thanks! Committed in hrev37019 through hrev37021.

Note: See TracTickets for help on using tickets.