Re: [PATCH 2/2] powerpc/44x: Fix xmon single step on PowerPC 44x

[Josh Boyer <jwboyer@linux.vnet.ibm.com>]

On Thu, Sep 24, 2009 at 07:34:34AM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2009-09-23 at 09:51 -0400, Josh Boyer wrote:
>> Prior to the arch/ppc -> arch/powerpc transition, xmon had support for single
>> stepping on 4xx boards.  The functionality was lost when arch/ppc was removed.
>> This patch restores single step support for 44x boards.
>> 
>> Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/xmon/xmon.c |   20 +++++++++++++++++++-
>>  1 files changed, 19 insertions(+), 1 deletions(-)
>> 
>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> index c6f0a71..fe2ad71 100644
>> --- a/arch/powerpc/xmon/xmon.c
>> +++ b/arch/powerpc/xmon/xmon.c
>> @@ -517,6 +517,15 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
>>  	in_xmon = 0;
>>  #endif
>>  
>> +#ifdef CONFIG_4xx
>> +	if ((regs->msr & (MSR_DE)) == (MSR_DE)) {
>
>Why not just if (regs->msr & MSR_DE) ?

Blind duplication of existing if case.  Will fix.

>> +		bp = at_breakpoint(regs->nip);
>> +		if (bp != NULL) {
>> +			regs->nip = (unsigned long) &bp->instr[0];
>> +			atomic_inc(&bp->ref_count);
>> +		}
>> +	}
>> +#else
>
>Any reason why that couldn't be in CONFIG_BOOKE ?

Off the top of my head, no.  I haven't tested on 40x yet though.  Will try
and do that and revise.

>>  	if ((regs->msr & (MSR_IR|MSR_PR|MSR_SF)) == (MSR_IR|MSR_SF)) {
>>  		bp = at_breakpoint(regs->nip);
>>  		if (bp != NULL) {
>> @@ -530,7 +539,7 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
>>  			}
>>  		}
>>  	}
>> -
>> +#endif
>>  	insert_cpu_bpts();
>>  
>>  	local_irq_restore(flags);
>> @@ -894,6 +903,14 @@ cmds(struct pt_regs *excp)
>>  	}
>>  }
>>  
>> +#ifdef CONFIG_44x
>
>Same comment about BOOKE intead of 44x

Nod.

>> +static int do_step(struct pt_regs *regs)
>> +{
>> +	regs->msr |= MSR_DE;
>> +	mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) | DBCR0_IC | DBCR0_IDM);
>
>I'm not sure about setting IDM... Won't that be a problem if you have
>an external debugger connected ?

It could be.  I have no external debugger, thus no way to check it.  You don't
get an exception without IDM set though, so it won't trap back into xmon like
it should.  This is how we did it in arch/ppc (which isn't always a great
thing) as well.

I don't see how you could get it working without IDM, unless you inserted a
trap (aka breakpoint) every time.  That seems sort of suboptimal when we have
the IC event we can use.

josh