[v2] powerpc: Disable DAWR on POWER9

Message ID 20180320062224.10666-1-mikey@neuling.org
State Superseded
Headers show
Series
  • [v2] powerpc: Disable DAWR on POWER9
Related show

Commit Message

Michael Neuling March 20, 2018, 6:22 a.m.
Using the DAWR on POWER9 can cause xstops, hence we need to disable
it.

The current CPU_FTR for DAWR is a bit messy. Despite having
CPU_FTR_DAWR, currently we assume DAWR exists in the KVM code based on
CPU_FTR_ARCH_207. In other places we assume DAWR exists if
CPU_FTR_DAWR is set.

This attempts to clear up the situation by always using CPU_FTR_DAWR
before setting the DAWR (to a non-zero value).

DAWR has 5 different ways of being set from userspace. ptrace,
h_set_mode, h_set_mode(DAWR), h_set_dabr(), kvmppc_set_one_reg() and
xmon.

For ptrace, we now advertise zero breakpoints on POWER9 via the
PPC_PTRACE_GETHWDBGINFO call. This results in GDB falling back to
software emulation of the watchpoint (which is slow).

h_set_mode() and h_set_dabr() will now return an error to the guest
when on a POWER9 host. Current Linux guests ignore this error, so they
will silently not get the DAWR (sigh). The same error codes are being
used by POWERVM in this case.

kvmppc_set_one_reg() will store the value in the vcpu but won't
actually set it on POWER9 hardware. This is done so we don't break
migration from P8 to P9, at the cost of silently losing the DAWR on
the migration. This is not ideal but hopefully the best overall
solution. This approach has been acked by paulus.

For xmon, the 'bd' command will return an error on P9.

Thanks to Pedro Franco de Carvalho for the initial version of this.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
v2:
  Move to CPU feature quirk from Nick's
  dbginfo.num_data_bps = 0 if !breakpoint_available() from Pedro
---
 arch/powerpc/include/asm/cputable.h     |  5 ++---
 arch/powerpc/include/asm/debug.h        |  1 +
 arch/powerpc/include/asm/hvcall.h       |  1 +
 arch/powerpc/kernel/dt_cpu_ftrs.c       |  3 +++
 arch/powerpc/kernel/hw_breakpoint.c     |  3 +++
 arch/powerpc/kernel/process.c           | 11 +++++++++++
 arch/powerpc/kernel/ptrace.c            | 18 +++++++++++++++---
 arch/powerpc/kvm/book3s_hv.c            |  2 ++
 arch/powerpc/kvm/book3s_hv_rmhandlers.S | 12 ++++++++++++
 arch/powerpc/xmon/xmon.c                |  5 +++++
 10 files changed, 55 insertions(+), 6 deletions(-)

Comments

Pedro Franco de Carvalho March 20, 2018, 9:02 p.m. | #1
I noticed that __set_breakpoint in process.c calls set_dabr if
CPU_FTR_DAWR is not set. One case which could cause this to happen is if
perf events are not configured (and CONFIG_HW_BREAKPOINT is not set),
and someone calls ptrace with SET_DEBUGREG and data with a non-zero
address and 0 in the RDWR bits. In this case set_bp will be false,
thread.hw_brk will be set, and __switch_to will call __set_breakpoint,
which will call set_dabr. I'm not sure what happens if mtspr with DARN
is called on a P8.

With respect to setting .features, from the V1 discussion:

> Yeah, currently you'll get PPC_DEBUG_FEATURE_DATA_BP_RANGE and no
> PPC_DEBUG_FEATURE_DATA_BP_DAWR. 
>
> If I set .features = 0 when !breakpoint_enabled() (as well as setting
> num_data_bps = 0 above), GDB seems to not like it when I run the program on P9:
>
>     (gdb) r
>     Starting program: /bin/true 
>     Warning:
>     Could not insert hardware watchpoint 1.
>     Could not insert hardware breakpoints:
>     You may have requested too many hardware breakpoints/watchpoints.
>     (gdb)
>
> So I think I'll leave this bits as I had in version 1 of the patch.

This would make existing GDB releases work nicely, but it should
probably be fixed in GDB.

When .features is 0, GDB ignores the rest of the info returned by
GETHWDBGINFO and assumes there will be at least one watchpoint register,
which leads to the behavior you described, and doesn't seem correct
(on the GDB side).

Thanks!
Pedro

Michael Neuling <mikey@neuling.org> writes:

