Skip to content

target/riscv: Fix some timeout check order #1265

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

Open
wants to merge 3 commits into
base: riscv
Choose a base branch
from
Open

target/riscv: Fix some timeout check order #1265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d5f2fe
Update image in `build32`
en-sc Apr 29, 2025
a68695b
target/riscv: active dm before get nextdm
lz-bro May 6, 2025
ded5f59
target/riscv: Fix some timeout check order
lz-bro Jun 13, 2025
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
2 changes: 1 addition & 1 deletion .github/workflows/linux-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ name: Linux Build
jobs:
# 32-bit, clang
build32:
runs-on: ubuntu-20.04
runs-on: ubuntu-latest
env:
CFLAGS: -m32
CC: clang
Expand Down
93 changes: 58 additions & 35 deletions src/target/riscv/riscv-013.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,33 @@ static int dm_write(struct target *target, uint32_t address, uint32_t value)
return dmi_write(target, riscv013_get_dmi_address(target, address), value);
}

static bool check_dbgbase_exists(struct target *target)
static int activate_dm(struct target *target, uint32_t dm_base_addr)
{
uint32_t dmcontrol = 0;

LOG_TARGET_DEBUG(target, "Activating the DM with DMI base address (dbgbase) = 0x%x", dm_base_addr);
if (dmi_write(target, DM_DMCONTROL + dm_base_addr, DM_DMCONTROL_DMACTIVE) != ERROR_OK)
return ERROR_FAIL;

const time_t start = time(NULL);
LOG_TARGET_DEBUG(target, "Waiting for the DM to become active.");
while (1) {
if (dmi_read(target, &dmcontrol, DM_DMCONTROL + dm_base_addr) != ERROR_OK)
return ERROR_FAIL;
if (get_field32(dmcontrol, DM_DMCONTROL_DMACTIVE))
break;
if (time(NULL) - start > riscv_get_command_timeout_sec()) {
LOG_TARGET_ERROR(target, "Debug Module (at address dbgbase=0x%" PRIx32 ") did not become active in %d s. "
"Increase the timeout with 'riscv set_command_timeout_sec'.",
dm_base_addr, riscv_get_command_timeout_sec());
return ERROR_TIMEOUT_REACHED;
}
}
LOG_TARGET_DEBUG(target, "DM has become active.");
return ERROR_OK;
}

static int check_dbgbase_exists(struct target *target)
{
uint32_t next_dm = 0;
unsigned int count = 1;
Expand All @@ -543,7 +569,14 @@ static bool check_dbgbase_exists(struct target *target)
while (1) {
uint32_t current_dm = next_dm;
if (current_dm == target->dbgbase)
return true;
return ERROR_OK;

uint32_t dmcontrol = 0;
if (dmi_read(target, &dmcontrol, DM_DMCONTROL + current_dm) != ERROR_OK)
break;
if (!get_field32(dmcontrol, DM_DMCONTROL_DMACTIVE) && activate_dm(target, current_dm) != ERROR_OK)
break;

if (dmi_read(target, &next_dm, DM_NEXTDM + current_dm) != ERROR_OK)
break;
LOG_TARGET_DEBUG(target, "dm @ 0x%x --> nextdm=0x%x", current_dm, next_dm);
Expand All @@ -558,7 +591,7 @@ static bool check_dbgbase_exists(struct target *target)
break;
}
}
return false;
return ERROR_FAIL;
}

