interesting
/
riscv-openocd
mirror of https://github.com/riscv/riscv-openocd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

target/riscv: manage triggers available to OpenOCD for internal use 

Before the change, if the user wrote to any `tdata*` register, OpenOCD
would sometimes start to disable all the triggers (by writing zeroes to
`tdata1`) and re-enable them again (by witing all trigger registers to the
values read before for each `tselect` value), e.g. on `step`
(see `disable/enable_triggers()`).

There are a couple of issues with such approach:
1. RISC-V Debug Specification does not require custom register types
   to support re-enabling by such sequence of writes (e.g. some custom
   trigger type may require writing a custom CSR to enable it).
2. OpenOCD may still overwrite these triggers when a user asks to set a
   new WP.

This commit introduces `riscv reserve_trigger ...` command to explicitly
mark the triggers OpenOCD should not touch.

Such approach allows to separate management of custom triggers and
offload it onto the user (e.g. disable/enable such triggers by setting up
an event handler on `step`-related events).

Change-Id: I3339000445185ab221368442a070f412bf44bfab
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
This commit is contained in:
Evgeniy Naydanov 2024-07-10 17:41:33 +03:00
parent 5afed58fcd
commit 5a8697b3cf
4 changed files with 131 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
doc/openocd.texi
View File

@ -11524,6 +11524,21 @@ as in the mie CSR (defined in the RISC-V Privileged Spec).
For details on this trigger type, see the RISC-V Debug Specification.
@end deffn
@deffn {Command} {riscv reserve_trigger} [index @option{on|off}]
Manages the set of reserved triggers. Reserving a trigger results in OpenOCD
not using it internally (e.g. skipping it when setting a watchpoint or a
hardware breakpoint), so that the user or the application has unfettered
control over the trigger. By default there are no reserved triggers.
@enumerate
@item @var{index} specifies the index of a trigger to reserve or free up.
@item The second argument specifies whether the trigger should be reserved
(@var{on}) or a prior reservation cancelled (@var{off}).
@item If called without parameters, returns indices of reserved triggers.
@end enumerate
@end deffn
@deffn {Command} {riscv itrigger clear}
Clear the type 4 trigger that was set using @command{riscv itrigger set}.
@end deffn

12
src/target/riscv/riscv-013_reg.c
View File

