[PATCH 1/4] powerpc: Revert show_regs() define for readability

[PATCH 1/4] powerpc: Revert show_regs() define for readability

[Kumar Gala]

We had an existing ifdef for 4xx & BOOKE processors that got changed to
CONFIG_PPC_ADV_DEBUG_REGS.  The define has nothing to do with
CONFIG_PPC_ADV_DEBUG_REGS.  The define really should be:

 #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)

and not

 #ifdef CONFIG_PPC_ADV_DEBUG_REGS

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/kernel/process.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 8f53954..a1b5981 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -657,7 +657,7 @@ void show_regs(struct pt_regs * regs)
 	if ((regs->trap != 0xc00) && cpu_has_feature(CPU_FTR_CFAR))
 		printk("CFAR: "REG"\n", regs->orig_gpr3);
 	if (trap == 0x300 || trap == 0x600)
-#ifdef CONFIG_PPC_ADV_DEBUG_REGS
+#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
 		printk("DEAR: "REG", ESR: "REG"\n", regs->dar, regs->dsisr);
 #else
 		printk("DAR: "REG", DSISR: %08lx\n", regs->dar, regs->dsisr);
-- 
1.7.3.4

[PATCH 2/4] powerpc: Remove extraneous CONFIG_PPC_ADV_DEBUG_REGS define

[Kumar Gala]

All of DebugException is already protected by CONFIG_PPC_ADV_DEBUG_REGS
there is no need to have another such ifdef inside the function.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/kernel/traps.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index f19d977..db733d3 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1291,14 +1291,12 @@ void __kprobes DebugException(struct pt_regs *regs, unsigned long debug_status)
 
 		if (user_mode(regs)) {
 			current->thread.dbcr0 &= ~DBCR0_IC;
-#ifdef CONFIG_PPC_ADV_DEBUG_REGS
 			if (DBCR_ACTIVE_EVENTS(current->thread.dbcr0,
 					       current->thread.dbcr1))
 				regs->msr |= MSR_DE;
 			else
 				/* Make sure the IDM bit is off */
 				current->thread.dbcr0 &= ~DBCR0_IDM;
-#endif
 		}
 
 		_exception(SIGTRAP, regs, TRAP_TRACE, regs->nip);
-- 
1.7.3.4

[PATCH 3/4] powerpc/book3e-64: Fix debug support for userspace

[Kumar Gala]

With the introduction of CONFIG_PPC_ADV_DEBUG_REGS user space debug is
broken on Book-E 64-bit parts that support delayed debug events.  When
switch_booke_debug_regs() sets DBCR0 we'll start getting debug events as
MSR_DE is also set and we aren't able to handle debug events from kernel
space.

We can remove the hack that always enables MSR_DE and loads up DBCR0 and
just utilize switch_booke_debug_regs() to get user space debug working
again.

We still need to handle critical/debug exception stacks & proper
save/restore of state for those exception levles to support debug events
from kernel space like we have on 32-bit.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/include/asm/reg_booke.h |    2 +-
 arch/powerpc/kernel/process.c        |   22 ----------------------
 2 files changed, 1 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index 9ec0b39..4dfa21c 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -31,7 +31,7 @@
 
 #define MSR_		MSR_ME | MSR_CE
 #define MSR_KERNEL	MSR_ | MSR_64BIT
-#define MSR_USER32	MSR_ | MSR_PR | MSR_EE | MSR_DE
+#define MSR_USER32	MSR_ | MSR_PR | MSR_EE
 #define MSR_USER64	MSR_USER32 | MSR_64BIT
 #elif defined (CONFIG_40x)
 #define MSR_KERNEL	(MSR_ME|MSR_RI|MSR_IR|MSR_DR|MSR_CE)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index a1b5981..269a309 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -486,28 +486,6 @@ struct task_struct *__switch_to(struct task_struct *prev,
 	new_thread = &new->thread;
 	old_thread = &current->thread;
 
-#if defined(CONFIG_PPC_BOOK3E_64)
-	/* XXX Current Book3E code doesn't deal with kernel side DBCR0,
-	 * we always hold the user values, so we set it now.
-	 *
-	 * However, we ensure the kernel MSR:DE is appropriately cleared too
-	 * to avoid spurrious single step exceptions in the kernel.
-	 *
-	 * This will have to change to merge with the ppc32 code at some point,
-	 * but I don't like much what ppc32 is doing today so there's some
-	 * thinking needed there
-	 */
-	if ((new_thread->dbcr0 | old_thread->dbcr0) & DBCR0_IDM) {
-		u32 dbcr0;
-
-		mtmsr(mfmsr() & ~MSR_DE);
-		isync();
-		dbcr0 = mfspr(SPRN_DBCR0);
-		dbcr0 = (dbcr0 & DBCR0_EDM) | new_thread->dbcr0;
-		mtspr(SPRN_DBCR0, dbcr0);
-	}
-#endif /* CONFIG_PPC64_BOOK3E */
-
 #ifdef CONFIG_PPC64
 	/*
 	 * Collect processor utilization data per process
-- 
1.7.3.4

[PATCH 4/4] powerpc/booke: Re-organize debug code

[Kumar Gala]

* set_dabr/do_dabr are no longer used when CNFIG_PPC_ADV_DEBUG_REGS is set
  refactor code a bit such that we only build the dabr code for
  !CONFIG_PPC_ADV_DEBUG_REGS and removed some CONFIG_PPC_ADV_DEBUG_REGS
  code in set_dabr that would never get built.

* Move do_send_trap into traps.c as its only used there

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/include/asm/system.h |    5 +--
 arch/powerpc/kernel/process.c     |   97 +++++++++++++-----------------------
 arch/powerpc/kernel/traps.c       |   17 +++++++
 3 files changed, 53 insertions(+), 66 deletions(-)

diff --git a/arch/powerpc/include/asm/system.h b/arch/powerpc/include/asm/system.h
index e30a13d..1dc5d9c 100644
--- a/arch/powerpc/include/asm/system.h
+++ b/arch/powerpc/include/asm/system.h
@@ -111,11 +111,8 @@ static inline int debugger_dabr_match(struct pt_regs *regs) { return 0; }
 static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
 #endif
 
+#ifndef CONFIG_PPC_ADV_DEBUG_REGS
 extern int set_dabr(unsigned long dabr);
-#ifdef CONFIG_PPC_ADV_DEBUG_REGS
-extern void do_send_trap(struct pt_regs *regs, unsigned long address,
-			 unsigned long error_code, int signal_code, int brkpt);
-#else
 extern void do_dabr(struct pt_regs *regs, unsigned long address,
 		    unsigned long error_code);
 #endif
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 269a309..989e574 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -251,50 +251,6 @@ void discard_lazy_cpu_state(void)
 #endif /* CONFIG_SMP */
 
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
-void do_send_trap(struct pt_regs *regs, unsigned long address,
-		  unsigned long error_code, int signal_code, int breakpt)
-{
-	siginfo_t info;
-
-	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
-			11, SIGSEGV) == NOTIFY_STOP)
-		return;
-
-	/* Deliver the signal to userspace */
-	info.si_signo = SIGTRAP;
-	info.si_errno = breakpt;	/* breakpoint or watchpoint id */
-	info.si_code = signal_code;
-	info.si_addr = (void __user *)address;
-	force_sig_info(SIGTRAP, &info, current);
-}
-#else	/* !CONFIG_PPC_ADV_DEBUG_REGS */
-void do_dabr(struct pt_regs *regs, unsigned long address,
-		    unsigned long error_code)
-{
-	siginfo_t info;
-
-	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
-			11, SIGSEGV) == NOTIFY_STOP)
-		return;
-
-	if (debugger_dabr_match(regs))
-		return;
-
-	/* Clear the DABR */
-	set_dabr(0);
-
-	/* Deliver the signal to userspace */
-	info.si_signo = SIGTRAP;
-	info.si_errno = 0;
-	info.si_code = TRAP_HWBKPT;
-	info.si_addr = (void __user *)address;
-	force_sig_info(SIGTRAP, &info, current);
-}
-#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
-
-static DEFINE_PER_CPU(unsigned long, current_dabr);
-
-#ifdef CONFIG_PPC_ADV_DEBUG_REGS
 /*
  * Set the debug registers back to their default "safe" values.
  */
