diff mbox

powerpc, xmon: Enable hardware instruction breakpoint support on POWER8

Message ID 1401451823-25547-1-git-send-email-khandual@linux.vnet.ibm.com
State Superseded
Headers show

Commit Message

Anshuman Khandual May 30, 2014, 12:10 p.m. UTC
This patch enables support for hardware instruction breakpoints on POWER8 with
the help of a new register called CIABR (Completed Instruction Address Breakpoint
Register). With this patch, single hardware instruction breakpoint can be added
and cleared during any active xmon debug session. This hardware based instruction
breakpoint mechanism works correctly along with the existing TRAP based instruction
breakpoints available on xmon. Example usage as follows.

(A) Start xmon:
$echo x > /proc/sysrq-trigger
SysRq : Entering xmon
cpu 0x0: Vector: 0  at [c000001f6c67f960]
    pc: c000000000072078: .sysrq_handle_xmon+0x58/0x60
    lr: c000000000072078: .sysrq_handle_xmon+0x58/0x60
    sp: c000001f6c67fac0
   msr: 9000000000009032
  current = 0xc000001f6e709ac0
  paca    = 0xc00000000fffa000   softe: 0        irq_happened: 0x00
    pid   = 3250, comm = bash
enter ? for help
0:mon> b
   type            address

(B) Set the breakpoint:
0:mon> ls .power_pmu_add
.power_pmu_add: c000000000078f50
0:mon> bi c000000000078f50
0:mon> b
   type            address
 1 inst   c000000000078f50  .power_pmu_add+0x0/0x2e0
0:mon> ls .perf_event_interrupt
.perf_event_interrupt: c00000000007aee0
0:mon> bi c00000000007aee0
One instruction breakpoint possible with CIABR
0:mon> x

(C) Run the workload (with the breakpoint):
$./perf record ls
cpu 0x2: Vector: d00 (Single Step) at [c000001f718133a0]
    pc: c000000000078f54: .power_pmu_add+0x4/0x2e0
    lr: c000000000155be0: .event_sched_in+0x90/0x1d0
    sp: c000001f71813620
   msr: 9000000040109032
  current = 0xc000001f6ce30000
  paca    = 0xc00000000fffa600   softe: 0        irq_happened: 0x01
    pid   = 3270, comm = ls
        std     r22,-80(r1)
enter ? for help

(D) Clear the breakpoint:
2:mon> bc
All breakpoints cleared
2:mon> x
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.002 MB perf.data (~66 samples) ]

(E) Run the workload again (without any breakpoints):
$./perf record ls
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.001 MB perf.data (~61 samples) ]

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
 arch/powerpc/xmon/xmon.c | 62 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 58 insertions(+), 4 deletions(-)

Comments

Aneesh Kumar K.V May 30, 2014, 1:42 p.m. UTC | #1
Anshuman Khandual <khandual@linux.vnet.ibm.com> writes:

> This patch enables support for hardware instruction breakpoints on POWER8 with
> the help of a new register called CIABR (Completed Instruction Address Breakpoint
> Register). With this patch, single hardware instruction breakpoint can be added
> and cleared during any active xmon debug session. This hardware based instruction
> breakpoint mechanism works correctly along with the existing TRAP based instruction
> breakpoints available on xmon. Example usage as follows.
>
> (A) Start xmon:
> $echo x > /proc/sysrq-trigger
> SysRq : Entering xmon
> cpu 0x0: Vector: 0  at [c000001f6c67f960]
>     pc: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>     lr: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>     sp: c000001f6c67fac0
>    msr: 9000000000009032
>   current = 0xc000001f6e709ac0
>   paca    = 0xc00000000fffa000   softe: 0        irq_happened: 0x00
>     pid   = 3250, comm = bash
> enter ? for help
> 0:mon> b
>    type            address
>
> (B) Set the breakpoint:
> 0:mon> ls .power_pmu_add
> .power_pmu_add: c000000000078f50
> 0:mon> bi c000000000078f50
> 0:mon> b
>    type            address
>  1 inst   c000000000078f50  .power_pmu_add+0x0/0x2e0
> 0:mon> ls .perf_event_interrupt
> .perf_event_interrupt: c00000000007aee0
> 0:mon> bi c00000000007aee0
> One instruction breakpoint possible with CIABR
> 0:mon> x
>
> (C) Run the workload (with the breakpoint):
> $./perf record ls
> cpu 0x2: Vector: d00 (Single Step) at [c000001f718133a0]
>     pc: c000000000078f54: .power_pmu_add+0x4/0x2e0
>     lr: c000000000155be0: .event_sched_in+0x90/0x1d0
>     sp: c000001f71813620
>    msr: 9000000040109032
>   current = 0xc000001f6ce30000
>   paca    = 0xc00000000fffa600   softe: 0        irq_happened: 0x01
>     pid   = 3270, comm = ls
>         std     r22,-80(r1)
> enter ? for help
>
> (D) Clear the breakpoint:
> 2:mon> bc
> All breakpoints cleared
> 2:mon> x
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.002 MB perf.data (~66 samples) ]
>
> (E) Run the workload again (without any breakpoints):
> $./perf record ls
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.001 MB perf.data (~61 samples) ]
>
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
>  arch/powerpc/xmon/xmon.c | 62 ++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 3fd1d9a..f74ec83 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -48,6 +48,7 @@
>  #ifdef CONFIG_PPC64
>  #include <asm/hvcall.h>
>  #include <asm/paca.h>
> +#include <asm/plpar_wrappers.h>
>  #endif
>  
>  #include "nonstdio.h"
> @@ -89,6 +90,7 @@ struct bpt {
>  /* Bits in bpt.enabled */
>  #define BP_IABR_TE	1		/* IABR translation enabled */
>  #define BP_IABR		2
> +#define BP_CIABR	4
>  #define BP_TRAP		8
>  #define BP_DABR		0x10
>  
> @@ -97,6 +99,7 @@ static struct bpt bpts[NBPTS];
>  static struct bpt dabr;
>  static struct bpt *iabr;
>  static unsigned bpinstr = 0x7fe00008;	/* trap */
> +static bool ciabr_used = false;		/* CIABR instruction breakpoint */
>  
>  #define BP_NUM(bp)	((bp) - bpts + 1)
>  
> @@ -269,6 +272,34 @@ static inline void cinval(void *p)
>  	asm volatile ("dcbi 0,%0; icbi 0,%0" : : "r" (p));
>  }
>  
> +static void write_ciabr(unsigned long ciabr)
> +{
> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
> +		mtspr(SPRN_CIABR, ciabr);
> +		return;
> +	}
> +
> +#ifdef CONFIG_PPC64
> +	plapr_set_ciabr(ciabr);
> +#endif
> +}
> +
> +static void set_ciabr(unsigned long addr)
> +{
> +	addr &= ~CIABR_PRIV;
> +	if (cpu_has_feature(CPU_FTR_HVMODE))
> +		addr |= CIABR_PRIV_HYPER;
> +	else
> +		addr |= CIABR_PRIV_SUPER;
> +	write_ciabr(addr);
> +}
> +
> +static void clear_ciabr(void)
> +{
> +	if (cpu_has_feature(CPU_FTR_ARCH_207S))
> +		write_ciabr(0);
> +}
> +
>  /*
>   * Disable surveillance (the service processor watchdog function)
>   * while we are in xmon.
> @@ -764,6 +795,9 @@ static void insert_cpu_bpts(void)
>  	if (iabr && cpu_has_feature(CPU_FTR_IABR))
>  		mtspr(SPRN_IABR, iabr->address
>  			 | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
> +
> +	if (iabr && cpu_has_feature(CPU_FTR_ARCH_207S))
> +		set_ciabr(iabr->address);
>  }
>  
>  static void remove_bpts(void)
> @@ -791,6 +825,7 @@ static void remove_cpu_bpts(void)
>  	hw_breakpoint_disable();
>  	if (cpu_has_feature(CPU_FTR_IABR))
>  		mtspr(SPRN_IABR, 0);
> +	clear_ciabr();
>  }
>  
>  /* Command interpreting routine */
> @@ -1124,7 +1159,7 @@ static char *breakpoint_help_string =
>      "b <addr> [cnt]   set breakpoint at given instr addr\n"
>      "bc               clear all breakpoints\n"
>      "bc <n/addr>      clear breakpoint number n or at addr\n"
> -    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64 only)\n"
> +    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64/POWER8 only)\n"
>      "bd <addr> [cnt]  set hardware data breakpoint\n"
>      "";
>  
> @@ -1163,11 +1198,20 @@ bpt_cmds(void)
>  		break;
>  
>  	case 'i':	/* bi - hardware instr breakpoint */
> -		if (!cpu_has_feature(CPU_FTR_IABR)) {
> +		if (!cpu_has_feature(CPU_FTR_IABR) && !cpu_has_feature(CPU_FTR_ARCH_207S)) {
>  			printf("Hardware instruction breakpoint "
>  			       "not supported on this cpu\n");
>  			break;
>  		}
> +
> +		if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
> +			if (ciabr_used) {
> +				printf("One instruction breakpoint "
> +					"possible with CIABR\n");
> +				break;
> +			}

We don't seem to do that with iabr ? Why keep ciabr different 

> +		}


