Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

provide riscv-specific controls to disable triggers from being used for watchpoints #917

Merged
merged 1 commit into from
Oct 11, 2023
+173 −44
Merged

provide riscv-specific controls to disable triggers from being used for watchpoints #917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions doc/openocd.texi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10871,6 +10871,21 @@ Control dcsr.ebreaku. When on (default), U-mode ebreak instructions trap to
OpenOCD. When off, they generate a breakpoint exception handled internally.
@end deffn

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_napot_trigger} [on|off]
When on (default), allow OpenOCD to use NAPOT trigger in watchpoints.
@end deffn

@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.
@end deffn

@subsection RISC-V Authentication Commands

The following commands can be used to authenticate to a RISC-V system. Eg. a
Expand Down
198 changes: 154 additions & 44 deletions src/target/riscv/riscv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -857,15 +857,16 @@ static struct match_triggers_tdata1_fields fill_match_triggers_tdata1_fields_t6(
return result;
}

static int maybe_add_trigger_t2_t6(struct target *target,
static int maybe_add_trigger_t2_t6_for_wp(struct target *target,
struct trigger *trigger, struct match_triggers_tdata1_fields fields)
{
int ret = ERROR_OK;
RISCV_INFO(r);
int ret = ERROR_FAIL;

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

if (can_use_napot_match(trigger)) {
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 |
Expand All @@ -877,52 +878,58 @@ static int maybe_add_trigger_t2_t6(struct target *target,
if (ret != ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
return ret;
}
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 |
fields.match.ge,
.tdata2 = trigger->address,
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
struct trigger_request_info lt_2 = {
.tdata1 = fields.common | fields.size.any | fields.chain.disable |
fields.match.lt,
.tdata2 = trigger->address + trigger->length,
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
ret = try_setup_chained_match_triggers(target, trigger, ge_1, lt_2);
if (ret != ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
return ret;

LOG_TARGET_DEBUG(target, "trying to setup LT+GE chained match trigger pair");
struct trigger_request_info lt_1 = {
.tdata1 = fields.common | fields.size.any | fields.chain.enable |
fields.match.lt,
.tdata2 = trigger->address + trigger->length,
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
struct trigger_request_info ge_2 = {
if (r->enable_ge_lt_trigger) {
Copy link
Collaborator

@aap-sc aap-sc Oct 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kr-sc , I'd like to request few additional improvements:

  1. enable_ge_lt_trigger should be renamed to wp_enable_ge_lt_trigger (this applies to other controls as well). This is to clearly identify that these controls are for watchpoints only.
  2. when trigger->length == 0 we should allow for equality match to be used regardless of this control (otherwise we won't be able to set such wathcpoints)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timsifive , @JanMatCodasip FYI

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 |
fields.match.ge,
.tdata2 = trigger->address,
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
struct trigger_request_info lt_2 = {
.tdata1 = fields.common | fields.size.any | fields.chain.disable |
fields.match.lt,
.tdata2 = trigger->address + trigger->length,
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
ret = try_setup_chained_match_triggers(target, trigger, ge_1, lt_2);
if (ret != ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
return ret;

LOG_TARGET_DEBUG(target, "trying to setup LT+GE chained match trigger pair");
struct trigger_request_info lt_1 = {
.tdata1 = fields.common | fields.size.any | fields.chain.enable |
fields.match.lt,
.tdata2 = trigger->address + trigger->length,
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
struct trigger_request_info ge_2 = {
.tdata1 = fields.common | fields.size.any | fields.chain.disable |
fields.match.ge,
.tdata2 = trigger->address,
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
ret = try_setup_chained_match_triggers(target, trigger, lt_1, ge_2);
if (ret != ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
return ret;
}
}

if (r->enable_equality_match_trigger) {
JanMatCodasip marked this conversation as resolved.
Show resolved Hide resolved
LOG_TARGET_DEBUG(target, "trying to setup equality match trigger");
struct trigger_request_info eq = {
.tdata1 = fields.common | fields.size.any | fields.chain.disable |
fields.match.ge,
fields.match.eq,
.tdata2 = trigger->address,
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
ret = try_setup_chained_match_triggers(target, trigger, lt_1, ge_2);
if (ret != ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
return ret;
ret = try_setup_single_match_trigger(target, trigger, eq);
if (ret != ERROR_OK)
return ERROR_FAIL;
}
LOG_TARGET_DEBUG(target, "trying to setup equality match trigger");
struct trigger_request_info eq = {
.tdata1 = fields.common | fields.size.any | fields.chain.disable |
fields.match.eq,
.tdata2 = trigger->address,
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
ret = try_setup_single_match_trigger(target, trigger, eq);
if (ret != ERROR_OK)
return ret;

if (trigger->length > 1) {
if (ret == ERROR_OK && trigger->length > 1) {
LOG_TARGET_DEBUG(target, "Trigger will match accesses at address 0x%" TARGET_PRIxADDR
", but may not match accesses at addresses in the inclusive range from 0x%"
TARGET_PRIxADDR " to 0x%" TARGET_PRIxADDR ".", trigger->address,
Expand All @@ -938,7 +945,34 @@ static int maybe_add_trigger_t2_t6(struct target *target,
"against the first address of the range.");
info->range_trigger_fallback_encountered = true;
}
return ERROR_OK;

return ret;
}

static int maybe_add_trigger_t2_t6_for_bp(struct target *target,
struct trigger *trigger, struct match_triggers_tdata1_fields fields)
{
LOG_TARGET_DEBUG(target, "trying to setup equality match trigger");
struct trigger_request_info eq = {
.tdata1 = fields.common | fields.size.any | fields.chain.disable |
fields.match.eq,
.tdata2 = trigger->address,
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};

return try_setup_single_match_trigger(target, trigger, eq);
}

static int maybe_add_trigger_t2_t6(struct target *target,
struct trigger *trigger, struct match_triggers_tdata1_fields fields)
{
if (trigger->is_execute) {
assert(!trigger->is_read && !trigger->is_write);
return maybe_add_trigger_t2_t6_for_bp(target, trigger, fields);
}

assert(trigger->is_read || trigger->is_write);
return maybe_add_trigger_t2_t6_for_wp(target, trigger, fields);
}

static int maybe_add_trigger_t3(struct target *target, bool vs, bool vu,
Expand Down Expand Up @@ -4273,6 +4307,57 @@ COMMAND_HANDLER(riscv_exec_progbuf)
return ERROR_OK;
}

COMMAND_HANDLER(riscv_set_enable_eq_match_trigger)
{
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;
}

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

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;
}

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;
}

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

static const struct command_registration riscv_exec_command_handlers[] = {
{
.name = "dump_sample_buf",
Expand Down Expand Up @@ -4519,6 +4604,27 @@ static const struct command_registration riscv_exec_command_handlers[] = {
.help = "Execute a sequence of 32-bit instructions using the program buffer. "
"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."
},
COMMAND_REGISTRATION_DONE
};

Expand Down Expand Up @@ -4654,6 +4760,10 @@ static void riscv_info_init(struct target *target, struct riscv_info *r)
r->riscv_ebreakm = true;
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;
}

static int riscv_resume_go_all_harts(struct target *target)
Expand Down
4 changes: 4 additions & 0 deletions src/target/riscv/riscv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,10 @@ struct riscv_info {
bool riscv_ebreakm;
bool riscv_ebreaks;
bool riscv_ebreaku;

bool enable_equality_match_trigger;
bool enable_napot_trigger;
bool enable_ge_lt_trigger;
};

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