From: OpenOCD-Gerrit <ope...@us...> - 2021-05-22 09:05:27
|
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "Main OpenOCD repository". The branch, master has been updated via 15dd48119a3564df2ceb5512fc2b62ca42f43614 (commit) via 510df38407da25aa0fa7427d26b9253455885d9b (commit) from 936cff887abf116427802c4f07096fca1f2b1d88 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- commit 15dd48119a3564df2ceb5512fc2b62ca42f43614 Author: Antonio Borneo <bor...@gm...> Date: Wed May 5 15:05:21 2021 +0200 target/aarch64: fix watchpoint management The early documentation for armv8a report the debug register WFAR as containing the address of the instruction that triggered the watchpoint. More recent documentation report the register EDWAR as containing the data memory address that triggered the watchpoint. The name of macros CPUV8_DBG_WFAR0 and CPUV8_DBG_WFAR1 is not correct as they point to the debug register EDWAR, so reading such register returns directly the data memory address that triggered the watchpoint. The code incorrectly passes this address value to the function armv8_dpm_report_wfar(); this function is supposed to adjust the PC value, decrementing it to remove the effects of the CPU pipeline. This pipeline offset, that has no meaning on the value in EDWAR, caused commit 651b861d5d5f ("target/aarch64: Add watchpoint support") to add back the offset while comparing the address with the watchpoint enabled. The upper 32 bits of EDWAR are not valid in aarch32 mode and have to be ignored. Rename CPUV8_DBG_WFAR0/1 as CPUV8_DBG_EDWAR0/1. Remove the function armv8_dpm_report_wfar(). Remove the offset while searching the matching watchpoint. Ignore the upper 32 bits of EDWAR in aarch32 mode. Fix a comment and the LOG text. Change-Id: I7cbdbeb766fa18e31cc72be098ca2bc501877ed1 Signed-off-by: Antonio Borneo <bor...@gm...> Reviewed-on: http://openocd.zylin.com/6205 Tested-by: jenkins Reviewed-by: Liming Sun <li...@nv...> diff --git a/src/target/aarch64.c b/src/target/aarch64.c index e6eb2cafd..612924f5a 100644 --- a/src/target/aarch64.c +++ b/src/target/aarch64.c @@ -988,25 +988,26 @@ static int aarch64_debug_entry(struct target *target) /* Examine debug reason */ armv8_dpm_report_dscr(dpm, dscr); - /* save address of instruction that triggered the watchpoint? */ + /* save the memory address that triggered the watchpoint */ if (target->debug_reason == DBG_REASON_WATCHPOINT) { uint32_t tmp; - uint64_t wfar = 0; retval = mem_ap_read_atomic_u32(armv8->debug_ap, - armv8->debug_base + CPUV8_DBG_WFAR1, - &tmp); + armv8->debug_base + CPUV8_DBG_EDWAR0, &tmp); if (retval != ERROR_OK) return retval; - wfar = tmp; - wfar = (wfar << 32); - retval = mem_ap_read_atomic_u32(armv8->debug_ap, - armv8->debug_base + CPUV8_DBG_WFAR0, - &tmp); - if (retval != ERROR_OK) - return retval; - wfar |= tmp; - armv8_dpm_report_wfar(&armv8->dpm, wfar); + target_addr_t edwar = tmp; + + /* EDWAR[63:32] has unknown content in aarch32 state */ + if (core_state == ARM_STATE_AARCH64) { + retval = mem_ap_read_atomic_u32(armv8->debug_ap, + armv8->debug_base + CPUV8_DBG_EDWAR1, &tmp); + if (retval != ERROR_OK) + return retval; + edwar |= ((target_addr_t)tmp) << 32; + } + + armv8->dpm.wp_addr = edwar; } retval = armv8_dpm_read_current_registers(&armv8->dpm); @@ -1853,7 +1854,7 @@ int aarch64_hit_watchpoint(struct target *target, struct armv8_common *armv8 = target_to_armv8(target); - uint64_t exception_address; + target_addr_t exception_address; struct watchpoint *wp; exception_address = armv8->dpm.wp_addr; @@ -1861,23 +1862,11 @@ int aarch64_hit_watchpoint(struct target *target, if (exception_address == 0xFFFFFFFF) return ERROR_FAIL; - /**********************************************************/ - /* see if a watchpoint address matches a value read from */ - /* the EDWAR register. Testing shows that on some ARM CPUs*/ - /* the EDWAR value needs to have 8 added to it so we add */ - /* that check as well not sure if that is a core bug) */ - /**********************************************************/ - for (exception_address = armv8->dpm.wp_addr; exception_address <= (armv8->dpm.wp_addr + 8); - exception_address += 8) { - for (wp = target->watchpoints; wp; wp = wp->next) { - if ((exception_address >= wp->address) && (exception_address < (wp->address + wp->length))) { - *hit_watchpoint = wp; - if (exception_address != armv8->dpm.wp_addr) - LOG_DEBUG("watchpoint hit required EDWAR to be increased by 8"); - return ERROR_OK; - } + for (wp = target->watchpoints; wp; wp = wp->next) + if (exception_address >= wp->address && exception_address < (wp->address + wp->length)) { + *hit_watchpoint = wp; + return ERROR_OK; } - } return ERROR_FAIL; } diff --git a/src/target/armv8.c b/src/target/armv8.c index e209e8896..0c5cf346e 100644 --- a/src/target/armv8.c +++ b/src/target/armv8.c @@ -1169,7 +1169,7 @@ int armv8_arch_state(struct target *target) armv8_show_fault_registers(target); if (target->debug_reason == DBG_REASON_WATCHPOINT) - LOG_USER("Watchpoint triggered at PC " TARGET_ADDR_FMT, armv8->dpm.wp_addr); + LOG_USER("Watchpoint triggered at " TARGET_ADDR_FMT, armv8->dpm.wp_addr); return ERROR_OK; } diff --git a/src/target/armv8.h b/src/target/armv8.h index 978b2ad4a..f09f3abfd 100644 --- a/src/target/armv8.h +++ b/src/target/armv8.h @@ -257,8 +257,8 @@ static inline bool is_armv8(struct armv8_common *armv8) #define CPUV8_DBG_EDESR 0x20 #define CPUV8_DBG_EDECR 0x24 -#define CPUV8_DBG_WFAR0 0x30 -#define CPUV8_DBG_WFAR1 0x34 +#define CPUV8_DBG_EDWAR0 0x30 +#define CPUV8_DBG_EDWAR1 0x34 #define CPUV8_DBG_DSCR 0x088 #define CPUV8_DBG_DRCR 0x090 #define CPUV8_DBG_ECCR 0x098 diff --git a/src/target/armv8_dpm.c b/src/target/armv8_dpm.c index a7b4f1d38..6ef8c33de 100644 --- a/src/target/armv8_dpm.c +++ b/src/target/armv8_dpm.c @@ -1279,27 +1279,6 @@ static int dpmv8_remove_watchpoint(struct target *target, struct watchpoint *wp) return retval; } -void armv8_dpm_report_wfar(struct arm_dpm *dpm, uint64_t addr) -{ - switch (dpm->arm->core_state) { - case ARM_STATE_ARM: - case ARM_STATE_AARCH64: - addr -= 8; - break; - case ARM_STATE_THUMB: - case ARM_STATE_THUMB_EE: - addr -= 4; - break; - case ARM_STATE_JAZELLE: - /* ?? */ - break; - default: - LOG_DEBUG("Unknown core_state"); - break; - } - dpm->wp_addr = addr; -} - /* * Handle exceptions taken in debug state. This happens mostly for memory * accesses that violated a MMU policy. Taking an exception while in debug diff --git a/src/target/armv8_dpm.h b/src/target/armv8_dpm.h index a6cade345..c30b04ffa 100644 --- a/src/target/armv8_dpm.h +++ b/src/target/armv8_dpm.h @@ -38,8 +38,6 @@ int armv8_dpm_modeswitch(struct arm_dpm *dpm, enum arm_mode mode); int armv8_dpm_write_dirty_registers(struct arm_dpm *dpm, bool bpwp); -void armv8_dpm_report_wfar(struct arm_dpm *dpm, uint64_t wfar); - /* DSCR bits; see ARMv7a arch spec section C10.3.1. * Not all v7 bits are valid in v6. */ commit 510df38407da25aa0fa7427d26b9253455885d9b Author: Antonio Borneo <bor...@gm...> Date: Wed May 5 12:27:11 2021 +0200 target/arm_dpm: rename 'wp_pc' as 'wp_addr' The field 'wp_pc' was originally introduced in commit 55eeea7fceb6 ("ARMv7a/Cortex-A8: report watchpoint trigger insn") in end 2009 to contain the address of the instruction which triggered a watchpoint. Later on with commit 651b861d5d5f ("target/aarch64: Add watchpoint support") it has been reused in to hold directly the memory address that triggered a watchpoint. Rename 'wp_pc' as 'wp_addr' and change its doxygen description. While there, fix the format string to print the field. Change-Id: I2e5ced1497e4a6fb6b38f91e881807512e8d8c47 Signed-off-by: Antonio Borneo <bor...@gm...> Reviewed-on: http://openocd.zylin.com/6204 Tested-by: jenkins Reviewed-by: Liming Sun <li...@nv...> diff --git a/src/target/aarch64.c b/src/target/aarch64.c index 4ba92c8a0..e6eb2cafd 100644 --- a/src/target/aarch64.c +++ b/src/target/aarch64.c @@ -1856,7 +1856,7 @@ int aarch64_hit_watchpoint(struct target *target, uint64_t exception_address; struct watchpoint *wp; - exception_address = armv8->dpm.wp_pc; + exception_address = armv8->dpm.wp_addr; if (exception_address == 0xFFFFFFFF) return ERROR_FAIL; @@ -1867,12 +1867,12 @@ int aarch64_hit_watchpoint(struct target *target, /* the EDWAR value needs to have 8 added to it so we add */ /* that check as well not sure if that is a core bug) */ /**********************************************************/ - for (exception_address = armv8->dpm.wp_pc; exception_address <= (armv8->dpm.wp_pc + 8); + for (exception_address = armv8->dpm.wp_addr; exception_address <= (armv8->dpm.wp_addr + 8); exception_address += 8) { for (wp = target->watchpoints; wp; wp = wp->next) { if ((exception_address >= wp->address) && (exception_address < (wp->address + wp->length))) { *hit_watchpoint = wp; - if (exception_address != armv8->dpm.wp_pc) + if (exception_address != armv8->dpm.wp_addr) LOG_DEBUG("watchpoint hit required EDWAR to be increased by 8"); return ERROR_OK; } diff --git a/src/target/arm11.c b/src/target/arm11.c index 68d4e1894..ff125d0ea 100644 --- a/src/target/arm11.c +++ b/src/target/arm11.c @@ -355,8 +355,7 @@ static int arm11_arch_state(struct target *target) /* REVISIT also display ARM11-specific MMU and cache status ... */ if (target->debug_reason == DBG_REASON_WATCHPOINT) - LOG_USER("Watchpoint triggered at PC %#08x", - (unsigned) arm11->dpm.wp_pc); + LOG_USER("Watchpoint triggered at PC " TARGET_ADDR_FMT, arm11->dpm.wp_addr); return retval; } diff --git a/src/target/arm_dpm.c b/src/target/arm_dpm.c index 6bfe355ba..e172fad37 100644 --- a/src/target/arm_dpm.c +++ b/src/target/arm_dpm.c @@ -1010,7 +1010,7 @@ void arm_dpm_report_wfar(struct arm_dpm *dpm, uint32_t addr) /* ?? */ break; } - dpm->wp_pc = addr; + dpm->wp_addr = addr; } /*----------------------------------------------------------------------*/ diff --git a/src/target/arm_dpm.h b/src/target/arm_dpm.h index 82707822b..80587f5fe 100644 --- a/src/target/arm_dpm.h +++ b/src/target/arm_dpm.h @@ -137,8 +137,12 @@ struct arm_dpm { struct dpm_bp *dbp; struct dpm_wp *dwp; - /** Address of the instruction which triggered a watchpoint. */ - target_addr_t wp_pc; + /** + * Target dependent watchpoint address. + * Either the address of the instruction which triggered a watchpoint + * or the memory address whose access triggered a watchpoint. + */ + target_addr_t wp_addr; /** Recent value of DSCR. */ uint32_t dscr; diff --git a/src/target/armv7a.c b/src/target/armv7a.c index abca3358f..5c9f3085c 100644 --- a/src/target/armv7a.c +++ b/src/target/armv7a.c @@ -571,8 +571,7 @@ int armv7a_arch_state(struct target *target) if (arm->core_mode == ARM_MODE_ABT) armv7a_show_fault_registers(target); if (target->debug_reason == DBG_REASON_WATCHPOINT) - LOG_USER("Watchpoint triggered at PC %#08x", - (unsigned) armv7a->dpm.wp_pc); + LOG_USER("Watchpoint triggered at PC " TARGET_ADDR_FMT, armv7a->dpm.wp_addr); return ERROR_OK; } diff --git a/src/target/armv8.c b/src/target/armv8.c index 6bf3b110d..e209e8896 100644 --- a/src/target/armv8.c +++ b/src/target/armv8.c @@ -1169,8 +1169,7 @@ int armv8_arch_state(struct target *target) armv8_show_fault_registers(target); if (target->debug_reason == DBG_REASON_WATCHPOINT) - LOG_USER("Watchpoint triggered at PC %#08x", - (unsigned) armv8->dpm.wp_pc); + LOG_USER("Watchpoint triggered at PC " TARGET_ADDR_FMT, armv8->dpm.wp_addr); return ERROR_OK; } diff --git a/src/target/armv8_dpm.c b/src/target/armv8_dpm.c index e7d0f864e..a7b4f1d38 100644 --- a/src/target/armv8_dpm.c +++ b/src/target/armv8_dpm.c @@ -1297,7 +1297,7 @@ void armv8_dpm_report_wfar(struct arm_dpm *dpm, uint64_t addr) LOG_DEBUG("Unknown core_state"); break; } - dpm->wp_pc = addr; + dpm->wp_addr = addr; } /* ----------------------------------------------------------------------- Summary of changes: src/target/aarch64.c | 51 ++++++++++++++++++++------------------------------ src/target/arm11.c | 3 +-- src/target/arm_dpm.c | 2 +- src/target/arm_dpm.h | 8 ++++++-- src/target/armv7a.c | 3 +-- src/target/armv8.c | 3 +-- src/target/armv8.h | 4 ++-- src/target/armv8_dpm.c | 21 --------------------- src/target/armv8_dpm.h | 2 -- 9 files changed, 32 insertions(+), 65 deletions(-) hooks/post-receive -- Main OpenOCD repository |