Why is this implemented different than existing iabr, You could do this
with iabr and iabr->enabled = BP_CIABR right ?

> +
>  		if (iabr) {
>  			iabr->enabled &= ~(BP_IABR | BP_IABR_TE);
>  			iabr = NULL;
> @@ -1178,7 +1222,12 @@ bpt_cmds(void)
>  			break;
>  		bp = new_breakpoint(a);
>  		if (bp != NULL) {
> -			bp->enabled |= BP_IABR | BP_IABR_TE;
> +			if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
> +				bp->enabled |= BP_CIABR;
> +				ciabr_used = true;
> +			}
> +			else
> +				bp->enabled |= BP_IABR | BP_IABR_TE;
>  			iabr = bp;
>  		}

-aneesh
Michael Neuling June 1, 2014, 6:18 a.m. UTC | #2
On Fri, 2014-05-30 at 17:40 +0530, Anshuman Khandual wrote:
> This patch enables support for hardware instruction breakpoints on POWER8 with
> the help of a new register called CIABR (Completed Instruction Address Breakpoint
> Register). With this patch, single hardware instruction breakpoint can be added
> and cleared during any active xmon debug session. This hardware based instruction
> breakpoint mechanism works correctly along with the existing TRAP based instruction
> breakpoints available on xmon. Example usage as follows.

Have you actually tried this on a guest?

Please also compile with a range of configs.  It doesn't compile with
ppc64e_defconfig.

In file included from /scratch/mikey/src/linux-ozlabs/arch/powerpc/xmon/xmon.c:51:0:
/scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h: In function 'get_cede_latency_hint':
/scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h:26:9: error: 'struct paca_struct' has no member named 'lppaca_ptr'
/scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h: In function 'set_cede_latency_hint':
/scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h:31:2: error: 'struct paca_struct' has no member named 'lppaca_ptr'
/scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h: In function 'plpar_page_set_loaned':
/scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h:98:2: error: implicit declaration of function 'cmo_get_page_size' [-Werror=implicit-function-declaration]
cc1: all warnings being treated as errors

> -				    (bp->enabled & BP_IABR)? "inst": "trap");
> +					(bp->enabled & (BP_IABR | BP_CIABR))?
> +				       				"inst": "trap");

Git complains about whitespace issues here.  Spaces before tabs.

Mikey
Anshuman Khandual June 2, 2014, 8:48 a.m. UTC | #3
On 06/01/2014 11:48 AM, Michael Neuling wrote:
> On Fri, 2014-05-30 at 17:40 +0530, Anshuman Khandual wrote:
>> This patch enables support for hardware instruction breakpoints on POWER8 with
>> the help of a new register called CIABR (Completed Instruction Address Breakpoint
>> Register). With this patch, single hardware instruction breakpoint can be added
>> and cleared during any active xmon debug session. This hardware based instruction
>> breakpoint mechanism works correctly along with the existing TRAP based instruction
>> breakpoints available on xmon. Example usage as follows.
> 
> Have you actually tried this on a guest?
> 

Yeah on a guest which runs on PVM.

> Please also compile with a range of configs.  It doesn't compile with
> ppc64e_defconfig.

