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

Jimi Xenidis jimix at pobox.com
Tue Nov 1 05:37:30 EST 2011


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

> 
> On Oct 28, 2011, at 2:37 PM, Jimi Xenidis wrote:
> 
>> 
>> 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.
>>> 
>>> * Move do_send_trap into traps.c as its only used there
>>> 
>>> Signed-off-by: Kumar Gala <galak at 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
>> 
>> 
>> 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)
>> 
>> 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)
>> 
>> 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?

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


> 
>> 
>> -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 */
>>> 
>>> #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;
> 



More information about the Linuxppc-dev mailing list