zcy
/
tinyriscv-openocd
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Handle DMI busy in sba write. (#437) 

* Handle DMI busy in sba write.

If we encounter DMI busy on the NOP after a read, we'll never get the
value out because DMI busy is sticky. The read must be retried, but we
don't know whether it was ever issued. Since the read has side effects
(incrementing of the address) this retry must be handled at a higher
layer. So now dmi_op_timeout can be told to retry or not, and if retry
is disabled it'll return an error when busy.

Also actually properly do the retry in dmi_op_timeout(). Previously the
code would not reissue the command and end up returning a garbage value.

Change-Id: I3b52ebd51ebbbedd6e425676ac861b57fbe711b1

* Fix whitespace.

Change-Id: Icb76d964e681b22346368d224d1930c9342343f3

* Handle a few more DMI busy cases.

Change-Id: I8503a44e4bf935c0ebfff0d598fe4c322fda702a

* Explain when to use dmi_op_timeout(retry).

Change-Id: I1a5c6d76ac41a84472a8f79faecb2f48105191ff

* dmi_reset does not affect the current transaction.

That means the retry scheme we had been using works fine. This does
contain some minor tweaks, and now we pass my tests which hammer the DMI
busy case harder.

Change-Id: I13eee384dbba82bc5a5b1d387c75c547afe557b5

* Remove unnecessary changes to make the PR readable

Change-Id: I87079876e6965563cf590e3936b3595aeab8715d

* Move idle to end of line...

... because we go through run-test/idle after the scan.

Change-Id: I21a8cff22471f0b895d8cd8d25373dced9bf1ca9

* Remove unused code.

Change-Id: I07a7cdd2d64ca40a4fe181111a34cf55ff1928d1
vector2
Tim Newsome 2020-01-13 15:10:43 -08:00 committed by GitHub
parent fcea4f79ba
commit 69e6891434
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 54 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/target/riscv/batch.c
View File

@ -162,14 +162,13 @@ void dump_field(int idle, const struct scan_field *field)
log_printf_lf(LOG_LVL_DEBUG, log_printf_lf(LOG_LVL_DEBUG,
__FILE__, __LINE__, __PRETTY_FUNCTION__, __FILE__, __LINE__, __PRETTY_FUNCTION__,
"%db %di %s %08x @%02x -> %s %08x @%02x", "%db %s %08x @%02x -> %s %08x @%02x; %di",
field->num_bits, idle, field->num_bits, op_string[out_op], out_data, out_address,
op_string[out_op], out_data, out_address, status_string[in_op], in_data, in_address, idle);
status_string[in_op], in_data, in_address);
} else { } else {
log_printf_lf(LOG_LVL_DEBUG, log_printf_lf(LOG_LVL_DEBUG,
__FILE__, __LINE__, __PRETTY_FUNCTION__, "%db %di %s %08x @%02x -> ?", __FILE__, __LINE__, __PRETTY_FUNCTION__, "%db %s %08x @%02x -> ?; %di",
field->num_bits, idle, op_string[out_op], out_data, out_address); field->num_bits, op_string[out_op], out_data, out_address, idle);
} }
} }

94
src/target/riscv/riscv-013.c
View File