@ -44,7 +44,6 @@ static int riscv013_reg_get(struct reg *reg)
static int riscv013_reg_set(struct reg *reg, uint8_t *buf)
{
struct target *target = riscv_reg_impl_get_target(reg);
RISCV_INFO(r);
char *str = buf_to_hex_str(buf, reg->size);
LOG_TARGET_DEBUG(target, "Write 0x%s to %s (valid=%d).", str, reg->name,
@ -57,17 +56,6 @@ static int riscv013_reg_set(struct reg *reg, uint8_t *buf)
buf_get_u64(buf, 0, reg->size) == 0)
return ERROR_OK;
if (reg->number == GDB_REGNO_TDATA1 ||
reg->number == GDB_REGNO_TDATA2) {
r->manual_hwbp_set = true;
/* When enumerating triggers, we clear any triggers with DMODE set,
* assuming they were left over from a previous debug session. So make
* sure that is done before a user might be setting their own triggers.
*/
if (riscv_enumerate_triggers(target) != ERROR_OK)
return ERROR_FAIL;
}
if (reg->number >= GDB_REGNO_V0 && reg->number <= GDB_REGNO_V31) {
if (riscv013_set_register_buf(target, reg->number, buf) != ERROR_OK)
return ERROR_FAIL;

187
src/target/riscv/riscv.c
View File

@ -527,6 +527,8 @@ static void riscv_deinit_target(struct target *target)
if (!info)
return;
free(info->reserved_triggers);
range_list_t *entry, *tmp;
list_for_each_entry_safe(entry, tmp, &info->hide_csr, list) {
free(entry->name);
@ -620,6 +622,15 @@ static int find_first_trigger_by_id(struct target *target, int unique_id)
static int set_trigger(struct target *target, unsigned int idx, riscv_reg_t tdata1, riscv_reg_t tdata2,
riscv_reg_t tdata1_ignore_mask)
{
RISCV_INFO(r);
assert(r->reserved_triggers);
assert(idx < r->trigger_count);
if (r->reserved_triggers[idx]) {
LOG_TARGET_DEBUG(target,
"Trigger %u is reserved by 'reserve_trigger' command.", idx);
return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
}
riscv_reg_t tdata1_rb, tdata2_rb;
// Select which trigger to use
if (riscv_reg_set(target, GDB_REGNO_TSELECT, idx) != ERROR_OK)
@ -2453,87 +2464,48 @@ static int riscv_deassert_reset(struct target *target)
return tt->deassert_reset(target);
}
/* state must be riscv_reg_t state[RISCV_MAX_HWBPS] = {0}; */
static int disable_triggers(struct target *target, riscv_reg_t *state)
/* "wp_is_set" array must have at least "r->trigger_count" items. */
static int disable_watchpoints(struct target *target, bool *wp_is_set)
{
RISCV_INFO(r);
LOG_TARGET_DEBUG(target, "Disabling triggers.");
if (riscv_enumerate_triggers(target) != ERROR_OK)
return ERROR_FAIL;
if (r->manual_hwbp_set) {
/* Look at every trigger that may have been set. */
riscv_reg_t tselect;
if (riscv_reg_get(target, &tselect, GDB_REGNO_TSELECT) != ERROR_OK)
return ERROR_FAIL;
for (unsigned int t = 0; t < r->trigger_count; t++) {
if (riscv_reg_set(target, GDB_REGNO_TSELECT, t) != ERROR_OK)
/* TODO: The algorithm is flawed and may result in a situation described in
* https://github.com/riscv-collab/riscv-openocd/issues/1108
*/
memset(wp_is_set, false, r->trigger_count);
struct watchpoint *watchpoint = target->watchpoints;
int i = 0;
while (watchpoint) {
LOG_TARGET_DEBUG(target, "Watchpoint %" PRIu32 ": set=%s",
watchpoint->unique_id,
wp_is_set[i] ? "true" : "false");
wp_is_set[i] = watchpoint->is_set;
if (watchpoint->is_set) {
if (riscv_remove_watchpoint(target, watchpoint) != ERROR_OK)
return ERROR_FAIL;
riscv_reg_t tdata1;
if (riscv_reg_get(target, &tdata1, GDB_REGNO_TDATA1) != ERROR_OK)
return ERROR_FAIL;
if (tdata1 & CSR_TDATA1_DMODE(riscv_xlen(target))) {
state[t] = tdata1;
if (riscv_reg_set(target, GDB_REGNO_TDATA1, 0) != ERROR_OK)
return ERROR_FAIL;
}
}
if (riscv_reg_set(target, GDB_REGNO_TSELECT, tselect) != ERROR_OK)
return ERROR_FAIL;
} else {
/* Just go through the triggers we manage. */
struct watchpoint *watchpoint = target->watchpoints;
int i = 0;
while (watchpoint) {
LOG_TARGET_DEBUG(target, "Watchpoint %d: set=%d", i, watchpoint->is_set);
state[i] = watchpoint->is_set;
if (watchpoint->is_set) {
if (riscv_remove_watchpoint(target, watchpoint) != ERROR_OK)
return ERROR_FAIL;
}
watchpoint = watchpoint->next;
i++;
}
watchpoint = watchpoint->next;
i++;
}
return ERROR_OK;
}
static int enable_triggers(struct target *target, riscv_reg_t *state)
static int enable_watchpoints(struct target *target, bool *wp_is_set)
{
RISCV_INFO(r);
if (r->manual_hwbp_set) {
/* Look at every trigger that may have been set. */
riscv_reg_t tselect;
if (riscv_reg_get(target, &tselect, GDB_REGNO_TSELECT) != ERROR_OK)
return ERROR_FAIL;
for (unsigned int t = 0; t < r->trigger_count; t++) {
if (state[t] != 0) {
if (riscv_reg_set(target, GDB_REGNO_TSELECT, t) != ERROR_OK)
return ERROR_FAIL;
if (riscv_reg_set(target, GDB_REGNO_TDATA1, state[t]) != ERROR_OK)
return ERROR_FAIL;
}
}
if (riscv_reg_set(target, GDB_REGNO_TSELECT, tselect) != ERROR_OK)
return ERROR_FAIL;
} else {
struct watchpoint *watchpoint = target->watchpoints;
int i = 0;
while (watchpoint) {
LOG_TARGET_DEBUG(target, "Watchpoint %d: cleared=%" PRId64, i, state[i]);
if (state[i]) {
if (riscv_add_watchpoint(target, watchpoint) != ERROR_OK)
return ERROR_FAIL;
}
watchpoint = watchpoint->next;
i++;
struct watchpoint *watchpoint = target->watchpoints;
int i = 0;
while (watchpoint) {
LOG_TARGET_DEBUG(target, "Watchpoint %" PRIu32
": %s to be re-enabled.", watchpoint->unique_id,
wp_is_set[i] ? "needs " : "does not need");
if (wp_is_set[i]) {
if (riscv_add_watchpoint(target, watchpoint) != ERROR_OK)
return ERROR_FAIL;
}
watchpoint = watchpoint->next;
i++;
}
return ERROR_OK;
@ -3781,10 +3753,17 @@ static int riscv_openocd_step_impl(struct target *target, int current,
return ERROR_FAIL;
}
riscv_reg_t trigger_state[RISCV_MAX_HWBPS] = {0};
if (disable_triggers(target, trigger_state) != ERROR_OK)
if (riscv_enumerate_triggers(target) != ERROR_OK)
return ERROR_FAIL;
RISCV_INFO(r);
bool *wps_to_enable = calloc(sizeof(*wps_to_enable), r->trigger_count);
if (disable_watchpoints(target, wps_to_enable) != ERROR_OK) {
LOG_TARGET_ERROR(target, "Failed to temporarily disable "
"watchpoints before single-step.");
return ERROR_FAIL;
}
bool success = true;
uint64_t current_mstatus;
RISCV_INFO(info);
@ -3814,9 +3793,10 @@ static int riscv_openocd_step_impl(struct target *target, int current,
}
_exit:
if (enable_triggers(target, trigger_state) != ERROR_OK) {
if (enable_watchpoints(target, wps_to_enable) != ERROR_OK) {
success = false;
LOG_TARGET_ERROR(target, "Unable to enable triggers.");
LOG_TARGET_ERROR(target, "Failed to re-enable watchpoints "
"after single-step.");
}
if (breakpoint && (riscv_add_breakpoint(target, breakpoint) != ERROR_OK)) {
@ -5031,6 +5011,57 @@ COMMAND_HANDLER(riscv_set_enable_trigger_feature)
return ERROR_OK;
}
static COMMAND_HELPER(report_reserved_triggers, struct target *target)
{
RISCV_INFO(r);
if (riscv_enumerate_triggers(target) != ERROR_OK)
return ERROR_FAIL;
const char *separator = "";
for (riscv_reg_t t = 0; t < r->trigger_count; ++t) {
if (r->reserved_triggers[t]) {
command_print_sameline(CMD, "%s%" PRIu64, separator, t);
separator = " ";
}
}
command_print_sameline(CMD, "\n");
return ERROR_OK;
}
COMMAND_HANDLER(handle_reserve_trigger)
{
struct target *target = get_current_target(CMD_CTX);
if (CMD_ARGC == 0)
return CALL_COMMAND_HANDLER(report_reserved_triggers, target);
if (CMD_ARGC != 2)
return ERROR_COMMAND_SYNTAX_ERROR;
riscv_reg_t t;
COMMAND_PARSE_NUMBER(u64, CMD_ARGV[0], t);
if (riscv_enumerate_triggers(target) != ERROR_OK)
return ERROR_FAIL;
RISCV_INFO(r);
if (r->trigger_count == 0) {
command_print(CMD, "Error: There are no triggers on the target.");
return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
}
if (t >= r->trigger_count) {
command_print(CMD, "Error: trigger with index %" PRIu64
" does not exist. There are only %u triggers"
" on the target (with indexes 0 .. %u).",
t, r->trigger_count, r->trigger_count - 1);
return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
}
if (r->trigger_unique_id[t] != -1) {
command_print(CMD, "Error: trigger with index %" PRIu64
" is already in use and can not be reserved.", t);
return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
}
COMMAND_PARSE_ON_OFF(CMD_ARGV[1], r->reserved_triggers[t]);
return ERROR_OK;
}
static const struct command_registration riscv_exec_command_handlers[] = {
{
.name = "dump_sample_buf",
@ -5287,6 +5318,14 @@ static const struct command_registration riscv_exec_command_handlers[] = {
.usage = "[('eq'|'napot'|'ge_lt'|'all') ('wp'|'none')]",
.help = "Control whether OpenOCD is allowed to use certain RISC-V trigger features for watchpoints."
},
{
.name = "reserve_trigger",
.handler = handle_reserve_trigger,
/* TODO: Move this to COMMAND_ANY */
.mode = COMMAND_EXEC,
.usage = "[index ('on'|'off')]",
.help = "Controls which RISC-V triggers shall not be touched by OpenOCD.",
},
COMMAND_REGISTRATION_DONE
};
@ -5711,6 +5750,8 @@ int riscv_enumerate_triggers(struct target *target)
"Assuming that triggers are not implemented.");
r->triggers_enumerated = true;
r->trigger_count = 0;
free(r->reserved_triggers);
r->reserved_triggers = NULL;
return ERROR_OK;
}
@ -5745,6 +5786,8 @@ int riscv_enumerate_triggers(struct target *target)
r->triggers_enumerated = true;
r->trigger_count = t;
LOG_TARGET_INFO(target, "Found %d triggers", r->trigger_count);
free(r->reserved_triggers);
r->reserved_triggers = calloc(sizeof(*r->reserved_triggers), t);
create_wp_trigger_cache(target);
return ERROR_OK;
}

4
src/target/riscv/riscv.h
View File

@ -272,9 +272,7 @@ struct riscv_info {
struct reg_data_type_union vector_union;
struct reg_data_type type_vector;
/* Set when trigger registers are changed by the user. This indicates we need
* to beware that we may hit a trigger that we didn't realize had been set. */
bool manual_hwbp_set;
bool *reserved_triggers;
/* Memory access methods to use, ordered by priority, highest to lowest. */
int mem_access_methods[RISCV_NUM_MEM_ACCESS_METHODS];