Yeah. Need to change the way we get the "plapr_set_ciabr" function from plpar_wrappers.h
header file. Will add this hunk of code in "xmon.h" header and remove the CONFIG_PPC64 ifdef
code from the function write_ciabr.

+#ifdef CONFIG_PPC_BOOK3S_64
+#include <asm/plpar_wrappers.h>
+#else
+static inline long plapr_set_ciabr(unsigned long ciabr) {return 0;};
+#endif

> 
> In file included from /scratch/mikey/src/linux-ozlabs/arch/powerpc/xmon/xmon.c:51:0:
> /scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h: In function 'get_cede_latency_hint':
> /scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h:26:9: error: 'struct paca_struct' has no member named 'lppaca_ptr'
> /scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h: In function 'set_cede_latency_hint':
> /scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h:31:2: error: 'struct paca_struct' has no member named 'lppaca_ptr'
> /scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h: In function 'plpar_page_set_loaned':
> /scratch/mikey/src/linux-ozlabs/arch/powerpc/include/asm/plpar_wrappers.h:98:2: error: implicit declaration of function 'cmo_get_page_size' [-Werror=implicit-function-declaration]
> cc1: all warnings being treated as errors
> 
>> -				    (bp->enabled & BP_IABR)? "inst": "trap");
>> +					(bp->enabled & (BP_IABR | BP_CIABR))?
>> +				       				"inst": "trap");
> 
> Git complains about whitespace issues here.  Spaces before tabs.

Will take care of this next version.
Anshuman Khandual June 3, 2014, 6:03 a.m. UTC | #4
On 05/30/2014 07:12 PM, Aneesh Kumar K.V wrote:
> Anshuman Khandual <khandual@linux.vnet.ibm.com> writes:
> 
>> This patch enables support for hardware instruction breakpoints on POWER8 with
>> the help of a new register called CIABR (Completed Instruction Address Breakpoint
>> Register). With this patch, single hardware instruction breakpoint can be added
>> and cleared during any active xmon debug session. This hardware based instruction
>> breakpoint mechanism works correctly along with the existing TRAP based instruction
>> breakpoints available on xmon. Example usage as follows.
>>
>> (A) Start xmon:
>> $echo x > /proc/sysrq-trigger
>> SysRq : Entering xmon
>> cpu 0x0: Vector: 0  at [c000001f6c67f960]
>>     pc: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>>     lr: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>>     sp: c000001f6c67fac0
>>    msr: 9000000000009032
>>   current = 0xc000001f6e709ac0
>>   paca    = 0xc00000000fffa000   softe: 0        irq_happened: 0x00
>>     pid   = 3250, comm = bash
>> enter ? for help
>> 0:mon> b
>>    type            address
>>
>> (B) Set the breakpoint:
>> 0:mon> ls .power_pmu_add
>> .power_pmu_add: c000000000078f50
>> 0:mon> bi c000000000078f50
>> 0:mon> b
>>    type            address
>>  1 inst   c000000000078f50  .power_pmu_add+0x0/0x2e0
>> 0:mon> ls .perf_event_interrupt
>> .perf_event_interrupt: c00000000007aee0
>> 0:mon> bi c00000000007aee0
>> One instruction breakpoint possible with CIABR
>> 0:mon> x
>>
>> (C) Run the workload (with the breakpoint):
>> $./perf record ls
>> cpu 0x2: Vector: d00 (Single Step) at [c000001f718133a0]
>>     pc: c000000000078f54: .power_pmu_add+0x4/0x2e0
>>     lr: c000000000155be0: .event_sched_in+0x90/0x1d0
>>     sp: c000001f71813620
>>    msr: 9000000040109032
>>   current = 0xc000001f6ce30000
>>   paca    = 0xc00000000fffa600   softe: 0        irq_happened: 0x01
>>     pid   = 3270, comm = ls
>>         std     r22,-80(r1)
>> enter ? for help
>>
>> (D) Clear the breakpoint:
>> 2:mon> bc
>> All breakpoints cleared
>> 2:mon> x
>> [ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 0.002 MB perf.data (~66 samples) ]
>>
>> (E) Run the workload again (without any breakpoints):
>> $./perf record ls
>> [ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 0.001 MB perf.data (~61 samples) ]
>>
>> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/xmon/xmon.c | 62 ++++++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 58 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> index 3fd1d9a..f74ec83 100644
>> --- a/arch/powerpc/xmon/xmon.c
>> +++ b/arch/powerpc/xmon/xmon.c
>> @@ -48,6 +48,7 @@
>>  #ifdef CONFIG_PPC64
>>  #include <asm/hvcall.h>
>>  #include <asm/paca.h>
>> +#include <asm/plpar_wrappers.h>
>>  #endif
>>  
>>  #include "nonstdio.h"
>> @@ -89,6 +90,7 @@ struct bpt {
>>  /* Bits in bpt.enabled */
>>  #define BP_IABR_TE	1		/* IABR translation enabled */
>>  #define BP_IABR		2
>> +#define BP_CIABR	4
>>  #define BP_TRAP		8
>>  #define BP_DABR		0x10
>>  
>> @@ -97,6 +99,7 @@ static struct bpt bpts[NBPTS];
>>  static struct bpt dabr;
>>  static struct bpt *iabr;
>>  static unsigned bpinstr = 0x7fe00008;	/* trap */
>> +static bool ciabr_used = false;		/* CIABR instruction breakpoint */
>>  
>>  #define BP_NUM(bp)	((bp) - bpts + 1)
>>  
>> @@ -269,6 +272,34 @@ static inline void cinval(void *p)
>>  	asm volatile ("dcbi 0,%0; icbi 0,%0" : : "r" (p));
>>  }
>>  
>> +static void write_ciabr(unsigned long ciabr)
>> +{
>> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
>> +		mtspr(SPRN_CIABR, ciabr);
>> +		return;
>> +	}
>> +
>> +#ifdef CONFIG_PPC64
>> +	plapr_set_ciabr(ciabr);
>> +#endif
>> +}
>> +
>> +static void set_ciabr(unsigned long addr)
>> +{
>> +	addr &= ~CIABR_PRIV;
>> +	if (cpu_has_feature(CPU_FTR_HVMODE))
>> +		addr |= CIABR_PRIV_HYPER;
>> +	else
>> +		addr |= CIABR_PRIV_SUPER;
>> +	write_ciabr(addr);
>> +}
>> +
>> +static void clear_ciabr(void)
>> +{
>> +	if (cpu_has_feature(CPU_FTR_ARCH_207S))
>> +		write_ciabr(0);
>> +}
>> +
>>  /*
>>   * Disable surveillance (the service processor watchdog function)
>>   * while we are in xmon.
>> @@ -764,6 +795,9 @@ static void insert_cpu_bpts(void)
>>  	if (iabr && cpu_has_feature(CPU_FTR_IABR))
>>  		mtspr(SPRN_IABR, iabr->address
>>  			 | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
>> +
>> +	if (iabr && cpu_has_feature(CPU_FTR_ARCH_207S))
>> +		set_ciabr(iabr->address);
>>  }
>>  
>>  static void remove_bpts(void)
>> @@ -791,6 +825,7 @@ static void remove_cpu_bpts(void)
>>  	hw_breakpoint_disable();
>>  	if (cpu_has_feature(CPU_FTR_IABR))
>>  		mtspr(SPRN_IABR, 0);
>> +	clear_ciabr();
>>  }
>>  
>>  /* Command interpreting routine */
>> @@ -1124,7 +1159,7 @@ static char *breakpoint_help_string =
>>      "b <addr> [cnt]   set breakpoint at given instr addr\n"
>>      "bc               clear all breakpoints\n"
>>      "bc <n/addr>      clear breakpoint number n or at addr\n"
>> -    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64 only)\n"
>> +    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64/POWER8 only)\n"
>>      "bd <addr> [cnt]  set hardware data breakpoint\n"
>>      "";
>>  
>> @@ -1163,11 +1198,20 @@ bpt_cmds(void)
>>  		break;
>>  
>>  	case 'i':	/* bi - hardware instr breakpoint */
>> -		if (!cpu_has_feature(CPU_FTR_IABR)) {
>> +		if (!cpu_has_feature(CPU_FTR_IABR) && !cpu_has_feature(CPU_FTR_ARCH_207S)) {
>>  			printf("Hardware instruction breakpoint "
>>  			       "not supported on this cpu\n");
>>  			break;
>>  		}
>> +
>> +		if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
>> +			if (ciabr_used) {
>> +				printf("One instruction breakpoint "
>> +					"possible with CIABR\n");
>> +				break;
>> +			}
> 
> We don't seem to do that with iabr ? Why keep ciabr different 

Right now with the IABR implementation if we try to set hardware instruction
breakpoint while one is already there, it just get overridden with the new
address without complaining. I thought with this at least for CIABR cases,
it will complain about it and require the user to clear the breakpoint
explicitly before allowing a new breakpoint. Okay will remove this.
 
> 
>> +		}
> 
> 
> Why is this implemented different than existing iabr, You could do this
> with iabr and iabr->enabled = BP_CIABR right ?