@ -395,10 +395,9 @@ static void dump_field(int idle, const struct scan_field *field)
log_printf_lf(LOG_LVL_DEBUG, log_printf_lf(LOG_LVL_DEBUG,
__FILE__, __LINE__, "scan", __FILE__, __LINE__, "scan",
"%db %di %s %08x @%02x -> %s %08x @%02x", "%db %s %08x @%02x -> %s %08x @%02x; %di",
field->num_bits, idle, field->num_bits, op_string[out_op], out_data, out_address,
op_string[out_op], out_data, out_address, status_string[in_op], in_data, in_address, idle);
status_string[in_op], in_data, in_address);
char out_text[500]; char out_text[500];
char in_text[500]; char in_text[500];
@ -542,8 +541,20 @@ static dmi_status_t dmi_scan(struct target *target, uint32_t *address_in,
return buf_get_u32(in, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH); return buf_get_u32(in, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH);
} }
/* If dmi_busy_encountered is non-NULL, this function will use it to tell the /**
* caller whether DMI was ever busy during this call. */ * @param data_in The data we received from the target.
* @param dmi_op The operation to perform (read/write/nop).
* @param dmi_busy_encountered
* If non-NULL, will be updated to reflect whether DMI busy was
* encountered while executing this operation or not.
* @param address The address argument to that operation.
* @param data_out The data to send to the target.
* @param exec When true, this scan will execute something, so extra RTI
* cycles may be added.
* @param ensure_success
* Scan a nop after the requested operation, ensuring the
* DMI operation succeeded.
*/
static int dmi_op_timeout(struct target *target, uint32_t *data_in, static int dmi_op_timeout(struct target *target, uint32_t *data_in,
bool *dmi_busy_encountered, int dmi_op, uint32_t address, bool *dmi_busy_encountered, int dmi_op, uint32_t address,
uint32_t data_out, int timeout_sec, bool exec, bool ensure_success) uint32_t data_out, int timeout_sec, bool exec, bool ensure_success)
@ -606,27 +617,23 @@ static int dmi_op_timeout(struct target *target, uint32_t *data_in,
false); false);
if (status == DMI_STATUS_BUSY) { if (status == DMI_STATUS_BUSY) {
increase_dmi_busy_delay(target); increase_dmi_busy_delay(target);
if (dmi_busy_encountered)
*dmi_busy_encountered = true;
} else if (status == DMI_STATUS_SUCCESS) { } else if (status == DMI_STATUS_SUCCESS) {
break; break;
} else { } else {
LOG_ERROR("failed %s (NOP) at 0x%x, status=%d", op_name, address, if (data_in) {
LOG_ERROR("Failed %s (NOP) at 0x%x; value=0x%x, status=%d",
op_name, address, *data_in, status);
} else {
LOG_ERROR("Failed %s (NOP) at 0x%x; status=%d", op_name, address,
status); status);
}
return ERROR_FAIL; return ERROR_FAIL;
} }
if (time(NULL) - start > timeout_sec) if (time(NULL) - start > timeout_sec)
return ERROR_TIMEOUT_REACHED; return ERROR_TIMEOUT_REACHED;
} }
if (status != DMI_STATUS_SUCCESS) {
if (status == DMI_STATUS_FAILED || !data_in) {
LOG_ERROR("Failed %s (NOP) at 0x%x; status=%d", op_name, address,
status);
} else {
LOG_ERROR("Failed %s (NOP) at 0x%x; value=0x%x, status=%d",
op_name, address, *data_in, status);
}
return ERROR_FAIL;
}
} }
return ERROR_OK; return ERROR_OK;
@ -2063,28 +2070,16 @@ static int read_memory_bus_word(struct target *target, target_addr_t address,
uint32_t size, uint8_t *buffer) uint32_t size, uint8_t *buffer)
{ {
uint32_t value; uint32_t value;
if (size > 12) { int result;
if (dmi_read(target, &value, DMI_SBDATA3) != ERROR_OK) static int sbdata[4] = { DMI_SBDATA0, DMI_SBDATA1, DMI_SBDATA2, DMI_SBDATA3 };
return ERROR_FAIL; assert(size <= 16);
write_to_buf(buffer + 12, value, 4); for (int i = (size-1) / 4; i >= 0; i--) {
log_memory_access(address + 12, value, 4, true); result = dmi_op(target, &value, NULL, DMI_OP_READ, sbdata[i], 0, false, true);
if (result != ERROR_OK)
return result;
write_to_buf(buffer + i * 4, value, MIN(size, 4));
log_memory_access(address + i * 4, value, MIN(size, 4), true);
} }
if (size > 8) {
if (dmi_read(target, &value, DMI_SBDATA2) != ERROR_OK)
return ERROR_FAIL;
write_to_buf(buffer + 8, value, 4);
log_memory_access(address + 8, value, 4, true);
}
if (size > 4) {
if (dmi_read(target, &value, DMI_SBDATA1) != ERROR_OK)
return ERROR_FAIL;
write_to_buf(buffer + 4, value, 4);
log_memory_access(address + 4, value, 4, true);
}
if (dmi_read(target, &value, DMI_SBDATA0) != ERROR_OK)
return ERROR_FAIL;
write_to_buf(buffer, value, MIN(size, 4));
log_memory_access(address, value, MIN(size, 4), true);
return ERROR_OK; return ERROR_OK;
} }
@ -2318,6 +2313,7 @@ static int read_memory_bus_v1(struct target *target, target_addr_t address,
return ERROR_FAIL; return ERROR_FAIL;
} }
/* Read the last word, after we disabled sbreadondata if necessary. */
if (!get_field(sbcs_read, DMI_SBCS_SBERROR) && if (!get_field(sbcs_read, DMI_SBCS_SBERROR) &&
!get_field(sbcs_read, DMI_SBCS_SBBUSYERROR)) { !get_field(sbcs_read, DMI_SBCS_SBBUSYERROR)) {
if (read_memory_bus_word(target, address + (count - 1) * size, size, if (read_memory_bus_word(target, address + (count - 1) * size, size,
@ -2499,10 +2495,10 @@ static int read_memory_progbuf_inner(struct target *target, target_addr_t addres
int result = ERROR_OK; int result = ERROR_OK;
/* Write address to S0, and execute buffer. */ /* Write address to S0. */
result = register_write_direct(target, GDB_REGNO_S0, address); result = register_write_direct(target, GDB_REGNO_S0, address);
if (result != ERROR_OK) if (result != ERROR_OK)
goto error; return result;
uint32_t command = access_register_command(target, GDB_REGNO_S1, uint32_t command = access_register_command(target, GDB_REGNO_S1,
riscv_xlen(target), riscv_xlen(target),
AC_ACCESS_REGISTER_TRANSFER | AC_ACCESS_REGISTER_POSTEXEC); AC_ACCESS_REGISTER_TRANSFER | AC_ACCESS_REGISTER_POSTEXEC);
@ -2510,7 +2506,6 @@ static int read_memory_progbuf_inner(struct target *target, target_addr_t addres
return ERROR_FAIL; return ERROR_FAIL;
/* First read has just triggered. Result is in s1. */ /* First read has just triggered. Result is in s1. */
if (count == 1) { if (count == 1) {
uint64_t value; uint64_t value;
if (register_read_direct(target, &value, GDB_REGNO_S1) != ERROR_OK) if (register_read_direct(target, &value, GDB_REGNO_S1) != ERROR_OK)
@ -3091,7 +3086,8 @@ static int write_memory_bus_v1(struct target *target, target_addr_t address,
return ERROR_FAIL; return ERROR_FAIL;
time_t start = time(NULL); time_t start = time(NULL);
while (get_field(sbcs, DMI_SBCS_SBBUSY)) { bool dmi_busy = dmi_busy_encountered;
while (get_field(sbcs, DMI_SBCS_SBBUSY) || dmi_busy) {
if (time(NULL) - start > riscv_command_timeout_sec) { if (time(NULL) - start > riscv_command_timeout_sec) {
LOG_ERROR("Timed out after %ds waiting for sbbusy to go low (sbcs=0x%x). " LOG_ERROR("Timed out after %ds waiting for sbbusy to go low (sbcs=0x%x). "
"Increase the timeout with riscv set_command_timeout_sec.", "Increase the timeout with riscv set_command_timeout_sec.",
@ -3099,15 +3095,18 @@ static int write_memory_bus_v1(struct target *target, target_addr_t address,
return ERROR_FAIL; return ERROR_FAIL;
} }
if (dmi_read(target, &sbcs, DMI_SBCS) != ERROR_OK) if (dmi_op(target, &sbcs, &dmi_busy, DMI_OP_READ,
DMI_SBCS, 0, false, true) != ERROR_OK)
return ERROR_FAIL; return ERROR_FAIL;
} }
if (get_field(sbcs, DMI_SBCS_SBBUSYERROR) || dmi_busy_encountered) { if (get_field(sbcs, DMI_SBCS_SBBUSYERROR)) {
/* We wrote while the target was busy. Slow down and try again. */ /* We wrote while the target was busy. Slow down and try again. */
dmi_write(target, DMI_SBCS, DMI_SBCS_SBBUSYERROR); dmi_write(target, DMI_SBCS, DMI_SBCS_SBBUSYERROR);
info->bus_master_write_delay += info->bus_master_write_delay / 10 + 1; info->bus_master_write_delay += info->bus_master_write_delay / 10 + 1;
}
if (get_field(sbcs, DMI_SBCS_SBBUSYERROR) || dmi_busy_encountered) {
next_address = sb_read_address(target); next_address = sb_read_address(target);
if (next_address < address) { if (next_address < address) {
/* This should never happen, probably buggy hardware. */ /* This should never happen, probably buggy hardware. */
@ -3271,8 +3270,9 @@ static int write_memory_progbuf(struct target *target, target_addr_t address,
uint32_t abstractcs; uint32_t abstractcs;
bool dmi_busy_encountered; bool dmi_busy_encountered;
if (dmi_op(target, &abstractcs, &dmi_busy_encountered, DMI_OP_READ, result = dmi_op(target, &abstractcs, &dmi_busy_encountered,
DMI_ABSTRACTCS, 0, false, true) != ERROR_OK) DMI_OP_READ, DMI_ABSTRACTCS, 0, false, true);
if (result != ERROR_OK)
goto error; goto error;
while (get_field(abstractcs, DMI_ABSTRACTCS_BUSY)) while (get_field(abstractcs, DMI_ABSTRACTCS_BUSY))
if (dmi_read(target, &abstractcs, DMI_ABSTRACTCS) != ERROR_OK) if (dmi_read(target, &abstractcs, DMI_ABSTRACTCS) != ERROR_OK)