From a50b280558cc2908f43dba600461aca256bfbf10 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 12 Oct 2022 08:57:00 -0700 Subject: [PATCH] Properly track selecting multiple harts at once. (#743) * Properly track selecting multiple harts at once. use_hasel is a bit of a hack. Change-Id: Ia589ebc16bca32038d915df9988361b88e940917 Signed-off-by: Tim Newsome * Clarifying comment. Co-authored-by: Jan Matyas <50193733+JanMatCodasip@users.noreply.github.com> Signed-off-by: Tim Newsome * Rename set_hartsel to set_dmcontrol_hartsel Change-Id: Iab28531281aa6fc604ec7d34974ed444ea9ea850 * Make set_dmcontrol_hartsel() more idiomatic. Change-Id: I56a885043c515359e33b9c8a03aed637c81d1486 * Use constant for multiple harts instead of -1. Change-Id: Iefeaf74202f2b4918d21f15f7ff7ca514175b8fb Signed-off-by: Tim Newsome Signed-off-by: Tim Newsome Co-authored-by: Jan Matyas <50193733+JanMatCodasip@users.noreply.github.com> --- src/target/riscv/riscv-013.c | 123 ++++++++++++++++++----------------- 1 file changed, 64 insertions(+), 59 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 19fe2ada8..bc5b262ca 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -31,7 +31,7 @@ static int riscv013_on_step_or_resume(struct target *target, bool step); static int riscv013_step_or_resume_current_hart(struct target *target, - bool step, bool use_hasel); + bool step); static void riscv013_clear_abstract_error(struct target *target); /* Implementations of the functions in riscv_info_t. */ @@ -129,6 +129,8 @@ typedef enum { YNM_NO } yes_no_maybe_t; +#define HART_INDEX_MULTIPLE -1 + typedef struct { struct list_head list; int abs_chain_position; @@ -140,9 +142,7 @@ typedef struct { /* Targets that are connected to this DM. */ struct list_head target_list; /* Contains the ID of the hart that is currently selected by this DM. - * (If multiple harts are selected this is meaningless, and that corner case - * is not currently handled well.) - */ + * If multiple harts are selected this is HART_INDEX_MULTIPLE. */ int current_hartid; bool hasel_supported; @@ -216,6 +216,9 @@ typedef struct { /* DM that provides access to this target. */ dm013_info_t *dm; + + /* This target was selected using hasel. */ + bool selected; } riscv013_info_t; LIST_HEAD(dm_list); @@ -256,7 +259,7 @@ dm013_info_t *get_dm(struct target *target) if (!dm) return NULL; dm->abs_chain_position = abs_chain_position; - dm->current_hartid = -1; + dm->current_hartid = 0; dm->hart_count = -1; INIT_LIST_HEAD(&dm->target_list); list_add(&dm->list, &dm_list); @@ -279,16 +282,21 @@ dm013_info_t *get_dm(struct target *target) return dm; } -static uint32_t set_hartsel(uint32_t initial, uint32_t index) +static uint32_t set_dmcontrol_hartsel(uint32_t initial, int hart_index) { - initial &= ~DM_DMCONTROL_HARTSELLO; - initial &= ~DM_DMCONTROL_HARTSELHI; - - uint32_t index_lo = index & ((1 << DM_DMCONTROL_HARTSELLO_LENGTH) - 1); - initial |= index_lo << DM_DMCONTROL_HARTSELLO_OFFSET; - uint32_t index_hi = index >> DM_DMCONTROL_HARTSELLO_LENGTH; - assert(index_hi < 1 << DM_DMCONTROL_HARTSELHI_LENGTH); - initial |= index_hi << DM_DMCONTROL_HARTSELHI_OFFSET; + if (hart_index >= 0) { + initial = set_field(initial, DM_DMCONTROL_HASEL, DM_DMCONTROL_HASEL_SINGLE); + uint32_t index_lo = hart_index & ((1 << DM_DMCONTROL_HARTSELLO_LENGTH) - 1); + initial = set_field(initial, DM_DMCONTROL_HARTSELLO, index_lo); + uint32_t index_hi = hart_index >> DM_DMCONTROL_HARTSELLO_LENGTH; + assert(index_hi < (1 << DM_DMCONTROL_HARTSELHI_LENGTH)); + initial = set_field(initial, DM_DMCONTROL_HARTSELHI, index_hi); + } else if (hart_index == HART_INDEX_MULTIPLE) { + initial = set_field(initial, DM_DMCONTROL_HASEL, DM_DMCONTROL_HASEL_MULTIPLE); + /* TODO: https://github.com/riscv/riscv-openocd/issues/748 */ + initial = set_field(initial, DM_DMCONTROL_HARTSELLO, 0); + initial = set_field(initial, DM_DMCONTROL_HARTSELHI, 0); + } return initial; } @@ -1684,7 +1692,7 @@ static int examine(struct target *target) if (get_field(s, DM_DMSTATUS_ANYHAVERESET)) dmi_write(target, DM_DMCONTROL, - set_hartsel(DM_DMCONTROL_DMACTIVE | DM_DMCONTROL_ACKHAVERESET, i)); + set_dmcontrol_hartsel(DM_DMCONTROL_DMACTIVE | DM_DMCONTROL_ACKHAVERESET, i)); } LOG_DEBUG("Detected %d harts.", dm->hart_count); @@ -1703,6 +1711,7 @@ static int examine(struct target *target) bool halted = riscv_is_halted(target); if (!halted) { + r->prepped = true; if (riscv013_halt_go(target) != ERROR_OK) { LOG_TARGET_ERROR(target, "Fatal: Hart %d failed to halt during examine()", info->index); @@ -1761,7 +1770,7 @@ static int examine(struct target *target) } if (!halted) - riscv013_step_or_resume_current_hart(target, false, false); + riscv013_step_or_resume_current_hart(target, false); if (target->smp) { bool haltgroup_supported; @@ -2357,7 +2366,7 @@ static int assert_reset(struct target *target) /* Set haltreq for each hart. */ uint32_t control = control_base; - control = set_hartsel(control_base, info->index); + control = set_dmcontrol_hartsel(control_base, info->index); control = set_field(control, DM_DMCONTROL_HALTREQ, target->reset_halt ? 1 : 0); dmi_write(target, DM_DMCONTROL, control); @@ -2368,7 +2377,7 @@ static int assert_reset(struct target *target) } else { /* Reset just this hart. */ - uint32_t control = set_hartsel(control_base, info->index); + uint32_t control = set_dmcontrol_hartsel(control_base, info->index); control = set_field(control, DM_DMCONTROL_HALTREQ, target->reset_halt ? 1 : 0); control = set_field(control, DM_DMCONTROL_NDMRESET, 1); @@ -2398,7 +2407,7 @@ static int deassert_reset(struct target *target) uint32_t control = 0; control = set_field(control, DM_DMCONTROL_HALTREQ, target->reset_halt ? 1 : 0); control = set_field(control, DM_DMCONTROL_DMACTIVE, 1); - dmi_write(target, DM_DMCONTROL, set_hartsel(control, info->index)); + dmi_write(target, DM_DMCONTROL, set_dmcontrol_hartsel(control, info->index)); uint32_t dmstatus; int dmi_busy_delay = info->dmi_busy_delay; @@ -2410,7 +2419,7 @@ static int deassert_reset(struct target *target) if (index != info->index) continue; dmi_write(target, DM_DMCONTROL, - set_hartsel(control, index)); + set_dmcontrol_hartsel(control, index)); } else { index = info->index; } @@ -2449,7 +2458,7 @@ static int deassert_reset(struct target *target) if (get_field(dmstatus, DM_DMSTATUS_ALLHAVERESET)) { /* Ack reset. */ dmi_write(target, DM_DMCONTROL, - set_hartsel(control, index) | + set_dmcontrol_hartsel(control, index) | DM_DMCONTROL_ACKHAVERESET); } @@ -4122,7 +4131,7 @@ static int dm013_select_hart(struct target *target, int hart_index) return ERROR_OK; uint32_t dmcontrol = DM_DMCONTROL_DMACTIVE; - dmcontrol = set_hartsel(dmcontrol, hart_index); + dmcontrol = set_dmcontrol_hartsel(dmcontrol, hart_index); if (dmi_write(target, DM_DMCONTROL, dmcontrol) != ERROR_OK) { /* Who knows what the state is? */ dm->current_hartid = -2; @@ -4134,15 +4143,14 @@ static int dm013_select_hart(struct target *target, int hart_index) /* Select all harts that were prepped and that are selectable, clearing the * prepped flag on the harts that actually were selected. */ -static int select_prepped_harts(struct target *target, bool *use_hasel) +static int select_prepped_harts(struct target *target) { + RISCV_INFO(r); dm013_info_t *dm = get_dm(target); if (!dm) return ERROR_FAIL; if (!dm->hasel_supported) { - RISCV_INFO(r); r->prepped = false; - *use_hasel = false; dm013_select_target(target); return ERROR_OK; } @@ -4155,27 +4163,33 @@ static int select_prepped_harts(struct target *target, bool *use_hasel) target_list_t *entry; unsigned total_selected = 0; + unsigned int selected_index = 0; list_for_each_entry(entry, &dm->target_list, list) { struct target *t = entry->target; - riscv_info_t *r = riscv_info(t); - riscv013_info_t *info = get_info(t); - unsigned index = info->index; - LOG_DEBUG("index=%d, coreid=%d, prepped=%d", index, t->coreid, r->prepped); - r->selected = r->prepped; - if (r->prepped) { + riscv_info_t *info = riscv_info(t); + riscv013_info_t *info_013 = get_info(t); + unsigned index = info_013->index; + LOG_DEBUG("index=%d, coreid=%d, prepped=%d", index, t->coreid, info->prepped); + if (info->prepped) { + info_013->selected = true; hawindow[index / 32] |= 1 << (index % 32); - r->prepped = false; + info->prepped = false; total_selected++; + selected_index = index; } - index++; } - /* Don't use hasel if we only need to talk to one hart. */ - if (total_selected <= 1) { - *use_hasel = false; - return ERROR_OK; + if (total_selected == 0) { + LOG_TARGET_ERROR(target, "No harts were prepped!"); + return ERROR_FAIL; + } else if (total_selected == 1) { + /* Don't use hasel if we only need to talk to one hart. */ + return dm013_select_hart(target, selected_index); } + if (dm013_select_hart(target, HART_INDEX_MULTIPLE) != ERROR_OK) + return ERROR_FAIL; + for (unsigned i = 0; i < hawindow_count; i++) { if (dmi_write(target, DM_HAWINDOWSEL, i) != ERROR_OK) return ERROR_FAIL; @@ -4183,7 +4197,6 @@ static int select_prepped_harts(struct target *target, bool *use_hasel) return ERROR_FAIL; } - *use_hasel = true; return ERROR_OK; } @@ -4194,19 +4207,18 @@ static int riscv013_halt_prep(struct target *target) static int riscv013_halt_go(struct target *target) { - RISCV013_INFO(info); + dm013_info_t *dm = get_dm(target); + if (!dm) + return ERROR_FAIL; - bool use_hasel = false; - if (select_prepped_harts(target, &use_hasel) != ERROR_OK) + if (select_prepped_harts(target) != ERROR_OK) return ERROR_FAIL; LOG_TARGET_DEBUG(target, "halting hart"); /* Issue the halt command, and then wait for the current hart to halt. */ uint32_t dmcontrol = DM_DMCONTROL_DMACTIVE | DM_DMCONTROL_HALTREQ; - if (use_hasel) - dmcontrol |= DM_DMCONTROL_HASEL; - dmcontrol = set_hartsel(dmcontrol, info->index); + dmcontrol = set_dmcontrol_hartsel(dmcontrol, dm->current_hartid); dmi_write(target, DM_DMCONTROL, dmcontrol); uint32_t dmstatus; for (size_t i = 0; i < 256; ++i) { @@ -4232,11 +4244,8 @@ static int riscv013_halt_go(struct target *target) dmcontrol = set_field(dmcontrol, DM_DMCONTROL_HALTREQ, 0); dmi_write(target, DM_DMCONTROL, dmcontrol); - if (use_hasel) { + if (dm->current_hartid == HART_INDEX_MULTIPLE) { target_list_t *entry; - dm013_info_t *dm = get_dm(target); - if (!dm) - return ERROR_FAIL; list_for_each_entry(entry, &dm->target_list, list) { struct target *t = entry->target; t->state = TARGET_HALTED; @@ -4251,16 +4260,15 @@ static int riscv013_halt_go(struct target *target) static int riscv013_resume_go(struct target *target) { - bool use_hasel = false; - if (select_prepped_harts(target, &use_hasel) != ERROR_OK) + if (select_prepped_harts(target) != ERROR_OK) return ERROR_FAIL; - return riscv013_step_or_resume_current_hart(target, false, use_hasel); + return riscv013_step_or_resume_current_hart(target, false); } static int riscv013_step_current_hart(struct target *target) { - return riscv013_step_or_resume_current_hart(target, true, false); + return riscv013_step_or_resume_current_hart(target, true); } static int riscv013_resume_prep(struct target *target) @@ -4296,7 +4304,7 @@ static bool riscv013_is_halted(struct target *target) /* TODO: Can we make this more obvious to eg. a gdb user? */ uint32_t dmcontrol = DM_DMCONTROL_DMACTIVE | DM_DMCONTROL_ACKHAVERESET; - dmcontrol = set_hartsel(dmcontrol, info->index); + dmcontrol = set_dmcontrol_hartsel(dmcontrol, info->index); /* If we had been halted when we reset, request another halt. If we * ended up running out of reset, then the user will (hopefully) get a * message that a reset happened, that the target is running, and then @@ -4799,9 +4807,9 @@ static int riscv013_on_step_or_resume(struct target *target, bool step) } static int riscv013_step_or_resume_current_hart(struct target *target, - bool step, bool use_hasel) + bool step) { - if (!riscv_is_halted(target)) { + if (target->state != TARGET_HALTED) { LOG_TARGET_ERROR(target, "Hart is not halted!"); return ERROR_FAIL; } @@ -4813,12 +4821,9 @@ static int riscv013_step_or_resume_current_hart(struct target *target, dm013_info_t *dm = get_dm(target); /* Issue the resume command, and then wait for the current hart to resume. */ uint32_t dmcontrol = DM_DMCONTROL_DMACTIVE | DM_DMCONTROL_RESUMEREQ; - if (use_hasel) - dmcontrol |= DM_DMCONTROL_HASEL; - dmcontrol = set_hartsel(dmcontrol, dm->current_hartid); + dmcontrol = set_dmcontrol_hartsel(dmcontrol, dm->current_hartid); dmi_write(target, DM_DMCONTROL, dmcontrol); - dmcontrol = set_field(dmcontrol, DM_DMCONTROL_HASEL, 0); dmcontrol = set_field(dmcontrol, DM_DMCONTROL_RESUMEREQ, 0); uint32_t dmstatus;