> Using the DAWR on POWER9 can cause xstops, hence we need to disable
> it.
>
> The current CPU_FTR for DAWR is a bit messy. Despite having
> CPU_FTR_DAWR, currently we assume DAWR exists in the KVM code based on
> CPU_FTR_ARCH_207. In other places we assume DAWR exists if
> CPU_FTR_DAWR is set.
>
> This attempts to clear up the situation by always using CPU_FTR_DAWR
> before setting the DAWR (to a non-zero value).
>
> DAWR has 5 different ways of being set from userspace. ptrace,
> h_set_mode, h_set_mode(DAWR), h_set_dabr(), kvmppc_set_one_reg() and
> xmon.
>
> For ptrace, we now advertise zero breakpoints on POWER9 via the
> PPC_PTRACE_GETHWDBGINFO call. This results in GDB falling back to
> software emulation of the watchpoint (which is slow).
>
> h_set_mode() and h_set_dabr() will now return an error to the guest
> when on a POWER9 host. Current Linux guests ignore this error, so they
> will silently not get the DAWR (sigh). The same error codes are being
> used by POWERVM in this case.
>
> kvmppc_set_one_reg() will store the value in the vcpu but won't
> actually set it on POWER9 hardware. This is done so we don't break
> migration from P8 to P9, at the cost of silently losing the DAWR on
> the migration. This is not ideal but hopefully the best overall
> solution. This approach has been acked by paulus.
>
> For xmon, the 'bd' command will return an error on P9.
>
> Thanks to Pedro Franco de Carvalho for the initial version of this.
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
> v2:
>   Move to CPU feature quirk from Nick's
>   dbginfo.num_data_bps = 0 if !breakpoint_available() from Pedro
> ---
>  arch/powerpc/include/asm/cputable.h     |  5 ++---
>  arch/powerpc/include/asm/debug.h        |  1 +
>  arch/powerpc/include/asm/hvcall.h       |  1 +
>  arch/powerpc/kernel/dt_cpu_ftrs.c       |  3 +++
>  arch/powerpc/kernel/hw_breakpoint.c     |  3 +++
>  arch/powerpc/kernel/process.c           | 11 +++++++++++
>  arch/powerpc/kernel/ptrace.c            | 18 +++++++++++++++---
>  arch/powerpc/kvm/book3s_hv.c            |  2 ++
>  arch/powerpc/kvm/book3s_hv_rmhandlers.S | 12 ++++++++++++
>  arch/powerpc/xmon/xmon.c                |  5 +++++
>  10 files changed, 55 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> index a2c5c95882..79a206bcd4 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -463,9 +463,8 @@ static inline void cpu_feature_keys_init(void) { }
>  	    CPU_FTR_DSCR | CPU_FTR_SAO  | \
>  	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
>  	    CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
> -	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
> -	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \
> -	    CPU_FTR_PKEY)
> +	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
> +	    CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | CPU_FTR_PKEY)
>  #define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \
>  			     (~CPU_FTR_SAO))
>  #define CPU_FTRS_POWER9_DD2_0 CPU_FTRS_POWER9
> diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
> index fc97404de0..36634a22f3 100644
> --- a/arch/powerpc/include/asm/debug.h
> +++ b/arch/powerpc/include/asm/debug.h
> @@ -47,6 +47,7 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
>
>  void set_breakpoint(struct arch_hw_breakpoint *brk);
>  void __set_breakpoint(struct arch_hw_breakpoint *brk);
> +bool breakpoint_available(void);
>  #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>  extern void do_send_trap(struct pt_regs *regs, unsigned long address,
>  			 unsigned long error_code, int brkpt);
> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> index eca3f9c689..e87d465af4 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -88,6 +88,7 @@
>  #define H_P8		-61
>  #define H_P9		-62
>  #define H_TOO_BIG	-64
> +#define H_UNSUPPORTED	-67
>  #define H_OVERLAP	-68
>  #define H_INTERRUPT	-69
>  #define H_BAD_DATA	-70
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index 945e2c29ad..941a5fbd8d 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -709,6 +709,9 @@ static __init void cpufeatures_cpu_quirks(void)
>  		cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD1;
>  	else if ((version & 0xffffefff) == 0x004e0201)
>  		cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
> +
> +	if ((version & 0xffff0000) == 0x004e0000)
> +		cur_cpu_spec->cpu_features &= ~(CPU_FTR_DAWR);
>  }
>
>  static void __init cpufeatures_setup_finished(void)
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
> index 53b9c1dfd7..bee3f702c5 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -33,6 +33,7 @@
>  #include <asm/hw_breakpoint.h>
>  #include <asm/processor.h>
>  #include <asm/sstep.h>
> +#include <asm/debug.h>
>  #include <linux/uaccess.h>
>
>  /*
> @@ -171,6 +172,8 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
>  	 * HW_BREAKPOINT_ALIGN by rounding off to the lower address, the
>  	 * 'symbolsize' should satisfy the check below.
>  	 */
> +	if (!breakpoint_available())
> +		return -ENODEV;
>  	length_max = 8; /* DABR */
>  	if (cpu_has_feature(CPU_FTR_DAWR)) {
>  		length_max = 512 ; /* 64 doublewords */
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 1738c4127b..c51d7133bf 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -827,6 +827,17 @@ void set_breakpoint(struct arch_hw_breakpoint *brk)
>  	preempt_enable();
>  }
>
> +/* Check if DAWR or DABR hardware */
> +bool breakpoint_available(void)
> +{
> +	if (cpu_has_feature(CPU_FTR_DAWR))
> +		return true; /* POWER8 DAWR */
> +	if (!cpu_has_feature(CPU_FTR_ARCH_207S))
> +		/* DABR: Everything but POWER8 and POWER9 */
> +		return true;
> +	return false;
> +}
> +
>  #ifdef CONFIG_PPC64
>  DEFINE_PER_CPU(struct cpu_usage, cpu_usage_array);
>  #endif
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index ca72d7391d..92e03418fd 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -41,6 +41,7 @@
>  #include <asm/switch_to.h>
>  #include <asm/tm.h>
>  #include <asm/asm-prototypes.h>
> +#include <asm/debug.h>
>
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/syscalls.h>
> @@ -2378,6 +2379,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
>  	struct perf_event_attr attr;
>  #endif /* CONFIG_HAVE_HW_BREAKPOINT */
>  #ifndef CONFIG_PPC_ADV_DEBUG_REGS
> +	bool set_bp = true;
>  	struct arch_hw_breakpoint hw_brk;
>  #endif
>
> @@ -2411,9 +2413,10 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
>  	hw_brk.address = data & (~HW_BRK_TYPE_DABR);
>  	hw_brk.type = (data & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL;
>  	hw_brk.len = 8;
> +	set_bp = (data) && (hw_brk.type & HW_BRK_TYPE_RDWR);
>  #ifdef CONFIG_HAVE_HW_BREAKPOINT
>  	bp = thread->ptrace_bps[0];
> -	if ((!data) || !(hw_brk.type & HW_BRK_TYPE_RDWR)) {
> +	if (!set_bp) {
>  		if (bp) {
>  			unregister_hw_breakpoint(bp);
>  			thread->ptrace_bps[0] = NULL;
> @@ -2450,7 +2453,10 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
>  		return PTR_ERR(bp);
>  	}
>
> -#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> +#else /* !CONFIG_HAVE_HW_BREAKPOINT */
> +	if (set_bp && (!breakpoint_available()))
> +		return -ENODEV;
> +#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
>  	task->thread.hw_brk = hw_brk;
>  #else /* CONFIG_PPC_ADV_DEBUG_REGS */
>  	/* As described above, it was assumed 3 bits were passed with the data
> @@ -2904,6 +2910,9 @@ static long ppc_set_hwdebug(struct task_struct *child,
>  	if (child->thread.hw_brk.address)
>  		return -ENOSPC;
>
> +	if (!breakpoint_available())
> +		return -ENODEV;
> +
>  	child->thread.hw_brk = brk;
>
>  	return 1;
> @@ -3052,7 +3061,10 @@ long arch_ptrace(struct task_struct *child, long request,
>  #endif
>  #else /* !CONFIG_PPC_ADV_DEBUG_REGS */
>  		dbginfo.num_instruction_bps = 0;
> -		dbginfo.num_data_bps = 1;
> +		if (breakpoint_available())
> +			dbginfo.num_data_bps = 1;
> +		else
> +			dbginfo.num_data_bps = 0;
>  		dbginfo.num_condition_regs = 0;
>  #ifdef CONFIG_PPC64
>  		dbginfo.data_bp_alignment = 8;
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 89707354c2..43ff667962 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -741,6 +741,8 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
>  	case H_SET_MODE_RESOURCE_SET_DAWR:
>  		if (!kvmppc_power8_compatible(vcpu))
>  			return H_P2;
> +		if (!breakpoint_available())
> +			return H_P2;
>  		if (mflags)
>  			return H_UNSUPPORTED_FLAG_START;
>  		if (value2 & DABRX_HYP)
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index f31f357b8c..1036aefe56 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -886,8 +886,14 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
>  	ld	r6, VCPU_DAWRX(r4)
>  	ld	r7, VCPU_CIABR(r4)
>  	ld	r8, VCPU_TAR(r4)
> +	/*
> +	 * Handle broken DAWR case by not writing it. This means we
> +	 * can still store the DAWR register for migration.
> +	 */
> +BEGIN_FTR_SECTION
>  	mtspr	SPRN_DAWR, r5
>  	mtspr	SPRN_DAWRX, r6
> +END_FTR_SECTION_IFSET(CPU_FTR_DAWR)
>  	mtspr	SPRN_CIABR, r7
>  	mtspr	SPRN_TAR, r8
>  	ld	r5, VCPU_IC(r4)
> @@ -1834,6 +1840,10 @@ BEGIN_FTR_SECTION
>  	ld	r6, STACK_SLOT_DAWR(r1)
>  	ld	r7, STACK_SLOT_DAWRX(r1)
>  	mtspr	SPRN_CIABR, r5
> +	/*
> +	 * If the DAWR doesn't work, it's ok to write these here as
> +	 * this value should always be zero
> +	*/
>  	mtspr	SPRN_DAWR, r6
>  	mtspr	SPRN_DAWRX, r7
>  END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> @@ -2512,8 +2522,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
>  	clrrdi	r4, r4, 3
>  	std	r4, VCPU_DAWR(r3)
>  	std	r5, VCPU_DAWRX(r3)
> +BEGIN_FTR_SECTION
>  	mtspr	SPRN_DAWR, r4
>  	mtspr	SPRN_DAWRX, r5
> +END_FTR_SECTION_IFSET(CPU_FTR_DAWR)
>  	li	r3, 0
>  	blr
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 82e1a3ee6e..1f85b0ddd2 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -1297,6 +1297,11 @@ bpt_cmds(void)
>  	static const char badaddr[] = "Only kernel addresses are permitted for breakpoints\n";
>  	int mode;
>  	case 'd':	/* bd - hardware data breakpoint */
> +		if (!breakpoint_available()) {
> +			printf("Hardware data breakpoint "
> +			       "not supported on this cpu\n");
> +			break;
> +		}
>  		mode = 7;
>  		cmd = inchar();
>  		if (cmd == 'r')
> -- 
> 2.14.1

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Neuling March 22, 2018, 4:53 a.m. | #2
On Tue, 2018-03-20 at 18:02 -0300, Pedro Franco de Carvalho wrote:
> I noticed that __set_breakpoint in process.c calls set_dabr if
> CPU_FTR_DAWR is not set. One case which could cause this to happen is if
> perf events are not configured (and CONFIG_HW_BREAKPOINT is not set),
> and someone calls ptrace with SET_DEBUGREG and data with a non-zero
> address and 0 in the RDWR bits. In this case set_bp will be false,
> thread.hw_brk will be set, and __switch_to will call __set_breakpoint,
> which will call set_dabr. I'm not sure what happens if mtspr with DARN
> is called on a P8.

This last sentence should read "I'm not sure what happens if mtspr with DABR is
called on a P9.", right?

If so, then it should be a nop, but we should fix it to make it clearer in the
code.

> > Yeah, currently you'll get PPC_DEBUG_FEATURE_DATA_BP_RANGE and no
> > PPC_DEBUG_FEATURE_DATA_BP_DAWR. 
> > 
> > If I set .features = 0 when !breakpoint_enabled() (as well as setting
> > num_data_bps = 0 above), GDB seems to not like it when I run the program on
> > P9:
> > 
> >     (gdb) r
> >     Starting program: /bin/true 
> >     Warning:
> >     Could not insert hardware watchpoint 1.
> >     Could not insert hardware breakpoints:
> >     You may have requested too many hardware breakpoints/watchpoints.
> >     (gdb)
> > 
> > So I think I'll leave this bits as I had in version 1 of the patch.
> 
> This would make existing GDB releases work nicely, but it should
> probably be fixed in GDB.
> 
> When .features is 0, GDB ignores the rest of the info returned by
> GETHWDBGINFO and assumes there will be at least one watchpoint register,
> which leads to the behavior you described, and doesn't seem correct
> (on the GDB side).

Yeah, agreed

Mikey
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pedro Franco de Carvalho March 22, 2018, 12:35 p.m. | #3
Michael Neuling <mikey@neuling.org> writes:

> This last sentence should read "I'm not sure what happens if mtspr with DABR is
> called on a P9.", right?

Yes, that's it!

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index a2c5c95882..79a206bcd4 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -463,9 +463,8 @@  static inline void cpu_feature_keys_init(void) { }
 	    CPU_FTR_DSCR | CPU_FTR_SAO  | \
 	    CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
 	    CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
-	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
-	    CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | \
-	    CPU_FTR_PKEY)
+	    CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
+	    CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | CPU_FTR_PKEY)
 #define CPU_FTRS_POWER9_DD1 ((CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1) & \
 			     (~CPU_FTR_SAO))
 #define CPU_FTRS_POWER9_DD2_0 CPU_FTRS_POWER9
diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
index fc97404de0..36634a22f3 100644
--- a/arch/powerpc/include/asm/debug.h
+++ b/arch/powerpc/include/asm/debug.h
@@ -47,6 +47,7 @@  static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
 
 void set_breakpoint(struct arch_hw_breakpoint *brk);
 void __set_breakpoint(struct arch_hw_breakpoint *brk);
+bool breakpoint_available(void);
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
 extern void do_send_trap(struct pt_regs *regs, unsigned long address,
 			 unsigned long error_code, int brkpt);
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index eca3f9c689..e87d465af4 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -88,6 +88,7 @@ 
 #define H_P8		-61
 #define H_P9		-62
 #define H_TOO_BIG	-64
+#define H_UNSUPPORTED	-67
 #define H_OVERLAP	-68
 #define H_INTERRUPT	-69
 #define H_BAD_DATA	-70
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 945e2c29ad..941a5fbd8d 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -709,6 +709,9 @@  static __init void cpufeatures_cpu_quirks(void)
 		cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD1;
 	else if ((version & 0xffffefff) == 0x004e0201)
 		cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
+
+	if ((version & 0xffff0000) == 0x004e0000)
+		cur_cpu_spec->cpu_features &= ~(CPU_FTR_DAWR);
 }
 
 static void __init cpufeatures_setup_finished(void)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 53b9c1dfd7..bee3f702c5 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -33,6 +33,7 @@ 
 #include <asm/hw_breakpoint.h>
 #include <asm/processor.h>
 #include <asm/sstep.h>
