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.

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 trigger setup #878

Merged
merged 1 commit into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 42 additions & 13 deletions src/target/riscv/riscv.c
Original file line number Diff line number Diff line change
Expand Up @@ -655,9 +655,18 @@ struct trigger_request_info {
riscv_reg_t tdata1_ignore_mask;
};

static void log_trigger_request_info(struct trigger_request_info trig_info)
{
LOG_DEBUG("tdata1=%" PRIx64 ", tdata2=%" PRIx64 ", tdata1_ignore_mask=%" PRIx64,
trig_info.tdata1, trig_info.tdata2, trig_info.tdata1_ignore_mask);
};

static int try_setup_single_match_trigger(struct target *target,
struct trigger *trigger, struct trigger_request_info trig_info)
{
LOG_TARGET_DEBUG(target, "trying to set up a match trigger");
log_trigger_request_info(trig_info);

int trigger_type =
get_field(trig_info.tdata1, CSR_MCONTROL_TYPE(riscv_xlen(target)));
int ret = ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
Expand All @@ -683,6 +692,9 @@ static int try_setup_chained_match_triggers(struct target *target,
struct trigger *trigger, struct trigger_request_info t1,
struct trigger_request_info t2)
{
LOG_TARGET_DEBUG(target, "trying to set up a chain of match triggers");
log_trigger_request_info(t1);
log_trigger_request_info(t2);
int trigger_type =
get_field(t1.tdata1, CSR_MCONTROL_TYPE(riscv_xlen(target)));
int ret = ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
Expand Down Expand Up @@ -717,8 +729,10 @@ static int try_setup_chained_match_triggers(struct target *target,
struct match_triggers_tdata1_fields {
riscv_reg_t common;
struct {
/* Other values are available for this field,
* but currently only `any` is needed.
*/
riscv_reg_t any;
riscv_reg_t s8bit;
} size;
struct {
riscv_reg_t enable;
Expand Down Expand Up @@ -752,10 +766,7 @@ static struct match_triggers_tdata1_fields fill_match_triggers_tdata1_fields_t2(
.size = {
.any =
field_value(CSR_MCONTROL_SIZELO, CSR_MCONTROL_SIZELO_ANY & 3) |
field_value(CSR_MCONTROL_SIZEHI, (CSR_MCONTROL_SIZELO_ANY >> 2) & 3),
.s8bit =
field_value(CSR_MCONTROL_SIZELO, CSR_MCONTROL_SIZELO_8BIT & 3) |
field_value(CSR_MCONTROL_SIZEHI, (CSR_MCONTROL_SIZELO_8BIT >> 2) & 3)
field_value(CSR_MCONTROL_SIZEHI, (CSR_MCONTROL_SIZELO_ANY >> 2) & 3)
},
.chain = {
.enable = field_value(CSR_MCONTROL_CHAIN, CSR_MCONTROL_CHAIN_ENABLED),
Expand Down Expand Up @@ -793,8 +804,7 @@ static struct match_triggers_tdata1_fields fill_match_triggers_tdata1_fields_t6(
field_value(CSR_MCONTROL6_LOAD, trigger->is_read) |
field_value(CSR_MCONTROL6_STORE, trigger->is_write),
.size = {
.any = field_value(CSR_MCONTROL6_SIZE, CSR_MCONTROL6_SIZE_ANY),
.s8bit = field_value(CSR_MCONTROL6_SIZE, CSR_MCONTROL6_SIZE_8BIT)
.any = field_value(CSR_MCONTROL6_SIZE, CSR_MCONTROL6_SIZE_ANY)
},
.chain = {
.enable = field_value(CSR_MCONTROL6_CHAIN, CSR_MCONTROL6_CHAIN_ENABLED),
Expand Down Expand Up @@ -852,13 +862,13 @@ static int maybe_add_trigger_t2_t6(struct target *target,
struct trigger_request_info lt_1 = {
.tdata1 = fields.common | fields.size.any | fields.chain.enable |
fields.match.lt,
.tdata2 = trigger->address,
.tdata2 = trigger->address + trigger->length,
timsifive marked this conversation as resolved.
Show resolved Hide resolved
.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 + trigger->length,
.tdata2 = trigger->address,
.tdata1_ignore_mask = fields.tdata1_ignore_mask
};
ret = try_setup_chained_match_triggers(target, trigger, lt_1, ge_2);
Expand All @@ -867,13 +877,32 @@ static int maybe_add_trigger_t2_t6(struct target *target,
}
LOG_TARGET_DEBUG(target, "trying to setup equality match trigger");
struct trigger_request_info eq = {
.tdata1 = fields.common |
(trigger->length == 1 ? fields.size.s8bit : fields.size.any) |
fields.chain.disable | fields.match.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);
ret = try_setup_single_match_trigger(target, trigger, eq);
if (ret != ERROR_OK)
Copy link
Contributor

Choose a reason for hiding this comment

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

@en-sc don't we need to handle ERROR_TARGET_RESOURCE_NOT_AVAILABLE return code here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the catch! Addressed in #899

return ERROR_FAIL;

if (trigger->length > 1) {
JanMatCodasip marked this conversation as resolved.
Show resolved Hide resolved
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,
trigger->address + 1, trigger->address + trigger->length - 1);
RISCV_INFO(info);
if (!info->range_trigger_fallback_encountered)
/* This message is displayed only once per target to avoid
* overwhelming the user with such messages on resume.
*/
LOG_TARGET_WARNING(target,
"Could not set a trigger that will match a whole address range. "
"As a fallback, this trigger (and maybe others) will only match "
"against the first address of the range.");
info->range_trigger_fallback_encountered = true;
}
return ERROR_OK;
}

static int maybe_add_trigger_t3(struct target *target, bool vs, bool vu,
Expand Down
2 changes: 2 additions & 0 deletions src/target/riscv/riscv.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,8 @@ struct riscv_info {
int64_t last_activity;

yes_no_maybe_t vsew64_supported;

bool range_trigger_fallback_encountered;
};

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