[PATCH 1/5] Powerpc/hw-breakpoint: Replace stale do_dabr() with do_break()
do_dabr() was renamed with do_break() long ago. But I still see
some comments mentioning do_dabr(). Replace it.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/kernel/hw_breakpoint.c | 2 +-
arch/powerpc/kernel/ptrace.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index a293a53b4365..1908e4fcc132 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -232,7 +232,7 @@ int hw_breakpoint_handler(struct die_args *args)
* Return early after invoking user-callback function without restoring
* DABR if the breakpoint is from ptrace which always operates in
* one-shot mode. The ptrace-ed process will receive the SIGTRAP signal
-* generated in do_dabr().
+* generated in do_break().
*/
if (bp->overflow_handler == ptrace_triggered) {
perf_bp_event(bp, regs);
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 684b0b315c32..44b823e5e8c8 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -2373,7 +2373,7 @@ void ptrace_triggered(struct perf_event *bp,
/*
* Disable the breakpoint request here since ptrace has defined a
* one-shot behaviour for breakpoint exceptions in PPC64.
-* The SIGTRAP signal is generated automatically for us in do_dabr().
+* The SIGTRAP signal is generated automatically for us in do_break().
* We don't have to do anything about that here
*/
attr = bp->attr;
--
2.20.1
[PATCH 5/5] Powerpc/Watchpoint: Fix length calculation for unaligned target
Watchpoint match range is always doubleword(8 bytes) aligned on
powerpc. If the given range is crossing doubleword boundary, we
need to increase the length such that next doubleword also get
covered. Ex,
address len = 6 bytes
|=.
|v--|--v|
| | | | | | | | | | | | | | | | |
|---|---|
<---8 bytes--->
In such case, current code configures hw as:
start_addr = address & ~HW_BREAKPOINT_ALIGN
len = 8 bytes
And thus read/write in last 4 bytes of the given range is ignored.
Fix this by including next doubleword in the length. Watchpoint
exception handler already ignores extraneous exceptions, so no
changes required for that.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/asm/hw_breakpoint.h | 7 ++--
arch/powerpc/kernel/hw_breakpoint.c | 44 +---
arch/powerpc/kernel/process.c| 34 --
3 files changed, 60 insertions(+), 25 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index 8acbbdd4a2d5..749a357164d5 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -34,6 +34,8 @@ struct arch_hw_breakpoint {
#define HW_BRK_TYPE_PRIV_ALL (HW_BRK_TYPE_USER | HW_BRK_TYPE_KERNEL | \
HW_BRK_TYPE_HYP)
+#define HW_BREAKPOINT_ALIGN 0x7
+
#ifdef CONFIG_HAVE_HW_BREAKPOINT
#include
#include
@@ -45,8 +47,6 @@ struct pmu;
struct perf_sample_data;
struct task_struct;
-#define HW_BREAKPOINT_ALIGN 0x7
-
extern int hw_breakpoint_slots(int type);
extern int arch_bp_generic_fields(int type, int *gen_bp_type);
extern int arch_check_bp_in_kernelspace(struct arch_hw_breakpoint *hw);
@@ -76,7 +76,8 @@ static inline void hw_breakpoint_disable(void)
}
extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
int hw_breakpoint_handler(struct die_args *args);
-
+extern u16 hw_breakpoint_get_final_len(struct arch_hw_breakpoint *brk,
+ unsigned long *start_addr, unsigned long *end_addr);
extern int set_dawr(struct arch_hw_breakpoint *brk);
extern bool dawr_force_enable;
static inline bool dawr_enabled(void)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 36bcf705df65..c122fd55aa44 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -126,6 +126,28 @@ int arch_bp_generic_fields(int type, int *gen_bp_type)
return 0;
}
+/* Maximum len for DABR is 8 bytes and DAWR is 512 bytes */
+static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
+{
+ u16 length_max = 8;
+ u16 final_len;
+ unsigned long start_addr, end_addr;
+
+ final_len = hw_breakpoint_get_final_len(hw, _addr, _addr);
+
+ if (dawr_enabled()) {
+ length_max = 512;
+ /* DAWR region can't cross 512 bytes boundary */
+ if ((start_addr >> 9) != (end_addr >> 9))
+ return -EINVAL;
+ }
+
+ if (final_len > length_max)
+ return -EINVAL;
+
+ return 0;
+}
+
/*
* Validate the arch-specific HW Breakpoint register settings
*/
@@ -133,12 +155,10 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
const struct perf_event_attr *attr,
struct arch_hw_breakpoint *hw)
{
- int length_max;
-
if (!ppc_breakpoint_available())
return -ENODEV;
- if (!bp)
+ if (!bp || !attr->bp_len)
return -EINVAL;
hw->type = HW_BRK_TYPE_TRANSLATE;
@@ -160,23 +180,7 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
hw->address = attr->bp_addr;
hw->len = attr->bp_len;
- length_max = 8; /* DABR */
- if (dawr_enabled()) {
- length_max = 512 ; /* 64 doublewords */
- /* DAWR region can't cross 512 bytes boundary */
- if ((hw->address >> 9) != ((hw->address + hw->len - 1) >> 9))
- return -EINVAL;
- }
-
- /*
-* Since breakpoint length can be a maximum of length_max and
-* breakpoint addresses are aligned to nearest double-word
-* HW_BREAKPOINT_ALIGN by rounding off to the lower address,
-* the 'symbolsize' should satisfy the check below.
-*/
- if (hw->len > (length_max - (hw->address & HW_BREAKPOINT_ALIGN)))
- return -EINVAL;
- return 0;
+ return hw_breakpoint_validate_len(hw);
}
/*
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 265fac9fb3a4..159aaa70de46 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -802,9 +802,39 @@ static int disable_dawr(void)
return 0;
}
+/*
+ * Watchpoint m
[PATCH 3/5] Powerpc/hw-breakpoint: Refactor set_dawr()
Remove unnecessary comments. Code itself is self explanatory. And, ISA already talks about MRD field. I Don't think we need to re-describe it. Signed-off-by: Ravi Bangoria --- arch/powerpc/kernel/process.c | 17 + 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index f0fbbf6a6a1f..f002d286 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -799,18 +799,11 @@ int set_dawr(struct arch_hw_breakpoint *brk) dawr = brk->address; - dawrx = (brk->type & (HW_BRK_TYPE_READ | HW_BRK_TYPE_WRITE)) \ - << (63 - 58); //* read/write bits */ - dawrx |= ((brk->type & (HW_BRK_TYPE_TRANSLATE)) >> 2) \ - << (63 - 59); //* translate */ - dawrx |= (brk->type & (HW_BRK_TYPE_PRIV_ALL)) \ - >> 3; //* PRIM bits */ - /* dawr length is stored in field MDR bits 48:53. Matches range in - doublewords (64 bits) baised by -1 eg. 0b00=1DW and - 0b11=64DW. - brk->len is in bytes. - This aligns up to double word size, shifts and does the bias. - */ + dawrx = (brk->type & HW_BRK_TYPE_RDWR) << (63 - 58); + dawrx |= ((brk->type & HW_BRK_TYPE_TRANSLATE) >> 2) << (63 - 59); + dawrx |= (brk->type & HW_BRK_TYPE_PRIV_ALL) >> 3; + + /* brk->len is in bytes. */ mrd = ((brk->len + 7) >> 3) - 1; dawrx |= (mrd & 0x3f) << (63 - 53); -- 2.20.1
Re: [PATCH 2/5] Powerpc/hw-breakpoint: Refactor hw_breakpoint_arch_parse()
On 6/18/19 11:51 AM, Christophe Leroy wrote: > > > Le 18/06/2019 à 06:27, Ravi Bangoria a écrit : >> Move feature availability check at the start of the function. >> Rearrange comment to it's associated code. Use hw->address and >> hw->len in the 512 bytes boundary check(to write if statement >> in a single line). Add spacing between code blocks. > > Are those cosmetic changes in the boundary check worth it since they > disappear in the final patch ? Nope.. not necessary. I was just going bit more patch by patch. I don't mind keeping the code as it is and then change it in the final patch.
Re: [PATCH 3/5] Powerpc/hw-breakpoint: Refactor set_dawr()
On 6/18/19 11:41 AM, Michael Neuling wrote: > This is going to collide with this patch > https://patchwork.ozlabs.org/patch/1109594/ Yeah, I'm aware of the patch. I just developed this on powerpc/next. I'll rebase my patches accordingly once mpe picks up that patche.
[PATCH v2 1/2] Powerpc/Watchpoint: Fix length calculation for unaligned target
Watchpoint match range is always doubleword(8 bytes) aligned on
powerpc. If the given range is crossing doubleword boundary, we
need to increase the length such that next doubleword also get
covered. Ex,
address len = 6 bytes
|=.
|v--|--v|
| | | | | | | | | | | | | | | | |
|---|---|
<---8 bytes--->
In such case, current code configures hw as:
start_addr = address & ~HW_BREAKPOINT_ALIGN
len = 8 bytes
And thus read/write in last 4 bytes of the given range is ignored.
Fix this by including next doubleword in the length. Watchpoint
exception handler already ignores extraneous exceptions, so no
changes required for that.
Signed-off-by: Ravi Bangoria
---
v1->v2:
- v1: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-June/192162.html
- No cosmetic patches.
- v1 fixed unaligned issue in core hw-breakpoint code only. But
ptrace was still messing up with address/len. Fix that in v2.
arch/powerpc/include/asm/debug.h | 1 +
arch/powerpc/include/asm/hw_breakpoint.h | 9 -
arch/powerpc/kernel/hw_breakpoint.c | 26 +++-
arch/powerpc/kernel/process.c| 50 +++-
arch/powerpc/kernel/ptrace.c | 37 ++
arch/powerpc/xmon/xmon.c | 3 +-
6 files changed, 83 insertions(+), 43 deletions(-)
diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
index 7756026b95ca..9c1b4aaa374b 100644
--- a/arch/powerpc/include/asm/debug.h
+++ b/arch/powerpc/include/asm/debug.h
@@ -45,6 +45,7 @@ static inline int debugger_break_match(struct pt_regs *regs)
{ return 0; }
static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
#endif
+int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw);
void __set_breakpoint(struct arch_hw_breakpoint *brk);
bool ppc_breakpoint_available(void);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index 0fe8c1e46bbc..ca2877be7f79 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -28,6 +28,7 @@ struct arch_hw_breakpoint {
unsigned long address;
u16 type;
u16 len; /* length of the target data symbol */
+ u16 hw_len; /* length programmed in hw */
};
/* Note: Don't change the the first 6 bits below as they are in the same order
@@ -47,6 +48,11 @@ struct arch_hw_breakpoint {
#define HW_BRK_TYPE_PRIV_ALL (HW_BRK_TYPE_USER | HW_BRK_TYPE_KERNEL | \
HW_BRK_TYPE_HYP)
+#define HW_BREAKPOINT_ALIGN 0x7
+
+#define DABR_MAX_LEN 8
+#define DAWR_MAX_LEN 512
+
#ifdef CONFIG_HAVE_HW_BREAKPOINT
#include
#include
@@ -58,8 +64,6 @@ struct pmu;
struct perf_sample_data;
struct task_struct;
-#define HW_BREAKPOINT_ALIGN 0x7
-
extern int hw_breakpoint_slots(int type);
extern int arch_bp_generic_fields(int type, int *gen_bp_type);
extern int arch_check_bp_in_kernelspace(struct arch_hw_breakpoint *hw);
@@ -84,6 +88,7 @@ static inline void hw_breakpoint_disable(void)
brk.address = 0;
brk.type = 0;
brk.len = 0;
+ brk.hw_len = 0;
if (ppc_breakpoint_available())
__set_breakpoint();
}
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index da307dd93ee3..a61a1266089a 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -147,9 +147,9 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
const struct perf_event_attr *attr,
struct arch_hw_breakpoint *hw)
{
- int ret = -EINVAL, length_max;
+ int ret = -EINVAL;
- if (!bp)
+ if (!bp || !attr->bp_len)
return ret;
hw->type = HW_BRK_TYPE_TRANSLATE;
@@ -169,26 +169,10 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
hw->address = attr->bp_addr;
hw->len = attr->bp_len;
- /*
-* Since breakpoint length can be a maximum of HW_BREAKPOINT_LEN(8)
-* and breakpoint addresses are aligned to nearest double-word
-* HW_BREAKPOINT_ALIGN by rounding off to the lower address, the
-* 'symbolsize' should satisfy the check below.
-*/
if (!ppc_breakpoint_available())
return -ENODEV;
- length_max = 8; /* DABR */
- if (dawr_enabled()) {
- length_max = 512 ; /* 64 doublewords */
- /* DAWR region can't cross 512 boundary */
- if ((attr->bp_addr >> 9) !=
- ((attr->bp_addr + attr->bp_len - 1) >> 9))
- return -EINVAL;
- }
- if (hw->len >
- (length_max - (hw->address & HW_BRE
[PATCH v2 2/2] Powerpc/Watchpoint: Rewrite ptrace-hwbreak.c selftest
ptrace-hwbreak.c selftest is logically broken. On powerpc, when
watchpoint is created with ptrace, signals are generated before
executing the instruction and user has to manually singlestep
the instruction with watchpoint disabled, which selftest never
does and thus it keeps on getting the signal at the same
instruction. If we fix it, selftest fails because the logical
connection between tracer(parent) and tracee(child) is also
broken. Rewrite the selftest and add new tests for unaligned
access.
With patch:
$ ./tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak
test: ptrace-hwbreak
tags: git_version:v5.2-rc2-33-ga247a75f90a9-dirty
PTRACE_SET_DEBUGREG, WO, len: 1: Ok
PTRACE_SET_DEBUGREG, WO, len: 2: Ok
PTRACE_SET_DEBUGREG, WO, len: 4: Ok
PTRACE_SET_DEBUGREG, WO, len: 8: Ok
PTRACE_SET_DEBUGREG, RO, len: 1: Ok
PTRACE_SET_DEBUGREG, RO, len: 2: Ok
PTRACE_SET_DEBUGREG, RO, len: 4: Ok
PTRACE_SET_DEBUGREG, RO, len: 8: Ok
PTRACE_SET_DEBUGREG, RW, len: 1: Ok
PTRACE_SET_DEBUGREG, RW, len: 2: Ok
PTRACE_SET_DEBUGREG, RW, len: 4: Ok
PTRACE_SET_DEBUGREG, RW, len: 8: Ok
PPC_PTRACE_SETHWDEBUG, MODE_EXACT, WO, len: 1: Ok
PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RO, len: 1: Ok
PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RW, len: 1: Ok
PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, WO, len: 6: Ok
PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RO, len: 6: Ok
PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RW, len: 6: Ok
PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, WO, len: 6: Ok
PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, RO, len: 6: Ok
PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, RW, len: 6: Ok
PPC_PTRACE_SETHWDEBUG, DAWR_MAX_LEN, RW, len: 512: Ok
success: ptrace-hwbreak
Signed-off-by: Ravi Bangoria
---
.../selftests/powerpc/ptrace/ptrace-hwbreak.c | 535 +++---
1 file changed, 325 insertions(+), 210 deletions(-)
diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c
b/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c
index 3066d310f32b..fb1e05d7f77c 100644
--- a/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c
+++ b/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c
@@ -22,318 +22,433 @@
#include
#include "ptrace.h"
-/* Breakpoint access modes */
-enum {
- BP_X = 1,
- BP_RW = 2,
- BP_W = 4,
-};
-
-static pid_t child_pid;
-static struct ppc_debug_info dbginfo;
-
-static void get_dbginfo(void)
-{
- int ret;
-
- ret = ptrace(PPC_PTRACE_GETHWDBGINFO, child_pid, NULL, );
- if (ret) {
- perror("Can't get breakpoint info\n");
- exit(-1);
- }
-}
-
-static bool hwbreak_present(void)
-{
- return (dbginfo.num_data_bps != 0);
-}
+/*
+ * Use volatile on all global var so that compiler doesn't
+ * optimise their load/stores. Otherwise selftest can fail.
+ */
+static volatile __u64 glvar;
-static bool dawr_present(void)
-{
- return !!(dbginfo.features & PPC_DEBUG_FEATURE_DATA_BP_DAWR);
-}
+#define DAWR_MAX_LEN 512
+static volatile __u8 big_var[DAWR_MAX_LEN] __attribute__((aligned(512)));
-static void set_breakpoint_addr(void *addr)
-{
- int ret;
+#define A_LEN 6
+#define B_LEN 6
+struct gstruct {
+ __u8 a[A_LEN]; /* double word aligned */
+ __u8 b[B_LEN]; /* double word unaligned */
+};
+static volatile struct gstruct gstruct __attribute__((aligned(512)));
- ret = ptrace(PTRACE_SET_DEBUGREG, child_pid, 0, addr);
- if (ret) {
- perror("Can't set breakpoint addr\n");
- exit(-1);
- }
-}
-static int set_hwbreakpoint_addr(void *addr, int range)
+static void get_dbginfo(pid_t child_pid, struct ppc_debug_info *dbginfo)
{
- int ret;
-
- struct ppc_hw_breakpoint info;
-
- info.version = 1;
- info.trigger_type = PPC_BREAKPOINT_TRIGGER_RW;
- info.addr_mode = PPC_BREAKPOINT_MODE_EXACT;
- if (range > 0)
- info.addr_mode = PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
- info.condition_mode = PPC_BREAKPOINT_CONDITION_NONE;
- info.addr = (__u64)addr;
- info.addr2 = (__u64)addr + range;
- info.condition_value = 0;
-
- ret = ptrace(PPC_PTRACE_SETHWDEBUG, child_pid, 0, );
- if (ret < 0) {
- perror("Can't set breakpoint\n");
+ if (ptrace(PPC_PTRACE_GETHWDBGINFO, child_pid, NULL, dbginfo)) {
+ perror("Can't get breakpoint info");
exit(-1);
}
- return ret;
}
-static int del_hwbreakpoint_addr(int watchpoint_handle)
+static bool dawr_present(struct ppc_debug_info *dbginfo)
{
- int ret;
-
- ret = ptrace(PPC_PTRACE_DELHWDEBUG, child_pid, 0, watchpoint_handle);
- if (ret < 0) {
- perror("Can't delete hw breakpoint\n");
- exit(-1);
- }
- return ret;
+ return !!(dbginfo->features & PPC_DEBUG_FEATURE_DATA_BP_DAWR);
}
Re: [RFC PATCH v2 09/12] powerpc/ptrace: split out ADV_DEBUG_REGS related functions.
On 6/28/19 9:17 PM, Christophe Leroy wrote:
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-adv.c
> b/arch/powerpc/kernel/ptrace/ptrace-adv.c
> new file mode 100644
> index ..86e71fa6c5c8
> --- /dev/null
> +++ b/arch/powerpc/kernel/ptrace/ptrace-adv.c
> @@ -0,0 +1,487 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +void user_enable_single_step(struct task_struct *task)
> +{
> + struct pt_regs *regs = task->thread.regs;
> +
> + if (regs != NULL) {
> + task->thread.debug.dbcr0 &= ~DBCR0_BT;
> + task->thread.debug.dbcr0 |= DBCR0_IDM | DBCR0_IC;
> + regs->msr |= MSR_DE;
> + }
> + set_tsk_thread_flag(task, TIF_SINGLESTEP);
> +}
> +
> +void user_enable_block_step(struct task_struct *task)
> +{
> + struct pt_regs *regs = task->thread.regs;
> +
> + if (regs != NULL) {
> + task->thread.debug.dbcr0 &= ~DBCR0_IC;
> + task->thread.debug.dbcr0 = DBCR0_IDM | DBCR0_BT;
> + regs->msr |= MSR_DE;
> + }
> + set_tsk_thread_flag(task, TIF_SINGLESTEP);
> +}
> +
> +void user_disable_single_step(struct task_struct *task)
> +{
> + struct pt_regs *regs = task->thread.regs;
> +
> + if (regs != NULL) {
> + /*
> + * The logic to disable single stepping should be as
> + * simple as turning off the Instruction Complete flag.
> + * And, after doing so, if all debug flags are off, turn
> + * off DBCR0(IDM) and MSR(DE) Torez
> + */
> + task->thread.debug.dbcr0 &= ~(DBCR0_IC|DBCR0_BT);
> + /*
> + * Test to see if any of the DBCR_ACTIVE_EVENTS bits are set.
> + */
> + if (!DBCR_ACTIVE_EVENTS(task->thread.debug.dbcr0,
> + task->thread.debug.dbcr1)) {
> + /*
> + * All debug events were off.
> + */
> + task->thread.debug.dbcr0 &= ~DBCR0_IDM;
> + regs->msr &= ~MSR_DE;
> + }
> + }
> + clear_tsk_thread_flag(task, TIF_SINGLESTEP);
> +}
> +
> +int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> unsigned long data)
> +{
> + /* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
> + * For embedded processors we support one DAC and no IAC's at the
> + * moment.
> + */
I guess mentioning DABR and IABR doesn't make sense in ptrace-adv.c?
> + if (addr > 0)
> + return -EINVAL;
> +
> + /* The bottom 3 bits in dabr are flags */
Same here.
> + if ((data & ~0x7UL) >= TASK_SIZE)
> + return -EIO;
> +
> + /* As described above, it was assumed 3 bits were passed with the data
> + * address, but we will assume only the mode bits will be passed
> + * as to not cause alignment restrictions for DAC-based processors.
> + */
> +
> + /* DAC's hold the whole address without any mode flags */
> + task->thread.debug.dac1 = data & ~0x3UL;
> +
> + if (task->thread.debug.dac1 == 0) {
> + dbcr_dac(task) &= ~(DBCR_DAC1R | DBCR_DAC1W);
> + if (!DBCR_ACTIVE_EVENTS(task->thread.debug.dbcr0,
> + task->thread.debug.dbcr1)) {
> + task->thread.regs->msr &= ~MSR_DE;
> + task->thread.debug.dbcr0 &= ~DBCR0_IDM;
> + }
> + return 0;
> + }
> +
> + /* Read or Write bits must be set */
> +
> + if (!(data & 0x3UL))
> + return -EINVAL;
> +
> + /* Set the Internal Debugging flag (IDM bit 1) for the DBCR0
> +register */
> + task->thread.debug.dbcr0 |= DBCR0_IDM;
> +
> + /* Check for write and read flags and set DBCR0
> +accordingly */
> + dbcr_dac(task) &= ~(DBCR_DAC1R|DBCR_DAC1W);
> + if (data & 0x1UL)
> + dbcr_dac(task) |= DBCR_DAC1R;
> + if (data & 0x2UL)
> + dbcr_dac(task) |= DBCR_DAC1W;
> + task->thread.regs->msr |= MSR_DE;
> + return 0;
> +}
> +
> +static long set_instruction_bp(struct task_struct *child,
> + struct ppc_hw_breakpoint *bp_info)
> +{
> + int slot;
> + int slot1_in_use = ((child->thread.debug.dbcr0 & DBCR0_IAC1) != 0);
> + int slot2_in_use = ((child->thread.debug.dbcr0 & DBCR0_IAC2) != 0);
> + int slot3_in_use = ((child->thread.debug.dbcr0 & DBCR0_IAC3) != 0);
> + int slot4_in_use = ((child->thread.debug.dbcr0 & DBCR0_IAC4) != 0);
> +
> + if (dbcr_iac_range(child)
Re: [RFC PATCH v2 10/12] powerpc/ptrace: create ptrace_get_debugreg()
On 6/28/19 9:17 PM, Christophe Leroy wrote:
> Create ptrace_get_debugreg() to handle PTRACE_GET_DEBUGREG and
> reduce ifdef mess
>
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/kernel/ptrace/ptrace-adv.c | 9 +
> arch/powerpc/kernel/ptrace/ptrace-decl.h | 2 ++
> arch/powerpc/kernel/ptrace/ptrace-noadv.c | 13 +
> arch/powerpc/kernel/ptrace/ptrace.c | 18 ++
> 4 files changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-adv.c
> b/arch/powerpc/kernel/ptrace/ptrace-adv.c
> index 86e71fa6c5c8..dcc765940344 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-adv.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace-adv.c
> @@ -83,6 +83,15 @@ void user_disable_single_step(struct task_struct *task)
> clear_tsk_thread_flag(task, TIF_SINGLESTEP);
> }
>
> +int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
> + unsigned long __user *datalp)
> +{
> + /* We only support one DABR and no IABRS at the moment */
No DABR / IABR in ptrace-adv.c
> + if (addr > 0)
> + return -EINVAL;
> + return put_user(child->thread.debug.dac1, datalp);
> +}
> +
> int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> unsigned long data)
> {
> /* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-decl.h
> b/arch/powerpc/kernel/ptrace/ptrace-decl.h
> index bdba09a87aea..4b4b6a1d508a 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-decl.h
> +++ b/arch/powerpc/kernel/ptrace/ptrace-decl.h
> @@ -176,6 +176,8 @@ int tm_cgpr32_set(struct task_struct *target, const
> struct user_regset *regset,
> extern const struct user_regset_view user_ppc_native_view;
>
> /* ptrace-(no)adv */
> +int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
> + unsigned long __user *datalp);
> int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> unsigned long data);
> long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint
> *bp_info);
> long ppc_del_hwdebug(struct task_struct *child, long data);
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> index 7db330c94538..985cca136f85 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> @@ -64,6 +64,19 @@ void user_disable_single_step(struct task_struct *task)
> clear_tsk_thread_flag(task, TIF_SINGLESTEP);
> }
>
> +int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
> + unsigned long __user *datalp)
> +{
> + unsigned long dabr_fake;
> +
> + /* We only support one DABR and no IABRS at the moment */
> + if (addr > 0)
> + return -EINVAL;
> + dabr_fake = ((child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
> + (child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
> + return put_user(dabr_fake, datalp);
> +}
> +
> int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> unsigned long data)
> {
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> diff --git a/arch/powerpc/kernel/ptrace/ptrace.c
> b/arch/powerpc/kernel/ptrace/ptrace.c
> index 377e0e541d5f..e789afae6f56 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> @@ -211,23 +211,9 @@ long arch_ptrace(struct task_struct *child, long request,
> break;
> }
>
> - case PTRACE_GET_DEBUGREG: {
> -#ifndef CONFIG_PPC_ADV_DEBUG_REGS
> - unsigned long dabr_fake;
> -#endif
> - ret = -EINVAL;
> - /* We only support one DABR and no IABRS at the moment */
> - if (addr > 0)
> - break;
> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
> - ret = put_user(child->thread.debug.dac1, datalp);
> -#else
> - dabr_fake = ((child->thread.hw_brk.address &
> (~HW_BRK_TYPE_DABR)) |
> - (child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
> - ret = put_user(dabr_fake, datalp);
> -#endif
> + case PTRACE_GET_DEBUGREG:
> + ret = ptrace_get_debugreg(child, addr, datalp);
> break;
> - }
>
> case PTRACE_SET_DEBUGREG:
> ret = ptrace_set_debugreg(child, addr, data);
>
Otherwise,
Reviewed-by: Ravi Bangoria
Re: [RFC PATCH v2 12/12] powerpc/ptrace: move ptrace_triggered() into hw_breakpoint.c
On 6/28/19 9:18 PM, Christophe Leroy wrote: > ptrace_triggered() is declared in asm/hw_breakpoint.h and > only needed when CONFIG_HW_BREAKPOINT is set, so move it > into hw_breakpoint.c > > Signed-off-by: Christophe Leroy Reviewed-by: Ravi Bangoria
Re: [RFC PATCH v2 11/12] powerpc/ptrace: create ppc_gethwdinfo()
On 6/28/19 9:18 PM, Christophe Leroy wrote: > Create ippc_gethwdinfo() to handle PPC_PTRACE_GETHWDBGINFO and > reduce ifdef mess > > Signed-off-by: Christophe Leroy > --- Reviewed-by: Ravi Bangoria
Re: Reminder: 22 open syzbot bugs in perf subsystem
On 7/2/19 11:13 AM, Eric Biggers wrote: > > Title: possible deadlock in uprobe_clear_state > Last occurred: 164 days ago > Reported: 201 days ago > Branches: Mainline > Dashboard link: > https://syzkaller.appspot.com/bug?id=a1ce9b3da349209c5085bb8c4fee753d68c3697f > Original thread: > https://lkml.kernel.org/lkml/10a9fb057cd14...@google.com/T/#u > > Unfortunately, this bug does not have a reproducer. > > No one replied to the original thread for this bug. > > If you fix this bug, please add the following tag to the commit: > Reported-by: syzbot+1068f09c44d151250...@syzkaller.appspotmail.com > > If you send any email or patch for this bug, please consider replying to the > original thread. For the git send-email command to use, or tips on how to > reply > if the thread isn't in your mailbox, see the "Reply instructions" at > https://lkml.kernel.org/r/10a9fb057cd14...@google.com > This is false positive: https://marc.info/?l=linux-kernel=154925313012615=2
Re: Reminder: 22 open syzbot bugs in perf subsystem
Hi Eric, On 7/3/19 9:25 AM, Eric Biggers wrote: > On Wed, Jul 03, 2019 at 09:09:55AM +0530, Ravi Bangoria wrote: >> >> >> On 7/2/19 11:13 AM, Eric Biggers wrote: >>> >>> Title: possible deadlock in uprobe_clear_state >>> Last occurred: 164 days ago >>> Reported: 201 days ago >>> Branches: Mainline >>> Dashboard link: >>> https://syzkaller.appspot.com/bug?id=a1ce9b3da349209c5085bb8c4fee753d68c3697f >>> Original thread: >>> https://lkml.kernel.org/lkml/10a9fb057cd14...@google.com/T/#u >>> >>> Unfortunately, this bug does not have a reproducer. >>> >>> No one replied to the original thread for this bug. >>> >>> If you fix this bug, please add the following tag to the commit: >>> Reported-by: syzbot+1068f09c44d151250...@syzkaller.appspotmail.com >>> >>> If you send any email or patch for this bug, please consider replying to the >>> original thread. For the git send-email command to use, or tips on how to >>> reply >>> if the thread isn't in your mailbox, see the "Reply instructions" at >>> https://lkml.kernel.org/r/10a9fb057cd14...@google.com >>> >> >> This is false positive: >> https://marc.info/?l=linux-kernel=154925313012615=2 >> > > What do you mean "false positive"? Your patch says there can be a deadlock. > Also, your patch hasn't been merged yet. So doesn't it still need to be > fixed? Please see Oleg's reply to the patch: https://marc.info/?l=linux-kernel=154946017315554=2 """ But this is false positive, right? if CPU1 calls update_ref_ctr() then either ->mm_users is already zero so binder_alloc_free_page()->mmget_not_zero() will fail, or the caller of update_ref_ctr() has a reference and thus binder_alloc_free_page()->mmput() can't trigger __mmput() ? """
Re: Reminder: 22 open syzbot bugs in perf subsystem
On 7/3/19 9:49 AM, Eric Biggers wrote: > On Wed, Jul 03, 2019 at 09:29:39AM +0530, Ravi Bangoria wrote: >> Hi Eric, >> >> On 7/3/19 9:25 AM, Eric Biggers wrote: >>> On Wed, Jul 03, 2019 at 09:09:55AM +0530, Ravi Bangoria wrote: >>>> >>>> >>>> On 7/2/19 11:13 AM, Eric Biggers wrote: >>>>> >>>>> Title: possible deadlock in uprobe_clear_state >>>>> Last occurred: 164 days ago >>>>> Reported: 201 days ago >>>>> Branches: Mainline >>>>> Dashboard link: >>>>> https://syzkaller.appspot.com/bug?id=a1ce9b3da349209c5085bb8c4fee753d68c3697f >>>>> Original thread: >>>>> https://lkml.kernel.org/lkml/10a9fb057cd14...@google.com/T/#u >>>>> >>>>> Unfortunately, this bug does not have a reproducer. >>>>> >>>>> No one replied to the original thread for this bug. >>>>> >>>>> If you fix this bug, please add the following tag to the commit: >>>>> Reported-by: syzbot+1068f09c44d151250...@syzkaller.appspotmail.com >>>>> >>>>> If you send any email or patch for this bug, please consider replying to >>>>> the >>>>> original thread. For the git send-email command to use, or tips on how >>>>> to reply >>>>> if the thread isn't in your mailbox, see the "Reply instructions" at >>>>> https://lkml.kernel.org/r/10a9fb057cd14...@google.com >>>>> >>>> >>>> This is false positive: >>>> https://marc.info/?l=linux-kernel=154925313012615=2 >>>> >>> >>> What do you mean "false positive"? Your patch says there can be a deadlock. >>> Also, your patch hasn't been merged yet. So doesn't it still need to be >>> fixed? >> >> Please see Oleg's reply to the patch: >> https://marc.info/?l=linux-kernel=154946017315554=2 >> >> """ >> But this is false positive, right? if CPU1 calls update_ref_ctr() then >> either ->mm_users is already zero so >> binder_alloc_free_page()->mmget_not_zero() >> will fail, or the caller of update_ref_ctr() has a reference and thus >> binder_alloc_free_page()->mmput() can't trigger __mmput() ? >> """ >> > > Even if it's a lockdep false positive you can't ignore it. People rely on > lockdep to find bugs, and they will keep sending you bug reports. So someone > has to fix something. Agreed. > Did you see Oleg's suggestion to change mmput() to > mmput_async() in binder_alloc_free_page()? > https://marc.info/?l=linux-kernel=155119805728815=2 > If you believe that is the right fix, Yes, fixing it in binderfs looks right to me. > I can reassign this report to binder > subsystem and nag the binder maintainers instead... Yes please :)
Re: [PATCH] powerpc/hw_breakpoint: move instruction stepping out of hw_breakpoint_handler()
On 6/28/19 9:25 PM, Christophe Leroy wrote:
> On 8xx, breakpoints stop after executing the instruction, so
> stepping/emulation is not needed. Move it into a sub-function and
> remove the #ifdefs.
>
> Signed-off-by: Christophe Leroy
> ---
Reviewed-by: Ravi Bangoria
Just one neat below...
[...]
> -#ifndef CONFIG_PPC_8xx
> - /* Do not emulate user-space instructions, instead single-step them */
> - if (user_mode(regs)) {
> - current->thread.last_hit_ubp = bp;
> - regs->msr |= MSR_SE;
> + if (!IS_ENABLED(CONFIG_PPC_8xx) && !stepping_handler(regs, bp,
> info->address))
May be split this line. It's 86 chars long and checkpatch.pl is warning
about this:
WARNING: line over 80 characters
#257: FILE: arch/powerpc/kernel/hw_breakpoint.c:282:
+ if (!IS_ENABLED(CONFIG_PPC_8xx) && !stepping_handler(regs, bp,
info->address))
Re: [RFC PATCH v2 00/12] Reduce ifdef mess in ptrace
On 6/28/19 9:17 PM, Christophe Leroy wrote: > The purpose of this series is to reduce the amount of #ifdefs > in ptrace.c > > This is a first try. Most of it is done, there are still some #ifdefs that > could go away. > > Please comment and tell whether it is worth continuing in that direction. Hi Christopher, I'm seeing this warning with all the patches. WARNING: Improper SPDX comment style for 'arch/powerpc/kernel/ptrace/ptrace-view.c', please use '//' instead #100: FILE: arch/powerpc/kernel/ptrace/ptrace-view.c:1: +/* SPDX-License-Identifier: GPL-2.0-or-later */
[PATCH] Powerpc/Watchpoint: Restore nvgprs while returning from exception
Powerpc hw triggers watchpoint before executing the instruction. To make trigger-after-execute behavior, kernel emulates the instruction. If the instruction is 'load something into non- volatile register', exception handler should restore emulated register state while returning back, otherwise there will be register state corruption. Ex, Adding a watchpoint on a list can corrput the list: # cat /proc/kallsyms | grep kthread_create_list c121c8b8 d kthread_create_list Add watchpoint on kthread_create_list->next: # perf record -e mem:0xc121c8c0 Run some workload such that new kthread gets invoked. Ex, I just logged out from console: list_add corruption. next->prev should be prev (c1214e00), \ but was c121c8b8. (next=c121c8b8). WARNING: CPU: 59 PID: 309 at lib/list_debug.c:25 __list_add_valid+0xb4/0xc0 CPU: 59 PID: 309 Comm: kworker/59:0 Kdump: loaded Not tainted 5.1.0-rc7+ #69 ... NIP __list_add_valid+0xb4/0xc0 LR __list_add_valid+0xb0/0xc0 Call Trace: __list_add_valid+0xb0/0xc0 (unreliable) __kthread_create_on_node+0xe0/0x260 kthread_create_on_node+0x34/0x50 create_worker+0xe8/0x260 worker_thread+0x444/0x560 kthread+0x160/0x1a0 ret_from_kernel_thread+0x5c/0x70 Signed-off-by: Ravi Bangoria --- arch/powerpc/kernel/exceptions-64s.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S index 9481a11..96de0d1 100644 --- a/arch/powerpc/kernel/exceptions-64s.S +++ b/arch/powerpc/kernel/exceptions-64s.S @@ -1753,7 +1753,7 @@ handle_dabr_fault: ld r5,_DSISR(r1) addir3,r1,STACK_FRAME_OVERHEAD bl do_break -12:b ret_from_except_lite +12:b ret_from_except #ifdef CONFIG_PPC_BOOK3S_64 -- 1.8.3.1
Re: [PATCH] Powerpc/Watchpoint: Restore nvgprs while returning from exception
On 6/6/19 12:59 PM, Ravi Bangoria wrote: > Powerpc hw triggers watchpoint before executing the instruction. > To make trigger-after-execute behavior, kernel emulates the > instruction. If the instruction is 'load something into non- > volatile register', exception handler should restore emulated > register state while returning back, otherwise there will be > register state corruption. Ex, Adding a watchpoint on a list > can corrput the list: > > # cat /proc/kallsyms | grep kthread_create_list > c121c8b8 d kthread_create_list > > Add watchpoint on kthread_create_list->next: s/kthread_create_list->next/kthread_create_list->prev/
Re: [PATCH] Powerpc/Watchpoint: Restore nvgprs while returning from exception
On 6/7/19 6:20 AM, Michael Neuling wrote:
> On Thu, 2019-06-06 at 12:59 +0530, Ravi Bangoria wrote:
>> Powerpc hw triggers watchpoint before executing the instruction.
>> To make trigger-after-execute behavior, kernel emulates the
>> instruction. If the instruction is 'load something into non-
>> volatile register', exception handler should restore emulated
>> register state while returning back, otherwise there will be
>> register state corruption. Ex, Adding a watchpoint on a list
>> can corrput the list:
>>
>> # cat /proc/kallsyms | grep kthread_create_list
>> c121c8b8 d kthread_create_list
>>
>> Add watchpoint on kthread_create_list->next:
>>
>> # perf record -e mem:0xc121c8c0
>>
>> Run some workload such that new kthread gets invoked. Ex, I
>> just logged out from console:
>>
>> list_add corruption. next->prev should be prev (c1214e00), \
>> but was c121c8b8. (next=c121c8b8).
>> WARNING: CPU: 59 PID: 309 at lib/list_debug.c:25 __list_add_valid+0xb4/0xc0
>> CPU: 59 PID: 309 Comm: kworker/59:0 Kdump: loaded Not tainted 5.1.0-rc7+
>> #69
>> ...
>> NIP __list_add_valid+0xb4/0xc0
>> LR __list_add_valid+0xb0/0xc0
>> Call Trace:
>> __list_add_valid+0xb0/0xc0 (unreliable)
>> __kthread_create_on_node+0xe0/0x260
>> kthread_create_on_node+0x34/0x50
>> create_worker+0xe8/0x260
>> worker_thread+0x444/0x560
>> kthread+0x160/0x1a0
>> ret_from_kernel_thread+0x5c/0x70
>>
>> Signed-off-by: Ravi Bangoria
>
> How long has this been around? Should we be CCing stable?
"bl .save_nvgprs" was added in the commit 5aae8a5370802 ("powerpc,
hw_breakpoints:
Implement hw_breakpoints for 64-bit server processors"), which was merged in
v2.6.36.
Re: [PATCH] Powerpc/Watchpoint: Restore nvgprs while returning from exception
On 6/7/19 11:20 AM, Michael Ellerman wrote: > Ravi Bangoria writes: > >> Powerpc hw triggers watchpoint before executing the instruction. >> To make trigger-after-execute behavior, kernel emulates the >> instruction. If the instruction is 'load something into non- >> volatile register', exception handler should restore emulated >> register state while returning back, otherwise there will be >> register state corruption. Ex, Adding a watchpoint on a list >> can corrput the list: >> >> # cat /proc/kallsyms | grep kthread_create_list >> c121c8b8 d kthread_create_list >> >> Add watchpoint on kthread_create_list->next: >> >> # perf record -e mem:0xc121c8c0 >> >> Run some workload such that new kthread gets invoked. Ex, I >> just logged out from console: >> >> list_add corruption. next->prev should be prev (c1214e00), \ >> but was c121c8b8. (next=c121c8b8). >> WARNING: CPU: 59 PID: 309 at lib/list_debug.c:25 __list_add_valid+0xb4/0xc0 >> CPU: 59 PID: 309 Comm: kworker/59:0 Kdump: loaded Not tainted 5.1.0-rc7+ >> #69 >> ... >> NIP __list_add_valid+0xb4/0xc0 >> LR __list_add_valid+0xb0/0xc0 >> Call Trace: >> __list_add_valid+0xb0/0xc0 (unreliable) >> __kthread_create_on_node+0xe0/0x260 >> kthread_create_on_node+0x34/0x50 >> create_worker+0xe8/0x260 >> worker_thread+0x444/0x560 >> kthread+0x160/0x1a0 >> ret_from_kernel_thread+0x5c/0x70 > > This all depends on what code the compiler generates for the list > access. True. list corruption is just an example. But any load instruction that uses non-volatile register and hits a watchpoint, will result in register state corruption. > Can you include a disassembly of the relevant code in your > kernel so we have an example of the bad case. Register state from WARN_ON(): GPR00: c059a3a0 c07ff23afb50 c1344e00 0075 GPR04: 001852af8bc1 GPR08: 0001 0007 0006 04aa GPR12: c07eb080 c0137038 c05ff62aaa00 GPR16: c07fffbe7600 c07fffbe7370 GPR20: c07fffbe7320 c07fffbe7300 c1373a00 GPR24: fef7 c012e320 c07ff23afcb0 c0cb8628 GPR28: c121c8b8 c1214e00 c07fef5b17e8 c07fef5b17c0 Snippet from __kthread_create_on_node: c0136be8: ed ff a2 3f addis r29,r2,-19 c0136bec: c0 7a bd eb ld r29,31424(r29) if (!__list_add_valid(new, prev, next)) c0136bf0: 78 f3 c3 7f mr r3,r30 c0136bf4: 78 e3 85 7f mr r5,r28 c0136bf8: 78 eb a4 7f mr r4,r29 c0136bfc: fd 36 46 48 bl c059a2f8 <__list_add_valid+0x8> Watchpoint hit at 0xc0136bec. addis r29,r2,-19 => r29 = 0xc1344e00 + (-19 << 16) => r29 = 0xc1214e00 ld r29,31424(r29) => r29 = *(0xc1214e00 + 31424) => r29 = *(0xc121c8c0) 0xc121c8c0 is where we placed a watchpoint and thus this instruction was emulated by emulate_step. But because handle_dabr_fault did not restore emulated register state, r29 still contains stale value in above register state.
[PATCH v2] perf ioctl: Add check for the sample_period value
perf_event_open() limits the sample_period to 63 bits. See
commit 0819b2e30ccb ("perf: Limit perf_event_attr::sample_period
to 63 bits"). Make ioctl() consistent with it.
Also on powerpc, negative sample_period could cause a recursive
PMIs leading to a hang (reported when running perf-fuzzer).
Signed-off-by: Ravi Bangoria
---
kernel/events/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index abbd4b3b96c2..e44c90378940 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5005,6 +5005,9 @@ static int perf_event_period(struct perf_event *event,
u64 __user *arg)
if (perf_event_check_period(event, value))
return -EINVAL;
+ if (!event->attr.freq && (value & (1ULL << 63)))
+ return -EINVAL;
+
event_function_call(event, __perf_event_period, );
return 0;
--
2.20.1
Re: [PATCH] powerpc/hw_breakpoint: move instruction stepping out of hw_breakpoint_handler()
On 7/6/19 1:56 PM, Christophe Leroy wrote:
>
>
> Le 03/07/2019 à 08:20, Ravi Bangoria a écrit :
>>
>>
>> On 6/28/19 9:25 PM, Christophe Leroy wrote:
>>> On 8xx, breakpoints stop after executing the instruction, so
>>> stepping/emulation is not needed. Move it into a sub-function and
>>> remove the #ifdefs.
>>>
>>> Signed-off-by: Christophe Leroy
>>> ---
>>
>> Reviewed-by: Ravi Bangoria
>>
>> Just one neat below...
>
> Thanks for the review.
>
>>
>> [...]
>>
>>> -#ifndef CONFIG_PPC_8xx
>>> - /* Do not emulate user-space instructions, instead single-step them */
>>> - if (user_mode(regs)) {
>>> - current->thread.last_hit_ubp = bp;
>>> - regs->msr |= MSR_SE;
>>> + if (!IS_ENABLED(CONFIG_PPC_8xx) && !stepping_handler(regs, bp,
>>> info->address))
>>
>> May be split this line. It's 86 chars long and checkpatch.pl is warning
>> about this:
>
> Didn't you use arch/powerpc/tools/checkpatch.sh ?
>
> powerpc accepts 90 chars per line.
Hmm.. wasn't aware of it. Thanks!
Re: [PATCH 4/5] Powerpc/hw-breakpoint: Optimize disable path
On 6/18/19 11:45 AM, Michael Neuling wrote: > On Tue, 2019-06-18 at 09:57 +0530, Ravi Bangoria wrote: >> Directly setting dawr and dawrx with 0 should be enough to >> disable watchpoint. No need to reset individual bits in >> variable and then set in hw. > > This seems like a pointless optimisation to me. > > I'm all for adding more code/complexity if it buys us some performance, but I > can't imagine this is a fast path (nor have you stated any performance > benefits). This gets called from sched_switch. I expected the improvement when we switch from monitored process to non-monitored process. With such scenario, I tried to measure the difference in execution time of set_dawr but I don't see any improvement. So I'll drop the patch.
Re: [PATCH 4/5] Powerpc/hw-breakpoint: Optimize disable path
On 6/18/19 12:01 PM, Christophe Leroy wrote:
>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>> index f002d286..265fac9fb3a4 100644
>> --- a/arch/powerpc/kernel/process.c
>> +++ b/arch/powerpc/kernel/process.c
>> @@ -793,10 +793,22 @@ static inline int set_dabr(struct arch_hw_breakpoint
>> *brk)
>> return __set_dabr(dabr, dabrx);
>> }
>> +static int disable_dawr(void)
>> +{
>> + if (ppc_md.set_dawr)
>> + return ppc_md.set_dawr(0, 0);
>> +
>> + mtspr(SPRN_DAWRX, 0);
>
> And SPRN_DAWR ?
Setting DAWRx with 0 should be enough to disable the breakpoint.
Re: [PATCH 5/5] Powerpc/Watchpoint: Fix length calculation for unaligned target
On 6/18/19 12:16 PM, Christophe Leroy wrote:
>> +/* Maximum len for DABR is 8 bytes and DAWR is 512 bytes */
>> +static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
>> +{
>> + u16 length_max = 8;
>> + u16 final_len;
>
> You should be more consistent in naming. If one is called final_len, the
> other one should be called max_len.
Copy/paste :). Will change it.
>
>> + unsigned long start_addr, end_addr;
>> +
>> + final_len = hw_breakpoint_get_final_len(hw, _addr, _addr);
>> +
>> + if (dawr_enabled()) {
>> + length_max = 512;
>> + /* DAWR region can't cross 512 bytes boundary */
>> + if ((start_addr >> 9) != (end_addr >> 9))
>> + return -EINVAL;
>> + }
>> +
>> + if (final_len > length_max)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>
> Is many places, we have those numeric 512 and 9 shift. Could we replace them
> by some symbol, for instance DAWR_SIZE and DAWR_SHIFT ?
I don't see any other place where we check for boundary limit.
[...]
>
>> +u16 hw_breakpoint_get_final_len(struct arch_hw_breakpoint *brk,
>> + unsigned long *start_addr,
>> + unsigned long *end_addr)
>> +{
>> + *start_addr = brk->address & ~HW_BREAKPOINT_ALIGN;
>> + *end_addr = (brk->address + brk->len - 1) | HW_BREAKPOINT_ALIGN;
>> + return *end_addr - *start_addr + 1;
>> +}
>
> This function gives horrible code (a couple of unneeded store/re-read and
> read/re-read).
>
> 06bc :
> 6bc: 81 23 00 00 lwz r9,0(r3)
> 6c0: 55 29 00 38 rlwinm r9,r9,0,0,28
> 6c4: 91 24 00 00 stw r9,0(r4)
> 6c8: 81 43 00 00 lwz r10,0(r3)
> 6cc: a1 23 00 06 lhz r9,6(r3)
> 6d0: 38 6a ff ff addi r3,r10,-1
> 6d4: 7c 63 4a 14 add r3,r3,r9
> 6d8: 60 63 00 07 ori r3,r3,7
> 6dc: 90 65 00 00 stw r3,0(r5)
> 6e0: 38 63 00 01 addi r3,r3,1
> 6e4: 81 24 00 00 lwz r9,0(r4)
> 6e8: 7c 69 18 50 subf r3,r9,r3
> 6ec: 54 63 04 3e clrlwi r3,r3,16
> 6f0: 4e 80 00 20 blr
>
> Below code gives something better:
>
> u16 hw_breakpoint_get_final_len(struct arch_hw_breakpoint *brk,
> unsigned long *start_addr,
> unsigned long *end_addr)
> {
> unsigned long address = brk->address;
> unsigned long len = brk->len;
> unsigned long start = address & ~HW_BREAKPOINT_ALIGN;
> unsigned long end = (address + len - 1) | HW_BREAKPOINT_ALIGN;
>
> *start_addr = start;
> *end_addr = end;
> return end - start + 1;
> }
>
> 06bc :
> 6bc: 81 43 00 00 lwz r10,0(r3)
> 6c0: a1 03 00 06 lhz r8,6(r3)
> 6c4: 39 2a ff ff addi r9,r10,-1
> 6c8: 7d 28 4a 14 add r9,r8,r9
> 6cc: 55 4a 00 38 rlwinm r10,r10,0,0,28
> 6d0: 61 29 00 07 ori r9,r9,7
> 6d4: 91 44 00 00 stw r10,0(r4)
> 6d8: 20 6a 00 01 subfic r3,r10,1
> 6dc: 91 25 00 00 stw r9,0(r5)
> 6e0: 7c 63 4a 14 add r3,r3,r9
> 6e4: 54 63 04 3e clrlwi r3,r3,16
> 6e8: 4e 80 00 20 blr
>
>
> And regardless, that's a pitty to have this function using pointers which are
> from local variables in the callers, as we loose the benefit of registers.
> Couldn't this function go in the .h as a static inline ? I'm sure the result
> would be worth it.
This is obviously a bit of optimization, but I like Mikey's idea of
storing start_addr and end_addr in the arch_hw_breakpoint. That way
we don't have to recalculate length every time in set_dawr.
Re: [PATCH 5/5] Powerpc/Watchpoint: Fix length calculation for unaligned target
On 6/18/19 7:02 PM, Michael Neuling wrote: > On Tue, 2019-06-18 at 09:57 +0530, Ravi Bangoria wrote: >> Watchpoint match range is always doubleword(8 bytes) aligned on >> powerpc. If the given range is crossing doubleword boundary, we >> need to increase the length such that next doubleword also get >> covered. Ex, >> >> address len = 6 bytes >> |=. >>|v--|--v| >>| | | | | | | | | | | | | | | | | >>|---|---| >> <---8 bytes---> >> >> In such case, current code configures hw as: >> start_addr = address & ~HW_BREAKPOINT_ALIGN >> len = 8 bytes >> >> And thus read/write in last 4 bytes of the given range is ignored. >> Fix this by including next doubleword in the length. Watchpoint >> exception handler already ignores extraneous exceptions, so no >> changes required for that. > > Nice catch. Thanks. > > I assume this has been broken forever? Should we be CCing stable? If so, it > would be nice to have this self contained (separate from the refactor) so we > can > more easily backport it. Yes this has been broken forever. I'll add Fixes: tag and cc stable. > > Also, can you update > tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c to catch this issue? Sure, will add the test case. [...] >> +u16 hw_breakpoint_get_final_len(struct arch_hw_breakpoint *brk, >> +unsigned long *start_addr, >> +unsigned long *end_addr) > > I don't really like this. "final" is not a good name. Something like hardware > would be better. > > Also, can you put the start_addr and end addr in the arch_hw_breakpoint rather > than doing what you have above. Call them hw_start_addr, hw_end_addr. > > We could even set these two new addresses where we set the set of > arch_hw_breakpoint rather than having this late call. Sure, will use 'hw_' prefix for them.
Re: [PATCH 0/5] Powerpc/hw-breakpoint: Fixes plus Code refactor
On 6/18/19 11:47 AM, Michael Neuling wrote: > On Tue, 2019-06-18 at 08:01 +0200, Christophe Leroy wrote: >> >> Le 18/06/2019 à 06:27, Ravi Bangoria a écrit : >>> patch 1-3: Code refactor >>> patch 4: Speedup disabling breakpoint >>> patch 5: Fix length calculation for unaligned targets >> >> While you are playing with hw breakpoints, did you have a look at >> https://github.com/linuxppc/issues/issues/38 ? > > Agreed and also: > > https://github.com/linuxppc/issues/issues/170 > > https://github.com/linuxppc/issues/issues/128 > Yes, I'm aware of those. Will have a look at them.
Re: [PATCH 2/2] powerpc/perf: Add support for outputting extended regs in perf intr_regs
Hi Anju,
Minor neats...
/*
diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h
b/arch/powerpc/include/uapi/asm/perf_regs.h
index f599064dd8dc..604b831378fe 100644
--- a/arch/powerpc/include/uapi/asm/perf_regs.h
+++ b/arch/powerpc/include/uapi/asm/perf_regs.h
@@ -48,6 +48,17 @@ enum perf_event_powerpc_regs {
PERF_REG_POWERPC_DSISR,
PERF_REG_POWERPC_SIER,
PERF_REG_POWERPC_MMCRA,
- PERF_REG_POWERPC_MAX,
+ /* Extended registers */
+ PERF_REG_POWERPC_MMCR0,
+ PERF_REG_POWERPC_MMCR1,
+ PERF_REG_POWERPC_MMCR2,
+ PERF_REG_EXTENDED_MAX,
+ /* Max regs without the extended regs */
+ PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
};
+
+#define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1)
Would it make sense to reuse PERF_REG_MASK? Userspace code already uses
that name for the same expression.
+#define PERF_REG_EXTENDED_MASK (((1ULL << (PERF_REG_EXTENDED_MAX)) \
+ - 1) - PERF_REG_PMU_MASK)
You don't need parenthesis in (PERF_REG_EXTENDED_MAX). Also, better to
keep that `- 1)` in first line.
+
#endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 3dcfecf858f3..f56b77800a7b 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2276,6 +2276,7 @@ int register_power_pmu(struct power_pmu *pmu)
power_pmu.attr_groups = ppmu->attr_groups;
+ power_pmu.capabilities |= (ppmu->capabilities & PERF_PMU_CAP_EXTENDED_REGS);
#ifdef MSR_HV
/*
* Use FCHV to ignore kernel events if MSR.HV is set.
diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
index a213a0aa5d25..57aa02568caf 100644
--- a/arch/powerpc/perf/perf_regs.c
+++ b/arch/powerpc/perf/perf_regs.c
@@ -15,7 +15,8 @@
#define PT_REGS_OFFSET(id, r) [id] = offsetof(struct pt_regs, r)
-#define REG_RESERVED (~((1ULL << PERF_REG_POWERPC_MAX) - 1))
+#define REG_RESERVED (~(PERF_REG_EXTENDED_MASK) & \
+ (~((1ULL << PERF_REG_POWERPC_MAX) - 1)))
Can we reuse PERF_REG_PMU_MASK here and simplify it to:
#define REG_RESERVED (~(PERF_REG_EXTENDED_MASK | PERF_REG_PMU_MASK))
static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
PT_REGS_OFFSET(PERF_REG_POWERPC_R0, gpr[0]),
@@ -69,10 +70,22 @@ static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
PT_REGS_OFFSET(PERF_REG_POWERPC_MMCRA, dsisr),
};
+/* Function to return the extended register values */
+static u64 get_ext_regs_value(int idx)
+{
+ switch (idx) {
+ case PERF_REG_POWERPC_MMCR0:
+ return mfspr(SPRN_MMCR0);
+ case PERF_REG_POWERPC_MMCR1:
+ return mfspr(SPRN_MMCR1);
+ case PERF_REG_POWERPC_MMCR2:
+ return mfspr(SPRN_MMCR2);
Unnecessary tabs.
[...]
diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
index f599064dd8dc..d66953294c73 100644
--- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
+++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
@@ -48,6 +48,17 @@ enum perf_event_powerpc_regs {
PERF_REG_POWERPC_DSISR,
PERF_REG_POWERPC_SIER,
PERF_REG_POWERPC_MMCRA,
- PERF_REG_POWERPC_MAX,
+ /* Extended arch registers */
+ PERF_REG_POWERPC_MMCR0,
+ PERF_REG_POWERPC_MMCR1,
+ PERF_REG_POWERPC_MMCR2,
+ PERF_REG_EXTENDED_MAX,
+ /* Max regs without extended arch regs */
+ PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
+
Unnecesasy line.
};
+#define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1)
+#define PERF_REG_EXTENDED_MASK (((1ULL << (PERF_REG_EXTENDED_MAX))\
+ - 1) - PERF_REG_PMU_MASK)
+
#endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
diff --git a/tools/perf/arch/powerpc/include/perf_regs.h
b/tools/perf/arch/powerpc/include/perf_regs.h
index e18a3556f5e3..f7bbdb816f88 100644
--- a/tools/perf/arch/powerpc/include/perf_regs.h
+++ b/tools/perf/arch/powerpc/include/perf_regs.h
@@ -64,7 +64,11 @@ static const char *reg_names[] = {
[PERF_REG_POWERPC_DAR] = "dar",
[PERF_REG_POWERPC_DSISR] = "dsisr",
[PERF_REG_POWERPC_SIER] = "sier",
- [PERF_REG_POWERPC_MMCRA] = "mmcra"
+ [PERF_REG_POWERPC_MMCRA] = "mmcra",
+ [PERF_REG_POWERPC_MMCR0] = "mmcr0",
+ [PERF_REG_POWERPC_MMCR1] = "mmcr1",
+ [PERF_REG_POWERPC_MMCR2] = "mmcr2",
+
Unnecesasy line.
Apart from those, for the series:
Reviewed-and-Tested-by: Ravi Bangoria
[PATCH] perf powerpc: Don't ignore sym-handling.c file
Commit 7eec00a74720 ("perf symbols: Consolidate symbol fixup issue")
removed powerpc specific sym-handling.c file from Build. This wasn't
caught by build CI because all functions in this file are declared
as __weak in common code. Fix it.
Fixes: 7eec00a74720 ("perf symbols: Consolidate symbol fixup issue")
Reported-by: Sandipan Das
Signed-off-by: Ravi Bangoria
---
tools/perf/arch/powerpc/util/Build | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/arch/powerpc/util/Build
b/tools/perf/arch/powerpc/util/Build
index e5c9504f8586..e86e210bf514 100644
--- a/tools/perf/arch/powerpc/util/Build
+++ b/tools/perf/arch/powerpc/util/Build
@@ -2,6 +2,7 @@ perf-y += header.o
perf-y += kvm-stat.o
perf-y += perf_regs.o
perf-y += mem-events.o
+perf-y += sym-handling.o
perf-$(CONFIG_DWARF) += dwarf-regs.o
perf-$(CONFIG_DWARF) += skip-callchain-idx.o
--
2.21.1
[PATCH v5 02/16] powerpc/watchpoint: Add SPRN macros for second DAWR
Power10 is introducing second DAWR. Add SPRN_ macros for the same. Signed-off-by: Ravi Bangoria Reviewed-by: Michael Neuling --- arch/powerpc/include/asm/reg.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index 156ee89fa9be..062e74cf41fd 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -284,6 +284,7 @@ #define CTRL_TE 0x00c0 /* thread enable */ #define CTRL_RUNLATCH0x1 #define SPRN_DAWR0 0xB4 +#define SPRN_DAWR1 0xB5 #define SPRN_RPR 0xBA/* Relative Priority Register */ #define SPRN_CIABR 0xBB #define CIABR_PRIV 0x3 @@ -291,6 +292,7 @@ #define CIABR_PRIV_SUPER 2 #define CIABR_PRIV_HYPER 3 #define SPRN_DAWRX00xBC +#define SPRN_DAWRX10xBD #define DAWRX_USER __MASK(0) #define DAWRX_KERNEL __MASK(1) #define DAWRX_HYP__MASK(2) -- 2.21.1
[PATCH v5 01/16] powerpc/watchpoint: Rename current DAWR macros
Power10 is introducing second DAWR. Use real register names from ISA
for current macros:
s/SPRN_DAWR/SPRN_DAWR0/
s/SPRN_DAWRX/SPRN_DAWRX0/
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/reg.h | 4 ++--
arch/powerpc/kernel/dawr.c | 4 ++--
arch/powerpc/kvm/book3s_hv.c| 12 ++--
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 18 +-
arch/powerpc/xmon/xmon.c| 2 +-
5 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index da5cab038e25..156ee89fa9be 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -283,14 +283,14 @@
#define CTRL_CT1 0x4000 /* thread 1 */
#define CTRL_TE 0x00c0 /* thread enable */
#define CTRL_RUNLATCH0x1
-#define SPRN_DAWR 0xB4
+#define SPRN_DAWR0 0xB4
#define SPRN_RPR 0xBA/* Relative Priority Register */
#define SPRN_CIABR 0xBB
#define CIABR_PRIV 0x3
#define CIABR_PRIV_USER 1
#define CIABR_PRIV_SUPER 2
#define CIABR_PRIV_HYPER 3
-#define SPRN_DAWRX 0xBC
+#define SPRN_DAWRX00xBC
#define DAWRX_USER __MASK(0)
#define DAWRX_KERNEL __MASK(1)
#define DAWRX_HYP__MASK(2)
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index cc14aa6c4a1b..e91b613bf137 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -39,8 +39,8 @@ int set_dawr(struct arch_hw_breakpoint *brk)
if (ppc_md.set_dawr)
return ppc_md.set_dawr(dawr, dawrx);
- mtspr(SPRN_DAWR, dawr);
- mtspr(SPRN_DAWRX, dawrx);
+ mtspr(SPRN_DAWR0, dawr);
+ mtspr(SPRN_DAWRX0, dawrx);
return 0;
}
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 93493f0cbfe8..db07199f0977 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3392,8 +3392,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu
*vcpu, u64 time_limit,
int trap;
unsigned long host_hfscr = mfspr(SPRN_HFSCR);
unsigned long host_ciabr = mfspr(SPRN_CIABR);
- unsigned long host_dawr = mfspr(SPRN_DAWR);
- unsigned long host_dawrx = mfspr(SPRN_DAWRX);
+ unsigned long host_dawr = mfspr(SPRN_DAWR0);
+ unsigned long host_dawrx = mfspr(SPRN_DAWRX0);
unsigned long host_psscr = mfspr(SPRN_PSSCR);
unsigned long host_pidr = mfspr(SPRN_PID);
@@ -3422,8 +3422,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu
*vcpu, u64 time_limit,
mtspr(SPRN_SPURR, vcpu->arch.spurr);
if (dawr_enabled()) {
- mtspr(SPRN_DAWR, vcpu->arch.dawr);
- mtspr(SPRN_DAWRX, vcpu->arch.dawrx);
+ mtspr(SPRN_DAWR0, vcpu->arch.dawr);
+ mtspr(SPRN_DAWRX0, vcpu->arch.dawrx);
}
mtspr(SPRN_CIABR, vcpu->arch.ciabr);
mtspr(SPRN_IC, vcpu->arch.ic);
@@ -3475,8 +3475,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu
*vcpu, u64 time_limit,
(local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
mtspr(SPRN_HFSCR, host_hfscr);
mtspr(SPRN_CIABR, host_ciabr);
- mtspr(SPRN_DAWR, host_dawr);
- mtspr(SPRN_DAWRX, host_dawrx);
+ mtspr(SPRN_DAWR0, host_dawr);
+ mtspr(SPRN_DAWRX0, host_dawrx);
mtspr(SPRN_PID, host_pidr);
/*
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 780a499c7114..70de3325d0e9 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -707,8 +707,8 @@ BEGIN_FTR_SECTION
END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
BEGIN_FTR_SECTION
mfspr r5, SPRN_CIABR
- mfspr r6, SPRN_DAWR
- mfspr r7, SPRN_DAWRX
+ mfspr r6, SPRN_DAWR0
+ mfspr r7, SPRN_DAWRX0
mfspr r8, SPRN_IAMR
std r5, STACK_SLOT_CIABR(r1)
std r6, STACK_SLOT_DAWR(r1)
@@ -803,8 +803,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
beq 1f
ld r5, VCPU_DAWR(r4)
ld r6, VCPU_DAWRX(r4)
- mtspr SPRN_DAWR, r5
- mtspr SPRN_DAWRX, r6
+ mtspr SPRN_DAWR0, r5
+ mtspr SPRN_DAWRX0, r6
1:
ld r7, VCPU_CIABR(r4)
ld r8, VCPU_TAR(r4)
@@ -1766,8 +1766,8 @@ BEGIN_FTR_SECTION
* 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
+ mtspr SPRN_DAWR0, r6
+ mtspr SPRN_DAWRX0, r7
END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
BEGIN_FTR_SECTION
ld r5, STACK_SLOT_TID(r1)
@@ -2577,8 +2577,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
mfmsr r6
andi. r6, r6, MSR_DR /* in real mode? */
[PATCH v5 05/16] powerpc/watchpoint: Provide DAWR number to set_dawr
Introduce new parameter 'nr' to set_dawr() which indicates which DAWR
should be programed.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/hw_breakpoint.h | 4 ++--
arch/powerpc/kernel/dawr.c | 15 ++-
arch/powerpc/kernel/process.c| 2 +-
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index 518b41eef924..5b3b02834e0b 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -104,10 +104,10 @@ static inline bool dawr_enabled(void)
{
return dawr_force_enable;
}
-int set_dawr(struct arch_hw_breakpoint *brk);
+int set_dawr(int nr, struct arch_hw_breakpoint *brk);
#else
static inline bool dawr_enabled(void) { return false; }
-static inline int set_dawr(struct arch_hw_breakpoint *brk) { return -1; }
+static inline int set_dawr(int nr, struct arch_hw_breakpoint *brk) { return
-1; }
#endif
#endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index e91b613bf137..8114ad3a8574 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -16,7 +16,7 @@
bool dawr_force_enable;
EXPORT_SYMBOL_GPL(dawr_force_enable);
-int set_dawr(struct arch_hw_breakpoint *brk)
+int set_dawr(int nr, struct arch_hw_breakpoint *brk)
{
unsigned long dawr, dawrx, mrd;
@@ -39,15 +39,20 @@ int set_dawr(struct arch_hw_breakpoint *brk)
if (ppc_md.set_dawr)
return ppc_md.set_dawr(dawr, dawrx);
- mtspr(SPRN_DAWR0, dawr);
- mtspr(SPRN_DAWRX0, dawrx);
+ if (nr == 0) {
+ mtspr(SPRN_DAWR0, dawr);
+ mtspr(SPRN_DAWRX0, dawrx);
+ } else {
+ mtspr(SPRN_DAWR1, dawr);
+ mtspr(SPRN_DAWRX1, dawrx);
+ }
return 0;
}
static void set_dawr_cb(void *info)
{
- set_dawr(info);
+ set_dawr(0, info);
}
static ssize_t dawr_write_file_bool(struct file *file,
@@ -60,7 +65,7 @@ static ssize_t dawr_write_file_bool(struct file *file,
/* Send error to user if they hypervisor won't allow us to write DAWR */
if (!dawr_force_enable &&
firmware_has_feature(FW_FEATURE_LPAR) &&
- set_dawr(_brk) != H_SUCCESS)
+ set_dawr(0, _brk) != H_SUCCESS)
return -ENODEV;
rc = debugfs_write_file_bool(file, user_buf, count, ppos);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 8479c762aef2..7488adf4d61c 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -806,7 +806,7 @@ void __set_breakpoint(struct arch_hw_breakpoint *brk)
if (dawr_enabled())
// Power8 or later
- set_dawr(brk);
+ set_dawr(0, brk);
else if (IS_ENABLED(CONFIG_PPC_8xx))
set_breakpoint_8xx(brk);
else if (!cpu_has_feature(CPU_FTR_ARCH_207S))
--
2.21.1
[PATCH v5 06/16] powerpc/watchpoint: Provide DAWR number to __set_breakpoint
Introduce new parameter 'nr' to __set_breakpoint() which indicates
which DAWR should be programed. Also convert current_brk variable
to an array.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/debug.h | 2 +-
arch/powerpc/include/asm/hw_breakpoint.h | 2 +-
arch/powerpc/kernel/hw_breakpoint.c | 8
arch/powerpc/kernel/process.c| 14 +++---
arch/powerpc/kernel/signal.c | 2 +-
arch/powerpc/xmon/xmon.c | 2 +-
6 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
index 7756026b95ca..ec57daf87f40 100644
--- a/arch/powerpc/include/asm/debug.h
+++ b/arch/powerpc/include/asm/debug.h
@@ -45,7 +45,7 @@ static inline int debugger_break_match(struct pt_regs *regs)
{ return 0; }
static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
#endif
-void __set_breakpoint(struct arch_hw_breakpoint *brk);
+void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk);
bool ppc_breakpoint_available(void);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
extern void do_send_trap(struct pt_regs *regs, unsigned long address,
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index 5b3b02834e0b..1120c7d9db58 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -85,7 +85,7 @@ static inline void hw_breakpoint_disable(void)
brk.len = 0;
brk.hw_len = 0;
if (ppc_breakpoint_available())
- __set_breakpoint();
+ __set_breakpoint(0, );
}
extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
int hw_breakpoint_handler(struct die_args *args);
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 4120349e2abe..5826f1f2cab9 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -63,7 +63,7 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
* If so, DABR will be populated in single_step_dabr_instruction().
*/
if (current->thread.last_hit_ubp != bp)
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
return 0;
}
@@ -221,7 +221,7 @@ void thread_change_pc(struct task_struct *tsk, struct
pt_regs *regs)
info = counter_arch_bp(tsk->thread.last_hit_ubp);
regs->msr &= ~MSR_SE;
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
tsk->thread.last_hit_ubp = NULL;
}
@@ -346,7 +346,7 @@ int hw_breakpoint_handler(struct die_args *args)
if (!(info->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
perf_bp_event(bp, regs);
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
out:
rcu_read_unlock();
return rc;
@@ -379,7 +379,7 @@ static int single_step_dabr_instruction(struct die_args
*args)
if (!(info->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
perf_bp_event(bp, regs);
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
current->thread.last_hit_ubp = NULL;
/*
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 7488adf4d61c..351fbd8d2c5b 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -637,7 +637,7 @@ void do_break (struct pt_regs *regs, unsigned long address,
}
#endif /* CONFIG_PPC_ADV_DEBUG_REGS */
-static DEFINE_PER_CPU(struct arch_hw_breakpoint, current_brk);
+static DEFINE_PER_CPU(struct arch_hw_breakpoint, current_brk[HBP_NUM_MAX]);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
/*
@@ -714,7 +714,7 @@ EXPORT_SYMBOL_GPL(switch_booke_debug_regs);
static void set_breakpoint(struct arch_hw_breakpoint *brk)
{
preempt_disable();
- __set_breakpoint(brk);
+ __set_breakpoint(0, brk);
preempt_enable();
}
@@ -800,13 +800,13 @@ static inline int set_breakpoint_8xx(struct
arch_hw_breakpoint *brk)
return 0;
}
-void __set_breakpoint(struct arch_hw_breakpoint *brk)
+void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
{
- memcpy(this_cpu_ptr(_brk), brk, sizeof(*brk));
+ memcpy(this_cpu_ptr(_brk[nr]), brk, sizeof(*brk));
if (dawr_enabled())
// Power8 or later
- set_dawr(0, brk);
+ set_dawr(nr, brk);
else if (IS_ENABLED(CONFIG_PPC_8xx))
set_breakpoint_8xx(brk);
else if (!cpu_has_feature(CPU_FTR_ARCH_207S))
@@ -1174,8 +1174,8 @@ struct task_struct *__switch_to(struct task_struct *prev,
* schedule DABR
*/
#ifndef CONFIG_HAVE_HW_BREAKPOINT
- if (unlikely(!hw_brk_match(this_cpu_ptr(_brk),
>thread.hw_brk)))
- __set_breakpoint(>thread.hw_brk);
+ if (unlikely(!hw_brk_match(this_cpu_ptr(_brk[0]),
>thread.hw_brk)))
+ __
[PATCH v5 04/16] powerpc/watchpoint/ptrace: Return actual num of available watchpoints
User can ask for num of available watchpoints(dbginfo.num_data_bps) using ptrace(PPC_PTRACE_GETHWDBGINFO). Return actual number of available watchpoints on the machine rather than hardcoded 1. Signed-off-by: Ravi Bangoria Reviewed-by: Michael Neuling --- arch/powerpc/kernel/ptrace/ptrace-noadv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c b/arch/powerpc/kernel/ptrace/ptrace-noadv.c index f87e7c5c3bf3..12962302d6a4 100644 --- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c +++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c @@ -44,7 +44,7 @@ void ppc_gethwdinfo(struct ppc_debug_info *dbginfo) dbginfo->version = 1; dbginfo->num_instruction_bps = 0; if (ppc_breakpoint_available()) - dbginfo->num_data_bps = 1; + dbginfo->num_data_bps = nr_wp_slots(); else dbginfo->num_data_bps = 0; dbginfo->num_condition_regs = 0; -- 2.21.1
[PATCH v5 00/16] powerpc/watchpoint: Preparation for more than one watchpoint
So far, powerpc Book3S code has been written with an assumption of only one watchpoint. But Power10[1] is introducing second watchpoint register (DAWR). Even though this patchset does not enable 2nd DAWR, it makes the infrastructure ready so that enabling 2nd DAWR should just be a matter of changing count. Existing functionality works fine with the patchset. I've tested it with perf, ptrace(gdb), xmon. All hw-breakpoint selftests are passing as well. And I've build tested for 8xx and 'AMCC 44x, 46x or 47x'. Note: kvm or PowerVM guest is not enabled yet. v4: https://lore.kernel.org/linuxppc-dev/20200430043417.30948-1-ravi.bango...@linux.ibm.com v4->v5: - s/future power architecture/Power10/ - s/dawr/DAWR/ in commit descriptions [1]: https://www-355.ibm.com/systems/power/openpower/ Ravi Bangoria (16): powerpc/watchpoint: Rename current DAWR macros powerpc/watchpoint: Add SPRN macros for second DAWR powerpc/watchpoint: Introduce function to get nr watchpoints dynamically powerpc/watchpoint/ptrace: Return actual num of available watchpoints powerpc/watchpoint: Provide DAWR number to set_dawr powerpc/watchpoint: Provide DAWR number to __set_breakpoint powerpc/watchpoint: Get watchpoint count dynamically while disabling them powerpc/watchpoint: Disable all available watchpoints when !dawr_force_enable powerpc/watchpoint: Convert thread_struct->hw_brk to an array powerpc/watchpoint: Use loop for thread_struct->ptrace_bps powerpc/watchpoint: Introduce is_ptrace_bp() function powerpc/watchpoint: Use builtin ALIGN*() macros powerpc/watchpoint: Prepare handler to handle more than one watcnhpoint powerpc/watchpoint: Don't allow concurrent perf and ptrace events powerpc/watchpoint/xmon: Don't allow breakpoint overwriting powerpc/watchpoint/xmon: Support 2nd DAWR arch/powerpc/include/asm/cputable.h | 6 +- arch/powerpc/include/asm/debug.h | 2 +- arch/powerpc/include/asm/hw_breakpoint.h | 32 +- arch/powerpc/include/asm/processor.h | 6 +- arch/powerpc/include/asm/reg.h| 6 +- arch/powerpc/include/asm/sstep.h | 2 + arch/powerpc/kernel/dawr.c| 23 +- arch/powerpc/kernel/hw_breakpoint.c | 645 ++ arch/powerpc/kernel/process.c | 85 +-- arch/powerpc/kernel/ptrace/ptrace-noadv.c | 72 ++- arch/powerpc/kernel/ptrace/ptrace32.c | 4 +- arch/powerpc/kernel/signal.c | 13 +- arch/powerpc/kvm/book3s_hv.c | 12 +- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 18 +- arch/powerpc/xmon/xmon.c | 99 +++- kernel/events/hw_breakpoint.c | 16 + 16 files changed, 814 insertions(+), 227 deletions(-) -- 2.21.1
[PATCH v5 03/16] powerpc/watchpoint: Introduce function to get nr watchpoints dynamically
So far we had only one watchpoint, so we have hardcoded HBP_NUM to 1.
But Power10 is introducing 2nd DAWR and thus kernel should be able to
dynamically find actual number of watchpoints supported by hw it's
running on. Introduce function for the same. Also convert HBP_NUM macro
to HBP_NUM_MAX, which will now represent maximum number of watchpoints
supported by Powerpc.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/cputable.h | 6 +-
arch/powerpc/include/asm/hw_breakpoint.h | 5 +
arch/powerpc/include/asm/processor.h | 2 +-
arch/powerpc/kernel/hw_breakpoint.c | 2 +-
4 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h
b/arch/powerpc/include/asm/cputable.h
index 40a4d3c6fd99..c67b94f3334c 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -614,7 +614,11 @@ enum {
};
#endif /* __powerpc64__ */
-#define HBP_NUM 1
+/*
+ * Maximum number of hw breakpoint supported on powerpc. Number of
+ * breakpoints supported by actual hw might be less than this.
+ */
+#define HBP_NUM_MAX1
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index f2f8d8aa8e3b..518b41eef924 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -43,6 +43,11 @@ struct arch_hw_breakpoint {
#define DABR_MAX_LEN 8
#define DAWR_MAX_LEN 512
+static inline int nr_wp_slots(void)
+{
+ return HBP_NUM_MAX;
+}
+
#ifdef CONFIG_HAVE_HW_BREAKPOINT
#include
#include
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index bfa336fbcfeb..a71bdd6bc284 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -180,7 +180,7 @@ struct thread_struct {
int fpexc_mode; /* floating-point exception mode */
unsigned intalign_ctl; /* alignment handling control */
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- struct perf_event *ptrace_bps[HBP_NUM];
+ struct perf_event *ptrace_bps[HBP_NUM_MAX];
/*
* Helps identify source of single-step exception and subsequent
* hw-breakpoint enablement
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 72f461bd70fb..4120349e2abe 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -38,7 +38,7 @@ static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
int hw_breakpoint_slots(int type)
{
if (type == TYPE_DATA)
- return HBP_NUM;
+ return nr_wp_slots();
return 0; /* no instruction breakpoints available */
}
--
2.21.1
[PATCH v5 07/16] powerpc/watchpoint: Get watchpoint count dynamically while disabling them
Instead of disabling only one watchpoint, get num of available
watchpoints dynamically and disable all of them.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/hw_breakpoint.h | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index 1120c7d9db58..d472b2eb757e 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -78,14 +78,14 @@ extern void ptrace_triggered(struct perf_event *bp,
struct perf_sample_data *data, struct pt_regs *regs);
static inline void hw_breakpoint_disable(void)
{
- struct arch_hw_breakpoint brk;
-
- brk.address = 0;
- brk.type = 0;
- brk.len = 0;
- brk.hw_len = 0;
- if (ppc_breakpoint_available())
- __set_breakpoint(0, );
+ int i;
+ struct arch_hw_breakpoint null_brk = {0};
+
+ if (!ppc_breakpoint_available())
+ return;
+
+ for (i = 0; i < nr_wp_slots(); i++)
+ __set_breakpoint(i, _brk);
}
extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
int hw_breakpoint_handler(struct die_args *args);
--
2.21.1
[PATCH v5 12/16] powerpc/watchpoint: Use builtin ALIGN*() macros
Currently we calculate hw aligned start and end addresses manually.
Replace them with builtin ALIGN_DOWN() and ALIGN() macros.
So far end_addr was inclusive but this patch makes it exclusive (by
avoiding -1) for better readability.
Suggested-by: Christophe Leroy
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/hw_breakpoint.h | 5 +++--
arch/powerpc/kernel/hw_breakpoint.c | 12 ++--
arch/powerpc/kernel/process.c | 8
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 2 +-
4 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index d472b2eb757e..add5aa076919 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -34,10 +34,11 @@ struct arch_hw_breakpoint {
#define HW_BRK_TYPE_PRIV_ALL (HW_BRK_TYPE_USER | HW_BRK_TYPE_KERNEL | \
HW_BRK_TYPE_HYP)
+/* Minimum granularity */
#ifdef CONFIG_PPC_8xx
-#define HW_BREAKPOINT_ALIGN 0x3
+#define HW_BREAKPOINT_SIZE 0x4
#else
-#define HW_BREAKPOINT_ALIGN 0x7
+#define HW_BREAKPOINT_SIZE 0x8
#endif
#define DABR_MAX_LEN 8
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 319a761b7412..ab0dd22fed5f 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -145,10 +145,10 @@ int arch_bp_generic_fields(int type, int *gen_bp_type)
*<---8 bytes--->
*
* In this case, we should configure hw as:
- * start_addr = address & ~HW_BREAKPOINT_ALIGN
+ * start_addr = address & ~(HW_BREAKPOINT_SIZE - 1)
* len = 16 bytes
*
- * @start_addr and @end_addr are inclusive.
+ * @start_addr is inclusive but @end_addr is exclusive.
*/
static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
{
@@ -156,14 +156,14 @@ static int hw_breakpoint_validate_len(struct
arch_hw_breakpoint *hw)
u16 hw_len;
unsigned long start_addr, end_addr;
- start_addr = hw->address & ~HW_BREAKPOINT_ALIGN;
- end_addr = (hw->address + hw->len - 1) | HW_BREAKPOINT_ALIGN;
- hw_len = end_addr - start_addr + 1;
+ start_addr = ALIGN_DOWN(hw->address, HW_BREAKPOINT_SIZE);
+ end_addr = ALIGN(hw->address + hw->len, HW_BREAKPOINT_SIZE);
+ hw_len = end_addr - start_addr;
if (dawr_enabled()) {
max_len = DAWR_MAX_LEN;
/* DAWR region can't cross 512 bytes boundary */
- if ((start_addr >> 9) != (end_addr >> 9))
+ if (ALIGN(start_addr, SZ_512M) != ALIGN(end_addr - 1, SZ_512M))
return -EINVAL;
} else if (IS_ENABLED(CONFIG_PPC_8xx)) {
/* 8xx can setup a range without limitation */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 41a59a37383b..dcf9c5b4ac59 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -800,12 +800,12 @@ static inline int set_breakpoint_8xx(struct
arch_hw_breakpoint *brk)
unsigned long lctrl1 = LCTRL1_CTE_GT | LCTRL1_CTF_LT | LCTRL1_CRWE_RW |
LCTRL1_CRWF_RW;
unsigned long lctrl2 = LCTRL2_LW0EN | LCTRL2_LW0LADC | LCTRL2_SLW0EN;
- unsigned long start_addr = brk->address & ~HW_BREAKPOINT_ALIGN;
- unsigned long end_addr = (brk->address + brk->len - 1) |
HW_BREAKPOINT_ALIGN;
+ unsigned long start_addr = ALIGN_DOWN(brk->address, HW_BREAKPOINT_SIZE);
+ unsigned long end_addr = ALIGN(brk->address + brk->len,
HW_BREAKPOINT_SIZE);
if (start_addr == 0)
lctrl2 |= LCTRL2_LW0LA_F;
- else if (end_addr == ~0U)
+ else if (end_addr == 0)
lctrl2 |= LCTRL2_LW0LA_E;
else
lctrl2 |= LCTRL2_LW0LA_EandF;
@@ -821,7 +821,7 @@ static inline int set_breakpoint_8xx(struct
arch_hw_breakpoint *brk)
lctrl1 |= LCTRL1_CRWE_WO | LCTRL1_CRWF_WO;
mtspr(SPRN_CMPE, start_addr - 1);
- mtspr(SPRN_CMPF, end_addr + 1);
+ mtspr(SPRN_CMPF, end_addr);
mtspr(SPRN_LCTRL1, lctrl1);
mtspr(SPRN_LCTRL2, lctrl2);
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 08cb8c1b504c..697c7e4b5877 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -216,7 +216,7 @@ long ppc_set_hwdebug(struct task_struct *child, struct
ppc_hw_breakpoint *bp_inf
if ((unsigned long)bp_info->addr >= TASK_SIZE)
return -EIO;
- brk.address = bp_info->addr & ~HW_BREAKPOINT_ALIGN;
+ brk.address = ALIGN_DOWN(bp_info->addr, HW_BREAKPOINT_SIZE);
brk.type = HW_BRK_TYPE_TRANSLATE;
brk.len = DABR_MAX_LEN;
if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
--
2.21.1
[PATCH v5 11/16] powerpc/watchpoint: Introduce is_ptrace_bp() function
Introduce is_ptrace_bp() function and move the check inside the
function. It will be utilize more in later set of patches.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/kernel/hw_breakpoint.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 772b2c953220..319a761b7412 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -90,6 +90,11 @@ void arch_uninstall_hw_breakpoint(struct perf_event *bp)
hw_breakpoint_disable();
}
+static bool is_ptrace_bp(struct perf_event *bp)
+{
+ return bp->overflow_handler == ptrace_triggered;
+}
+
/*
* Perform cleanup of arch-specific counters during unregistration
* of the perf-event
@@ -324,7 +329,7 @@ int hw_breakpoint_handler(struct die_args *args)
* one-shot mode. The ptrace-ed process will receive the SIGTRAP signal
* generated in do_dabr().
*/
- if (bp->overflow_handler == ptrace_triggered) {
+ if (is_ptrace_bp(bp)) {
perf_bp_event(bp, regs);
rc = NOTIFY_DONE;
goto out;
--
2.21.1
[PATCH v5 14/16] powerpc/watchpoint: Don't allow concurrent perf and ptrace events
With Book3s DAWR, ptrace and perf watchpoints on powerpc behaves
differently. Ptrace watchpoint works in one-shot mode and generates
signal before executing instruction. It's ptrace user's job to
single-step the instruction and re-enable the watchpoint. OTOH, in
case of perf watchpoint, kernel emulates/single-steps the instruction
and then generates event. If perf and ptrace creates two events with
same or overlapping address ranges, it's ambiguous to decide who
should single-step the instruction. Because of this issue, don't
allow perf and ptrace watchpoint at the same time if their address
range overlaps.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/hw_breakpoint.h | 2 +
arch/powerpc/kernel/hw_breakpoint.c | 221 +++
kernel/events/hw_breakpoint.c| 16 ++
3 files changed, 239 insertions(+)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index add5aa076919..f42a55eb77d2 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -70,6 +70,8 @@ extern int hw_breakpoint_exceptions_notify(struct
notifier_block *unused,
unsigned long val, void *data);
int arch_install_hw_breakpoint(struct perf_event *bp);
void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+int arch_reserve_bp_slot(struct perf_event *bp);
+void arch_release_bp_slot(struct perf_event *bp);
void arch_unregister_hw_breakpoint(struct perf_event *bp);
void hw_breakpoint_pmu_read(struct perf_event *bp);
extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 28d57d841642..c8623708c9c7 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -123,6 +123,227 @@ static bool is_ptrace_bp(struct perf_event *bp)
return bp->overflow_handler == ptrace_triggered;
}
+struct breakpoint {
+ struct list_head list;
+ struct perf_event *bp;
+ bool ptrace_bp;
+};
+
+static DEFINE_PER_CPU(struct breakpoint *, cpu_bps[HBP_NUM_MAX]);
+static LIST_HEAD(task_bps);
+
+static struct breakpoint *alloc_breakpoint(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ tmp = kzalloc(sizeof(*tmp), GFP_KERNEL);
+ if (!tmp)
+ return ERR_PTR(-ENOMEM);
+ tmp->bp = bp;
+ tmp->ptrace_bp = is_ptrace_bp(bp);
+ return tmp;
+}
+
+static bool bp_addr_range_overlap(struct perf_event *bp1, struct perf_event
*bp2)
+{
+ __u64 bp1_saddr, bp1_eaddr, bp2_saddr, bp2_eaddr;
+
+ bp1_saddr = ALIGN_DOWN(bp1->attr.bp_addr, HW_BREAKPOINT_SIZE);
+ bp1_eaddr = ALIGN(bp1->attr.bp_addr + bp1->attr.bp_len,
HW_BREAKPOINT_SIZE);
+ bp2_saddr = ALIGN_DOWN(bp2->attr.bp_addr, HW_BREAKPOINT_SIZE);
+ bp2_eaddr = ALIGN(bp2->attr.bp_addr + bp2->attr.bp_len,
HW_BREAKPOINT_SIZE);
+
+ return (bp1_saddr < bp2_eaddr && bp1_eaddr > bp2_saddr);
+}
+
+static bool alternate_infra_bp(struct breakpoint *b, struct perf_event *bp)
+{
+ return is_ptrace_bp(bp) ? !b->ptrace_bp : b->ptrace_bp;
+}
+
+static bool can_co_exist(struct breakpoint *b, struct perf_event *bp)
+{
+ return !(alternate_infra_bp(b, bp) && bp_addr_range_overlap(b->bp, bp));
+}
+
+static int task_bps_add(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ tmp = alloc_breakpoint(bp);
+ if (IS_ERR(tmp))
+ return PTR_ERR(tmp);
+
+ list_add(>list, _bps);
+ return 0;
+}
+
+static void task_bps_remove(struct perf_event *bp)
+{
+ struct list_head *pos, *q;
+
+ list_for_each_safe(pos, q, _bps) {
+ struct breakpoint *tmp = list_entry(pos, struct breakpoint,
list);
+
+ if (tmp->bp == bp) {
+ list_del(>list);
+ kfree(tmp);
+ break;
+ }
+ }
+}
+
+/*
+ * If any task has breakpoint from alternate infrastructure,
+ * return true. Otherwise return false.
+ */
+static bool all_task_bps_check(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ list_for_each_entry(tmp, _bps, list) {
+ if (!can_co_exist(tmp, bp))
+ return true;
+ }
+ return false;
+}
+
+/*
+ * If same task has breakpoint from alternate infrastructure,
+ * return true. Otherwise return false.
+ */
+static bool same_task_bps_check(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ list_for_each_entry(tmp, _bps, list) {
+ if (tmp->bp->hw.target == bp->hw.target &&
+ !can_co_exist(tmp, bp))
+ return true;
+ }
+ return false;
+}
+
+static int cpu_bps_add(struct perf_event *bp)
+{
+ struct breakpoint **cpu_bp;
+
[PATCH v5 08/16] powerpc/watchpoint: Disable all available watchpoints when !dawr_force_enable
Instead of disabling only first watchpoint, disable all available
watchpoints while clearing dawr_force_enable.
Callback function is used only for disabling watchpoint, rename it
to disable_dawrs_cb(). And null_brk parameter is not really required
while disabling watchpoint, remove it.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/kernel/dawr.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index 8114ad3a8574..500f52fa4711 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -50,9 +50,13 @@ int set_dawr(int nr, struct arch_hw_breakpoint *brk)
return 0;
}
-static void set_dawr_cb(void *info)
+static void disable_dawrs_cb(void *info)
{
- set_dawr(0, info);
+ struct arch_hw_breakpoint null_brk = {0};
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++)
+ set_dawr(i, _brk);
}
static ssize_t dawr_write_file_bool(struct file *file,
@@ -74,7 +78,7 @@ static ssize_t dawr_write_file_bool(struct file *file,
/* If we are clearing, make sure all CPUs have the DAWR cleared */
if (!dawr_force_enable)
- smp_call_function(set_dawr_cb, _brk, 0);
+ smp_call_function(disable_dawrs_cb, NULL, 0);
return rc;
}
--
2.21.1
[PATCH v5 16/16] powerpc/watchpoint/xmon: Support 2nd DAWR
Add support for 2nd DAWR in xmon. With this, we can have two
simultaneous breakpoints from xmon.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/xmon/xmon.c | 101 ++-
1 file changed, 69 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 99e9138661e4..01da49b666db 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -111,7 +111,7 @@ struct bpt {
#define NBPTS 256
static struct bpt bpts[NBPTS];
-static struct bpt dabr;
+static struct bpt dabr[HBP_NUM_MAX];
static struct bpt *iabr;
static unsigned bpinstr = 0x7fe8; /* trap */
@@ -787,10 +787,17 @@ static int xmon_sstep(struct pt_regs *regs)
static int xmon_break_match(struct pt_regs *regs)
{
+ int i;
+
if ((regs->msr & (MSR_IR|MSR_PR|MSR_64BIT)) != (MSR_IR|MSR_64BIT))
return 0;
- if (dabr.enabled == 0)
- return 0;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (dabr[i].enabled)
+ goto found;
+ }
+ return 0;
+
+found:
xmon_core(regs, 0);
return 1;
}
@@ -929,13 +936,16 @@ static void insert_bpts(void)
static void insert_cpu_bpts(void)
{
+ int i;
struct arch_hw_breakpoint brk;
- if (dabr.enabled) {
- brk.address = dabr.address;
- brk.type = (dabr.enabled & HW_BRK_TYPE_DABR) |
HW_BRK_TYPE_PRIV_ALL;
- brk.len = DABR_MAX_LEN;
- __set_breakpoint(0, );
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (dabr[i].enabled) {
+ brk.address = dabr[i].address;
+ brk.type = (dabr[i].enabled & HW_BRK_TYPE_DABR) |
HW_BRK_TYPE_PRIV_ALL;
+ brk.len = 8;
+ __set_breakpoint(i, );
+ }
}
if (iabr)
@@ -1349,6 +1359,35 @@ static long check_bp_loc(unsigned long addr)
return 1;
}
+static int find_free_data_bpt(void)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!dabr[i].enabled)
+ return i;
+ }
+ printf("Couldn't find free breakpoint register\n");
+ return -1;
+}
+
+static void print_data_bpts(void)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!dabr[i].enabled)
+ continue;
+
+ printf(" data "REG" [", dabr[i].address);
+ if (dabr[i].enabled & 1)
+ printf("r");
+ if (dabr[i].enabled & 2)
+ printf("w");
+ printf("]\n");
+ }
+}
+
static char *breakpoint_help_string =
"Breakpoint command usage:\n"
"bshow breakpoints\n"
@@ -1382,10 +1421,9 @@ bpt_cmds(void)
printf("Hardware data breakpoint not supported on this
cpu\n");
break;
}
- if (dabr.enabled) {
- printf("Couldn't find free breakpoint register\n");
+ i = find_free_data_bpt();
+ if (i < 0)
break;
- }
mode = 7;
cmd = inchar();
if (cmd == 'r')
@@ -1394,15 +1432,15 @@ bpt_cmds(void)
mode = 6;
else
termch = cmd;
- dabr.address = 0;
- dabr.enabled = 0;
- if (scanhex()) {
- if (!is_kernel_addr(dabr.address)) {
+ dabr[i].address = 0;
+ dabr[i].enabled = 0;
+ if (scanhex([i].address)) {
+ if (!is_kernel_addr(dabr[i].address)) {
printf(badaddr);
break;
}
- dabr.address &= ~HW_BRK_TYPE_DABR;
- dabr.enabled = mode | BP_DABR;
+ dabr[i].address &= ~HW_BRK_TYPE_DABR;
+ dabr[i].enabled = mode | BP_DABR;
}
force_enable_xmon();
@@ -1441,7 +1479,9 @@ bpt_cmds(void)
for (i = 0; i < NBPTS; ++i)
bpts[i].enabled = 0;
iabr = NULL;
- dabr.enabled = 0;
+ for (i = 0; i < nr_wp_slots(); i++)
+ dabr[i].enabled = 0;
+
printf("All breakpoints cleared\n");
break;
}
@@ -1475,14 +1515,7 @@ bpt_cmds(void)
if (xmon_is_ro || !scanhex()) {
/* print all breakpoints */
[PATCH v5 09/16] powerpc/watchpoint: Convert thread_struct->hw_brk to an array
So far powerpc hw supported only one watchpoint. But Power10 is
introducing 2nd DAWR. Convert thread_struct->hw_brk into an array.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/processor.h | 2 +-
arch/powerpc/kernel/process.c | 60 ++-
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 40 ++-
arch/powerpc/kernel/ptrace/ptrace32.c | 4 +-
arch/powerpc/kernel/signal.c | 13 +++--
5 files changed, 78 insertions(+), 41 deletions(-)
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index a71bdd6bc284..668c02c67b61 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -187,7 +187,7 @@ struct thread_struct {
*/
struct perf_event *last_hit_ubp;
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
- struct arch_hw_breakpoint hw_brk; /* info on the hardware breakpoint */
+ struct arch_hw_breakpoint hw_brk[HBP_NUM_MAX]; /* hardware breakpoint
info */
unsigned long trap_nr;/* last trap # on this thread */
u8 load_slb;/* Ages out SLB preload cache entries */
u8 load_fp;
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 351fbd8d2c5b..6d1b7cede900 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -711,21 +711,49 @@ void switch_booke_debug_regs(struct debug_reg *new_debug)
EXPORT_SYMBOL_GPL(switch_booke_debug_regs);
#else /* !CONFIG_PPC_ADV_DEBUG_REGS */
#ifndef CONFIG_HAVE_HW_BREAKPOINT
-static void set_breakpoint(struct arch_hw_breakpoint *brk)
+static void set_breakpoint(int i, struct arch_hw_breakpoint *brk)
{
preempt_disable();
- __set_breakpoint(0, brk);
+ __set_breakpoint(i, brk);
preempt_enable();
}
static void set_debug_reg_defaults(struct thread_struct *thread)
{
- thread->hw_brk.address = 0;
- thread->hw_brk.type = 0;
- thread->hw_brk.len = 0;
- thread->hw_brk.hw_len = 0;
- if (ppc_breakpoint_available())
- set_breakpoint(>hw_brk);
+ int i;
+ struct arch_hw_breakpoint null_brk = {0};
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ thread->hw_brk[i] = null_brk;
+ if (ppc_breakpoint_available())
+ set_breakpoint(i, >hw_brk[i]);
+ }
+}
+
+static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
+ struct arch_hw_breakpoint *b)
+{
+ if (a->address != b->address)
+ return false;
+ if (a->type != b->type)
+ return false;
+ if (a->len != b->len)
+ return false;
+ /* no need to check hw_len. it's calculated from address and len */
+ return true;
+}
+
+static void switch_hw_breakpoint(struct task_struct *new)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (likely(hw_brk_match(this_cpu_ptr(_brk[i]),
+ >thread.hw_brk[i])))
+ continue;
+
+ __set_breakpoint(i, >thread.hw_brk[i]);
+ }
}
#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
#endif /* CONFIG_PPC_ADV_DEBUG_REGS */
@@ -829,19 +857,6 @@ bool ppc_breakpoint_available(void)
}
EXPORT_SYMBOL_GPL(ppc_breakpoint_available);
-static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
- struct arch_hw_breakpoint *b)
-{
- if (a->address != b->address)
- return false;
- if (a->type != b->type)
- return false;
- if (a->len != b->len)
- return false;
- /* no need to check hw_len. it's calculated from address and len */
- return true;
-}
-
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
static inline bool tm_enabled(struct task_struct *tsk)
@@ -1174,8 +1189,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
* schedule DABR
*/
#ifndef CONFIG_HAVE_HW_BREAKPOINT
- if (unlikely(!hw_brk_match(this_cpu_ptr(_brk[0]),
>thread.hw_brk)))
- __set_breakpoint(0, >thread.hw_brk);
+ switch_hw_breakpoint(new);
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 12962302d6a4..0dbb35392dd2 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -67,11 +67,16 @@ int ptrace_get_debugreg(struct task_struct *child, unsigned
long addr,
/* We only support one DABR and no IABRS at the moment */
if (addr > 0)
return -EINVAL;
- dabr_fake = ((child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
-(child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
+ dabr_fake = ((child->th
[PATCH v5 15/16] powerpc/watchpoint/xmon: Don't allow breakpoint overwriting
Xmon allows overwriting breakpoints because it's supported by only
one DAWR. But with multiple DAWRs, overwriting becomes ambiguous
or unnecessary complicated. So let's not allow it.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/xmon/xmon.c | 4
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index d8c0f01e4b24..99e9138661e4 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1382,6 +1382,10 @@ bpt_cmds(void)
printf("Hardware data breakpoint not supported on this
cpu\n");
break;
}
+ if (dabr.enabled) {
+ printf("Couldn't find free breakpoint register\n");
+ break;
+ }
mode = 7;
cmd = inchar();
if (cmd == 'r')
--
2.21.1
[PATCH v5 10/16] powerpc/watchpoint: Use loop for thread_struct->ptrace_bps
ptrace_bps is already an array of size HBP_NUM_MAX. But we use
hardcoded index 0 while fetching/updating it. Convert such code
to loop over array.
ptrace interface to use multiple watchpoint remains same. eg:
two PPC_PTRACE_SETHWDEBUG calls will create two watchpoint if
underneath hw supports it.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/kernel/hw_breakpoint.c | 7 --
arch/powerpc/kernel/process.c | 6 -
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 28 +--
3 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 5826f1f2cab9..772b2c953220 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -419,10 +419,13 @@ NOKPROBE_SYMBOL(hw_breakpoint_exceptions_notify);
*/
void flush_ptrace_hw_breakpoint(struct task_struct *tsk)
{
+ int i;
struct thread_struct *t = >thread;
- unregister_hw_breakpoint(t->ptrace_bps[0]);
- t->ptrace_bps[0] = NULL;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ unregister_hw_breakpoint(t->ptrace_bps[i]);
+ t->ptrace_bps[i] = NULL;
+ }
}
void hw_breakpoint_pmu_read(struct perf_event *bp)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 6d1b7cede900..41a59a37383b 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1604,6 +1604,9 @@ int copy_thread_tls(unsigned long clone_flags, unsigned
long usp,
void (*f)(void);
unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
struct thread_info *ti = task_thread_info(p);
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ int i;
+#endif
klp_init_thread_info(p);
@@ -1663,7 +1666,8 @@ int copy_thread_tls(unsigned long clone_flags, unsigned
long usp,
p->thread.ksp_limit = (unsigned long)end_of_stack(p);
#endif
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- p->thread.ptrace_bps[0] = NULL;
+ for (i = 0; i < nr_wp_slots(); i++)
+ p->thread.ptrace_bps[i] = NULL;
#endif
p->thread.fp_save_area = NULL;
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 0dbb35392dd2..08cb8c1b504c 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -168,6 +168,19 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned
long addr, unsigned l
return 0;
}
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+static int find_empty_ptrace_bp(struct thread_struct *thread)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!thread->ptrace_bps[i])
+ return i;
+ }
+ return -1;
+}
+#endif
+
static int find_empty_hw_brk(struct thread_struct *thread)
{
int i;
@@ -217,8 +230,9 @@ long ppc_set_hwdebug(struct task_struct *child, struct
ppc_hw_breakpoint *bp_inf
len = 1;
else
return -EINVAL;
- bp = thread->ptrace_bps[0];
- if (bp)
+
+ i = find_empty_ptrace_bp(thread);
+ if (i < 0)
return -ENOSPC;
/* Create a new breakpoint request if one doesn't exist already */
@@ -228,13 +242,13 @@ long ppc_set_hwdebug(struct task_struct *child, struct
ppc_hw_breakpoint *bp_inf
arch_bp_generic_fields(brk.type, _type);
bp = register_user_hw_breakpoint(, ptrace_triggered, NULL, child);
- thread->ptrace_bps[0] = bp;
+ thread->ptrace_bps[i] = bp;
if (IS_ERR(bp)) {
- thread->ptrace_bps[0] = NULL;
+ thread->ptrace_bps[i] = NULL;
return PTR_ERR(bp);
}
- return 1;
+ return i + 1;
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT)
@@ -263,10 +277,10 @@ long ppc_del_hwdebug(struct task_struct *child, long data)
return -EINVAL;
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- bp = thread->ptrace_bps[0];
+ bp = thread->ptrace_bps[data - 1];
if (bp) {
unregister_hw_breakpoint(bp);
- thread->ptrace_bps[0] = NULL;
+ thread->ptrace_bps[data - 1] = NULL;
} else {
ret = -ENOENT;
}
--
2.21.1
[PATCH v5 13/16] powerpc/watchpoint: Prepare handler to handle more than one watcnhpoint
Currently we assume that we have only one watchpoint supported by hw.
Get rid of that assumption and use dynamic loop instead. This should
make supporting more watchpoints very easy.
With more than one watchpoint, exception handler needs to know which
DAWR caused the exception, and hw currently does not provide it. So
we need sw logic for the same. To figure out which DAWR caused the
exception, check all different combinations of user specified range,
DAWR address range, actual access range and DAWRX constrains. For ex,
if user specified range and actual access range overlaps but DAWRX is
configured for readonly watchpoint and the instruction is store, this
DAWR must not have caused exception.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/processor.h | 2 +-
arch/powerpc/include/asm/sstep.h | 2 +
arch/powerpc/kernel/hw_breakpoint.c | 400 +--
arch/powerpc/kernel/process.c| 3 -
4 files changed, 315 insertions(+), 92 deletions(-)
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index 668c02c67b61..251f50eec9fa 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -185,7 +185,7 @@ struct thread_struct {
* Helps identify source of single-step exception and subsequent
* hw-breakpoint enablement
*/
- struct perf_event *last_hit_ubp;
+ struct perf_event *last_hit_ubp[HBP_NUM_MAX];
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
struct arch_hw_breakpoint hw_brk[HBP_NUM_MAX]; /* hardware breakpoint
info */
unsigned long trap_nr;/* last trap # on this thread */
diff --git a/arch/powerpc/include/asm/sstep.h b/arch/powerpc/include/asm/sstep.h
index 769f055509c9..38919b27a6fa 100644
--- a/arch/powerpc/include/asm/sstep.h
+++ b/arch/powerpc/include/asm/sstep.h
@@ -48,6 +48,8 @@ enum instruction_type {
#define INSTR_TYPE_MASK0x1f
+#define OP_IS_LOAD(type) ((LOAD <= (type) && (type) <= LOAD_VSX) ||
(type) == LARX)
+#define OP_IS_STORE(type) ((STORE <= (type) && (type) <= STORE_VSX) ||
(type) == STCX)
#define OP_IS_LOAD_STORE(type) (LOAD <= (type) && (type) <= STCX)
/* Compute flags, ORed in with type */
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index ab0dd22fed5f..28d57d841642 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -30,7 +30,7 @@
* Stores the breakpoints currently in use on each breakpoint address
* register for every cpu
*/
-static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
+static DEFINE_PER_CPU(struct perf_event *, bp_per_reg[HBP_NUM_MAX]);
/*
* Returns total number of data or instruction breakpoints available.
@@ -42,6 +42,17 @@ int hw_breakpoint_slots(int type)
return 0; /* no instruction breakpoints available */
}
+static bool single_step_pending(void)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (current->thread.last_hit_ubp[i])
+ return true;
+ }
+ return false;
+}
+
/*
* Install a perf counter breakpoint.
*
@@ -54,16 +65,26 @@ int hw_breakpoint_slots(int type)
int arch_install_hw_breakpoint(struct perf_event *bp)
{
struct arch_hw_breakpoint *info = counter_arch_bp(bp);
- struct perf_event **slot = this_cpu_ptr(_per_reg);
+ struct perf_event **slot;
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ slot = this_cpu_ptr(_per_reg[i]);
+ if (!*slot) {
+ *slot = bp;
+ break;
+ }
+ }
- *slot = bp;
+ if (WARN_ONCE(i == nr_wp_slots(), "Can't find any breakpoint slot"))
+ return -EBUSY;
/*
* Do not install DABR values if the instruction must be single-stepped.
* If so, DABR will be populated in single_step_dabr_instruction().
*/
- if (current->thread.last_hit_ubp != bp)
- __set_breakpoint(0, info);
+ if (!single_step_pending())
+ __set_breakpoint(i, info);
return 0;
}
@@ -79,15 +100,22 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
*/
void arch_uninstall_hw_breakpoint(struct perf_event *bp)
{
- struct perf_event **slot = this_cpu_ptr(_per_reg);
+ struct arch_hw_breakpoint null_brk = {0};
+ struct perf_event **slot;
+ int i;
- if (*slot != bp) {
- WARN_ONCE(1, "Can't find the breakpoint");
- return;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ slot = this_cpu_ptr(_per_reg[i]);
+ if (*slot == bp) {
+ *slot = NULL;
+ break;
+ }
}
- *slot = NULL;
- h
[PATCH v6 00/16] powerpc/watchpoint: Preparation for more than one watchpoint
So far, powerpc Book3S code has been written with an assumption of only one watchpoint. But Power10[1] is introducing second watchpoint register (DAWR). Even though this patchset does not enable 2nd DAWR, it makes the infrastructure ready so that enabling 2nd DAWR should just be a matter of changing count. Existing functionality works fine with the patchset. I've tested it with perf, ptrace(gdb), xmon. All hw-breakpoint selftests are passing as well. And I've build tested for 8xx and 'AMCC 44x, 46x or 47x'. Note: kvm or PowerVM guest is not enabled yet. v5: https://lore.kernel.org/linuxppc-dev/20200511025911.212827-1-ravi.bango...@linux.ibm.com v5->v6: - Rebased to powerpc/next-test which has prefix-instruction support - Adopt 'struct ppc_inst' in hw-breakpoint code as well [1]: https://www-355.ibm.com/systems/power/openpower/ Ravi Bangoria (16): powerpc/watchpoint: Rename current DAWR macros powerpc/watchpoint: Add SPRN macros for second DAWR powerpc/watchpoint: Introduce function to get nr watchpoints dynamically powerpc/watchpoint/ptrace: Return actual num of available watchpoints powerpc/watchpoint: Provide DAWR number to set_dawr powerpc/watchpoint: Provide DAWR number to __set_breakpoint powerpc/watchpoint: Get watchpoint count dynamically while disabling them powerpc/watchpoint: Disable all available watchpoints when !dawr_force_enable powerpc/watchpoint: Convert thread_struct->hw_brk to an array powerpc/watchpoint: Use loop for thread_struct->ptrace_bps powerpc/watchpoint: Introduce is_ptrace_bp() function powerpc/watchpoint: Use builtin ALIGN*() macros powerpc/watchpoint: Prepare handler to handle more than one watcnhpoint powerpc/watchpoint: Don't allow concurrent perf and ptrace events powerpc/watchpoint/xmon: Don't allow breakpoint overwriting powerpc/watchpoint/xmon: Support 2nd DAWR arch/powerpc/include/asm/cputable.h | 6 +- arch/powerpc/include/asm/debug.h | 2 +- arch/powerpc/include/asm/hw_breakpoint.h | 32 +- arch/powerpc/include/asm/processor.h | 6 +- arch/powerpc/include/asm/reg.h| 6 +- arch/powerpc/include/asm/sstep.h | 2 + arch/powerpc/kernel/dawr.c| 23 +- arch/powerpc/kernel/hw_breakpoint.c | 642 ++ arch/powerpc/kernel/process.c | 85 +-- arch/powerpc/kernel/ptrace/ptrace-noadv.c | 72 ++- arch/powerpc/kernel/ptrace/ptrace32.c | 4 +- arch/powerpc/kernel/signal.c | 13 +- arch/powerpc/kvm/book3s_hv.c | 12 +- arch/powerpc/kvm/book3s_hv_rmhandlers.S | 18 +- arch/powerpc/xmon/xmon.c | 99 +++- kernel/events/hw_breakpoint.c | 16 + 16 files changed, 811 insertions(+), 227 deletions(-) -- 2.26.2
[PATCH v6 09/16] powerpc/watchpoint: Convert thread_struct->hw_brk to an array
So far powerpc hw supported only one watchpoint. But Power10 is
introducing 2nd DAWR. Convert thread_struct->hw_brk into an array.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/processor.h | 2 +-
arch/powerpc/kernel/process.c | 60 ++-
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 40 ++-
arch/powerpc/kernel/ptrace/ptrace32.c | 4 +-
arch/powerpc/kernel/signal.c | 13 +++--
5 files changed, 78 insertions(+), 41 deletions(-)
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index f209c5703ee2..fba6b586e3c8 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -187,7 +187,7 @@ struct thread_struct {
*/
struct perf_event *last_hit_ubp;
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
- struct arch_hw_breakpoint hw_brk; /* info on the hardware breakpoint */
+ struct arch_hw_breakpoint hw_brk[HBP_NUM_MAX]; /* hardware breakpoint
info */
unsigned long trap_nr;/* last trap # on this thread */
u8 load_slb;/* Ages out SLB preload cache entries */
u8 load_fp;
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index f303aea61794..d94d8925711c 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -711,21 +711,49 @@ void switch_booke_debug_regs(struct debug_reg *new_debug)
EXPORT_SYMBOL_GPL(switch_booke_debug_regs);
#else /* !CONFIG_PPC_ADV_DEBUG_REGS */
#ifndef CONFIG_HAVE_HW_BREAKPOINT
-static void set_breakpoint(struct arch_hw_breakpoint *brk)
+static void set_breakpoint(int i, struct arch_hw_breakpoint *brk)
{
preempt_disable();
- __set_breakpoint(0, brk);
+ __set_breakpoint(i, brk);
preempt_enable();
}
static void set_debug_reg_defaults(struct thread_struct *thread)
{
- thread->hw_brk.address = 0;
- thread->hw_brk.type = 0;
- thread->hw_brk.len = 0;
- thread->hw_brk.hw_len = 0;
- if (ppc_breakpoint_available())
- set_breakpoint(>hw_brk);
+ int i;
+ struct arch_hw_breakpoint null_brk = {0};
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ thread->hw_brk[i] = null_brk;
+ if (ppc_breakpoint_available())
+ set_breakpoint(i, >hw_brk[i]);
+ }
+}
+
+static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
+ struct arch_hw_breakpoint *b)
+{
+ if (a->address != b->address)
+ return false;
+ if (a->type != b->type)
+ return false;
+ if (a->len != b->len)
+ return false;
+ /* no need to check hw_len. it's calculated from address and len */
+ return true;
+}
+
+static void switch_hw_breakpoint(struct task_struct *new)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (likely(hw_brk_match(this_cpu_ptr(_brk[i]),
+ >thread.hw_brk[i])))
+ continue;
+
+ __set_breakpoint(i, >thread.hw_brk[i]);
+ }
}
#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
#endif /* CONFIG_PPC_ADV_DEBUG_REGS */
@@ -829,19 +857,6 @@ bool ppc_breakpoint_available(void)
}
EXPORT_SYMBOL_GPL(ppc_breakpoint_available);
-static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
- struct arch_hw_breakpoint *b)
-{
- if (a->address != b->address)
- return false;
- if (a->type != b->type)
- return false;
- if (a->len != b->len)
- return false;
- /* no need to check hw_len. it's calculated from address and len */
- return true;
-}
-
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
static inline bool tm_enabled(struct task_struct *tsk)
@@ -1174,8 +1189,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
* schedule DABR
*/
#ifndef CONFIG_HAVE_HW_BREAKPOINT
- if (unlikely(!hw_brk_match(this_cpu_ptr(_brk[0]),
>thread.hw_brk)))
- __set_breakpoint(0, >thread.hw_brk);
+ switch_hw_breakpoint(new);
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 12962302d6a4..0dbb35392dd2 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -67,11 +67,16 @@ int ptrace_get_debugreg(struct task_struct *child, unsigned
long addr,
/* We only support one DABR and no IABRS at the moment */
if (addr > 0)
return -EINVAL;
- dabr_fake = ((child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
-(child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
+ dabr_fake = ((child->th
[PATCH v6 01/16] powerpc/watchpoint: Rename current DAWR macros
Power10 is introducing second DAWR. Use real register names from ISA
for current macros:
s/SPRN_DAWR/SPRN_DAWR0/
s/SPRN_DAWRX/SPRN_DAWRX0/
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/reg.h | 4 ++--
arch/powerpc/kernel/dawr.c | 4 ++--
arch/powerpc/kvm/book3s_hv.c| 12 ++--
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 18 +-
arch/powerpc/xmon/xmon.c| 2 +-
5 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index f95eb8f97756..60a21b6b2057 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -283,14 +283,14 @@
#define CTRL_CT1 0x4000 /* thread 1 */
#define CTRL_TE 0x00c0 /* thread enable */
#define CTRL_RUNLATCH0x1
-#define SPRN_DAWR 0xB4
+#define SPRN_DAWR0 0xB4
#define SPRN_RPR 0xBA/* Relative Priority Register */
#define SPRN_CIABR 0xBB
#define CIABR_PRIV 0x3
#define CIABR_PRIV_USER 1
#define CIABR_PRIV_SUPER 2
#define CIABR_PRIV_HYPER 3
-#define SPRN_DAWRX 0xBC
+#define SPRN_DAWRX00xBC
#define DAWRX_USER __MASK(0)
#define DAWRX_KERNEL __MASK(1)
#define DAWRX_HYP__MASK(2)
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index cc14aa6c4a1b..e91b613bf137 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -39,8 +39,8 @@ int set_dawr(struct arch_hw_breakpoint *brk)
if (ppc_md.set_dawr)
return ppc_md.set_dawr(dawr, dawrx);
- mtspr(SPRN_DAWR, dawr);
- mtspr(SPRN_DAWRX, dawrx);
+ mtspr(SPRN_DAWR0, dawr);
+ mtspr(SPRN_DAWRX0, dawrx);
return 0;
}
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 93493f0cbfe8..db07199f0977 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3392,8 +3392,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu
*vcpu, u64 time_limit,
int trap;
unsigned long host_hfscr = mfspr(SPRN_HFSCR);
unsigned long host_ciabr = mfspr(SPRN_CIABR);
- unsigned long host_dawr = mfspr(SPRN_DAWR);
- unsigned long host_dawrx = mfspr(SPRN_DAWRX);
+ unsigned long host_dawr = mfspr(SPRN_DAWR0);
+ unsigned long host_dawrx = mfspr(SPRN_DAWRX0);
unsigned long host_psscr = mfspr(SPRN_PSSCR);
unsigned long host_pidr = mfspr(SPRN_PID);
@@ -3422,8 +3422,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu
*vcpu, u64 time_limit,
mtspr(SPRN_SPURR, vcpu->arch.spurr);
if (dawr_enabled()) {
- mtspr(SPRN_DAWR, vcpu->arch.dawr);
- mtspr(SPRN_DAWRX, vcpu->arch.dawrx);
+ mtspr(SPRN_DAWR0, vcpu->arch.dawr);
+ mtspr(SPRN_DAWRX0, vcpu->arch.dawrx);
}
mtspr(SPRN_CIABR, vcpu->arch.ciabr);
mtspr(SPRN_IC, vcpu->arch.ic);
@@ -3475,8 +3475,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu
*vcpu, u64 time_limit,
(local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
mtspr(SPRN_HFSCR, host_hfscr);
mtspr(SPRN_CIABR, host_ciabr);
- mtspr(SPRN_DAWR, host_dawr);
- mtspr(SPRN_DAWRX, host_dawrx);
+ mtspr(SPRN_DAWR0, host_dawr);
+ mtspr(SPRN_DAWRX0, host_dawrx);
mtspr(SPRN_PID, host_pidr);
/*
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 780a499c7114..70de3325d0e9 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -707,8 +707,8 @@ BEGIN_FTR_SECTION
END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
BEGIN_FTR_SECTION
mfspr r5, SPRN_CIABR
- mfspr r6, SPRN_DAWR
- mfspr r7, SPRN_DAWRX
+ mfspr r6, SPRN_DAWR0
+ mfspr r7, SPRN_DAWRX0
mfspr r8, SPRN_IAMR
std r5, STACK_SLOT_CIABR(r1)
std r6, STACK_SLOT_DAWR(r1)
@@ -803,8 +803,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
beq 1f
ld r5, VCPU_DAWR(r4)
ld r6, VCPU_DAWRX(r4)
- mtspr SPRN_DAWR, r5
- mtspr SPRN_DAWRX, r6
+ mtspr SPRN_DAWR0, r5
+ mtspr SPRN_DAWRX0, r6
1:
ld r7, VCPU_CIABR(r4)
ld r8, VCPU_TAR(r4)
@@ -1766,8 +1766,8 @@ BEGIN_FTR_SECTION
* 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
+ mtspr SPRN_DAWR0, r6
+ mtspr SPRN_DAWRX0, r7
END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
BEGIN_FTR_SECTION
ld r5, STACK_SLOT_TID(r1)
@@ -2577,8 +2577,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
mfmsr r6
andi. r6, r6, MSR_DR /* in real mode? */
[PATCH v6 06/16] powerpc/watchpoint: Provide DAWR number to __set_breakpoint
Introduce new parameter 'nr' to __set_breakpoint() which indicates
which DAWR should be programed. Also convert current_brk variable
to an array.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/debug.h | 2 +-
arch/powerpc/include/asm/hw_breakpoint.h | 2 +-
arch/powerpc/kernel/hw_breakpoint.c | 8
arch/powerpc/kernel/process.c| 14 +++---
arch/powerpc/kernel/signal.c | 2 +-
arch/powerpc/xmon/xmon.c | 2 +-
6 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h
index 7756026b95ca..ec57daf87f40 100644
--- a/arch/powerpc/include/asm/debug.h
+++ b/arch/powerpc/include/asm/debug.h
@@ -45,7 +45,7 @@ static inline int debugger_break_match(struct pt_regs *regs)
{ return 0; }
static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
#endif
-void __set_breakpoint(struct arch_hw_breakpoint *brk);
+void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk);
bool ppc_breakpoint_available(void);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
extern void do_send_trap(struct pt_regs *regs, unsigned long address,
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index 5b3b02834e0b..1120c7d9db58 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -85,7 +85,7 @@ static inline void hw_breakpoint_disable(void)
brk.len = 0;
brk.hw_len = 0;
if (ppc_breakpoint_available())
- __set_breakpoint();
+ __set_breakpoint(0, );
}
extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
int hw_breakpoint_handler(struct die_args *args);
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 01f07d91df70..f5472402c06d 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -64,7 +64,7 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
* If so, DABR will be populated in single_step_dabr_instruction().
*/
if (current->thread.last_hit_ubp != bp)
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
return 0;
}
@@ -222,7 +222,7 @@ void thread_change_pc(struct task_struct *tsk, struct
pt_regs *regs)
info = counter_arch_bp(tsk->thread.last_hit_ubp);
regs->msr &= ~MSR_SE;
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
tsk->thread.last_hit_ubp = NULL;
}
@@ -347,7 +347,7 @@ int hw_breakpoint_handler(struct die_args *args)
if (!(info->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
perf_bp_event(bp, regs);
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
out:
rcu_read_unlock();
return rc;
@@ -380,7 +380,7 @@ static int single_step_dabr_instruction(struct die_args
*args)
if (!(info->type & HW_BRK_TYPE_EXTRANEOUS_IRQ))
perf_bp_event(bp, regs);
- __set_breakpoint(info);
+ __set_breakpoint(0, info);
current->thread.last_hit_ubp = NULL;
/*
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index dc161b0adc82..f303aea61794 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -637,7 +637,7 @@ void do_break (struct pt_regs *regs, unsigned long address,
}
#endif /* CONFIG_PPC_ADV_DEBUG_REGS */
-static DEFINE_PER_CPU(struct arch_hw_breakpoint, current_brk);
+static DEFINE_PER_CPU(struct arch_hw_breakpoint, current_brk[HBP_NUM_MAX]);
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
/*
@@ -714,7 +714,7 @@ EXPORT_SYMBOL_GPL(switch_booke_debug_regs);
static void set_breakpoint(struct arch_hw_breakpoint *brk)
{
preempt_disable();
- __set_breakpoint(brk);
+ __set_breakpoint(0, brk);
preempt_enable();
}
@@ -800,13 +800,13 @@ static inline int set_breakpoint_8xx(struct
arch_hw_breakpoint *brk)
return 0;
}
-void __set_breakpoint(struct arch_hw_breakpoint *brk)
+void __set_breakpoint(int nr, struct arch_hw_breakpoint *brk)
{
- memcpy(this_cpu_ptr(_brk), brk, sizeof(*brk));
+ memcpy(this_cpu_ptr(_brk[nr]), brk, sizeof(*brk));
if (dawr_enabled())
// Power8 or later
- set_dawr(0, brk);
+ set_dawr(nr, brk);
else if (IS_ENABLED(CONFIG_PPC_8xx))
set_breakpoint_8xx(brk);
else if (!cpu_has_feature(CPU_FTR_ARCH_207S))
@@ -1174,8 +1174,8 @@ struct task_struct *__switch_to(struct task_struct *prev,
* schedule DABR
*/
#ifndef CONFIG_HAVE_HW_BREAKPOINT
- if (unlikely(!hw_brk_match(this_cpu_ptr(_brk),
>thread.hw_brk)))
- __set_breakpoint(>thread.hw_brk);
+ if (unlikely(!hw_brk_match(this_cpu_ptr(_brk[0]),
>thread.hw_brk)))
+ __
[PATCH v6 10/16] powerpc/watchpoint: Use loop for thread_struct->ptrace_bps
ptrace_bps is already an array of size HBP_NUM_MAX. But we use
hardcoded index 0 while fetching/updating it. Convert such code
to loop over array.
ptrace interface to use multiple watchpoint remains same. eg:
two PPC_PTRACE_SETHWDEBUG calls will create two watchpoint if
underneath hw supports it.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/kernel/hw_breakpoint.c | 7 --
arch/powerpc/kernel/process.c | 6 -
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 28 +--
3 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index f5472402c06d..917cca73dbc3 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -420,10 +420,13 @@ NOKPROBE_SYMBOL(hw_breakpoint_exceptions_notify);
*/
void flush_ptrace_hw_breakpoint(struct task_struct *tsk)
{
+ int i;
struct thread_struct *t = >thread;
- unregister_hw_breakpoint(t->ptrace_bps[0]);
- t->ptrace_bps[0] = NULL;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ unregister_hw_breakpoint(t->ptrace_bps[i]);
+ t->ptrace_bps[i] = NULL;
+ }
}
void hw_breakpoint_pmu_read(struct perf_event *bp)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index d94d8925711c..77ec1299e2fd 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1604,6 +1604,9 @@ int copy_thread_tls(unsigned long clone_flags, unsigned
long usp,
void (*f)(void);
unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
struct thread_info *ti = task_thread_info(p);
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ int i;
+#endif
klp_init_thread_info(p);
@@ -1663,7 +1666,8 @@ int copy_thread_tls(unsigned long clone_flags, unsigned
long usp,
p->thread.ksp_limit = (unsigned long)end_of_stack(p);
#endif
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- p->thread.ptrace_bps[0] = NULL;
+ for (i = 0; i < nr_wp_slots(); i++)
+ p->thread.ptrace_bps[i] = NULL;
#endif
p->thread.fp_save_area = NULL;
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 0dbb35392dd2..08cb8c1b504c 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -168,6 +168,19 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned
long addr, unsigned l
return 0;
}
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+static int find_empty_ptrace_bp(struct thread_struct *thread)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!thread->ptrace_bps[i])
+ return i;
+ }
+ return -1;
+}
+#endif
+
static int find_empty_hw_brk(struct thread_struct *thread)
{
int i;
@@ -217,8 +230,9 @@ long ppc_set_hwdebug(struct task_struct *child, struct
ppc_hw_breakpoint *bp_inf
len = 1;
else
return -EINVAL;
- bp = thread->ptrace_bps[0];
- if (bp)
+
+ i = find_empty_ptrace_bp(thread);
+ if (i < 0)
return -ENOSPC;
/* Create a new breakpoint request if one doesn't exist already */
@@ -228,13 +242,13 @@ long ppc_set_hwdebug(struct task_struct *child, struct
ppc_hw_breakpoint *bp_inf
arch_bp_generic_fields(brk.type, _type);
bp = register_user_hw_breakpoint(, ptrace_triggered, NULL, child);
- thread->ptrace_bps[0] = bp;
+ thread->ptrace_bps[i] = bp;
if (IS_ERR(bp)) {
- thread->ptrace_bps[0] = NULL;
+ thread->ptrace_bps[i] = NULL;
return PTR_ERR(bp);
}
- return 1;
+ return i + 1;
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT)
@@ -263,10 +277,10 @@ long ppc_del_hwdebug(struct task_struct *child, long data)
return -EINVAL;
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- bp = thread->ptrace_bps[0];
+ bp = thread->ptrace_bps[data - 1];
if (bp) {
unregister_hw_breakpoint(bp);
- thread->ptrace_bps[0] = NULL;
+ thread->ptrace_bps[data - 1] = NULL;
} else {
ret = -ENOENT;
}
--
2.26.2
[PATCH v6 04/16] powerpc/watchpoint/ptrace: Return actual num of available watchpoints
User can ask for num of available watchpoints(dbginfo.num_data_bps) using ptrace(PPC_PTRACE_GETHWDBGINFO). Return actual number of available watchpoints on the machine rather than hardcoded 1. Signed-off-by: Ravi Bangoria Reviewed-by: Michael Neuling --- arch/powerpc/kernel/ptrace/ptrace-noadv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c b/arch/powerpc/kernel/ptrace/ptrace-noadv.c index f87e7c5c3bf3..12962302d6a4 100644 --- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c +++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c @@ -44,7 +44,7 @@ void ppc_gethwdinfo(struct ppc_debug_info *dbginfo) dbginfo->version = 1; dbginfo->num_instruction_bps = 0; if (ppc_breakpoint_available()) - dbginfo->num_data_bps = 1; + dbginfo->num_data_bps = nr_wp_slots(); else dbginfo->num_data_bps = 0; dbginfo->num_condition_regs = 0; -- 2.26.2
[PATCH v6 07/16] powerpc/watchpoint: Get watchpoint count dynamically while disabling them
Instead of disabling only one watchpoint, get num of available
watchpoints dynamically and disable all of them.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/hw_breakpoint.h | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index 1120c7d9db58..d472b2eb757e 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -78,14 +78,14 @@ extern void ptrace_triggered(struct perf_event *bp,
struct perf_sample_data *data, struct pt_regs *regs);
static inline void hw_breakpoint_disable(void)
{
- struct arch_hw_breakpoint brk;
-
- brk.address = 0;
- brk.type = 0;
- brk.len = 0;
- brk.hw_len = 0;
- if (ppc_breakpoint_available())
- __set_breakpoint(0, );
+ int i;
+ struct arch_hw_breakpoint null_brk = {0};
+
+ if (!ppc_breakpoint_available())
+ return;
+
+ for (i = 0; i < nr_wp_slots(); i++)
+ __set_breakpoint(i, _brk);
}
extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
int hw_breakpoint_handler(struct die_args *args);
--
2.26.2
[PATCH v6 05/16] powerpc/watchpoint: Provide DAWR number to set_dawr
Introduce new parameter 'nr' to set_dawr() which indicates which DAWR
should be programed.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/hw_breakpoint.h | 4 ++--
arch/powerpc/kernel/dawr.c | 15 ++-
arch/powerpc/kernel/process.c| 2 +-
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index 518b41eef924..5b3b02834e0b 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -104,10 +104,10 @@ static inline bool dawr_enabled(void)
{
return dawr_force_enable;
}
-int set_dawr(struct arch_hw_breakpoint *brk);
+int set_dawr(int nr, struct arch_hw_breakpoint *brk);
#else
static inline bool dawr_enabled(void) { return false; }
-static inline int set_dawr(struct arch_hw_breakpoint *brk) { return -1; }
+static inline int set_dawr(int nr, struct arch_hw_breakpoint *brk) { return
-1; }
#endif
#endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index e91b613bf137..8114ad3a8574 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -16,7 +16,7 @@
bool dawr_force_enable;
EXPORT_SYMBOL_GPL(dawr_force_enable);
-int set_dawr(struct arch_hw_breakpoint *brk)
+int set_dawr(int nr, struct arch_hw_breakpoint *brk)
{
unsigned long dawr, dawrx, mrd;
@@ -39,15 +39,20 @@ int set_dawr(struct arch_hw_breakpoint *brk)
if (ppc_md.set_dawr)
return ppc_md.set_dawr(dawr, dawrx);
- mtspr(SPRN_DAWR0, dawr);
- mtspr(SPRN_DAWRX0, dawrx);
+ if (nr == 0) {
+ mtspr(SPRN_DAWR0, dawr);
+ mtspr(SPRN_DAWRX0, dawrx);
+ } else {
+ mtspr(SPRN_DAWR1, dawr);
+ mtspr(SPRN_DAWRX1, dawrx);
+ }
return 0;
}
static void set_dawr_cb(void *info)
{
- set_dawr(info);
+ set_dawr(0, info);
}
static ssize_t dawr_write_file_bool(struct file *file,
@@ -60,7 +65,7 @@ static ssize_t dawr_write_file_bool(struct file *file,
/* Send error to user if they hypervisor won't allow us to write DAWR */
if (!dawr_force_enable &&
firmware_has_feature(FW_FEATURE_LPAR) &&
- set_dawr(_brk) != H_SUCCESS)
+ set_dawr(0, _brk) != H_SUCCESS)
return -ENODEV;
rc = debugfs_write_file_bool(file, user_buf, count, ppos);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index db766252238f..dc161b0adc82 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -806,7 +806,7 @@ void __set_breakpoint(struct arch_hw_breakpoint *brk)
if (dawr_enabled())
// Power8 or later
- set_dawr(brk);
+ set_dawr(0, brk);
else if (IS_ENABLED(CONFIG_PPC_8xx))
set_breakpoint_8xx(brk);
else if (!cpu_has_feature(CPU_FTR_ARCH_207S))
--
2.26.2
[PATCH v6 16/16] powerpc/watchpoint/xmon: Support 2nd DAWR
Add support for 2nd DAWR in xmon. With this, we can have two
simultaneous breakpoints from xmon.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/xmon/xmon.c | 101 ++-
1 file changed, 69 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 094bf4715f2c..de585204d1d2 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -112,7 +112,7 @@ struct bpt {
#define BP_DABR4
static struct bpt bpts[NBPTS];
-static struct bpt dabr;
+static struct bpt dabr[HBP_NUM_MAX];
static struct bpt *iabr;
static unsigned bpinstr = 0x7fe8; /* trap */
@@ -784,10 +784,17 @@ static int xmon_sstep(struct pt_regs *regs)
static int xmon_break_match(struct pt_regs *regs)
{
+ int i;
+
if ((regs->msr & (MSR_IR|MSR_PR|MSR_64BIT)) != (MSR_IR|MSR_64BIT))
return 0;
- if (dabr.enabled == 0)
- return 0;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (dabr[i].enabled)
+ goto found;
+ }
+ return 0;
+
+found:
xmon_core(regs, 0);
return 1;
}
@@ -948,13 +955,16 @@ static void insert_bpts(void)
static void insert_cpu_bpts(void)
{
+ int i;
struct arch_hw_breakpoint brk;
- if (dabr.enabled) {
- brk.address = dabr.address;
- brk.type = (dabr.enabled & HW_BRK_TYPE_DABR) |
HW_BRK_TYPE_PRIV_ALL;
- brk.len = DABR_MAX_LEN;
- __set_breakpoint(0, );
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (dabr[i].enabled) {
+ brk.address = dabr[i].address;
+ brk.type = (dabr[i].enabled & HW_BRK_TYPE_DABR) |
HW_BRK_TYPE_PRIV_ALL;
+ brk.len = 8;
+ __set_breakpoint(i, );
+ }
}
if (iabr)
@@ -1366,6 +1376,35 @@ static long check_bp_loc(unsigned long addr)
return 1;
}
+static int find_free_data_bpt(void)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!dabr[i].enabled)
+ return i;
+ }
+ printf("Couldn't find free breakpoint register\n");
+ return -1;
+}
+
+static void print_data_bpts(void)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (!dabr[i].enabled)
+ continue;
+
+ printf(" data "REG" [", dabr[i].address);
+ if (dabr[i].enabled & 1)
+ printf("r");
+ if (dabr[i].enabled & 2)
+ printf("w");
+ printf("]\n");
+ }
+}
+
static char *breakpoint_help_string =
"Breakpoint command usage:\n"
"bshow breakpoints\n"
@@ -1399,10 +1438,9 @@ bpt_cmds(void)
printf("Hardware data breakpoint not supported on this
cpu\n");
break;
}
- if (dabr.enabled) {
- printf("Couldn't find free breakpoint register\n");
+ i = find_free_data_bpt();
+ if (i < 0)
break;
- }
mode = 7;
cmd = inchar();
if (cmd == 'r')
@@ -1411,15 +1449,15 @@ bpt_cmds(void)
mode = 6;
else
termch = cmd;
- dabr.address = 0;
- dabr.enabled = 0;
- if (scanhex()) {
- if (!is_kernel_addr(dabr.address)) {
+ dabr[i].address = 0;
+ dabr[i].enabled = 0;
+ if (scanhex([i].address)) {
+ if (!is_kernel_addr(dabr[i].address)) {
printf(badaddr);
break;
}
- dabr.address &= ~HW_BRK_TYPE_DABR;
- dabr.enabled = mode | BP_DABR;
+ dabr[i].address &= ~HW_BRK_TYPE_DABR;
+ dabr[i].enabled = mode | BP_DABR;
}
force_enable_xmon();
@@ -1458,7 +1496,9 @@ bpt_cmds(void)
for (i = 0; i < NBPTS; ++i)
bpts[i].enabled = 0;
iabr = NULL;
- dabr.enabled = 0;
+ for (i = 0; i < nr_wp_slots(); i++)
+ dabr[i].enabled = 0;
+
printf("All breakpoints cleared\n");
break;
}
@@ -1492,14 +1532,7 @@ bpt_cmds(void)
if (xmon_is_ro || !scanhex()) {
/* print all breakpoints */
[PATCH v6 02/16] powerpc/watchpoint: Add SPRN macros for second DAWR
Power10 is introducing second DAWR. Add SPRN_ macros for the same. Signed-off-by: Ravi Bangoria Reviewed-by: Michael Neuling --- arch/powerpc/include/asm/reg.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index 60a21b6b2057..054f8a71d686 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -284,6 +284,7 @@ #define CTRL_TE 0x00c0 /* thread enable */ #define CTRL_RUNLATCH0x1 #define SPRN_DAWR0 0xB4 +#define SPRN_DAWR1 0xB5 #define SPRN_RPR 0xBA/* Relative Priority Register */ #define SPRN_CIABR 0xBB #define CIABR_PRIV 0x3 @@ -291,6 +292,7 @@ #define CIABR_PRIV_SUPER 2 #define CIABR_PRIV_HYPER 3 #define SPRN_DAWRX00xBC +#define SPRN_DAWRX10xBD #define DAWRX_USER __MASK(0) #define DAWRX_KERNEL __MASK(1) #define DAWRX_HYP__MASK(2) -- 2.26.2
[PATCH v6 11/16] powerpc/watchpoint: Introduce is_ptrace_bp() function
Introduce is_ptrace_bp() function and move the check inside the
function. It will be utilize more in later set of patches.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/kernel/hw_breakpoint.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 917cca73dbc3..8028a2704874 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -91,6 +91,11 @@ void arch_uninstall_hw_breakpoint(struct perf_event *bp)
hw_breakpoint_disable();
}
+static bool is_ptrace_bp(struct perf_event *bp)
+{
+ return bp->overflow_handler == ptrace_triggered;
+}
+
/*
* Perform cleanup of arch-specific counters during unregistration
* of the perf-event
@@ -325,7 +330,7 @@ int hw_breakpoint_handler(struct die_args *args)
* one-shot mode. The ptrace-ed process will receive the SIGTRAP signal
* generated in do_dabr().
*/
- if (bp->overflow_handler == ptrace_triggered) {
+ if (is_ptrace_bp(bp)) {
perf_bp_event(bp, regs);
rc = NOTIFY_DONE;
goto out;
--
2.26.2
[PATCH v6 03/16] powerpc/watchpoint: Introduce function to get nr watchpoints dynamically
So far we had only one watchpoint, so we have hardcoded HBP_NUM to 1.
But Power10 is introducing 2nd DAWR and thus kernel should be able to
dynamically find actual number of watchpoints supported by hw it's
running on. Introduce function for the same. Also convert HBP_NUM macro
to HBP_NUM_MAX, which will now represent maximum number of watchpoints
supported by Powerpc.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/cputable.h | 6 +-
arch/powerpc/include/asm/hw_breakpoint.h | 5 +
arch/powerpc/include/asm/processor.h | 2 +-
arch/powerpc/kernel/hw_breakpoint.c | 2 +-
4 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h
b/arch/powerpc/include/asm/cputable.h
index 40a4d3c6fd99..c67b94f3334c 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -614,7 +614,11 @@ enum {
};
#endif /* __powerpc64__ */
-#define HBP_NUM 1
+/*
+ * Maximum number of hw breakpoint supported on powerpc. Number of
+ * breakpoints supported by actual hw might be less than this.
+ */
+#define HBP_NUM_MAX1
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index f2f8d8aa8e3b..518b41eef924 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -43,6 +43,11 @@ struct arch_hw_breakpoint {
#define DABR_MAX_LEN 8
#define DAWR_MAX_LEN 512
+static inline int nr_wp_slots(void)
+{
+ return HBP_NUM_MAX;
+}
+
#ifdef CONFIG_HAVE_HW_BREAKPOINT
#include
#include
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index 5ab202055d5a..f209c5703ee2 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -180,7 +180,7 @@ struct thread_struct {
int fpexc_mode; /* floating-point exception mode */
unsigned intalign_ctl; /* alignment handling control */
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- struct perf_event *ptrace_bps[HBP_NUM];
+ struct perf_event *ptrace_bps[HBP_NUM_MAX];
/*
* Helps identify source of single-step exception and subsequent
* hw-breakpoint enablement
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 423603c92c0f..01f07d91df70 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -39,7 +39,7 @@ static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
int hw_breakpoint_slots(int type)
{
if (type == TYPE_DATA)
- return HBP_NUM;
+ return nr_wp_slots();
return 0; /* no instruction breakpoints available */
}
--
2.26.2
[PATCH v6 15/16] powerpc/watchpoint/xmon: Don't allow breakpoint overwriting
Xmon allows overwriting breakpoints because it's supported by only
one DAWR. But with multiple DAWRs, overwriting becomes ambiguous
or unnecessary complicated. So let's not allow it.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/xmon/xmon.c | 4
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 30b3e3d99c0d..094bf4715f2c 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1399,6 +1399,10 @@ bpt_cmds(void)
printf("Hardware data breakpoint not supported on this
cpu\n");
break;
}
+ if (dabr.enabled) {
+ printf("Couldn't find free breakpoint register\n");
+ break;
+ }
mode = 7;
cmd = inchar();
if (cmd == 'r')
--
2.26.2
[PATCH v6 08/16] powerpc/watchpoint: Disable all available watchpoints when !dawr_force_enable
Instead of disabling only first watchpoint, disable all available
watchpoints while clearing dawr_force_enable.
Callback function is used only for disabling watchpoint, rename it
to disable_dawrs_cb(). And null_brk parameter is not really required
while disabling watchpoint, remove it.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/kernel/dawr.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index 8114ad3a8574..500f52fa4711 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -50,9 +50,13 @@ int set_dawr(int nr, struct arch_hw_breakpoint *brk)
return 0;
}
-static void set_dawr_cb(void *info)
+static void disable_dawrs_cb(void *info)
{
- set_dawr(0, info);
+ struct arch_hw_breakpoint null_brk = {0};
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++)
+ set_dawr(i, _brk);
}
static ssize_t dawr_write_file_bool(struct file *file,
@@ -74,7 +78,7 @@ static ssize_t dawr_write_file_bool(struct file *file,
/* If we are clearing, make sure all CPUs have the DAWR cleared */
if (!dawr_force_enable)
- smp_call_function(set_dawr_cb, _brk, 0);
+ smp_call_function(disable_dawrs_cb, NULL, 0);
return rc;
}
--
2.26.2
[PATCH v6 13/16] powerpc/watchpoint: Prepare handler to handle more than one watcnhpoint
Currently we assume that we have only one watchpoint supported by hw.
Get rid of that assumption and use dynamic loop instead. This should
make supporting more watchpoints very easy.
With more than one watchpoint, exception handler needs to know which
DAWR caused the exception, and hw currently does not provide it. So
we need sw logic for the same. To figure out which DAWR caused the
exception, check all different combinations of user specified range,
DAWR address range, actual access range and DAWRX constrains. For ex,
if user specified range and actual access range overlaps but DAWRX is
configured for readonly watchpoint and the instruction is store, this
DAWR must not have caused exception.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/processor.h | 2 +-
arch/powerpc/include/asm/sstep.h | 2 +
arch/powerpc/kernel/hw_breakpoint.c | 397 +--
arch/powerpc/kernel/process.c| 3 -
4 files changed, 312 insertions(+), 92 deletions(-)
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index fba6b586e3c8..4e53df163b92 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -185,7 +185,7 @@ struct thread_struct {
* Helps identify source of single-step exception and subsequent
* hw-breakpoint enablement
*/
- struct perf_event *last_hit_ubp;
+ struct perf_event *last_hit_ubp[HBP_NUM_MAX];
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
struct arch_hw_breakpoint hw_brk[HBP_NUM_MAX]; /* hardware breakpoint
info */
unsigned long trap_nr;/* last trap # on this thread */
diff --git a/arch/powerpc/include/asm/sstep.h b/arch/powerpc/include/asm/sstep.h
index 9b200a5f8794..3b01c69a44aa 100644
--- a/arch/powerpc/include/asm/sstep.h
+++ b/arch/powerpc/include/asm/sstep.h
@@ -49,6 +49,8 @@ enum instruction_type {
#define INSTR_TYPE_MASK0x1f
+#define OP_IS_LOAD(type) ((LOAD <= (type) && (type) <= LOAD_VSX) ||
(type) == LARX)
+#define OP_IS_STORE(type) ((STORE <= (type) && (type) <= STORE_VSX) ||
(type) == STCX)
#define OP_IS_LOAD_STORE(type) (LOAD <= (type) && (type) <= STCX)
/* Compute flags, ORed in with type */
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 4366bd0c90c4..da858bc6a414 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -31,7 +31,7 @@
* Stores the breakpoints currently in use on each breakpoint address
* register for every cpu
*/
-static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
+static DEFINE_PER_CPU(struct perf_event *, bp_per_reg[HBP_NUM_MAX]);
/*
* Returns total number of data or instruction breakpoints available.
@@ -43,6 +43,17 @@ int hw_breakpoint_slots(int type)
return 0; /* no instruction breakpoints available */
}
+static bool single_step_pending(void)
+{
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ if (current->thread.last_hit_ubp[i])
+ return true;
+ }
+ return false;
+}
+
/*
* Install a perf counter breakpoint.
*
@@ -55,16 +66,26 @@ int hw_breakpoint_slots(int type)
int arch_install_hw_breakpoint(struct perf_event *bp)
{
struct arch_hw_breakpoint *info = counter_arch_bp(bp);
- struct perf_event **slot = this_cpu_ptr(_per_reg);
+ struct perf_event **slot;
+ int i;
+
+ for (i = 0; i < nr_wp_slots(); i++) {
+ slot = this_cpu_ptr(_per_reg[i]);
+ if (!*slot) {
+ *slot = bp;
+ break;
+ }
+ }
- *slot = bp;
+ if (WARN_ONCE(i == nr_wp_slots(), "Can't find any breakpoint slot"))
+ return -EBUSY;
/*
* Do not install DABR values if the instruction must be single-stepped.
* If so, DABR will be populated in single_step_dabr_instruction().
*/
- if (current->thread.last_hit_ubp != bp)
- __set_breakpoint(0, info);
+ if (!single_step_pending())
+ __set_breakpoint(i, info);
return 0;
}
@@ -80,15 +101,22 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
*/
void arch_uninstall_hw_breakpoint(struct perf_event *bp)
{
- struct perf_event **slot = this_cpu_ptr(_per_reg);
+ struct arch_hw_breakpoint null_brk = {0};
+ struct perf_event **slot;
+ int i;
- if (*slot != bp) {
- WARN_ONCE(1, "Can't find the breakpoint");
- return;
+ for (i = 0; i < nr_wp_slots(); i++) {
+ slot = this_cpu_ptr(_per_reg[i]);
+ if (*slot == bp) {
+ *slot = NULL;
+ break;
+ }
}
- *slot = NULL;
- h
[PATCH v6 12/16] powerpc/watchpoint: Use builtin ALIGN*() macros
Currently we calculate hw aligned start and end addresses manually.
Replace them with builtin ALIGN_DOWN() and ALIGN() macros.
So far end_addr was inclusive but this patch makes it exclusive (by
avoiding -1) for better readability.
Suggested-by: Christophe Leroy
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/hw_breakpoint.h | 5 +++--
arch/powerpc/kernel/hw_breakpoint.c | 12 ++--
arch/powerpc/kernel/process.c | 8
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 2 +-
4 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index d472b2eb757e..add5aa076919 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -34,10 +34,11 @@ struct arch_hw_breakpoint {
#define HW_BRK_TYPE_PRIV_ALL (HW_BRK_TYPE_USER | HW_BRK_TYPE_KERNEL | \
HW_BRK_TYPE_HYP)
+/* Minimum granularity */
#ifdef CONFIG_PPC_8xx
-#define HW_BREAKPOINT_ALIGN 0x3
+#define HW_BREAKPOINT_SIZE 0x4
#else
-#define HW_BREAKPOINT_ALIGN 0x7
+#define HW_BREAKPOINT_SIZE 0x8
#endif
#define DABR_MAX_LEN 8
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 8028a2704874..4366bd0c90c4 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -146,10 +146,10 @@ int arch_bp_generic_fields(int type, int *gen_bp_type)
*<---8 bytes--->
*
* In this case, we should configure hw as:
- * start_addr = address & ~HW_BREAKPOINT_ALIGN
+ * start_addr = address & ~(HW_BREAKPOINT_SIZE - 1)
* len = 16 bytes
*
- * @start_addr and @end_addr are inclusive.
+ * @start_addr is inclusive but @end_addr is exclusive.
*/
static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw)
{
@@ -157,14 +157,14 @@ static int hw_breakpoint_validate_len(struct
arch_hw_breakpoint *hw)
u16 hw_len;
unsigned long start_addr, end_addr;
- start_addr = hw->address & ~HW_BREAKPOINT_ALIGN;
- end_addr = (hw->address + hw->len - 1) | HW_BREAKPOINT_ALIGN;
- hw_len = end_addr - start_addr + 1;
+ start_addr = ALIGN_DOWN(hw->address, HW_BREAKPOINT_SIZE);
+ end_addr = ALIGN(hw->address + hw->len, HW_BREAKPOINT_SIZE);
+ hw_len = end_addr - start_addr;
if (dawr_enabled()) {
max_len = DAWR_MAX_LEN;
/* DAWR region can't cross 512 bytes boundary */
- if ((start_addr >> 9) != (end_addr >> 9))
+ if (ALIGN(start_addr, SZ_512M) != ALIGN(end_addr - 1, SZ_512M))
return -EINVAL;
} else if (IS_ENABLED(CONFIG_PPC_8xx)) {
/* 8xx can setup a range without limitation */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 77ec1299e2fd..9b11575dcb8a 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -800,12 +800,12 @@ static inline int set_breakpoint_8xx(struct
arch_hw_breakpoint *brk)
unsigned long lctrl1 = LCTRL1_CTE_GT | LCTRL1_CTF_LT | LCTRL1_CRWE_RW |
LCTRL1_CRWF_RW;
unsigned long lctrl2 = LCTRL2_LW0EN | LCTRL2_LW0LADC | LCTRL2_SLW0EN;
- unsigned long start_addr = brk->address & ~HW_BREAKPOINT_ALIGN;
- unsigned long end_addr = (brk->address + brk->len - 1) |
HW_BREAKPOINT_ALIGN;
+ unsigned long start_addr = ALIGN_DOWN(brk->address, HW_BREAKPOINT_SIZE);
+ unsigned long end_addr = ALIGN(brk->address + brk->len,
HW_BREAKPOINT_SIZE);
if (start_addr == 0)
lctrl2 |= LCTRL2_LW0LA_F;
- else if (end_addr == ~0U)
+ else if (end_addr == 0)
lctrl2 |= LCTRL2_LW0LA_E;
else
lctrl2 |= LCTRL2_LW0LA_EandF;
@@ -821,7 +821,7 @@ static inline int set_breakpoint_8xx(struct
arch_hw_breakpoint *brk)
lctrl1 |= LCTRL1_CRWE_WO | LCTRL1_CRWF_WO;
mtspr(SPRN_CMPE, start_addr - 1);
- mtspr(SPRN_CMPF, end_addr + 1);
+ mtspr(SPRN_CMPF, end_addr);
mtspr(SPRN_LCTRL1, lctrl1);
mtspr(SPRN_LCTRL2, lctrl2);
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 08cb8c1b504c..697c7e4b5877 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -216,7 +216,7 @@ long ppc_set_hwdebug(struct task_struct *child, struct
ppc_hw_breakpoint *bp_inf
if ((unsigned long)bp_info->addr >= TASK_SIZE)
return -EIO;
- brk.address = bp_info->addr & ~HW_BREAKPOINT_ALIGN;
+ brk.address = ALIGN_DOWN(bp_info->addr, HW_BREAKPOINT_SIZE);
brk.type = HW_BRK_TYPE_TRANSLATE;
brk.len = DABR_MAX_LEN;
if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
--
2.26.2
[PATCH v6 14/16] powerpc/watchpoint: Don't allow concurrent perf and ptrace events
With Book3s DAWR, ptrace and perf watchpoints on powerpc behaves
differently. Ptrace watchpoint works in one-shot mode and generates
signal before executing instruction. It's ptrace user's job to
single-step the instruction and re-enable the watchpoint. OTOH, in
case of perf watchpoint, kernel emulates/single-steps the instruction
and then generates event. If perf and ptrace creates two events with
same or overlapping address ranges, it's ambiguous to decide who
should single-step the instruction. Because of this issue, don't
allow perf and ptrace watchpoint at the same time if their address
range overlaps.
Signed-off-by: Ravi Bangoria
Reviewed-by: Michael Neuling
---
arch/powerpc/include/asm/hw_breakpoint.h | 2 +
arch/powerpc/kernel/hw_breakpoint.c | 221 +++
kernel/events/hw_breakpoint.c| 16 ++
3 files changed, 239 insertions(+)
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index add5aa076919..f42a55eb77d2 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -70,6 +70,8 @@ extern int hw_breakpoint_exceptions_notify(struct
notifier_block *unused,
unsigned long val, void *data);
int arch_install_hw_breakpoint(struct perf_event *bp);
void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+int arch_reserve_bp_slot(struct perf_event *bp);
+void arch_release_bp_slot(struct perf_event *bp);
void arch_unregister_hw_breakpoint(struct perf_event *bp);
void hw_breakpoint_pmu_read(struct perf_event *bp);
extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index da858bc6a414..80396f3f7e90 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -124,6 +124,227 @@ static bool is_ptrace_bp(struct perf_event *bp)
return bp->overflow_handler == ptrace_triggered;
}
+struct breakpoint {
+ struct list_head list;
+ struct perf_event *bp;
+ bool ptrace_bp;
+};
+
+static DEFINE_PER_CPU(struct breakpoint *, cpu_bps[HBP_NUM_MAX]);
+static LIST_HEAD(task_bps);
+
+static struct breakpoint *alloc_breakpoint(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ tmp = kzalloc(sizeof(*tmp), GFP_KERNEL);
+ if (!tmp)
+ return ERR_PTR(-ENOMEM);
+ tmp->bp = bp;
+ tmp->ptrace_bp = is_ptrace_bp(bp);
+ return tmp;
+}
+
+static bool bp_addr_range_overlap(struct perf_event *bp1, struct perf_event
*bp2)
+{
+ __u64 bp1_saddr, bp1_eaddr, bp2_saddr, bp2_eaddr;
+
+ bp1_saddr = ALIGN_DOWN(bp1->attr.bp_addr, HW_BREAKPOINT_SIZE);
+ bp1_eaddr = ALIGN(bp1->attr.bp_addr + bp1->attr.bp_len,
HW_BREAKPOINT_SIZE);
+ bp2_saddr = ALIGN_DOWN(bp2->attr.bp_addr, HW_BREAKPOINT_SIZE);
+ bp2_eaddr = ALIGN(bp2->attr.bp_addr + bp2->attr.bp_len,
HW_BREAKPOINT_SIZE);
+
+ return (bp1_saddr < bp2_eaddr && bp1_eaddr > bp2_saddr);
+}
+
+static bool alternate_infra_bp(struct breakpoint *b, struct perf_event *bp)
+{
+ return is_ptrace_bp(bp) ? !b->ptrace_bp : b->ptrace_bp;
+}
+
+static bool can_co_exist(struct breakpoint *b, struct perf_event *bp)
+{
+ return !(alternate_infra_bp(b, bp) && bp_addr_range_overlap(b->bp, bp));
+}
+
+static int task_bps_add(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ tmp = alloc_breakpoint(bp);
+ if (IS_ERR(tmp))
+ return PTR_ERR(tmp);
+
+ list_add(>list, _bps);
+ return 0;
+}
+
+static void task_bps_remove(struct perf_event *bp)
+{
+ struct list_head *pos, *q;
+
+ list_for_each_safe(pos, q, _bps) {
+ struct breakpoint *tmp = list_entry(pos, struct breakpoint,
list);
+
+ if (tmp->bp == bp) {
+ list_del(>list);
+ kfree(tmp);
+ break;
+ }
+ }
+}
+
+/*
+ * If any task has breakpoint from alternate infrastructure,
+ * return true. Otherwise return false.
+ */
+static bool all_task_bps_check(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ list_for_each_entry(tmp, _bps, list) {
+ if (!can_co_exist(tmp, bp))
+ return true;
+ }
+ return false;
+}
+
+/*
+ * If same task has breakpoint from alternate infrastructure,
+ * return true. Otherwise return false.
+ */
+static bool same_task_bps_check(struct perf_event *bp)
+{
+ struct breakpoint *tmp;
+
+ list_for_each_entry(tmp, _bps, list) {
+ if (tmp->bp->hw.target == bp->hw.target &&
+ !can_co_exist(tmp, bp))
+ return true;
+ }
+ return false;
+}
+
+static int cpu_bps_add(struct perf_event *bp)
+{
+ struct breakpoint **cpu_bp;
+
[PATCH v3 1/9] powerpc/watchpoint: Fix 512 byte boundary limit
Milton Miller reported that we are aligning start and end address to
wrong size SZ_512M. It should be SZ_512. Fix that.
While doing this change I also found a case where ALIGN() comparison
fails. Within a given aligned range, ALIGN() of two addresses does not
match when start address is pointing to the first byte and end address
is pointing to any other byte except the first one. But that's not true
for ALIGN_DOWN(). ALIGN_DOWN() of any two addresses within that range
will always point to the first byte. So use ALIGN_DOWN() instead of
ALIGN().
Fixes: e68ef121c1f4 ("powerpc/watchpoint: Use builtin ALIGN*() macros")
Reported-by: Milton Miller
Signed-off-by: Ravi Bangoria
---
arch/powerpc/kernel/hw_breakpoint.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index daf0e1da..031e6defc08e 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -419,7 +419,7 @@ static int hw_breakpoint_validate_len(struct
arch_hw_breakpoint *hw)
if (dawr_enabled()) {
max_len = DAWR_MAX_LEN;
/* DAWR region can't cross 512 bytes boundary */
- if (ALIGN(start_addr, SZ_512M) != ALIGN(end_addr - 1, SZ_512M))
+ if (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1,
SZ_512))
return -EINVAL;
} else if (IS_ENABLED(CONFIG_PPC_8xx)) {
/* 8xx can setup a range without limitation */
--
2.26.2
[PATCH v3 4/9] powerpc/dt_cpu_ftrs: Add feature for 2nd DAWR
Add new device-tree feature for 2nd DAWR. If this feature is present,
2nd DAWR is supported, otherwise not.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/asm/cputable.h | 7 +--
arch/powerpc/kernel/dt_cpu_ftrs.c | 7 +++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h
b/arch/powerpc/include/asm/cputable.h
index e506d429b1af..3445c86e1f6f 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -214,6 +214,7 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTR_P9_TLBIE_ERAT_BUG LONG_ASM_CONST(0x0001)
#define CPU_FTR_P9_RADIX_PREFETCH_BUG LONG_ASM_CONST(0x0002)
#define CPU_FTR_ARCH_31
LONG_ASM_CONST(0x0004)
+#define CPU_FTR_DAWR1 LONG_ASM_CONST(0x0008)
#ifndef __ASSEMBLY__
@@ -497,14 +498,16 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTRS_POSSIBLE \
(CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | CPU_FTRS_POWER8 | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_VSX_COMP | CPU_FTRS_POWER9 | \
-CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 |
\
+CPU_FTR_DAWR1)
#else
#define CPU_FTRS_POSSIBLE \
(CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | \
CPU_FTRS_POWER6 | CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | \
CPU_FTRS_POWER8 | CPU_FTRS_CELL | CPU_FTRS_PA6T | \
CPU_FTR_VSX_COMP | CPU_FTR_ALTIVEC_COMP | CPU_FTRS_POWER9 | \
-CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 |
\
+CPU_FTR_DAWR1)
#endif /* CONFIG_CPU_LITTLE_ENDIAN */
#endif
#else
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
b/arch/powerpc/kernel/dt_cpu_ftrs.c
index a0edeb391e3e..be694567cebd 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -573,6 +573,12 @@ static int __init feat_enable_mma(struct dt_cpu_feature *f)
return 1;
}
+static int __init feat_enable_debug_facilities_v31(struct dt_cpu_feature *f)
+{
+ cur_cpu_spec->cpu_features |= CPU_FTR_DAWR1;
+ return 1;
+}
+
struct dt_cpu_feature_match {
const char *name;
int (*enable)(struct dt_cpu_feature *f);
@@ -648,6 +654,7 @@ static struct dt_cpu_feature_match __initdata
{"wait-v3", feat_enable, 0},
{"prefix-instructions", feat_enable, 0},
{"matrix-multiply-assist", feat_enable_mma, 0},
+ {"debug-facilities-v31", feat_enable_debug_facilities_v31, 0},
};
static bool __initdata using_dt_cpu_ftrs;
--
2.26.2
[PATCH v3 3/9] powerpc/watchpoint: Enable watchpoint functionality on power10 guest
CPU_FTR_DAWR is by default enabled for host via CPU_FTRS_DT_CPU_BASE
(controlled by CONFIG_PPC_DT_CPU_FTRS). But cpu-features device-tree
node is not PAPR compatible and thus not yet used by kvm or pHyp
guests. Enable watchpoint functionality on power10 guest (both kvm
and powervm) by adding CPU_FTR_DAWR to CPU_FTRS_POWER10. Note that
this change does not enable 2nd DAWR support.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/asm/cputable.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/cputable.h
b/arch/powerpc/include/asm/cputable.h
index bac2252c839e..e506d429b1af 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -478,7 +478,7 @@ static inline void cpu_feature_keys_init(void) { }
CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | CPU_FTR_PKEY | \
- CPU_FTR_ARCH_31)
+ CPU_FTR_ARCH_31 | CPU_FTR_DAWR)
#define CPU_FTRS_CELL (CPU_FTR_LWSYNC | \
CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
--
2.26.2
[PATCH v3 2/9] powerpc/watchpoint: Fix DAWR exception constraint
Pedro Miraglia Franco de Carvalho noticed that on p8, DAR value is
inconsistent with different type of load/store. Like for byte,word
etc. load/stores, DAR is set to the address of the first byte of
overlap between watch range and real access. But for quadword load/
store it's set to the address of the first byte of real access. This
issue has been fixed in p10. In p10(ISA 3.1), DAR is always set to
the address of the first byte of overlap. Commit 27985b2a640e
("powerpc/watchpoint: Don't ignore extraneous exceptions blindly")
wrongly assumes that DAR is set to the address of the first byte of
overlap for all load/stores on p8 as well. Fix that. With the fix,
we now rely on 'ea' provided by analyse_instr(). If analyse_instr()
fails, generate event unconditionally on p8, and on p10 generate
event only if DAR is within a DAWR range.
Note: 8xx is not affected.
Fixes: 27985b2a640e ("powerpc/watchpoint: Don't ignore extraneous exceptions
blindly")
Fixes: 74c6881019b7 ("powerpc/watchpoint: Prepare handler to handle more than
one watchpoint")
Reported-by: Pedro Miraglia Franco de Carvalho
Signed-off-by: Ravi Bangoria
---
arch/powerpc/kernel/hw_breakpoint.c | 93 +++--
1 file changed, 63 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 031e6defc08e..7a66c370a105 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -498,11 +498,11 @@ static bool dar_in_user_range(unsigned long dar, struct
arch_hw_breakpoint *info
return ((info->address <= dar) && (dar - info->address < info->len));
}
-static bool dar_user_range_overlaps(unsigned long dar, int size,
- struct arch_hw_breakpoint *info)
+static bool ea_user_range_overlaps(unsigned long ea, int size,
+ struct arch_hw_breakpoint *info)
{
- return ((dar < info->address + info->len) &&
- (dar + size > info->address));
+ return ((ea < info->address + info->len) &&
+ (ea + size > info->address));
}
static bool dar_in_hw_range(unsigned long dar, struct arch_hw_breakpoint *info)
@@ -515,20 +515,22 @@ static bool dar_in_hw_range(unsigned long dar, struct
arch_hw_breakpoint *info)
return ((hw_start_addr <= dar) && (hw_end_addr > dar));
}
-static bool dar_hw_range_overlaps(unsigned long dar, int size,
- struct arch_hw_breakpoint *info)
+static bool ea_hw_range_overlaps(unsigned long ea, int size,
+struct arch_hw_breakpoint *info)
{
unsigned long hw_start_addr, hw_end_addr;
hw_start_addr = ALIGN_DOWN(info->address, HW_BREAKPOINT_SIZE);
hw_end_addr = ALIGN(info->address + info->len, HW_BREAKPOINT_SIZE);
- return ((dar < hw_end_addr) && (dar + size > hw_start_addr));
+ return ((ea < hw_end_addr) && (ea + size > hw_start_addr));
}
/*
* If hw has multiple DAWR registers, we also need to check all
* dawrx constraint bits to confirm this is _really_ a valid event.
+ * If type is UNKNOWN, but privilege level matches, consider it as
+ * a positive match.
*/
static bool check_dawrx_constraints(struct pt_regs *regs, int type,
struct arch_hw_breakpoint *info)
@@ -536,7 +538,12 @@ static bool check_dawrx_constraints(struct pt_regs *regs,
int type,
if (OP_IS_LOAD(type) && !(info->type & HW_BRK_TYPE_READ))
return false;
- if (OP_IS_STORE(type) && !(info->type & HW_BRK_TYPE_WRITE))
+ /*
+* The Cache Management instructions other than dcbz never
+* cause a match. i.e. if type is CACHEOP, the instruction
+* is dcbz, and dcbz is treated as Store.
+*/
+ if ((OP_IS_STORE(type) || type == CACHEOP) && !(info->type &
HW_BRK_TYPE_WRITE))
return false;
if (is_kernel_addr(regs->nip) && !(info->type & HW_BRK_TYPE_KERNEL))
@@ -553,7 +560,8 @@ static bool check_dawrx_constraints(struct pt_regs *regs,
int type,
* including extraneous exception. Otherwise return false.
*/
static bool check_constraints(struct pt_regs *regs, struct ppc_inst instr,
- int type, int size, struct arch_hw_breakpoint
*info)
+ unsigned long ea, int type, int size,
+ struct arch_hw_breakpoint *info)
{
bool in_user_range = dar_in_user_range(regs->dar, info);
bool dawrx_constraints;
@@ -569,11 +577,10 @@ static bool check_constraints(struct pt_regs *regs,
struct ppc_inst instr,
}
if (unlikely(ppc_inst_equal(instr, ppc_inst(0 {
- if (in_user_ran
[PATCH v3 0/9] powerpc/watchpoint: Enable 2nd DAWR on baremetal and powervm
Last series[1] was to add basic infrastructure support for more than one watchpoint on Book3S powerpc. This series actually enables the 2nd DAWR for baremetal and powervm. Kvm guest is still not supported. v2: https://lore.kernel.org/linuxppc-dev/20200604033443.70591-1-ravi.bango...@linux.ibm.com/ v2->v3: - patch #2 is new. It fixes an issue with DAWR exception constraint - Rename dawr1 to debug-facilities-v31 in dt cpu feature, suggested by Nick Piggin. - Rebased to powerpc/next [1]: https://lore.kernel.org/linuxppc-dev/20200514111741.97993-1-ravi.bango...@linux.ibm.com/ Ravi Bangoria (9): powerpc/watchpoint: Fix 512 byte boundary limit powerpc/watchpoint: Fix DAWR exception constraint powerpc/watchpoint: Enable watchpoint functionality on power10 guest powerpc/dt_cpu_ftrs: Add feature for 2nd DAWR powerpc/watchpoint: Set CPU_FTR_DAWR1 based on pa-features bit powerpc/watchpoint: Rename current H_SET_MODE DAWR macro powerpc/watchpoint: Guest support for 2nd DAWR hcall powerpc/watchpoint: Return available watchpoints dynamically powerpc/watchpoint: Remove 512 byte boundary arch/powerpc/include/asm/cputable.h | 13 ++- arch/powerpc/include/asm/hvcall.h | 3 +- arch/powerpc/include/asm/hw_breakpoint.h | 5 +- arch/powerpc/include/asm/machdep.h| 2 +- arch/powerpc/include/asm/plpar_wrappers.h | 7 +- arch/powerpc/kernel/dawr.c| 2 +- arch/powerpc/kernel/dt_cpu_ftrs.c | 7 ++ arch/powerpc/kernel/hw_breakpoint.c | 98 +++ arch/powerpc/kernel/prom.c| 2 + arch/powerpc/kvm/book3s_hv.c | 2 +- arch/powerpc/platforms/pseries/setup.c| 7 +- 11 files changed, 103 insertions(+), 45 deletions(-) -- 2.26.2
[PATCH v3 8/9] powerpc/watchpoint: Return available watchpoints dynamically
So far Book3S Powerpc supported only one watchpoint. Power10 is
introducing 2nd DAWR. Enable 2nd DAWR support for Power10.
Availability of 2nd DAWR will depend on CPU_FTR_DAWR1.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/asm/cputable.h | 4 +++-
arch/powerpc/include/asm/hw_breakpoint.h | 5 +++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h
b/arch/powerpc/include/asm/cputable.h
index 3445c86e1f6f..36a0851a7a9b 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -633,7 +633,9 @@ enum {
* Maximum number of hw breakpoint supported on powerpc. Number of
* breakpoints supported by actual hw might be less than this.
*/
-#define HBP_NUM_MAX1
+#define HBP_NUM_MAX2
+#define HBP_NUM_ONE1
+#define HBP_NUM_TWO2
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index cb424799da0d..d4eab1694bcd 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -5,10 +5,11 @@
* Copyright 2010, IBM Corporation.
* Author: K.Prasad
*/
-
#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#include
+
#ifdef __KERNEL__
struct arch_hw_breakpoint {
unsigned long address;
@@ -46,7 +47,7 @@ struct arch_hw_breakpoint {
static inline int nr_wp_slots(void)
{
- return HBP_NUM_MAX;
+ return cpu_has_feature(CPU_FTR_DAWR1) ? HBP_NUM_TWO : HBP_NUM_ONE;
}
#ifdef CONFIG_HAVE_HW_BREAKPOINT
--
2.26.2
[PATCH v3 7/9] powerpc/watchpoint: Guest support for 2nd DAWR hcall
2nd DAWR can be set/unset using H_SET_MODE hcall with resource value 5.
Enable powervm guest support with that. This has no effect on kvm guest
because kvm will return error if guest does hcall with resource value 5.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/asm/hvcall.h | 1 +
arch/powerpc/include/asm/machdep.h| 2 +-
arch/powerpc/include/asm/plpar_wrappers.h | 5 +
arch/powerpc/kernel/dawr.c| 2 +-
arch/powerpc/platforms/pseries/setup.c| 7 +--
5 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h
b/arch/powerpc/include/asm/hvcall.h
index a7f6f1aeda6b..3f170b9496a1 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -357,6 +357,7 @@
#define H_SET_MODE_RESOURCE_SET_DAWR0 2
#define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE3
#define H_SET_MODE_RESOURCE_LE 4
+#define H_SET_MODE_RESOURCE_SET_DAWR1 5
/* Values for argument to H_SIGNAL_SYS_RESET */
#define H_SIGNAL_SYS_RESET_ALL -1
diff --git a/arch/powerpc/include/asm/machdep.h
b/arch/powerpc/include/asm/machdep.h
index 7bcb6a39..a90b892f0bfe 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -131,7 +131,7 @@ struct machdep_calls {
unsigned long dabrx);
/* Set DAWR for this platform, leave empty for default implementation */
- int (*set_dawr)(unsigned long dawr,
+ int (*set_dawr)(int nr, unsigned long dawr,
unsigned long dawrx);
#ifdef CONFIG_PPC32/* XXX for now */
diff --git a/arch/powerpc/include/asm/plpar_wrappers.h
b/arch/powerpc/include/asm/plpar_wrappers.h
index 93eb133d572c..d7a1acc83593 100644
--- a/arch/powerpc/include/asm/plpar_wrappers.h
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
@@ -315,6 +315,11 @@ static inline long plpar_set_watchpoint0(unsigned long
dawr0, unsigned long dawr
return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR0, dawr0, dawrx0);
}
+static inline long plpar_set_watchpoint1(unsigned long dawr1, unsigned long
dawrx1)
+{
+ return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR1, dawr1, dawrx1);
+}
+
static inline long plpar_signal_sys_reset(long cpu)
{
return plpar_hcall_norets(H_SIGNAL_SYS_RESET, cpu);
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index 500f52fa4711..cdc2dccb987d 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -37,7 +37,7 @@ int set_dawr(int nr, struct arch_hw_breakpoint *brk)
dawrx |= (mrd & 0x3f) << (63 - 53);
if (ppc_md.set_dawr)
- return ppc_md.set_dawr(dawr, dawrx);
+ return ppc_md.set_dawr(nr, dawr, dawrx);
if (nr == 0) {
mtspr(SPRN_DAWR0, dawr);
diff --git a/arch/powerpc/platforms/pseries/setup.c
b/arch/powerpc/platforms/pseries/setup.c
index 2db8469e475f..d516ee8eb7fc 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -831,12 +831,15 @@ static int pseries_set_xdabr(unsigned long dabr, unsigned
long dabrx)
return plpar_hcall_norets(H_SET_XDABR, dabr, dabrx);
}
-static int pseries_set_dawr(unsigned long dawr, unsigned long dawrx)
+static int pseries_set_dawr(int nr, unsigned long dawr, unsigned long dawrx)
{
/* PAPR says we can't set HYP */
dawrx &= ~DAWRX_HYP;
- return plpar_set_watchpoint0(dawr, dawrx);
+ if (nr == 0)
+ return plpar_set_watchpoint0(dawr, dawrx);
+ else
+ return plpar_set_watchpoint1(dawr, dawrx);
}
#define CMO_CHARACTERISTICS_TOKEN 44
--
2.26.2
[PATCH v3 5/9] powerpc/watchpoint: Set CPU_FTR_DAWR1 based on pa-features bit
As per the PAPR, bit 0 of byte 64 in pa-features property indicates
availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
DAWR is present, otherwise not. Host generally uses "cpu-features",
which masks "pa-features". But "cpu-features" are still not used for
guests and thus this change is mostly applicable for guests only.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/kernel/prom.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 9cc49f265c86..c76c09b97bc8 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -175,6 +175,8 @@ static struct ibm_pa_feature {
*/
{ .pabyte = 22, .pabit = 0, .cpu_features = CPU_FTR_TM_COMP,
.cpu_user_ftrs2 = PPC_FEATURE2_HTM_COMP | PPC_FEATURE2_HTM_NOSC_COMP
},
+
+ { .pabyte = 64, .pabit = 0, .cpu_features = CPU_FTR_DAWR1 },
};
static void __init scan_features(unsigned long node, const unsigned char *ftrs,
--
2.26.2
[PATCH v3 6/9] powerpc/watchpoint: Rename current H_SET_MODE DAWR macro
Current H_SET_MODE hcall macro name for setting/resetting DAWR0 is
H_SET_MODE_RESOURCE_SET_DAWR. Add suffix 0 to macro name as well.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/asm/hvcall.h | 2 +-
arch/powerpc/include/asm/plpar_wrappers.h | 2 +-
arch/powerpc/kvm/book3s_hv.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h
b/arch/powerpc/include/asm/hvcall.h
index e90c073e437e..a7f6f1aeda6b 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -354,7 +354,7 @@
/* Values for 2nd argument to H_SET_MODE */
#define H_SET_MODE_RESOURCE_SET_CIABR 1
-#define H_SET_MODE_RESOURCE_SET_DAWR 2
+#define H_SET_MODE_RESOURCE_SET_DAWR0 2
#define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE3
#define H_SET_MODE_RESOURCE_LE 4
diff --git a/arch/powerpc/include/asm/plpar_wrappers.h
b/arch/powerpc/include/asm/plpar_wrappers.h
index 4497c8afb573..93eb133d572c 100644
--- a/arch/powerpc/include/asm/plpar_wrappers.h
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
@@ -312,7 +312,7 @@ static inline long plpar_set_ciabr(unsigned long ciabr)
static inline long plpar_set_watchpoint0(unsigned long dawr0, unsigned long
dawrx0)
{
- return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR, dawr0, dawrx0);
+ return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR0, dawr0, dawrx0);
}
static inline long plpar_signal_sys_reset(long cpu)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 6bf66649ab92..7ad692c2d7c7 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -764,7 +764,7 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu,
unsigned long mflags,
return H_P3;
vcpu->arch.ciabr = value1;
return H_SUCCESS;
- case H_SET_MODE_RESOURCE_SET_DAWR:
+ case H_SET_MODE_RESOURCE_SET_DAWR0:
if (!kvmppc_power8_compatible(vcpu))
return H_P2;
if (!ppc_breakpoint_available())
--
2.26.2
[PATCH v3 9/9] powerpc/watchpoint: Remove 512 byte boundary
Power10 has removed 512 bytes boundary from match criteria. i.e. The watch
range can cross 512 bytes boundary.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/kernel/hw_breakpoint.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 7a66c370a105..270fbb4d01ce 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -418,8 +418,9 @@ static int hw_breakpoint_validate_len(struct
arch_hw_breakpoint *hw)
if (dawr_enabled()) {
max_len = DAWR_MAX_LEN;
- /* DAWR region can't cross 512 bytes boundary */
- if (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1,
SZ_512))
+ /* DAWR region can't cross 512 bytes boundary on p10
predecessors */
+ if (!cpu_has_feature(CPU_FTR_ARCH_31) &&
+ (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1,
SZ_512)))
return -EINVAL;
} else if (IS_ENABLED(CONFIG_PPC_8xx)) {
/* 8xx can setup a range without limitation */
--
2.26.2
[PATCH] powerpc/watchpoint/ptrace: Introduce PPC_DEBUG_FEATURE_DATA_BP_DAWR_ARCH_31
PPC_DEBUG_FEATURE_DATA_BP_DAWR_ARCH_31 can be used to determine
whether we are running on an ISA 3.1 compliant machine. Which is
needed to determine DAR behaviour, 512 byte boundary limit etc.
This was requested by Pedro Miraglia Franco de Carvalho for
extending watchpoint features in gdb. Note that availability of
2nd DAWR is independent of this flag and should be checked using
ppc_debug_info->num_data_bps.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/uapi/asm/ptrace.h| 1 +
arch/powerpc/kernel/ptrace/ptrace-noadv.c | 5 -
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/uapi/asm/ptrace.h
b/arch/powerpc/include/uapi/asm/ptrace.h
index f5f1ccc740fc..0a87bcd4300a 100644
--- a/arch/powerpc/include/uapi/asm/ptrace.h
+++ b/arch/powerpc/include/uapi/asm/ptrace.h
@@ -222,6 +222,7 @@ struct ppc_debug_info {
#define PPC_DEBUG_FEATURE_DATA_BP_RANGE0x0004
#define PPC_DEBUG_FEATURE_DATA_BP_MASK 0x0008
#define PPC_DEBUG_FEATURE_DATA_BP_DAWR 0x0010
+#define PPC_DEBUG_FEATURE_DATA_BP_DAWR_ARCH_31 0x0020
#ifndef __ASSEMBLY__
diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
index 697c7e4b5877..b2de874d650b 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
@@ -52,8 +52,11 @@ void ppc_gethwdinfo(struct ppc_debug_info *dbginfo)
dbginfo->sizeof_condition = 0;
if (IS_ENABLED(CONFIG_HAVE_HW_BREAKPOINT)) {
dbginfo->features = PPC_DEBUG_FEATURE_DATA_BP_RANGE;
- if (dawr_enabled())
+ if (dawr_enabled()) {
dbginfo->features |= PPC_DEBUG_FEATURE_DATA_BP_DAWR;
+ if (cpu_has_feature(CPU_FTR_ARCH_31))
+ dbginfo->features |=
PPC_DEBUG_FEATURE_DATA_BP_DAWR_ARCH_31;
+ }
} else {
dbginfo->features = 0;
}
--
2.26.2
Re: [PATCH v8 00/12] Introduce CAP_PERFMON to secure system performance monitoring and observability
Hi Alexey,
Currently access to perf_events, i915_perf and other performance
monitoring and observability subsystems of the kernel is open only for
a privileged process [1] with CAP_SYS_ADMIN capability enabled in the
process effective set [2].
This patch set introduces CAP_PERFMON capability designed to secure
system performance monitoring and observability operations so that
CAP_PERFMON would assist CAP_SYS_ADMIN capability in its governing role
for performance monitoring and observability subsystems of the kernel.
I'm seeing an issue with CAP_PERFMON when I try to record data for a
specific target. I don't know whether this is sort of a regression or
an expected behavior.
Without setting CAP_PERFMON:
$ getcap ./perf
$ ./perf stat -a ls
Error:
Access to performance monitoring and observability operations is limited.
$ ./perf stat ls
Performance counter stats for 'ls':
2.06 msec task-clock:u #0.418 CPUs utilized
0 context-switches:u#0.000 K/sec
0 cpu-migrations:u #0.000 K/sec
With CAP_PERFMON:
$ getcap ./perf
./perf = cap_perfmon+ep
$ ./perf stat -a ls
Performance counter stats for 'system wide':
142.42 msec cpu-clock # 25.062 CPUs utilized
182 context-switches #0.001 M/sec
48 cpu-migrations#0.337 K/sec
$ ./perf stat ls
Error:
Access to performance monitoring and observability operations is limited.
Am I missing something silly?
Analysis:
-
A bit more analysis lead me to below kernel code fs/exec.c:
begin_new_exec()
{
...
if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
!(uid_eq(current_euid(), current_uid()) &&
gid_eq(current_egid(), current_gid(
set_dumpable(current->mm, suid_dumpable);
else
set_dumpable(current->mm, SUID_DUMP_USER);
...
commit_creds(bprm->cred);
}
When I execute './perf stat ls', it's going into else condition and thus sets
dumpable flag as SUID_DUMP_USER. Then in commit_creds():
int commit_creds(struct cred *new)
{
...
/* dumpability changes */
if (...
!cred_cap_issubset(old, new)) {
if (task->mm)
set_dumpable(task->mm, suid_dumpable);
}
!cred_cap_issubset(old, new) fails for perf without any capability and thus
it doesn't execute set_dumpable(). Whereas that condition passes for perf
with CAP_PERFMON and thus it overwrites old value (SUID_DUMP_USER) with
suid_dumpable in mm_flags. On an Ubuntu, suid_dumpable default value is
SUID_DUMP_ROOT. On Fedora, it's SUID_DUMP_DISABLE. (/proc/sys/fs/suid_dumpable).
Now while opening an event:
perf_event_open()
ptrace_may_access()
__ptrace_may_access() {
...
if (mm &&
((get_dumpable(mm) != SUID_DUMP_USER) &&
!ptrace_has_cap(cred, mm->user_ns, mode)))
return -EPERM;
}
This if condition passes for perf with CAP_PERFMON and thus it returns -EPERM.
But it fails for perf without CAP_PERFMON and thus it goes ahead and returns
success. So opening an event fails when perf has CAP_PREFMON and tries to open
process specific event as normal user.
Workarounds:
Based on above analysis, I found couple of workarounds (examples are on
Ubuntu 18.04.4 powerpc):
Workaround1:
Setting SUID_DUMP_USER as default (in /proc/sys/fs/suid_dumpable) solves the
issue.
# echo 1 > /proc/sys/fs/suid_dumpable
$ getcap ./perf
./perf = cap_perfmon+ep
$ ./perf stat ls
Performance counter stats for 'ls':
1.47 msec task-clock#0.806 CPUs utilized
0 context-switches #0.000 K/sec
0 cpu-migrations#0.000 K/sec
Workaround2:
Using CAP_SYS_PTRACE along with CAP_PERFMON solves the issue.
$ cat /proc/sys/fs/suid_dumpable
2
# setcap "cap_perfmon,cap_sys_ptrace=ep" ./perf
$ ./perf stat ls
Performance counter stats for 'ls':
1.41 msec task-clock#0.826 CPUs utilized
0 context-switches #0.000 K/sec
0 cpu-migrations#0.000 K/sec
Workaround3:
Adding CAP_PERFMON to parent of perf (/bin/bash) also solves the issue.
$ cat /proc/sys/fs/suid_dumpable
2
# setcap "cap_perfmon=ep" /bin/bash
# setcap "cap_perfmon=ep" ./perf
$ bash
$ ./perf stat ls
Performance counter stats for 'ls':
1.47 msec task-clock#0.806 CPUs utilized
0 context-switches #0.000 K/sec
0 cpu-migrations#0.000 K/sec
- Ravi
[PATCH 1/2] libsubcmd: Fix OPT_CALLBACK_SET()
Any option macro with _SET suffix should set opt->set variable which
is not happening for OPT_CALLBACK_SET(). This is causing issues with
perf record --switch-output-event. Fix that.
Before:
# ./perf record --overwrite -e sched:*switch,syscalls:sys_enter_mmap \
--switch-output-event syscalls:sys_enter_mmap
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.297 MB perf.data (657 samples) ]
After:
$ ./perf record --overwrite -e sched:*switch,syscalls:sys_enter_mmap \
--switch-output-event syscalls:sys_enter_mmap
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2020061918144542 ]
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2020061918144608 ]
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2020061918144660 ]
^C[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2020061918144784 ]
[ perf record: Woken up 0 times to write data ]
[ perf record: Dump perf.data.2020061918144803 ]
[ perf record: Captured and wrote 0.419 MB perf.data. ]
Fixes: 636eb4d001b1 ("libsubcmd: Introduce OPT_CALLBACK_SET()")
Signed-off-by: Ravi Bangoria
---
tools/lib/subcmd/parse-options.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c
index dbb9efbf718a..39ebf6192016 100644
--- a/tools/lib/subcmd/parse-options.c
+++ b/tools/lib/subcmd/parse-options.c
@@ -237,6 +237,9 @@ static int get_value(struct parse_opt_ctx_t *p,
return err;
case OPTION_CALLBACK:
+ if (opt->set)
+ *(bool *)opt->set = true;
+
if (unset)
return (*opt->callback)(opt, NULL, 1) ? (-1) : 0;
if (opt->flags & PARSE_OPT_NOARG)
--
2.17.1
[PATCH 2/2] perf record: Fix --switch-output=time documentation
perf record man page uses word 'size' while describing --switch-output=time option. Fix that. Signed-off-by: Ravi Bangoria --- tools/perf/Documentation/perf-record.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt index fa8a5fcd27ab..5256f5e12dde 100644 --- a/tools/perf/Documentation/perf-record.txt +++ b/tools/perf/Documentation/perf-record.txt @@ -540,7 +540,7 @@ based on 'mode' value: "signal" - when receiving a SIGUSR2 (default value) or - when reaching the size threshold, size is expected to be a number with appended unit character - B/K/M/G - - when reaching the time threshold, size is expected to + - when reaching the time threshold, time is expected to be a number with appended unit character - s/m/h/d Note: the precision of the size threshold hugely depends -- 2.17.1
[PATCH v2 0/8] powerpc/watchpoint: Enable 2nd DAWR on baremetal and powervm
Last series[1] was to add basic infrastructure support for more than one watchpoint on Book3S powerpc. This series actually enables the 2nd DAWR for baremetal and powervm. Kvm guest is still not supported. v1: https://lore.kernel.org/linuxppc-dev/20200602040106.127693-1-ravi.bango...@linux.ibm.com v1->v2: - Milton reported an issue with one patch in last series[1]. patch #1 fixes that. So patch#1 is new. - Rebased to powerpc/next which now has "Base support for POWER10"[2] series included. [1]: https://lore.kernel.org/linuxppc-dev/20200514111741.97993-1-ravi.bango...@linux.ibm.com/ [2]: https://lore.kernel.org/linuxppc-dev/20200521014341.29095-1-alist...@popple.id.au Ravi Bangoria (8): powerpc/watchpoint: Fix 512 byte boundary limit powerpc/watchpoint: Enable watchpoint functionality on power10 guest powerpc/dt_cpu_ftrs: Add feature for 2nd DAWR powerpc/watchpoint: Set CPU_FTR_DAWR1 based on pa-features bit powerpc/watchpoint: Rename current H_SET_MODE DAWR macro powerpc/watchpoint: Guest support for 2nd DAWR hcall powerpc/watchpoint: Return available watchpoints dynamically powerpc/watchpoint: Remove 512 byte boundary arch/powerpc/include/asm/cputable.h | 13 + arch/powerpc/include/asm/hvcall.h | 3 ++- arch/powerpc/include/asm/hw_breakpoint.h | 5 +++-- arch/powerpc/include/asm/machdep.h| 2 +- arch/powerpc/include/asm/plpar_wrappers.h | 7 ++- arch/powerpc/kernel/dawr.c| 2 +- arch/powerpc/kernel/dt_cpu_ftrs.c | 7 +++ arch/powerpc/kernel/hw_breakpoint.c | 5 +++-- arch/powerpc/kernel/prom.c| 2 ++ arch/powerpc/kvm/book3s_hv.c | 2 +- arch/powerpc/platforms/pseries/setup.c| 7 +-- 11 files changed, 40 insertions(+), 15 deletions(-) -- 2.26.2
[PATCH v2 6/8] powerpc/watchpoint: Guest support for 2nd DAWR hcall
2nd DAWR can be set/unset using H_SET_MODE hcall with resource value 5.
Enable powervm guest support with that. This has no effect on kvm guest
because kvm will return error if guest does hcall with resource value 5.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/asm/hvcall.h | 1 +
arch/powerpc/include/asm/machdep.h| 2 +-
arch/powerpc/include/asm/plpar_wrappers.h | 5 +
arch/powerpc/kernel/dawr.c| 2 +-
arch/powerpc/platforms/pseries/setup.c| 7 +--
5 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h
b/arch/powerpc/include/asm/hvcall.h
index a7f6f1aeda6b..3f170b9496a1 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -357,6 +357,7 @@
#define H_SET_MODE_RESOURCE_SET_DAWR0 2
#define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE3
#define H_SET_MODE_RESOURCE_LE 4
+#define H_SET_MODE_RESOURCE_SET_DAWR1 5
/* Values for argument to H_SIGNAL_SYS_RESET */
#define H_SIGNAL_SYS_RESET_ALL -1
diff --git a/arch/powerpc/include/asm/machdep.h
b/arch/powerpc/include/asm/machdep.h
index 7bcb6a39..a90b892f0bfe 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -131,7 +131,7 @@ struct machdep_calls {
unsigned long dabrx);
/* Set DAWR for this platform, leave empty for default implementation */
- int (*set_dawr)(unsigned long dawr,
+ int (*set_dawr)(int nr, unsigned long dawr,
unsigned long dawrx);
#ifdef CONFIG_PPC32/* XXX for now */
diff --git a/arch/powerpc/include/asm/plpar_wrappers.h
b/arch/powerpc/include/asm/plpar_wrappers.h
index 93eb133d572c..d7a1acc83593 100644
--- a/arch/powerpc/include/asm/plpar_wrappers.h
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
@@ -315,6 +315,11 @@ static inline long plpar_set_watchpoint0(unsigned long
dawr0, unsigned long dawr
return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR0, dawr0, dawrx0);
}
+static inline long plpar_set_watchpoint1(unsigned long dawr1, unsigned long
dawrx1)
+{
+ return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR1, dawr1, dawrx1);
+}
+
static inline long plpar_signal_sys_reset(long cpu)
{
return plpar_hcall_norets(H_SIGNAL_SYS_RESET, cpu);
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index 500f52fa4711..cdc2dccb987d 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -37,7 +37,7 @@ int set_dawr(int nr, struct arch_hw_breakpoint *brk)
dawrx |= (mrd & 0x3f) << (63 - 53);
if (ppc_md.set_dawr)
- return ppc_md.set_dawr(dawr, dawrx);
+ return ppc_md.set_dawr(nr, dawr, dawrx);
if (nr == 0) {
mtspr(SPRN_DAWR0, dawr);
diff --git a/arch/powerpc/platforms/pseries/setup.c
b/arch/powerpc/platforms/pseries/setup.c
index 64d18f4bf093..b001cde1a2d7 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -832,12 +832,15 @@ static int pseries_set_xdabr(unsigned long dabr, unsigned
long dabrx)
return plpar_hcall_norets(H_SET_XDABR, dabr, dabrx);
}
-static int pseries_set_dawr(unsigned long dawr, unsigned long dawrx)
+static int pseries_set_dawr(int nr, unsigned long dawr, unsigned long dawrx)
{
/* PAPR says we can't set HYP */
dawrx &= ~DAWRX_HYP;
- return plpar_set_watchpoint0(dawr, dawrx);
+ if (nr == 0)
+ return plpar_set_watchpoint0(dawr, dawrx);
+ else
+ return plpar_set_watchpoint1(dawr, dawrx);
}
#define CMO_CHARACTERISTICS_TOKEN 44
--
2.26.2
[PATCH v2 3/8] powerpc/dt_cpu_ftrs: Add feature for 2nd DAWR
Add new device-tree feature for 2nd DAWR. If this feature is present,
2nd DAWR is supported, otherwise not.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/asm/cputable.h | 7 +--
arch/powerpc/kernel/dt_cpu_ftrs.c | 7 +++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h
b/arch/powerpc/include/asm/cputable.h
index e506d429b1af..3445c86e1f6f 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -214,6 +214,7 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTR_P9_TLBIE_ERAT_BUG LONG_ASM_CONST(0x0001)
#define CPU_FTR_P9_RADIX_PREFETCH_BUG LONG_ASM_CONST(0x0002)
#define CPU_FTR_ARCH_31
LONG_ASM_CONST(0x0004)
+#define CPU_FTR_DAWR1 LONG_ASM_CONST(0x0008)
#ifndef __ASSEMBLY__
@@ -497,14 +498,16 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTRS_POSSIBLE \
(CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | CPU_FTRS_POWER8 | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_VSX_COMP | CPU_FTRS_POWER9 | \
-CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 |
\
+CPU_FTR_DAWR1)
#else
#define CPU_FTRS_POSSIBLE \
(CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | \
CPU_FTRS_POWER6 | CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | \
CPU_FTRS_POWER8 | CPU_FTRS_CELL | CPU_FTRS_PA6T | \
CPU_FTR_VSX_COMP | CPU_FTR_ALTIVEC_COMP | CPU_FTRS_POWER9 | \
-CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
+CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 |
\
+CPU_FTR_DAWR1)
#endif /* CONFIG_CPU_LITTLE_ENDIAN */
#endif
#else
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 3a409517c031..eafd713ca23d 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -574,6 +574,12 @@ static int __init feat_enable_mma(struct dt_cpu_feature *f)
return 1;
}
+static int __init feat_enable_dawr1(struct dt_cpu_feature *f)
+{
+ cur_cpu_spec->cpu_features |= CPU_FTR_DAWR1;
+ return 1;
+}
+
struct dt_cpu_feature_match {
const char *name;
int (*enable)(struct dt_cpu_feature *f);
@@ -649,6 +655,7 @@ static struct dt_cpu_feature_match __initdata
{"wait-v3", feat_enable, 0},
{"prefix-instructions", feat_enable, 0},
{"matrix-multiply-assist", feat_enable_mma, 0},
+ {"dawr1", feat_enable_dawr1, 0},
};
static bool __initdata using_dt_cpu_ftrs;
--
2.26.2
[PATCH v2 5/8] powerpc/watchpoint: Rename current H_SET_MODE DAWR macro
Current H_SET_MODE hcall macro name for setting/resetting DAWR0 is
H_SET_MODE_RESOURCE_SET_DAWR. Add suffix 0 to macro name as well.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/asm/hvcall.h | 2 +-
arch/powerpc/include/asm/plpar_wrappers.h | 2 +-
arch/powerpc/kvm/book3s_hv.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h
b/arch/powerpc/include/asm/hvcall.h
index e90c073e437e..a7f6f1aeda6b 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -354,7 +354,7 @@
/* Values for 2nd argument to H_SET_MODE */
#define H_SET_MODE_RESOURCE_SET_CIABR 1
-#define H_SET_MODE_RESOURCE_SET_DAWR 2
+#define H_SET_MODE_RESOURCE_SET_DAWR0 2
#define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE3
#define H_SET_MODE_RESOURCE_LE 4
diff --git a/arch/powerpc/include/asm/plpar_wrappers.h
b/arch/powerpc/include/asm/plpar_wrappers.h
index 4497c8afb573..93eb133d572c 100644
--- a/arch/powerpc/include/asm/plpar_wrappers.h
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
@@ -312,7 +312,7 @@ static inline long plpar_set_ciabr(unsigned long ciabr)
static inline long plpar_set_watchpoint0(unsigned long dawr0, unsigned long
dawrx0)
{
- return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR, dawr0, dawrx0);
+ return plpar_set_mode(0, H_SET_MODE_RESOURCE_SET_DAWR0, dawr0, dawrx0);
}
static inline long plpar_signal_sys_reset(long cpu)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index a0cf17597838..26820b7bd75c 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -766,7 +766,7 @@ static int kvmppc_h_set_mode(struct kvm_vcpu *vcpu,
unsigned long mflags,
return H_P3;
vcpu->arch.ciabr = value1;
return H_SUCCESS;
- case H_SET_MODE_RESOURCE_SET_DAWR:
+ case H_SET_MODE_RESOURCE_SET_DAWR0:
if (!kvmppc_power8_compatible(vcpu))
return H_P2;
if (!ppc_breakpoint_available())
--
2.26.2
[PATCH v2 8/8] powerpc/watchpoint: Remove 512 byte boundary
Power10 has removed 512 bytes boundary from match criteria. i.e. The watch
range can cross 512 bytes boundary.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/kernel/hw_breakpoint.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index 031e6defc08e..9a2899f25aae 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -418,8 +418,9 @@ static int hw_breakpoint_validate_len(struct
arch_hw_breakpoint *hw)
if (dawr_enabled()) {
max_len = DAWR_MAX_LEN;
- /* DAWR region can't cross 512 bytes boundary */
- if (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1,
SZ_512))
+ /* DAWR region can't cross 512 bytes boundary on p10
predecessors */
+ if (!cpu_has_feature(CPU_FTR_ARCH_31) &&
+ (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1,
SZ_512)))
return -EINVAL;
} else if (IS_ENABLED(CONFIG_PPC_8xx)) {
/* 8xx can setup a range without limitation */
--
2.26.2
[PATCH v2 1/8] powerpc/watchpoint: Fix 512 byte boundary limit
Milton reported that we are aligning start and end address to wrong
size SZ_512M. It should be SZ_512. Fix that.
While doing this change I also found a case where ALIGN() comparison
fails. Within a given aligned range, ALIGN() of two addresses does not
match when start address is pointing to the first byte and end address
is pointing to any other byte except the first one. But that's not true
for ALIGN_DOWN(). ALIGN_DOWN() of any two addresses within that range
will always point to the first byte. So use ALIGN_DOWN() instead of
ALIGN().
Fixes: e68ef121c1f4 ("powerpc/watchpoint: Use builtin ALIGN*() macros")
Reported-by: Milton Miller
Signed-off-by: Ravi Bangoria
---
arch/powerpc/kernel/hw_breakpoint.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/hw_breakpoint.c
b/arch/powerpc/kernel/hw_breakpoint.c
index daf0e1da..031e6defc08e 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -419,7 +419,7 @@ static int hw_breakpoint_validate_len(struct
arch_hw_breakpoint *hw)
if (dawr_enabled()) {
max_len = DAWR_MAX_LEN;
/* DAWR region can't cross 512 bytes boundary */
- if (ALIGN(start_addr, SZ_512M) != ALIGN(end_addr - 1, SZ_512M))
+ if (ALIGN_DOWN(start_addr, SZ_512) != ALIGN_DOWN(end_addr - 1,
SZ_512))
return -EINVAL;
} else if (IS_ENABLED(CONFIG_PPC_8xx)) {
/* 8xx can setup a range without limitation */
--
2.26.2
[PATCH v2 7/8] powerpc/watchpoint: Return available watchpoints dynamically
So far Book3S Powerpc supported only one watchpoint. Power10 is
introducing 2nd DAWR. Enable 2nd DAWR support for Power10.
Availability of 2nd DAWR will depend on CPU_FTR_DAWR1.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/asm/cputable.h | 4 +++-
arch/powerpc/include/asm/hw_breakpoint.h | 5 +++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h
b/arch/powerpc/include/asm/cputable.h
index 3445c86e1f6f..36a0851a7a9b 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -633,7 +633,9 @@ enum {
* Maximum number of hw breakpoint supported on powerpc. Number of
* breakpoints supported by actual hw might be less than this.
*/
-#define HBP_NUM_MAX1
+#define HBP_NUM_MAX2
+#define HBP_NUM_ONE1
+#define HBP_NUM_TWO2
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/hw_breakpoint.h
b/arch/powerpc/include/asm/hw_breakpoint.h
index cb424799da0d..d4eab1694bcd 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -5,10 +5,11 @@
* Copyright 2010, IBM Corporation.
* Author: K.Prasad
*/
-
#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#include
+
#ifdef __KERNEL__
struct arch_hw_breakpoint {
unsigned long address;
@@ -46,7 +47,7 @@ struct arch_hw_breakpoint {
static inline int nr_wp_slots(void)
{
- return HBP_NUM_MAX;
+ return cpu_has_feature(CPU_FTR_DAWR1) ? HBP_NUM_TWO : HBP_NUM_ONE;
}
#ifdef CONFIG_HAVE_HW_BREAKPOINT
--
2.26.2
[PATCH v2 2/8] powerpc/watchpoint: Enable watchpoint functionality on power10 guest
CPU_FTR_DAWR is by default enabled for host via CPU_FTRS_DT_CPU_BASE
(controlled by CONFIG_PPC_DT_CPU_FTRS). But cpu-features device-tree
node is not PAPR compatible and thus not yet used by kvm or pHyp
guests. Enable watchpoint functionality on power10 guest (both kvm
and powervm) by adding CPU_FTR_DAWR to CPU_FTRS_POWER10. Note that
this change does not enable 2nd DAWR support.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/include/asm/cputable.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/cputable.h
b/arch/powerpc/include/asm/cputable.h
index bac2252c839e..e506d429b1af 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -478,7 +478,7 @@ static inline void cpu_feature_keys_init(void) { }
CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | CPU_FTR_PKEY | \
- CPU_FTR_ARCH_31)
+ CPU_FTR_ARCH_31 | CPU_FTR_DAWR)
#define CPU_FTRS_CELL (CPU_FTR_LWSYNC | \
CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
--
2.26.2
[PATCH v2 4/8] powerpc/watchpoint: Set CPU_FTR_DAWR1 based on pa-features bit
As per the PAPR, bit 0 of byte 64 in pa-features property indicates
availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
DAWR is present, otherwise not. Host generally uses "cpu-features",
which masks "pa-features". But "cpu-features" are still not used for
guests and thus this change is mostly applicable for guests only.
Signed-off-by: Ravi Bangoria
---
arch/powerpc/kernel/prom.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 6a3bac357e24..34272cef8ae6 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -175,6 +175,8 @@ static struct ibm_pa_feature {
*/
{ .pabyte = 22, .pabit = 0, .cpu_features = CPU_FTR_TM_COMP,
.cpu_user_ftrs2 = PPC_FEATURE2_HTM_COMP | PPC_FEATURE2_HTM_NOSC_COMP
},
+
+ { .pabyte = 64, .pabit = 0, .cpu_features = CPU_FTR_DAWR1 },
};
static void __init scan_features(unsigned long node, const unsigned char *ftrs,
--
2.26.2
Re: [PATCH] perf mem/c2c: Fix perf_mem_events to support powerpc
On 1/14/19 9:44 AM, Ravi Bangoria wrote: > Powerpc hw does not have inbuilt latency filter (--ldlat) for mem-load > event and, perf_mem_events by default includes ldlat=30 which is > causing failure on powerpc. Refactor code to support perf mem/c2c on > powerpc. > > This patch depends on kernel side changes done my Madhavan: > https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-December/182596.html > > Signed-off-by: Ravi Bangoria Arnaldo / Michael, Any thoughts? Thanks.
Re: [PATCH] perf mem/c2c: Fix perf_mem_events to support powerpc
On 1/29/19 3:23 PM, Arnaldo Carvalho de Melo wrote: > I think its just a tooling side, I haven't processed it because I'm > waiting for Ravi to address Jiri's comment, after that I'm happy to put > it in my perf/urgent branch that I'm brewing to push to Ingo today or > tomorrow. Ah.. Will try to send v2 today. Thanks.
[PATCH v2] perf mem/c2c: Fix perf_mem_events to support powerpc
Powerpc hw does not have inbuilt latency filter (--ldlat) for mem-load
event and, perf_mem_events by default includes ldlat=30 which is
causing failure on powerpc. Refactor code to support perf mem/c2c on
powerpc.
This patch depends on kernel side changes done my Madhavan:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-December/182596.html
Signed-off-by: Ravi Bangoria
---
tools/perf/Documentation/perf-c2c.txt | 16
tools/perf/Documentation/perf-mem.txt | 2 +-
tools/perf/arch/powerpc/util/Build| 1 +
tools/perf/arch/powerpc/util/mem-events.c | 11 +++
tools/perf/util/mem-events.c | 2 +-
5 files changed, 26 insertions(+), 6 deletions(-)
create mode 100644 tools/perf/arch/powerpc/util/mem-events.c
diff --git a/tools/perf/Documentation/perf-c2c.txt
b/tools/perf/Documentation/perf-c2c.txt
index 095aebd..e6150f2 100644
--- a/tools/perf/Documentation/perf-c2c.txt
+++ b/tools/perf/Documentation/perf-c2c.txt
@@ -19,8 +19,11 @@ C2C stands for Cache To Cache.
The perf c2c tool provides means for Shared Data C2C/HITM analysis. It allows
you to track down the cacheline contentions.
-The tool is based on x86's load latency and precise store facility events
-provided by Intel CPUs. These events provide:
+On x86, the tool is based on load latency and precise store facility events
+provided by Intel CPUs. On PowerPC, the tool uses random instruction sampling
+with thresholding feature.
+
+These events provide:
- memory address of the access
- type of the access (load and store details)
- latency (in cycles) of the load access
@@ -46,7 +49,7 @@ RECORD OPTIONS
-l::
--ldlat::
- Configure mem-loads latency.
+ Configure mem-loads latency. (x86 only)
-k::
--all-kernel::
@@ -119,11 +122,16 @@ Following perf record options are configured by default:
-W,-d,--phys-data,--sample-cpu
Unless specified otherwise with '-e' option, following events are monitored by
-default:
+default on x86:
cpu/mem-loads,ldlat=30/P
cpu/mem-stores/P
+and following on PowerPC:
+
+ cpu/mem-loads/
+ cpu/mem-stores/
+
User can pass any 'perf record' option behind '--' mark, like (to enable
callchains and system wide monitoring):
diff --git a/tools/perf/Documentation/perf-mem.txt
b/tools/perf/Documentation/perf-mem.txt
index f8d2167..199ea0f 100644
--- a/tools/perf/Documentation/perf-mem.txt
+++ b/tools/perf/Documentation/perf-mem.txt
@@ -82,7 +82,7 @@ RECORD OPTIONS
Be more verbose (show counter open errors, etc)
--ldlat ::
- Specify desired latency for loads event.
+ Specify desired latency for loads event. (x86 only)
In addition, for report all perf report options are valid, and for record
all perf record options.
diff --git a/tools/perf/arch/powerpc/util/Build
b/tools/perf/arch/powerpc/util/Build
index 2e659531..ba98bd0 100644
--- a/tools/perf/arch/powerpc/util/Build
+++ b/tools/perf/arch/powerpc/util/Build
@@ -2,6 +2,7 @@ libperf-y += header.o
libperf-y += sym-handling.o
libperf-y += kvm-stat.o
libperf-y += perf_regs.o
+libperf-y += mem-events.o
libperf-$(CONFIG_DWARF) += dwarf-regs.o
libperf-$(CONFIG_DWARF) += skip-callchain-idx.o
diff --git a/tools/perf/arch/powerpc/util/mem-events.c
b/tools/perf/arch/powerpc/util/mem-events.c
new file mode 100644
index 000..f1194fc
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/mem-events.c
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "mem-events.h"
+
+/* PowerPC does not support 'ldlat' parameter. */
+char *perf_mem_events__name(int i)
+{
+ if (i == PERF_MEM_EVENTS__LOAD)
+ return (char *) "cpu/mem-loads/";
+
+ return (char *) "cpu/mem-stores/";
+}
diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
index 93f74d8..42c3e5a 100644
--- a/tools/perf/util/mem-events.c
+++ b/tools/perf/util/mem-events.c
@@ -28,7 +28,7 @@ struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] =
{
static char mem_loads_name[100];
static bool mem_loads_name__init;
-char *perf_mem_events__name(int i)
+char * __weak perf_mem_events__name(int i)
{
if (i == PERF_MEM_EVENTS__LOAD) {
if (!mem_loads_name__init) {
--
1.8.3.1
[PATCH] perf mem/c2c: Fix perf_mem_events to support powerpc
Powerpc hw does not have inbuilt latency filter (--ldlat) for mem-load
event and, perf_mem_events by default includes ldlat=30 which is
causing failure on powerpc. Refactor code to support perf mem/c2c on
powerpc.
This patch depends on kernel side changes done my Madhavan:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-December/182596.html
Signed-off-by: Ravi Bangoria
---
tools/perf/Documentation/perf-c2c.txt | 16
tools/perf/Documentation/perf-mem.txt | 2 +-
tools/perf/arch/x86/util/Build| 1 +
tools/perf/arch/x86/util/mem-events.c | 25 +
tools/perf/util/mem-events.c | 26 --
tools/perf/util/mem-events.h | 2 ++
6 files changed, 45 insertions(+), 27 deletions(-)
create mode 100644 tools/perf/arch/x86/util/mem-events.c
diff --git a/tools/perf/Documentation/perf-c2c.txt
b/tools/perf/Documentation/perf-c2c.txt
index 095aebd..4e12551 100644
--- a/tools/perf/Documentation/perf-c2c.txt
+++ b/tools/perf/Documentation/perf-c2c.txt
@@ -19,8 +19,11 @@ C2C stands for Cache To Cache.
The perf c2c tool provides means for Shared Data C2C/HITM analysis. It allows
you to track down the cacheline contentions.
-The tool is based on x86's load latency and precise store facility events
-provided by Intel CPUs. These events provide:
+On x86, the tool is based on load latency and precise store facility events
+provided by Intel CPUs. On PowerPC, the tool uses random instruction sampling
+with thresholding feature.
+
+These events provide:
- memory address of the access
- type of the access (load and store details)
- latency (in cycles) of the load access
@@ -46,7 +49,7 @@ RECORD OPTIONS
-l::
--ldlat::
- Configure mem-loads latency.
+ Configure mem-loads latency. (x86 only)
-k::
--all-kernel::
@@ -119,11 +122,16 @@ Following perf record options are configured by default:
-W,-d,--phys-data,--sample-cpu
Unless specified otherwise with '-e' option, following events are monitored by
-default:
+default on x86:
cpu/mem-loads,ldlat=30/P
cpu/mem-stores/P
+and following on PowerPC:
+
+ cpu/mem-loads/P
+ cpu/mem-stores/P
+
User can pass any 'perf record' option behind '--' mark, like (to enable
callchains and system wide monitoring):
diff --git a/tools/perf/Documentation/perf-mem.txt
b/tools/perf/Documentation/perf-mem.txt
index f8d2167..199ea0f 100644
--- a/tools/perf/Documentation/perf-mem.txt
+++ b/tools/perf/Documentation/perf-mem.txt
@@ -82,7 +82,7 @@ RECORD OPTIONS
Be more verbose (show counter open errors, etc)
--ldlat ::
- Specify desired latency for loads event.
+ Specify desired latency for loads event. (x86 only)
In addition, for report all perf report options are valid, and for record
all perf record options.
diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
index 844b8f3..9827240 100644
--- a/tools/perf/arch/x86/util/Build
+++ b/tools/perf/arch/x86/util/Build
@@ -6,6 +6,7 @@ libperf-y += perf_regs.o
libperf-y += group.o
libperf-y += machine.o
libperf-y += event.o
+libperf-y += mem-events.o
libperf-$(CONFIG_DWARF) += dwarf-regs.o
libperf-$(CONFIG_BPF_PROLOGUE) += dwarf-regs.o
diff --git a/tools/perf/arch/x86/util/mem-events.c
b/tools/perf/arch/x86/util/mem-events.c
new file mode 100644
index 000..5b4dcfe
--- /dev/null
+++ b/tools/perf/arch/x86/util/mem-events.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "mem-events.h"
+
+struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
+ PERF_MEM_EVENT("ldlat-loads", "cpu/mem-loads,ldlat=%u/P", "mem-loads"),
+ PERF_MEM_EVENT("ldlat-stores", "cpu/mem-stores/P", "mem-stores"),
+};
+
+static char mem_loads_name[100];
+static bool mem_loads_name__init;
+
+char *perf_mem_events__name(int i)
+{
+ if (i == PERF_MEM_EVENTS__LOAD) {
+ if (!mem_loads_name__init) {
+ mem_loads_name__init = true;
+ scnprintf(mem_loads_name, sizeof(mem_loads_name),
+ perf_mem_events[i].name,
+ perf_mem_events__loads_ldlat);
+ }
+ return mem_loads_name;
+ }
+
+ return (char *)perf_mem_events[i].name;
+}
diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
index 93f74d8..1ffefd3 100644
--- a/tools/perf/util/mem-events.c
+++ b/tools/perf/util/mem-events.c
@@ -15,31 +15,13 @@
unsigned int perf_mem_events__loads_ldlat = 30;
-#define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s }
-
-struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
- E("ldlat-loads","cpu/mem-loads,ldlat=%u/P", "mem-loads"),
- E("ldlat-stores", "cpu/mem-stores/P", "mem-stores"),
+struct p
[PATCH] Uprobes: Fix deadlock between delayed_uprobe_lock and fs_reclaim
There can be a deadlock between delayed_uprobe_lock and
fs_reclaim like:
CPU0 CPU1
lock(fs_reclaim);
lock(delayed_uprobe_lock);
lock(fs_reclaim);
lock(delayed_uprobe_lock);
Here CPU0 is a file system code path which results in
mmput()->__mmput()->uprobe_clear_state() with fs_reclaim
locked. And, CPU1 is a uprobe event creation path.
This was reported by syzbot at [1]. Though, the reproducer
is nither available by syzbot nor I can reproduce it locally.
Callchains from syzbot report:
-> #1 (fs_reclaim){+.+.}:
__fs_reclaim_acquire mm/page_alloc.c:3730 [inline]
fs_reclaim_acquire.part.97+0x24/0x30 mm/page_alloc.c:3741
fs_reclaim_acquire+0x14/0x20 mm/page_alloc.c:3742
slab_pre_alloc_hook mm/slab.h:418 [inline]
slab_alloc mm/slab.c:3378 [inline]
kmem_cache_alloc_trace+0x2d/0x750 mm/slab.c:3618
kmalloc include/linux/slab.h:546 [inline]
kzalloc include/linux/slab.h:741 [inline]
delayed_uprobe_add kernel/events/uprobes.c:313 [inline]
update_ref_ctr+0x36f/0x590 kernel/events/uprobes.c:447
uprobe_write_opcode+0x94b/0xc50 kernel/events/uprobes.c:496
set_swbp+0x2a/0x40
install_breakpoint.isra.24+0x161/0x840 kernel/events/uprobes.c:885
register_for_each_vma+0xa38/0xee0 kernel/events/uprobes.c:1041
uprobe_apply+0xee/0x140 kernel/events/uprobes.c:1192
uprobe_perf_open kernel/trace/trace_uprobe.c:1087 [inline]
trace_uprobe_register+0x771/0xcf0 kernel/trace/trace_uprobe.c:1227
perf_trace_event_open kernel/trace/trace_event_perf.c:181 [inline]
perf_trace_event_init+0x1a5/0x990 kernel/trace/trace_event_perf.c:203
perf_uprobe_init+0x1f1/0x280 kernel/trace/trace_event_perf.c:329
perf_uprobe_event_init+0x106/0x1a0 kernel/events/core.c:8503
perf_try_init_event+0x137/0x2f0 kernel/events/core.c:9770
perf_init_event kernel/events/core.c:9801 [inline]
perf_event_alloc.part.94+0x1d54/0x3740 kernel/events/core.c:10074
perf_event_alloc kernel/events/core.c:10430 [inline]
__do_sys_perf_event_open+0xada/0x3020 kernel/events/core.c:10531
__se_sys_perf_event_open kernel/events/core.c:10420 [inline]
__x64_sys_perf_event_open+0xbe/0x150 kernel/events/core.c:10420
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
-> #0 (delayed_uprobe_lock){+.+.}:
lock_acquire+0x1ed/0x520 kernel/locking/lockdep.c:3844
__mutex_lock_common kernel/locking/mutex.c:925 [inline]
__mutex_lock+0x166/0x16f0 kernel/locking/mutex.c:1072
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
uprobe_clear_state+0xb4/0x390 kernel/events/uprobes.c:1511
__mmput kernel/fork.c:1041 [inline]
mmput+0x1bc/0x610 kernel/fork.c:1066
binder_alloc_free_page+0x5ab/0x1520 drivers/android/binder_alloc.c:983
__list_lru_walk_one+0x29d/0x8c0 mm/list_lru.c:234
list_lru_walk_one+0xa5/0xe0 mm/list_lru.c:278
list_lru_walk_node+0x43/0x280 mm/list_lru.c:307
list_lru_walk include/linux/list_lru.h:214 [inline]
binder_shrink_scan+0x164/0x220 drivers/android/binder_alloc.c:1019
do_shrink_slab+0x501/0xd30 mm/vmscan.c:557
shrink_slab+0x389/0x8c0 mm/vmscan.c:706
shrink_node+0x431/0x16b0 mm/vmscan.c:2758
shrink_zones mm/vmscan.c:2987 [inline]
do_try_to_free_pages+0x3e7/0x1290 mm/vmscan.c:3049
try_to_free_pages+0x4d0/0xb90 mm/vmscan.c:3264
__perform_reclaim mm/page_alloc.c:3773 [inline]
__alloc_pages_direct_reclaim mm/page_alloc.c:3795 [inline]
__alloc_pages_slowpath+0xa48/0x2de0 mm/page_alloc.c:4185
__alloc_pages_nodemask+0xad8/0xea0 mm/page_alloc.c:4393
__alloc_pages include/linux/gfp.h:473 [inline]
__alloc_pages_node include/linux/gfp.h:486 [inline]
khugepaged_alloc_page+0x95/0x190 mm/khugepaged.c:773
collapse_huge_page mm/khugepaged.c:963 [inline]
khugepaged_scan_pmd+0x1715/0x3d60 mm/khugepaged.c:1216
khugepaged_scan_mm_slot mm/khugepaged.c:1725 [inline]
khugepaged_do_scan mm/khugepaged.c:1806 [inline]
khugepaged+0xf20/0x1750 mm/khugepaged.c:1851
kthread+0x35a/0x440 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
[1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1876397.html
Reported-by: syzbot+1068f09c44d151250...@syzkaller.appspotmail.com
Suggested-by: Aneesh Kumar K.V
Signed-off-by: Ravi Bangoria
---
kernel/events/uprobes.c | 24
1 file changed, 24 insertions(+)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 8aef47ee7bfa..8be39a83d83a 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -95,6 +95,11 @@ struct delayed_uprobe {
struct mm_struct *mm;
};
+/*
+ * Any memory allocation happening within lock(delayed_uprobe_lock)
+ * must use memalloc_nofs_save()/memalloc_
Re: [PATCH v2] perf scripts python: Add Python 3 support to check-perf-trace.py
Hi Seeteena, Jiri,
On 1/17/19 6:02 PM, Jiri Olsa wrote:
> On Thu, Jan 17, 2019 at 03:15:28PM +0530, Seeteena Thoufeek wrote:
>> Support both Python 2 and Python 3 in check-perf-trace.py.
>> ``print`` is now a function rather than a statement. This should have
>> no functional change.
>>
>> Fix indentation issue, replace spaces with tab
>>
>> Signed-off-by: Seeteena Thoufeek
>> Reviewed-by: Ravi Bangoria
>
> hum, could you please add some info about testing those changes?
> (or even some global into 0/.. patch)
>
> this is working for me on python2:
>
> [root@krava perf]# perf script rec check-perf-trace
> ^C
> [root@krava perf]# perf script -s scripts/python/check-perf-trace.py
> trace_begin
>
> unhandled events:
>
>
> event count
>
> ---
>
> raw_syscalls__sys_enter 3509879
>
>
> but fails for python3:
>
> [root@ibm-x3650m4-01-vm-04 perf]# perf script rec check-perf-trace
> ^C[ perf record: Woken up 0 times to write data ]
> Warning:
> 1 out of order events recorded.
> [ perf record: Captured and wrote 43.132 MB perf.data (490171 samples) ]
>
> [root@ibm-x3650m4-01-vm-04 perf]# perf script -s
> scripts/python/check-perf-trace.py
> Traceback (most recent call last):
> File "scripts/python/check-perf-trace.py", line 18, in
> from perf_trace_context import *
> ModuleNotFoundError: No module named 'perf_trace_context'
> Error running python script scripts/python/check-perf-trace.py
Yes I'm seeing the same error with check-perf-trace.py.
I think the issue happens only when we use 'perf script -s path/to/script.py'
instead of 'perf script report script_name':
Ex:
# PERF_EXEC_PATH=`pwd` ./perf.python3 script record stackcollapse
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.263 MB perf.data (497 samples) ]
# PERF_EXEC_PATH=`pwd` ./perf.python3 script report stackcollapse
migration/21 1
migration/41 1
perf 9
rngd 1
sshd 1
swapper 482
xfsaild/sda5 2
# PERF_EXEC_PATH=`pwd` ./perf.python3 script -s
scripts/python/stackcollapse.py
Traceback (most recent call last):
File "scripts/python/stackcollapse.py", line 31, in
from perf_trace_context import *
ModuleNotFoundError: No module named 'perf_trace_context'
Error running python script scripts/python/stackcollapse.py
So, it seems, this is not the script issue. Something is wrong when we build
perf
with python3?
Here is my test summary on Powerpc Fedora 29:
Python2 Python3
check-perf-trace.py okFAIL (No module named
'perf_trace_context')
event_analyzing_sample.pyokok
export-to-sqlite.py okok
exported-sql-viewer.py okFAIL (details at the end)
export-to-postgresql.py FAIL FAIL
failed-syscalls-by-pid.pyokok
futex-contention.py okok
intel-pt-events.py NOT TESTEDNOT TESTED
mem-phys-addr.py NOT TESTEDNOT TESTED
net_dropmonitor.py okok
netdev-times.py okok
powerpc-hcalls.pyokok
sctop.py okok
stackcollapse.py okok
stat-cpi.py FAIL (Seg fault) FAIL (Seg fault)
syscall-counts-by-pid.py okok
syscall-counts.pyokok
exported-sql-viewer.py FAILURE:
===
# PERF_EXEC_PATH=`pwd` ./perf.python3 script -s
./scripts/python/exported-sql-viewer.py my_test_db
Traceback (most recent call last):
File "./scripts/python/exported-sql-viewer.py", line 2614, in
Main()
File "./scripts/python/exported-sql-viewer.py", line 2601, in Main
db, dbname = dbref.Open("main")
File "./scripts/python/exported-sql-viewer.py", line 2573, in Open
raise Exception("Failed to open database " + dbname + " error: " +
db.lastError().text())
Exception: Failed to open database my_test_db error: could not connect to
server: No such file or directory
Is the server running locally and accepting
connections on Unix domain socket "/var/run/postgresql/.s.PGSQL.5432"?
QPSQL: Unable to connect
Error running python script ./scripts/python/exported-sql-viewer.py
Thanks,
Ravi
Re: System crash with perf_fuzzer (kernel: 5.0.0-rc3)
Hi Andi, On 1/25/19 9:30 PM, Andi Kleen wrote: >> [Fri Jan 25 10:28:53 2019] perf: interrupt took too long (2501 > 2500), >> lowering kernel.perf_event_max_sample_rate to 79750 >> [Fri Jan 25 10:29:08 2019] perf: interrupt took too long (3136 > 3126), >> lowering kernel.perf_event_max_sample_rate to 63750 >> [Fri Jan 25 10:29:11 2019] perf: interrupt took too long (4140 > 3920), >> lowering kernel.perf_event_max_sample_rate to 48250 >> [Fri Jan 25 10:29:11 2019] perf: interrupt took too long (5231 > 5175), >> lowering kernel.perf_event_max_sample_rate to 38000 >> [Fri Jan 25 10:29:11 2019] perf: interrupt took too long (6736 > 6538), >> lowering kernel.perf_event_max_sample_rate to 29500 > > These are fairly normal. I understand that throttling mechanism is designed exactly to do this. But I've observed that, everytime I run the fuzzer, max_sample_rates is been throttled down to 250 (which is CONFIG_HZ I guess). Doesn't this mean the interrupt time is somehow increasing gradually? Is that fine? Here is the sample dmesg: [Thu Jan 31 09:25:40 2019] perf: interrupt took too long (2928 > 2500), lowering kernel.perf_event_max_sample_rate to 68250 [Thu Jan 31 09:25:40 2019] perf: interrupt took too long (4363 > 3660), lowering kernel.perf_event_max_sample_rate to 45750 [Thu Jan 31 09:25:40 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 2.183 msecs [Thu Jan 31 09:25:40 2019] perf: interrupt took too long (21382 > 5453), lowering kernel.perf_event_max_sample_rate to 9250 [Thu Jan 31 09:25:40 2019] perf: interrupt took too long (34548 > 26727), lowering kernel.perf_event_max_sample_rate to 5750 [Thu Jan 31 09:25:40 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 3.509 msecs [Thu Jan 31 09:25:40 2019] perf: interrupt took too long (61682 > 43185), lowering kernel.perf_event_max_sample_rate to 3000 [Thu Jan 31 09:25:40 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 3.593 msecs [Thu Jan 31 09:25:40 2019] perf: interrupt took too long (89206 > 77102), lowering kernel.perf_event_max_sample_rate to 2000 [Thu Jan 31 09:25:40 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 3.619 msecs [Thu Jan 31 09:25:40 2019] perf: interrupt took too long (120188 > 111507), lowering kernel.perf_event_max_sample_rate to 1500 [Thu Jan 31 09:25:40 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 3.782 msecs [Thu Jan 31 09:25:40 2019] perf: interrupt took too long (171065 > 150235), lowering kernel.perf_event_max_sample_rate to 1000 [Thu Jan 31 09:25:40 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 4.066 msecs [Thu Jan 31 09:25:40 2019] perf: interrupt took too long (226815 > 213831), lowering kernel.perf_event_max_sample_rate to 750 [Thu Jan 31 09:25:40 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 5.364 msecs [Thu Jan 31 09:25:40 2019] perf: interrupt took too long (300844 > 283518), lowering kernel.perf_event_max_sample_rate to 500 [Thu Jan 31 09:33:43 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 6.136 msecs [Thu Jan 31 09:50:35 2019] perf: interrupt took too long (378352 > 376055), lowering kernel.perf_event_max_sample_rate to 500 [Thu Jan 31 09:53:47 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 6.456 msecs [Thu Jan 31 09:57:31 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 11.002 msecs [Thu Jan 31 10:01:30 2019] perf: interrupt took too long (478447 > 472940), lowering kernel.perf_event_max_sample_rate to 250 [Thu Jan 31 12:28:31 2019] perf: interrupt took too long (601630 > 598058), lowering kernel.perf_event_max_sample_rate to 250 [Thu Jan 31 12:28:31 2019] perf: interrupt took too long (754288 > 752037), lowering kernel.perf_event_max_sample_rate to 250 [Thu Jan 31 12:43:13 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 12.781 msecs [Thu Jan 31 12:43:13 2019] INFO: NMI handler (perf_event_nmi_handler) took too long to run: 13.583 msecs Thanks, Ravi
Re: [PATCH] Uprobes: Fix deadlock between delayed_uprobe_lock and fs_reclaim
On 2/6/19 7:06 PM, Oleg Nesterov wrote: > Ravi, I am on vacation till the end of this week, can't read your patch > carefully. > > I am not sure I fully understand the problem, but shouldn't we change > binder_alloc_free_page() to use mmput_async() ? Like it does if trylock > fails. I don't understand binderfs code much so I'll let Sherry comment on this. > > In any case, I don't think memalloc_nofs_save() is what we need, see below. > > On 02/04, Ravi Bangoria wrote: >> >> There can be a deadlock between delayed_uprobe_lock and >> fs_reclaim like: >> >>CPU0 CPU1 >> >>lock(fs_reclaim); >> lock(delayed_uprobe_lock); >> lock(fs_reclaim); >>lock(delayed_uprobe_lock); >> >> Here CPU0 is a file system code path which results in >> mmput()->__mmput()->uprobe_clear_state() with fs_reclaim >> locked. And, CPU1 is a uprobe event creation path. > > But this is false positive, right? if CPU1 calls update_ref_ctr() then > either ->mm_users is already zero so > binder_alloc_free_page()->mmget_not_zero() > will fail, or the caller of update_ref_ctr() has a reference and thus > binder_alloc_free_page()->mmput() can't trigger __mmput() ? Yes, it seems so. So, IIUC, even though the locking sequence are actually opposite, *actual* instances of the locks will never be able to lock simultaneously on both the code path as warned by lockdep. Please correct me if I misunderstood. [...] >> +nofs_flags = memalloc_nofs_save(); >> mutex_lock(_uprobe_lock); >> if (d > 0) >> ret = delayed_uprobe_add(uprobe, mm); >> else >> delayed_uprobe_remove(uprobe, mm); >> mutex_unlock(_uprobe_lock); >> +memalloc_nofs_restore(nofs_flags); > > PF_MEMALLOC_NOFS is only needed when we are going to call delayed_uprobe_add() > which does kzalloc(GFP_KERNEL). Can't we simply change it tuse use use > GFP_NOFS > instead? Yes, I can use GFP_NOFS. (and same was suggested by Aneesh as well) But from https://lwn.net/Articles/710545/, I found that community is planning to deprecate the GFP_NOFS flag? -Ravi
Re: [PATCH v5 06/10] Uprobes: Support SDT markers having reference count (semaphore)
Hi Oleg,
On 07/10/2018 08:55 PM, Oleg Nesterov wrote:
> Hi Ravi,
>
> On 07/04, Ravi Bangoria wrote:
>>
>>> Now I understand what did you mean by "for each consumer". So if we move
>>> this logic
>>> into install/remove_breakpoint as I tried to suggest, we will also need
>>> another error
>>> code for the case when verify_opcode() returns false.
>>
>> Ok so if we can use verify_opcode() inside install_breakpoint(), we can
>> probably
>> move implementation logic in install/remove_breakpoint(). Let me explore
>> that more.
>
> No, sorry for confusion, I meant another thing... But please forget. If we
> rely on
> verify_opcode() I no longer think it would be more clean to move this logic
> into
> install/remove_breakpoint.
>
> However, I still think it would be better to avoid uprobe exporting and
> modifying
> set_swbp/set_orig_insn. May be we can simply kill both set_swbp() and
> set_orig_insn(),
> I'll re-check...
Good that you bring this up. Actually, we can implement same logic
without exporting uprobe. We can do "uprobe = container_of(arch_uprobe)"
in uprobe_write_opcode(). No need to export struct uprobe outside,
no need to change set_swbp() / set_orig_insn() syntax. Just that we
need to pass arch_uprobe object to uprobe_write_opcode().
But, I wanted to discuss about making ref_ctr_offset a uprobe property
or a consumer property, before posting v6:
If we make it a consumer property, the design becomes flexible for
user. User will have an option to either depend on kernel to handle
reference counter or he can create normal uprobe and manipulate
reference counter on his own. This will not require any changes to
existing tools. With this approach we need to increment / decrement
reference counter for each consumer. But, because of the fact that our
install_breakpoint() / remove_breakpoint() are not balanced, we have
to keep track of which reference counter have been updated in which
mm, for which uprobe and for which consumer. I.e. Maintain a list of
{uprobe, consumer, mm}. This will make kernel implementation quite
complex because there are chances of races. What we gain in return
is flexibility for users. Please let me know if there are any other
advantages of this approach.
By making it a uprobe property, we are forcing user to use
uprobe_register_refctr() for uprobes having reference counter. Because
we don't allow same uprobe with multiple reference counter and thus
user can't use both uprobe_register_refctr() and uprobe_register() in
parallel for same uprobe. Which means user does not have a flexibility
to create normal uprobe with uprobe_register() and maintain reference
counter on his own. But kernel implementation becomes simple with this
approach. Though, this will require changes in tools which already
supports SDT events, systemtap and bcc I'm aware of. But AFAICS, the
change should not be major. In fact, the change should make tool code
simpler because they don't have to worry about maintaining reference
counter.
Third options: How about allowing 0 as a special value for reference
counter? I mean allow uprobe_register() and uprobe_register_refctr()
in parallel but do not allow two uprobe_register_refctr() with two
different reference counter. If we can do this, the issue of forcing
legacy user to use uprobe_register_refctr() will be solved. I.e. no
change needed on tools side and still kernel implementation will
remain simple. I'll explore this option.
Let me know your thoughts.
Thanks,
Ravi
PS: We can't abuse MSB with first approach because any userspace tool
can also abuse MSB in parallel. Probably, we can abuse MSB in second
and third approach, though, there is no need to.
[PATCH 1/3] perf script: Add missing output fields in a hint
Few fields are missing in a perf script -F hint. Add them.
Signed-off-by: Ravi Bangoria
---
tools/perf/builtin-script.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index b3bf35512d21..4d1cee68cfd2 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3113,8 +3113,9 @@ int cmd_script(int argc, const char **argv)
"+field to add and -field to remove."
"Valid types: hw,sw,trace,raw,synth. "
"Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,"
-"addr,symoff,period,iregs,uregs,brstack,brstacksym,flags,"
-
"bpf-output,callindent,insn,insnlen,brstackinsn,synth,phys_addr",
+"addr,symoff,srcline,period,iregs,uregs,brstack,"
+"brstacksym,flags,bpf-output,brstackinsn,brstackoff,"
+"callindent,insn,insnlen,synth,phys_addr,metric,misc",
parse_output_fields),
OPT_BOOLEAN('a', "all-cpus", _wide,
"system-wide collection from all CPUs"),
--
2.14.4
[PATCH 0/3] perf script: Few trivial fixes
First patch fixes perf output field hint. Second and third fixes crash when used in a piped mode. Ravi Bangoria (3): perf script: Add missing output fields in a hint perf script: Fix crash because of missing evsel->priv perf script: Fix crash because of missing feat_op[] entry tools/perf/builtin-script.c | 19 +-- tools/perf/util/header.c| 7 ++- 2 files changed, 23 insertions(+), 3 deletions(-) -- 2.14.4
[PATCH 3/3] perf script: Fix crash because of missing feat_op[] entry
perf_event__process_feature() tries to access feat_ops[feat].process
which is not defined for feat = HEADER_LAST_FEATURE and thus perf is
crashing. Add dummy entry for HEADER_LAST_FEATURE in the feat_ops.
Before:
# ./perf record -o - ls | ./perf script
Segmentation fault (core dumped)
After:
# ./perf record -o - ls | ./perf script
ls 7031 4392.099856: 25 cpu-clock:uhH: 7f5e0ce7cd60
ls 7031 4392.100355: 25 cpu-clock:uhH: 7f5e0c706ef7
Signed-off-by: Ravi Bangoria
Fixes: 57b5de463925 ("perf report: Support forced leader feature in pipe mode")
---
tools/perf/util/header.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 540cd2dcd3e7..de8e3e29d870 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2555,14 +2555,18 @@ struct feature_ops {
}
/* feature_ops not implemented: */
+#define write_last_feature NULL
+
#define print_tracing_data NULL
#define print_build_id NULL
+#define print_last_feature NULL
#define process_branch_stack NULL
#define process_stat NULL
+#define process_last_feature NULL
-static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
+static const struct feature_ops feat_ops[HEADER_LAST_FEATURE + 1] = {
FEAT_OPN(TRACING_DATA, tracing_data, false),
FEAT_OPN(BUILD_ID, build_id, false),
FEAT_OPR(HOSTNAME, hostname, false),
@@ -2585,6 +2589,7 @@ static const struct feature_ops
feat_ops[HEADER_LAST_FEATURE] = {
FEAT_OPN(CACHE, cache, true),
FEAT_OPR(SAMPLE_TIME, sample_time,false),
FEAT_OPR(MEM_TOPOLOGY, mem_topology, true),
+ FEAT_OPN(LAST_FEATURE, last_feature, false),
};
struct header_print_data {
--
2.14.4
[PATCH 2/3] perf script: Fix crash because of missing evsel->priv
perf script in pipped mode is crashing because evsel->priv is not
set properly. Fix it.
Before:
# ./perf record -o - -- ls | ./perf script
Segmentation fault (core dumped)
After:
# ./perf record -o - -- ls | ./perf script
ls 2282 1031.731974: 25 cpu-clock:uhH: 7effe4b3d29e
ls 2282 1031.73: 25 cpu-clock:uhH: 7effe4b3a650
Signed-off-by: Ravi Bangoria
Fixes: a14390fde64e ("perf script: Allow creating per-event dump files")
---
tools/perf/builtin-script.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 4d1cee68cfd2..acee05562f5e 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1822,6 +1822,7 @@ static int process_attr(struct perf_tool *tool, union
perf_event *event,
struct perf_evlist *evlist;
struct perf_evsel *evsel, *pos;
int err;
+ static struct perf_evsel_script *es;
err = perf_event__process_attr(tool, event, pevlist);
if (err)
@@ -1830,6 +1831,19 @@ static int process_attr(struct perf_tool *tool, union
perf_event *event,
evlist = *pevlist;
evsel = perf_evlist__last(*pevlist);
+ if (!evsel->priv) {
+ if (scr->per_event_dump) {
+ evsel->priv = perf_evsel_script__new(evsel,
+ scr->session->data);
+ } else {
+ es = zalloc(sizeof(*es));
+ if (!es)
+ return -ENOMEM;
+ es->fp = stdout;
+ evsel->priv = es;
+ }
+ }
+
if (evsel->attr.type >= PERF_TYPE_MAX &&
evsel->attr.type != PERF_TYPE_SYNTH)
return 0;
--
2.14.4
Re: [PATCH 2/3] perf script: Fix crash because of missing evsel->priv
Hi Arnaldo,
On 06/20/2018 07:22 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jun 20, 2018 at 07:00:29PM +0530, Ravi Bangoria escreveu:
>> perf script in pipped mode is crashing because evsel->priv is not
>> set properly. Fix it.
>>
>> Before:
>> # ./perf record -o - -- ls | ./perf script
>> Segmentation fault (core dumped)
>>
>> After:
>> # ./perf record -o - -- ls | ./perf script
>> ls 2282 1031.731974: 25 cpu-clock:uhH: 7effe4b3d29e
>> ls 2282 1031.73: 25 cpu-clock:uhH: 7effe4b3a650
>>
>> Signed-off-by: Ravi Bangoria
>> Fixes: a14390fde64e ("perf script: Allow creating per-event dump files")
>
> Humm, this cset doesn't set evsel->priv to a 'struct perf_evsel_script'
> object, will check which one does to continue review.
Possible. Actually, git bisect led me to this commit.
Ravi
