Skip to content

Commit

Permalink
Merge pull request #977 from kr-sc/kr-sc/improve-riscv-controls
Browse files Browse the repository at this point in the history
target/riscv: Improve riscv controls that manage the set of available triggers for watchpoints
  • Loading branch information
en-sc authored Feb 27, 2024
2 parents 3c88a95 + 5003b36 commit ca7d882
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 88 deletions.
28 changes: 20 additions & 8 deletions doc/openocd.texi
Original file line number Diff line number Diff line change
Expand Up @@ -11379,16 +11379,28 @@ OpenOCD. When off, they generate a breakpoint exception handled internally.
The commands below can be used to prevent OpenOCD from using certain RISC-V trigger features.
For example in cases when there are known issues in the target hardware.

@deffn {Command} {riscv set_enable_eq_match_trigger} [on|off]
When on (default), allow OpenOCD to use exact-match triggers in watchpoints.
@end deffn
@deffn {Command} {riscv set_enable_trigger_feature} [(@option{eq}|@option{napot}|@option{ge_lt}|@option{all}) (@option{wp}|@option{none})]
Control which RISC-V trigger features can be used by OpenOCD placing watchpoints.
All trigger features are allowed by default. Only new watchpoints, inserted after this command,
are affected (watchpoints that were already placed before are not changed).

@deffn {Command} {riscv set_enable_napot_trigger} [on|off]
When on (default), allow OpenOCD to use NAPOT trigger in watchpoints.
@end deffn
The first argument selects one of the configurable RISC-V trigger features:

@itemize @minus
@item @option{eq}: Equality match trigger
@item @option{napot}: NAPOT trigger
@item @option{ge_lt}: Chained pair of `greater-equal` and `less-than` triggers
@item @option{all}: All trigger features which were described above
@end itemize

The second argument configures how OpenOCD should use the selected trigger feature:

@itemize @minus
@item @option{wp}: Enable this trigger feature for watchpoints - allow OpenOCD to use it. (Default.)
@item @option{none}: Disable the use of this trigger feature. OpenOCD will not attempt to use it.
@end itemize

@deffn {Command} {riscv set_enable_ge_lt_trigger} [on|off]
When on (default), allow OpenOCD to use a pair of chained less-than & greater-than triggers in watchpoints.
With no parameters, prints current trigger features configuration.
@end deffn

