ARM64: More flexible HW watchpoint
Subsystems (4)
v3 cover ARM64: More flexible HW watchpoint
Currently, we do not support all the byte select option provided by ARM64 specs for a HW watchpoint.
This patch set will help user to instrument a watchpoint with all possible byte select options.
Changes since v2:
- used __ffs() instead of ffs() - 1. Similarly for fls().
- fixed ptrace_hbp_get_addr() to report correct address to user space
- handling stepping for inexact watchpoint as well now.
Changes since v1:
- Introduced a new patch 3/5 where it takes care of the situation when HW does not report a watchpoint hit with the address that matches one of the watchpoints set.
- Added corresponding test case to test that functionality.
Pavel Labath (1):
arm64: hw_breakpoint: Handle inexact watchpoint addresses
Pratyush Anand (4):
hw_breakpoint: Allow watchpoint of length 3,5,6 and 7
arm64: Allow hw watchpoint at varied offset from base address
arm64: Allow hw watchpoint of length 3,5,6 and 7
selftests: arm64: add test for unaligned/inexact watchpoint handling
arch/arm64/include/asm/hw_breakpoint.h | 6 +- arch/arm64/kernel/hw_breakpoint.c | 153 +++++++++---- arch/arm64/kernel/ptrace.c | 7 +- include/uapi/linux/hw_breakpoint.h | 4 + tools/include/uapi/linux/hw_breakpoint.h | 4 + tools/testing/selftests/breakpoints/Makefile | 5 +- .../selftests/breakpoints/breakpoint_test_arm64.c | 236 +++++++++++++++++++++ 7 files changed, 372 insertions(+), 43 deletions(-) create mode 100644 tools/testing/selftests/breakpoints/breakpoint_test_arm64.c
Signature · 1 line
2.7.4
PATCH v3 1/5 hw_breakpoint: Allow watchpoint of length 3,5,6 and 7
We only support breakpoint/watchpoint of length 1, 2, 4 and 8. If we can support other length as well, then user may watch more data with less number of watchpoints (provided hardware supports it). For example: if we have to watch only 4th, 5th and 6th byte from a 64 bit aligned address, we will have to use two slots to implement it currently. One slot will watch a half word at offset 4 and other a byte at offset 6. If we can have a watchpoint of length 3 then we can watch it with single slot as well.
ARM64 hardware does support such functionality, therefore adding these new definitions in generic layer.
--- include/uapi/linux/hw_breakpoint.h | 4 ++++ tools/include/uapi/linux/hw_breakpoint.h | 4 ++++ 2 files changed, 8 insertions(+)
include/uapi/linux/hw_breakpoint.h +4 -0
@@ -4,7 +4,11 @@enum {HW_BREAKPOINT_LEN_1 =1 ,HW_BREAKPOINT_LEN_2 =2 , +HW_BREAKPOINT_LEN_3 =3 ,HW_BREAKPOINT_LEN_4 =4 , +HW_BREAKPOINT_LEN_5 =5 , +HW_BREAKPOINT_LEN_6 =6 , +HW_BREAKPOINT_LEN_7 =7 ,HW_BREAKPOINT_LEN_8 =8 , };
tools/include/uapi/linux/hw_breakpoint.h +4 -0
@@ -4,7 +4,11 @@enum {HW_BREAKPOINT_LEN_1 =1 ,HW_BREAKPOINT_LEN_2 =2 , +HW_BREAKPOINT_LEN_3 =3 ,HW_BREAKPOINT_LEN_4 =4 , +HW_BREAKPOINT_LEN_5 =5 , +HW_BREAKPOINT_LEN_6 =6 , +HW_BREAKPOINT_LEN_7 =7 ,HW_BREAKPOINT_LEN_8 =8 , };
Signature · 1 line
2.7.4
PATCH v3 2/5 arm64: Allow hw watchpoint at varied offset from base address
In reply to Pratyush AnandARM64 hardware supports watchpoint at any double word aligned address. However, it can select any consecutive bytes from offset 0 to 7 from that base address. For example, if base address is programmed as 0x420030 and byte select is 0x1C, then access of 0x420032,0x420033 and 0x420034 will generate a watchpoint exception.
Currently, we do not have such modularity. We can only program byte, halfword, word and double word access exception from any base address.
This patch adds support to overcome above limitations.
--- arch/arm64/include/asm/hw_breakpoint.h | 2 +- arch/arm64/kernel/hw_breakpoint.c | 47 +++++++++++++++++----------------- arch/arm64/kernel/ptrace.c | 7 ++--- 3 files changed, 28 insertions(+), 28 deletions(-)
arch/arm64/include/asm/hw_breakpoint.h +1 -1
@@ -119,7 +119,7 @@ struct perf_event;struct pmu ;extern int arch_bp_generic_fields (struct arch_hw_breakpoint_ctrl ctrl, -int *gen_len,int *gen_type); +int *gen_len,int *gen_type,int *offset);extern int arch_check_bp_in_kernelspace (struct perf_event *bp);extern int arch_validate_hwbkpt_settings (struct perf_event *bp); extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
arch/arm64/kernel/hw_breakpoint.c +23 -24
89 lines not on this page · raw message
arch/arm64/kernel/ptrace.c +4 -3
@@ -327,13 +327,13 @@ static int ptrace_hbp_fill_attr_ctrl(unsigned int note_type,struct arch_hw_breakpoint_ctrl ctrl, struct perf_event_attr *attr) { - int err, len, type, disabled = !ctrl.enabled; + int err, len, type, offset, disabled = !ctrl.enabled; attr->disabled = disabled;if (disabled)return 0 ; - err =arch_bp_generic_fields (ctrl, &len, &type); + err =arch_bp_generic_fields (ctrl, &len, &type, &offset);if (err)return err;
@@ -352,6 +352,7 @@ static int ptrace_hbp_fill_attr_ctrl(unsigned int note_type, attr->bp_len = len; attr->bp_type = type; + attr->bp_addr += offset;return 0 ; }
@@ -404,7 +405,7 @@ static int ptrace_hbp_get_addr(unsigned int note_type,if (IS_ERR (bp))return PTR_ERR (bp); - *addr = bp ? bp->attr.bp_addr :0 ; + *addr = bp ?counter_arch_bp (bp)->address :0 ;return 0 ; }
Signature · 1 line
2.7.4
PATCH v3 3/5 arm64: hw_breakpoint: Handle inexact watchpoint addresses
In reply to Pratyush AnandFrom: Pavel Labath <[email protected]>
Arm64 hardware does not always report a watchpoint hit address that matches one of the watchpoints set. It can also report an address "near" the watchpoint if a single instruction access both watched and unwatched addresses. There is no straight-forward way, short of disassembling the offending instruction, to map that address back to the watchpoint.
Previously, when the hardware reported a watchpoint hit on an address that did not match our watchpoint (this happens in case of instructions which access large chunks of memory such as "stp") the process would enter a loop where we would be continually resuming it (because we did not recognise that watchpoint hit) and it would keep hitting the watchpoint again and again. The tracing process would never get notified of the watchpoint hit.
This commit fixes the problem by looking at the watchpoints near the address reported by the hardware. If the address does not exactly match one of the watchpoints we have set, it attributes the hit to the nearest watchpoint we have. This heuristic is a bit dodgy, but I don't think we can do much more, given the hardware limitations.
[panand: reworked to rebase on his patches]
--- arch/arm64/kernel/hw_breakpoint.c | 96 ++++++++++++++++++++++++++++----------- 1 file changed, 69 insertions(+), 27 deletions(-)
arch/arm64/kernel/hw_breakpoint.c +69 -27
@@ -664,11 +664,46 @@ static int breakpoint_handler(unsigned long unused, unsigned int esr, }NOKPROBE_SYMBOL (breakpoint_handler); +/* +* Arm64 hardware does not always report a watchpoint hit address that matches +* one of the watchpoints set. It can also report an address "near" the +* watchpoint if a single instruction access both watched and unwatched +* addresses. There is no straight-forward way, short of disassembling the +* offending instruction, to map that address back to the watchpoint. This +* function computes the distance of the memory access from the watchpoint as a +* heuristic for the likelyhood that a given access triggered the watchpoint. +* +* See Section D2.10.5 "Determining the memory location that caused a Watchpoint +* exception" of ARMv8 Architecture Reference Manual for details. +* +* The function returns the distance of the address from the bytes watched by +* the watchpoint. In case of an exact match, it returns 0. +*/ +static u64 get_distance_from_watchpoint (unsigned long addr,u64 val, +struct arch_hw_breakpoint_ctrl *ctrl) +{ +u64 wp_low, wp_high; +u32 lens, lene; + + lens =ffs (ctrl->len) -1 ; + lene =fls (ctrl->len) -1 ; + + wp_low = val + lens; + wp_high = val + lene; +if (addr < wp_low) +return wp_low - addr; +else if (addr > wp_high) +return addr - wp_high; +else +return 0 ; +} +static int watchpoint_handler (unsigned long addr,unsigned int esr,struct pt_regs *regs) { -int i, step =0 , *kernel_step, access; -u32 ctrl_reg, lens, lene; +int i, step =0 , *kernel_step, access, closest_match =0 ; +u64 min_dist =-1 , dist; +u32 ctrl_reg;u64 val;struct perf_event *wp, **slots;struct debug_info *debug_info;
@@ -678,31 +713,15 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr, slots =this_cpu_ptr (wp_on_reg); debug_info = ¤t->thread.debug; +/* +* Find all watchpoints that match the reported address. If no exact +* match is found. Attribute the hit to the closest watchpoint. +*/ +rcu_read_lock ();for (i =0 ; i < core_num_wrps; ++i) { -rcu_read_lock (); - wp = slots[i]; -if (wp ==NULL ) - goto unlock; - - info =counter_arch_bp (wp); - -/* Check if the watchpoint value and byte select match. */ - val =read_wb_reg (AARCH64_DBG_REG_WVR , i); - ctrl_reg =read_wb_reg (AARCH64_DBG_REG_WCR , i); -decode_ctrl_reg (ctrl_reg, &ctrl); - lens =ffs (ctrl.len) -1 ; - lene =fls (ctrl.len) -1 ; -/* -* FIXME: reported address can be anywhere between "the -* lowest address accessed by the memory access that -* triggered the watchpoint" and "the highest watchpointed -* address accessed by the memory access". So, it may not -* lie in the interval of watchpoint address range. -*/ -if (addr < val + lens || addr > val + lene) - goto unlock; +continue ;/* * Check that the access type matches.
@@ -711,18 +730,41 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr, access = (esr &AARCH64_ESR_ACCESS_MASK ) ?HW_BREAKPOINT_W :HW_BREAKPOINT_R ;if (!(access &hw_breakpoint_type (wp))) - goto unlock; +continue ; +/* Check if the watchpoint value and byte select match. */ + val =read_wb_reg (AARCH64_DBG_REG_WVR , i); + ctrl_reg =read_wb_reg (AARCH64_DBG_REG_WCR , i); +decode_ctrl_reg (ctrl_reg, &ctrl); + dist =get_distance_from_watchpoint (addr, val, &ctrl); +if (dist < min_dist) { + min_dist = dist; + closest_match = i; + } +/* Is this an exact match? */ +if (dist !=0 ) +continue ; + + info =counter_arch_bp (wp); info->trigger = addr;perf_bp_event (wp, regs);/* Do we need to handle the stepping? */ if (is_default_overflow_handler (wp)) step =1 ; + } +if (min_dist >0 && min_dist !=-1 ) { +/* No exact match found. */ + wp = slots[closest_match]; + info =counter_arch_bp (wp); + info->trigger = addr; +perf_bp_event (wp, regs); -unlock: -rcu_read_unlock (); +/* Do we need to handle the stepping? */ +if (is_default_overflow_handler (wp)) + step =1 ; } +rcu_read_unlock ();if (!step)return 0 ;
Signature · 1 line
2.7.4
PATCH v3 4/5 arm64: Allow hw watchpoint of length 3,5,6 and 7
In reply to Pratyush AnandSince, arm64 can support all offset within a double word limit. Therefore, now support other lengths within that range as well.
--- arch/arm64/include/asm/hw_breakpoint.h | 4 ++++ arch/arm64/kernel/hw_breakpoint.c | 36 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+)
arch/arm64/include/asm/hw_breakpoint.h +4 -0
@@ -77,7 +77,11 @@ static inline void decode_ctrl_reg(u32 reg,/* Lengths */ #define ARM_BREAKPOINT_LEN_1 0x1#define ARM_BREAKPOINT_LEN_2 0x3 +#define ARM_BREAKPOINT_LEN_3 0x7#define ARM_BREAKPOINT_LEN_4 0xf +#define ARM_BREAKPOINT_LEN_5 0x1f +#define ARM_BREAKPOINT_LEN_6 0x3f +#define ARM_BREAKPOINT_LEN_7 0x7f#define ARM_BREAKPOINT_LEN_8 0xff/* Kernel stepping */
arch/arm64/kernel/hw_breakpoint.c +36 -0
@@ -317,9 +317,21 @@ static int get_hbp_len(u8 hbp_len)case ARM_BREAKPOINT_LEN_2 : len_in_bytes =2 ;break ; +case ARM_BREAKPOINT_LEN_3 : + len_in_bytes =3 ; +break ;case ARM_BREAKPOINT_LEN_4 : len_in_bytes =4 ;break ; +case ARM_BREAKPOINT_LEN_5 : + len_in_bytes =5 ; +break ; +case ARM_BREAKPOINT_LEN_6 : + len_in_bytes =6 ; +break ; +case ARM_BREAKPOINT_LEN_7 : + len_in_bytes =7 ; +break ;case ARM_BREAKPOINT_LEN_8 : len_in_bytes =8 ;break ;
@@ -381,9 +393,21 @@ int arch_bp_generic_fields(struct arch_hw_breakpoint_ctrl ctrl,case ARM_BREAKPOINT_LEN_2 : *gen_len =HW_BREAKPOINT_LEN_2 ;break ; +case ARM_BREAKPOINT_LEN_3 : + *gen_len =HW_BREAKPOINT_LEN_3 ; +break ;case ARM_BREAKPOINT_LEN_4 : *gen_len =HW_BREAKPOINT_LEN_4 ;break ; +case ARM_BREAKPOINT_LEN_5 : + *gen_len =HW_BREAKPOINT_LEN_5 ; +break ; +case ARM_BREAKPOINT_LEN_6 : + *gen_len =HW_BREAKPOINT_LEN_6 ; +break ; +case ARM_BREAKPOINT_LEN_7 : + *gen_len =HW_BREAKPOINT_LEN_7 ; +break ;case ARM_BREAKPOINT_LEN_8 : *gen_len =HW_BREAKPOINT_LEN_8 ;break ;
@@ -427,9 +451,21 @@ static int arch_build_bp_info(struct perf_event *bp)case HW_BREAKPOINT_LEN_2 : info->ctrl.len =ARM_BREAKPOINT_LEN_2 ;break ; +case HW_BREAKPOINT_LEN_3 : + info->ctrl.len =ARM_BREAKPOINT_LEN_3 ; +break ;case HW_BREAKPOINT_LEN_4 : info->ctrl.len =ARM_BREAKPOINT_LEN_4 ;break ; +case HW_BREAKPOINT_LEN_5 : + info->ctrl.len =ARM_BREAKPOINT_LEN_5 ; +break ; +case HW_BREAKPOINT_LEN_6 : + info->ctrl.len =ARM_BREAKPOINT_LEN_6 ; +break ; +case HW_BREAKPOINT_LEN_7 : + info->ctrl.len =ARM_BREAKPOINT_LEN_7 ; +break ;case HW_BREAKPOINT_LEN_8 : info->ctrl.len =ARM_BREAKPOINT_LEN_8 ;break ;
Signature · 1 line
2.7.4
PATCH v3 5/5 selftests: arm64: add test for unaligned/inexact watchpoint handling
In reply to Pratyush AnandARM64 hardware expects 64bit aligned address for watchpoint invocation. However, it provides byte selection method to select any number of consecutive byte set within the range of 1-8.
This patch adds support to test all such byte selection option for different memory write sizes.
Patch also adds a test for handling the case when the cpu does not report an address which exactly matches one of the regions we have been watching (which is a situation permitted by the spec if an instruction accesses both watched and unwatched regions). The test was failing on a MSM8996pro before this patch series and is passing now.
--- tools/testing/selftests/breakpoints/Makefile | 5 +- .../selftests/breakpoints/breakpoint_test_arm64.c | 236 +++++++++++++++++++++ 2 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/breakpoints/breakpoint_test_arm64.c
tools/testing/selftests/breakpoints/Makefile +4 -1
@@ -5,6 +5,9 @@ ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/) ifeq ($(ARCH),x86) TEST_PROGS := breakpoint_test endif +ifeq ($(ARCH),aarch64) +TEST_PROGS := breakpoint_test_arm64 +endif TEST_PROGS += step_after_suspend_test
@@ -13,4 +16,4 @@ all: $(TEST_PROGS) include ../lib.mk clean: - rm -fr breakpoint_test step_after_suspend_test + rm -fr breakpoint_test breakpoint_test_arm64 step_after_suspend_test
tools/testing/selftests/breakpoints/breakpoint_test_arm64.c new file +236 -0
236 lines not on this page · raw message
Signature · 1 line
2.7.4