From f93ede5401c711e55d9852986aa399c0318efb22 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 4 Nov 2019 11:04:30 -0800 Subject: [PATCH] Add support for 64-bit memory reads/writes (#419) * 64-bit progbuf memory reads work. Change-Id: Ia3dbc0ee39a31ed0e5c38bbb3d9e089b2533f399 * 64-bit writes work. Change-Id: Iae78711d715b6682817bb7cce366b0094bda8b23 * Let targets indicate number of supported data bits. This is used by the default memory read/write functions when creating an aligned block. I'm adding this mainly to ensure I get coverage of the 64-bit progbuf memory read/write code. Change-Id: Ie5909fe537c9ec3360a8d2837f84be00a63de77b * Make mingw32 happy. Change-Id: Iade8c1fdfc72ccafc82f2f34923577032b668916 --- src/target/riscv/program.c | 10 +++ src/target/riscv/program.h | 2 + src/target/riscv/riscv-013.c | 146 +++++++++++++++++++++++------------ src/target/riscv/riscv.c | 7 +- src/target/riscv/riscv.h | 2 +- src/target/target.c | 19 ++++- src/target/target.h | 7 ++ src/target/target_type.h | 5 ++ 8 files changed, 142 insertions(+), 56 deletions(-) diff --git a/src/target/riscv/program.c b/src/target/riscv/program.c index bd08ac1c1..8645ed6be 100644 --- a/src/target/riscv/program.c +++ b/src/target/riscv/program.c @@ -79,6 +79,11 @@ int riscv_program_exec(struct riscv_program *p, struct target *t) return ERROR_OK; } +int riscv_program_sdr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno b, int offset) +{ + return riscv_program_insert(p, sd(d, b, offset)); +} + int riscv_program_swr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno b, int offset) { return riscv_program_insert(p, sw(d, b, offset)); @@ -94,6 +99,11 @@ int riscv_program_sbr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno return riscv_program_insert(p, sb(d, b, offset)); } +int riscv_program_ldr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno b, int offset) +{ + return riscv_program_insert(p, ld(d, b, offset)); +} + int riscv_program_lwr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno b, int offset) { return riscv_program_insert(p, lw(d, b, offset)); diff --git a/src/target/riscv/program.h b/src/target/riscv/program.h index cc3ffd4c5..3bc0e5228 100644 --- a/src/target/riscv/program.h +++ b/src/target/riscv/program.h @@ -55,10 +55,12 @@ int riscv_program_save_to_dscratch(struct riscv_program *p, enum gdb_regno to_sa /* Helpers to assemble various instructions. Return 0 on success. These might * assemble into a multi-instruction sequence that overwrites some other * register, but those will be properly saved and restored. */ +int riscv_program_ldr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno a, int o); int riscv_program_lwr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno a, int o); int riscv_program_lhr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno a, int o); int riscv_program_lbr(struct riscv_program *p, enum gdb_regno d, enum gdb_regno a, int o); +int riscv_program_sdr(struct riscv_program *p, enum gdb_regno s, enum gdb_regno a, int o); int riscv_program_swr(struct riscv_program *p, enum gdb_regno s, enum gdb_regno a, int o); int riscv_program_shr(struct riscv_program *p, enum gdb_regno s, enum gdb_regno a, int o); int riscv_program_sbr(struct riscv_program *p, enum gdb_regno s, enum gdb_regno a, int o); diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 1de894dde..dda639eda 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1261,8 +1261,8 @@ static int register_write_direct(struct target *target, unsigned number, RISCV013_INFO(info); RISCV_INFO(r); - LOG_DEBUG("{%d} reg[0x%x] <- 0x%" PRIx64, riscv_current_hartid(target), - number, value); + LOG_DEBUG("{%d} %s <- 0x%" PRIx64, riscv_current_hartid(target), + gdb_regno_name(number), value); int result = register_write_abstract(target, number, value, register_size(target, number)); @@ -1449,8 +1449,8 @@ static int register_read_direct(struct target *target, uint64_t *value, uint32_t } if (result == ERROR_OK) { - LOG_DEBUG("{%d} reg[0x%x] = 0x%" PRIx64, riscv_current_hartid(target), - number, *value); + LOG_DEBUG("{%d} %s = 0x%" PRIx64, riscv_current_hartid(target), + gdb_regno_name(number), *value); } return result; @@ -1949,25 +1949,32 @@ static int deassert_reset(struct target *target) /** * @par size in bytes */ -static void read_from_buf(uint64_t *value, const uint8_t *buffer, unsigned size) +static uint64_t read_from_buf(const uint8_t *buffer, unsigned size) { switch (size) { case 1: - *value = buffer[0]; - break; + return buffer[0]; case 2: - *value = buffer[0] + return buffer[0] | ((uint64_t) buffer[1] << 8); - break; case 4: - *value = buffer[0] + return buffer[0] | ((uint64_t) buffer[1] << 8) | ((uint64_t) buffer[2] << 16) | ((uint64_t) buffer[3] << 24); - break; + case 8: + return buffer[0] + | ((uint64_t) buffer[1] << 8) + | ((uint64_t) buffer[2] << 16) + | ((uint64_t) buffer[3] << 24) + | ((uint64_t) buffer[4] << 32) + | ((uint64_t) buffer[5] << 40) + | ((uint64_t) buffer[6] << 48) + | ((uint64_t) buffer[7] << 56); default: assert(false); } + return -1; } /** @@ -2042,7 +2049,21 @@ static void log_memory_access(target_addr_t address, uint64_t value, char fmt[80]; sprintf(fmt, "M[0x%" TARGET_PRIxADDR "] %ss 0x%%0%d" PRIx64, address, read ? "read" : "write", size_bytes * 2); - value &= (((uint64_t) 0x1) << (size_bytes * 8)) - 1; + switch (size_bytes) { + case 1: + value &= 0xff; + break; + case 2: + value &= 0xffff; + break; + case 4: + value &= 0xffffffff; + break; + case 8: + break; + default: + assert(false); + } LOG_DEBUG(fmt, value); } @@ -2446,8 +2467,7 @@ static int write_memory_abstract(struct target *target, target_addr_t address, bool updateaddr = true; for (uint32_t c = 0; c < count; c++) { /* Move data to arg0 */ - riscv_reg_t value = 0; - read_from_buf(&value, p, size); + riscv_reg_t value = read_from_buf(p, size); result = write_abstract_arg(target, 0, value, riscv_xlen(target)); if (result != ERROR_OK) { LOG_ERROR("Failed to write arg0 during write_memory_abstract()."); @@ -2542,6 +2562,8 @@ static int read_memory_progbuf_inner(struct target *target, target_addr_t addres size_t reads = 0; for (riscv_addr_t addr = read_addr; addr < fin_addr; addr += size) { + if (size > 4) + riscv_batch_add_dmi_read(batch, DMI_DATA1); riscv_batch_add_dmi_read(batch, DMI_DATA0); reads++; @@ -2577,7 +2599,7 @@ static int read_memory_progbuf_inner(struct target *target, target_addr_t addres dmi_write(target, DMI_ABSTRACTAUTO, 0); - uint32_t dmi_data0; + uint32_t dmi_data0, dmi_data1 = 0; /* This is definitely a good version of the value that we * attempted to read when we discovered that the target was * busy. */ @@ -2585,6 +2607,10 @@ static int read_memory_progbuf_inner(struct target *target, target_addr_t addres riscv_batch_free(batch); goto error; } + if (size > 4 && dmi_read(target, &dmi_data1, DMI_DATA1) != ERROR_OK) { + riscv_batch_free(batch); + goto error; + } /* See how far we got, clobbering dmi_data0. */ result = register_read_direct(target, &next_read_addr, @@ -2593,8 +2619,9 @@ static int read_memory_progbuf_inner(struct target *target, target_addr_t addres riscv_batch_free(batch); goto error; } - write_to_buf(buffer + next_read_addr - 2 * size - address, dmi_data0, size); - log_memory_access(next_read_addr - 2 * size, dmi_data0, size, true); + uint64_t value64 = (((uint64_t) dmi_data1) << 32) | dmi_data0; + write_to_buf(buffer + next_read_addr - 2 * size - address, value64, size); + log_memory_access(next_read_addr - 2 * size, value64, size, true); /* Restore the command, and execute it. * Now DMI_DATA0 contains the next value just as it would if no @@ -2618,15 +2645,16 @@ static int read_memory_progbuf_inner(struct target *target, target_addr_t addres /* Now read whatever we got out of the batch. */ dmi_status_t status = DMI_STATUS_SUCCESS; + riscv_addr_t receive_addr = read_addr - size * 2; + unsigned read = 0; for (size_t i = 0; i < reads; i++) { - riscv_addr_t receive_addr = read_addr + (i-2) * size; assert(receive_addr < address + size * count); if (receive_addr < address) continue; if (receive_addr > next_read_addr - (3 + ignore_last) * size) break; - uint64_t dmi_out = riscv_batch_get_dmi_read(batch, i); + uint64_t dmi_out = riscv_batch_get_dmi_read(batch, read++); status = get_field(dmi_out, DTM_DMI_OP); if (status != DMI_STATUS_SUCCESS) { /* If we're here because of busy count, dmi_busy_delay will @@ -2643,7 +2671,20 @@ static int read_memory_progbuf_inner(struct target *target, target_addr_t addres result = ERROR_FAIL; goto error; } - uint32_t value = get_field(dmi_out, DTM_DMI_DATA); + uint64_t value = get_field(dmi_out, DTM_DMI_DATA); + if (size > 4) { + dmi_out = riscv_batch_get_dmi_read(batch, read++); + status = get_field(dmi_out, DTM_DMI_OP); + if (status != DMI_STATUS_SUCCESS) { + LOG_WARNING("Batch memory read encountered DMI error %d. " + "Falling back on slower reads.", status); + riscv_batch_free(batch); + result = ERROR_FAIL; + goto error; + } + value <<= 32; + value |= get_field(dmi_out, DTM_DMI_DATA); + } riscv_addr_t offset = receive_addr - address; write_to_buf(buffer + offset, value, size); log_memory_access(receive_addr, value, size, true); @@ -2660,11 +2701,14 @@ static int read_memory_progbuf_inner(struct target *target, target_addr_t addres if (count > 1) { /* Read the penultimate word. */ - uint32_t value; - if (dmi_read(target, &value, DMI_DATA0) != ERROR_OK) + uint32_t dmi_data0, dmi_data1 = 0; + if (dmi_read(target, &dmi_data0, DMI_DATA0) != ERROR_OK) return ERROR_FAIL; - write_to_buf(buffer + size * (count-2), value, size); - log_memory_access(address + size * (count-2), value, size, true); + if (size > 4 && dmi_read(target, &dmi_data1, DMI_DATA1) != ERROR_OK) + return ERROR_FAIL; + uint64_t value64 = (((uint64_t) dmi_data1) << 32) | dmi_data0; + write_to_buf(buffer + size * (count-2), value64, size); + log_memory_access(address + size * (count-2), value64, size, true); } /* Read the last word. */ @@ -2715,6 +2759,9 @@ static int read_memory_progbuf_one(struct target *target, target_addr_t address, case 4: riscv_program_lwr(&program, GDB_REGNO_S0, GDB_REGNO_S0, 0); break; + case 8: + riscv_program_ldr(&program, GDB_REGNO_S0, GDB_REGNO_S0, 0); + break; default: LOG_ERROR("Unsupported size: %d", size); return ERROR_FAIL; @@ -2740,6 +2787,7 @@ static int read_memory_progbuf_one(struct target *target, target_addr_t address, if (register_read(target, &value, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; write_to_buf(buffer, value, size); + log_memory_access(address, value, size, true); if (riscv_set_register(target, GDB_REGNO_S0, s0) != ERROR_OK) return ERROR_FAIL; @@ -2760,6 +2808,12 @@ static int read_memory_progbuf(struct target *target, target_addr_t address, { RISCV013_INFO(info); + if (riscv_xlen(target) < size * 8) { + LOG_ERROR("XLEN (%d) is too short for %d-bit memory read.", + riscv_xlen(target), size * 8); + return ERROR_FAIL; + } + int result = ERROR_OK; LOG_DEBUG("reading %d words of %d bytes from 0x%" TARGET_PRIxADDR, count, @@ -2805,10 +2859,14 @@ static int read_memory_progbuf(struct target *target, target_addr_t address, case 4: riscv_program_lwr(&program, GDB_REGNO_S1, GDB_REGNO_S0, 0); break; + case 8: + riscv_program_ldr(&program, GDB_REGNO_S1, GDB_REGNO_S0, 0); + break; default: LOG_ERROR("Unsupported size: %d", size); return ERROR_FAIL; } + if (riscv_enable_virtual && info->progbufsize >= 4 && get_field(mstatus, MSTATUS_MPRV)) riscv_program_csrrci(&program, GDB_REGNO_ZERO, CSR_DCSR_MPRVEN, GDB_REGNO_DCSR); riscv_program_addi(&program, GDB_REGNO_S0, GDB_REGNO_S0, size); @@ -3088,6 +3146,12 @@ static int write_memory_progbuf(struct target *target, target_addr_t address, { RISCV013_INFO(info); + if (riscv_xlen(target) < size * 8) { + LOG_ERROR("XLEN (%d) is too short for %d-bit memory write.", + riscv_xlen(target), size * 8); + return ERROR_FAIL; + } + LOG_DEBUG("writing %d words of %d bytes to 0x%08lx", count, size, (long)address); select_dmi(target); @@ -3124,8 +3188,11 @@ static int write_memory_progbuf(struct target *target, target_addr_t address, case 4: riscv_program_swr(&program, GDB_REGNO_S1, GDB_REGNO_S0, 0); break; + case 8: + riscv_program_sdr(&program, GDB_REGNO_S1, GDB_REGNO_S0, 0); + break; default: - LOG_ERROR("Unsupported size: %d", size); + LOG_ERROR("write_memory_progbuf(): Unsupported size: %d", size); result = ERROR_FAIL; goto error; } @@ -3158,28 +3225,7 @@ static int write_memory_progbuf(struct target *target, target_addr_t address, unsigned offset = size*i; const uint8_t *t_buffer = buffer + offset; - /* TODO: Test with read_from_buf(&value, t_buffer, size)*/ - uint32_t value; - switch (size) { - case 1: - value = t_buffer[0]; - break; - case 2: - value = t_buffer[0] - | ((uint32_t) t_buffer[1] << 8); - break; - case 4: - value = t_buffer[0] - | ((uint32_t) t_buffer[1] << 8) - | ((uint32_t) t_buffer[2] << 16) - | ((uint32_t) t_buffer[3] << 24); - break; - default: - LOG_ERROR("unsupported access size: %d", size); - riscv_batch_free(batch); - result = ERROR_FAIL; - goto error; - } + uint64_t value = read_from_buf(t_buffer, size); log_memory_access(address + offset, value, size, false); cur_addr += size; @@ -3193,12 +3239,14 @@ static int write_memory_progbuf(struct target *target, target_addr_t address, } /* Write value. */ + if (size > 4) + dmi_write(target, DMI_DATA1, value >> 32); dmi_write(target, DMI_DATA0, value); /* Write and execute command that moves value into S1 and * executes program buffer. */ uint32_t command = access_register_command(target, - GDB_REGNO_S1, 32, + GDB_REGNO_S1, size > 4 ? 64 : 32, AC_ACCESS_REGISTER_POSTEXEC | AC_ACCESS_REGISTER_TRANSFER | AC_ACCESS_REGISTER_WRITE); @@ -3214,6 +3262,8 @@ static int write_memory_progbuf(struct target *target, target_addr_t address, setup_needed = false; } else { + if (size > 4) + riscv_batch_add_dmi_write(batch, DMI_DATA1, value >> 32); riscv_batch_add_dmi_write(batch, DMI_DATA0, value); if (riscv_batch_full(batch)) break; diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 5e3539941..963f67076 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -2457,7 +2457,7 @@ const struct command_registration riscv_command_handlers[] = { COMMAND_REGISTRATION_DONE }; -unsigned riscv_address_bits(struct target *target) +static unsigned riscv_xlen_nonconst(struct target *target) { return riscv_xlen(target); } @@ -2500,7 +2500,8 @@ struct target_type riscv_target = { .commands = riscv_command_handlers, - .address_bits = riscv_address_bits + .address_bits = riscv_xlen_nonconst, + .data_bits = riscv_xlen_nonconst }; /*** RISC-V Interface ***/ @@ -2609,7 +2610,7 @@ bool riscv_supports_extension(struct target *target, int hartid, char letter) return r->misa[hartid] & (1 << num); } -int riscv_xlen(const struct target *target) +unsigned riscv_xlen(const struct target *target) { return riscv_xlen_of_hart(target, riscv_current_hartid(target)); } diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index a0d912c40..0b0bc50f2 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -218,7 +218,7 @@ int riscv_step_rtos_hart(struct target *target); bool riscv_supports_extension(struct target *target, int hartid, char letter); /* Returns XLEN for the given (or current) hart. */ -int riscv_xlen(const struct target *target); +unsigned riscv_xlen(const struct target *target); int riscv_xlen_of_hart(const struct target *target, int hartid); bool riscv_rtos_enabled(const struct target *target); diff --git a/src/target/target.c b/src/target/target.c index 2c4a7a014..3d375304b 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -1284,6 +1284,13 @@ unsigned target_address_bits(struct target *target) return 32; } +unsigned target_data_bits(struct target *target) +{ + if (target->type->data_bits) + return target->type->data_bits(target); + return 32; +} + int target_profiling(struct target *target, uint32_t *samples, uint32_t max_num_samples, uint32_t *num_samples, uint32_t seconds) { @@ -2189,9 +2196,11 @@ static int target_write_buffer_default(struct target *target, { uint32_t size; - /* Align up to maximum 4 bytes. The loop condition makes sure the next pass + /* Align up to maximum bytes. The loop condition makes sure the next pass * will have something to do with the size we leave to it. */ - for (size = 1; size < 4 && count >= size * 2 + (address & size); size *= 2) { + for (size = 1; + size < target_data_bits(target) / 8 && count >= size * 2 + (address & size); + size *= 2) { if (address & size) { int retval = target_write_memory(target, address, size, 1, buffer); if (retval != ERROR_OK) @@ -2250,9 +2259,11 @@ static int target_read_buffer_default(struct target *target, target_addr_t addre { uint32_t size; - /* Align up to maximum 4 bytes. The loop condition makes sure the next pass + /* Align up to maximum bytes. The loop condition makes sure the next pass * will have something to do with the size we leave to it. */ - for (size = 1; size < 4 && count >= size * 2 + (address & size); size *= 2) { + for (size = 1; + size < target_data_bits(target) / 8 && count >= size * 2 + (address & size); + size *= 2) { if (address & size) { int retval = target_read_memory(target, address, size, 1, buffer); if (retval != ERROR_OK) diff --git a/src/target/target.h b/src/target/target.h index 957a7d041..89077144b 100644 --- a/src/target/target.h +++ b/src/target/target.h @@ -664,6 +664,13 @@ target_addr_t target_address_max(struct target *target); */ unsigned target_address_bits(struct target *target); +/** + * Return the number of data bits this target supports. + * + * This routine is a wrapper for target->type->data_bits. + */ +unsigned target_data_bits(struct target *target); + /** Return the *name* of this targets current state */ const char *target_state_name(struct target *target); diff --git a/src/target/target_type.h b/src/target/target_type.h index 4bdea721e..b94be12ba 100644 --- a/src/target/target_type.h +++ b/src/target/target_type.h @@ -296,6 +296,11 @@ struct target_type { * typically be 32 for 32-bit targets, and 64 for 64-bit targets. If not * implemented, it's assumed to be 32. */ unsigned (*address_bits)(struct target *target); + + /* Return the number of data bits this target supports. This will + * typically be 32 for 32-bit targets, and 64 for 64-bit targets. If not + * implemented, it's assumed to be 32. */ + unsigned (*data_bits)(struct target *target); }; #endif /* OPENOCD_TARGET_TARGET_TYPE_H */