From 9e174604b95433db106ae06267c5cb33884c1e43 Mon Sep 17 00:00:00 2001 From: Jan Matyas <50193733+JanMatCodasip@users.noreply.github.com> Date: Thu, 4 Feb 2021 19:04:01 +0100 Subject: [PATCH] Fix in write_memory_bus_v1: Read sbcs properly (#578) * Fix in write_memory_bus_v1: Read sbcs properly Fixed an issue with system bus write: sberrors were not properly detected because of an incomplete read of "sbcs". The read was initiated but not completed (value not acquired by a second DMI scan). Added debug prints in case sberror != 0. Added few comments to explain the algorithm. Change-Id: Id5eb07f2f1bf8e9afee2dec04b9ff5c5a57f606b Signed-off-by: Jan Matyas * Updated per review discussion at #577. Change-Id: I65c07edcd4e86eaa5327280a81f74db0b9c84f9c Signed-off-by: Jan Matyas * Empty commit to re-trigger Travis build. Change-Id: I95deeb28584a891203c8904be621e48003f069dc Co-authored-by: Jan Matyas --- src/target/riscv/riscv-013.c | 54 +++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index b8b513d6c..87aff0683 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -3713,54 +3713,76 @@ static int write_memory_bus_v1(struct target *target, target_addr_t address, next_address += size; } + /* Execute the batch of writes */ result = batch_run(target, batch); riscv_batch_free(batch); if (result != ERROR_OK) return result; + /* Read sbcs value. + * At the same time, detect if DMI busy has occurred during the batch write. */ bool dmi_busy_encountered; if (dmi_op(target, &sbcs, &dmi_busy_encountered, DMI_OP_READ, - DM_SBCS, 0, false, false) != ERROR_OK) + DM_SBCS, 0, false, true) != ERROR_OK) return ERROR_FAIL; + if (dmi_busy_encountered) + LOG_DEBUG("DMI busy encountered during system bus write."); + /* Wait until sbbusy goes low */ time_t start = time(NULL); - bool dmi_busy = dmi_busy_encountered; - while (get_field(sbcs, DM_SBCS_SBBUSY) || dmi_busy) { + while (get_field(sbcs, DM_SBCS_SBBUSY)) { if (time(NULL) - start > riscv_command_timeout_sec) { LOG_ERROR("Timed out after %ds waiting for sbbusy to go low (sbcs=0x%x). " - "Increase the timeout with riscv set_command_timeout_sec.", - riscv_command_timeout_sec, sbcs); + "Increase the timeout with riscv set_command_timeout_sec.", + riscv_command_timeout_sec, sbcs); return ERROR_FAIL; } - - if (dmi_op(target, &sbcs, &dmi_busy, DMI_OP_READ, - DM_SBCS, 0, false, true) != ERROR_OK) + if (dmi_read(target, &sbcs, DM_SBCS) != ERROR_OK) return ERROR_FAIL; } if (get_field(sbcs, DM_SBCS_SBBUSYERROR)) { - /* We wrote while the target was busy. Slow down and try again. */ + /* We wrote while the target was busy. */ + LOG_DEBUG("Sbbusyerror encountered during system bus write."); + /* Clear the sticky error flag. */ dmi_write(target, DM_SBCS, sbcs | DM_SBCS_SBBUSYERROR); + /* Slow down before trying again. */ info->bus_master_write_delay += info->bus_master_write_delay / 10 + 1; } if (get_field(sbcs, DM_SBCS_SBBUSYERROR) || dmi_busy_encountered) { + /* Recover from the case when the write commands were issued too fast. + * Determine the address from which to resume writing. */ next_address = sb_read_address(target); if (next_address < address) { /* This should never happen, probably buggy hardware. */ - LOG_DEBUG("unexpected system bus address 0x%" TARGET_PRIxADDR, - next_address); + LOG_DEBUG("unexpected sbaddress=0x%" TARGET_PRIxADDR + " - buggy sbautoincrement in hw?", next_address); + /* Fail the whole operation. */ return ERROR_FAIL; } - + /* Try again - resume writing. */ continue; } - unsigned error = get_field(sbcs, DM_SBCS_SBERROR); - if (error != 0) { - /* Some error indicating the bus access failed, but not because of - * something we did wrong. */ + unsigned sberror = get_field(sbcs, DM_SBCS_SBERROR); + if (sberror != 0) { + /* Sberror indicates the bus access failed, but not because we issued the writes + * too fast. Cannot recover. Sbaddress holds the address where the error occurred + * (unless sbautoincrement in the HW is buggy). + */ + target_addr_t sbaddress = sb_read_address(target); + LOG_DEBUG("System bus access failed with sberror=%u (sbaddress=0x%" TARGET_PRIxADDR ")", + sberror, sbaddress); + if (sbaddress < address) { + /* This should never happen, probably buggy hardware. + * Make a note to the user not to trust the sbaddress value. */ + LOG_DEBUG("unexpected sbaddress=0x%" TARGET_PRIxADDR + " - buggy sbautoincrement in hw?", next_address); + } + /* Clear the sticky error flag */ dmi_write(target, DM_SBCS, DM_SBCS_SBERROR); + /* Fail the whole operation */ return ERROR_FAIL; } }