+#include <asm/debug.h>
 #include <linux/uaccess.h>
 
 /*
@@ -171,6 +172,8 @@  int arch_validate_hwbkpt_settings(struct perf_event *bp)
 	 * HW_BREAKPOINT_ALIGN by rounding off to the lower address, the
 	 * 'symbolsize' should satisfy the check below.
 	 */
+	if (!breakpoint_available())
+		return -ENODEV;
 	length_max = 8; /* DABR */
 	if (cpu_has_feature(CPU_FTR_DAWR)) {
 		length_max = 512 ; /* 64 doublewords */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 1738c4127b..c51d7133bf 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -827,6 +827,17 @@  void set_breakpoint(struct arch_hw_breakpoint *brk)
 	preempt_enable();
 }
 
+/* Check if DAWR or DABR hardware */
+bool breakpoint_available(void)
+{
+	if (cpu_has_feature(CPU_FTR_DAWR))
+		return true; /* POWER8 DAWR */
+	if (!cpu_has_feature(CPU_FTR_ARCH_207S))
+		/* DABR: Everything but POWER8 and POWER9 */
+		return true;
+	return false;
+}
+
 #ifdef CONFIG_PPC64
 DEFINE_PER_CPU(struct cpu_usage, cpu_usage_array);
 #endif
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index ca72d7391d..92e03418fd 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -41,6 +41,7 @@ 
 #include <asm/switch_to.h>
 #include <asm/tm.h>
 #include <asm/asm-prototypes.h>
