PPC: Enable the Watchdog vector for 405
PATCH 1/1 PPC: Enable the Watchdog vector for 405
Move the body of the PIT exception out of line to make room.
--- arch/powerpc/kernel/head_40x.S | 26 +++++++++++++------------- arch/powerpc/kernel/traps.c | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-)
arch/powerpc/kernel/head_40x.S +13 -13
@@ -431,29 +431,19 @@ label: /* 0x1000 - Programmable Interval Timer (PIT) Exception */ START_EXCEPTION(0x1000, Decrementer) - NORMAL_EXCEPTION_PROLOG - lis r0,TSR_PIS@h - mtspr SPRN_TSR,r0 /* Clear the PIT exception */ - addi r3,r1,STACK_FRAME_OVERHEAD - EXC_XFER_LITE(0x1000, timer_interrupt) + b pit_longer -#if 0 /* NOTE: - * FIT and WDT handlers are not implemented yet. + * FIT handler is not implemented yet. */ /* 0x1010 - Fixed Interval Timer (FIT) Exception */ - STND_EXCEPTION(0x1010, FITException, unknown_exception) +// STND_EXCEPTION(0x1010, FITException, unknown_exception) /* 0x1020 - Watchdog Timer (WDT) Exception */ -#ifdef CONFIG_BOOKE_WDT CRITICAL_EXCEPTION(0x1020, WDTException, WatchdogException) -#else - CRITICAL_EXCEPTION(0x1020, WDTException, unknown_exception) -#endif -#endif /* 0x1100 - Data TLB Miss Exception * As the name implies, translation is not in the MMU, so search the
@@ -738,6 +728,16 @@ label: (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), \ NOCOPY, crit_transfer_to_handler, ret_from_crit_exc) + /* Programmable Interval Timer (PIT) Exception. The PIT runs into + the space reserved for other exceptions, so we branch down + to here. */ +pit_longer: + NORMAL_EXCEPTION_PROLOG + lis r0,TSR_PIS@h + mtspr SPRN_TSR,r0 /* Clear the PIT exception */ + addi r3,r1,STACK_FRAME_OVERHEAD + EXC_XFER_LITE(0x1000, timer_interrupt) + /* * The other Data TLB exceptions bail out to this point * if they can't resolve the lightweight TLB fault.
arch/powerpc/kernel/traps.c +1 -1
@@ -1514,7 +1514,7 @@ void unrecoverable_exception(struct pt_regs *regs)die ("Unrecoverable exception" , regs,SIGABRT ); } -#ifdef CONFIG_BOOKE_WDT +#if defined(CONFIG_BOOKE_WDT ) | defined(CONFIG_40x)/* * Default handler for a Watchdog exception, * spins until a reboot occurs
Signature · 1 line
1.7.4.1
Quoted 1 line · Jason Gunthorpe
Move the body of the PIT exception out of line to make room.
What boards did you test this on? What driver are you using for the watchdog?
Quoted 21 lines
Signed-off-by Jason Gunthorpe <[email protected]>--- arch/powerpc/kernel/head_40x.S | 26 +++++++++++++------------- arch/powerpc/kernel/traps.c | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-)arch/powerpc/kernel/head_40x.S +1
-5@@ -431,29 +431,19 @@ label: /* 0x1000 - Programmable Interval Timer (PIT) Exception */ START_EXCEPTION(0x1000, Decrementer) - NORMAL_EXCEPTION_PROLOG - lis r0,TSR_PIS@h - mtspr SPRN_TSR,r0 /* Clear the PIT exception */ - addi r3,r1,STACK_FRAME_OVERHEAD - EXC_XFER_LITE(0x1000, timer_interrupt) + b pit_longer
arch/powerpc/kernel/head_40x.S label: · line 442
Quoted 10 lines
arch/powerpc/kernel/head_40x.S +2
-3-#if 0 /* NOTE: - * FIT and WDT handlers are not implemented yet. + * FIT handler is not implemented yet. */ /* 0x1010 - Fixed Interval Timer (FIT) Exception */ - STND_EXCEPTION(0x1010, FITException, unknown_exception) +// STND_EXCEPTION(0x1010, FITException, unknown_exception)
Please just move the #endif for the #if 0 up instead of putting a C++ style comment here.
arch/powerpc/kernel/head_40x.S label: · line 456
Quoted 8 lines
arch/powerpc/kernel/head_40x.S +0
-5/* 0x1020 - Watchdog Timer (WDT) Exception */ -#ifdef CONFIG_BOOKE_WDT CRITICAL_EXCEPTION(0x1020, WDTException, WatchdogException) -#else - CRITICAL_EXCEPTION(0x1020, WDTException, unknown_exception) -#endif -#endif
Please leave this wrapped in CONFIG_BOOKE_WDT. I don't agree with unconditionally enabling this for every 405 chip out there.
Quoted 24 lines
arch/powerpc/kernel/head_40x.S +10
-0/* 0x1100 - Data TLB Miss Exception * As the name implies, translation is not in the MMU, so search the@@ -738,6 +728,16 @@ label: (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), \ NOCOPY, crit_transfer_to_handler, ret_from_crit_exc) + /* Programmable Interval Timer (PIT) Exception. The PIT runs into + the space reserved for other exceptions, so we branch down + to here. */ +pit_longer: + NORMAL_EXCEPTION_PROLOG + lis r0,TSR_PIS@h + mtspr SPRN_TSR,r0 /* Clear the PIT exception */ + addi r3,r1,STACK_FRAME_OVERHEAD + EXC_XFER_LITE(0x1000, timer_interrupt) + /* * The other Data TLB exceptions bail out to this point * if they can't resolve the lightweight TLB fault.arch/powerpc/kernel/traps.c +0
-0
arch/powerpc/kernel/traps.c void unrecoverable_exception(struct pt_regs *regs) · line 1517
Quoted 5 lines
arch/powerpc/kernel/traps.c +1
-1@@ -1514,7 +1514,7 @@ void unrecoverable_exception(struct pt_regs *regs)die ("Unrecoverable exception" , regs,SIGABRT ); } -#ifdef CONFIG_BOOKE_WDT +#if defined(CONFIG_BOOKE_WDT ) | defined(CONFIG_40x)
Pretty sure you meant || here? Thought if you just enable the existing config option, I don't think you'd need to edit this file at all.
josh
Quoted 6 lines · Josh Boyer
Quoted 1 line · Jason Gunthorpe
Move the body of the PIT exception out of line to make room.
What boards did you test this on? What driver are you using for the watchdog?
Tested on a 405F6 core (Xilinx's variant), the board is custom, and the control for the watchdog SPRs was bundled into a watchdog driver for the board's watchdog controller.
Quoted 7 lines
arch/powerpc/kernel/head_40x.S label: · line 442
Quoted 4 lines
/*
0x1010- Fixed Interval Timer (FIT) Exception
*/arch/powerpc/kernel/head_40x.S +1
-1- STND_EXCEPTION(0x1010, FITException, unknown_exception) +// STND_EXCEPTION(0x1010, FITException, unknown_exception)Please just move the #endif for the #if 0 up instead of putting a C++ style comment here.
Sure
Quoted 11 lines
arch/powerpc/kernel/head_40x.S label: · line 456
Quoted 8 lines
/*
0x1020- Watchdog Timer (WDT) Exception
*/arch/powerpc/kernel/head_40x.S +0
-5-#ifdef CONFIG_BOOKE_WDT CRITICAL_EXCEPTION(0x1020, WDTException, WatchdogException) -#else - CRITICAL_EXCEPTION(0x1020, WDTException, unknown_exception) -#endif -#endifPlease leave this wrapped in
CONFIG_BOOKE_WDT. I don't agree with unconditionally enabling this for every 405 chip out there.
What are you concerned with? If some core varient does not put a watchdog there, then you still get a panic from the default watchdog exception handler..
Quoted 5 lines
arch/powerpc/kernel/traps.c void unrecoverable_exception(struct pt_regs *regs) · line 1517
Quoted 2 lines
arch/powerpc/kernel/traps.c +1
-1-#ifdef CONFIG_BOOKE_WDT +#if defined(CONFIG_BOOKE_WDT ) | defined(CONFIG_40x)Pretty sure you meant || here? Thought if you just enable the existing config option, I don't think you'd need to edit this file at all.
Yes, I didn't want to use BOOKE_WDT because I have not tested that driver, nor do I want that driver included in my kernel.. I think the watchdog driver in use should be orthogonal to having the exception wired in?
Jason
Quoted 24 lines · Jason Gunthorpe
Quoted 6 lines · Josh Boyer
Quoted 1 line · Jason Gunthorpe
Move the body of the PIT exception out of line to make room.
What boards did you test this on? What driver are you using for the watchdog?
Tested on a 405F6 core (Xilinx's variant), the board is custom, and the control for the watchdog SPRs was bundled into a watchdog driver for the board's watchdog controller.
Quoted 11 lines
arch/powerpc/kernel/head_40x.S label: · line 456
Quoted 8 lines
/*
0x1020- Watchdog Timer (WDT) Exception
*/arch/powerpc/kernel/head_40x.S +0
-5-#ifdef CONFIG_BOOKE_WDT CRITICAL_EXCEPTION(0x1020, WDTException, WatchdogException) -#else - CRITICAL_EXCEPTION(0x1020, WDTException, unknown_exception) -#endif -#endifPlease leave this wrapped in
CONFIG_BOOKE_WDT. I don't agree with unconditionally enabling this for every 405 chip out there.
Quoted 3 lines
What are you concerned with? If some core varient does not put a watchdog there, then you still get a panic from the default watchdog exception handler..
I'm concerned with the fact that you've moved PIT and now enabled something that's been enabled for years. There's no need to do it like that.
Quoted 10 lines
Quoted 5 lines
arch/powerpc/kernel/traps.c void unrecoverable_exception(struct pt_regs *regs) · line 1517
Quoted 2 lines
arch/powerpc/kernel/traps.c +1
-1-#ifdef CONFIG_BOOKE_WDT +#if defined(CONFIG_BOOKE_WDT ) | defined(CONFIG_40x)Pretty sure you meant || here? Thought if you just enable the existing config option, I don't think you'd need to edit this file at all.
Yes, I didn't want to use
BOOKE_WDTbecause I have not tested that driver, nor do I want that driver included in my kernel.. I think the watchdog driver in use should be orthogonal to having the exception wired in?
And it certainly can be. Just make the driver a module and don't install it or load it. The #ifdef will still evaluate to true.
josh
Quoted 5 lines · Josh Boyer
On Mon, Oct 1, 2012 at 12:25 PM, Jason Gunthorpe
Quoted 3 lines
Quoted 2 lines
Please leave this wrapped in
CONFIG_BOOKE_WDT. I don't agree with unconditionally enabling this for every 405 chip out there.
Quoted 7 lines
Quoted 3 lines
What are you concerned with? If some core varient does not put a watchdog there, then you still get a panic from the default watchdog exception handler..
I'm concerned with the fact that you've moved PIT and now enabled something that's been enabled for years. There's no need to do it like that.
Well, just moving the ifdef still keeps the PIT change, and either the vector is never called and it is harmless to add the new entry point, or CPUs have been randomly calling into DTLBMiss for years, which seems worth discovering.
FWIW, this patch has been carried in our tree since about 2.6.14, mind you we only use two 405 varients.
Quoted 7 lines
Quoted 4 lines
Yes, I didn't want to use
BOOKE_WDTbecause I have not tested that driver, nor do I want that driver included in my kernel.. I think the watchdog driver in use should be orthogonal to having the exception wired in?And it certainly can be. Just make the driver a module and don't install it or load it. The #ifdef will still evaluate to true.
Well, we use non-modular kernels, but I can certainly patch the driver out.
If I resend using BOOKE_WDT will you take it?
Thanks,
Jason
Quoted 5 lines · Jason Gunthorpe
arch/powerpc/kernel/head_40x.S +0
-0
arch/powerpc/kernel/head_40x.S label: · line 434
Quoted 9 lines
arch/powerpc/kernel/head_40x.S +1
-5@@ -431,29 +431,19 @@ label: /* 0x1000 - Programmable Interval Timer (PIT) Exception */ START_EXCEPTION(0x1000, Decrementer) - NORMAL_EXCEPTION_PROLOG - lis r0,TSR_PIS@h - mtspr SPRN_TSR,r0 /* Clear the PIT exception */ - addi r3,r1,STACK_FRAME_OVERHEAD - EXC_XFER_LITE(0x1000, timer_interrupt) + b pit_longer
Looks like you indeed have no choice but move it down, though I dislike the label name :-)
Look at how we do a similar thing in exceptions-64.S, we basically just don't use START_EXCEPTION at that location. We put a .=xxxx and a branch, and use the real exception name at the target label.
Or just name it "pit_exception" if you want to keep things simple. I just don't like "pit_longer" :-)
arch/powerpc/kernel/head_40x.S label: · line 438
Quoted 5 lines
arch/powerpc/kernel/head_40x.S +1
-2-#if 0 /* NOTE: - * FIT and WDT handlers are not implemented yet. + * FIT handler is not implemented yet. */
Any reason to comment that out ? Better off also branching out of line to a stub similar to the PIT one that then calls unknown_exception. That way if it triggers by accident, you'll get a clean trace.
arch/powerpc/kernel/head_40x.S label: · line 456
Quoted 13 lines
arch/powerpc/kernel/head_40x.S +1
-6/* 0x1010 - Fixed Interval Timer (FIT) Exception */ - STND_EXCEPTION(0x1010, FITException, unknown_exception) +// STND_EXCEPTION(0x1010, FITException, unknown_exception) /* 0x1020 - Watchdog Timer (WDT) Exception */ -#ifdef CONFIG_BOOKE_WDT CRITICAL_EXCEPTION(0x1020, WDTException, WatchdogException) -#else - CRITICAL_EXCEPTION(0x1020, WDTException, unknown_exception) -#endif -#endif
Move it out of line too please. When a given vector "slot" gets crowded, I prefer moving everything in it out of line to keep things consistent.
Quoted 24 lines
arch/powerpc/kernel/head_40x.S +10
-0/* 0x1100 - Data TLB Miss Exception * As the name implies, translation is not in the MMU, so search the@@ -738,6 +728,16 @@ label: (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), \ NOCOPY, crit_transfer_to_handler, ret_from_crit_exc) + /* Programmable Interval Timer (PIT) Exception. The PIT runs into + the space reserved for other exceptions, so we branch down + to here. */ +pit_longer: + NORMAL_EXCEPTION_PROLOG + lis r0,TSR_PIS@h + mtspr SPRN_TSR,r0 /* Clear the PIT exception */ + addi r3,r1,STACK_FRAME_OVERHEAD + EXC_XFER_LITE(0x1000, timer_interrupt) + /* * The other Data TLB exceptions bail out to this point * if they can't resolve the lightweight TLB fault.arch/powerpc/kernel/traps.c +0
-0
arch/powerpc/kernel/traps.c void unrecoverable_exception(struct pt_regs *regs) · line 1520
Quoted 8 lines
arch/powerpc/kernel/traps.c +1
-1@@ -1514,7 +1514,7 @@ void unrecoverable_exception(struct pt_regs *regs)die ("Unrecoverable exception" , regs,SIGABRT ); } -#ifdef CONFIG_BOOKE_WDT +#if defined(CONFIG_BOOKE_WDT ) | defined(CONFIG_40x)/* * Default handler for a Watchdog exception, * spins until a reboot occurs
Cheers,
Ben.
The watchdog and FIT code has been #if 0'd for ever, if the CPU takes an exception to either of those vectors it will jump into the middle of the PIT or Data TLB code and surely crash.
At least some (all?) 405 cores have both the WDT and FIT vectors defined, so lets have proper entry points for them.
Tested that the WDT vector works on a 405F6 core.
--- arch/powerpc/kernel/head_40x.S | 47 ++++++++++++++++++++++++--------------- arch/powerpc/kernel/traps.c | 2 +- 2 files changed, 30 insertions(+), 19 deletions(-)
Changes in v2:
- Move all three exception vectors out of line with an explicit branch like exceptions-64.S (Benjamin)
- Fix the #if .. | .. typo to be || (Josh)
- Include the stub for FITException, and figured out what the very obsolete STDN_EXCEPTION might have done. (Benjamin)
- I kept the WatchdogException trap unconditional, but this would be trivial to change, per Josh's comments
The disadvantage to moving the WDT out of line is that the CRITICAL_EXCEPTION macro has to be expanded..
Thanks everyone
Action shot:
PowerPC Book-E Watchdog Exception
Oops: Watchdog Timeout NIP: c0006b54 LR: c0006ad4 CTR: c003c800 REGS: c3ffbf50 TRAP: 1022 Not tainted (3.6.0-00040-ga9db8b3) MSR: 00029030 <EE,ME,IR,DR> CR: 24002482 XER: 00000007
TASK = c03c32b0[0] 'swapper' THREAD: c03e0000
GPR00: 00000008 c03e1f90 c03c32b0 00000000 c1872f00 00000001 00000000 00000000
GPR08: 01000000 c03e73c0 00000000 00000000 22000482 10152d00 7ffe7b60 7ffe7b70
GPR16: 7ffe7be0 7ffe7d20 100f7560 7ffe7d9c 7ffe7db4 7ffe7d30 7ffe7dcc 7ffe7dd0
GPR24: 00000000 00000000 00000000 c049e680 c03e0000 c03e003c 00000008 c03e0000 NIP [c0006b54] cpu_idle+0xd0/0xe8
LR [c0006ad4] cpu_idle+0x50/0xe8
Call Trace: [c03e1f90] [c0006ad4] cpu_idle+0x50/0xe8 (unreliable) [c03e1fb0] [c0002990] rest_init+0x64/0x74 [c03e1fc0] [c0296774] start_kernel+0x2cc/0x2d4 [c03e1ff0] [c0002440] start_here+0x34/0x84 Instruction dump: 40820010 7c0000a6 60008000 7c000124 7c00e828 7c00f378 7c00e92d 40a2fff4 4800000c 7c210b78 7ffffb78 801f003c <700b0004> 4182ff80 7c421378 48048585 Kernel panic - not syncing: Watchdog Timeout Call Trace:
Rebooting in 180 seconds..
arch/powerpc/kernel/head_40x.S +29 -18
@@ -430,30 +430,18 @@ label: EXCEPTION(0x0F00, Trap_0F, unknown_exception, EXC_XFER_EE) /* 0x1000 - Programmable Interval Timer (PIT) Exception */ - START_EXCEPTION(0x1000, Decrementer) - NORMAL_EXCEPTION_PROLOG - lis r0,TSR_PIS@h - mtspr SPRN_TSR,r0 /* Clear the PIT exception */ - addi r3,r1,STACK_FRAME_OVERHEAD - EXC_XFER_LITE(0x1000, timer_interrupt) - -#if 0 -/* NOTE: - * FIT and WDT handlers are not implemented yet. - */ + . = 0x1000 + b Decrementer /* 0x1010 - Fixed Interval Timer (FIT) Exception */ - STND_EXCEPTION(0x1010, FITException, unknown_exception) + . = 0x1010 + b FITException /* 0x1020 - Watchdog Timer (WDT) Exception */ -#ifdef CONFIG_BOOKE_WDT - CRITICAL_EXCEPTION(0x1020, WDTException, WatchdogException) -#else - CRITICAL_EXCEPTION(0x1020, WDTException, unknown_exception) -#endif -#endif + . = 0x1020 + b WDTException /* 0x1100 - Data TLB Miss Exception * As the name implies, translation is not in the MMU, so search the
@@ -738,6 +726,29 @@ label: (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), \ NOCOPY, crit_transfer_to_handler, ret_from_crit_exc) + /* Programmable Interval Timer (PIT) Exception. (from 0x1000) */ +Decrementer: + NORMAL_EXCEPTION_PROLOG + lis r0,TSR_PIS@h + mtspr SPRN_TSR,r0 /* Clear the PIT exception */ + addi r3,r1,STACK_FRAME_OVERHEAD + EXC_XFER_LITE(0x1000, timer_interrupt) + + /* Fixed Interval Timer (FIT) Exception. (from 0x1010) */ +FITException: + NORMAL_EXCEPTION_PROLOG + addi r3,r1,STACK_FRAME_OVERHEAD; + EXC_XFER_EE(0x1010, unknown_exception) + + /* Watchdog Timer (WDT) Exception. (from 0x1020) */ +WDTException: + CRITICAL_EXCEPTION_PROLOG; + addi r3,r1,STACK_FRAME_OVERHEAD; + EXC_XFER_TEMPLATE(WatchdogException, 0x1020+2, + (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), + NOCOPY, crit_transfer_to_handler, + ret_from_crit_exc) + /* * The other Data TLB exceptions bail out to this point * if they can't resolve the lightweight TLB fault.
arch/powerpc/kernel/traps.c +1 -1
@@ -1514,7 +1514,7 @@ void unrecoverable_exception(struct pt_regs *regs)die ("Unrecoverable exception" , regs,SIGABRT ); } -#ifdef CONFIG_BOOKE_WDT +#if defined(CONFIG_BOOKE_WDT ) || defined(CONFIG_40x)/* * Default handler for a Watchdog exception, * spins until a reboot occurs
Signature · 1 line
1.7.5.4