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

target/riscv: cleanup riscv_enumerate_triggers() #909

Merged
merged 1 commit into from
Sep 8, 2023
Merged

target/riscv: cleanup riscv_enumerate_triggers() #909

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
4 changes: 3 additions & 1 deletion doc/openocd.texi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10734,7 +10734,9 @@ address, or to sample a changing value in a memory-mapped device.
@end deffn

@deffn {Command} {riscv info}
Displays some information OpenOCD detected about the target.
Displays some information OpenOCD detected about the target. Output's format
allows to use it directly with TCL's `array set` function. In case obtaining an
info point failed, the corresponding value is displayed as "unavailable".
@end deffn

@deffn {Command} {riscv reset_delays} [wait]
Expand Down
196 changes: 122 additions & 74 deletions src/target/riscv/riscv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4162,15 +4162,25 @@ COMMAND_HANDLER(handle_dump_sample_buf_command)
return result;
}

COMMAND_HELPER(riscv_print_info_line, const char *section, const char *key,
unsigned int value)
static COMMAND_HELPER(riscv_print_info_line_if_available, const char *section,
const char *key, unsigned int value, bool is_available)
{
char full_key[80];
snprintf(full_key, sizeof(full_key), "%s.%s", section, key);
command_print(CMD, "%-21s %3d", full_key, value);
if (is_available)
command_print(CMD, "%-21s %3d", full_key, value);
else
command_print(CMD, "%-21s unavailable", full_key);
return 0;
}

COMMAND_HELPER(riscv_print_info_line, const char *section, const char *key,
unsigned int value)
{
return CALL_COMMAND_HANDLER(riscv_print_info_line_if_available, section,
key, value, /*is_available*/ true);
}

COMMAND_HANDLER(handle_info)
{
struct target *target = get_current_target(CMD_CTX);
Expand All @@ -4179,10 +4189,11 @@ COMMAND_HANDLER(handle_info)
/* This output format can be fed directly into TCL's "array set". */

riscv_print_info_line(CMD, "hart", "xlen", riscv_xlen(target));
riscv_enumerate_triggers(target);
riscv_print_info_line(CMD, "hart", "trigger_count",
r->trigger_count);

const bool trigger_count_available =
riscv_enumerate_triggers(target) == ERROR_OK;
riscv_print_info_line_if_available(CMD, "hart", "trigger_count",
r->trigger_count, trigger_count_available);
Comment on lines +4193 to +4196
Copy link
Collaborator

Choose a reason for hiding this comment

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

You shouldn't print "unavailable" if the problem is that the number of triggers could not be detected, because it could also be interpreted to mean that there are no triggers present. Something like "detection failed" is much more clear.

Instead of creating riscv_print_info_line_if_available, I prefer adding riscv_print_info_line_str and renaming riscv_print_info_line to riscv_print_info_line_int. Then the caller, which knows much more about the details, can decide what to show the user.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

According to this comment, the format of the riscv info command is intended to be used directly with TCL's array set.
So I have assumed that it is intended for the output to be machine-readable. Providing a programmer with a way to output an arbitrary string will greatly complicate the parsing of the output.
I can change the string that is currently used.
Currently:

hart.trigger_count           unavailable

Reports that trigger_count is not available.
I can document this in doc/openocd.texi.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point about the TCL array command. Please update the documentation and then leave the code as-is.

if (r->print_info)
return CALL_COMMAND_HANDLER(r->print_info, target);

Expand Down Expand Up @@ -5029,6 +5040,78 @@ int riscv_dmi_write_u64_bits(struct target *target)
return r->dmi_write_u64_bits(target);
}

static int check_if_trigger_exists(struct target *target, unsigned int index)
{
/* If we can't write tselect, then this hart does not support triggers. */
if (riscv_set_register(target, GDB_REGNO_TSELECT, index) != ERROR_OK)
return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
riscv_reg_t tselect_rb;
if (riscv_get_register(target, &tselect_rb, GDB_REGNO_TSELECT) != ERROR_OK)
return ERROR_FAIL;
/* Mask off the top bit, which is used as tdrmode in legacy RISC-V Debug Spec
* (old revisions of v0.11 spec). */
tselect_rb &= ~(1ULL << (riscv_xlen(target) - 1));
if (tselect_rb != index)
return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
return ERROR_OK;
}