Did not get it, yeah its implemented in that way only.
Anshuman Khandual June 3, 2014, 8:31 a.m. UTC | #5
On 06/02/2014 02:18 PM, Anshuman Khandual wrote:
> On 06/01/2014 11:48 AM, Michael Neuling wrote:
>> > On Fri, 2014-05-30 at 17:40 +0530, Anshuman Khandual wrote:
>>> >> This patch enables support for hardware instruction breakpoints on POWER8 with
>>> >> the help of a new register called CIABR (Completed Instruction Address Breakpoint
>>> >> Register). With this patch, single hardware instruction breakpoint can be added
>>> >> and cleared during any active xmon debug session. This hardware based instruction
>>> >> breakpoint mechanism works correctly along with the existing TRAP based instruction
>>> >> breakpoints available on xmon. Example usage as follows.
>> > 
>> > Have you actually tried this on a guest?
>> > 
> Yeah on a guest which runs on PVM.
> 
>> > Please also compile with a range of configs.  It doesn't compile with
>> > ppc64e_defconfig.
> Yeah. Need to change the way we get the "plapr_set_ciabr" function from plpar_wrappers.h
> header file. Will add this hunk of code in "xmon.h" header and remove the CONFIG_PPC64 ifdef
> code from the function write_ciabr.
> 
> +#ifdef CONFIG_PPC_BOOK3S_64

"#if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_SPLPAR)" here actually makes it build
on all these configurations listed below.

