From 9f4cac5a3812be137913995e2d1d4be4b34e0744 Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Tue, 20 Jun 2017 17:12:53 -0700 Subject: [PATCH 01/11] Set current_hartid from coreid This avoids a bunch of RTOS special cases. --- src/target/riscv/riscv-013.c | 2 +- src/target/riscv/riscv.c | 8 +++----- src/target/riscv/riscv.h | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 60846debe..2fab9a1ff 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -706,7 +706,7 @@ static int init_target(struct command_context *cmd_ctx, LOG_DEBUG("init"); riscv_info_t *generic_info = (riscv_info_t *) target->arch_info; - riscv_info_init(generic_info); + riscv_info_init(target, generic_info); generic_info->get_register = &riscv013_get_register; generic_info->set_register = &riscv013_set_register; generic_info->select_current_hart = &riscv013_select_current_hart; diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 7a40467f1..ad5eddbef 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -872,11 +872,12 @@ struct target_type riscv_target = /*** RISC-V Interface ***/ -void riscv_info_init(riscv_info_t *r) +void riscv_info_init(struct target *target, riscv_info_t *r) { memset(r, 0, sizeof(*r)); r->dtm_version = 1; r->registers_initialized = false; + r->current_hartid = target->coreid; for (size_t h = 0; h < RISCV_MAX_HARTS; ++h) { r->xlen[h] = -1; @@ -1068,10 +1069,7 @@ void riscv_invalidate_register_cache(struct target *target) int riscv_current_hartid(const struct target *target) { RISCV_INFO(r); - if (riscv_rtos_enabled(target)) - return r->current_hartid; - else - return target->coreid; + return r->current_hartid; } void riscv_set_all_rtos_harts(struct target *target) diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index e23d49d1f..53fb08673 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -134,7 +134,7 @@ int riscv_openocd_deassert_reset(struct target *target); /*** RISC-V Interface ***/ /* Initializes the shared RISC-V structure. */ -void riscv_info_init(riscv_info_t *r); +void riscv_info_init(struct target *target, riscv_info_t *r); /* Run control, possibly for multiple harts. The _all_harts versions resume * all the enabled harts, which when running in RTOS mode is all the harts on From 788908fcf03e1fb449f206bf267ccad2290f24bc Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Tue, 20 Jun 2017 17:14:18 -0700 Subject: [PATCH 02/11] Factor out checking if harts should be used Rather than having a bunch of "if rtos" stuff, I now just check "if hart_enabled". This makes some code paths cleaner, all of which were buggy in the non-RTOS multi-hart mode. --- src/target/riscv/riscv-013.c | 6 +++++ src/target/riscv/riscv.c | 44 +++++++++++++++++++++--------------- src/target/riscv/riscv.h | 3 +++ 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 2fab9a1ff..352f737d5 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1143,6 +1143,9 @@ static int examine(struct target *target) /* Find the address of the program buffer, which must be done without * knowing anything about the target. */ for (int i = 0; i < riscv_count_harts(target); ++i) { + if (!riscv_hart_enabled(target, i)) + continue; + riscv_set_current_hartid(target, i); /* Without knowing anything else we can at least mess with the @@ -1213,6 +1216,9 @@ static int examine(struct target *target) /* Then we check the number of triggers availiable to each hart. */ for (int i = 0; i < riscv_count_harts(target); ++i) { + if (!riscv_hart_enabled(target, i)) + continue; + for (uint32_t t = 0; t < RISCV_MAX_TRIGGERS; ++t) { riscv_set_current_hartid(target, i); diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index ad5eddbef..785938393 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -890,11 +890,11 @@ void riscv_info_init(struct target *target, riscv_info_t *r) int riscv_halt_all_harts(struct target *target) { - if (riscv_rtos_enabled(target)) { - for (int i = 0; i < riscv_count_harts(target); ++i) - riscv_halt_one_hart(target, i); - } else { - riscv_halt_one_hart(target, riscv_current_hartid(target)); + for (int i = 0; i < riscv_count_harts(target); ++i) { + if (!riscv_hart_enabled(target, i)) + continue; + + riscv_halt_one_hart(target, i); } return ERROR_OK; @@ -916,11 +916,11 @@ int riscv_halt_one_hart(struct target *target, int hartid) int riscv_resume_all_harts(struct target *target) { - if (riscv_rtos_enabled(target)) { - for (int i = 0; i < riscv_count_harts(target); ++i) - riscv_resume_one_hart(target, i); - } else { - riscv_resume_one_hart(target, riscv_current_hartid(target)); + for (int i = 0; i < riscv_count_harts(target); ++i) { + if (!riscv_hart_enabled(target, i)) + continue; + + riscv_resume_one_hart(target, i); } riscv_invalidate_register_cache(target); @@ -944,11 +944,11 @@ int riscv_resume_one_hart(struct target *target, int hartid) int riscv_reset_all_harts(struct target *target) { - if (riscv_rtos_enabled(target)) { - for (int i = 0; i < riscv_count_harts(target); ++i) - riscv_reset_one_hart(target, i); - } else { - riscv_reset_one_hart(target, riscv_current_hartid(target)); + for (int i = 0; i < riscv_count_harts(target); ++i) { + if (!riscv_hart_enabled(target, i)) + continue; + + riscv_reset_one_hart(target, i); } riscv_invalidate_register_cache(target); @@ -1018,10 +1018,9 @@ void riscv_set_current_hartid(struct target *target, int hartid) int previous_hartid = riscv_current_hartid(target); r->current_hartid = hartid; - assert(riscv_rtos_enabled(target) || target->coreid == hartid); + assert(riscv_hart_enabled(target, hartid)); LOG_DEBUG("setting hartid to %d, was %d", hartid, previous_hartid); - if (riscv_rtos_enabled(target)) - r->select_current_hart(target); + r->select_current_hart(target); /* This might get called during init, in which case we shouldn't be * setting up the register cache. */ @@ -1234,3 +1233,12 @@ int riscv_dmi_write_u64_bits(struct target *target) RISCV_INFO(r); return r->dmi_write_u64_bits(target); } + +bool riscv_hart_enabled(struct target *target, int hartid) +{ + /* FIXME: Add a hart mask to the RTOS. */ + if (riscv_rtos_enabled(target)) + return hartid < riscv_count_harts(target); + + return hartid == target->coreid; +} diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 53fb08673..94a6080f2 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -215,4 +215,7 @@ int riscv_dmi_write_u64_bits(struct target *target); /* Invalidates the register cache. */ void riscv_invalidate_register_cache(struct target *target); +/* Returns TRUE when a hart is enabled in this target. */ +bool riscv_hart_enabled(struct target *target, int hartid); + #endif From a277416a39114336299f07bc52f3b26fcf07bcbe Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Tue, 20 Jun 2017 17:15:56 -0700 Subject: [PATCH 03/11] Refactor examine, to avoid some assertions Now that we're supporting non-RTOS multi-hart mode there's some more assertions that you're running on the right hart. Those assertions aren't sane very early in examine, so I avoid them. --- src/target/riscv/riscv-013.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 352f737d5..e770430c3 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1123,17 +1123,23 @@ static int examine(struct target *target) /* Before doing anything else we must first enumerate the harts. */ RISCV_INFO(r); - if (riscv_rtos_enabled(target)) { - for (int i = 0; i < RISCV_MAX_HARTS; ++i) { - riscv_set_current_hartid(target, i); - uint32_t s = dmi_read(target, DMI_DMSTATUS); - if (get_field(s, DMI_DMSTATUS_ANYNONEXISTENT)) - break; - r->hart_count = i + 1; + int original_coreid = target->coreid; + for (int i = 0; i < RISCV_MAX_HARTS; ++i) { + /* Fake being a non-RTOS targeted to this core so we can see if + * it exists. This avoids the assertion in + * riscv_set_current_hartid() that ensures non-RTOS targets + * don't touch the harts they're not assigned to. */ + target->coreid = i; + r->hart_count = i + 1; + riscv_set_current_hartid(target, i); + + uint32_t s = dmi_read(target, DMI_DMSTATUS); + if (get_field(s, DMI_DMSTATUS_ANYNONEXISTENT)) { + r->hart_count--; + break; } - } else { - r->hart_count = 1; } + target->coreid = original_coreid; LOG_DEBUG("Enumerated %d harts", r->hart_count); @@ -1233,6 +1239,7 @@ static int examine(struct target *target) /* Resumes all the harts, so the debugger can later pause them. */ riscv_resume_all_harts(target); + target->state = TARGET_RUNNING; target_set_examined(target); if (target->rtos) { From 4bdb0422242559e2e00a4368d51c193e7f2f0bd1 Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Tue, 20 Jun 2017 17:17:28 -0700 Subject: [PATCH 04/11] Allow memory writes to proceed on all harts --- src/target/riscv/riscv-013.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index e770430c3..a68a1c7d7 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1334,9 +1334,6 @@ 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 * associated address are stored at some location in memory. The @@ -1532,9 +1529,6 @@ 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 * associated address are stored at some location in memory. The From 689d0fcaf6764b6a8efbbfed51e048a94c03f3bf Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Tue, 20 Jun 2017 17:17:54 -0700 Subject: [PATCH 05/11] No longer hard-code the non-RTOS hart to 0 I was just being lazy here. --- src/target/riscv/riscv.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 785938393..377ab78cf 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -448,11 +448,12 @@ static int riscv_get_gdb_reg_list(struct target *target, { RISCV_INFO(r); LOG_DEBUG("reg_class=%d", reg_class); - LOG_DEBUG("riscv_get_gdb_reg_list: rtos_hartid=%d current_hartid=%d", r->rtos_hartid, r->current_hartid); - if (r->rtos_hartid != -1) + LOG_DEBUG("rtos_hartid=%d current_hartid=%d", r->rtos_hartid, r->current_hartid); + + if (r->rtos_hartid != -1 && riscv_rtos_enabled(target)) riscv_set_current_hartid(target, r->rtos_hartid); else - riscv_set_current_hartid(target, 0); + riscv_set_current_hartid(target, target->coreid); switch (reg_class) { case REG_CLASS_GENERAL: From d77c4a953c1f2a6e1f84c28e64bf9296a4bb398a Mon Sep 17 00:00:00 2001 From: Palmer Dabbelt Date: Wed, 21 Jun 2017 10:16:25 -0700 Subject: [PATCH 06/11] Don't set breakpoints on disabled harts --- src/target/riscv/riscv-013.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index a68a1c7d7..86f5f1b42 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -832,6 +832,9 @@ static int add_trigger(struct target *target, struct trigger *trigger) uint64_t tdata1_rb; for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { + if (!riscv_hart_enabled(target, i)) + continue; + riscv_set_current_hartid(target, hartid); if (hartid > 0) { @@ -918,6 +921,9 @@ static int remove_trigger(struct target *target, struct trigger *trigger) } LOG_DEBUG("Stop using resource %d for bp %d", i, trigger->unique_id); for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { + if (!riscv_hart_enabled(target, i)) + continue; + riscv_set_current_hartid(target, hartid); register_write_direct(target, GDB_REGNO_TSELECT, i); register_write_direct(target, GDB_REGNO_TDATA1, 0); From b6f8efbf44e2488ef0eb9c6362a2d9e5a5f28f41 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 27 Jun 2017 15:11:06 -0700 Subject: [PATCH 07/11] Check for errors in read_csr(). Also slightly improve debugging output. --- src/target/riscv/riscv-011.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index 79f10f10e..bc7d45927 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -764,10 +764,10 @@ static void cache_set32(struct target *target, unsigned int index, uint32_t data if (info->dram_cache[index].valid && info->dram_cache[index].data == data) { // This is already preset on the target. - LOG_DEBUG("cache[0x%x] = 0x%x (hit)", index, data); + LOG_DEBUG("cache[0x%x] = 0x%08x: DASM(0x%x) (hit)", index, data, data); return; } - LOG_DEBUG("cache[0x%x] = 0x%x", index, data); + LOG_DEBUG("cache[0x%x] = 0x%08x: DASM(0x%x)", index, data, data); info->dram_cache[index].data = data; info->dram_cache[index].valid = true; info->dram_cache[index].dirty = true; @@ -1033,6 +1033,7 @@ static int wait_for_state(struct target *target, enum target_state state) static int read_csr(struct target *target, uint64_t *value, uint32_t csr) { + riscv011_info_t *info = get_info(target); cache_set32(target, 0, csrr(S0, csr)); cache_set_store(target, 1, S0, SLOT0); cache_set_jump(target, 2); @@ -1042,6 +1043,13 @@ static int read_csr(struct target *target, uint64_t *value, uint32_t csr) *value = cache_get(target, SLOT0); LOG_DEBUG("csr 0x%x = 0x%" PRIx64, csr, *value); + uint32_t exception = cache_get32(target, info->dramsize-1); + if (exception) { + LOG_ERROR("Got exception 0x%x when reading CSR 0x%x", exception, csr); + *value = ~0; + return ERROR_FAIL; + } + return ERROR_OK; } From 7d451e00f5c94a8f72f5abfd80a467e0e6a39bc6 Mon Sep 17 00:00:00 2001 From: Dmitry Ryzhov Date: Fri, 30 Jun 2017 19:15:58 +0300 Subject: [PATCH 08/11] Restore value of temporary register (s0) in examine OpenOCD procedure in case of core can not execute 64 bit instruction. --- src/target/riscv/riscv-013.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 60846debe..f45ba4fd5 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1175,6 +1175,12 @@ static int examine(struct target *target) * In order to make this work we first need to */ int offset = (progbuf_addr % 8 == 0) ? -4 : 0; + /* This program uses a temporary register. If the core can not + * execute 64 bit instruction, the original value of temporary + * register will not be restored due to an exception. So we have to + * restore it manually in that case. */ + uint64_t s0 = riscv_get_register(target, GDB_REGNO_S0); + struct riscv_program program64; riscv_program_init(&program64, target); riscv_program_csrrw(&program64, GDB_REGNO_S0, GDB_REGNO_S0, GDB_REGNO_DSCRATCH); @@ -1190,6 +1196,8 @@ static int examine(struct target *target) + dmi_read(target, DMI_PROGBUF0 + (4 + offset) / 4) - 4; r->xlen[i] = 64; + } else { + riscv_set_register(target, GDB_REGNO_S0, s0); } /* Display this as early as possible to help people who are using From 99a36735073724bb29270dadaa3fb8f71f9aa923 Mon Sep 17 00:00:00 2001 From: Dmitry Ryzhov Date: Sat, 1 Jul 2017 15:09:23 +0300 Subject: [PATCH 09/11] Fix comment about saving the temporary register in examine procedure. --- src/target/riscv/riscv-013.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index f45ba4fd5..213c6fc1d 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -1177,8 +1177,12 @@ static int examine(struct target *target) /* This program uses a temporary register. If the core can not * execute 64 bit instruction, the original value of temporary - * register will not be restored due to an exception. So we have to - * restore it manually in that case. */ + * register (s0) will not be restored due to an exception. + * So we have to save it and restore manually in that case. + * If the core can execute 64 bit instruction, the saved value + * is wrong, because it was read with 32 bit lw instruction, + * but the value of s0 will be restored by the reverse swap + * of s0 and dscratch registers. */ uint64_t s0 = riscv_get_register(target, GDB_REGNO_S0); struct riscv_program program64; From 6c627e9ea9a756f725a7b9ce9cd962690af872a8 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 29 Jun 2017 10:06:06 -0700 Subject: [PATCH 10/11] Add back support for type 1 triggers. They were implemented, and people want to keep using them. Also make OpenOCD tolerate cores that have $misa at 0xf10 instead of the current address of 0x301. Actually return an error when we fail to read a CSR. Tweak cache_set32() debug output. --- src/target/riscv/riscv-011.c | 162 ++++++++++++++++++++++++++--------- 1 file changed, 120 insertions(+), 42 deletions(-) diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index bc7d45927..95328037b 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -1504,6 +1504,104 @@ static void deinit_target(struct target *target) info->version_specific = NULL; } +static int maybe_add_trigger_t1(struct target *target, struct trigger *trigger, + uint64_t tdata1) +{ + riscv011_info_t *info = get_info(target); + + const uint32_t bpcontrol_x = 1<<0; + const uint32_t bpcontrol_w = 1<<1; + const uint32_t bpcontrol_r = 1<<2; + const uint32_t bpcontrol_u = 1<<3; + const uint32_t bpcontrol_s = 1<<4; + const uint32_t bpcontrol_h = 1<<5; + const uint32_t bpcontrol_m = 1<<6; + const uint32_t bpcontrol_bpmatch = 0xf << 7; + const uint32_t bpcontrol_bpaction = 0xff << 11; + + if (tdata1 & (bpcontrol_r | bpcontrol_w | bpcontrol_x)) { + // Trigger is already in use, presumably by user code. + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + } + + tdata1 = set_field(tdata1, bpcontrol_r, trigger->read); + tdata1 = set_field(tdata1, bpcontrol_w, trigger->write); + tdata1 = set_field(tdata1, bpcontrol_x, trigger->execute); + tdata1 = set_field(tdata1, bpcontrol_u, !!(info->misa & (1 << ('U' - 'A')))); + tdata1 = set_field(tdata1, bpcontrol_s, !!(info->misa & (1 << ('S' - 'A')))); + tdata1 = set_field(tdata1, bpcontrol_h, !!(info->misa & (1 << ('H' - 'A')))); + tdata1 |= bpcontrol_m; + tdata1 = set_field(tdata1, bpcontrol_bpmatch, 0); // exact match + tdata1 = set_field(tdata1, bpcontrol_bpaction, 0); // cause bp exception + + write_csr(target, CSR_TDATA1, tdata1); + + uint64_t tdata1_rb; + read_csr(target, &tdata1_rb, CSR_TDATA1); + LOG_DEBUG("tdata1=0x%" PRIx64, tdata1_rb); + + if (tdata1 != tdata1_rb) { + LOG_DEBUG("Trigger doesn't support what we need; After writing 0x%" + PRIx64 " to tdata1 it contains 0x%" PRIx64, + tdata1, tdata1_rb); + write_csr(target, CSR_TDATA1, 0); + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + } + + write_csr(target, CSR_TDATA2, trigger->address); + + return ERROR_OK; +} + +static int maybe_add_trigger_t2(struct target *target, struct trigger *trigger, + uint64_t tdata1) +{ + riscv011_info_t *info = get_info(target); + // tselect is already set + if (tdata1 & (MCONTROL_EXECUTE | MCONTROL_STORE | MCONTROL_LOAD)) { + // Trigger is already in use, presumably by user code. + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + } + + // 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; + + write_csr(target, CSR_TDATA1, tdata1); + + uint64_t tdata1_rb; + read_csr(target, &tdata1_rb, CSR_TDATA1); + LOG_DEBUG("tdata1=0x%" PRIx64, tdata1_rb); + + if (tdata1 != tdata1_rb) { + LOG_DEBUG("Trigger doesn't support what we need; After writing 0x%" + PRIx64 " to tdata1 it contains 0x%" PRIx64, + tdata1, tdata1_rb); + write_csr(target, CSR_TDATA1, 0); + return ERROR_TARGET_RESOURCE_NOT_AVAILABLE; + } + + write_csr(target, CSR_TDATA2, trigger->address); + + return ERROR_OK; +} + static int add_trigger(struct target *target, struct trigger *trigger) { riscv011_info_t *info = get_info(target); @@ -1522,51 +1620,23 @@ static int add_trigger(struct target *target, struct trigger *trigger) read_csr(target, &tdata1, CSR_TDATA1); int type = get_field(tdata1, MCONTROL_TYPE(riscv_xlen(target))); - if (type != 2) { - continue; + int result; + switch (type) { + case 1: + result = maybe_add_trigger_t1(target, trigger, tdata1); + break; + case 2: + result = maybe_add_trigger_t2(target, trigger, tdata1); + break; + default: + LOG_DEBUG("trigger %d has unknown type %d", i, type); + continue; } - if (tdata1 & (MCONTROL_EXECUTE | MCONTROL_STORE | MCONTROL_LOAD)) { - // Trigger is already in use, presumably by user code. + if (result != ERROR_OK) { 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; - - write_csr(target, CSR_TDATA1, tdata1); - - uint64_t tdata1_rb; - read_csr(target, &tdata1_rb, CSR_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); - write_csr(target, CSR_TDATA1, 0); - continue; - } - - write_csr(target, CSR_TDATA2, trigger->address); - LOG_DEBUG("Using resource %d for bp %d", i, trigger->unique_id); info->trigger_unique_id[i] = trigger->unique_id; @@ -1906,8 +1976,13 @@ static int examine(struct target *target) update_reg_list(target); if (read_csr(target, &info->misa, CSR_MISA) != ERROR_OK) { - LOG_ERROR("Failed to read misa."); - return ERROR_FAIL; + LOG_WARNING("Failed to read misa at 0x%x.", CSR_MISA); + if (read_csr(target, &info->misa, 0xf10) != ERROR_OK) { + // Maybe this is an old core that still has $misa at the old + // address. + LOG_ERROR("Failed to read misa at 0x%x.", 0xf10); + return ERROR_FAIL; + } } info->never_halted = true; @@ -2137,6 +2212,9 @@ static int handle_halt(struct target *target, bool announce) write_csr(target, CSR_TSELECT, info->trigger_count); uint64_t tselect_rb; read_csr(target, &tselect_rb, CSR_TSELECT); + // Mask off the top bit, which is used as tdrmode in old + // implementations. + tselect_rb &= ~(1ULL << (riscv_xlen(target)-1)); if (info->trigger_count != tselect_rb) break; uint64_t tdata1; From f18fd83ac7219cd1e5e62a121e6cd6474e5f53dc Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 3 Jul 2017 11:52:35 -0700 Subject: [PATCH 11/11] Fix trigger set/clear bug. --- src/target/riscv/riscv-013.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 86f5f1b42..ab5b993ce 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -832,7 +832,7 @@ static int add_trigger(struct target *target, struct trigger *trigger) uint64_t tdata1_rb; for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { - if (!riscv_hart_enabled(target, i)) + if (!riscv_hart_enabled(target, hartid)) continue; riscv_set_current_hartid(target, hartid); @@ -921,7 +921,7 @@ static int remove_trigger(struct target *target, struct trigger *trigger) } LOG_DEBUG("Stop using resource %d for bp %d", i, trigger->unique_id); for (int hartid = 0; hartid < riscv_count_harts(target); ++hartid) { - if (!riscv_hart_enabled(target, i)) + if (!riscv_hart_enabled(target, hartid)) continue; riscv_set_current_hartid(target, hartid);