/**
* This function reads `tinfo` or `tdata1`, when reading `tinfo` fails,
* to determine trigger types supported by a trigger.
* It is assumed that the trigger is already selected via writing `tselect`.
*/
static int get_trigger_types(struct target *target, unsigned int *trigger_tinfo,
timsifive marked this conversation as resolved.
Show resolved Hide resolved
riscv_reg_t tdata1)
{
assert(trigger_tinfo);
riscv_reg_t tinfo;
JanMatCodasip marked this conversation as resolved.
Show resolved Hide resolved
if (riscv_get_register(target, &tinfo, GDB_REGNO_TINFO) == ERROR_OK) {
/* tinfo.INFO == 1: trigger doesn’t exist
* tinfo == 0 or tinfo.INFO != 1 and tinfo LSB is set: invalid tinfo */
if (tinfo == 0 || tinfo & 0x1)
return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
*trigger_tinfo = tinfo;
return ERROR_OK;
}
const unsigned int type = get_field(tdata1, CSR_TDATA1_TYPE(riscv_xlen(target)));
if (type == 0)
return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
*trigger_tinfo = 1 << type;
return ERROR_OK;
}

static int disable_trigger_if_dmode(struct target *target, riscv_reg_t tdata1)
{
bool dmode_is_set = false;
switch (get_field(tdata1, CSR_TDATA1_TYPE(riscv_xlen(target)))) {
case CSR_TDATA1_TYPE_LEGACY:
/* On these older cores we don't support software using
* triggers. */
dmode_is_set = true;
break;
case CSR_TDATA1_TYPE_MCONTROL:
dmode_is_set = tdata1 & CSR_MCONTROL_DMODE(riscv_xlen(target));
break;
case CSR_TDATA1_TYPE_MCONTROL6:
dmode_is_set = tdata1 & CSR_MCONTROL6_DMODE(riscv_xlen(target));
break;
case CSR_TDATA1_TYPE_ICOUNT:
dmode_is_set = tdata1 & CSR_ICOUNT_DMODE(riscv_xlen(target));
break;
case CSR_TDATA1_TYPE_ITRIGGER:
dmode_is_set = tdata1 & CSR_ITRIGGER_DMODE(riscv_xlen(target));
break;
case CSR_TDATA1_TYPE_ETRIGGER:
dmode_is_set = tdata1 & CSR_ETRIGGER_DMODE(riscv_xlen(target));
break;
}
if (!dmode_is_set)
/* Nothing to do */
return ERROR_OK;
return riscv_set_register(target, GDB_REGNO_TDATA1, 0);
}

/**
* Count triggers, and initialize trigger_count for each hart.
* trigger_count is initialized even if this function fails to discover
Expand All @@ -5043,89 +5126,54 @@ int riscv_enumerate_triggers(struct target *target)
if (r->triggers_enumerated)
return ERROR_OK;

r->triggers_enumerated = true; /* At the very least we tried. */
JanMatCodasip marked this conversation as resolved.
Show resolved Hide resolved
if (target->state != TARGET_HALTED) {
LOG_TARGET_ERROR(target, "Unable to enumerate triggers: target not halted.");
return ERROR_FAIL;
}