pseries_defconfig
ppc64_defconfig
ppc64e_defconfig
pmac32_defconfig
ppc44x_defconfig
ppc6xx_defconfig
mpc85xx_smp_defconfig
mpc85xx_defconfig
chroma_defconfig
ps3_defconfig
celleb_defconfig
allnoconfig
Anshuman Khandual June 4, 2014, 8:48 a.m. UTC | #6
On 06/03/2014 11:33 AM, Anshuman Khandual wrote:
> On 05/30/2014 07:12 PM, Aneesh Kumar K.V wrote:
>> > Anshuman Khandual <khandual@linux.vnet.ibm.com> writes:
>> > 
>>> >> This patch enables support for hardware instruction breakpoints on POWER8 with
>>> >> the help of a new register called CIABR (Completed Instruction Address Breakpoint
>>> >> Register). With this patch, single hardware instruction breakpoint can be added
>>> >> and cleared during any active xmon debug session. This hardware based instruction
>>> >> breakpoint mechanism works correctly along with the existing TRAP based instruction
>>> >> breakpoints available on xmon. Example usage as follows.
>>> >>
>>> >> (A) Start xmon:
>>> >> $echo x > /proc/sysrq-trigger
>>> >> SysRq : Entering xmon
>>> >> cpu 0x0: Vector: 0  at [c000001f6c67f960]
>>> >>     pc: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>>> >>     lr: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>>> >>     sp: c000001f6c67fac0
>>> >>    msr: 9000000000009032
>>> >>   current = 0xc000001f6e709ac0
>>> >>   paca    = 0xc00000000fffa000   softe: 0        irq_happened: 0x00
>>> >>     pid   = 3250, comm = bash
>>> >> enter ? for help
>>> >> 0:mon> b
>>> >>    type            address
>>> >>
>>> >> (B) Set the breakpoint:
>>> >> 0:mon> ls .power_pmu_add
>>> >> .power_pmu_add: c000000000078f50
>>> >> 0:mon> bi c000000000078f50
>>> >> 0:mon> b
>>> >>    type            address
>>> >>  1 inst   c000000000078f50  .power_pmu_add+0x0/0x2e0
>>> >> 0:mon> ls .perf_event_interrupt
>>> >> .perf_event_interrupt: c00000000007aee0
>>> >> 0:mon> bi c00000000007aee0
>>> >> One instruction breakpoint possible with CIABR
>>> >> 0:mon> x
>>> >>
>>> >> (C) Run the workload (with the breakpoint):
>>> >> $./perf record ls
>>> >> cpu 0x2: Vector: d00 (Single Step) at [c000001f718133a0]
>>> >>     pc: c000000000078f54: .power_pmu_add+0x4/0x2e0
>>> >>     lr: c000000000155be0: .event_sched_in+0x90/0x1d0
>>> >>     sp: c000001f71813620
>>> >>    msr: 9000000040109032
>>> >>   current = 0xc000001f6ce30000
>>> >>   paca    = 0xc00000000fffa600   softe: 0        irq_happened: 0x01
>>> >>     pid   = 3270, comm = ls
>>> >>         std     r22,-80(r1)
>>> >> enter ? for help
>>> >>
>>> >> (D) Clear the breakpoint:
>>> >> 2:mon> bc
>>> >> All breakpoints cleared
>>> >> 2:mon> x
>>> >> [ perf record: Woken up 1 times to write data ]
>>> >> [ perf record: Captured and wrote 0.002 MB perf.data (~66 samples) ]
>>> >>
>>> >> (E) Run the workload again (without any breakpoints):
>>> >> $./perf record ls
>>> >> [ perf record: Woken up 1 times to write data ]
>>> >> [ perf record: Captured and wrote 0.001 MB perf.data (~61 samples) ]
>>> >>
>>> >> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>>> >> ---
>>> >>  arch/powerpc/xmon/xmon.c | 62 ++++++++++++++++++++++++++++++++++++++++++++----
>>> >>  1 file changed, 58 insertions(+), 4 deletions(-)
>>> >>
>>> >> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>> >> index 3fd1d9a..f74ec83 100644
>>> >> --- a/arch/powerpc/xmon/xmon.c
>>> >> +++ b/arch/powerpc/xmon/xmon.c
>>> >> @@ -48,6 +48,7 @@
>>> >>  #ifdef CONFIG_PPC64
>>> >>  #include <asm/hvcall.h>
>>> >>  #include <asm/paca.h>
>>> >> +#include <asm/plpar_wrappers.h>
>>> >>  #endif
>>> >>  
>>> >>  #include "nonstdio.h"
>>> >> @@ -89,6 +90,7 @@ struct bpt {
>>> >>  /* Bits in bpt.enabled */
>>> >>  #define BP_IABR_TE	1		/* IABR translation enabled */
>>> >>  #define BP_IABR		2
>>> >> +#define BP_CIABR	4
>>> >>  #define BP_TRAP		8
>>> >>  #define BP_DABR		0x10
>>> >>  
>>> >> @@ -97,6 +99,7 @@ static struct bpt bpts[NBPTS];
>>> >>  static struct bpt dabr;
>>> >>  static struct bpt *iabr;
>>> >>  static unsigned bpinstr = 0x7fe00008;	/* trap */
>>> >> +static bool ciabr_used = false;		/* CIABR instruction breakpoint */
>>> >>  
>>> >>  #define BP_NUM(bp)	((bp) - bpts + 1)
>>> >>  
>>> >> @@ -269,6 +272,34 @@ static inline void cinval(void *p)
>>> >>  	asm volatile ("dcbi 0,%0; icbi 0,%0" : : "r" (p));
>>> >>  }
>>> >>  
>>> >> +static void write_ciabr(unsigned long ciabr)
>>> >> +{
>>> >> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
>>> >> +		mtspr(SPRN_CIABR, ciabr);
>>> >> +		return;
>>> >> +	}
>>> >> +
>>> >> +#ifdef CONFIG_PPC64
>>> >> +	plapr_set_ciabr(ciabr);
>>> >> +#endif
>>> >> +}
>>> >> +
>>> >> +static void set_ciabr(unsigned long addr)
>>> >> +{
>>> >> +	addr &= ~CIABR_PRIV;
>>> >> +	if (cpu_has_feature(CPU_FTR_HVMODE))
>>> >> +		addr |= CIABR_PRIV_HYPER;
>>> >> +	else
>>> >> +		addr |= CIABR_PRIV_SUPER;
>>> >> +	write_ciabr(addr);
>>> >> +}
>>> >> +
>>> >> +static void clear_ciabr(void)
>>> >> +{
>>> >> +	if (cpu_has_feature(CPU_FTR_ARCH_207S))
>>> >> +		write_ciabr(0);
>>> >> +}
>>> >> +
>>> >>  /*
>>> >>   * Disable surveillance (the service processor watchdog function)
>>> >>   * while we are in xmon.
>>> >> @@ -764,6 +795,9 @@ static void insert_cpu_bpts(void)
>>> >>  	if (iabr && cpu_has_feature(CPU_FTR_IABR))
>>> >>  		mtspr(SPRN_IABR, iabr->address
>>> >>  			 | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
>>> >> +
>>> >> +	if (iabr && cpu_has_feature(CPU_FTR_ARCH_207S))
>>> >> +		set_ciabr(iabr->address);
>>> >>  }
>>> >>  
>>> >>  static void remove_bpts(void)
>>> >> @@ -791,6 +825,7 @@ static void remove_cpu_bpts(void)
>>> >>  	hw_breakpoint_disable();
>>> >>  	if (cpu_has_feature(CPU_FTR_IABR))
>>> >>  		mtspr(SPRN_IABR, 0);
>>> >> +	clear_ciabr();
>>> >>  }
>>> >>  
>>> >>  /* Command interpreting routine */
>>> >> @@ -1124,7 +1159,7 @@ static char *breakpoint_help_string =
>>> >>      "b <addr> [cnt]   set breakpoint at given instr addr\n"
>>> >>      "bc               clear all breakpoints\n"
>>> >>      "bc <n/addr>      clear breakpoint number n or at addr\n"
>>> >> -    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64 only)\n"
>>> >> +    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64/POWER8 only)\n"
>>> >>      "bd <addr> [cnt]  set hardware data breakpoint\n"
>>> >>      "";
>>> >>  
>>> >> @@ -1163,11 +1198,20 @@ bpt_cmds(void)
>>> >>  		break;
>>> >>  
>>> >>  	case 'i':	/* bi - hardware instr breakpoint */
>>> >> -		if (!cpu_has_feature(CPU_FTR_IABR)) {
>>> >> +		if (!cpu_has_feature(CPU_FTR_IABR) && !cpu_has_feature(CPU_FTR_ARCH_207S)) {
>>> >>  			printf("Hardware instruction breakpoint "
>>> >>  			       "not supported on this cpu\n");
>>> >>  			break;
>>> >>  		}
>>> >> +
>>> >> +		if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
>>> >> +			if (ciabr_used) {
>>> >> +				printf("One instruction breakpoint "
>>> >> +					"possible with CIABR\n");
>>> >> +				break;
>>> >> +			}
>> > 
>> > We don't seem to do that with iabr ? Why keep ciabr different 

> Right now with the IABR implementation if we try to set hardware instruction
> breakpoint while one is already there, it just get overridden with the new
> address without complaining. I thought with this at least for CIABR cases,
> it will complain about it and require the user to clear the breakpoint
> explicitly before allowing a new breakpoint. Okay will remove this.
>  

I tried removing the "ciabr_used" variable and all related checks/assignments on it.
Then I was able to add all these three address as hardware instruction breakpoint
and xmon never complained that it might override the existing actual checkpoint
implemented with CIABR. It accepted all the three addresses which can later be
listed as below.

0:mon> b
   type            address
 1 inst   c0000000000830d0  .power_pmu_add+0x0/0x2e0                                                
 2 inst   c000000000084690  .power_pmu_del+0x0/0x3a0                                   
 3 inst   c000000000085060  .perf_event_interrupt+0x0/0x480

But in reality, only perf_event_interrupt function's address got written into CIABR
register and got triggered with the workload. I dont have a system which has IABR
support to test it's behaviour for this situation. But this does not sound okay,
we should explicitly inform the user that the hardware instruction breakpoint has
been overridden with the latest command or reject the attempt. Looking for some
suggestions in this regard. Thank you.
Anshuman Khandual June 12, 2014, 8:19 a.m. UTC | #7
On 06/04/2014 02:18 PM, Anshuman Khandual wrote:
> On 06/03/2014 11:33 AM, Anshuman Khandual wrote:
>> On 05/30/2014 07:12 PM, Aneesh Kumar K.V wrote:
>>>> Anshuman Khandual <khandual@linux.vnet.ibm.com> writes:
>>>>
>>>>>> This patch enables support for hardware instruction breakpoints on POWER8 with
>>>>>> the help of a new register called CIABR (Completed Instruction Address Breakpoint
>>>>>> Register). With this patch, single hardware instruction breakpoint can be added
>>>>>> and cleared during any active xmon debug session. This hardware based instruction
>>>>>> breakpoint mechanism works correctly along with the existing TRAP based instruction
>>>>>> breakpoints available on xmon. Example usage as follows.
>>>>>>
>>>>>> (A) Start xmon:
>>>>>> $echo x > /proc/sysrq-trigger
>>>>>> SysRq : Entering xmon
>>>>>> cpu 0x0: Vector: 0  at [c000001f6c67f960]
>>>>>>     pc: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>>>>>>     lr: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>>>>>>     sp: c000001f6c67fac0
>>>>>>    msr: 9000000000009032
>>>>>>   current = 0xc000001f6e709ac0
>>>>>>   paca    = 0xc00000000fffa000   softe: 0        irq_happened: 0x00
>>>>>>     pid   = 3250, comm = bash
>>>>>> enter ? for help
>>>>>> 0:mon> b
>>>>>>    type            address
>>>>>>
>>>>>> (B) Set the breakpoint:
>>>>>> 0:mon> ls .power_pmu_add
>>>>>> .power_pmu_add: c000000000078f50
>>>>>> 0:mon> bi c000000000078f50
>>>>>> 0:mon> b
>>>>>>    type            address
>>>>>>  1 inst   c000000000078f50  .power_pmu_add+0x0/0x2e0
>>>>>> 0:mon> ls .perf_event_interrupt
>>>>>> .perf_event_interrupt: c00000000007aee0
>>>>>> 0:mon> bi c00000000007aee0
>>>>>> One instruction breakpoint possible with CIABR
>>>>>> 0:mon> x
>>>>>>
>>>>>> (C) Run the workload (with the breakpoint):
>>>>>> $./perf record ls
>>>>>> cpu 0x2: Vector: d00 (Single Step) at [c000001f718133a0]
>>>>>>     pc: c000000000078f54: .power_pmu_add+0x4/0x2e0
>>>>>>     lr: c000000000155be0: .event_sched_in+0x90/0x1d0
>>>>>>     sp: c000001f71813620
>>>>>>    msr: 9000000040109032
>>>>>>   current = 0xc000001f6ce30000
>>>>>>   paca    = 0xc00000000fffa600   softe: 0        irq_happened: 0x01
>>>>>>     pid   = 3270, comm = ls
>>>>>>         std     r22,-80(r1)
>>>>>> enter ? for help
>>>>>>
>>>>>> (D) Clear the breakpoint:
>>>>>> 2:mon> bc
>>>>>> All breakpoints cleared
>>>>>> 2:mon> x
>>>>>> [ perf record: Woken up 1 times to write data ]
>>>>>> [ perf record: Captured and wrote 0.002 MB perf.data (~66 samples) ]
>>>>>>
>>>>>> (E) Run the workload again (without any breakpoints):
>>>>>> $./perf record ls
>>>>>> [ perf record: Woken up 1 times to write data ]
>>>>>> [ perf record: Captured and wrote 0.001 MB perf.data (~61 samples) ]
>>>>>>
>>>>>> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>>>>>> ---
>>>>>>  arch/powerpc/xmon/xmon.c | 62 ++++++++++++++++++++++++++++++++++++++++++++----
>>>>>>  1 file changed, 58 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>>>>> index 3fd1d9a..f74ec83 100644
>>>>>> --- a/arch/powerpc/xmon/xmon.c
>>>>>> +++ b/arch/powerpc/xmon/xmon.c
>>>>>> @@ -48,6 +48,7 @@
>>>>>>  #ifdef CONFIG_PPC64
>>>>>>  #include <asm/hvcall.h>
>>>>>>  #include <asm/paca.h>
>>>>>> +#include <asm/plpar_wrappers.h>
>>>>>>  #endif
>>>>>>  
>>>>>>  #include "nonstdio.h"
>>>>>> @@ -89,6 +90,7 @@ struct bpt {
>>>>>>  /* Bits in bpt.enabled */
>>>>>>  #define BP_IABR_TE	1		/* IABR translation enabled */
>>>>>>  #define BP_IABR		2
>>>>>> +#define BP_CIABR	4
>>>>>>  #define BP_TRAP		8
>>>>>>  #define BP_DABR		0x10
>>>>>>  
>>>>>> @@ -97,6 +99,7 @@ static struct bpt bpts[NBPTS];
>>>>>>  static struct bpt dabr;
>>>>>>  static struct bpt *iabr;
>>>>>>  static unsigned bpinstr = 0x7fe00008;	/* trap */
>>>>>> +static bool ciabr_used = false;		/* CIABR instruction breakpoint */
>>>>>>  
>>>>>>  #define BP_NUM(bp)	((bp) - bpts + 1)
>>>>>>  
>>>>>> @@ -269,6 +272,34 @@ static inline void cinval(void *p)
>>>>>>  	asm volatile ("dcbi 0,%0; icbi 0,%0" : : "r" (p));
>>>>>>  }
>>>>>>  
>>>>>> +static void write_ciabr(unsigned long ciabr)
>>>>>> +{
>>>>>> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
>>>>>> +		mtspr(SPRN_CIABR, ciabr);
>>>>>> +		return;
>>>>>> +	}
>>>>>> +
>>>>>> +#ifdef CONFIG_PPC64
>>>>>> +	plapr_set_ciabr(ciabr);
>>>>>> +#endif
>>>>>> +}
>>>>>> +
>>>>>> +static void set_ciabr(unsigned long addr)
>>>>>> +{
>>>>>> +	addr &= ~CIABR_PRIV;
>>>>>> +	if (cpu_has_feature(CPU_FTR_HVMODE))
>>>>>> +		addr |= CIABR_PRIV_HYPER;
>>>>>> +	else
>>>>>> +		addr |= CIABR_PRIV_SUPER;
>>>>>> +	write_ciabr(addr);
>>>>>> +}
>>>>>> +
>>>>>> +static void clear_ciabr(void)
>>>>>> +{
>>>>>> +	if (cpu_has_feature(CPU_FTR_ARCH_207S))
>>>>>> +		write_ciabr(0);
>>>>>> +}
>>>>>> +
>>>>>>  /*
>>>>>>   * Disable surveillance (the service processor watchdog function)
>>>>>>   * while we are in xmon.
>>>>>> @@ -764,6 +795,9 @@ static void insert_cpu_bpts(void)
>>>>>>  	if (iabr && cpu_has_feature(CPU_FTR_IABR))
>>>>>>  		mtspr(SPRN_IABR, iabr->address
>>>>>>  			 | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
>>>>>> +
>>>>>> +	if (iabr && cpu_has_feature(CPU_FTR_ARCH_207S))
>>>>>> +		set_ciabr(iabr->address);
>>>>>>  }
>>>>>>  
>>>>>>  static void remove_bpts(void)
>>>>>> @@ -791,6 +825,7 @@ static void remove_cpu_bpts(void)
>>>>>>  	hw_breakpoint_disable();
>>>>>>  	if (cpu_has_feature(CPU_FTR_IABR))
>>>>>>  		mtspr(SPRN_IABR, 0);
>>>>>> +	clear_ciabr();
>>>>>>  }
>>>>>>  
>>>>>>  /* Command interpreting routine */
>>>>>> @@ -1124,7 +1159,7 @@ static char *breakpoint_help_string =
>>>>>>      "b <addr> [cnt]   set breakpoint at given instr addr\n"
>>>>>>      "bc               clear all breakpoints\n"
>>>>>>      "bc <n/addr>      clear breakpoint number n or at addr\n"
>>>>>> -    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64 only)\n"
>>>>>> +    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64/POWER8 only)\n"
>>>>>>      "bd <addr> [cnt]  set hardware data breakpoint\n"
>>>>>>      "";
>>>>>>  
>>>>>> @@ -1163,11 +1198,20 @@ bpt_cmds(void)
>>>>>>  		break;
>>>>>>  
>>>>>>  	case 'i':	/* bi - hardware instr breakpoint */
>>>>>> -		if (!cpu_has_feature(CPU_FTR_IABR)) {
>>>>>> +		if (!cpu_has_feature(CPU_FTR_IABR) && !cpu_has_feature(CPU_FTR_ARCH_207S)) {
>>>>>>  			printf("Hardware instruction breakpoint "
>>>>>>  			       "not supported on this cpu\n");
>>>>>>  			break;
>>>>>>  		}
>>>>>> +
>>>>>> +		if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
>>>>>> +			if (ciabr_used) {
>>>>>> +				printf("One instruction breakpoint "
>>>>>> +					"possible with CIABR\n");
>>>>>> +				break;
>>>>>> +			}
>>>>
>>>> We don't seem to do that with iabr ? Why keep ciabr different 
> 
>> Right now with the IABR implementation if we try to set hardware instruction
>> breakpoint while one is already there, it just get overridden with the new
>> address without complaining. I thought with this at least for CIABR cases,
>> it will complain about it and require the user to clear the breakpoint
>> explicitly before allowing a new breakpoint. Okay will remove this.
>>  
> 
> I tried removing the "ciabr_used" variable and all related checks/assignments on it.
> Then I was able to add all these three address as hardware instruction breakpoint
> and xmon never complained that it might override the existing actual checkpoint
> implemented with CIABR. It accepted all the three addresses which can later be
> listed as below.
> 
> 0:mon> b
>    type            address
>  1 inst   c0000000000830d0  .power_pmu_add+0x0/0x2e0                                                
>  2 inst   c000000000084690  .power_pmu_del+0x0/0x3a0                                   
>  3 inst   c000000000085060  .perf_event_interrupt+0x0/0x480
> 
> But in reality, only perf_event_interrupt function's address got written into CIABR
> register and got triggered with the workload. I dont have a system which has IABR
> support to test it's behaviour for this situation. But this does not sound okay,
> we should explicitly inform the user that the hardware instruction breakpoint has
> been overridden with the latest command or reject the attempt. Looking for some
> suggestions in this regard. Thank you.

Hey Ben/MPE,

Any suggestions on this ^^^^^^^^^^^^ ?
diff mbox

Patch

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 3fd1d9a..f74ec83 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -48,6 +48,7 @@ 
 #ifdef CONFIG_PPC64
 #include <asm/hvcall.h>
 #include <asm/paca.h>
+#include <asm/plpar_wrappers.h>
 #endif
 
 #include "nonstdio.h"
@@ -89,6 +90,7 @@  struct bpt {
 /* Bits in bpt.enabled */
 #define BP_IABR_TE	1		/* IABR translation enabled */
 #define BP_IABR		2
+#define BP_CIABR	4
 #define BP_TRAP		8
 #define BP_DABR		0x10
 
@@ -97,6 +99,7 @@  static struct bpt bpts[NBPTS];
 static struct bpt dabr;
 static struct bpt *iabr;
 static unsigned bpinstr = 0x7fe00008;	/* trap */
+static bool ciabr_used = false;		/* CIABR instruction breakpoint */
 
 #define BP_NUM(bp)	((bp) - bpts + 1)
 
@@ -269,6 +272,34 @@  static inline void cinval(void *p)
 	asm volatile ("dcbi 0,%0; icbi 0,%0" : : "r" (p));
 }
 
+static void write_ciabr(unsigned long ciabr)
+{
+	if (cpu_has_feature(CPU_FTR_HVMODE)) {
+		mtspr(SPRN_CIABR, ciabr);
+		return;
+	}
+
+#ifdef CONFIG_PPC64
+	plapr_set_ciabr(ciabr);
+#endif
+}
+
+static void set_ciabr(unsigned long addr)
+{
+	addr &= ~CIABR_PRIV;
+	if (cpu_has_feature(CPU_FTR_HVMODE))
+		addr |= CIABR_PRIV_HYPER;
+	else
+		addr |= CIABR_PRIV_SUPER;
+	write_ciabr(addr);
+}
+
+static void clear_ciabr(void)
+{
+	if (cpu_has_feature(CPU_FTR_ARCH_207S))
+		write_ciabr(0);
+}
+
 /*
  * Disable surveillance (the service processor watchdog function)
  * while we are in xmon.
@@ -764,6 +795,9 @@  static void insert_cpu_bpts(void)
 	if (iabr && cpu_has_feature(CPU_FTR_IABR))
 		mtspr(SPRN_IABR, iabr->address
 			 | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
+
+	if (iabr && cpu_has_feature(CPU_FTR_ARCH_207S))
+		set_ciabr(iabr->address);
 }
 
 static void remove_bpts(void)
@@ -791,6 +825,7 @@  static void remove_cpu_bpts(void)
 	hw_breakpoint_disable();
 	if (cpu_has_feature(CPU_FTR_IABR))
 		mtspr(SPRN_IABR, 0);
+	clear_ciabr();
 }
 
 /* Command interpreting routine */
@@ -1124,7 +1159,7 @@  static char *breakpoint_help_string =
     "b <addr> [cnt]   set breakpoint at given instr addr\n"
     "bc               clear all breakpoints\n"
     "bc <n/addr>      clear breakpoint number n or at addr\n"
-    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64 only)\n"
+    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64/POWER8 only)\n"
     "bd <addr> [cnt]  set hardware data breakpoint\n"
     "";
 
