* [PATCH v6 0/7] Powerpc/Watchpoint: Few important fixes
@ 2019-10-17 9:31 ` Ravi Bangoria
0 siblings, 0 replies; 23+ messages in thread
From: Ravi Bangoria @ 2019-10-17 9:31 UTC (permalink / raw)
To: christophe.leroy, mpe, mikey
Cc: npiggin, benh, paulus, naveen.n.rao, linuxppc-dev, linux-kernel,
Ravi Bangoria
v5: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198069.html
v5->v6:
- patch 6/7: mpe reported that the perf-hwbreak.c doesn't compile with older
gcc:
perf-hwbreak.c:182:2: error: dereferencing type-punned pointer will
break strict-aliasing rules [-Werror=strict-aliasing]
temp16 = *((__u16 *)target);
^
Fixed that.
Ravi Bangoria (7):
Powerpc/Watchpoint: Introduce macros for watchpoint length
Powerpc/Watchpoint: Fix length calculation for unaligned target
Powerpc/Watchpoint: Fix ptrace code that muck around with address/len
Powerpc/Watchpoint: Don't ignore extraneous exceptions blindly
Powerpc/Watchpoint: Rewrite ptrace-hwbreak.c selftest
Powerpc/Watchpoint: Add dar outside test in perf-hwbreak.c selftest
Powerpc/Watchpoint: Support for 8xx in ptrace-hwbreak.c selftest
arch/powerpc/include/asm/hw_breakpoint.h | 9 +-
arch/powerpc/kernel/dawr.c | 6 +-
arch/powerpc/kernel/hw_breakpoint.c | 119 ++--
arch/powerpc/kernel/process.c | 3 +
arch/powerpc/kernel/ptrace.c | 16 +-
arch/powerpc/xmon/xmon.c | 2 +-
.../selftests/powerpc/ptrace/perf-hwbreak.c | 119 +++-
.../selftests/powerpc/ptrace/ptrace-hwbreak.c | 581 +++++++++++-------
8 files changed, 590 insertions(+), 265 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH v6 0/7] Powerpc/Watchpoint: Few important fixes @ 2019-10-17 9:31 ` Ravi Bangoria 0 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:31 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: Ravi Bangoria, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev v5: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198069.html v5->v6: - patch 6/7: mpe reported that the perf-hwbreak.c doesn't compile with older gcc: perf-hwbreak.c:182:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing] temp16 = *((__u16 *)target); ^ Fixed that. Ravi Bangoria (7): Powerpc/Watchpoint: Introduce macros for watchpoint length Powerpc/Watchpoint: Fix length calculation for unaligned target Powerpc/Watchpoint: Fix ptrace code that muck around with address/len Powerpc/Watchpoint: Don't ignore extraneous exceptions blindly Powerpc/Watchpoint: Rewrite ptrace-hwbreak.c selftest Powerpc/Watchpoint: Add dar outside test in perf-hwbreak.c selftest Powerpc/Watchpoint: Support for 8xx in ptrace-hwbreak.c selftest arch/powerpc/include/asm/hw_breakpoint.h | 9 +- arch/powerpc/kernel/dawr.c | 6 +- arch/powerpc/kernel/hw_breakpoint.c | 119 ++-- arch/powerpc/kernel/process.c | 3 + arch/powerpc/kernel/ptrace.c | 16 +- arch/powerpc/xmon/xmon.c | 2 +- .../selftests/powerpc/ptrace/perf-hwbreak.c | 119 +++- .../selftests/powerpc/ptrace/ptrace-hwbreak.c | 581 +++++++++++------- 8 files changed, 590 insertions(+), 265 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v6 1/7] Powerpc/Watchpoint: Introduce macros for watchpoint length 2019-10-17 9:31 ` Ravi Bangoria @ 2019-10-17 9:31 ` Ravi Bangoria -1 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:31 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: npiggin, benh, paulus, naveen.n.rao, linuxppc-dev, linux-kernel, Ravi Bangoria We are hadrcoding length everywhere in the watchpoint code. Introduce macros for the length and use them. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- arch/powerpc/include/asm/hw_breakpoint.h | 3 +++ arch/powerpc/kernel/hw_breakpoint.c | 4 ++-- arch/powerpc/kernel/ptrace.c | 6 +++--- arch/powerpc/xmon/xmon.c | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h index 67e2da195eae..4a887e85a5f4 100644 --- a/arch/powerpc/include/asm/hw_breakpoint.h +++ b/arch/powerpc/include/asm/hw_breakpoint.h @@ -33,6 +33,9 @@ struct arch_hw_breakpoint { #define HW_BRK_TYPE_PRIV_ALL (HW_BRK_TYPE_USER | HW_BRK_TYPE_KERNEL | \ HW_BRK_TYPE_HYP) +#define DABR_MAX_LEN 8 +#define DAWR_MAX_LEN 512 + #ifdef CONFIG_HAVE_HW_BREAKPOINT #include <linux/kdebug.h> #include <asm/reg.h> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index 1007ec36b4cb..677041cb3c3e 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -163,9 +163,9 @@ int hw_breakpoint_arch_parse(struct perf_event *bp, */ if (!ppc_breakpoint_available()) return -ENODEV; - length_max = 8; /* DABR */ + length_max = DABR_MAX_LEN; /* DABR */ if (dawr_enabled()) { - length_max = 512 ; /* 64 doublewords */ + length_max = DAWR_MAX_LEN; /* 64 doublewords */ /* DAWR region can't cross 512 boundary */ if ((attr->bp_addr >> 9) != ((attr->bp_addr + attr->bp_len - 1) >> 9)) diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index 8c92febf5f44..f22e773a416a 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -2425,7 +2425,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, return -EIO; hw_brk.address = data & (~HW_BRK_TYPE_DABR); hw_brk.type = (data & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL; - hw_brk.len = 8; + hw_brk.len = DABR_MAX_LEN; set_bp = (data) && (hw_brk.type & HW_BRK_TYPE_RDWR); #ifdef CONFIG_HAVE_HW_BREAKPOINT bp = thread->ptrace_bps[0]; @@ -2456,7 +2456,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, /* Create a new breakpoint request if one doesn't exist already */ hw_breakpoint_init(&attr); attr.bp_addr = hw_brk.address; - attr.bp_len = 8; + attr.bp_len = DABR_MAX_LEN; arch_bp_generic_fields(hw_brk.type, &attr.bp_type); @@ -2882,7 +2882,7 @@ static long ppc_set_hwdebug(struct task_struct *child, brk.address = bp_info->addr & ~7UL; brk.type = HW_BRK_TYPE_TRANSLATE; - brk.len = 8; + brk.len = DABR_MAX_LEN; if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ) brk.type |= HW_BRK_TYPE_READ; if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE) diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index d83364ebc5c5..d547e540c230 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c @@ -884,7 +884,7 @@ static void insert_cpu_bpts(void) if (dabr.enabled) { brk.address = dabr.address; brk.type = (dabr.enabled & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL; - brk.len = 8; + brk.len = DABR_MAX_LEN; __set_breakpoint(&brk); } -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 1/7] Powerpc/Watchpoint: Introduce macros for watchpoint length @ 2019-10-17 9:31 ` Ravi Bangoria 0 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:31 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: Ravi Bangoria, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev We are hadrcoding length everywhere in the watchpoint code. Introduce macros for the length and use them. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- arch/powerpc/include/asm/hw_breakpoint.h | 3 +++ arch/powerpc/kernel/hw_breakpoint.c | 4 ++-- arch/powerpc/kernel/ptrace.c | 6 +++--- arch/powerpc/xmon/xmon.c | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h index 67e2da195eae..4a887e85a5f4 100644 --- a/arch/powerpc/include/asm/hw_breakpoint.h +++ b/arch/powerpc/include/asm/hw_breakpoint.h @@ -33,6 +33,9 @@ struct arch_hw_breakpoint { #define HW_BRK_TYPE_PRIV_ALL (HW_BRK_TYPE_USER | HW_BRK_TYPE_KERNEL | \ HW_BRK_TYPE_HYP) +#define DABR_MAX_LEN 8 +#define DAWR_MAX_LEN 512 + #ifdef CONFIG_HAVE_HW_BREAKPOINT #include <linux/kdebug.h> #include <asm/reg.h> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index 1007ec36b4cb..677041cb3c3e 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -163,9 +163,9 @@ int hw_breakpoint_arch_parse(struct perf_event *bp, */ if (!ppc_breakpoint_available()) return -ENODEV; - length_max = 8; /* DABR */ + length_max = DABR_MAX_LEN; /* DABR */ if (dawr_enabled()) { - length_max = 512 ; /* 64 doublewords */ + length_max = DAWR_MAX_LEN; /* 64 doublewords */ /* DAWR region can't cross 512 boundary */ if ((attr->bp_addr >> 9) != ((attr->bp_addr + attr->bp_len - 1) >> 9)) diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index 8c92febf5f44..f22e773a416a 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -2425,7 +2425,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, return -EIO; hw_brk.address = data & (~HW_BRK_TYPE_DABR); hw_brk.type = (data & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL; - hw_brk.len = 8; + hw_brk.len = DABR_MAX_LEN; set_bp = (data) && (hw_brk.type & HW_BRK_TYPE_RDWR); #ifdef CONFIG_HAVE_HW_BREAKPOINT bp = thread->ptrace_bps[0]; @@ -2456,7 +2456,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, /* Create a new breakpoint request if one doesn't exist already */ hw_breakpoint_init(&attr); attr.bp_addr = hw_brk.address; - attr.bp_len = 8; + attr.bp_len = DABR_MAX_LEN; arch_bp_generic_fields(hw_brk.type, &attr.bp_type); @@ -2882,7 +2882,7 @@ static long ppc_set_hwdebug(struct task_struct *child, brk.address = bp_info->addr & ~7UL; brk.type = HW_BRK_TYPE_TRANSLATE; - brk.len = 8; + brk.len = DABR_MAX_LEN; if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ) brk.type |= HW_BRK_TYPE_READ; if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE) diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index d83364ebc5c5..d547e540c230 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c @@ -884,7 +884,7 @@ static void insert_cpu_bpts(void) if (dabr.enabled) { brk.address = dabr.address; brk.type = (dabr.enabled & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL; - brk.len = 8; + brk.len = DABR_MAX_LEN; __set_breakpoint(&brk); } -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v6 1/7] Powerpc/Watchpoint: Introduce macros for watchpoint length 2019-10-17 9:31 ` Ravi Bangoria (?) @ 2019-11-14 9:07 ` Michael Ellerman -1 siblings, 0 replies; 23+ messages in thread From: Michael Ellerman @ 2019-11-14 9:07 UTC (permalink / raw) To: Ravi Bangoria, christophe.leroy, mikey Cc: Ravi Bangoria, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev On Thu, 2019-10-17 at 09:31:58 UTC, Ravi Bangoria wrote: > We are hadrcoding length everywhere in the watchpoint code. > Introduce macros for the length and use them. > > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> Series applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/b811be615cb78c90fca42bbd5b958427d03ba7e0 cheers ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v6 2/7] Powerpc/Watchpoint: Fix length calculation for unaligned target 2019-10-17 9:31 ` Ravi Bangoria @ 2019-10-17 9:31 ` Ravi Bangoria -1 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:31 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: npiggin, benh, paulus, naveen.n.rao, linuxppc-dev, linux-kernel, Ravi Bangoria 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. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- arch/powerpc/include/asm/hw_breakpoint.h | 2 + arch/powerpc/kernel/dawr.c | 6 +-- arch/powerpc/kernel/hw_breakpoint.c | 67 +++++++++++++++++------- arch/powerpc/kernel/process.c | 3 ++ arch/powerpc/kernel/ptrace.c | 1 + 5 files changed, 56 insertions(+), 23 deletions(-) diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h index 4a887e85a5f4..ea91ac7f5a27 100644 --- a/arch/powerpc/include/asm/hw_breakpoint.h +++ b/arch/powerpc/include/asm/hw_breakpoint.h @@ -14,6 +14,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 @@ -73,6 +74,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(&brk); } diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c index 5f66b95b6858..cc14aa6c4a1b 100644 --- a/arch/powerpc/kernel/dawr.c +++ b/arch/powerpc/kernel/dawr.c @@ -30,10 +30,10 @@ int set_dawr(struct arch_hw_breakpoint *brk) * DAWR length is stored in field MDR bits 48:53. Matches range in * doublewords (64 bits) baised by -1 eg. 0b000000=1DW and * 0b111111=64DW. - * brk->len is in bytes. + * brk->hw_len is in bytes. * This aligns up to double word size, shifts and does the bias. */ - mrd = ((brk->len + 7) >> 3) - 1; + mrd = ((brk->hw_len + 7) >> 3) - 1; dawrx |= (mrd & 0x3f) << (63 - 53); if (ppc_md.set_dawr) @@ -54,7 +54,7 @@ static ssize_t dawr_write_file_bool(struct file *file, const char __user *user_buf, size_t count, loff_t *ppos) { - struct arch_hw_breakpoint null_brk = {0, 0, 0}; + struct arch_hw_breakpoint null_brk = {0}; size_t rc; /* Send error to user if they hypervisor won't allow us to write DAWR */ diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index 677041cb3c3e..f36274d426ed 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -126,6 +126,49 @@ int arch_bp_generic_fields(int type, int *gen_bp_type) return 0; } +/* + * 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 this case, we should configure hw as: + * start_addr = address & ~HW_BREAKPOINT_ALIGN + * len = 16 bytes + * + * @start_addr and @end_addr are inclusive. + */ +static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw) +{ + u16 max_len = DABR_MAX_LEN; + 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; + + if (dawr_enabled()) { + max_len = DAWR_MAX_LEN; + /* DAWR region can't cross 512 bytes boundary */ + if ((start_addr >> 9) != (end_addr >> 9)) + return -EINVAL; + } + + if (hw_len > max_len) + return -EINVAL; + + hw->hw_len = hw_len; + return 0; +} + /* * Validate the arch-specific HW Breakpoint register settings */ @@ -133,9 +176,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; @@ -155,26 +198,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 = DABR_MAX_LEN; /* DABR */ - if (dawr_enabled()) { - length_max = DAWR_MAX_LEN; /* 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_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 639ceae7da9d..4df94b6e2f32 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -715,6 +715,8 @@ 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(&thread->hw_brk); } @@ -816,6 +818,7 @@ static inline bool hw_brk_match(struct arch_hw_breakpoint *a, return false; if (a->len != b->len) return false; + /* no need to check hw_len. it's calculated from address and len */ return true; } diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index f22e773a416a..c861b12337bd 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -2426,6 +2426,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, hw_brk.address = data & (~HW_BRK_TYPE_DABR); hw_brk.type = (data & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL; hw_brk.len = DABR_MAX_LEN; + hw_brk.hw_len = DABR_MAX_LEN; set_bp = (data) && (hw_brk.type & HW_BRK_TYPE_RDWR); #ifdef CONFIG_HAVE_HW_BREAKPOINT bp = thread->ptrace_bps[0]; -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 2/7] Powerpc/Watchpoint: Fix length calculation for unaligned target @ 2019-10-17 9:31 ` Ravi Bangoria 0 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:31 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: Ravi Bangoria, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev 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. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- arch/powerpc/include/asm/hw_breakpoint.h | 2 + arch/powerpc/kernel/dawr.c | 6 +-- arch/powerpc/kernel/hw_breakpoint.c | 67 +++++++++++++++++------- arch/powerpc/kernel/process.c | 3 ++ arch/powerpc/kernel/ptrace.c | 1 + 5 files changed, 56 insertions(+), 23 deletions(-) diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h index 4a887e85a5f4..ea91ac7f5a27 100644 --- a/arch/powerpc/include/asm/hw_breakpoint.h +++ b/arch/powerpc/include/asm/hw_breakpoint.h @@ -14,6 +14,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 @@ -73,6 +74,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(&brk); } diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c index 5f66b95b6858..cc14aa6c4a1b 100644 --- a/arch/powerpc/kernel/dawr.c +++ b/arch/powerpc/kernel/dawr.c @@ -30,10 +30,10 @@ int set_dawr(struct arch_hw_breakpoint *brk) * DAWR length is stored in field MDR bits 48:53. Matches range in * doublewords (64 bits) baised by -1 eg. 0b000000=1DW and * 0b111111=64DW. - * brk->len is in bytes. + * brk->hw_len is in bytes. * This aligns up to double word size, shifts and does the bias. */ - mrd = ((brk->len + 7) >> 3) - 1; + mrd = ((brk->hw_len + 7) >> 3) - 1; dawrx |= (mrd & 0x3f) << (63 - 53); if (ppc_md.set_dawr) @@ -54,7 +54,7 @@ static ssize_t dawr_write_file_bool(struct file *file, const char __user *user_buf, size_t count, loff_t *ppos) { - struct arch_hw_breakpoint null_brk = {0, 0, 0}; + struct arch_hw_breakpoint null_brk = {0}; size_t rc; /* Send error to user if they hypervisor won't allow us to write DAWR */ diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index 677041cb3c3e..f36274d426ed 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -126,6 +126,49 @@ int arch_bp_generic_fields(int type, int *gen_bp_type) return 0; } +/* + * 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 this case, we should configure hw as: + * start_addr = address & ~HW_BREAKPOINT_ALIGN + * len = 16 bytes + * + * @start_addr and @end_addr are inclusive. + */ +static int hw_breakpoint_validate_len(struct arch_hw_breakpoint *hw) +{ + u16 max_len = DABR_MAX_LEN; + 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; + + if (dawr_enabled()) { + max_len = DAWR_MAX_LEN; + /* DAWR region can't cross 512 bytes boundary */ + if ((start_addr >> 9) != (end_addr >> 9)) + return -EINVAL; + } + + if (hw_len > max_len) + return -EINVAL; + + hw->hw_len = hw_len; + return 0; +} + /* * Validate the arch-specific HW Breakpoint register settings */ @@ -133,9 +176,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; @@ -155,26 +198,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 = DABR_MAX_LEN; /* DABR */ - if (dawr_enabled()) { - length_max = DAWR_MAX_LEN; /* 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_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 639ceae7da9d..4df94b6e2f32 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -715,6 +715,8 @@ 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(&thread->hw_brk); } @@ -816,6 +818,7 @@ static inline bool hw_brk_match(struct arch_hw_breakpoint *a, return false; if (a->len != b->len) return false; + /* no need to check hw_len. it's calculated from address and len */ return true; } diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index f22e773a416a..c861b12337bd 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -2426,6 +2426,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, hw_brk.address = data & (~HW_BRK_TYPE_DABR); hw_brk.type = (data & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL; hw_brk.len = DABR_MAX_LEN; + hw_brk.hw_len = DABR_MAX_LEN; set_bp = (data) && (hw_brk.type & HW_BRK_TYPE_RDWR); #ifdef CONFIG_HAVE_HW_BREAKPOINT bp = thread->ptrace_bps[0]; -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 3/7] Powerpc/Watchpoint: Fix ptrace code that muck around with address/len 2019-10-17 9:31 ` Ravi Bangoria @ 2019-10-17 9:32 ` Ravi Bangoria -1 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:32 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: npiggin, benh, paulus, naveen.n.rao, linuxppc-dev, linux-kernel, Ravi Bangoria ptrace_set_debugreg() does not consider new length while overwriting the watchpoint. Fix that. ppc_set_hwdebug() aligns watchpoint address to doubleword boundary but does not change the length. If address range is crossing doubleword boundary and length is less then 8, we will loose samples from second doubleword. So fix that as well. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- arch/powerpc/include/asm/hw_breakpoint.h | 4 ++-- arch/powerpc/kernel/ptrace.c | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h index ea91ac7f5a27..27ac6f5d2891 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 + #define DABR_MAX_LEN 8 #define DAWR_MAX_LEN 512 @@ -48,8 +50,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); diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index c861b12337bd..8ea6f01531f1 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -2440,6 +2440,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, if (bp) { attr = bp->attr; attr.bp_addr = hw_brk.address; + attr.bp_len = DABR_MAX_LEN; arch_bp_generic_fields(hw_brk.type, &attr.bp_type); /* Enable breakpoint */ @@ -2881,7 +2882,7 @@ static long ppc_set_hwdebug(struct task_struct *child, if ((unsigned long)bp_info->addr >= TASK_SIZE) return -EIO; - brk.address = bp_info->addr & ~7UL; + brk.address = bp_info->addr & ~HW_BREAKPOINT_ALIGN; brk.type = HW_BRK_TYPE_TRANSLATE; brk.len = DABR_MAX_LEN; if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ) @@ -2889,10 +2890,6 @@ static long ppc_set_hwdebug(struct task_struct *child, if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE) brk.type |= HW_BRK_TYPE_WRITE; #ifdef CONFIG_HAVE_HW_BREAKPOINT - /* - * Check if the request is for 'range' breakpoints. We can - * support it if range < 8 bytes. - */ if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE) len = bp_info->addr2 - bp_info->addr; else if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_EXACT) @@ -2905,7 +2902,7 @@ static long ppc_set_hwdebug(struct task_struct *child, /* Create a new breakpoint request if one doesn't exist already */ hw_breakpoint_init(&attr); - attr.bp_addr = (unsigned long)bp_info->addr & ~HW_BREAKPOINT_ALIGN; + attr.bp_addr = (unsigned long)bp_info->addr; attr.bp_len = len; arch_bp_generic_fields(brk.type, &attr.bp_type); -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 3/7] Powerpc/Watchpoint: Fix ptrace code that muck around with address/len @ 2019-10-17 9:32 ` Ravi Bangoria 0 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:32 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: Ravi Bangoria, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev ptrace_set_debugreg() does not consider new length while overwriting the watchpoint. Fix that. ppc_set_hwdebug() aligns watchpoint address to doubleword boundary but does not change the length. If address range is crossing doubleword boundary and length is less then 8, we will loose samples from second doubleword. So fix that as well. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- arch/powerpc/include/asm/hw_breakpoint.h | 4 ++-- arch/powerpc/kernel/ptrace.c | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h index ea91ac7f5a27..27ac6f5d2891 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 + #define DABR_MAX_LEN 8 #define DAWR_MAX_LEN 512 @@ -48,8 +50,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); diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index c861b12337bd..8ea6f01531f1 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -2440,6 +2440,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, if (bp) { attr = bp->attr; attr.bp_addr = hw_brk.address; + attr.bp_len = DABR_MAX_LEN; arch_bp_generic_fields(hw_brk.type, &attr.bp_type); /* Enable breakpoint */ @@ -2881,7 +2882,7 @@ static long ppc_set_hwdebug(struct task_struct *child, if ((unsigned long)bp_info->addr >= TASK_SIZE) return -EIO; - brk.address = bp_info->addr & ~7UL; + brk.address = bp_info->addr & ~HW_BREAKPOINT_ALIGN; brk.type = HW_BRK_TYPE_TRANSLATE; brk.len = DABR_MAX_LEN; if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ) @@ -2889,10 +2890,6 @@ static long ppc_set_hwdebug(struct task_struct *child, if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE) brk.type |= HW_BRK_TYPE_WRITE; #ifdef CONFIG_HAVE_HW_BREAKPOINT - /* - * Check if the request is for 'range' breakpoints. We can - * support it if range < 8 bytes. - */ if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE) len = bp_info->addr2 - bp_info->addr; else if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_EXACT) @@ -2905,7 +2902,7 @@ static long ppc_set_hwdebug(struct task_struct *child, /* Create a new breakpoint request if one doesn't exist already */ hw_breakpoint_init(&attr); - attr.bp_addr = (unsigned long)bp_info->addr & ~HW_BREAKPOINT_ALIGN; + attr.bp_addr = (unsigned long)bp_info->addr; attr.bp_len = len; arch_bp_generic_fields(brk.type, &attr.bp_type); -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 4/7] Powerpc/Watchpoint: Don't ignore extraneous exceptions blindly 2019-10-17 9:31 ` Ravi Bangoria @ 2019-10-17 9:32 ` Ravi Bangoria -1 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:32 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: npiggin, benh, paulus, naveen.n.rao, linuxppc-dev, linux-kernel, Ravi Bangoria On Powerpc, watchpoint match range is double-word granular. On a watchpoint hit, DAR is set to the first byte of overlap between actual access and watched range. And thus it's quite possible that DAR does not point inside user specified range. Ex, say user creates a watchpoint with address range 0x1004 to 0x1007. So hw would be configured to watch from 0x1000 to 0x1007. If there is a 4 byte access from 0x1002 to 0x1005, DAR will point to 0x1002 and thus interrupt handler considers it as extraneous, but it's actually not, because part of the access belongs to what user has asked. Instead of blindly ignoring the exception, get actual address range by analysing an instruction, and ignore only if actual range does not overlap with user specified range. Note: The behavior is unchanged for 8xx. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- arch/powerpc/kernel/hw_breakpoint.c | 52 +++++++++++++++++------------ 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index f36274d426ed..58ce3d37c2a3 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -222,33 +222,49 @@ void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs) tsk->thread.last_hit_ubp = NULL; } -static bool is_larx_stcx_instr(struct pt_regs *regs, unsigned int instr) +static bool dar_within_range(unsigned long dar, struct arch_hw_breakpoint *info) { - int ret, type; - struct instruction_op op; + return ((info->address <= dar) && (dar - info->address < info->len)); +} - ret = analyse_instr(&op, regs, instr); - type = GETTYPE(op.type); - return (!ret && (type == LARX || type == STCX)); +static bool +dar_range_overlaps(unsigned long dar, int size, struct arch_hw_breakpoint *info) +{ + return ((dar <= info->address + info->len - 1) && + (dar + size - 1 >= info->address)); } /* * Handle debug exception notifications. */ static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp, - unsigned long addr) + struct arch_hw_breakpoint *info) { unsigned int instr = 0; + int ret, type, size; + struct instruction_op op; + unsigned long addr = info->address; if (__get_user_inatomic(instr, (unsigned int *)regs->nip)) goto fail; - if (is_larx_stcx_instr(regs, instr)) { + ret = analyse_instr(&op, regs, instr); + type = GETTYPE(op.type); + size = GETSIZE(op.type); + + if (!ret && (type == LARX || type == STCX)) { printk_ratelimited("Breakpoint hit on instruction that can't be emulated." " Breakpoint at 0x%lx will be disabled.\n", addr); goto disable; } + /* + * If it's extraneous event, we still need to emulate/single- + * step the instruction, but we don't generate an event. + */ + if (size && !dar_range_overlaps(regs->dar, size, info)) + info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ; + /* Do not emulate user-space instructions, instead single-step them */ if (user_mode(regs)) { current->thread.last_hit_ubp = bp; @@ -280,7 +296,6 @@ int hw_breakpoint_handler(struct die_args *args) struct perf_event *bp; struct pt_regs *regs = args->regs; struct arch_hw_breakpoint *info; - unsigned long dar = regs->dar; /* Disable breakpoints during exception handling */ hw_breakpoint_disable(); @@ -312,19 +327,14 @@ int hw_breakpoint_handler(struct die_args *args) goto out; } - /* - * Verify if dar lies within the address range occupied by the symbol - * being watched to filter extraneous exceptions. If it doesn't, - * we still need to single-step the instruction, but we don't - * generate an event. - */ info->type &= ~HW_BRK_TYPE_EXTRANEOUS_IRQ; - if (!((bp->attr.bp_addr <= dar) && - (dar - bp->attr.bp_addr < bp->attr.bp_len))) - info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ; - - if (!IS_ENABLED(CONFIG_PPC_8xx) && !stepping_handler(regs, bp, info->address)) - goto out; + if (IS_ENABLED(CONFIG_PPC_8xx)) { + if (!dar_within_range(regs->dar, info)) + info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ; + } else { + if (!stepping_handler(regs, bp, info)) + goto out; + } /* * As a policy, the callback is invoked in a 'trigger-after-execute' -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 4/7] Powerpc/Watchpoint: Don't ignore extraneous exceptions blindly @ 2019-10-17 9:32 ` Ravi Bangoria 0 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:32 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: Ravi Bangoria, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev On Powerpc, watchpoint match range is double-word granular. On a watchpoint hit, DAR is set to the first byte of overlap between actual access and watched range. And thus it's quite possible that DAR does not point inside user specified range. Ex, say user creates a watchpoint with address range 0x1004 to 0x1007. So hw would be configured to watch from 0x1000 to 0x1007. If there is a 4 byte access from 0x1002 to 0x1005, DAR will point to 0x1002 and thus interrupt handler considers it as extraneous, but it's actually not, because part of the access belongs to what user has asked. Instead of blindly ignoring the exception, get actual address range by analysing an instruction, and ignore only if actual range does not overlap with user specified range. Note: The behavior is unchanged for 8xx. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- arch/powerpc/kernel/hw_breakpoint.c | 52 +++++++++++++++++------------ 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c index f36274d426ed..58ce3d37c2a3 100644 --- a/arch/powerpc/kernel/hw_breakpoint.c +++ b/arch/powerpc/kernel/hw_breakpoint.c @@ -222,33 +222,49 @@ void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs) tsk->thread.last_hit_ubp = NULL; } -static bool is_larx_stcx_instr(struct pt_regs *regs, unsigned int instr) +static bool dar_within_range(unsigned long dar, struct arch_hw_breakpoint *info) { - int ret, type; - struct instruction_op op; + return ((info->address <= dar) && (dar - info->address < info->len)); +} - ret = analyse_instr(&op, regs, instr); - type = GETTYPE(op.type); - return (!ret && (type == LARX || type == STCX)); +static bool +dar_range_overlaps(unsigned long dar, int size, struct arch_hw_breakpoint *info) +{ + return ((dar <= info->address + info->len - 1) && + (dar + size - 1 >= info->address)); } /* * Handle debug exception notifications. */ static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp, - unsigned long addr) + struct arch_hw_breakpoint *info) { unsigned int instr = 0; + int ret, type, size; + struct instruction_op op; + unsigned long addr = info->address; if (__get_user_inatomic(instr, (unsigned int *)regs->nip)) goto fail; - if (is_larx_stcx_instr(regs, instr)) { + ret = analyse_instr(&op, regs, instr); + type = GETTYPE(op.type); + size = GETSIZE(op.type); + + if (!ret && (type == LARX || type == STCX)) { printk_ratelimited("Breakpoint hit on instruction that can't be emulated." " Breakpoint at 0x%lx will be disabled.\n", addr); goto disable; } + /* + * If it's extraneous event, we still need to emulate/single- + * step the instruction, but we don't generate an event. + */ + if (size && !dar_range_overlaps(regs->dar, size, info)) + info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ; + /* Do not emulate user-space instructions, instead single-step them */ if (user_mode(regs)) { current->thread.last_hit_ubp = bp; @@ -280,7 +296,6 @@ int hw_breakpoint_handler(struct die_args *args) struct perf_event *bp; struct pt_regs *regs = args->regs; struct arch_hw_breakpoint *info; - unsigned long dar = regs->dar; /* Disable breakpoints during exception handling */ hw_breakpoint_disable(); @@ -312,19 +327,14 @@ int hw_breakpoint_handler(struct die_args *args) goto out; } - /* - * Verify if dar lies within the address range occupied by the symbol - * being watched to filter extraneous exceptions. If it doesn't, - * we still need to single-step the instruction, but we don't - * generate an event. - */ info->type &= ~HW_BRK_TYPE_EXTRANEOUS_IRQ; - if (!((bp->attr.bp_addr <= dar) && - (dar - bp->attr.bp_addr < bp->attr.bp_len))) - info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ; - - if (!IS_ENABLED(CONFIG_PPC_8xx) && !stepping_handler(regs, bp, info->address)) - goto out; + if (IS_ENABLED(CONFIG_PPC_8xx)) { + if (!dar_within_range(regs->dar, info)) + info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ; + } else { + if (!stepping_handler(regs, bp, info)) + goto out; + } /* * As a policy, the callback is invoked in a 'trigger-after-execute' -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 5/7] Powerpc/Watchpoint: Rewrite ptrace-hwbreak.c selftest 2019-10-17 9:31 ` Ravi Bangoria @ 2019-10-17 9:32 ` Ravi Bangoria -1 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:32 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: npiggin, benh, paulus, naveen.n.rao, linuxppc-dev, linux-kernel, Ravi Bangoria 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:powerpc-5.3-4-224-g218b868240c7-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, MODE_RANGE, DW UNALIGNED, DAR OUTSIDE, RW, len: 6: Ok PPC_PTRACE_SETHWDEBUG, DAWR_MAX_LEN, RW, len: 512: Ok success: ptrace-hwbreak Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- .../selftests/powerpc/ptrace/ptrace-hwbreak.c | 571 +++++++++++------- 1 file changed, 361 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..916e97f5f8b1 100644 --- a/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c +++ b/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c @@ -22,318 +22,469 @@ #include <sys/wait.h> #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, &dbginfo); - 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, &info); - 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); } -#define DAWR_LENGTH_MAX 512 - -/* Dummy variables to test read/write accesses */ -static unsigned long long - dummy_array[DAWR_LENGTH_MAX / sizeof(unsigned long long)] - __attribute__((aligned(512))); -static unsigned long long *dummy_var = dummy_array; - static void write_var(int len) { - long long *plval; - char *pcval; - short *psval; - int *pival; + __u8 *pcvar; + __u16 *psvar; + __u32 *pivar; + __u64 *plvar; switch (len) { case 1: - pcval = (char *)dummy_var; - *pcval = 0xff; + pcvar = (__u8 *)&glvar; + *pcvar = 0xff; break; case 2: - psval = (short *)dummy_var; - *psval = 0xffff; + psvar = (__u16 *)&glvar; + *psvar = 0xffff; break; case 4: - pival = (int *)dummy_var; - *pival = 0xffffffff; + pivar = (__u32 *)&glvar; + *pivar = 0xffffffff; break; case 8: - plval = (long long *)dummy_var; - *plval = 0xffffffffffffffffLL; + plvar = (__u64 *)&glvar; + *plvar = 0xffffffffffffffffLL; break; } } static void read_var(int len) { - char cval __attribute__((unused)); - short sval __attribute__((unused)); - int ival __attribute__((unused)); - long long lval __attribute__((unused)); + __u8 cvar __attribute__((unused)); + __u16 svar __attribute__((unused)); + __u32 ivar __attribute__((unused)); + __u64 lvar __attribute__((unused)); switch (len) { case 1: - cval = *(char *)dummy_var; + cvar = (__u8)glvar; break; case 2: - sval = *(short *)dummy_var; + svar = (__u16)glvar; break; case 4: - ival = *(int *)dummy_var; + ivar = (__u32)glvar; break; case 8: - lval = *(long long *)dummy_var; + lvar = (__u64)glvar; break; } } -/* - * Do the r/w accesses to trigger the breakpoints. And run - * the usual traps. - */ -static void trigger_tests(void) +static void test_workload(void) { - int len, ret; + __u8 cvar __attribute__((unused)); + __u32 ivar __attribute__((unused)); + int len = 0; - ret = ptrace(PTRACE_TRACEME, 0, NULL, 0); - if (ret) { - perror("Can't be traced?\n"); - return; + if (ptrace(PTRACE_TRACEME, 0, NULL, 0)) { + perror("Child can't be traced?"); + exit(-1); } /* Wake up father so that it sets up the first test */ kill(getpid(), SIGUSR1); - /* Test write watchpoints */ - for (len = 1; len <= sizeof(long); len <<= 1) + /* PTRACE_SET_DEBUGREG, WO test */ + for (len = 1; len <= sizeof(glvar); len <<= 1) write_var(len); - /* Test read/write watchpoints (on read accesses) */ - for (len = 1; len <= sizeof(long); len <<= 1) + /* PTRACE_SET_DEBUGREG, RO test */ + for (len = 1; len <= sizeof(glvar); len <<= 1) read_var(len); - /* Test when breakpoint is unset */ - - /* Test write watchpoints */ - for (len = 1; len <= sizeof(long); len <<= 1) - write_var(len); + /* PTRACE_SET_DEBUGREG, RW test */ + for (len = 1; len <= sizeof(glvar); len <<= 1) { + if (rand() % 2) + read_var(len); + else + write_var(len); + } - /* Test read/write watchpoints (on read accesses) */ - for (len = 1; len <= sizeof(long); len <<= 1) - read_var(len); + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, WO test */ + write_var(1); + + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RO test */ + read_var(1); + + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RW test */ + if (rand() % 2) + write_var(1); + else + read_var(1); + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, WO test */ + gstruct.a[rand() % A_LEN] = 'a'; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RO test */ + cvar = gstruct.a[rand() % A_LEN]; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RW test */ + if (rand() % 2) + gstruct.a[rand() % A_LEN] = 'a'; + else + cvar = gstruct.a[rand() % A_LEN]; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, WO test */ + gstruct.b[rand() % B_LEN] = 'b'; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, RO test */ + cvar = gstruct.b[rand() % B_LEN]; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, RW test */ + if (rand() % 2) + gstruct.b[rand() % B_LEN] = 'b'; + else + cvar = gstruct.b[rand() % B_LEN]; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, DAR OUTSIDE, RW test */ + if (rand() % 2) + *((int *)(gstruct.a + 4)) = 10; + else + ivar = *((int *)(gstruct.a + 4)); + + /* PPC_PTRACE_SETHWDEBUG. DAWR_MAX_LEN. RW test */ + if (rand() % 2) + big_var[rand() % DAWR_MAX_LEN] = 'a'; + else + cvar = big_var[rand() % DAWR_MAX_LEN]; } -static void check_success(const char *msg) +static void check_success(pid_t child_pid, const char *name, const char *type, + unsigned long saddr, int len) { - const char *msg2; int status; + siginfo_t siginfo; + unsigned long eaddr = (saddr + len - 1) | 0x7; + + saddr &= ~0x7; /* Wait for the child to SIGTRAP */ wait(&status); - msg2 = "Failed"; + ptrace(PTRACE_GETSIGINFO, child_pid, NULL, &siginfo); - if (WIFSTOPPED(status) && WSTOPSIG(status) == SIGTRAP) { - msg2 = "Child process hit the breakpoint"; + if (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGTRAP || + (unsigned long)siginfo.si_addr < saddr || + (unsigned long)siginfo.si_addr > eaddr) { + printf("%s, %s, len: %d: Fail\n", name, type, len); + exit(-1); } - printf("%s Result: [%s]\n", msg, msg2); + printf("%s, %s, len: %d: Ok\n", name, type, len); + + /* + * For ptrace registered watchpoint, signal is generated + * before executing load/store. Singlestep the instruction + * and then continue the test. + */ + ptrace(PTRACE_SINGLESTEP, child_pid, NULL, 0); + wait(NULL); } -static void launch_watchpoints(char *buf, int mode, int len, - struct ppc_debug_info *dbginfo, bool dawr) +static void ptrace_set_debugreg(pid_t child_pid, unsigned long wp_addr) { - const char *mode_str; - unsigned long data = (unsigned long)(dummy_var); - int wh, range; - - data &= ~0x7UL; - - if (mode == BP_W) { - data |= (1UL << 1); - mode_str = "write"; - } else { - data |= (1UL << 0); - data |= (1UL << 1); - mode_str = "read"; + if (ptrace(PTRACE_SET_DEBUGREG, child_pid, 0, wp_addr)) { + perror("PTRACE_SET_DEBUGREG failed"); + exit(-1); } +} - /* Set DABR_TRANSLATION bit */ - data |= (1UL << 2); - - /* use PTRACE_SET_DEBUGREG breakpoints */ - set_breakpoint_addr((void *)data); - ptrace(PTRACE_CONT, child_pid, NULL, 0); - sprintf(buf, "Test %s watchpoint with len: %d ", mode_str, len); - check_success(buf); - /* Unregister hw brkpoint */ - set_breakpoint_addr(NULL); +static int ptrace_sethwdebug(pid_t child_pid, struct ppc_hw_breakpoint *info) +{ + int wh = ptrace(PPC_PTRACE_SETHWDEBUG, child_pid, 0, info); - data = (data & ~7); /* remove dabr control bits */ + if (wh <= 0) { + perror("PPC_PTRACE_SETHWDEBUG failed"); + exit(-1); + } + return wh; +} - /* use PPC_PTRACE_SETHWDEBUG breakpoint */ - if (!(dbginfo->features & PPC_DEBUG_FEATURE_DATA_BP_RANGE)) - return; /* not supported */ - wh = set_hwbreakpoint_addr((void *)data, 0); - ptrace(PTRACE_CONT, child_pid, NULL, 0); - sprintf(buf, "Test %s watchpoint with len: %d ", mode_str, len); - check_success(buf); - /* Unregister hw brkpoint */ - del_hwbreakpoint_addr(wh); - - /* try a wider range */ - range = 8; - if (dawr) - range = 512 - ((int)data & (DAWR_LENGTH_MAX - 1)); - wh = set_hwbreakpoint_addr((void *)data, range); - ptrace(PTRACE_CONT, child_pid, NULL, 0); - sprintf(buf, "Test %s watchpoint with len: %d ", mode_str, len); - check_success(buf); - /* Unregister hw brkpoint */ - del_hwbreakpoint_addr(wh); +static void ptrace_delhwdebug(pid_t child_pid, int wh) +{ + if (ptrace(PPC_PTRACE_DELHWDEBUG, child_pid, 0, wh) < 0) { + perror("PPC_PTRACE_DELHWDEBUG failed"); + exit(-1); + } } -/* Set the breakpoints and check the child successfully trigger them */ -static int launch_tests(bool dawr) +#define DABR_READ_SHIFT 0 +#define DABR_WRITE_SHIFT 1 +#define DABR_TRANSLATION_SHIFT 2 + +static int test_set_debugreg(pid_t child_pid) { - char buf[1024]; - int len, i, status; + unsigned long wp_addr = (unsigned long)&glvar; + char *name = "PTRACE_SET_DEBUGREG"; + int len; + + /* PTRACE_SET_DEBUGREG, WO test*/ + wp_addr &= ~0x7UL; + wp_addr |= (1UL << DABR_WRITE_SHIFT); + wp_addr |= (1UL << DABR_TRANSLATION_SHIFT); + for (len = 1; len <= sizeof(glvar); len <<= 1) { + ptrace_set_debugreg(child_pid, wp_addr); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "WO", wp_addr, len); + } - struct ppc_debug_info dbginfo; + /* PTRACE_SET_DEBUGREG, RO test */ + wp_addr &= ~0x7UL; + wp_addr |= (1UL << DABR_READ_SHIFT); + wp_addr |= (1UL << DABR_TRANSLATION_SHIFT); + for (len = 1; len <= sizeof(glvar); len <<= 1) { + ptrace_set_debugreg(child_pid, wp_addr); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RO", wp_addr, len); + } - i = ptrace(PPC_PTRACE_GETHWDBGINFO, child_pid, NULL, &dbginfo); - if (i) { - perror("Can't set breakpoint info\n"); - exit(-1); + /* PTRACE_SET_DEBUGREG, RW test */ + wp_addr &= ~0x7UL; + wp_addr |= (1Ul << DABR_READ_SHIFT); + wp_addr |= (1UL << DABR_WRITE_SHIFT); + wp_addr |= (1UL << DABR_TRANSLATION_SHIFT); + for (len = 1; len <= sizeof(glvar); len <<= 1) { + ptrace_set_debugreg(child_pid, wp_addr); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); } - if (!(dbginfo.features & PPC_DEBUG_FEATURE_DATA_BP_RANGE)) - printf("WARNING: Kernel doesn't support PPC_PTRACE_SETHWDEBUG\n"); - /* Write watchpoint */ - for (len = 1; len <= sizeof(long); len <<= 1) - launch_watchpoints(buf, BP_W, len, &dbginfo, dawr); + ptrace_set_debugreg(child_pid, 0); + return 0; +} - /* Read-Write watchpoint */ - for (len = 1; len <= sizeof(long); len <<= 1) - launch_watchpoints(buf, BP_RW, len, &dbginfo, dawr); +static void get_ppc_hw_breakpoint(struct ppc_hw_breakpoint *info, int type, + unsigned long addr, int len) +{ + info->version = 1; + info->trigger_type = type; + info->condition_mode = PPC_BREAKPOINT_CONDITION_NONE; + info->addr = (__u64)addr; + info->addr2 = (__u64)addr + len; + info->condition_value = 0; + if (!len) + info->addr_mode = PPC_BREAKPOINT_MODE_EXACT; + else + info->addr_mode = PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE; +} +static void test_sethwdebug_exact(pid_t child_pid) +{ + struct ppc_hw_breakpoint info; + unsigned long wp_addr = (unsigned long)&glvar; + char *name = "PPC_PTRACE_SETHWDEBUG, MODE_EXACT"; + int len = 1; /* hardcoded in kernel */ + int wh; + + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, WO test */ + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_WRITE, wp_addr, 0); + wh = ptrace_sethwdebug(child_pid, &info); ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "WO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); - /* - * Now we have unregistered the breakpoint, access by child - * should not cause SIGTRAP. - */ + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RO test */ + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_READ, wp_addr, 0); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); - wait(&status); + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RW test */ + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_RW, wp_addr, 0); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); +} - if (WIFSTOPPED(status) && WSTOPSIG(status) == SIGTRAP) { - printf("FAIL: Child process hit the breakpoint, which is not expected\n"); - ptrace(PTRACE_CONT, child_pid, NULL, 0); - return TEST_FAIL; - } +static void test_sethwdebug_range_aligned(pid_t child_pid) +{ + struct ppc_hw_breakpoint info; + unsigned long wp_addr; + char *name = "PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED"; + int len; + int wh; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, WO test */ + wp_addr = (unsigned long)&gstruct.a; + len = A_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_WRITE, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "WO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RO test */ + wp_addr = (unsigned long)&gstruct.a; + len = A_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_READ, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RW test */ + wp_addr = (unsigned long)&gstruct.a; + len = A_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_RW, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); +} - if (WIFEXITED(status)) - printf("Child exited normally\n"); +static void test_sethwdebug_range_unaligned(pid_t child_pid) +{ + struct ppc_hw_breakpoint info; + unsigned long wp_addr; + char *name = "PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED"; + int len; + int wh; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, WO test */ + wp_addr = (unsigned long)&gstruct.b; + len = B_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_WRITE, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "WO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, RO test */ + wp_addr = (unsigned long)&gstruct.b; + len = B_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_READ, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, RW test */ + wp_addr = (unsigned long)&gstruct.b; + len = B_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_RW, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); - return TEST_PASS; +} + +static void test_sethwdebug_range_unaligned_dar(pid_t child_pid) +{ + struct ppc_hw_breakpoint info; + unsigned long wp_addr; + char *name = "PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, DAR OUTSIDE"; + int len; + int wh; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, DAR OUTSIDE, RW test */ + wp_addr = (unsigned long)&gstruct.b; + len = B_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_WRITE, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); +} + +static void test_sethwdebug_dawr_max_range(pid_t child_pid) +{ + struct ppc_hw_breakpoint info; + unsigned long wp_addr; + char *name = "PPC_PTRACE_SETHWDEBUG, DAWR_MAX_LEN"; + int len; + int wh; + + /* PPC_PTRACE_SETHWDEBUG, DAWR_MAX_LEN, RW test */ + wp_addr = (unsigned long)big_var; + len = DAWR_MAX_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_RW, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); +} + +/* Set the breakpoints and check the child successfully trigger them */ +static void +run_tests(pid_t child_pid, struct ppc_debug_info *dbginfo, bool dawr) +{ + test_set_debugreg(child_pid); + if (dbginfo->features & PPC_DEBUG_FEATURE_DATA_BP_RANGE) { + test_sethwdebug_exact(child_pid); + test_sethwdebug_range_aligned(child_pid); + if (dawr) { + test_sethwdebug_range_unaligned(child_pid); + test_sethwdebug_range_unaligned_dar(child_pid); + test_sethwdebug_dawr_max_range(child_pid); + } + } } static int ptrace_hwbreak(void) { - pid_t pid; - int ret; + pid_t child_pid; + struct ppc_debug_info dbginfo; bool dawr; - pid = fork(); - if (!pid) { - trigger_tests(); + child_pid = fork(); + if (!child_pid) { + test_workload(); return 0; } wait(NULL); - child_pid = pid; - - get_dbginfo(); - SKIP_IF(!hwbreak_present()); - dawr = dawr_present(); + get_dbginfo(child_pid, &dbginfo); + SKIP_IF(dbginfo.num_data_bps == 0); - ret = launch_tests(dawr); + dawr = dawr_present(&dbginfo); + run_tests(child_pid, &dbginfo, dawr); + /* Let the child exit first. */ + ptrace(PTRACE_CONT, child_pid, NULL, 0); wait(NULL); - return ret; + /* + * Testcases exits immediately with -1 on any failure. If + * it has reached here, it means all tests were successful. + */ + return TEST_PASS; } int main(int argc, char **argv, char **envp) -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 5/7] Powerpc/Watchpoint: Rewrite ptrace-hwbreak.c selftest @ 2019-10-17 9:32 ` Ravi Bangoria 0 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:32 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: Ravi Bangoria, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev 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:powerpc-5.3-4-224-g218b868240c7-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, MODE_RANGE, DW UNALIGNED, DAR OUTSIDE, RW, len: 6: Ok PPC_PTRACE_SETHWDEBUG, DAWR_MAX_LEN, RW, len: 512: Ok success: ptrace-hwbreak Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- .../selftests/powerpc/ptrace/ptrace-hwbreak.c | 571 +++++++++++------- 1 file changed, 361 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..916e97f5f8b1 100644 --- a/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c +++ b/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c @@ -22,318 +22,469 @@ #include <sys/wait.h> #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, &dbginfo); - 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, &info); - 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); } -#define DAWR_LENGTH_MAX 512 - -/* Dummy variables to test read/write accesses */ -static unsigned long long - dummy_array[DAWR_LENGTH_MAX / sizeof(unsigned long long)] - __attribute__((aligned(512))); -static unsigned long long *dummy_var = dummy_array; - static void write_var(int len) { - long long *plval; - char *pcval; - short *psval; - int *pival; + __u8 *pcvar; + __u16 *psvar; + __u32 *pivar; + __u64 *plvar; switch (len) { case 1: - pcval = (char *)dummy_var; - *pcval = 0xff; + pcvar = (__u8 *)&glvar; + *pcvar = 0xff; break; case 2: - psval = (short *)dummy_var; - *psval = 0xffff; + psvar = (__u16 *)&glvar; + *psvar = 0xffff; break; case 4: - pival = (int *)dummy_var; - *pival = 0xffffffff; + pivar = (__u32 *)&glvar; + *pivar = 0xffffffff; break; case 8: - plval = (long long *)dummy_var; - *plval = 0xffffffffffffffffLL; + plvar = (__u64 *)&glvar; + *plvar = 0xffffffffffffffffLL; break; } } static void read_var(int len) { - char cval __attribute__((unused)); - short sval __attribute__((unused)); - int ival __attribute__((unused)); - long long lval __attribute__((unused)); + __u8 cvar __attribute__((unused)); + __u16 svar __attribute__((unused)); + __u32 ivar __attribute__((unused)); + __u64 lvar __attribute__((unused)); switch (len) { case 1: - cval = *(char *)dummy_var; + cvar = (__u8)glvar; break; case 2: - sval = *(short *)dummy_var; + svar = (__u16)glvar; break; case 4: - ival = *(int *)dummy_var; + ivar = (__u32)glvar; break; case 8: - lval = *(long long *)dummy_var; + lvar = (__u64)glvar; break; } } -/* - * Do the r/w accesses to trigger the breakpoints. And run - * the usual traps. - */ -static void trigger_tests(void) +static void test_workload(void) { - int len, ret; + __u8 cvar __attribute__((unused)); + __u32 ivar __attribute__((unused)); + int len = 0; - ret = ptrace(PTRACE_TRACEME, 0, NULL, 0); - if (ret) { - perror("Can't be traced?\n"); - return; + if (ptrace(PTRACE_TRACEME, 0, NULL, 0)) { + perror("Child can't be traced?"); + exit(-1); } /* Wake up father so that it sets up the first test */ kill(getpid(), SIGUSR1); - /* Test write watchpoints */ - for (len = 1; len <= sizeof(long); len <<= 1) + /* PTRACE_SET_DEBUGREG, WO test */ + for (len = 1; len <= sizeof(glvar); len <<= 1) write_var(len); - /* Test read/write watchpoints (on read accesses) */ - for (len = 1; len <= sizeof(long); len <<= 1) + /* PTRACE_SET_DEBUGREG, RO test */ + for (len = 1; len <= sizeof(glvar); len <<= 1) read_var(len); - /* Test when breakpoint is unset */ - - /* Test write watchpoints */ - for (len = 1; len <= sizeof(long); len <<= 1) - write_var(len); + /* PTRACE_SET_DEBUGREG, RW test */ + for (len = 1; len <= sizeof(glvar); len <<= 1) { + if (rand() % 2) + read_var(len); + else + write_var(len); + } - /* Test read/write watchpoints (on read accesses) */ - for (len = 1; len <= sizeof(long); len <<= 1) - read_var(len); + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, WO test */ + write_var(1); + + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RO test */ + read_var(1); + + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RW test */ + if (rand() % 2) + write_var(1); + else + read_var(1); + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, WO test */ + gstruct.a[rand() % A_LEN] = 'a'; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RO test */ + cvar = gstruct.a[rand() % A_LEN]; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RW test */ + if (rand() % 2) + gstruct.a[rand() % A_LEN] = 'a'; + else + cvar = gstruct.a[rand() % A_LEN]; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, WO test */ + gstruct.b[rand() % B_LEN] = 'b'; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, RO test */ + cvar = gstruct.b[rand() % B_LEN]; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, RW test */ + if (rand() % 2) + gstruct.b[rand() % B_LEN] = 'b'; + else + cvar = gstruct.b[rand() % B_LEN]; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, DAR OUTSIDE, RW test */ + if (rand() % 2) + *((int *)(gstruct.a + 4)) = 10; + else + ivar = *((int *)(gstruct.a + 4)); + + /* PPC_PTRACE_SETHWDEBUG. DAWR_MAX_LEN. RW test */ + if (rand() % 2) + big_var[rand() % DAWR_MAX_LEN] = 'a'; + else + cvar = big_var[rand() % DAWR_MAX_LEN]; } -static void check_success(const char *msg) +static void check_success(pid_t child_pid, const char *name, const char *type, + unsigned long saddr, int len) { - const char *msg2; int status; + siginfo_t siginfo; + unsigned long eaddr = (saddr + len - 1) | 0x7; + + saddr &= ~0x7; /* Wait for the child to SIGTRAP */ wait(&status); - msg2 = "Failed"; + ptrace(PTRACE_GETSIGINFO, child_pid, NULL, &siginfo); - if (WIFSTOPPED(status) && WSTOPSIG(status) == SIGTRAP) { - msg2 = "Child process hit the breakpoint"; + if (!WIFSTOPPED(status) || WSTOPSIG(status) != SIGTRAP || + (unsigned long)siginfo.si_addr < saddr || + (unsigned long)siginfo.si_addr > eaddr) { + printf("%s, %s, len: %d: Fail\n", name, type, len); + exit(-1); } - printf("%s Result: [%s]\n", msg, msg2); + printf("%s, %s, len: %d: Ok\n", name, type, len); + + /* + * For ptrace registered watchpoint, signal is generated + * before executing load/store. Singlestep the instruction + * and then continue the test. + */ + ptrace(PTRACE_SINGLESTEP, child_pid, NULL, 0); + wait(NULL); } -static void launch_watchpoints(char *buf, int mode, int len, - struct ppc_debug_info *dbginfo, bool dawr) +static void ptrace_set_debugreg(pid_t child_pid, unsigned long wp_addr) { - const char *mode_str; - unsigned long data = (unsigned long)(dummy_var); - int wh, range; - - data &= ~0x7UL; - - if (mode == BP_W) { - data |= (1UL << 1); - mode_str = "write"; - } else { - data |= (1UL << 0); - data |= (1UL << 1); - mode_str = "read"; + if (ptrace(PTRACE_SET_DEBUGREG, child_pid, 0, wp_addr)) { + perror("PTRACE_SET_DEBUGREG failed"); + exit(-1); } +} - /* Set DABR_TRANSLATION bit */ - data |= (1UL << 2); - - /* use PTRACE_SET_DEBUGREG breakpoints */ - set_breakpoint_addr((void *)data); - ptrace(PTRACE_CONT, child_pid, NULL, 0); - sprintf(buf, "Test %s watchpoint with len: %d ", mode_str, len); - check_success(buf); - /* Unregister hw brkpoint */ - set_breakpoint_addr(NULL); +static int ptrace_sethwdebug(pid_t child_pid, struct ppc_hw_breakpoint *info) +{ + int wh = ptrace(PPC_PTRACE_SETHWDEBUG, child_pid, 0, info); - data = (data & ~7); /* remove dabr control bits */ + if (wh <= 0) { + perror("PPC_PTRACE_SETHWDEBUG failed"); + exit(-1); + } + return wh; +} - /* use PPC_PTRACE_SETHWDEBUG breakpoint */ - if (!(dbginfo->features & PPC_DEBUG_FEATURE_DATA_BP_RANGE)) - return; /* not supported */ - wh = set_hwbreakpoint_addr((void *)data, 0); - ptrace(PTRACE_CONT, child_pid, NULL, 0); - sprintf(buf, "Test %s watchpoint with len: %d ", mode_str, len); - check_success(buf); - /* Unregister hw brkpoint */ - del_hwbreakpoint_addr(wh); - - /* try a wider range */ - range = 8; - if (dawr) - range = 512 - ((int)data & (DAWR_LENGTH_MAX - 1)); - wh = set_hwbreakpoint_addr((void *)data, range); - ptrace(PTRACE_CONT, child_pid, NULL, 0); - sprintf(buf, "Test %s watchpoint with len: %d ", mode_str, len); - check_success(buf); - /* Unregister hw brkpoint */ - del_hwbreakpoint_addr(wh); +static void ptrace_delhwdebug(pid_t child_pid, int wh) +{ + if (ptrace(PPC_PTRACE_DELHWDEBUG, child_pid, 0, wh) < 0) { + perror("PPC_PTRACE_DELHWDEBUG failed"); + exit(-1); + } } -/* Set the breakpoints and check the child successfully trigger them */ -static int launch_tests(bool dawr) +#define DABR_READ_SHIFT 0 +#define DABR_WRITE_SHIFT 1 +#define DABR_TRANSLATION_SHIFT 2 + +static int test_set_debugreg(pid_t child_pid) { - char buf[1024]; - int len, i, status; + unsigned long wp_addr = (unsigned long)&glvar; + char *name = "PTRACE_SET_DEBUGREG"; + int len; + + /* PTRACE_SET_DEBUGREG, WO test*/ + wp_addr &= ~0x7UL; + wp_addr |= (1UL << DABR_WRITE_SHIFT); + wp_addr |= (1UL << DABR_TRANSLATION_SHIFT); + for (len = 1; len <= sizeof(glvar); len <<= 1) { + ptrace_set_debugreg(child_pid, wp_addr); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "WO", wp_addr, len); + } - struct ppc_debug_info dbginfo; + /* PTRACE_SET_DEBUGREG, RO test */ + wp_addr &= ~0x7UL; + wp_addr |= (1UL << DABR_READ_SHIFT); + wp_addr |= (1UL << DABR_TRANSLATION_SHIFT); + for (len = 1; len <= sizeof(glvar); len <<= 1) { + ptrace_set_debugreg(child_pid, wp_addr); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RO", wp_addr, len); + } - i = ptrace(PPC_PTRACE_GETHWDBGINFO, child_pid, NULL, &dbginfo); - if (i) { - perror("Can't set breakpoint info\n"); - exit(-1); + /* PTRACE_SET_DEBUGREG, RW test */ + wp_addr &= ~0x7UL; + wp_addr |= (1Ul << DABR_READ_SHIFT); + wp_addr |= (1UL << DABR_WRITE_SHIFT); + wp_addr |= (1UL << DABR_TRANSLATION_SHIFT); + for (len = 1; len <= sizeof(glvar); len <<= 1) { + ptrace_set_debugreg(child_pid, wp_addr); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); } - if (!(dbginfo.features & PPC_DEBUG_FEATURE_DATA_BP_RANGE)) - printf("WARNING: Kernel doesn't support PPC_PTRACE_SETHWDEBUG\n"); - /* Write watchpoint */ - for (len = 1; len <= sizeof(long); len <<= 1) - launch_watchpoints(buf, BP_W, len, &dbginfo, dawr); + ptrace_set_debugreg(child_pid, 0); + return 0; +} - /* Read-Write watchpoint */ - for (len = 1; len <= sizeof(long); len <<= 1) - launch_watchpoints(buf, BP_RW, len, &dbginfo, dawr); +static void get_ppc_hw_breakpoint(struct ppc_hw_breakpoint *info, int type, + unsigned long addr, int len) +{ + info->version = 1; + info->trigger_type = type; + info->condition_mode = PPC_BREAKPOINT_CONDITION_NONE; + info->addr = (__u64)addr; + info->addr2 = (__u64)addr + len; + info->condition_value = 0; + if (!len) + info->addr_mode = PPC_BREAKPOINT_MODE_EXACT; + else + info->addr_mode = PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE; +} +static void test_sethwdebug_exact(pid_t child_pid) +{ + struct ppc_hw_breakpoint info; + unsigned long wp_addr = (unsigned long)&glvar; + char *name = "PPC_PTRACE_SETHWDEBUG, MODE_EXACT"; + int len = 1; /* hardcoded in kernel */ + int wh; + + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, WO test */ + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_WRITE, wp_addr, 0); + wh = ptrace_sethwdebug(child_pid, &info); ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "WO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); - /* - * Now we have unregistered the breakpoint, access by child - * should not cause SIGTRAP. - */ + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RO test */ + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_READ, wp_addr, 0); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); - wait(&status); + /* PPC_PTRACE_SETHWDEBUG, MODE_EXACT, RW test */ + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_RW, wp_addr, 0); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); +} - if (WIFSTOPPED(status) && WSTOPSIG(status) == SIGTRAP) { - printf("FAIL: Child process hit the breakpoint, which is not expected\n"); - ptrace(PTRACE_CONT, child_pid, NULL, 0); - return TEST_FAIL; - } +static void test_sethwdebug_range_aligned(pid_t child_pid) +{ + struct ppc_hw_breakpoint info; + unsigned long wp_addr; + char *name = "PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED"; + int len; + int wh; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, WO test */ + wp_addr = (unsigned long)&gstruct.a; + len = A_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_WRITE, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "WO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RO test */ + wp_addr = (unsigned long)&gstruct.a; + len = A_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_READ, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW ALIGNED, RW test */ + wp_addr = (unsigned long)&gstruct.a; + len = A_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_RW, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); +} - if (WIFEXITED(status)) - printf("Child exited normally\n"); +static void test_sethwdebug_range_unaligned(pid_t child_pid) +{ + struct ppc_hw_breakpoint info; + unsigned long wp_addr; + char *name = "PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED"; + int len; + int wh; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, WO test */ + wp_addr = (unsigned long)&gstruct.b; + len = B_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_WRITE, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "WO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, RO test */ + wp_addr = (unsigned long)&gstruct.b; + len = B_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_READ, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RO", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, RW test */ + wp_addr = (unsigned long)&gstruct.b; + len = B_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_RW, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); - return TEST_PASS; +} + +static void test_sethwdebug_range_unaligned_dar(pid_t child_pid) +{ + struct ppc_hw_breakpoint info; + unsigned long wp_addr; + char *name = "PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, DAR OUTSIDE"; + int len; + int wh; + + /* PPC_PTRACE_SETHWDEBUG, MODE_RANGE, DW UNALIGNED, DAR OUTSIDE, RW test */ + wp_addr = (unsigned long)&gstruct.b; + len = B_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_WRITE, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); +} + +static void test_sethwdebug_dawr_max_range(pid_t child_pid) +{ + struct ppc_hw_breakpoint info; + unsigned long wp_addr; + char *name = "PPC_PTRACE_SETHWDEBUG, DAWR_MAX_LEN"; + int len; + int wh; + + /* PPC_PTRACE_SETHWDEBUG, DAWR_MAX_LEN, RW test */ + wp_addr = (unsigned long)big_var; + len = DAWR_MAX_LEN; + get_ppc_hw_breakpoint(&info, PPC_BREAKPOINT_TRIGGER_RW, wp_addr, len); + wh = ptrace_sethwdebug(child_pid, &info); + ptrace(PTRACE_CONT, child_pid, NULL, 0); + check_success(child_pid, name, "RW", wp_addr, len); + ptrace_delhwdebug(child_pid, wh); +} + +/* Set the breakpoints and check the child successfully trigger them */ +static void +run_tests(pid_t child_pid, struct ppc_debug_info *dbginfo, bool dawr) +{ + test_set_debugreg(child_pid); + if (dbginfo->features & PPC_DEBUG_FEATURE_DATA_BP_RANGE) { + test_sethwdebug_exact(child_pid); + test_sethwdebug_range_aligned(child_pid); + if (dawr) { + test_sethwdebug_range_unaligned(child_pid); + test_sethwdebug_range_unaligned_dar(child_pid); + test_sethwdebug_dawr_max_range(child_pid); + } + } } static int ptrace_hwbreak(void) { - pid_t pid; - int ret; + pid_t child_pid; + struct ppc_debug_info dbginfo; bool dawr; - pid = fork(); - if (!pid) { - trigger_tests(); + child_pid = fork(); + if (!child_pid) { + test_workload(); return 0; } wait(NULL); - child_pid = pid; - - get_dbginfo(); - SKIP_IF(!hwbreak_present()); - dawr = dawr_present(); + get_dbginfo(child_pid, &dbginfo); + SKIP_IF(dbginfo.num_data_bps == 0); - ret = launch_tests(dawr); + dawr = dawr_present(&dbginfo); + run_tests(child_pid, &dbginfo, dawr); + /* Let the child exit first. */ + ptrace(PTRACE_CONT, child_pid, NULL, 0); wait(NULL); - return ret; + /* + * Testcases exits immediately with -1 on any failure. If + * it has reached here, it means all tests were successful. + */ + return TEST_PASS; } int main(int argc, char **argv, char **envp) -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 6/7] Powerpc/Watchpoint: Add dar outside test in perf-hwbreak.c selftest 2019-10-17 9:31 ` Ravi Bangoria @ 2019-10-17 9:32 ` Ravi Bangoria -1 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:32 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: npiggin, benh, paulus, naveen.n.rao, linuxppc-dev, linux-kernel, Ravi Bangoria So far we used to ignore exception if dar points outside of user specified range. But now we are ignoring it only if actual load/ store range does not overlap with user specified range. Include selftests for the same: # ./tools/testing/selftests/powerpc/ptrace/perf-hwbreak ... TESTED: No overlap TESTED: Partial overlap TESTED: Partial overlap TESTED: No overlap TESTED: Full overlap success: perf_hwbreak Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- .../selftests/powerpc/ptrace/perf-hwbreak.c | 119 +++++++++++++++++- 1 file changed, 118 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c b/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c index 200337daec42..c1f324afdbf3 100644 --- a/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c +++ b/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c @@ -148,6 +148,121 @@ static int runtestsingle(int readwriteflag, int exclude_user, int arraytest) return 0; } +static int runtest_dar_outside(void) +{ + void *target; + volatile __u16 temp16; + volatile __u64 temp64; + struct perf_event_attr attr; + int break_fd; + unsigned long long breaks; + int fail = 0; + size_t res; + + target = malloc(8); + if (!target) { + perror("malloc failed"); + exit(EXIT_FAILURE); + } + + /* setup counters */ + memset(&attr, 0, sizeof(attr)); + attr.disabled = 1; + attr.type = PERF_TYPE_BREAKPOINT; + attr.exclude_kernel = 1; + attr.exclude_hv = 1; + attr.exclude_guest = 1; + attr.bp_type = HW_BREAKPOINT_RW; + /* watch middle half of target array */ + attr.bp_addr = (__u64)(target + 2); + attr.bp_len = 4; + break_fd = sys_perf_event_open(&attr, 0, -1, -1, 0); + if (break_fd < 0) { + free(target); + perror("sys_perf_event_open"); + exit(EXIT_FAILURE); + } + + /* Shouldn't hit. */ + ioctl(break_fd, PERF_EVENT_IOC_RESET); + ioctl(break_fd, PERF_EVENT_IOC_ENABLE); + temp16 = *((__u16 *)target); + *((__u16 *)target) = temp16; + ioctl(break_fd, PERF_EVENT_IOC_DISABLE); + res = read(break_fd, &breaks, sizeof(unsigned long long)); + assert(res == sizeof(unsigned long long)); + if (breaks == 0) { + printf("TESTED: No overlap\n"); + } else { + printf("FAILED: No overlap: %lld != 0\n", breaks); + fail = 1; + } + + /* Hit */ + ioctl(break_fd, PERF_EVENT_IOC_RESET); + ioctl(break_fd, PERF_EVENT_IOC_ENABLE); + temp16 = *((__u16 *)(target + 1)); + *((__u16 *)(target + 1)) = temp16; + ioctl(break_fd, PERF_EVENT_IOC_DISABLE); + res = read(break_fd, &breaks, sizeof(unsigned long long)); + assert(res == sizeof(unsigned long long)); + if (breaks == 2) { + printf("TESTED: Partial overlap\n"); + } else { + printf("FAILED: Partial overlap: %lld != 2\n", breaks); + fail = 1; + } + + /* Hit */ + ioctl(break_fd, PERF_EVENT_IOC_RESET); + ioctl(break_fd, PERF_EVENT_IOC_ENABLE); + temp16 = *((__u16 *)(target + 5)); + *((__u16 *)(target + 5)) = temp16; + ioctl(break_fd, PERF_EVENT_IOC_DISABLE); + res = read(break_fd, &breaks, sizeof(unsigned long long)); + assert(res == sizeof(unsigned long long)); + if (breaks == 2) { + printf("TESTED: Partial overlap\n"); + } else { + printf("FAILED: Partial overlap: %lld != 2\n", breaks); + fail = 1; + } + + /* Shouldn't Hit */ + ioctl(break_fd, PERF_EVENT_IOC_RESET); + ioctl(break_fd, PERF_EVENT_IOC_ENABLE); + temp16 = *((__u16 *)(target + 6)); + *((__u16 *)(target + 6)) = temp16; + ioctl(break_fd, PERF_EVENT_IOC_DISABLE); + res = read(break_fd, &breaks, sizeof(unsigned long long)); + assert(res == sizeof(unsigned long long)); + if (breaks == 0) { + printf("TESTED: No overlap\n"); + } else { + printf("FAILED: No overlap: %lld != 0\n", breaks); + fail = 1; + } + + /* Hit */ + ioctl(break_fd, PERF_EVENT_IOC_RESET); + ioctl(break_fd, PERF_EVENT_IOC_ENABLE); + temp64 = *((__u64 *)target); + *((__u64 *)target) = temp64; + ioctl(break_fd, PERF_EVENT_IOC_DISABLE); + res = read(break_fd, &breaks, sizeof(unsigned long long)); + assert(res == sizeof(unsigned long long)); + if (breaks == 2) { + printf("TESTED: Full overlap\n"); + } else { + printf("FAILED: Full overlap: %lld != 2\n", breaks); + fail = 1; + } + + free(target); + close(break_fd); + return fail; +} + static int runtest(void) { int rwflag; @@ -172,7 +287,9 @@ static int runtest(void) return ret; } } - return 0; + + ret = runtest_dar_outside(); + return ret; } -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 6/7] Powerpc/Watchpoint: Add dar outside test in perf-hwbreak.c selftest @ 2019-10-17 9:32 ` Ravi Bangoria 0 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:32 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: Ravi Bangoria, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev So far we used to ignore exception if dar points outside of user specified range. But now we are ignoring it only if actual load/ store range does not overlap with user specified range. Include selftests for the same: # ./tools/testing/selftests/powerpc/ptrace/perf-hwbreak ... TESTED: No overlap TESTED: Partial overlap TESTED: Partial overlap TESTED: No overlap TESTED: Full overlap success: perf_hwbreak Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- .../selftests/powerpc/ptrace/perf-hwbreak.c | 119 +++++++++++++++++- 1 file changed, 118 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c b/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c index 200337daec42..c1f324afdbf3 100644 --- a/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c +++ b/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c @@ -148,6 +148,121 @@ static int runtestsingle(int readwriteflag, int exclude_user, int arraytest) return 0; } +static int runtest_dar_outside(void) +{ + void *target; + volatile __u16 temp16; + volatile __u64 temp64; + struct perf_event_attr attr; + int break_fd; + unsigned long long breaks; + int fail = 0; + size_t res; + + target = malloc(8); + if (!target) { + perror("malloc failed"); + exit(EXIT_FAILURE); + } + + /* setup counters */ + memset(&attr, 0, sizeof(attr)); + attr.disabled = 1; + attr.type = PERF_TYPE_BREAKPOINT; + attr.exclude_kernel = 1; + attr.exclude_hv = 1; + attr.exclude_guest = 1; + attr.bp_type = HW_BREAKPOINT_RW; + /* watch middle half of target array */ + attr.bp_addr = (__u64)(target + 2); + attr.bp_len = 4; + break_fd = sys_perf_event_open(&attr, 0, -1, -1, 0); + if (break_fd < 0) { + free(target); + perror("sys_perf_event_open"); + exit(EXIT_FAILURE); + } + + /* Shouldn't hit. */ + ioctl(break_fd, PERF_EVENT_IOC_RESET); + ioctl(break_fd, PERF_EVENT_IOC_ENABLE); + temp16 = *((__u16 *)target); + *((__u16 *)target) = temp16; + ioctl(break_fd, PERF_EVENT_IOC_DISABLE); + res = read(break_fd, &breaks, sizeof(unsigned long long)); + assert(res == sizeof(unsigned long long)); + if (breaks == 0) { + printf("TESTED: No overlap\n"); + } else { + printf("FAILED: No overlap: %lld != 0\n", breaks); + fail = 1; + } + + /* Hit */ + ioctl(break_fd, PERF_EVENT_IOC_RESET); + ioctl(break_fd, PERF_EVENT_IOC_ENABLE); + temp16 = *((__u16 *)(target + 1)); + *((__u16 *)(target + 1)) = temp16; + ioctl(break_fd, PERF_EVENT_IOC_DISABLE); + res = read(break_fd, &breaks, sizeof(unsigned long long)); + assert(res == sizeof(unsigned long long)); + if (breaks == 2) { + printf("TESTED: Partial overlap\n"); + } else { + printf("FAILED: Partial overlap: %lld != 2\n", breaks); + fail = 1; + } + + /* Hit */ + ioctl(break_fd, PERF_EVENT_IOC_RESET); + ioctl(break_fd, PERF_EVENT_IOC_ENABLE); + temp16 = *((__u16 *)(target + 5)); + *((__u16 *)(target + 5)) = temp16; + ioctl(break_fd, PERF_EVENT_IOC_DISABLE); + res = read(break_fd, &breaks, sizeof(unsigned long long)); + assert(res == sizeof(unsigned long long)); + if (breaks == 2) { + printf("TESTED: Partial overlap\n"); + } else { + printf("FAILED: Partial overlap: %lld != 2\n", breaks); + fail = 1; + } + + /* Shouldn't Hit */ + ioctl(break_fd, PERF_EVENT_IOC_RESET); + ioctl(break_fd, PERF_EVENT_IOC_ENABLE); + temp16 = *((__u16 *)(target + 6)); + *((__u16 *)(target + 6)) = temp16; + ioctl(break_fd, PERF_EVENT_IOC_DISABLE); + res = read(break_fd, &breaks, sizeof(unsigned long long)); + assert(res == sizeof(unsigned long long)); + if (breaks == 0) { + printf("TESTED: No overlap\n"); + } else { + printf("FAILED: No overlap: %lld != 0\n", breaks); + fail = 1; + } + + /* Hit */ + ioctl(break_fd, PERF_EVENT_IOC_RESET); + ioctl(break_fd, PERF_EVENT_IOC_ENABLE); + temp64 = *((__u64 *)target); + *((__u64 *)target) = temp64; + ioctl(break_fd, PERF_EVENT_IOC_DISABLE); + res = read(break_fd, &breaks, sizeof(unsigned long long)); + assert(res == sizeof(unsigned long long)); + if (breaks == 2) { + printf("TESTED: Full overlap\n"); + } else { + printf("FAILED: Full overlap: %lld != 2\n", breaks); + fail = 1; + } + + free(target); + close(break_fd); + return fail; +} + static int runtest(void) { int rwflag; @@ -172,7 +287,9 @@ static int runtest(void) return ret; } } - return 0; + + ret = runtest_dar_outside(); + return ret; } -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 7/7] Powerpc/Watchpoint: Support for 8xx in ptrace-hwbreak.c selftest 2019-10-17 9:31 ` Ravi Bangoria @ 2019-10-17 9:32 ` Ravi Bangoria -1 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:32 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: npiggin, benh, paulus, naveen.n.rao, linuxppc-dev, linux-kernel, Ravi Bangoria On the 8xx, signals are generated after executing the instruction. So no need to manually single-step on 8xx. Also, 8xx __set_dabr() currently ignores length and hardcodes the length to 8 bytes. So all unaligned and 512 byte testcase will fail on 8xx. Ignore those testcases on 8xx. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- .../selftests/powerpc/ptrace/ptrace-hwbreak.c | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c b/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c index 916e97f5f8b1..7deedbc16b0b 100644 --- a/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c +++ b/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c @@ -22,6 +22,11 @@ #include <sys/wait.h> #include "ptrace.h" +#define SPRN_PVR 0x11F +#define PVR_8xx 0x00500000 + +bool is_8xx; + /* * Use volatile on all global var so that compiler doesn't * optimise their load/stores. Otherwise selftest can fail. @@ -205,13 +210,15 @@ static void check_success(pid_t child_pid, const char *name, const char *type, printf("%s, %s, len: %d: Ok\n", name, type, len); - /* - * For ptrace registered watchpoint, signal is generated - * before executing load/store. Singlestep the instruction - * and then continue the test. - */ - ptrace(PTRACE_SINGLESTEP, child_pid, NULL, 0); - wait(NULL); + if (!is_8xx) { + /* + * For ptrace registered watchpoint, signal is generated + * before executing load/store. Singlestep the instruction + * and then continue the test. + */ + ptrace(PTRACE_SINGLESTEP, child_pid, NULL, 0); + wait(NULL); + } } static void ptrace_set_debugreg(pid_t child_pid, unsigned long wp_addr) @@ -447,8 +454,10 @@ run_tests(pid_t child_pid, struct ppc_debug_info *dbginfo, bool dawr) test_set_debugreg(child_pid); if (dbginfo->features & PPC_DEBUG_FEATURE_DATA_BP_RANGE) { test_sethwdebug_exact(child_pid); - test_sethwdebug_range_aligned(child_pid); - if (dawr) { + + if (!is_8xx) + test_sethwdebug_range_aligned(child_pid); + if (dawr && !is_8xx) { test_sethwdebug_range_unaligned(child_pid); test_sethwdebug_range_unaligned_dar(child_pid); test_sethwdebug_dawr_max_range(child_pid); @@ -489,5 +498,10 @@ static int ptrace_hwbreak(void) int main(int argc, char **argv, char **envp) { + int pvr = 0; + asm __volatile__ ("mfspr %0,%1" : "=r"(pvr) : "i"(SPRN_PVR)); + if (pvr == PVR_8xx) + is_8xx = true; + return test_harness(ptrace_hwbreak, "ptrace-hwbreak"); } -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 7/7] Powerpc/Watchpoint: Support for 8xx in ptrace-hwbreak.c selftest @ 2019-10-17 9:32 ` Ravi Bangoria 0 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-17 9:32 UTC (permalink / raw) To: christophe.leroy, mpe, mikey Cc: Ravi Bangoria, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev On the 8xx, signals are generated after executing the instruction. So no need to manually single-step on 8xx. Also, 8xx __set_dabr() currently ignores length and hardcodes the length to 8 bytes. So all unaligned and 512 byte testcase will fail on 8xx. Ignore those testcases on 8xx. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com> --- .../selftests/powerpc/ptrace/ptrace-hwbreak.c | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c b/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c index 916e97f5f8b1..7deedbc16b0b 100644 --- a/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c +++ b/tools/testing/selftests/powerpc/ptrace/ptrace-hwbreak.c @@ -22,6 +22,11 @@ #include <sys/wait.h> #include "ptrace.h" +#define SPRN_PVR 0x11F +#define PVR_8xx 0x00500000 + +bool is_8xx; + /* * Use volatile on all global var so that compiler doesn't * optimise their load/stores. Otherwise selftest can fail. @@ -205,13 +210,15 @@ static void check_success(pid_t child_pid, const char *name, const char *type, printf("%s, %s, len: %d: Ok\n", name, type, len); - /* - * For ptrace registered watchpoint, signal is generated - * before executing load/store. Singlestep the instruction - * and then continue the test. - */ - ptrace(PTRACE_SINGLESTEP, child_pid, NULL, 0); - wait(NULL); + if (!is_8xx) { + /* + * For ptrace registered watchpoint, signal is generated + * before executing load/store. Singlestep the instruction + * and then continue the test. + */ + ptrace(PTRACE_SINGLESTEP, child_pid, NULL, 0); + wait(NULL); + } } static void ptrace_set_debugreg(pid_t child_pid, unsigned long wp_addr) @@ -447,8 +454,10 @@ run_tests(pid_t child_pid, struct ppc_debug_info *dbginfo, bool dawr) test_set_debugreg(child_pid); if (dbginfo->features & PPC_DEBUG_FEATURE_DATA_BP_RANGE) { test_sethwdebug_exact(child_pid); - test_sethwdebug_range_aligned(child_pid); - if (dawr) { + + if (!is_8xx) + test_sethwdebug_range_aligned(child_pid); + if (dawr && !is_8xx) { test_sethwdebug_range_unaligned(child_pid); test_sethwdebug_range_unaligned_dar(child_pid); test_sethwdebug_dawr_max_range(child_pid); @@ -489,5 +498,10 @@ static int ptrace_hwbreak(void) int main(int argc, char **argv, char **envp) { + int pvr = 0; + asm __volatile__ ("mfspr %0,%1" : "=r"(pvr) : "i"(SPRN_PVR)); + if (pvr == PVR_8xx) + is_8xx = true; + return test_harness(ptrace_hwbreak, "ptrace-hwbreak"); } -- 2.21.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v6 0/7] Powerpc/Watchpoint: Few important fixes 2019-10-17 9:31 ` Ravi Bangoria @ 2019-10-29 4:54 ` Ravi Bangoria -1 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-29 4:54 UTC (permalink / raw) To: christophe.leroy Cc: mpe, mikey, npiggin, benh, paulus, naveen.n.rao, linuxppc-dev, linux-kernel, Ravi Bangoria On 10/17/19 3:01 PM, Ravi Bangoria wrote: > v5: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198069.html > > v5->v6: > - patch 6/7: mpe reported that the perf-hwbreak.c doesn't compile with older > gcc: > perf-hwbreak.c:182:2: error: dereferencing type-punned pointer will > break strict-aliasing rules [-Werror=strict-aliasing] > temp16 = *((__u16 *)target); > ^ > Fixed that. > Hi Christophe, Are you ok with the series wrt 8xx? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6 0/7] Powerpc/Watchpoint: Few important fixes @ 2019-10-29 4:54 ` Ravi Bangoria 0 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-10-29 4:54 UTC (permalink / raw) To: christophe.leroy Cc: Ravi Bangoria, mikey, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev On 10/17/19 3:01 PM, Ravi Bangoria wrote: > v5: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198069.html > > v5->v6: > - patch 6/7: mpe reported that the perf-hwbreak.c doesn't compile with older > gcc: > perf-hwbreak.c:182:2: error: dereferencing type-punned pointer will > break strict-aliasing rules [-Werror=strict-aliasing] > temp16 = *((__u16 *)target); > ^ > Fixed that. > Hi Christophe, Are you ok with the series wrt 8xx? ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6 0/7] Powerpc/Watchpoint: Few important fixes 2019-10-29 4:54 ` Ravi Bangoria @ 2019-10-29 14:01 ` Christophe Leroy -1 siblings, 0 replies; 23+ messages in thread From: Christophe Leroy @ 2019-10-29 14:01 UTC (permalink / raw) To: Ravi Bangoria Cc: mpe, mikey, npiggin, benh, paulus, naveen.n.rao, linuxppc-dev, linux-kernel Le 29/10/2019 à 05:54, Ravi Bangoria a écrit : > > > On 10/17/19 3:01 PM, Ravi Bangoria wrote: >> v5: >> https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198069.html >> >> v5->v6: >> - patch 6/7: mpe reported that the perf-hwbreak.c doesn't compile >> with older >> gcc: >> perf-hwbreak.c:182:2: error: dereferencing type-punned >> pointer will >> break strict-aliasing rules [-Werror=strict-aliasing] >> temp16 = *((__u16 *)target); >> ^ >> Fixed that. >> > > Hi Christophe, Are you ok with the series wrt 8xx? Yes it looks ok on my 885: root@vgoip:~# ./ptrace-hwbreak test: ptrace-hwbreak tags: git_version:v5.4-rc4-835-gb235e63aa9f0-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 success: ptrace-hwbreak Also ok on book3s/32: root@vgoippro:~# ./ptrace-hwbreak test: ptrace-hwbreak tags: git_version:v5.4-rc4-835-gb235e63aa9f0-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 success: ptrace-hwbreak Christophe ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6 0/7] Powerpc/Watchpoint: Few important fixes @ 2019-10-29 14:01 ` Christophe Leroy 0 siblings, 0 replies; 23+ messages in thread From: Christophe Leroy @ 2019-10-29 14:01 UTC (permalink / raw) To: Ravi Bangoria Cc: mikey, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev Le 29/10/2019 à 05:54, Ravi Bangoria a écrit : > > > On 10/17/19 3:01 PM, Ravi Bangoria wrote: >> v5: >> https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198069.html >> >> v5->v6: >> - patch 6/7: mpe reported that the perf-hwbreak.c doesn't compile >> with older >> gcc: >> perf-hwbreak.c:182:2: error: dereferencing type-punned >> pointer will >> break strict-aliasing rules [-Werror=strict-aliasing] >> temp16 = *((__u16 *)target); >> ^ >> Fixed that. >> > > Hi Christophe, Are you ok with the series wrt 8xx? Yes it looks ok on my 885: root@vgoip:~# ./ptrace-hwbreak test: ptrace-hwbreak tags: git_version:v5.4-rc4-835-gb235e63aa9f0-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 success: ptrace-hwbreak Also ok on book3s/32: root@vgoippro:~# ./ptrace-hwbreak test: ptrace-hwbreak tags: git_version:v5.4-rc4-835-gb235e63aa9f0-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 success: ptrace-hwbreak Christophe ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6 0/7] Powerpc/Watchpoint: Few important fixes 2019-10-29 14:01 ` Christophe Leroy @ 2019-11-07 14:30 ` Ravi Bangoria -1 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-11-07 14:30 UTC (permalink / raw) To: mpe Cc: Christophe Leroy, mikey, npiggin, benh, paulus, naveen.n.rao, linuxppc-dev, linux-kernel, Ravi Bangoria On 10/29/19 7:31 PM, Christophe Leroy wrote: > > > Le 29/10/2019 à 05:54, Ravi Bangoria a écrit : >> >> >> On 10/17/19 3:01 PM, Ravi Bangoria wrote: >>> v5: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198069.html >>> >>> v5->v6: >>> - patch 6/7: mpe reported that the perf-hwbreak.c doesn't compile with older >>> gcc: >>> perf-hwbreak.c:182:2: error: dereferencing type-punned pointer will >>> break strict-aliasing rules [-Werror=strict-aliasing] >>> temp16 = *((__u16 *)target); >>> ^ >>> Fixed that. >>> >> >> Hi Christophe, Are you ok with the series wrt 8xx? > > Yes it looks ok on my 885: > > root@vgoip:~# ./ptrace-hwbreak > test: ptrace-hwbreak > tags: git_version:v5.4-rc4-835-gb235e63aa9f0-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 > success: ptrace-hwbreak > > Also ok on book3s/32: mpe, Can you please pull the series. Ravi ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6 0/7] Powerpc/Watchpoint: Few important fixes @ 2019-11-07 14:30 ` Ravi Bangoria 0 siblings, 0 replies; 23+ messages in thread From: Ravi Bangoria @ 2019-11-07 14:30 UTC (permalink / raw) To: mpe Cc: Ravi Bangoria, mikey, linux-kernel, npiggin, paulus, naveen.n.rao, linuxppc-dev On 10/29/19 7:31 PM, Christophe Leroy wrote: > > > Le 29/10/2019 à 05:54, Ravi Bangoria a écrit : >> >> >> On 10/17/19 3:01 PM, Ravi Bangoria wrote: >>> v5: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-October/198069.html >>> >>> v5->v6: >>> - patch 6/7: mpe reported that the perf-hwbreak.c doesn't compile with older >>> gcc: >>> perf-hwbreak.c:182:2: error: dereferencing type-punned pointer will >>> break strict-aliasing rules [-Werror=strict-aliasing] >>> temp16 = *((__u16 *)target); >>> ^ >>> Fixed that. >>> >> >> Hi Christophe, Are you ok with the series wrt 8xx? > > Yes it looks ok on my 885: > > root@vgoip:~# ./ptrace-hwbreak > test: ptrace-hwbreak > tags: git_version:v5.4-rc4-835-gb235e63aa9f0-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 > success: ptrace-hwbreak > > Also ok on book3s/32: mpe, Can you please pull the series. Ravi ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2019-11-14 9:07 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-17 9:31 [PATCH v6 0/7] Powerpc/Watchpoint: Few important fixes Ravi Bangoria 2019-10-17 9:31 ` Ravi Bangoria 2019-10-17 9:31 ` [PATCH v6 1/7] Powerpc/Watchpoint: Introduce macros for watchpoint length Ravi Bangoria 2019-10-17 9:31 ` Ravi Bangoria 2019-11-14 9:07 ` Michael Ellerman 2019-10-17 9:31 ` [PATCH v6 2/7] Powerpc/Watchpoint: Fix length calculation for unaligned target Ravi Bangoria 2019-10-17 9:31 ` Ravi Bangoria 2019-10-17 9:32 ` [PATCH v6 3/7] Powerpc/Watchpoint: Fix ptrace code that muck around with address/len Ravi Bangoria 2019-10-17 9:32 ` Ravi Bangoria 2019-10-17 9:32 ` [PATCH v6 4/7] Powerpc/Watchpoint: Don't ignore extraneous exceptions blindly Ravi Bangoria 2019-10-17 9:32 ` Ravi Bangoria 2019-10-17 9:32 ` [PATCH v6 5/7] Powerpc/Watchpoint: Rewrite ptrace-hwbreak.c selftest Ravi Bangoria 2019-10-17 9:32 ` Ravi Bangoria 2019-10-17 9:32 ` [PATCH v6 6/7] Powerpc/Watchpoint: Add dar outside test in perf-hwbreak.c selftest Ravi Bangoria 2019-10-17 9:32 ` Ravi Bangoria 2019-10-17 9:32 ` [PATCH v6 7/7] Powerpc/Watchpoint: Support for 8xx in ptrace-hwbreak.c selftest Ravi Bangoria 2019-10-17 9:32 ` Ravi Bangoria 2019-10-29 4:54 ` [PATCH v6 0/7] Powerpc/Watchpoint: Few important fixes Ravi Bangoria 2019-10-29 4:54 ` Ravi Bangoria 2019-10-29 14:01 ` Christophe Leroy 2019-10-29 14:01 ` Christophe Leroy 2019-11-07 14:30 ` Ravi Bangoria 2019-11-07 14:30 ` Ravi Bangoria
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.