@@ -357,16 +313,7 @@ static void switch_booke_debug_regs(struct thread_struct *new_thread)
 			prime_debug_regs(new_thread);
 }
 #else	/* !CONFIG_PPC_ADV_DEBUG_REGS */
-#ifndef CONFIG_HAVE_HW_BREAKPOINT
-static void set_debug_reg_defaults(struct thread_struct *thread)
-{
-	if (thread->dabr) {
-		thread->dabr = 0;
-		set_dabr(0);
-	}
-}
-#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
-#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
+static DEFINE_PER_CPU(unsigned long, current_dabr);
 
 int set_dabr(unsigned long dabr)
 {
@@ -376,19 +323,45 @@ int set_dabr(unsigned long dabr)
 		return ppc_md.set_dabr(dabr);
 
 	/* XXX should we have a CPU_FTR_HAS_DABR ? */
-#ifdef CONFIG_PPC_ADV_DEBUG_REGS
-	mtspr(SPRN_DAC1, dabr);
-#ifdef CONFIG_PPC_47x
-	isync();
-#endif
-#elif defined(CONFIG_PPC_BOOK3S)
 	mtspr(SPRN_DABR, dabr);
-#endif
-
 
 	return 0;
 }
 
+void do_dabr(struct pt_regs *regs, unsigned long address,
+		    unsigned long error_code)
+{
+	siginfo_t info;
+
+	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
+			11, SIGSEGV) == NOTIFY_STOP)
+		return;
+
+	if (debugger_dabr_match(regs))
+		return;
+
+	/* Clear the DABR */
+	set_dabr(0);
+
+	/* Deliver the signal to userspace */
+	info.si_signo = SIGTRAP;
+	info.si_errno = 0;
+	info.si_code = TRAP_HWBKPT;
+	info.si_addr = (void __user *)address;
+	force_sig_info(SIGTRAP, &info, current);
+}
+
+#ifndef CONFIG_HAVE_HW_BREAKPOINT
+static void set_debug_reg_defaults(struct thread_struct *thread)
+{
+	if (thread->dabr) {
+		thread->dabr = 0;
+		set_dabr(0);
+	}
+}
+#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
+#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
+
 #ifdef CONFIG_PPC64
 DEFINE_PER_CPU(struct cpu_usage, cpu_usage_array);
 #endif
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index db733d3..edc1108 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1184,6 +1184,23 @@ void SoftwareEmulation(struct pt_regs *regs)
 #endif /* CONFIG_8xx */
 
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
+static void do_send_trap(struct pt_regs *regs, unsigned long address,
+		  unsigned long error_code, int signal_code, int breakpt)
+{
+	siginfo_t info;
+
+	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
+			11, SIGSEGV) == NOTIFY_STOP)
+		return;
+
+	/* Deliver the signal to userspace */
+	info.si_signo = SIGTRAP;
+	info.si_errno = breakpt;	/* breakpoint or watchpoint id */
+	info.si_code = signal_code;
+	info.si_addr = (void __user *)address;
+	force_sig_info(SIGTRAP, &info, current);
+}
+
 static void handle_debug(struct pt_regs *regs, unsigned long debug_status)
 {
 	int changed = 0;
-- 
1.7.3.4

Re: [4/4] powerpc/booke: Re-organize debug code

[Jimi Xenidis]

On Oct 5, 2011, at 9:53 PM, Kumar Gala wrote:

> * set_dabr/do_dabr are no longer used when CNFIG_PPC_ADV_DEBUG_REGS is =
set
>  refactor code a bit such that we only build the dabr code for
>  !CONFIG_PPC_ADV_DEBUG_REGS and removed some CONFIG_PPC_ADV_DEBUG_REGS
>  code in set_dabr that would never get built.
>=20
> * Move do_send_trap into traps.c as its only used there
>=20
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>=20
> ---
> arch/powerpc/include/asm/system.h |    5 +--
> arch/powerpc/kernel/process.c     |   97 =
+++++++++++++-----------------------
> arch/powerpc/kernel/traps.c       |   17 +++++++
> 3 files changed, 53 insertions(+), 66 deletions(-)
>=20
> diff --git a/arch/powerpc/include/asm/system.h =
b/arch/powerpc/include/asm/system.h
> index e30a13d..1dc5d9c 100644
> --- a/arch/powerpc/include/asm/system.h
> +++ b/arch/powerpc/include/asm/system.h
> @@ -111,11 +111,8 @@ static inline int debugger_dabr_match(struct =
pt_regs *regs) { return 0; }
> static inline int debugger_fault_handler(struct pt_regs *regs) { =
return 0; }
> #endif
>=20
> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
> extern int set_dabr(unsigned long dabr);
> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
> -extern void do_send_trap(struct pt_regs *regs, unsigned long address,
> -			 unsigned long error_code, int signal_code, int =
brkpt);
> -#else


This part of the patch breaks xmon.c
Naively I simply wrapped the xmon call:

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index f08836a..b5911b2 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -738,8 +738,10 @@ static void insert_bpts(void)
=20
 static void insert_cpu_bpts(void)
 {
+#ifndef CONFIG_PPC_ADV_DEBUG_REGS
 	if (dabr.enabled)
 		set_dabr(dabr.address | (dabr.enabled & 7));
+#endif
 	if (iabr && cpu_has_feature(CPU_FTR_IABR))
 		mtspr(SPRN_IABR, iabr->address
 			 | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
@@ -767,7 +769,9 @@ static void remove_bpts(void)
=20
 static void remove_cpu_bpts(void)
 {
+#ifndef CONFIG_PPC_ADV_DEBUG_REGS
 	set_dabr(0);
+#endif
 	if (cpu_has_feature(CPU_FTR_IABR))
 		mtspr(SPRN_IABR, 0);
 }

-JX


> extern void do_dabr(struct pt_regs *regs, unsigned long address,
> 		    unsigned long error_code);
> #endif
> diff --git a/arch/powerpc/kernel/process.c =
b/arch/powerpc/kernel/process.c
> index 269a309..989e574 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -251,50 +251,6 @@ void discard_lazy_cpu_state(void)
> #endif /* CONFIG_SMP */
>=20
> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> -void do_send_trap(struct pt_regs *regs, unsigned long address,
> -		  unsigned long error_code, int signal_code, int =
breakpt)
> -{
> -	siginfo_t info;
> -
> -	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
> -			11, SIGSEGV) =3D=3D NOTIFY_STOP)
> -		return;
> -
> -	/* Deliver the signal to userspace */
> -	info.si_signo =3D SIGTRAP;
> -	info.si_errno =3D breakpt;	/* breakpoint or watchpoint id =
*/
> -	info.si_code =3D signal_code;
> -	info.si_addr =3D (void __user *)address;
> -	force_sig_info(SIGTRAP, &info, current);
> -}
> -#else	/* !CONFIG_PPC_ADV_DEBUG_REGS */
> -void do_dabr(struct pt_regs *regs, unsigned long address,
> -		    unsigned long error_code)
> -{
> -	siginfo_t info;
> -
> -	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
> -			11, SIGSEGV) =3D=3D NOTIFY_STOP)
> -		return;
> -
> -	if (debugger_dabr_match(regs))
> -		return;
> -
> -	/* Clear the DABR */
> -	set_dabr(0);
> -
> -	/* Deliver the signal to userspace */
> -	info.si_signo =3D SIGTRAP;
> -	info.si_errno =3D 0;
> -	info.si_code =3D TRAP_HWBKPT;
> -	info.si_addr =3D (void __user *)address;
> -	force_sig_info(SIGTRAP, &info, current);
> -}
> -#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
> -
> -static DEFINE_PER_CPU(unsigned long, current_dabr);
> -
> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
> /*
>  * Set the debug registers back to their default "safe" values.
>  */
> @@ -357,16 +313,7 @@ static void switch_booke_debug_regs(struct =
thread_struct *new_thread)
> 			prime_debug_regs(new_thread);
> }
> #else	/* !CONFIG_PPC_ADV_DEBUG_REGS */
> -#ifndef CONFIG_HAVE_HW_BREAKPOINT
> -static void set_debug_reg_defaults(struct thread_struct *thread)
> -{
> -	if (thread->dabr) {
> -		thread->dabr =3D 0;
> -		set_dabr(0);
> -	}
> -}
> -#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
> -#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
> +static DEFINE_PER_CPU(unsigned long, current_dabr);
>=20
> int set_dabr(unsigned long dabr)
> {
> @@ -376,19 +323,45 @@ int set_dabr(unsigned long dabr)
> 		return ppc_md.set_dabr(dabr);
>=20
> 	/* XXX should we have a CPU_FTR_HAS_DABR ? */
> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
> -	mtspr(SPRN_DAC1, dabr);
> -#ifdef CONFIG_PPC_47x
> -	isync();
> -#endif
> -#elif defined(CONFIG_PPC_BOOK3S)
> 	mtspr(SPRN_DABR, dabr);
> -#endif
> -
>=20
> 	return 0;
> }
>=20
> +void do_dabr(struct pt_regs *regs, unsigned long address,
> +		    unsigned long error_code)
> +{
> +	siginfo_t info;
> +
> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
> +		return;
> +
> +	if (debugger_dabr_match(regs))
> +		return;
> +
> +	/* Clear the DABR */
> +	set_dabr(0);
> +
> +	/* Deliver the signal to userspace */
> +	info.si_signo =3D SIGTRAP;
> +	info.si_errno =3D 0;
> +	info.si_code =3D TRAP_HWBKPT;
> +	info.si_addr =3D (void __user *)address;
> +	force_sig_info(SIGTRAP, &info, current);
> +}
> +
> +#ifndef CONFIG_HAVE_HW_BREAKPOINT
> +static void set_debug_reg_defaults(struct thread_struct *thread)
> +{
> +	if (thread->dabr) {
> +		thread->dabr =3D 0;
> +		set_dabr(0);
> +	}
> +}
> +#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
> +#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
> +
> #ifdef CONFIG_PPC64
> DEFINE_PER_CPU(struct cpu_usage, cpu_usage_array);
> #endif
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index db733d3..edc1108 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1184,6 +1184,23 @@ void SoftwareEmulation(struct pt_regs *regs)
> #endif /* CONFIG_8xx */
>=20
> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> +static void do_send_trap(struct pt_regs *regs, unsigned long address,
> +		  unsigned long error_code, int signal_code, int =
breakpt)
> +{
> +	siginfo_t info;
> +
> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
> +		return;
> +
> +	/* Deliver the signal to userspace */
> +	info.si_signo =3D SIGTRAP;
> +	info.si_errno =3D breakpt;	/* breakpoint or watchpoint id =
*/
> +	info.si_code =3D signal_code;
> +	info.si_addr =3D (void __user *)address;
> +	force_sig_info(SIGTRAP, &info, current);
> +}
> +
> static void handle_debug(struct pt_regs *regs, unsigned long =
debug_status)
> {
> 	int changed =3D 0;

Re: [1/4] powerpc: Revert show_regs() define for readability

[Jimi Xenidis]

On Oct 5, 2011, at 9:53 PM, Kumar Gala wrote:

> We had an existing ifdef for 4xx & BOOKE processors that got changed to
> CONFIG_PPC_ADV_DEBUG_REGS.  The define has nothing to do with
> CONFIG_PPC_ADV_DEBUG_REGS.  The define really should be:
> 
> #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
> 
> and not
> 
> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> 
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> 
> ---
> arch/powerpc/kernel/process.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 8f53954..a1b5981 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -657,7 +657,7 @@ void show_regs(struct pt_regs * regs)
> 	if ((regs->trap != 0xc00) && cpu_has_feature(CPU_FTR_CFAR))
> 		printk("CFAR: "REG"\n", regs->orig_gpr3);
> 	if (trap == 0x300 || trap == 0x600)
> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
> +#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
> 		printk("DEAR: "REG", ESR: "REG"\n", regs->dar, regs->dsisr);

I'll be needing "|| defined(CONFIG_PPC_BOOK3E)" added to this please.
-jx


> #else
> 		printk("DAR: "REG", DSISR: %08lx\n", regs->dar, regs->dsisr);

Re: [1/4] powerpc: Revert show_regs() define for readability

[Kumar Gala]

On Oct 28, 2011, at 2:40 PM, Jimi Xenidis wrote:

> 
> On Oct 5, 2011, at 9:53 PM, Kumar Gala wrote:
> 
>> We had an existing ifdef for 4xx & BOOKE processors that got changed to
>> CONFIG_PPC_ADV_DEBUG_REGS.  The define has nothing to do with
>> CONFIG_PPC_ADV_DEBUG_REGS.  The define really should be:
>> 
>> #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
>> 
>> and not
>> 
>> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>> 
>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>> 
>> ---
>> arch/powerpc/kernel/process.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>> index 8f53954..a1b5981 100644
>> --- a/arch/powerpc/kernel/process.c
>> +++ b/arch/powerpc/kernel/process.c
>> @@ -657,7 +657,7 @@ void show_regs(struct pt_regs * regs)
>> 	if ((regs->trap != 0xc00) && cpu_has_feature(CPU_FTR_CFAR))
>> 		printk("CFAR: "REG"\n", regs->orig_gpr3);
>> 	if (trap == 0x300 || trap == 0x600)
>> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
>> +#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
>> 		printk("DEAR: "REG", ESR: "REG"\n", regs->dar, regs->dsisr);
> 
> I'll be needing "|| defined(CONFIG_PPC_BOOK3E)" added to this please.
> -jx

Under what platform is CONFIG_PPC_BOOK3E set and CONFIG_BOOKE is not?

- k

Re: [4/4] powerpc/booke: Re-organize debug code

[Kumar Gala]

On Oct 28, 2011, at 2:37 PM, Jimi Xenidis wrote:

>=20
> On Oct 5, 2011, at 9:53 PM, Kumar Gala wrote:
>=20
>> * set_dabr/do_dabr are no longer used when CNFIG_PPC_ADV_DEBUG_REGS =
is set
>> refactor code a bit such that we only build the dabr code for
>> !CONFIG_PPC_ADV_DEBUG_REGS and removed some CONFIG_PPC_ADV_DEBUG_REGS
>> code in set_dabr that would never get built.
>>=20
>> * Move do_send_trap into traps.c as its only used there
>>=20
>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>>=20
>> ---
>> arch/powerpc/include/asm/system.h |    5 +--
>> arch/powerpc/kernel/process.c     |   97 =
+++++++++++++-----------------------
>> arch/powerpc/kernel/traps.c       |   17 +++++++
>> 3 files changed, 53 insertions(+), 66 deletions(-)
>>=20
>> diff --git a/arch/powerpc/include/asm/system.h =
b/arch/powerpc/include/asm/system.h
>> index e30a13d..1dc5d9c 100644
>> --- a/arch/powerpc/include/asm/system.h
>> +++ b/arch/powerpc/include/asm/system.h
>> @@ -111,11 +111,8 @@ static inline int debugger_dabr_match(struct =
pt_regs *regs) { return 0; }
>> static inline int debugger_fault_handler(struct pt_regs *regs) { =
return 0; }
>> #endif
>>=20
>> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
>> extern int set_dabr(unsigned long dabr);
>> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
>> -extern void do_send_trap(struct pt_regs *regs, unsigned long =
address,
>> -			 unsigned long error_code, int signal_code, int =
brkpt);
>> -#else
>=20
>=20
> This part of the patch breaks xmon.c
> Naively I simply wrapped the xmon call:
>=20
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index f08836a..b5911b2 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -738,8 +738,10 @@ static void insert_bpts(void)
>=20
> static void insert_cpu_bpts(void)
> {
> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
> 	if (dabr.enabled)
> 		set_dabr(dabr.address | (dabr.enabled & 7));
> +#endif
> 	if (iabr && cpu_has_feature(CPU_FTR_IABR))
> 		mtspr(SPRN_IABR, iabr->address
> 			 | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
> @@ -767,7 +769,9 @@ static void remove_bpts(void)
>=20
> static void remove_cpu_bpts(void)
> {
> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
> 	set_dabr(0);
> +#endif
> 	if (cpu_has_feature(CPU_FTR_IABR))
> 		mtspr(SPRN_IABR, 0);
> }

Shouldn't all of these functions be #ifndef'd out as we don't support =
cpu_bpts on book-e parts in xmon code today?

>=20
> -JX
>=20
>=20
>> extern void do_dabr(struct pt_regs *regs, unsigned long address,
>> 		    unsigned long error_code);
>> #endif
>> diff --git a/arch/powerpc/kernel/process.c =
b/arch/powerpc/kernel/process.c
>> index 269a309..989e574 100644
>> --- a/arch/powerpc/kernel/process.c
>> +++ b/arch/powerpc/kernel/process.c
>> @@ -251,50 +251,6 @@ void discard_lazy_cpu_state(void)
>> #endif /* CONFIG_SMP */
>>=20
>> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>> -void do_send_trap(struct pt_regs *regs, unsigned long address,
>> -		  unsigned long error_code, int signal_code, int =
breakpt)
>> -{
>> -	siginfo_t info;
>> -
>> -	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>> -			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>> -		return;
>> -
>> -	/* Deliver the signal to userspace */
>> -	info.si_signo =3D SIGTRAP;
>> -	info.si_errno =3D breakpt;	/* breakpoint or watchpoint id =
*/
>> -	info.si_code =3D signal_code;
>> -	info.si_addr =3D (void __user *)address;
>> -	force_sig_info(SIGTRAP, &info, current);
>> -}
>> -#else	/* !CONFIG_PPC_ADV_DEBUG_REGS */
>> -void do_dabr(struct pt_regs *regs, unsigned long address,
>> -		    unsigned long error_code)
>> -{
>> -	siginfo_t info;
>> -
>> -	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>> -			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>> -		return;
>> -
>> -	if (debugger_dabr_match(regs))
>> -		return;
>> -
>> -	/* Clear the DABR */
>> -	set_dabr(0);
>> -
>> -	/* Deliver the signal to userspace */
>> -	info.si_signo =3D SIGTRAP;
>> -	info.si_errno =3D 0;
>> -	info.si_code =3D TRAP_HWBKPT;
>> -	info.si_addr =3D (void __user *)address;
>> -	force_sig_info(SIGTRAP, &info, current);
>> -}
>> -#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
>> -
>> -static DEFINE_PER_CPU(unsigned long, current_dabr);
>> -
>> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
>> /*
>> * Set the debug registers back to their default "safe" values.
>> */
>> @@ -357,16 +313,7 @@ static void switch_booke_debug_regs(struct =
thread_struct *new_thread)
>> 			prime_debug_regs(new_thread);
>> }
>> #else	/* !CONFIG_PPC_ADV_DEBUG_REGS */
>> -#ifndef CONFIG_HAVE_HW_BREAKPOINT
>> -static void set_debug_reg_defaults(struct thread_struct *thread)
>> -{
>> -	if (thread->dabr) {
>> -		thread->dabr =3D 0;
>> -		set_dabr(0);
>> -	}
>> -}
>> -#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
>> -#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
>> +static DEFINE_PER_CPU(unsigned long, current_dabr);
>>=20
>> int set_dabr(unsigned long dabr)
>> {
>> @@ -376,19 +323,45 @@ int set_dabr(unsigned long dabr)
>> 		return ppc_md.set_dabr(dabr);
>>=20
>> 	/* XXX should we have a CPU_FTR_HAS_DABR ? */
>> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
>> -	mtspr(SPRN_DAC1, dabr);
>> -#ifdef CONFIG_PPC_47x
>> -	isync();
>> -#endif
>> -#elif defined(CONFIG_PPC_BOOK3S)
>> 	mtspr(SPRN_DABR, dabr);
>> -#endif
>> -
>>=20
>> 	return 0;
>> }
>>=20
>> +void do_dabr(struct pt_regs *regs, unsigned long address,
>> +		    unsigned long error_code)
>> +{
>> +	siginfo_t info;
>> +
>> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>> +		return;
>> +
>> +	if (debugger_dabr_match(regs))
>> +		return;
>> +
>> +	/* Clear the DABR */
>> +	set_dabr(0);
>> +
>> +	/* Deliver the signal to userspace */
>> +	info.si_signo =3D SIGTRAP;
>> +	info.si_errno =3D 0;
>> +	info.si_code =3D TRAP_HWBKPT;
>> +	info.si_addr =3D (void __user *)address;
>> +	force_sig_info(SIGTRAP, &info, current);
>> +}
>> +
>> +#ifndef CONFIG_HAVE_HW_BREAKPOINT
>> +static void set_debug_reg_defaults(struct thread_struct *thread)
>> +{
>> +	if (thread->dabr) {
>> +		thread->dabr =3D 0;
>> +		set_dabr(0);
>> +	}
>> +}
>> +#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
>> +#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
>> +
>> #ifdef CONFIG_PPC64
>> DEFINE_PER_CPU(struct cpu_usage, cpu_usage_array);
>> #endif
>> diff --git a/arch/powerpc/kernel/traps.c =
b/arch/powerpc/kernel/traps.c
>> index db733d3..edc1108 100644
>> --- a/arch/powerpc/kernel/traps.c
>> +++ b/arch/powerpc/kernel/traps.c
>> @@ -1184,6 +1184,23 @@ void SoftwareEmulation(struct pt_regs *regs)
>> #endif /* CONFIG_8xx */
>>=20
>> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>> +static void do_send_trap(struct pt_regs *regs, unsigned long =
address,
>> +		  unsigned long error_code, int signal_code, int =
breakpt)
>> +{
>> +	siginfo_t info;
>> +
>> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>> +		return;
>> +
>> +	/* Deliver the signal to userspace */
>> +	info.si_signo =3D SIGTRAP;
>> +	info.si_errno =3D breakpt;	/* breakpoint or watchpoint id =
*/
>> +	info.si_code =3D signal_code;
>> +	info.si_addr =3D (void __user *)address;
>> +	force_sig_info(SIGTRAP, &info, current);
>> +}
>> +
>> static void handle_debug(struct pt_regs *regs, unsigned long =
debug_status)
>> {
>> 	int changed =3D 0;

Re: [1/4] powerpc: Revert show_regs() define for readability

[Jimi Xenidis]

On Oct 31, 2011, at 9:18 AM, Kumar Gala wrote:

>=20
> On Oct 28, 2011, at 2:40 PM, Jimi Xenidis wrote:
>=20
>>=20
>> On Oct 5, 2011, at 9:53 PM, Kumar Gala wrote:
>>=20
>>> We had an existing ifdef for 4xx & BOOKE processors that got changed =
to
>>> CONFIG_PPC_ADV_DEBUG_REGS.  The define has nothing to do with
>>> CONFIG_PPC_ADV_DEBUG_REGS.  The define really should be:
>>>=20
>>> #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
>>>=20
>>> and not
>>>=20
>>> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>>=20
>>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>>>=20
>>> ---
>>> arch/powerpc/kernel/process.c |    2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>=20
>>> diff --git a/arch/powerpc/kernel/process.c =
b/arch/powerpc/kernel/process.c
>>> index 8f53954..a1b5981 100644
>>> --- a/arch/powerpc/kernel/process.c
>>> +++ b/arch/powerpc/kernel/process.c
>>> @@ -657,7 +657,7 @@ void show_regs(struct pt_regs * regs)
>>> 	if ((regs->trap !=3D 0xc00) && cpu_has_feature(CPU_FTR_CFAR))
>>> 		printk("CFAR: "REG"\n", regs->orig_gpr3);
>>> 	if (trap =3D=3D 0x300 || trap =3D=3D 0x600)
>>> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>> +#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
>>> 		printk("DEAR: "REG", ESR: "REG"\n", regs->dar, =
regs->dsisr);
>>=20
>> I'll be needing "|| defined(CONFIG_PPC_BOOK3E)" added to this please.
>> -jx
>=20
> Under what platform is CONFIG_PPC_BOOK3E set and CONFIG_BOOKE is not?

this was a grep typo on my part.  sorry.
-jx


>=20
> - k
>=20

Re: [4/4] powerpc/booke: Re-organize debug code

[Jimi Xenidis]

On Oct 31, 2011, at 9:21 AM, Kumar Gala wrote:

>=20
> On Oct 28, 2011, at 2:37 PM, Jimi Xenidis wrote:
>=20
>>=20
>> On Oct 5, 2011, at 9:53 PM, Kumar Gala wrote:
>>=20
>>> * set_dabr/do_dabr are no longer used when CNFIG_PPC_ADV_DEBUG_REGS =
is set
>>> refactor code a bit such that we only build the dabr code for
>>> !CONFIG_PPC_ADV_DEBUG_REGS and removed some =
CONFIG_PPC_ADV_DEBUG_REGS
>>> code in set_dabr that would never get built.
>>>=20
>>> * Move do_send_trap into traps.c as its only used there
>>>=20
>>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>>>=20
>>> ---
>>> arch/powerpc/include/asm/system.h |    5 +--
>>> arch/powerpc/kernel/process.c     |   97 =
+++++++++++++-----------------------
>>> arch/powerpc/kernel/traps.c       |   17 +++++++
>>> 3 files changed, 53 insertions(+), 66 deletions(-)
>>>=20
>>> diff --git a/arch/powerpc/include/asm/system.h =
b/arch/powerpc/include/asm/system.h
>>> index e30a13d..1dc5d9c 100644
>>> --- a/arch/powerpc/include/asm/system.h
>>> +++ b/arch/powerpc/include/asm/system.h
>>> @@ -111,11 +111,8 @@ static inline int debugger_dabr_match(struct =
pt_regs *regs) { return 0; }
>>> static inline int debugger_fault_handler(struct pt_regs *regs) { =
return 0; }
>>> #endif
>>>=20
>>> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
>>> extern int set_dabr(unsigned long dabr);
>>> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>> -extern void do_send_trap(struct pt_regs *regs, unsigned long =
address,
>>> -			 unsigned long error_code, int signal_code, int =
brkpt);
>>> -#else
>>=20
>>=20
>> This part of the patch breaks xmon.c
>> Naively I simply wrapped the xmon call:
>>=20
>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> index f08836a..b5911b2 100644
>> --- a/arch/powerpc/xmon/xmon.c
>> +++ b/arch/powerpc/xmon/xmon.c
>> @@ -738,8 +738,10 @@ static void insert_bpts(void)
>>=20
>> static void insert_cpu_bpts(void)
>> {
>> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
>> 	if (dabr.enabled)
>> 		set_dabr(dabr.address | (dabr.enabled & 7));
>> +#endif
>> 	if (iabr && cpu_has_feature(CPU_FTR_IABR))
>> 		mtspr(SPRN_IABR, iabr->address
>> 			 | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
>> @@ -767,7 +769,9 @@ static void remove_bpts(void)
>>=20
>> static void remove_cpu_bpts(void)
>> {
>> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
>> 	set_dabr(0);
>> +#endif
>> 	if (cpu_has_feature(CPU_FTR_IABR))
>> 		mtspr(SPRN_IABR, 0);
>> }
>=20
> Shouldn't all of these functions be #ifndef'd out as we don't support =
cpu_bpts on book-e parts in xmon code today?

Well I guess this is one for benh, because I would have expected xmon to =
test and call ppc_md.dabr.
Actually, should everyone be doing that?
-jx


>=20
>>=20
>> -JX
>>=20
>>=20
>>> extern void do_dabr(struct pt_regs *regs, unsigned long address,
>>> 		    unsigned long error_code);
>>> #endif
>>> diff --git a/arch/powerpc/kernel/process.c =
b/arch/powerpc/kernel/process.c
>>> index 269a309..989e574 100644
>>> --- a/arch/powerpc/kernel/process.c
>>> +++ b/arch/powerpc/kernel/process.c
>>> @@ -251,50 +251,6 @@ void discard_lazy_cpu_state(void)
>>> #endif /* CONFIG_SMP */
>>>=20
>>> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>> -void do_send_trap(struct pt_regs *regs, unsigned long address,
>>> -		  unsigned long error_code, int signal_code, int =
breakpt)
>>> -{
>>> -	siginfo_t info;
>>> -
>>> -	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>>> -			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>>> -		return;
>>> -
>>> -	/* Deliver the signal to userspace */
>>> -	info.si_signo =3D SIGTRAP;
>>> -	info.si_errno =3D breakpt;	/* breakpoint or watchpoint id =
*/
>>> -	info.si_code =3D signal_code;
>>> -	info.si_addr =3D (void __user *)address;
>>> -	force_sig_info(SIGTRAP, &info, current);
>>> -}
>>> -#else	/* !CONFIG_PPC_ADV_DEBUG_REGS */
>>> -void do_dabr(struct pt_regs *regs, unsigned long address,
>>> -		    unsigned long error_code)
>>> -{
>>> -	siginfo_t info;
>>> -
>>> -	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>>> -			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>>> -		return;
>>> -
>>> -	if (debugger_dabr_match(regs))
>>> -		return;
>>> -
>>> -	/* Clear the DABR */
>>> -	set_dabr(0);
>>> -
>>> -	/* Deliver the signal to userspace */
>>> -	info.si_signo =3D SIGTRAP;
>>> -	info.si_errno =3D 0;
>>> -	info.si_code =3D TRAP_HWBKPT;
>>> -	info.si_addr =3D (void __user *)address;
>>> -	force_sig_info(SIGTRAP, &info, current);
>>> -}
>>> -#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
>>> -
>>> -static DEFINE_PER_CPU(unsigned long, current_dabr);
>>> -
>>> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>> /*
>>> * Set the debug registers back to their default "safe" values.
>>> */
>>> @@ -357,16 +313,7 @@ static void switch_booke_debug_regs(struct =
thread_struct *new_thread)
>>> 			prime_debug_regs(new_thread);
>>> }
>>> #else	/* !CONFIG_PPC_ADV_DEBUG_REGS */
>>> -#ifndef CONFIG_HAVE_HW_BREAKPOINT
>>> -static void set_debug_reg_defaults(struct thread_struct *thread)
>>> -{
>>> -	if (thread->dabr) {
>>> -		thread->dabr =3D 0;
>>> -		set_dabr(0);
>>> -	}
>>> -}
>>> -#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
>>> -#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
>>> +static DEFINE_PER_CPU(unsigned long, current_dabr);
>>>=20
>>> int set_dabr(unsigned long dabr)
>>> {
>>> @@ -376,19 +323,45 @@ int set_dabr(unsigned long dabr)
>>> 		return ppc_md.set_dabr(dabr);
>>>=20
>>> 	/* XXX should we have a CPU_FTR_HAS_DABR ? */
>>> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>> -	mtspr(SPRN_DAC1, dabr);
>>> -#ifdef CONFIG_PPC_47x
>>> -	isync();
>>> -#endif
>>> -#elif defined(CONFIG_PPC_BOOK3S)
>>> 	mtspr(SPRN_DABR, dabr);
>>> -#endif
>>> -
>>>=20
>>> 	return 0;
>>> }
>>>=20
>>> +void do_dabr(struct pt_regs *regs, unsigned long address,
>>> +		    unsigned long error_code)
>>> +{
>>> +	siginfo_t info;
>>> +
>>> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>>> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>>> +		return;
>>> +
>>> +	if (debugger_dabr_match(regs))
>>> +		return;
>>> +
>>> +	/* Clear the DABR */
>>> +	set_dabr(0);
>>> +
>>> +	/* Deliver the signal to userspace */
>>> +	info.si_signo =3D SIGTRAP;
>>> +	info.si_errno =3D 0;
>>> +	info.si_code =3D TRAP_HWBKPT;
>>> +	info.si_addr =3D (void __user *)address;
>>> +	force_sig_info(SIGTRAP, &info, current);
>>> +}
>>> +
>>> +#ifndef CONFIG_HAVE_HW_BREAKPOINT
>>> +static void set_debug_reg_defaults(struct thread_struct *thread)
>>> +{
>>> +	if (thread->dabr) {
>>> +		thread->dabr =3D 0;
>>> +		set_dabr(0);
>>> +	}
>>> +}
>>> +#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
>>> +#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
>>> +
>>> #ifdef CONFIG_PPC64
>>> DEFINE_PER_CPU(struct cpu_usage, cpu_usage_array);
>>> #endif
>>> diff --git a/arch/powerpc/kernel/traps.c =
b/arch/powerpc/kernel/traps.c
>>> index db733d3..edc1108 100644
>>> --- a/arch/powerpc/kernel/traps.c
>>> +++ b/arch/powerpc/kernel/traps.c
>>> @@ -1184,6 +1184,23 @@ void SoftwareEmulation(struct pt_regs *regs)
>>> #endif /* CONFIG_8xx */
>>>=20
>>> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>> +static void do_send_trap(struct pt_regs *regs, unsigned long =
address,
>>> +		  unsigned long error_code, int signal_code, int =
breakpt)
>>> +{
>>> +	siginfo_t info;
>>> +
>>> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>>> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>>> +		return;
>>> +
>>> +	/* Deliver the signal to userspace */
>>> +	info.si_signo =3D SIGTRAP;
>>> +	info.si_errno =3D breakpt;	/* breakpoint or watchpoint id =
*/
>>> +	info.si_code =3D signal_code;
>>> +	info.si_addr =3D (void __user *)address;
>>> +	force_sig_info(SIGTRAP, &info, current);
>>> +}
>>> +
>>> static void handle_debug(struct pt_regs *regs, unsigned long =
debug_status)
>>> {
>>> 	int changed =3D 0;
>=20

Re: [4/4] powerpc/booke: Re-organize debug code

[Kumar Gala]

>>>> * set_dabr/do_dabr are no longer used when CNFIG_PPC_ADV_DEBUG_REGS =
is set
>>>> refactor code a bit such that we only build the dabr code for
>>>> !CONFIG_PPC_ADV_DEBUG_REGS and removed some =
CONFIG_PPC_ADV_DEBUG_REGS
>>>> code in set_dabr that would never get built.
>>>>=20
>>>> * Move do_send_trap into traps.c as its only used there
>>>>=20
>>>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>>>>=20
>>>> ---
>>>> arch/powerpc/include/asm/system.h |    5 +--
>>>> arch/powerpc/kernel/process.c     |   97 =
+++++++++++++-----------------------
>>>> arch/powerpc/kernel/traps.c       |   17 +++++++
>>>> 3 files changed, 53 insertions(+), 66 deletions(-)
>>>>=20
>>>> diff --git a/arch/powerpc/include/asm/system.h =
b/arch/powerpc/include/asm/system.h
>>>> index e30a13d..1dc5d9c 100644
>>>> --- a/arch/powerpc/include/asm/system.h
>>>> +++ b/arch/powerpc/include/asm/system.h
>>>> @@ -111,11 +111,8 @@ static inline int debugger_dabr_match(struct =
pt_regs *regs) { return 0; }
>>>> static inline int debugger_fault_handler(struct pt_regs *regs) { =
return 0; }
>>>> #endif
>>>>=20
>>>> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
>>>> extern int set_dabr(unsigned long dabr);
>>>> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>>> -extern void do_send_trap(struct pt_regs *regs, unsigned long =
address,
>>>> -			 unsigned long error_code, int signal_code, int =
brkpt);
>>>> -#else
>>>=20
>>>=20
>>> This part of the patch breaks xmon.c
>>> Naively I simply wrapped the xmon call:
>>>=20
>>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>> index f08836a..b5911b2 100644
>>> --- a/arch/powerpc/xmon/xmon.c
>>> +++ b/arch/powerpc/xmon/xmon.c
>>> @@ -738,8 +738,10 @@ static void insert_bpts(void)
>>>=20
>>> static void insert_cpu_bpts(void)
>>> {
>>> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
>>> 	if (dabr.enabled)
>>> 		set_dabr(dabr.address | (dabr.enabled & 7));
>>> +#endif
>>> 	if (iabr && cpu_has_feature(CPU_FTR_IABR))
>>> 		mtspr(SPRN_IABR, iabr->address
>>> 			 | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
>>> @@ -767,7 +769,9 @@ static void remove_bpts(void)
>>>=20
>>> static void remove_cpu_bpts(void)
>>> {
>>> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
>>> 	set_dabr(0);
>>> +#endif
>>> 	if (cpu_has_feature(CPU_FTR_IABR))
>>> 		mtspr(SPRN_IABR, 0);
>>> }
>>=20
>> Shouldn't all of these functions be #ifndef'd out as we don't support =
cpu_bpts on book-e parts in xmon code today?
>=20
> Well I guess this is one for benh, because I would have expected xmon =
to test and call ppc_md.dabr.
> Actually, should everyone be doing that?
> -jx

Ben,

Any comment on direction here ?

- k

>>>> extern void do_dabr(struct pt_regs *regs, unsigned long address,
>>>> 		    unsigned long error_code);
>>>> #endif
>>>> diff --git a/arch/powerpc/kernel/process.c =
b/arch/powerpc/kernel/process.c
>>>> index 269a309..989e574 100644
>>>> --- a/arch/powerpc/kernel/process.c
>>>> +++ b/arch/powerpc/kernel/process.c
>>>> @@ -251,50 +251,6 @@ void discard_lazy_cpu_state(void)
>>>> #endif /* CONFIG_SMP */
>>>>=20
>>>> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>>> -void do_send_trap(struct pt_regs *regs, unsigned long address,
>>>> -		  unsigned long error_code, int signal_code, int =
breakpt)
>>>> -{
>>>> -	siginfo_t info;
>>>> -
>>>> -	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>>>> -			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>>>> -		return;
>>>> -
>>>> -	/* Deliver the signal to userspace */
>>>> -	info.si_signo =3D SIGTRAP;
>>>> -	info.si_errno =3D breakpt;	/* breakpoint or watchpoint id =
*/
>>>> -	info.si_code =3D signal_code;
>>>> -	info.si_addr =3D (void __user *)address;
>>>> -	force_sig_info(SIGTRAP, &info, current);
>>>> -}
>>>> -#else	/* !CONFIG_PPC_ADV_DEBUG_REGS */
>>>> -void do_dabr(struct pt_regs *regs, unsigned long address,
>>>> -		    unsigned long error_code)
>>>> -{
>>>> -	siginfo_t info;
>>>> -
>>>> -	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>>>> -			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>>>> -		return;
>>>> -
>>>> -	if (debugger_dabr_match(regs))
>>>> -		return;
>>>> -
>>>> -	/* Clear the DABR */
>>>> -	set_dabr(0);
>>>> -
>>>> -	/* Deliver the signal to userspace */
>>>> -	info.si_signo =3D SIGTRAP;
>>>> -	info.si_errno =3D 0;
>>>> -	info.si_code =3D TRAP_HWBKPT;
>>>> -	info.si_addr =3D (void __user *)address;
>>>> -	force_sig_info(SIGTRAP, &info, current);
>>>> -}
>>>> -#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
>>>> -
>>>> -static DEFINE_PER_CPU(unsigned long, current_dabr);
>>>> -
>>>> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>>> /*
>>>> * Set the debug registers back to their default "safe" values.
>>>> */
>>>> @@ -357,16 +313,7 @@ static void switch_booke_debug_regs(struct =
thread_struct *new_thread)
>>>> 			prime_debug_regs(new_thread);
>>>> }
>>>> #else	/* !CONFIG_PPC_ADV_DEBUG_REGS */
>>>> -#ifndef CONFIG_HAVE_HW_BREAKPOINT
>>>> -static void set_debug_reg_defaults(struct thread_struct *thread)
>>>> -{
>>>> -	if (thread->dabr) {
>>>> -		thread->dabr =3D 0;
>>>> -		set_dabr(0);
>>>> -	}
>>>> -}
>>>> -#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
>>>> -#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
>>>> +static DEFINE_PER_CPU(unsigned long, current_dabr);
>>>>=20
>>>> int set_dabr(unsigned long dabr)
>>>> {
>>>> @@ -376,19 +323,45 @@ int set_dabr(unsigned long dabr)
>>>> 		return ppc_md.set_dabr(dabr);
>>>>=20
>>>> 	/* XXX should we have a CPU_FTR_HAS_DABR ? */
>>>> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>>> -	mtspr(SPRN_DAC1, dabr);
>>>> -#ifdef CONFIG_PPC_47x
>>>> -	isync();
>>>> -#endif
>>>> -#elif defined(CONFIG_PPC_BOOK3S)
>>>> 	mtspr(SPRN_DABR, dabr);
>>>> -#endif
>>>> -
>>>>=20
>>>> 	return 0;
>>>> }
>>>>=20
>>>> +void do_dabr(struct pt_regs *regs, unsigned long address,
>>>> +		    unsigned long error_code)
>>>> +{
>>>> +	siginfo_t info;
>>>> +
>>>> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>>>> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>>>> +		return;
>>>> +
>>>> +	if (debugger_dabr_match(regs))
>>>> +		return;
>>>> +
>>>> +	/* Clear the DABR */
>>>> +	set_dabr(0);
>>>> +
>>>> +	/* Deliver the signal to userspace */
>>>> +	info.si_signo =3D SIGTRAP;
>>>> +	info.si_errno =3D 0;
>>>> +	info.si_code =3D TRAP_HWBKPT;
>>>> +	info.si_addr =3D (void __user *)address;
>>>> +	force_sig_info(SIGTRAP, &info, current);
>>>> +}
>>>> +
>>>> +#ifndef CONFIG_HAVE_HW_BREAKPOINT
>>>> +static void set_debug_reg_defaults(struct thread_struct *thread)
>>>> +{
>>>> +	if (thread->dabr) {
>>>> +		thread->dabr =3D 0;
>>>> +		set_dabr(0);
>>>> +	}
>>>> +}
>>>> +#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
>>>> +#endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
>>>> +
>>>> #ifdef CONFIG_PPC64
>>>> DEFINE_PER_CPU(struct cpu_usage, cpu_usage_array);
>>>> #endif
>>>> diff --git a/arch/powerpc/kernel/traps.c =
b/arch/powerpc/kernel/traps.c
>>>> index db733d3..edc1108 100644
>>>> --- a/arch/powerpc/kernel/traps.c
>>>> +++ b/arch/powerpc/kernel/traps.c
>>>> @@ -1184,6 +1184,23 @@ void SoftwareEmulation(struct pt_regs *regs)
>>>> #endif /* CONFIG_8xx */
>>>>=20
>>>> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>>>> +static void do_send_trap(struct pt_regs *regs, unsigned long =
address,
>>>> +		  unsigned long error_code, int signal_code, int =
breakpt)
>>>> +{
>>>> +	siginfo_t info;
>>>> +
>>>> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>>>> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
>>>> +		return;
>>>> +
>>>> +	/* Deliver the signal to userspace */
>>>> +	info.si_signo =3D SIGTRAP;
>>>> +	info.si_errno =3D breakpt;	/* breakpoint or watchpoint id =
*/
>>>> +	info.si_code =3D signal_code;
>>>> +	info.si_addr =3D (void __user *)address;
>>>> +	force_sig_info(SIGTRAP, &info, current);
>>>> +}
>>>> +
>>>> static void handle_debug(struct pt_regs *regs, unsigned long =
debug_status)
>>>> {
>>>> 	int changed =3D 0;
>>=20