From 9b6f4e8e80e9347e27901192df39c4fe66e28162 Mon Sep 17 00:00:00 2001 From: Kulyatskaya Alexandra Date: Fri, 11 Jul 2025 18:08:08 +0300 Subject: [PATCH 1/3] [backport] target: support auto-selection of software breakpoint size Add default breakpoint length specifier. When passed, derivation of breakpoint length is delegated to target-type-specific method. Link: https://review.openocd.org/c/openocd/+/9116 Change-Id: Ie3473514beace15b76714aa6d5441122cd3262aa Signed-off-by: Kulyatskaya Alexandra --- doc/openocd.texi | 5 +++-- src/target/target.c | 36 +++++++++++++++++++++++++++++++----- src/target/target_type.h | 3 +++ 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/doc/openocd.texi b/doc/openocd.texi index 5d6622cae1..c00b393746 100644 --- a/doc/openocd.texi +++ b/doc/openocd.texi @@ -9640,10 +9640,11 @@ hardware support for a handful of code breakpoints and data watchpoints. In addition, CPUs almost always support software breakpoints. -@deffn {Command} {bp} [address len [@option{hw}]] +@deffn {Command} {bp} [address (len|@option{default}) [@option{hw}]] With no parameters, lists all active breakpoints. Else sets a breakpoint on code execution starting -at @var{address} for @var{length} bytes. +at @var{address} for @var{length} bytes. When @option{default} is passed +the length is derived automatically. This is a software breakpoint, unless @option{hw} is specified in which case it will be a hardware breakpoint. diff --git a/src/target/target.c b/src/target/target.c index 0c6c9c4433..3f24fd5cd6 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -1310,6 +1310,14 @@ int target_add_breakpoint(struct target *target, return target->type->add_breakpoint(target, breakpoint); } +static int target_get_default_breakpoint_size(struct target *target, target_addr_t addr, + uint32_t asid, int hw, unsigned int *length) +{ + if (!target->type->get_default_breakpoint_length) + return ERROR_NOT_IMPLEMENTED; + return target->type->get_default_breakpoint_length(target, addr, asid, hw, length); +} + int target_add_context_breakpoint(struct target *target, struct breakpoint *breakpoint) { @@ -3981,6 +3989,24 @@ static int handle_bp_command_set(struct command_invocation *cmd, return retval; } +static COMMAND_HELPER(parse_bp_length, uint32_t asid, target_addr_t addr, + int hw, unsigned int *length) +{ + if (strcmp(CMD_ARGV[1], "default") == 0) { + struct target *target = get_current_target(CMD_CTX); + int ret_errno = target_get_default_breakpoint_size(target, addr, asid, hw, length); + if (ret_errno == ERROR_NOT_IMPLEMENTED) { + command_print(CMD, "Default breakpoint size derivation is " + "not implemented on target %s", target_name(target)); + } else if (ret_errno != ERROR_OK) { + command_print(CMD, "Unknown error when deriving default breakpoint size"); + } + return ret_errno; + } + COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], *length); + return ERROR_OK; +} + COMMAND_HANDLER(handle_bp_command) { target_addr_t addr; @@ -3995,21 +4021,21 @@ COMMAND_HANDLER(handle_bp_command) case 2: asid = 0; COMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr); - COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], length); + CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); return handle_bp_command_set(CMD, addr, asid, length, hw); case 3: if (strcmp(CMD_ARGV[2], "hw") == 0) { hw = BKPT_HARD; COMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr); - COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], length); asid = 0; + CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); return handle_bp_command_set(CMD, addr, asid, length, hw); } else if (strcmp(CMD_ARGV[2], "hw_ctx") == 0) { hw = BKPT_HARD; COMMAND_PARSE_NUMBER(u32, CMD_ARGV[0], asid); - COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], length); addr = 0; + CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); return handle_bp_command_set(CMD, addr, asid, length, hw); } /* fallthrough */ @@ -4017,7 +4043,7 @@ COMMAND_HANDLER(handle_bp_command) hw = BKPT_HARD; COMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr); COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], asid); - COMMAND_PARSE_NUMBER(u32, CMD_ARGV[2], length); + CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); return handle_bp_command_set(CMD, addr, asid, length, hw); default: @@ -6662,7 +6688,7 @@ static const struct command_registration target_exec_command_handlers[] = { .handler = handle_bp_command, .mode = COMMAND_EXEC, .help = "list or set hardware or software breakpoint", - .usage = "[
[] ['hw'|'hw_ctx']]", + .usage = "[
[] (|'default') ['hw'|'hw_ctx']]", }, { .name = "rbp", diff --git a/src/target/target_type.h b/src/target/target_type.h index d72fac394d..911bc8a839 100644 --- a/src/target/target_type.h +++ b/src/target/target_type.h @@ -309,6 +309,9 @@ struct target_type { * will typically be 32 for 32-bit targets, and 64 for 64-bit targets. If * not implemented, it's assumed to be 32. */ unsigned int (*data_bits)(struct target *target); + + int (*get_default_breakpoint_length)(struct target *target, target_addr_t addr, + uint32_t asid, int hw, unsigned int *length); }; extern struct target_type aarch64_target; From 501c3ea548a153890981a07eeef1a6ad72cbe186 Mon Sep 17 00:00:00 2001 From: Kulyatskaya Alexandra Date: Wed, 16 Jul 2025 19:05:38 +0300 Subject: [PATCH 2/3] [backport] target/riscv/riscv.c: support auto-selection of software breakpoint size Link: https://github.com/riscv-collab/riscv-openocd/pull/1288 Add auto-selection of breakpoint size for RISC-V targets. The new length depends on C extension. If the C extension is used, the auto-selected length is 2 bytes. Otherwise, the default length is 4 bytes. Change-Id: I0cf4e2a06b202b61048322ee61a06101cb55a23a Signed-off-by: Kulyatskaya Alexandra --- src/target/riscv/riscv.c | 8 ++++++++ src/target/target.c | 41 ++++++++++++++++++++++++---------------- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 46bee5b391..8f18e673d8 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1595,6 +1595,13 @@ int riscv_read_by_any_size(struct target *target, target_addr_t address, uint32_ return ERROR_FAIL; } +static int riscv_get_default_breakpoint_length(struct target *target, target_addr_t addr, + uint32_t asid, int hw, unsigned int *length) +{ + *length = riscv_supports_extension(target, 'c') ? 2 : 4; + return ERROR_OK; +} + int riscv_add_breakpoint(struct target *target, struct breakpoint *breakpoint) { LOG_TARGET_DEBUG(target, "@0x%" TARGET_PRIxADDR, breakpoint->address); @@ -5889,6 +5896,7 @@ struct target_type riscv_target = { .get_gdb_reg_list = riscv_get_gdb_reg_list, .get_gdb_reg_list_noread = riscv_get_gdb_reg_list_noread, + .get_default_breakpoint_length = riscv_get_default_breakpoint_length, .add_breakpoint = riscv_add_breakpoint, .remove_breakpoint = riscv_remove_breakpoint, diff --git a/src/target/target.c b/src/target/target.c index 3f24fd5cd6..574c07f82f 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -3992,19 +3992,19 @@ static int handle_bp_command_set(struct command_invocation *cmd, static COMMAND_HELPER(parse_bp_length, uint32_t asid, target_addr_t addr, int hw, unsigned int *length) { - if (strcmp(CMD_ARGV[1], "default") == 0) { - struct target *target = get_current_target(CMD_CTX); - int ret_errno = target_get_default_breakpoint_size(target, addr, asid, hw, length); - if (ret_errno == ERROR_NOT_IMPLEMENTED) { - command_print(CMD, "Default breakpoint size derivation is " - "not implemented on target %s", target_name(target)); - } else if (ret_errno != ERROR_OK) { - command_print(CMD, "Unknown error when deriving default breakpoint size"); - } - return ret_errno; + if (strcmp(CMD_ARGV[1], "default") != 0) { + COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], *length); + return ERROR_OK; } - COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], *length); - return ERROR_OK; + struct target *target = get_current_target(CMD_CTX); + int ret_errno = target_get_default_breakpoint_size(target, addr, asid, hw, length); + if (ret_errno == ERROR_NOT_IMPLEMENTED) { + command_print(CMD, "Default breakpoint size derivation is " + "not implemented on target %s", target_name(target)); + } else if (ret_errno != ERROR_OK) { + command_print(CMD, "Unknown error when deriving default breakpoint size"); + } + return ret_errno; } COMMAND_HANDLER(handle_bp_command) @@ -4013,6 +4013,7 @@ COMMAND_HANDLER(handle_bp_command) uint32_t asid; uint32_t length; int hw = BKPT_SOFT; + int ret_errno; switch (CMD_ARGC) { case 0: @@ -4021,7 +4022,9 @@ COMMAND_HANDLER(handle_bp_command) case 2: asid = 0; COMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr); - CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); + ret_errno = CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); + if (ret_errno != ERROR_OK) + return ret_errno; return handle_bp_command_set(CMD, addr, asid, length, hw); case 3: @@ -4029,13 +4032,17 @@ COMMAND_HANDLER(handle_bp_command) hw = BKPT_HARD; COMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr); asid = 0; - CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); + ret_errno = CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); + if (ret_errno != ERROR_OK) + return ret_errno; return handle_bp_command_set(CMD, addr, asid, length, hw); } else if (strcmp(CMD_ARGV[2], "hw_ctx") == 0) { hw = BKPT_HARD; COMMAND_PARSE_NUMBER(u32, CMD_ARGV[0], asid); addr = 0; - CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); + ret_errno = CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); + if (ret_errno != ERROR_OK) + return ret_errno; return handle_bp_command_set(CMD, addr, asid, length, hw); } /* fallthrough */ @@ -4043,7 +4050,9 @@ COMMAND_HANDLER(handle_bp_command) hw = BKPT_HARD; COMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr); COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], asid); - CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); + ret_errno = CALL_COMMAND_HANDLER(parse_bp_length, asid, addr, hw, &length); + if (ret_errno != ERROR_OK) + return ret_errno; return handle_bp_command_set(CMD, addr, asid, length, hw); default: From d434542bf4cd2f46beda93dfa2750f2a0744d080 Mon Sep 17 00:00:00 2001 From: Kulyatskaya Alexandra Date: Thu, 4 Sep 2025 17:23:46 +0300 Subject: [PATCH 3/3] [syntacore] src/target/riscv: Add default breakpoint size for scr5 Reuse the default RISC-V implementation. Change-Id: I9641155c5ff263d0397036428a73e2009bc340a2 Signed-off-by: Kulyatskaya Alexandra --- src/target/riscv/riscv.c | 2 +- src/target/riscv/riscv.h | 2 ++ src/target/riscv/scr5.c | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 8f18e673d8..29d1cd7311 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1595,7 +1595,7 @@ int riscv_read_by_any_size(struct target *target, target_addr_t address, uint32_ return ERROR_FAIL; } -static int riscv_get_default_breakpoint_length(struct target *target, target_addr_t addr, +int riscv_get_default_breakpoint_length(struct target *target, target_addr_t addr, uint32_t asid, int hw, unsigned int *length) { *length = riscv_supports_extension(target, 'c') ? 2 : 4; diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 6ed4168ed7..f536a8cc06 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -551,5 +551,7 @@ int riscv_write_by_any_size(struct target *target, target_addr_t address, uint32 int riscv_interrupts_disable(struct target *target, uint64_t ie_mask, uint64_t *old_mstatus); int riscv_interrupts_restore(struct target *target, uint64_t old_mstatus); +int riscv_get_default_breakpoint_length(struct target *target, target_addr_t addr, + uint32_t asid, int hw, unsigned int *length); #endif /* OPENOCD_TARGET_RISCV_RISCV_H */ diff --git a/src/target/riscv/scr5.c b/src/target/riscv/scr5.c index 15946053a7..29fa44f1fc 100644 --- a/src/target/riscv/scr5.c +++ b/src/target/riscv/scr5.c @@ -320,6 +320,7 @@ struct target_type scr5_target = { .get_gdb_arch = riscv_get_gdb_arch, .get_gdb_reg_list = riscv_get_gdb_reg_list, .get_gdb_reg_list_noread = riscv_get_gdb_reg_list_noread, + .get_default_breakpoint_length = riscv_get_default_breakpoint_length, .add_breakpoint = riscv_add_breakpoint, .remove_breakpoint = riscv_remove_breakpoint, .add_watchpoint = riscv_add_watchpoint,