+#include <asm/debug.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/syscalls.h>
@@ -2378,6 +2379,7 @@  static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
 	struct perf_event_attr attr;
 #endif /* CONFIG_HAVE_HW_BREAKPOINT */
 #ifndef CONFIG_PPC_ADV_DEBUG_REGS
+	bool set_bp = true;
 	struct arch_hw_breakpoint hw_brk;
 #endif
 
@@ -2411,9 +2413,10 @@  static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
 	hw_brk.address = data & (~HW_BRK_TYPE_DABR);
 	hw_brk.type = (data & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL;
 	hw_brk.len = 8;
+	set_bp = (data) && (hw_brk.type & HW_BRK_TYPE_RDWR);
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
 	bp = thread->ptrace_bps[0];
-	if ((!data) || !(hw_brk.type & HW_BRK_TYPE_RDWR)) {
+	if (!set_bp) {
 		if (bp) {
 			unregister_hw_breakpoint(bp);
 			thread->ptrace_bps[0] = NULL;
@@ -2450,7 +2453,10 @@  static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
 		return PTR_ERR(bp);
 	}
 
-#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+#else /* !CONFIG_HAVE_HW_BREAKPOINT */
+	if (set_bp && (!breakpoint_available()))
+		return -ENODEV;
+#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
 	task->thread.hw_brk = hw_brk;
 #else /* CONFIG_PPC_ADV_DEBUG_REGS */
 	/* As described above, it was assumed 3 bits were passed with the data
@@ -2904,6 +2910,9 @@  static long ppc_set_hwdebug(struct task_struct *child,
 	if (child->thread.hw_brk.address)
 		return -ENOSPC;
 
+	if (!breakpoint_available())
+		return -ENODEV;
+
 	child->thread.hw_brk = brk;
 
 	return 1;
@@ -3052,7 +3061,10 @@  long arch_ptrace(struct task_struct *child, long request,
 #endif
 #else /* !CONFIG_PPC_ADV_DEBUG_REGS */
 		dbginfo.num_instruction_bps = 0;
-		dbginfo.num_data_bps = 1;
+		if (breakpoint_available())
+			dbginfo.num_data_bps = 1;
+		else
+			dbginfo.num_data_bps = 0;
 		dbginfo.num_condition_regs = 0;
 #ifdef CONFIG_PPC64
 		dbginfo.data_bp_alignment = 8;
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 89707354c2..43ff667962 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -741,6 +741,8 @@  static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu, unsigned long mflags,
 	case H_SET_MODE_RESOURCE_SET_DAWR:
 		if (!kvmppc_power8_compatible(vcpu))
 			return H_P2;
+		if (!breakpoint_available())
+			return H_P2;
 		if (mflags)
 			return H_UNSUPPORTED_FLAG_START;
 		if (value2 & DABRX_HYP)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index f31f357b8c..1036aefe56 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -886,8 +886,14 @@  END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
 	ld	r6, VCPU_DAWRX(r4)
 	ld	r7, VCPU_CIABR(r4)
 	ld	r8, VCPU_TAR(r4)
+	/*
+	 * Handle broken DAWR case by not writing it. This means we
+	 * can still store the DAWR register for migration.
+	 */
+BEGIN_FTR_SECTION
 	mtspr	SPRN_DAWR, r5
 	mtspr	SPRN_DAWRX, r6
+END_FTR_SECTION_IFSET(CPU_FTR_DAWR)
 	mtspr	SPRN_CIABR, r7
 	mtspr	SPRN_TAR, r8
 	ld	r5, VCPU_IC(r4)
@@ -1834,6 +1840,10 @@  BEGIN_FTR_SECTION
 	ld	r6, STACK_SLOT_DAWR(r1)
 	ld	r7, STACK_SLOT_DAWRX(r1)
 	mtspr	SPRN_CIABR, r5
+	/*
+	 * If the DAWR doesn't work, it's ok to write these here as
+	 * this value should always be zero
+	*/
 	mtspr	SPRN_DAWR, r6
 	mtspr	SPRN_DAWRX, r7
 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
@@ -2512,8 +2522,10 @@  END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
 	clrrdi	r4, r4, 3
 	std	r4, VCPU_DAWR(r3)
 	std	r5, VCPU_DAWRX(r3)
+BEGIN_FTR_SECTION
 	mtspr	SPRN_DAWR, r4
 	mtspr	SPRN_DAWRX, r5
+END_FTR_SECTION_IFSET(CPU_FTR_DAWR)
 	li	r3, 0
 	blr
 
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 82e1a3ee6e..1f85b0ddd2 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1297,6 +1297,11 @@  bpt_cmds(void)
 	static const char badaddr[] = "Only kernel addresses are permitted for breakpoints\n";
 	int mode;
 	case 'd':	/* bd - hardware data breakpoint */
+		if (!breakpoint_available()) {
+			printf("Hardware data breakpoint "
+			       "not supported on this cpu\n");
+			break;
+		}
 		mode = 7;
 		cmd = inchar();
 		if (cmd == 'r')