From beac00149cc07a05234119727068bc5b85087429 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 9 Jan 2018 12:29:16 -0800 Subject: [PATCH 1/7] Use new debug_defines.h Change-Id: Iefc8424343dbed05fa9dacc626829955fc16f299 --- src/target/riscv/debug_defines.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/target/riscv/debug_defines.h b/src/target/riscv/debug_defines.h index e970293bf..58826f08b 100644 --- a/src/target/riscv/debug_defines.h +++ b/src/target/riscv/debug_defines.h @@ -1007,14 +1007,14 @@ #define DMI_COMMAND_CONTROL (0xffffffU << DMI_COMMAND_CONTROL_OFFSET) #define DMI_ABSTRACTAUTO 0x18 /* -* When a bit in this field is 1, read or write accesses the corresponding {\tt progbuf} word +* When a bit in this field is 1, read or write accesses to the corresponding {\tt progbuf} word * cause the command in \Rcommand to be executed again. */ #define DMI_ABSTRACTAUTO_AUTOEXECPROGBUF_OFFSET 16 #define DMI_ABSTRACTAUTO_AUTOEXECPROGBUF_LENGTH 16 #define DMI_ABSTRACTAUTO_AUTOEXECPROGBUF (0xffffU << DMI_ABSTRACTAUTO_AUTOEXECPROGBUF_OFFSET) /* -* When a bit in this field is 1, read or write accesses the corresponding {\tt data} word +* When a bit in this field is 1, read or write accesses to the corresponding {\tt data} word * cause the command in \Rcommand to be executed again. */ #define DMI_ABSTRACTAUTO_AUTOEXECDATA_OFFSET 0 @@ -1043,15 +1043,15 @@ #define DMI_AUTHDATA_DATA (0xffffffffU << DMI_AUTHDATA_DATA_OFFSET) #define DMI_SBCS 0x38 /* -* When a 1 is written here, triggers a read at the address in {\tt -* sbaddress} using the access size set by \Fsbaccess. +* When a 1, every write to \Rsbaddresszero automatically triggers a +* system bus read at the new address. */ -#define DMI_SBCS_SBSINGLEREAD_OFFSET 20 -#define DMI_SBCS_SBSINGLEREAD_LENGTH 1 -#define DMI_SBCS_SBSINGLEREAD (0x1U << DMI_SBCS_SBSINGLEREAD_OFFSET) +#define DMI_SBCS_SBREADONADDR_OFFSET 20 +#define DMI_SBCS_SBREADONADDR_LENGTH 1 +#define DMI_SBCS_SBREADONADDR (0x1U << DMI_SBCS_SBREADONADDR_OFFSET) /* * Select the access size to use for system bus accesses triggered by -* writes to the {\tt sbaddress} registers or \Rsbdatazero. +* writes to \Rsbaddresszero or \Rsbdatazero. * * 0: 8-bit * @@ -1080,9 +1080,9 @@ * When 1, every read from \Rsbdatazero automatically triggers a * system bus read at the (possibly auto-incremented) address. */ -#define DMI_SBCS_SBAUTOREAD_OFFSET 15 -#define DMI_SBCS_SBAUTOREAD_LENGTH 1 -#define DMI_SBCS_SBAUTOREAD (0x1U << DMI_SBCS_SBAUTOREAD_OFFSET) +#define DMI_SBCS_SBREADONDATA_OFFSET 15 +#define DMI_SBCS_SBREADONDATA_LENGTH 1 +#define DMI_SBCS_SBREADONDATA (0x1U << DMI_SBCS_SBREADONDATA_OFFSET) /* * When the debug module's system bus * master causes a bus error, this field gets set. The bits in this From b67379700b2c7b706ae01577775e5b55c8a1b73e Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 18 Jan 2018 14:32:01 -0800 Subject: [PATCH 2/7] Add support for v1 system bus access. This is functional, but doesn't handle errors. Change-Id: Ifb46af1b0b567f3c2a6135b2ad5eb7ba63a3f595 --- src/target/riscv/debug_defines.h | 45 +++++-- src/target/riscv/riscv-013.c | 214 +++++++++++++++++++++++++++++-- 2 files changed, 240 insertions(+), 19 deletions(-) diff --git a/src/target/riscv/debug_defines.h b/src/target/riscv/debug_defines.h index 58826f08b..8cf2d9658 100644 --- a/src/target/riscv/debug_defines.h +++ b/src/target/riscv/debug_defines.h @@ -1041,17 +1041,47 @@ #define DMI_AUTHDATA_DATA_OFFSET 0 #define DMI_AUTHDATA_DATA_LENGTH 32 #define DMI_AUTHDATA_DATA (0xffffffffU << DMI_AUTHDATA_DATA_OFFSET) +#define DMI_SBADDRESS3 0x37 +/* +* Accesses bits 127:96 of the physical address in {\tt sbaddress} (if +* the system address bus is that wide). + */ +#define DMI_SBADDRESS3_ADDRESS_OFFSET 0 +#define DMI_SBADDRESS3_ADDRESS_LENGTH 32 +#define DMI_SBADDRESS3_ADDRESS (0xffffffffU << DMI_SBADDRESS3_ADDRESS_OFFSET) #define DMI_SBCS 0x38 /* -* When a 1, every write to \Rsbaddresszero automatically triggers a +* 0: The System Bus interface conforms to mainline drafts of this +* spec older than 1 January, 2018. +* +* 1: The System Bus interface conforms to this version of the spec. +* +* Other values are reserved for future versions. + */ +#define DMI_SBCS_SBVERSION_OFFSET 29 +#define DMI_SBCS_SBVERSION_LENGTH 3 +#define DMI_SBCS_SBVERSION (0x7U << DMI_SBCS_SBVERSION_OFFSET) +/* +* When 1, indicates the system bus master is busy. (Whether the +* system bus itself is busy is related, but not the same thing.) This +* bit goes high immediately when a read or write is requested for any +* reason, and does not go low until the access is fully completed. +* +* To avoid race conditions, debuggers must not try to clear \Fsberror +* until they read \Fsbbusy as 0. + */ +#define DMI_SBCS_SBBUSY_OFFSET 21 +#define DMI_SBCS_SBBUSY_LENGTH 1 +#define DMI_SBCS_SBBUSY (0x1U << DMI_SBCS_SBBUSY_OFFSET) +/* +* When 1, every write to \Rsbaddresszero automatically triggers a * system bus read at the new address. */ #define DMI_SBCS_SBREADONADDR_OFFSET 20 #define DMI_SBCS_SBREADONADDR_LENGTH 1 #define DMI_SBCS_SBREADONADDR (0x1U << DMI_SBCS_SBREADONADDR_OFFSET) /* -* Select the access size to use for system bus accesses triggered by -* writes to \Rsbaddresszero or \Rsbdatazero. +* Select the access size to use for system bus accesses. * * 0: 8-bit * @@ -1063,8 +1093,8 @@ * * 4: 128-bit * -* If an unsupported system bus access size is written here, the DM -* does not perform the access and sberror is set to 3. +* If \Fsbaccess has an unsupported value when the DM starts a bus +* access, the access is not performed and \Fsberror is set to 3. */ #define DMI_SBCS_SBACCESS_OFFSET 17 #define DMI_SBCS_SBACCESS_LENGTH 3 @@ -1098,9 +1128,8 @@ * * 3: There was some other error (eg. alignment). * -* 4: The system bus master was busy when one of the -* {\tt sbaddress} or {\tt sbdata} registers was written, -* or \Rsbdatazero was read when it had stale data. +* 4: The system bus master was busy when one of the {\tt sbaddress} +* was written, or one of the {\tt sbdata} registers was accessed. */ #define DMI_SBCS_SBERROR_OFFSET 12 #define DMI_SBCS_SBERROR_LENGTH 3 diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 48390a6f0..166be750d 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -143,6 +143,9 @@ typedef struct { /* Number of words in the Program Buffer. */ unsigned progbufsize; + /* We cache the read-only bits of sbcs here. */ + uint32_t sbcs; + yes_no_maybe_t progbuf_writable; /* We only need the address so that we know the alignment of the buffer. */ riscv_addr_t progbuf_address; @@ -218,6 +221,18 @@ static void decode_dmi(char *text, unsigned address, unsigned data) { DMI_ABSTRACTCS, DMI_ABSTRACTCS_DATACOUNT, "datacount" }, { DMI_COMMAND, DMI_COMMAND_CMDTYPE, "cmdtype" }, + + { DMI_SBCS, DMI_SBCS_SBREADONADDR, "sbreadonaddr" }, + { DMI_SBCS, DMI_SBCS_SBACCESS, "sbaccess" }, + { DMI_SBCS, DMI_SBCS_SBAUTOINCREMENT, "sbautoincrement" }, + { DMI_SBCS, DMI_SBCS_SBREADONDATA, "sbreadondata" }, + { DMI_SBCS, DMI_SBCS_SBERROR, "sberror" }, + { DMI_SBCS, DMI_SBCS_SBASIZE, "sbasize" }, + { DMI_SBCS, DMI_SBCS_SBACCESS128, "sbaccess128" }, + { DMI_SBCS, DMI_SBCS_SBACCESS64, "sbaccess64" }, + { DMI_SBCS, DMI_SBCS_SBACCESS32, "sbaccess32" }, + { DMI_SBCS, DMI_SBCS_SBACCESS16, "sbaccess16" }, + { DMI_SBCS, DMI_SBCS_SBACCESS8, "sbaccess8" }, }; text[0] = 0; @@ -1148,6 +1163,7 @@ static int examine(struct target *target) info->dtmcontrol_idle = get_field(dtmcontrol, DTM_DTMCS_IDLE); uint32_t dmstatus = dmi_read(target, DMI_DMSTATUS); + LOG_DEBUG("dmstatus: 0x%08x", dmstatus); if (get_field(dmstatus, DMI_DMSTATUS_VERSION) != 2) { LOG_ERROR("OpenOCD only supports Debug Module version 2, not %d " "(dmstatus=0x%x)", get_field(dmstatus, DMI_DMSTATUS_VERSION), dmstatus); @@ -1158,16 +1174,7 @@ static int examine(struct target *target) dmi_write(target, DMI_DMCONTROL, 0); dmi_write(target, DMI_DMCONTROL, DMI_DMCONTROL_DMACTIVE); uint32_t dmcontrol = dmi_read(target, DMI_DMCONTROL); - - uint32_t hartinfo = dmi_read(target, DMI_HARTINFO); - LOG_DEBUG("dmcontrol: 0x%08x", dmcontrol); - LOG_DEBUG("dmstatus: 0x%08x", dmstatus); - LOG_DEBUG("hartinfo: 0x%08x", hartinfo); - - info->datasize = get_field(hartinfo, DMI_HARTINFO_DATASIZE); - info->dataaccess = get_field(hartinfo, DMI_HARTINFO_DATAACCESS); - info->dataaddr = get_field(hartinfo, DMI_HARTINFO_DATAADDR); if (!get_field(dmcontrol, DMI_DMCONTROL_DMACTIVE)) { LOG_ERROR("Debug Module did not become active. dmcontrol=0x%x", @@ -1175,6 +1182,13 @@ static int examine(struct target *target) return ERROR_FAIL; } + uint32_t hartinfo = dmi_read(target, DMI_HARTINFO); + LOG_DEBUG("hartinfo: 0x%08x", hartinfo); + + info->datasize = get_field(hartinfo, DMI_HARTINFO_DATASIZE); + info->dataaccess = get_field(hartinfo, DMI_HARTINFO_DATAACCESS); + info->dataaddr = get_field(hartinfo, DMI_HARTINFO_DATAADDR); + if (!get_field(dmstatus, DMI_DMSTATUS_AUTHENTICATED)) { LOG_ERROR("Authentication required by RISC-V core but not " "supported by OpenOCD. dmcontrol=0x%x", dmcontrol); @@ -1191,6 +1205,8 @@ static int examine(struct target *target) return ERROR_FAIL; } + info->sbcs = dmi_read(target, DMI_SBCS); + /* Check that abstract data registers are accessible. */ uint32_t abstractcs = dmi_read(target, DMI_ABSTRACTCS); info->datacount = get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); @@ -1439,11 +1455,102 @@ static void log_memory_access(target_addr_t address, uint64_t value, LOG_DEBUG(fmt, value); } +/* Read the relevant sbdata regs depending on size, and put the results into + * buffer. */ +static int read_memory_bus_word(struct target *target, uint32_t size, + uint8_t *buffer) +{ + if (size > 12) + write_to_buf(buffer + 12, dmi_read(target, DMI_SBDATA3), 4); + if (size > 8) + write_to_buf(buffer + 8, dmi_read(target, DMI_SBDATA2), 4); + if (size > 4) + write_to_buf(buffer + 4, dmi_read(target, DMI_SBDATA1), 4); + write_to_buf(buffer, dmi_read(target, DMI_SBDATA0), MIN(size, 4)); + return ERROR_OK; +} + +static uint32_t sb_sbaccess(unsigned size_bytes) +{ + switch (size_bytes) { + case 1: + return set_field(0, DMI_SBCS_SBACCESS, 0); + case 2: + return set_field(0, DMI_SBCS_SBACCESS, 1); + case 4: + return set_field(0, DMI_SBCS_SBACCESS, 2); + case 8: + return set_field(0, DMI_SBCS_SBACCESS, 3); + case 16: + return set_field(0, DMI_SBCS_SBACCESS, 4); + } + assert(0); +} + +static int sb_write_address(struct target *target, target_addr_t address) +{ + RISCV013_INFO(info); + unsigned sbasize = get_field(info->sbcs, DMI_SBCS_SBASIZE); + /* There currently is no support for >64-bit addresses in OpenOCD. */ + if (sbasize > 96) { + dmi_write(target, DMI_SBADDRESS3, 0); + } + if (sbasize > 64) { + dmi_write(target, DMI_SBADDRESS2, 0); + } + if (sbasize > 32) { +#if BUILD_TARGET64 + dmi_write(target, DMI_SBADDRESS1, address >> 32); +#else + dmi_write(target, DMI_SBADDRESS1, 0); +#endif + } + return dmi_write(target, DMI_SBADDRESS0, address); +} + +/** + * Read the requested memory using the system bus interface. + */ +static int read_memory_bus(struct target *target, target_addr_t address, + uint32_t size, uint32_t count, uint8_t *buffer) +{ + uint32_t sbcs = set_field(0, DMI_SBCS_SBREADONADDR, 1); + sbcs |= sb_sbaccess(size); + sbcs = set_field(sbcs, DMI_SBCS_SBAUTOINCREMENT, 1); + sbcs = set_field(sbcs, DMI_SBCS_SBREADONDATA, count > 1); + dmi_write(target, DMI_SBCS, sbcs); + + /* This address write will trigger the first read. */ + sb_write_address(target, address); + + for (uint32_t i = 0; i < count - 1; i++) { + read_memory_bus_word(target, size, buffer + i * size); + } + + sbcs = set_field(sbcs, DMI_SBCS_SBREADONDATA, 0); + dmi_write(target, DMI_SBCS, sbcs); + + read_memory_bus_word(target, size, buffer + (count - 1) * size); + + sbcs = dmi_read(target, DMI_SBCS); + unsigned error = get_field(sbcs, DMI_SBCS_SBERROR); + if (error == 0) { + return ERROR_OK; + } else if (error == 1 || error == 2 || error == 3) { + /* Bus timeout, bus error, other bus error. */ + return ERROR_FAIL; + } else if (error == 4) { + assert(0); + /* TODO: Reading too fast. We should deal with this properly. */ + } + return ERROR_FAIL; +} + /** * Read the requested memory, taking care to execute every read exactly once, * even if cmderr=busy is encountered. */ -static int read_memory(struct target *target, target_addr_t address, +static int read_memory_progbuf(struct target *target, target_addr_t address, uint32_t size, uint32_t count, uint8_t *buffer) { RISCV013_INFO(info); @@ -1679,7 +1786,76 @@ error: return result; } -static int write_memory(struct target *target, target_addr_t address, +static int read_memory(struct target *target, target_addr_t address, + uint32_t size, uint32_t count, uint8_t *buffer) +{ + RISCV013_INFO(info); + if ((get_field(info->sbcs, DMI_SBCS_SBVERSION) == 1) && ( + (get_field(info->sbcs, DMI_SBCS_SBACCESS8) && size == 1) || + (get_field(info->sbcs, DMI_SBCS_SBACCESS16) && size == 2) || + (get_field(info->sbcs, DMI_SBCS_SBACCESS32) && size == 4) || + (get_field(info->sbcs, DMI_SBCS_SBACCESS64) && size == 8) || + (get_field(info->sbcs, DMI_SBCS_SBACCESS128) && size == 16))) { + return read_memory_bus(target, address, size, count, buffer); + } else { + return read_memory_progbuf(target, address, size, count, buffer); + } +} + +static int write_memory_bus(struct target *target, target_addr_t address, + uint32_t size, uint32_t count, const uint8_t *buffer) +{ + uint32_t sbcs = sb_sbaccess(size); + sbcs = set_field(sbcs, DMI_SBCS_SBAUTOINCREMENT, 1); + dmi_write(target, DMI_SBCS, sbcs); + + sb_write_address(target, address); + + for (uint32_t i = 0; i < count; i++) { + const uint8_t *p = buffer + i * size; + if (size > 12) + dmi_write(target, DMI_SBDATA3, + ((uint32_t) p[12]) | + (((uint32_t) p[13]) << 8) | + (((uint32_t) p[14]) << 16) | + (((uint32_t) p[15]) << 24)); + if (size > 8) + dmi_write(target, DMI_SBDATA2, + ((uint32_t) p[8]) | + (((uint32_t) p[9]) << 8) | + (((uint32_t) p[10]) << 16) | + (((uint32_t) p[11]) << 24)); + if (size > 4) + dmi_write(target, DMI_SBDATA1, + ((uint32_t) p[4]) | + (((uint32_t) p[5]) << 8) | + (((uint32_t) p[6]) << 16) | + (((uint32_t) p[7]) << 24)); + uint32_t value = p[0]; + if (size > 2) { + value |= ((uint32_t) p[2]) << 16; + value |= ((uint32_t) p[3]) << 24; + } + if (size > 1) + value |= ((uint32_t) p[1]) << 8; + dmi_write(target, DMI_SBDATA0, value); + } + + sbcs = dmi_read(target, DMI_SBCS); + unsigned error = get_field(sbcs, DMI_SBCS_SBERROR); + if (error == 0) { + return ERROR_OK; + } else if (error == 1 || error == 2 || error == 3) { + /* Bus timeout, bus error, other bus error. */ + return ERROR_FAIL; + } else if (error == 4) { + assert(0); + /* TODO: Reading too fast. We should deal with this properly. */ + } + return ERROR_FAIL; +} + +static int write_memory_progbuf(struct target *target, target_addr_t address, uint32_t size, uint32_t count, const uint8_t *buffer) { RISCV013_INFO(info); @@ -1856,6 +2032,22 @@ error: return result; } +static int write_memory(struct target *target, target_addr_t address, + uint32_t size, uint32_t count, const uint8_t *buffer) +{ + RISCV013_INFO(info); + if ((get_field(info->sbcs, DMI_SBCS_SBVERSION) == 1) && ( + (get_field(info->sbcs, DMI_SBCS_SBACCESS8) && size == 1) || + (get_field(info->sbcs, DMI_SBCS_SBACCESS16) && size == 2) || + (get_field(info->sbcs, DMI_SBCS_SBACCESS32) && size == 4) || + (get_field(info->sbcs, DMI_SBCS_SBACCESS64) && size == 8) || + (get_field(info->sbcs, DMI_SBCS_SBACCESS128) && size == 16))) { + return write_memory_bus(target, address, size, count, buffer); + } else { + return write_memory_progbuf(target, address, size, count, buffer); + } +} + static int arch_state(struct target *target) { return ERROR_OK; From 5184c321250fa35a3dd1426b596120831270b147 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 18 Jan 2018 19:25:12 -0800 Subject: [PATCH 3/7] Clear errors that we see. Also WIP towards handling busy errors, but I'm putting that on hold while I change the spec... Change-Id: Iccf47048da46e75b0d769e56004fd783bba1dbf0 --- src/target/riscv/riscv-013.c | 104 +++++++++++++++++++++-------------- 1 file changed, 64 insertions(+), 40 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 166be750d..1dffd65e5 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1487,6 +1487,21 @@ static uint32_t sb_sbaccess(unsigned size_bytes) assert(0); } +static target_addr_t sb_read_address(struct target *target) +{ + RISCV013_INFO(info); + unsigned sbasize = get_field(info->sbcs, DMI_SBCS_SBASIZE); + target_addr_t address = 0; + if (sbasize > 32) { +#if BUILD_TARGET64 + address |= dmi_read(target, DMI_SBADDRESS1); + address <<= 32; +#endif + } + address |= dmi_read(target, DMI_SBADDRESS0); + return address; +} + static int sb_write_address(struct target *target, target_addr_t address) { RISCV013_INFO(info); @@ -1538,6 +1553,7 @@ static int read_memory_bus(struct target *target, target_addr_t address, return ERROR_OK; } else if (error == 1 || error == 2 || error == 3) { /* Bus timeout, bus error, other bus error. */ + dmi_write(target, DMI_SBCS, DMI_SBCS_SBERROR); return ERROR_FAIL; } else if (error == 4) { assert(0); @@ -1809,50 +1825,58 @@ static int write_memory_bus(struct target *target, target_addr_t address, sbcs = set_field(sbcs, DMI_SBCS_SBAUTOINCREMENT, 1); dmi_write(target, DMI_SBCS, sbcs); - sb_write_address(target, address); + target_addr_t next_address = address; + target_addr_t end_address = address + count * size; - for (uint32_t i = 0; i < count; i++) { - const uint8_t *p = buffer + i * size; - if (size > 12) - dmi_write(target, DMI_SBDATA3, - ((uint32_t) p[12]) | - (((uint32_t) p[13]) << 8) | - (((uint32_t) p[14]) << 16) | - (((uint32_t) p[15]) << 24)); - if (size > 8) - dmi_write(target, DMI_SBDATA2, - ((uint32_t) p[8]) | - (((uint32_t) p[9]) << 8) | - (((uint32_t) p[10]) << 16) | - (((uint32_t) p[11]) << 24)); - if (size > 4) - dmi_write(target, DMI_SBDATA1, - ((uint32_t) p[4]) | - (((uint32_t) p[5]) << 8) | - (((uint32_t) p[6]) << 16) | - (((uint32_t) p[7]) << 24)); - uint32_t value = p[0]; - if (size > 2) { - value |= ((uint32_t) p[2]) << 16; - value |= ((uint32_t) p[3]) << 24; + sb_write_address(target, next_address); + while (next_address < end_address) { + for (uint32_t i = (next_address - address) / size; i < count; i++) { + const uint8_t *p = buffer + i * size; + if (size > 12) + dmi_write(target, DMI_SBDATA3, + ((uint32_t) p[12]) | + (((uint32_t) p[13]) << 8) | + (((uint32_t) p[14]) << 16) | + (((uint32_t) p[15]) << 24)); + if (size > 8) + dmi_write(target, DMI_SBDATA2, + ((uint32_t) p[8]) | + (((uint32_t) p[9]) << 8) | + (((uint32_t) p[10]) << 16) | + (((uint32_t) p[11]) << 24)); + if (size > 4) + dmi_write(target, DMI_SBDATA1, + ((uint32_t) p[4]) | + (((uint32_t) p[5]) << 8) | + (((uint32_t) p[6]) << 16) | + (((uint32_t) p[7]) << 24)); + uint32_t value = p[0]; + if (size > 2) { + value |= ((uint32_t) p[2]) << 16; + value |= ((uint32_t) p[3]) << 24; + } + if (size > 1) + value |= ((uint32_t) p[1]) << 8; + dmi_write(target, DMI_SBDATA0, value); + } + + sbcs = dmi_read(target, DMI_SBCS); + unsigned error = get_field(sbcs, DMI_SBCS_SBERROR); + if (error == 0) { + next_address = end_address; + } else if (error == 1 || error == 2 || error == 3) { + /* Bus timeout, bus error, other bus error. */ + dmi_write(target, DMI_SBCS, DMI_SBCS_SBERROR); + return ERROR_FAIL; + } else if (error == 4) { + /* We wrote while the target was busy. Slow down and try again. */ + next_address = sb_read_address(target); + dmi_write(target, DMI_SBCS, DMI_SBCS_SBERROR); + // <<< TODO: slow down } - if (size > 1) - value |= ((uint32_t) p[1]) << 8; - dmi_write(target, DMI_SBDATA0, value); } - sbcs = dmi_read(target, DMI_SBCS); - unsigned error = get_field(sbcs, DMI_SBCS_SBERROR); - if (error == 0) { - return ERROR_OK; - } else if (error == 1 || error == 2 || error == 3) { - /* Bus timeout, bus error, other bus error. */ - return ERROR_FAIL; - } else if (error == 4) { - assert(0); - /* TODO: Reading too fast. We should deal with this properly. */ - } - return ERROR_FAIL; + return ERROR_OK; } static int write_memory_progbuf(struct target *target, target_addr_t address, From ee93a9b2f1913beeb25e77106223b2e503e185c1 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 30 Jan 2018 08:53:46 -0800 Subject: [PATCH 4/7] Add error handling code to system bus read/write It's not tested because spike never reports any busy errors since every access happens instantaneously. Change-Id: If43ea233a99f98cd419701dc98f0f4a62aa866eb --- src/target/riscv/riscv-013.c | 176 +++++++++++++++++++++++++---------- 1 file changed, 128 insertions(+), 48 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index f69f631fa..405c7a5a1 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -159,6 +159,10 @@ typedef struct { * in between accesses. */ unsigned int dmi_busy_delay; + /* Number of run-test/idle cycles to add between consecutive bus master + * reads/writes respectively. */ + unsigned int bus_master_write_delay, bus_master_read_delay; + /* This value is increased every time we tried to execute two commands * consecutively, and the second one failed because the previous hadn't * completed yet. It's used to add extra run-test/idle cycles after @@ -1123,6 +1127,8 @@ static int init_target(struct command_context *cmd_ctx, info->progbufsize = -1; info->dmi_busy_delay = 0; + info->bus_master_read_delay = 0; + info->bus_master_write_delay = 0; info->ac_busy_delay = 0; /* Assume all these abstract commands are supported until we learn @@ -1469,16 +1475,28 @@ static void log_memory_access(target_addr_t address, uint64_t value, /* Read the relevant sbdata regs depending on size, and put the results into * buffer. */ -static int read_memory_bus_word(struct target *target, uint32_t size, - uint8_t *buffer) +static int read_memory_bus_word(struct target *target, target_addr_t address, + uint32_t size, uint8_t *buffer) { - if (size > 12) - write_to_buf(buffer + 12, dmi_read(target, DMI_SBDATA3), 4); - if (size > 8) - write_to_buf(buffer + 8, dmi_read(target, DMI_SBDATA2), 4); - if (size > 4) - write_to_buf(buffer + 4, dmi_read(target, DMI_SBDATA1), 4); - write_to_buf(buffer, dmi_read(target, DMI_SBDATA0), MIN(size, 4)); + uint32_t value; + if (size > 12) { + value = dmi_read(target, DMI_SBDATA3); + write_to_buf(buffer + 12, value, 4); + log_memory_access(address + 12, value, 4, true); + } + if (size > 8) { + value = dmi_read(target, DMI_SBDATA2); + write_to_buf(buffer + 8, value, 4); + log_memory_access(address + 8, value, 4, true); + } + if (size > 4) { + value = dmi_read(target, DMI_SBDATA1); + write_to_buf(buffer + 4, value, 4); + log_memory_access(address + 4, value, 4, true); + } + value = dmi_read(target, DMI_SBDATA0); + write_to_buf(buffer, value, MIN(size, 4)); + log_memory_access(address, value, MIN(size, 4), true); return ERROR_OK; } @@ -1535,43 +1553,83 @@ static int sb_write_address(struct target *target, target_addr_t address) return dmi_write(target, DMI_SBADDRESS0, address); } +static int read_sbcs_nonbusy(struct target *target, uint32_t *sbcs) +{ + time_t start = time(NULL); + while (1) { + *sbcs = dmi_read(target, DMI_SBCS); + if (!get_field(*sbcs, DMI_SBCS_SBBUSY)) + return ERROR_OK; + 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); + } + return ERROR_FAIL; + } +} + /** * Read the requested memory using the system bus interface. */ static int read_memory_bus(struct target *target, target_addr_t address, uint32_t size, uint32_t count, uint8_t *buffer) { - uint32_t sbcs = set_field(0, DMI_SBCS_SBREADONADDR, 1); - sbcs |= sb_sbaccess(size); - sbcs = set_field(sbcs, DMI_SBCS_SBAUTOINCREMENT, 1); - sbcs = set_field(sbcs, DMI_SBCS_SBREADONDATA, count > 1); - dmi_write(target, DMI_SBCS, sbcs); + RISCV013_INFO(info); + target_addr_t next_address = address; + target_addr_t end_address = address + count * size; - /* This address write will trigger the first read. */ - sb_write_address(target, address); + while (next_address < end_address) { + uint32_t sbcs = set_field(0, DMI_SBCS_SBREADONADDR, 1); + sbcs |= sb_sbaccess(size); + sbcs = set_field(sbcs, DMI_SBCS_SBAUTOINCREMENT, 1); + sbcs = set_field(sbcs, DMI_SBCS_SBREADONDATA, count > 1); + dmi_write(target, DMI_SBCS, sbcs); - for (uint32_t i = 0; i < count - 1; i++) { - read_memory_bus_word(target, size, buffer + i * size); + /* This address write will trigger the first read. */ + sb_write_address(target, next_address); + + if (info->bus_master_read_delay) { + jtag_add_runtest(info->bus_master_read_delay, TAP_IDLE); + if (jtag_execute_queue() != ERROR_OK) { + LOG_ERROR("Failed to scan idle sequence"); + return ERROR_FAIL; + } + } + + for (uint32_t i = (next_address - address) / size; i < count - 1; i++) { + read_memory_bus_word(target, address + i * size, size, + buffer + i * size); + } + + sbcs = set_field(sbcs, DMI_SBCS_SBREADONDATA, 0); + dmi_write(target, DMI_SBCS, sbcs); + + read_memory_bus_word(target, address + (count - 1) * size, size, + buffer + (count - 1) * size); + + if (read_sbcs_nonbusy(target, &sbcs) != ERROR_OK) + return ERROR_FAIL; + + if (get_field(sbcs, DMI_SBCS_SBBUSYERROR)) { + /* We read while the target was busy. Slow down and try again. */ + dmi_write(target, DMI_SBCS, DMI_SBCS_SBBUSYERROR); + next_address = sb_read_address(target); + info->bus_master_read_delay += info->bus_master_read_delay / 10 + 1; + } + + unsigned error = get_field(sbcs, DMI_SBCS_SBERROR); + if (error == 0) { + next_address = end_address; + } else { + /* Some error indicating the bus access failed, but not because of + * something we did wrong. */ + dmi_write(target, DMI_SBCS, DMI_SBCS_SBERROR); + return ERROR_FAIL; + } } - sbcs = set_field(sbcs, DMI_SBCS_SBREADONDATA, 0); - dmi_write(target, DMI_SBCS, sbcs); - - read_memory_bus_word(target, size, buffer + (count - 1) * size); - - sbcs = dmi_read(target, DMI_SBCS); - unsigned error = get_field(sbcs, DMI_SBCS_SBERROR); - if (error == 0) { - return ERROR_OK; - } else if (error == 1 || error == 2 || error == 3) { - /* Bus timeout, bus error, other bus error. */ - dmi_write(target, DMI_SBCS, DMI_SBCS_SBERROR); - return ERROR_FAIL; - } else if (error == 4) { - assert(0); - /* TODO: Reading too fast. We should deal with this properly. */ - } - return ERROR_FAIL; + return ERROR_OK; } /** @@ -1818,7 +1876,9 @@ static int read_memory(struct target *target, target_addr_t address, uint32_t size, uint32_t count, uint8_t *buffer) { RISCV013_INFO(info); - if ((get_field(info->sbcs, DMI_SBCS_SBVERSION) == 1) && ( + if (info->progbufsize >= 2) { + return read_memory_progbuf(target, address, size, count, buffer); + } else if ((get_field(info->sbcs, DMI_SBCS_SBVERSION) == 1) && ( (get_field(info->sbcs, DMI_SBCS_SBACCESS8) && size == 1) || (get_field(info->sbcs, DMI_SBCS_SBACCESS16) && size == 2) || (get_field(info->sbcs, DMI_SBCS_SBACCESS32) && size == 4) || @@ -1826,13 +1886,15 @@ static int read_memory(struct target *target, target_addr_t address, (get_field(info->sbcs, DMI_SBCS_SBACCESS128) && size == 16))) { return read_memory_bus(target, address, size, count, buffer); } else { - return read_memory_progbuf(target, address, size, count, buffer); + LOG_ERROR("Don't know how to read memory on this target."); + return ERROR_FAIL; } } static int write_memory_bus(struct target *target, target_addr_t address, uint32_t size, uint32_t count, const uint8_t *buffer) { + RISCV013_INFO(info); uint32_t sbcs = sb_sbaccess(size); sbcs = set_field(sbcs, DMI_SBCS_SBAUTOINCREMENT, 1); dmi_write(target, DMI_SBCS, sbcs); @@ -1870,21 +1932,36 @@ static int write_memory_bus(struct target *target, target_addr_t address, if (size > 1) value |= ((uint32_t) p[1]) << 8; dmi_write(target, DMI_SBDATA0, value); + + log_memory_access(address + i * size, value, size, false); + + if (info->bus_master_write_delay) { + jtag_add_runtest(info->bus_master_write_delay, TAP_IDLE); + if (jtag_execute_queue() != ERROR_OK) { + LOG_ERROR("Failed to scan idle sequence"); + return ERROR_FAIL; + } + } + } + + if (read_sbcs_nonbusy(target, &sbcs) != ERROR_OK) + return ERROR_FAIL; + + if (get_field(sbcs, DMI_SBCS_SBBUSYERROR)) { + /* We wrote while the target was busy. Slow down and try again. */ + dmi_write(target, DMI_SBCS, DMI_SBCS_SBBUSYERROR); + next_address = sb_read_address(target); + info->bus_master_write_delay += info->bus_master_write_delay / 10 + 1; } - sbcs = dmi_read(target, DMI_SBCS); unsigned error = get_field(sbcs, DMI_SBCS_SBERROR); if (error == 0) { next_address = end_address; - } else if (error == 1 || error == 2 || error == 3) { - /* Bus timeout, bus error, other bus error. */ + } else { + /* Some error indicating the bus access failed, but not because of + * something we did wrong. */ dmi_write(target, DMI_SBCS, DMI_SBCS_SBERROR); return ERROR_FAIL; - } else if (error == 4) { - /* We wrote while the target was busy. Slow down and try again. */ - next_address = sb_read_address(target); - dmi_write(target, DMI_SBCS, DMI_SBCS_SBERROR); - // <<< TODO: slow down } } @@ -2072,7 +2149,9 @@ static int write_memory(struct target *target, target_addr_t address, uint32_t size, uint32_t count, const uint8_t *buffer) { RISCV013_INFO(info); - if ((get_field(info->sbcs, DMI_SBCS_SBVERSION) == 1) && ( + if (info->progbufsize >= 2) { + return write_memory_progbuf(target, address, size, count, buffer); + } else if ((get_field(info->sbcs, DMI_SBCS_SBVERSION) == 1) && ( (get_field(info->sbcs, DMI_SBCS_SBACCESS8) && size == 1) || (get_field(info->sbcs, DMI_SBCS_SBACCESS16) && size == 2) || (get_field(info->sbcs, DMI_SBCS_SBACCESS32) && size == 4) || @@ -2080,7 +2159,8 @@ static int write_memory(struct target *target, target_addr_t address, (get_field(info->sbcs, DMI_SBCS_SBACCESS128) && size == 16))) { return write_memory_bus(target, address, size, count, buffer); } else { - return write_memory_progbuf(target, address, size, count, buffer); + LOG_ERROR("Don't know how to write memory on this target."); + return ERROR_FAIL; } } From bb2c25c5cef5edc8435e72b3a9ea2d9b2d95d56b Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 31 Jan 2018 15:33:45 -0800 Subject: [PATCH 5/7] Make OpenOCD work when there is no program buffer. Fixed abstract register access for registers that aren't XLEN wide. Avoided excessive errors cases where we attempted to execute a fence but failed. Don't mark all the CSRs as caller-save. gdb was saving/restoring dscratch, which broke function calls as a side effect. dscratch is accessible for people who really know what they're doing, but gdb should never quietly access it. The same is probably true for other CSRs. Change-Id: I7bcdbbcb7e3c22ad92cbc205bf537c1fe548b160 --- src/target/riscv/riscv-013.c | 94 +++++++++++++++++++++++++----------- src/target/riscv/riscv.c | 4 +- 2 files changed, 68 insertions(+), 30 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 405c7a5a1..80a768292 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -611,14 +611,14 @@ static int execute_abstract_command(struct target *target, uint32_t command) return ERROR_OK; } -static riscv_reg_t read_abstract_arg(struct target *target, unsigned index) +static riscv_reg_t read_abstract_arg(struct target *target, unsigned index, + unsigned size_bits) { riscv_reg_t value = 0; - unsigned xlen = riscv_xlen(target); - unsigned offset = index * xlen / 32; - switch (xlen) { + unsigned offset = index * size_bits / 32; + switch (size_bits) { default: - LOG_ERROR("Unsupported xlen: %d", xlen); + LOG_ERROR("Unsupported size: %d", size_bits); return ~0; case 64: value |= ((uint64_t) dmi_read(target, DMI_DATA0 + offset + 1)) << 32; @@ -629,14 +629,13 @@ static riscv_reg_t read_abstract_arg(struct target *target, unsigned index) } static int write_abstract_arg(struct target *target, unsigned index, - riscv_reg_t value) + riscv_reg_t value, unsigned size_bits) { - unsigned xlen = riscv_xlen(target); - unsigned offset = index * xlen / 32; - switch (xlen) { + unsigned offset = index * size_bits / 32; + switch (size_bits) { default: - LOG_ERROR("Unsupported xlen: %d", xlen); - return ~0; + LOG_ERROR("Unsupported size: %d", size_bits); + return ERROR_FAIL; case 64: dmi_write(target, DMI_DATA0 + offset + 1, value >> 32); case 32: @@ -687,7 +686,7 @@ static int register_read_abstract(struct target *target, uint64_t *value, RISCV013_INFO(info); if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31 && - !info->abstract_read_fpr_supported) + !info->abstract_write_fpr_supported) return ERROR_FAIL; if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095 && !info->abstract_read_csr_supported) @@ -711,7 +710,7 @@ static int register_read_abstract(struct target *target, uint64_t *value, } if (value) - *value = read_abstract_arg(target, 0); + *value = read_abstract_arg(target, 0, size); return ERROR_OK; } @@ -732,7 +731,7 @@ static int register_write_abstract(struct target *target, uint32_t number, AC_ACCESS_REGISTER_TRANSFER | AC_ACCESS_REGISTER_WRITE); - if (write_abstract_arg(target, 0, value) != ERROR_OK) + if (write_abstract_arg(target, 0, value, size) != ERROR_OK) return ERROR_FAIL; int result = execute_abstract_command(target, command); @@ -946,16 +945,31 @@ static int scratch_write64(struct target *target, scratch_mem_t *scratch, return ERROR_OK; } +/** Return register size in bits. */ +static unsigned register_size(struct target *target, unsigned number) +{ + /* If reg_cache hasn't been initialized yet, make a guess. We need this for + * when this function is called during examine(). */ + if (target->reg_cache) + return target->reg_cache->reg_list[number].size; + else + return riscv_xlen(target); +} + static int register_write_direct(struct target *target, unsigned number, uint64_t value) { + RISCV013_INFO(info); + RISCV_INFO(r); + LOG_DEBUG("[%d] reg[0x%x] <- 0x%" PRIx64, riscv_current_hartid(target), number, value); int result = register_write_abstract(target, number, value, - riscv_xlen(target)); - if (result == ERROR_OK) - return ERROR_OK; + register_size(target, number)); + if (result == ERROR_OK || + info->progbufsize + r->impebreak < 2) + return result; struct riscv_program program; riscv_program_init(&program, target); @@ -1011,10 +1025,14 @@ static int register_write_direct(struct target *target, unsigned number, /** Actually read registers from the target right now. */ static int register_read_direct(struct target *target, uint64_t *value, uint32_t number) { - int result = register_read_abstract(target, value, number, - riscv_xlen(target)); + RISCV013_INFO(info); + RISCV_INFO(r); - if (result != ERROR_OK) { + int result = register_read_abstract(target, value, number, + register_size(target, number)); + + if (result != ERROR_OK && + info->progbufsize + r->impebreak >= 2) { assert(number != GDB_REGNO_S0); struct riscv_program program; @@ -1224,9 +1242,18 @@ static int examine(struct target *target) info->datacount = get_field(abstractcs, DMI_ABSTRACTCS_DATACOUNT); info->progbufsize = get_field(abstractcs, DMI_ABSTRACTCS_PROGBUFSIZE); + LOG_INFO("datacount=%d progbufsize=%d", info->datacount, info->progbufsize); + RISCV_INFO(r); r->impebreak = get_field(dmstatus, DMI_DMSTATUS_IMPEBREAK); + if (info->progbufsize + r->impebreak < 2) { + LOG_WARNING("We won't be able to execute fence instructions on this " + "target. Memory may not always appear consistent. " + "(progbufsize=%d, impebreak=%d)", info->progbufsize, + r->impebreak); + } + /* Before doing anything else we must first enumerate the harts. */ /* Don't call any riscv_* functions until after we've counted the number of @@ -2400,14 +2427,26 @@ int riscv013_dmi_write_u64_bits(struct target *target) return info->abits + DTM_DMI_DATA_LENGTH + DTM_DMI_OP_LENGTH; } +static int maybe_execute_fence_i(struct target *target) +{ + RISCV013_INFO(info); + RISCV_INFO(r); + if (info->progbufsize + r->impebreak >= 2) { + struct riscv_program program; + riscv_program_init(&program, target); + if (riscv_program_fence_i(&program) != ERROR_OK) + return ERROR_FAIL; + if (riscv_program_exec(&program, target) != ERROR_OK) + return ERROR_FAIL; + } + return ERROR_OK; +} + /* Helper Functions. */ static int riscv013_on_step_or_resume(struct target *target, bool step) { - struct riscv_program program; - riscv_program_init(&program, target); - riscv_program_fence_i(&program); - if (riscv_program_exec(&program, target) != ERROR_OK) - LOG_ERROR("Unable to execute fence.i"); + if (maybe_execute_fence_i(target) != ERROR_OK) + return ERROR_FAIL; /* We want to twiddle some bits in the debug CSR so debugging works. */ riscv_reg_t dcsr; @@ -2430,10 +2469,7 @@ static int riscv013_step_or_resume_current_hart(struct target *target, bool step return ERROR_FAIL; } - struct riscv_program program; - riscv_program_init(&program, target); - riscv_program_fence_i(&program); - if (riscv_program_exec(&program, target) != ERROR_OK) + if (maybe_execute_fence_i(target) != ERROR_OK) return ERROR_FAIL; /* Issue the resume command, and then wait for the current hart to resume. */ diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index f8a273958..8f8376d44 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1884,7 +1884,6 @@ int riscv_init_registers(struct target *target) * between). */ for (uint32_t number = 0; number < GDB_REGNO_COUNT; number++) { struct reg *r = &target->reg_cache->reg_list[number]; - r->caller_save = true; r->dirty = false; r->valid = false; r->exist = true; @@ -1896,6 +1895,7 @@ int riscv_init_registers(struct target *target) * target is in theory allowed to change XLEN on us. But I expect a lot * of other things to break in that case as well. */ if (number <= GDB_REGNO_XPR31) { + r->caller_save = true; switch (number) { case GDB_REGNO_ZERO: r->name = "zero"; @@ -1997,10 +1997,12 @@ int riscv_init_registers(struct target *target) r->group = "general"; r->feature = &feature_cpu; } else if (number == GDB_REGNO_PC) { + r->caller_save = true; sprintf(reg_name, "pc"); r->group = "general"; r->feature = &feature_cpu; } else if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31) { + r->caller_save = true; if (riscv_supports_extension(target, 'D')) { r->reg_data_type = &type_ieee_double; r->size = 64; From 7114ef485c51425b786b4cc3c1424a3e7cce487d Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 31 Jan 2018 16:45:33 -0800 Subject: [PATCH 6/7] Fix cut and paste bug. Change-Id: I1c554cbe3d7cb7845bc62f14ae6b8dff107eb192 --- src/target/riscv/riscv-013.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 80a768292..3d6783b2e 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -686,7 +686,7 @@ static int register_read_abstract(struct target *target, uint64_t *value, RISCV013_INFO(info); if (number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31 && - !info->abstract_write_fpr_supported) + !info->abstract_read_fpr_supported) return ERROR_FAIL; if (number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095 && !info->abstract_read_csr_supported) From a80ab87efd7af21b53dd5e719132da48c8322add Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 31 Jan 2018 16:55:43 -0800 Subject: [PATCH 7/7] Add unreachable return for mingw build. Change-Id: I8c0c4d7be8f6f28638cc2b5ae8114f5c8f95f94b --- src/target/riscv/riscv-013.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 3d6783b2e..11a1c2e0d 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1542,6 +1542,7 @@ static uint32_t sb_sbaccess(unsigned size_bytes) return set_field(0, DMI_SBCS_SBACCESS, 4); } assert(0); + return 0; /* Make mingw happy. */ } static target_addr_t sb_read_address(struct target *target)