From 9c1f6ea28b87704474e5a33cb2062a00ddd3a255 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 4 Apr 2017 16:22:18 -0700 Subject: [PATCH 1/2] riscv: Correct the autoexec in read_mem --- src/target/riscv/riscv-013.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 8dfdde448..ec9f95146 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1859,11 +1859,21 @@ static int read_memory(struct target *target, uint32_t address, return ERROR_FAIL; } + // Set up autoexec s.t. each read of the the result that was in S1 + // will start another run of reading the address pointed to by S0, + // copying it to S1, and storing S1 into Data 0. + if (count > 1) { + dmi_write(target, DMI_ABSTRACTAUTO, 0x1 << DMI_ABSTRACTAUTO_AUTOEXECDATA_OFFSET); + } + uint32_t abstractcs; for (uint32_t i = 0; i < count; i++) { uint32_t value = dmi_read(target, DMI_DATA0); - if (i == 0) - dmi_write(target, DMI_ABSTRACTAUTO, 0x1 << DMI_ABSTRACTAUTO_AUTOEXECDATA_OFFSET); + // On last iteration, turn off autoexec before reading the value + // so that we don't inadvertently read too far into memory. + if ((count > 1) && ((i + 1) == count)) { + dmi_write(target, DMI_ABSTRACTAUTO, 0); + } switch (size) { case 1: buffer[i] = value; @@ -1889,8 +1899,7 @@ static int read_memory(struct target *target, uint32_t address, return ERROR_FAIL; } } - dmi_write(target, DMI_ABSTRACTAUTO, 0); - abstractcs = dmi_read(target, DMI_ABSTRACTCS); + abstractcs = dmi_read(target, DMI_ABSTRACTCS); unsigned cmderr = get_field(abstractcs, DMI_ABSTRACTCS_CMDERR); if (cmderr == CMDERR_BUSY) { // Clear the error and wait longer. From 14e26040b8c303d5feba7d735b62382e1b7aa325 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 4 Apr 2017 16:33:17 -0700 Subject: [PATCH 2/2] riscv: move value read to after autoexec is cleared. --- src/target/riscv/riscv-013.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index ec9f95146..2cd186d22 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1868,12 +1868,25 @@ static int read_memory(struct target *target, uint32_t address, uint32_t abstractcs; for (uint32_t i = 0; i < count; i++) { - uint32_t value = dmi_read(target, DMI_DATA0); + // On last iteration, turn off autoexec before reading the value // so that we don't inadvertently read too far into memory. if ((count > 1) && ((i + 1) == count)) { dmi_write(target, DMI_ABSTRACTAUTO, 0); } + + uint32_t value = dmi_read(target, DMI_DATA0); + // If autoexec was set, the above dmi_read started an abstract command. + // If we just immediately loop and do another read here, + // we'll probably get a busy error. Wait for idle first, + // or otherwise take ac_command_busy into account (this defeats the purpose + // of autoexec, this whole code needs optimization). + if ((count > 1) && ((i + 1) < count)) { + if (wait_for_idle(target, &abstractcs) != ERROR_OK) { + dmi_write(target, DMI_ABSTRACTAUTO, 0); + return ERROR_FAIL; + } + } switch (size) { case 1: buffer[i] = value; @@ -1891,14 +1904,8 @@ static int read_memory(struct target *target, uint32_t address, default: return ERROR_FAIL; } - // The above dmi_read started an abstract command. If we just - // immediately read here, we'll probably get a busy error. Wait for idle first, - // or otherwise take ac_command_busy into account (this defeats the purpose - // of autoexec, this whole code needs optimization). - if (wait_for_idle(target, &abstractcs) != ERROR_OK) { - return ERROR_FAIL; - } } + abstractcs = dmi_read(target, DMI_ABSTRACTCS); unsigned cmderr = get_field(abstractcs, DMI_ABSTRACTCS_CMDERR); if (cmderr == CMDERR_BUSY) {