riscv_reg_t tselect;
int result = riscv_get_register(target, &tselect, GDB_REGNO_TSELECT);
riscv_reg_t orig_tselect;
int result = riscv_get_register(target, &orig_tselect, GDB_REGNO_TSELECT);
/* If tselect is not readable, the trigger module is likely not
* implemented. There are no triggers to enumerate then and no error
* should be thrown. */
* implemented. */
if (result != ERROR_OK) {
LOG_TARGET_DEBUG(target, "Cannot access tselect register. "
LOG_TARGET_INFO(target, "Cannot access tselect register. "
"Assuming that triggers are not implemented.");
r->triggers_enumerated = true;
r->trigger_count = 0;
return ERROR_OK;
}

for (unsigned int t = 0; t < RISCV_MAX_TRIGGERS; ++t) {
r->trigger_count = t;

/* If we can't write tselect, then this hart does not support triggers. */
if (riscv_set_register(target, GDB_REGNO_TSELECT, t) != ERROR_OK)
unsigned int t = 0;
for (; t < ARRAY_SIZE(r->trigger_tinfo); ++t) {
result = check_if_trigger_exists(target, t);
if (result == ERROR_FAIL)
return ERROR_FAIL;
JanMatCodasip marked this conversation as resolved.
Show resolved Hide resolved
if (result == ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
break;
uint64_t tselect_rb;
result = riscv_get_register(target, &tselect_rb, GDB_REGNO_TSELECT);
if (result != ERROR_OK)
return result;
/* Mask off the top bit, which is used as tdrmode in old
* implementations. */
tselect_rb &= ~(1ULL << (riscv_xlen(target) - 1));
if (tselect_rb != t)

riscv_reg_t tdata1;
if (riscv_get_register(target, &tdata1, GDB_REGNO_TDATA1) != ERROR_OK)
return ERROR_FAIL;

result = get_trigger_types(target, &r->trigger_tinfo[t], tdata1);
if (result == ERROR_FAIL)
return ERROR_FAIL;
if (result == ERROR_TARGET_RESOURCE_NOT_AVAILABLE)
break;

uint64_t tinfo;
result = riscv_get_register(target, &tinfo, GDB_REGNO_TINFO);
if (result == ERROR_OK) {
/* tinfo == 0 invalid tinfo
* tinfo == 1 trigger doesn’t exist */
if (tinfo == 0 || tinfo == 1)
break;
r->trigger_tinfo[t] = tinfo;
} else {
uint64_t tdata1;
result = riscv_get_register(target, &tdata1, GDB_REGNO_TDATA1);
if (result != ERROR_OK)
return result;
LOG_TARGET_DEBUG(target, "Trigger %u: supported types (mask) = 0x%08x",
t, r->trigger_tinfo[t]);

int type = get_field(tdata1, CSR_TDATA1_TYPE(riscv_xlen(target)));
if (type == 0)
break;
switch (type) {
case CSR_TDATA1_TYPE_LEGACY:
/* On these older cores we don't support software using
* triggers. */
riscv_set_register(target, GDB_REGNO_TDATA1, 0);
break;
case CSR_TDATA1_TYPE_MCONTROL:
if (tdata1 & CSR_MCONTROL_DMODE(riscv_xlen(target)))
riscv_set_register(target, GDB_REGNO_TDATA1, 0);
break;
case CSR_TDATA1_TYPE_MCONTROL6:
if (tdata1 & CSR_MCONTROL6_DMODE(riscv_xlen(target)))
riscv_set_register(target, GDB_REGNO_TDATA1, 0);
break;
case CSR_TDATA1_TYPE_ICOUNT:
if (tdata1 & CSR_ICOUNT_DMODE(riscv_xlen(target)))
riscv_set_register(target, GDB_REGNO_TDATA1, 0);
break;
case CSR_TDATA1_TYPE_ITRIGGER:
if (tdata1 & CSR_ITRIGGER_DMODE(riscv_xlen(target)))
riscv_set_register(target, GDB_REGNO_TDATA1, 0);
break;
case CSR_TDATA1_TYPE_ETRIGGER:
if (tdata1 & CSR_ETRIGGER_DMODE(riscv_xlen(target)))
riscv_set_register(target, GDB_REGNO_TDATA1, 0);
break;
}
r->trigger_tinfo[t] = 1 << type;
}
LOG_TARGET_DEBUG(target, "Trigger %u: supported types (mask) = 0x%08x", t, r->trigger_tinfo[t]);
if (disable_trigger_if_dmode(target, tdata1) != ERROR_OK)
return ERROR_FAIL;
}

riscv_set_register(target, GDB_REGNO_TSELECT, tselect);

LOG_TARGET_INFO(target, "Found %d triggers.", r->trigger_count);
if (riscv_set_register(target, GDB_REGNO_TSELECT, orig_tselect) != ERROR_OK)
return ERROR_FAIL;

r->triggers_enumerated = true;
r->trigger_count = t;
LOG_TARGET_INFO(target, "Found %d triggers", r->trigger_count);
return ERROR_OK;
}

Expand Down
Loading