This is an automated email from Gerrit. "Alexandra Kulyatskaya <[email protected]>" just uploaded a new patch set to Gerrit, which you can find at https://review.openocd.org/c/openocd/+/9493
-- gerrit commit 8ba529b48c588cb80b087ff96787b328f2fb470a Author: Kulyatskaya Alexandra <[email protected]> Date: Thu Mar 5 16:36:46 2026 +0300 [src/target] Redesign bp/wp interface - Redesign breakpoint command interface - Add support for named arguments (-hw, -address, -length, -asid) - Implement default breakpoint length handling Change-Id: If6bd5ed347195e458fab6275a10f0d2071d55b6a Signed-off-by: Kulyatskaya Alexandra <[email protected]> diff --git a/doc/openocd.texi b/doc/openocd.texi index 9540e69f3f..7bc910d509 100644 --- a/doc/openocd.texi +++ b/doc/openocd.texi @@ -9892,11 +9892,14 @@ hardware support for a handful of code breakpoints and data watchpoints. In addition, CPUs almost always support software breakpoints. -@deffn {Command} {bp} [address [asid] len [@option{hw} | @option{hw_ctx}]] +@deffn {Command} {bp} [[['-addr'] address] [['-length'] length] @ +['-hw'] [['-asid'] asid]] With no parameters, lists all active breakpoints. Else sets a breakpoint on code execution starting -at @var{address} for @var{length} bytes. -This is a software breakpoint, unless @option{hw} or @option{hw_ctx} +at @var{address} for @var{length} bytes. When a parameter is not specified, +it can be derived automatically, provided that the target supports this +feature. +This is a software breakpoint, unless @option{-hw} is specified in which case it will be a hardware, context or hybrid breakpoint. The context and hybrid breakpoints require an additional parameter @var{asid}: address space identifier. diff --git a/src/helper/types.h b/src/helper/types.h index 8b02d213b1..d6c4ea9006 100644 --- a/src/helper/types.h +++ b/src/helper/types.h @@ -285,4 +285,12 @@ typedef uint64_t target_addr_t; #define TARGET_PRIXADDR PRIX64 #define TARGET_ADDR_FMT "0x%8.8" TARGET_PRIxADDR +enum target_bp_param { + BP_ADDRESS, + BP_LENGTH, + BP_ASID, + BP_HW, + BP_N_OPTS +}; + #endif /* OPENOCD_HELPER_TYPES_H */ diff --git a/src/target/target.c b/src/target/target.c index bdf0ff244d..19afd6ccb3 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -1495,6 +1495,9 @@ static int target_init_one(struct command_context *cmd_ctx, if (!type->check_reset) type->check_reset = default_check_reset; + if (!type->set_default_breakpoint_params) + type->set_default_breakpoint_params = set_default_breakpoint_params; + assert(type->init_target); int retval = type->init_target(cmd_ctx, target); @@ -3918,6 +3921,24 @@ COMMAND_HANDLER(handle_test_image_command) return CALL_COMMAND_HANDLER(handle_verify_image_command_internal, IMAGE_TEST); } +static int set_default_breakpoint_hw(struct set_bp_opts bp_params[static BP_N_OPTS]) +{ + if (!bp_params[BP_HW].is_set) { + bp_params[BP_HW].hw = bp_params[BP_ASID].is_set ? + BKPT_HARD : BKPT_SOFT; + bp_params[BP_HW].is_set = true; + } + return ERROR_OK; +} + +COMMAND_HELPER(set_default_breakpoint_params, + const struct target *target, + struct set_bp_opts bp_params[static BP_N_OPTS]) +{ + int retval = set_default_breakpoint_hw(bp_params); + return retval; +} + static int handle_bp_command_list(struct command_invocation *cmd) { struct target *target = get_current_target(cmd->ctx); @@ -3953,83 +3974,132 @@ static int handle_bp_command_list(struct command_invocation *cmd) return ERROR_OK; } -static int handle_bp_command_set(struct command_invocation *cmd, - target_addr_t addr, uint32_t asid, unsigned int length, int hw) +static const struct nvp bp_config_opts[] = { + { .name = "-addr", BP_ADDRESS }, + { .name = "-length", BP_LENGTH }, + { .name = "-asid", BP_ASID }, + { .name = "-hw", BP_HW }, + { .name = "hw", BP_HW }, + { .name = NULL, -1 } +}; + +static COMMAND_HELPER(handle_bp_command_set, + struct set_bp_opts bp_params[static BP_N_OPTS]) { - struct target *target = get_current_target(cmd->ctx); + struct target *target = get_current_target(CMD->ctx); int retval; - if (asid == 0) { - retval = breakpoint_add(target, addr, length, hw); + if (!bp_params[BP_LENGTH].is_set || !bp_params[BP_HW].is_set) { + command_print(CMD, "-length and -hw parameters are required"); + return ERROR_COMMAND_ARGUMENT_INVALID; + } + + if (!bp_params[BP_ASID].is_set) { + if (!bp_params[BP_ADDRESS].is_set) { + command_print(CMD, "can't locate breakpoint without -adress"); + return ERROR_COMMAND_ARGUMENT_INVALID; + } + retval = breakpoint_add(target, bp_params[BP_ADDRESS].addr, + bp_params[BP_LENGTH].length, bp_params[BP_HW].hw); /* error is always logged in breakpoint_add(), do not print it again */ if (retval == ERROR_OK) - command_print(cmd, "breakpoint set at " TARGET_ADDR_FMT "", addr); + command_print(CMD, "breakpoint set at " TARGET_ADDR_FMT "", + bp_params[BP_ADDRESS].addr); - } else if (addr == 0) { + } else if (!bp_params[BP_ADDRESS].is_set) { if (!target->type->add_context_breakpoint) { LOG_TARGET_ERROR(target, "Context breakpoint not available"); return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; } - retval = context_breakpoint_add(target, asid, length, hw); + retval = context_breakpoint_add(target, bp_params[BP_ASID].asid, + bp_params[BP_LENGTH].length, bp_params[BP_HW].hw); + /* error is always logged in context_breakpoint_add(), do not print it again */ if (retval == ERROR_OK) - command_print(cmd, "Context breakpoint set at 0x%8.8" PRIx32, asid); + command_print(CMD, "Context breakpoint set at 0x%8.8" PRIx32 "", + bp_params[BP_ASID].asid); + } else { if (!target->type->add_hybrid_breakpoint) { LOG_TARGET_ERROR(target, "Hybrid breakpoint not available"); return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; } - retval = hybrid_breakpoint_add(target, addr, asid, length, hw); + retval = hybrid_breakpoint_add(target, bp_params[BP_ADDRESS].addr, + bp_params[BP_ASID].asid, bp_params[BP_LENGTH].length, + bp_params[BP_HW].hw); /* error is always logged in hybrid_breakpoint_add(), do not print it again */ if (retval == ERROR_OK) - command_print(cmd, "Hybrid breakpoint set at 0x%8.8" PRIx32, asid); + command_print(CMD, "Hybrid breakpoint set at 0x%8.8" PRIx32 "", + bp_params[BP_ASID].asid); + } return retval; } COMMAND_HANDLER(handle_bp_command) { - target_addr_t addr; - uint32_t asid; - uint32_t length; - int hw = BKPT_SOFT; + struct set_bp_opts bp_params[BP_N_OPTS] = {{.is_set = false}}; + int retval; - switch (CMD_ARGC) { - case 0: + if (CMD_ARGC == 0) return handle_bp_command_list(CMD); - case 2: - asid = 0; - COMMAND_PARSE_ADDRESS(CMD_ARGV[0], addr); - COMMAND_PARSE_NUMBER(u32, CMD_ARGV[1], length); - return handle_bp_command_set(CMD, addr, asid, length, hw); + for (unsigned int index = 0; index < CMD_ARGC; ) { + const struct nvp *n = nvp_name2value(bp_config_opts, CMD_ARGV[index]); + if (!n->name) { + if (!bp_params[BP_ADDRESS].is_set) { + COMMAND_PARSE_ADDRESS(CMD_ARGV[index++], + bp_params[BP_ADDRESS].addr); + bp_params[BP_ADDRESS].is_set = true; + } else if (!bp_params[BP_LENGTH].is_set) { + COMMAND_PARSE_NUMBER(u32, CMD_ARGV[index++], + bp_params[BP_LENGTH].length); + bp_params[BP_LENGTH].is_set = true; + } else { + return ERROR_COMMAND_SYNTAX_ERROR; + } + continue; + } - 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; - 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; - return handle_bp_command_set(CMD, addr, asid, length, hw); + if (bp_params[n->value].is_set) { + command_print(CMD, "Two identical options: %s", n->name); + return ERROR_COMMAND_SYNTAX_ERROR; + } + bp_params[n->value].is_set = true; + if (++index == CMD_ARGC && n->value != BP_HW) { + command_print(CMD, "Option \"%s\" expect arg", n->name); + return ERROR_COMMAND_SYNTAX_ERROR; } - /* fallthrough */ - case 4: - 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); - return handle_bp_command_set(CMD, addr, asid, length, hw); - default: - return ERROR_COMMAND_SYNTAX_ERROR; + switch (n->value) { + case BP_ADDRESS: + COMMAND_PARSE_ADDRESS(CMD_ARGV[index++], + bp_params[BP_ADDRESS].addr); + break; + case BP_LENGTH: + COMMAND_PARSE_NUMBER(u32, CMD_ARGV[index++], + bp_params[BP_LENGTH].length); + break; + case BP_ASID: + COMMAND_PARSE_NUMBER(u32, CMD_ARGV[index++], + bp_params[BP_ASID].asid); + break; + case BP_HW: + bp_params[BP_HW].hw = BKPT_HARD; + break; + default: + assert(false); + return ERROR_FAIL; + } } + + struct target *target = get_current_target(CMD_CTX); + retval = CALL_COMMAND_HANDLER(target->type->set_default_breakpoint_params, + target, bp_params); + + return (retval == ERROR_OK) ? + CALL_COMMAND_HANDLER(handle_bp_command_set, bp_params) : retval; } COMMAND_HANDLER(handle_rbp_command) @@ -6653,7 +6723,8 @@ 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 = "[<address> [<asid>] <length> ['hw'|'hw_ctx']]", + .usage = "[[['-addr'] address] [['-length'] length] ['-hw']" + "[['-asid'] asid]]", }, { .name = "rbp", diff --git a/src/target/target.h b/src/target/target.h index b850b49cf3..ca00437788 100644 --- a/src/target/target.h +++ b/src/target/target.h @@ -24,6 +24,8 @@ #include "helper/replacements.h" #include "helper/system.h" #include <helper/types.h> +#include "helper/command.h" +#include "helper/types.h" #include <jim.h> struct reg; @@ -803,4 +805,18 @@ extern bool get_target_reset_nag(void); const char *target_debug_reason_str(enum target_debug_reason reason); +struct set_bp_opts { + bool is_set; + union{ + target_addr_t addr; + uint32_t asid; + uint32_t length; + int hw; + }; +}; + +COMMAND_HELPER(set_default_breakpoint_params, + const struct target *target, + struct set_bp_opts bp_params[static BP_N_OPTS]); + #endif /* OPENOCD_TARGET_TARGET_H */ diff --git a/src/target/target_type.h b/src/target/target_type.h index ccbe03a476..1dbef9acf1 100644 --- a/src/target/target_type.h +++ b/src/target/target_type.h @@ -15,8 +15,11 @@ #define OPENOCD_TARGET_TARGET_TYPE_H #include <helper/jim-nvp.h> +#include "helper/command.h" +#include "helper/types.h" struct target; +struct set_bp_opts; /** * This holds methods shared between all instances of a given target @@ -305,6 +308,15 @@ 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); + + /* Set default breakpoint parameters for the target. + * This function is called when a breakpoint command does not specify all + * options. It fills the provided bp_params array with + * architecture‑specific default values. + */ + COMMAND_HELPER((*set_default_breakpoint_params), + const struct target *target, + struct set_bp_opts bp_params[static BP_N_OPTS]); }; // Keep in alphabetic order this list of targets --