static int dmstatus_read(struct target *target, uint32_t *dmstatus,
Expand Down Expand Up @@ -1854,41 +1887,29 @@ static int reset_dm(struct target *target)

const time_t start = time(NULL);
LOG_TARGET_DEBUG(target, "Waiting for the DM to acknowledge reset.");
do {
while (1) {
result = dm_read(target, &dmcontrol, DM_DMCONTROL);
if (result != ERROR_OK)
return result;

if (!get_field32(dmcontrol, DM_DMCONTROL_DMACTIVE)) {
LOG_TARGET_DEBUG(target, "The DM has just become deactivated (dmcontrol.dmactive: 1 -> 0).");
break;
}
if (time(NULL) - start > riscv_get_command_timeout_sec()) {
LOG_TARGET_ERROR(target, "DM didn't acknowledge reset in %d s. "
"Increase the timeout with 'riscv set_command_timeout_sec'.",
riscv_get_command_timeout_sec());
return ERROR_TIMEOUT_REACHED;
}
} while (get_field32(dmcontrol, DM_DMCONTROL_DMACTIVE));
LOG_TARGET_DEBUG(target, "DM reset initiated.");
}
}
/* TODO: Move the code above into `deactivate_dm()` function
* (a logical counterpart to activate_dm()). */

LOG_TARGET_DEBUG(target, "Activating the DM.");
result = dm_write(target, DM_DMCONTROL, DM_DMCONTROL_DMACTIVE);
result = activate_dm(target, dm->base);
if (result != ERROR_OK)
return result;

const time_t start = time(NULL);
LOG_TARGET_DEBUG(target, "Waiting for the DM to come out of reset.");
do {
result = dm_read(target, &dmcontrol, DM_DMCONTROL);
if (result != ERROR_OK)
return result;

if (time(NULL) - start > riscv_get_command_timeout_sec()) {
LOG_TARGET_ERROR(target, "Debug Module did not become active in %d s. "
"Increase the timeout with 'riscv set_command_timeout_sec'.",
riscv_get_command_timeout_sec());
return ERROR_TIMEOUT_REACHED;
}
} while (!get_field32(dmcontrol, DM_DMCONTROL_DMACTIVE));

LOG_TARGET_DEBUG(target, "DM successfully reset.");
dm->was_reset = true;
return ERROR_OK;
Expand Down Expand Up @@ -2043,7 +2064,7 @@ static int examine(struct target *target)
info->abits, RISCV013_DTMCS_ABITS_MIN);
}

if (!check_dbgbase_exists(target)) {
if (check_dbgbase_exists(target) != ERROR_OK) {
LOG_TARGET_ERROR(target, "Could not find debug module with DMI base address (dbgbase) = 0x%x", target->dbgbase);
return ERROR_FAIL;
}
Expand Down Expand Up @@ -2980,10 +3001,20 @@ static int deassert_reset(struct target *target)
RISCV_DELAY_BASE);
time_t start = time(NULL);
LOG_TARGET_DEBUG(target, "Waiting for hart to come out of reset.");
do {
while (1) {
result = dmstatus_read(target, &dmstatus, true);
if (result != ERROR_OK)
return result;
/* Certain debug modules, like the one in GD32VF103
* MCUs, violate the specification's requirement that
* each hart is in "exactly one of four states" and,
* during reset, report harts as both unavailable and
* halted/running. To work around this, we check for
* the absence of the unavailable state rather than
* the presence of any other state. */
if (!get_field(dmstatus, DM_DMSTATUS_ALLUNAVAIL) ||
get_field(dmstatus, DM_DMSTATUS_ALLHAVERESET))
break;

if (time(NULL) - start > riscv_get_command_timeout_sec()) {
LOG_TARGET_ERROR(target, "Hart didn't leave reset in %ds; "
Expand All @@ -2994,15 +3025,7 @@ static int deassert_reset(struct target *target)
get_field(dmstatus, DM_DMSTATUS_ALLHAVERESET) ? "true" : "false");
return ERROR_TIMEOUT_REACHED;
}
/* Certain debug modules, like the one in GD32VF103
* MCUs, violate the specification's requirement that
* each hart is in "exactly one of four states" and,
* during reset, report harts as both unavailable and
* halted/running. To work around this, we check for
* the absence of the unavailable state rather than
* the presence of any other state. */
} while (get_field(dmstatus, DM_DMSTATUS_ALLUNAVAIL) &&
!get_field(dmstatus, DM_DMSTATUS_ALLHAVERESET));
}

riscv_scan_set_delay(&info->learned_delays, RISCV_DELAY_BASE,
orig_base_delay);
Expand Down