@subsection RISC-V Authentication Commands
Expand Down
142 changes: 65 additions & 77 deletions src/target/riscv/riscv.c
Original file line number Diff line number Diff line change
Expand Up @@ -1011,25 +1011,29 @@ static int maybe_add_trigger_t2_t6_for_wp(struct target *target,
struct trigger *trigger, struct match_triggers_tdata1_fields fields)
{
RISCV_INFO(r);
int ret = ERROR_FAIL;
int ret = ERROR_TARGET_RESOURCE_NOT_AVAILABLE;

if (trigger->length > 0) {
/* Setting a load/store trigger ("watchpoint") on a range of addresses */

if (r->enable_napot_trigger && can_use_napot_match(trigger)) {
LOG_TARGET_DEBUG(target, "trying to setup NAPOT match trigger");
struct trigger_request_info napot = {
.tdata1 = fields.common | fields.size.any |
fields.chain.disable | fields.match.napot,
.tdata2 = trigger->address | ((trigger->length - 1) >> 1),
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
ret = try_setup_single_match_trigger(target, trigger, napot);
if (ret != ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
return ret;
if (can_use_napot_match(trigger)) {
if (r->wp_allow_napot_trigger) {
LOG_TARGET_DEBUG(target, "trying to setup NAPOT match trigger");
struct trigger_request_info napot = {
.tdata1 = fields.common | fields.size.any |
fields.chain.disable | fields.match.napot,
.tdata2 = trigger->address | ((trigger->length - 1) >> 1),
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
ret = try_setup_single_match_trigger(target, trigger, napot);
if (ret != ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
return ret;
} else {
LOG_TARGET_DEBUG(target, "NAPOT match triggers are disabled for watchpoints. "
"Use 'riscv set_enable_trigger_feature napot wp' to enable it.");
}
}

if (r->enable_ge_lt_trigger) {
if (r->wp_allow_ge_lt_trigger) {
LOG_TARGET_DEBUG(target, "trying to setup GE+LT chained match trigger pair");
struct trigger_request_info ge_1 = {
.tdata1 = fields.common | fields.size.any | fields.chain.enable |
Expand Down Expand Up @@ -1063,10 +1067,13 @@ static int maybe_add_trigger_t2_t6_for_wp(struct target *target,
ret = try_setup_chained_match_triggers(target, trigger, lt_1, ge_2);
if (ret != ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
return ret;
} else {
LOG_TARGET_DEBUG(target, "LT+GE chained match triggers are disabled for watchpoints. "
"Use 'riscv set_enable_trigger_feature ge_lt wp' to enable it.");
}
}

if (r->enable_equality_match_trigger) {
if (r->wp_allow_equality_match_trigger) {
LOG_TARGET_DEBUG(target, "trying to setup equality match trigger");
struct trigger_request_info eq = {
.tdata1 = fields.common | fields.size.any | fields.chain.disable |
Expand All @@ -1077,6 +1084,9 @@ static int maybe_add_trigger_t2_t6_for_wp(struct target *target,
ret = try_setup_single_match_trigger(target, trigger, eq);
if (ret != ERROR_OK)
return ret;
} else {
LOG_TARGET_DEBUG(target, "equality match triggers are disabled for watchpoints. "
"Use 'riscv set_enable_trigger_feature eq wp' to enable it.");
}

if (ret == ERROR_OK && trigger->length > 1) {
Expand Down Expand Up @@ -4505,55 +4515,47 @@ COMMAND_HANDLER(riscv_exec_progbuf)
return ERROR_OK;
}

COMMAND_HANDLER(riscv_set_enable_eq_match_trigger)
COMMAND_HANDLER(riscv_set_enable_trigger_feature)
{
struct target *target = get_current_target(CMD_CTX);
RISCV_INFO(r);

if (CMD_ARGC == 0) {
command_print(CMD, "equality match trigger enabled: %s", r->enable_equality_match_trigger ? "on" : "off");
return ERROR_OK;
} else if (CMD_ARGC == 1) {
COMMAND_PARSE_ON_OFF(CMD_ARGV[0], r->enable_equality_match_trigger);
return ERROR_OK;
}

LOG_ERROR("Command takes 0 or 1 parameters");
return ERROR_COMMAND_SYNTAX_ERROR;
}
if (CMD_ARGC == 2) {
bool enable_for_wp = true;

COMMAND_HANDLER(riscv_set_enable_napot_trigger)
{
struct target *target = get_current_target(CMD_CTX);
RISCV_INFO(r);
if (!strcmp(CMD_ARGV[1], "wp"))
enable_for_wp = true;
else if (!strcmp(CMD_ARGV[1], "none"))
enable_for_wp = false;
else
return ERROR_COMMAND_SYNTAX_ERROR;

if (CMD_ARGC == 0) {
command_print(CMD, "NAPOT trigger enabled: %s", r->enable_napot_trigger ? "on" : "off");
return ERROR_OK;
} else if (CMD_ARGC == 1) {
COMMAND_PARSE_ON_OFF(CMD_ARGV[0], r->enable_napot_trigger);
return ERROR_OK;
if (!strcmp(CMD_ARGV[0], "all")) {
r->wp_allow_equality_match_trigger = enable_for_wp;
r->wp_allow_napot_trigger = enable_for_wp;
r->wp_allow_ge_lt_trigger = enable_for_wp;
} else if (!strcmp(CMD_ARGV[0], "eq")) {
r->wp_allow_equality_match_trigger = enable_for_wp;
} else if (!strcmp(CMD_ARGV[0], "napot")) {
r->wp_allow_napot_trigger = enable_for_wp;
} else if (!strcmp(CMD_ARGV[0], "ge_lt")) {
r->wp_allow_ge_lt_trigger = enable_for_wp;
} else {
return ERROR_COMMAND_SYNTAX_ERROR;
}
} else if (CMD_ARGC != 0) {
return ERROR_COMMAND_SYNTAX_ERROR;
}

LOG_ERROR("Command takes 0 or 1 parameters");
return ERROR_COMMAND_SYNTAX_ERROR;
}

COMMAND_HANDLER(riscv_set_enable_ge_lt_trigger)
{
struct target *target = get_current_target(CMD_CTX);
RISCV_INFO(r);

if (CMD_ARGC == 0) {
command_print(CMD, "ge-lt triggers enabled: %s", r->enable_ge_lt_trigger ? "on" : "off");
return ERROR_OK;
} else if (CMD_ARGC == 1) {
COMMAND_PARSE_ON_OFF(CMD_ARGV[0], r->enable_ge_lt_trigger);
return ERROR_OK;
}
command_print(CMD, "Triggers feature configuration:\n"
"Equality match trigger: for wp (%s)\n"
"NAPOT trigger: for wp (%s)\n"
"ge-lt chained triggers: for wp (%s)",
r->wp_allow_equality_match_trigger ? "enabled" : "disabled",
r->wp_allow_napot_trigger ? "enabled" : "disabled",
r->wp_allow_ge_lt_trigger ? "enabled" : "disabled");

LOG_ERROR("Command takes 0 or 1 parameters");
return ERROR_COMMAND_SYNTAX_ERROR;
return ERROR_OK;
}

static const struct command_registration riscv_exec_command_handlers[] = {
Expand Down Expand Up @@ -4806,25 +4808,11 @@ static const struct command_registration riscv_exec_command_handlers[] = {
"The final ebreak instruction is added automatically, if needed."
},
{
.name = "set_enable_eq_match_trigger",
.handler = riscv_set_enable_eq_match_trigger,
.mode = COMMAND_CONFIG,
.usage = "[on|off]",
.help = "When on, allow OpenOCD to use equality match trigger in wp."
},
{
.name = "set_enable_napot_trigger",
.handler = riscv_set_enable_napot_trigger,
.mode = COMMAND_CONFIG,
.usage = "[on|off]",
.help = "When on, allow OpenOCD to use NAPOT trigger in wp."
},
{
.name = "set_enable_ge_lt_trigger",
.handler = riscv_set_enable_ge_lt_trigger,
.mode = COMMAND_CONFIG,
.usage = "[on|off]",
.help = "When on, allow OpenOCD to use GE/LT triggers in wp."
.name = "set_enable_trigger_feature",
.handler = riscv_set_enable_trigger_feature,
.mode = COMMAND_ANY,
.usage = "[('eq'|'napot'|'ge_lt'|'all') ('wp'|'none')]",
.help = "Control whether OpenOCD is allowed to use certain RISC-V trigger features for watchpoints."
},
COMMAND_REGISTRATION_DONE
};
Expand Down Expand Up @@ -4962,9 +4950,9 @@ static void riscv_info_init(struct target *target, struct riscv_info *r)
r->riscv_ebreaks = true;
r->riscv_ebreaku = true;

r->enable_equality_match_trigger = true;
r->enable_ge_lt_trigger = true;
r->enable_napot_trigger = true;
r->wp_allow_equality_match_trigger = true;
r->wp_allow_ge_lt_trigger = true;
r->wp_allow_napot_trigger = true;
}

static int riscv_resume_go_all_harts(struct target *target)
Expand Down
6 changes: 3 additions & 3 deletions src/target/riscv/riscv.h
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,9 @@ struct riscv_info {
bool riscv_ebreaks;
bool riscv_ebreaku;

bool enable_equality_match_trigger;
bool enable_napot_trigger;
bool enable_ge_lt_trigger;
bool wp_allow_equality_match_trigger;
bool wp_allow_napot_trigger;
bool wp_allow_ge_lt_trigger;
};

COMMAND_HELPER(riscv_print_info_line, const char *section, const char *key,
Expand Down

0 comments on commit ca7d882

Please sign in to comment.