From 927f9d887379a0af17cc5ab51a008c7ab89007df Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 19 Jun 2017 08:58:07 -0700 Subject: Update list of "threads" when harts are discovered. This ensures that "info threads" is accurate as soon as gdb connects. Also print out number of triggers that is discovered in examine(). --- src/rtos/riscv_debug.c | 4 +--- src/rtos/riscv_debug.h | 4 ++++ src/target/riscv/riscv-013.c | 10 ++++++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/rtos/riscv_debug.c b/src/rtos/riscv_debug.c index dbd7238..d6458e9 100644 --- a/src/rtos/riscv_debug.c +++ b/src/rtos/riscv_debug.c @@ -5,10 +5,8 @@ #include "riscv_debug.h" #include "target/target.h" #include "target/riscv/riscv.h" -#include "rtos.h" #include "server/gdb_server.h" -static int riscv_update_threads(struct rtos *rtos); static int riscv_gdb_thread_packet(struct connection *connection, const char *packet, int packet_size); static int riscv_gdb_v_packet(struct connection *connection, const char *packet, int packet_size); @@ -40,7 +38,7 @@ static int riscv_create_rtos(struct target *target) return JIM_OK; } -static int riscv_update_threads(struct rtos *rtos) +int riscv_update_threads(struct rtos *rtos) { LOG_DEBUG("Updating the RISC-V Hart List"); diff --git a/src/rtos/riscv_debug.h b/src/rtos/riscv_debug.h index bcc7411..539edf2 100644 --- a/src/rtos/riscv_debug.h +++ b/src/rtos/riscv_debug.h @@ -1,9 +1,13 @@ #ifndef RTOS__RISCV_H #define RTOS__RISCV_H +#include "rtos.h" + struct riscv_rtos { /* The index into the thread list used to handle */ int qs_thread_info_offset; }; +int riscv_update_threads(struct rtos *rtos); + #endif diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index f1d4cfb..9053feb 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -20,6 +20,7 @@ #include "target/breakpoints.h" #include "helper/time_support.h" #include "riscv.h" +#include "rtos/riscv_debug.h" #include "debug_defines.h" #include "rtos/rtos.h" #include "program.h" @@ -1192,14 +1193,19 @@ static int examine(struct target *target) riscv_resume_all_harts(target); target_set_examined(target); + if (target->rtos) { + riscv_update_threads(target->rtos); + } + // Some regression suites rely on seeing 'Examined RISC-V core' to know // when they can connect with gdb/telnet. // We will need to update those suites if we want to change that text. LOG_INFO("Examined RISC-V core; found %d harts", riscv_count_harts(target)); for (int i = 0; i < riscv_count_harts(target); ++i) { - LOG_INFO(" hart %d: XLEN=%d, program buffer at 0x%" PRIx64, i, - r->xlen[i], r->debug_buffer_addr[i]); + LOG_INFO(" hart %d: XLEN=%d, program buffer at 0x%" PRIx64 + ", %d triggers", i, r->xlen[i], r->debug_buffer_addr[i], + r->trigger_count[i]); } return ERROR_OK; } -- cgit v1.1 From ccdd26e3ef8309e26fcda3581ce87dc4eaeb37b2 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 19 Jun 2017 10:56:11 -0700 Subject: Comment curious code. --- src/target/riscv/riscv-013.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 9053feb..c411940 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1285,6 +1285,8 @@ static int read_memory(struct target *target, target_addr_t address, size, address); select_dmi(target); + /* There was a bug in the memory system and only accesses from hart 0 actually + * worked correctly. This should be obselete now. -palmer */ riscv_set_current_hartid(target, 0); /* This program uses two temporary registers. A word of data and the @@ -1481,6 +1483,8 @@ static int write_memory(struct target *target, target_addr_t address, LOG_DEBUG("writing %d words of %d bytes to 0x%08lx", count, size, (long)address); select_dmi(target); + /* There was a bug in the memory system and only accesses from hart 0 actually + * worked correctly. This should be obselete now. -palmer */ riscv_set_current_hartid(target, 0); /* This program uses two temporary registers. A word of data and the -- cgit v1.1 From 10518351bb4208545f8b80c7c8e21b7585fe0925 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 19 Jun 2017 12:23:38 -0700 Subject: Don't immediately segfault with -rtos on v0.11. --- src/target/riscv/riscv.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 31a6db6..7a40467 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1012,6 +1012,9 @@ bool riscv_rtos_enabled(const struct target *target) void riscv_set_current_hartid(struct target *target, int hartid) { RISCV_INFO(r); + if (!r->select_current_hart) + return; + int previous_hartid = riscv_current_hartid(target); r->current_hartid = hartid; assert(riscv_rtos_enabled(target) || target->coreid == hartid); -- cgit v1.1 From 9cd98058a0c68dc8545c2ac3218204799ee0920f Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 19 Jun 2017 13:36:25 -0700 Subject: Set hardware triggers on all harts. Right now we're using "threads" to represent harts. gdb/OpenOCD assume there's only one set of hardware breakpoints among all threads. Make it so. --- src/target/riscv/riscv-013.c | 102 +++++++++++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 33 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index c411940..60846de 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -579,6 +579,10 @@ static int register_write_direct(struct target *target, unsigned number, uint64_t value) { struct riscv_program program; + + LOG_DEBUG("[%d] reg[0x%x] <- 0x%" PRIx64, riscv_current_hartid(target), + number, value); + riscv_program_init(&program, target); riscv_addr_t input = riscv_program_alloc_d(&program); @@ -641,7 +645,8 @@ static int register_read_direct(struct target *target, uint64_t *value, uint32_t *value = 0; *value |= ((uint64_t)(riscv_program_read_ram(&program, output + 4))) << 32; *value |= riscv_program_read_ram(&program, output); - LOG_DEBUG("register 0x%x = 0x%" PRIx64, number, *value); + LOG_DEBUG("[%d] reg[0x%x] = 0x%" PRIx64, riscv_current_hartid(target), + number, *value); return ERROR_OK; } @@ -794,6 +799,14 @@ static void deinit_target(struct target *target) static int add_trigger(struct target *target, struct trigger *trigger) { riscv013_info_t *info = get_info(target); + + // While we're using threads to fake harts, both gdb and OpenOCD assume + // that hardware breakpoints are shared among threads. Make this true by + // setting the same breakpoints on all harts. + + // Assume that all triggers are configured the same on all harts. + riscv_set_current_hartid(target, 0); + maybe_read_tselect(target); int i; @@ -817,42 +830,59 @@ static int add_trigger(struct target *target, struct trigger *trigger) continue; } - // address/data match trigger - tdata1 |= MCONTROL_DMODE(riscv_xlen(target)); - tdata1 = set_field(tdata1, MCONTROL_ACTION, - MCONTROL_ACTION_DEBUG_MODE); - tdata1 = set_field(tdata1, MCONTROL_MATCH, MCONTROL_MATCH_EQUAL); - tdata1 |= MCONTROL_M; - if (info->misa & (1 << ('H' - 'A'))) - tdata1 |= MCONTROL_H; - if (info->misa & (1 << ('S' - 'A'))) - tdata1 |= MCONTROL_S; - if (info->misa & (1 << ('U' - 'A'))) - tdata1 |= MCONTROL_U; - - if (trigger->execute) - tdata1 |= MCONTROL_EXECUTE; - if (trigger->read) - tdata1 |= MCONTROL_LOAD; - if (trigger->write) - tdata1 |= MCONTROL_STORE; - - register_write_direct(target, GDB_REGNO_TDATA1, tdata1); - uint64_t tdata1_rb; - register_read_direct(target, &tdata1_rb, GDB_REGNO_TDATA1); - LOG_DEBUG("tdata1=0x%" PRIx64, tdata1_rb); + for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { + riscv_set_current_hartid(target, hartid); + + if (hartid > 0) { + register_write_direct(target, GDB_REGNO_TSELECT, i); + } + + // address/data match trigger + tdata1 |= MCONTROL_DMODE(riscv_xlen(target)); + tdata1 = set_field(tdata1, MCONTROL_ACTION, + MCONTROL_ACTION_DEBUG_MODE); + tdata1 = set_field(tdata1, MCONTROL_MATCH, MCONTROL_MATCH_EQUAL); + tdata1 |= MCONTROL_M; + if (info->misa & (1 << ('H' - 'A'))) + tdata1 |= MCONTROL_H; + if (info->misa & (1 << ('S' - 'A'))) + tdata1 |= MCONTROL_S; + if (info->misa & (1 << ('U' - 'A'))) + tdata1 |= MCONTROL_U; + + if (trigger->execute) + tdata1 |= MCONTROL_EXECUTE; + if (trigger->read) + tdata1 |= MCONTROL_LOAD; + if (trigger->write) + tdata1 |= MCONTROL_STORE; + + register_write_direct(target, GDB_REGNO_TDATA1, tdata1); + + register_read_direct(target, &tdata1_rb, GDB_REGNO_TDATA1); + LOG_DEBUG("tdata1=0x%" PRIx64, tdata1_rb); + + if (tdata1 != tdata1_rb) { + LOG_DEBUG("Trigger %d doesn't support what we need; After writing 0x%" + PRIx64 " to tdata1 it contains 0x%" PRIx64, + i, tdata1, tdata1_rb); + register_write_direct(target, GDB_REGNO_TDATA1, 0); + if (hartid > 0) { + LOG_ERROR("Setting hardware breakpoints requires " + "homogeneous harts."); + return ERROR_FAIL; + } + break; + } + + register_write_direct(target, GDB_REGNO_TDATA2, trigger->address); + } if (tdata1 != tdata1_rb) { - LOG_DEBUG("Trigger %d doesn't support what we need; After writing 0x%" - PRIx64 " to tdata1 it contains 0x%" PRIx64, - i, tdata1, tdata1_rb); - register_write_direct(target, GDB_REGNO_TDATA1, 0); continue; } - register_write_direct(target, GDB_REGNO_TDATA2, trigger->address); - LOG_DEBUG("Using resource %d for bp %d", i, trigger->unique_id); info->trigger_unique_id[i] = trigger->unique_id; @@ -870,6 +900,9 @@ static int remove_trigger(struct target *target, struct trigger *trigger) { riscv013_info_t *info = get_info(target); + // Assume that all triggers are configured the same on all harts. + riscv_set_current_hartid(target, 0); + maybe_read_tselect(target); int i; @@ -884,8 +917,11 @@ static int remove_trigger(struct target *target, struct trigger *trigger) return ERROR_FAIL; } LOG_DEBUG("Stop using resource %d for bp %d", i, trigger->unique_id); - register_write_direct(target, GDB_REGNO_TSELECT, i); - register_write_direct(target, GDB_REGNO_TDATA1, 0); + for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { + riscv_set_current_hartid(target, hartid); + register_write_direct(target, GDB_REGNO_TSELECT, i); + register_write_direct(target, GDB_REGNO_TDATA1, 0); + } info->trigger_unique_id[i] = -1; return ERROR_OK; -- cgit v1.1