@@ -1163,11 +1198,20 @@  bpt_cmds(void)
 		break;
 
 	case 'i':	/* bi - hardware instr breakpoint */
-		if (!cpu_has_feature(CPU_FTR_IABR)) {
+		if (!cpu_has_feature(CPU_FTR_IABR) && !cpu_has_feature(CPU_FTR_ARCH_207S)) {
 			printf("Hardware instruction breakpoint "
 			       "not supported on this cpu\n");
 			break;
 		}
+
+		if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
+			if (ciabr_used) {
+				printf("One instruction breakpoint "
+					"possible with CIABR\n");
+				break;
+			}
+		}
+
 		if (iabr) {
 			iabr->enabled &= ~(BP_IABR | BP_IABR_TE);
 			iabr = NULL;
@@ -1178,7 +1222,12 @@  bpt_cmds(void)
 			break;
 		bp = new_breakpoint(a);
 		if (bp != NULL) {
-			bp->enabled |= BP_IABR | BP_IABR_TE;
+			if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
+				bp->enabled |= BP_CIABR;
+				ciabr_used = true;
+			}
+			else
+				bp->enabled |= BP_IABR | BP_IABR_TE;
 			iabr = bp;
 		}
 		break;
@@ -1191,6 +1240,7 @@  bpt_cmds(void)
 				bpts[i].enabled = 0;
 			iabr = NULL;
 			dabr.enabled = 0;
+			ciabr_used = false;
 			printf("All breakpoints cleared\n");
 			break;
 		}
@@ -1207,6 +1257,9 @@  bpt_cmds(void)
 			}
 		}
 
+		if (bp->enabled & BP_CIABR)
+			ciabr_used = false;
+
 		printf("Cleared breakpoint %lx (", BP_NUM(bp));
 		xmon_print_symbol(bp->address, " ", ")\n");
 		bp->enabled = 0;
@@ -1235,7 +1288,8 @@  bpt_cmds(void)
 				if (!bp->enabled)
 					continue;
 				printf("%2x %s   ", BP_NUM(bp),
-				    (bp->enabled & BP_IABR)? "inst": "trap");
+					(bp->enabled & (BP_IABR | BP_CIABR))?
+				       				"inst": "trap");
 				xmon_print_symbol(bp->address, "  ", "\n");
 			}
 			break;