From patchwork Fri Mar 16 02:54:56 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Neuling X-Patchwork-Id: 886601 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=kvm-ppc-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=neuling.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 402VSP28QJz9sVw for ; Fri, 16 Mar 2018 13:55:17 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932766AbeCPCzP (ORCPT ); Thu, 15 Mar 2018 22:55:15 -0400 Received: from ozlabs.org ([103.22.144.67]:53729 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932605AbeCPCzO (ORCPT ); Thu, 15 Mar 2018 22:55:14 -0400 Received: from localhost.localdomain (localhost [127.0.0.1]) by ozlabs.org (Postfix) with ESMTP id 402VSJ3zYVz9sCc; Fri, 16 Mar 2018 13:55:12 +1100 (AEDT) Received: by localhost.localdomain (Postfix, from userid 1000) id 54B02EE78BC; Fri, 16 Mar 2018 13:55:12 +1100 (AEDT) From: Michael Neuling To: mpe@ellerman.id.au Cc: linuxppc-dev@neuling.org, paulus@ozlabs.org, david@gibson.dropbear.id.au, npiggin@gmail.com, pedromfc@linux.vnet.ibm.com, kvm-ppc@vger.kernel.org, Ananth N Mavinakayanahalli , Michael Neuling Subject: [PATCH] powerpc: Disable DAWR on POWER9 Date: Fri, 16 Mar 2018 13:54:56 +1100 Message-Id: <20180316025456.24036-1-mikey@neuling.org> X-Mailer: git-send-email 2.14.1 Sender: kvm-ppc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm-ppc@vger.kernel.org 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 return an error if run on a POWER9. GDB gives this error when you attempt to run a program with a breakpoint set on POWER9: "Unexpected error setting breakpoint or watchpoint: No such device." 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 --- 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 | 1 - arch/powerpc/kernel/hw_breakpoint.c | 3 +++ arch/powerpc/kernel/process.c | 11 +++++++++++ arch/powerpc/kernel/ptrace.c | 13 +++++++++++-- arch/powerpc/kvm/book3s_hv.c | 2 ++ arch/powerpc/kvm/book3s_hv_rmhandlers.S | 12 ++++++++++++ arch/powerpc/xmon/xmon.c | 5 +++++ 10 files changed, 48 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..dfb77b43f1 100644 --- a/arch/powerpc/kernel/dt_cpu_ftrs.c +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c @@ -62,7 +62,6 @@ struct dt_cpu_feature { CPU_FTR_COHERENT_ICACHE | \ CPU_FTR_STCX_CHECKS_ADDRESS |\ CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \ - CPU_FTR_DAWR | \ CPU_FTR_ARCH_206 |\ CPU_FTR_ARCH_207S) 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 #include #include +#include #include /* @@ -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..f4c4f9882c 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -41,6 +41,7 @@ #include #include #include +#include #define CREATE_TRACE_POINTS #include @@ -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; 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')