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