From 365c79c3ff5ed0c1927ade8b1c09dd356f1d6415 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Wed, 27 Dec 2017 13:04:30 -0800 Subject: [PATCH] Get rid of abort() calls. Also changed a few asserts that could trigger due to broken hardware. Fixes Issue #142. Change-Id: Ia2b99baa82f30ebcb2fd7e4902f0e67046ce4ed2 --- src/target/riscv/program.c | 3 +- src/target/riscv/riscv-011.c | 5 +-- src/target/riscv/riscv-013.c | 76 +++++++++++++++++++++--------------- src/target/riscv/riscv.c | 32 +++++++++------ src/target/riscv/riscv.h | 15 +++---- 5 files changed, 77 insertions(+), 54 deletions(-) diff --git a/src/target/riscv/program.c b/src/target/riscv/program.c index 76f63b8fd..a7a2e859d 100644 --- a/src/target/riscv/program.c +++ b/src/target/riscv/program.c @@ -55,7 +55,6 @@ int riscv_program_exec(struct riscv_program *p, struct target *t) LOG_ERROR("Unable to write ebreak"); for (size_t i = 0; i < riscv_debug_buffer_size(p->target); ++i) LOG_ERROR("ram[%02x]: DASM(0x%08lx) [0x%08lx]", (int)i, (long)p->debug_buffer[i], (long)p->debug_buffer[i]); - abort(); return ERROR_FAIL; } @@ -152,7 +151,7 @@ int riscv_program_insert(struct riscv_program *p, riscv_insn_t i) LOG_ERROR("Unable to insert instruction:"); LOG_ERROR(" instruction_count=%d", (int)p->instruction_count); LOG_ERROR(" buffer size =%d", (int)riscv_debug_buffer_size(p->target)); - abort(); + return ERROR_FAIL; } p->debug_buffer[p->instruction_count] = i; diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index 9490f7a58..18e31c730 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -1329,12 +1329,11 @@ static riscv_reg_t get_register(struct target *target, int hartid, int regid) return value; } -static void set_register(struct target *target, int hartid, int regid, +static int set_register(struct target *target, int hartid, int regid, uint64_t value) { assert(hartid == 0); - /* TODO: propagate errors */ - register_write(target, regid, value); + return register_write(target, regid, value); } static int halt(struct target *target) diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 25a875b56..6251765de 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -31,22 +31,22 @@ #define DMI_PROGBUF1 (DMI_PROGBUF0 + 1) static void riscv013_on_step_or_resume(struct target *target, bool step); -static void riscv013_step_or_resume_current_hart(struct target *target, bool step); +static int riscv013_step_or_resume_current_hart(struct target *target, bool step); static void riscv013_clear_abstract_error(struct target *target); /* Implementations of the functions in riscv_info_t. */ static riscv_reg_t riscv013_get_register(struct target *target, int hartid, int regid); -static void riscv013_set_register(struct target *target, int hartid, int regid, uint64_t value); +static int riscv013_set_register(struct target *target, int hartid, int regid, uint64_t value); static void riscv013_select_current_hart(struct target *target); -static void riscv013_halt_current_hart(struct target *target); -static void riscv013_resume_current_hart(struct target *target); -static void riscv013_step_current_hart(struct target *target); +static int riscv013_halt_current_hart(struct target *target); +static int riscv013_resume_current_hart(struct target *target); +static int riscv013_step_current_hart(struct target *target); static void riscv013_on_halt(struct target *target); static void riscv013_on_step(struct target *target); static void riscv013_on_resume(struct target *target); static bool riscv013_is_halted(struct target *target); static enum riscv_halt_reason riscv013_halt_reason(struct target *target); -static void riscv013_write_debug_buffer(struct target *target, unsigned index, +static int riscv013_write_debug_buffer(struct target *target, unsigned index, riscv_insn_t d); static riscv_insn_t riscv013_read_debug_buffer(struct target *target, unsigned index); @@ -418,7 +418,7 @@ static uint64_t dmi_read(struct target *target, uint16_t address) if (status != DMI_STATUS_SUCCESS) { LOG_ERROR("Failed read from 0x%x; status=%d", address, status); - abort(); + return ~0ULL; } /* This second loop ensures that we got the read @@ -441,13 +441,13 @@ static uint64_t dmi_read(struct target *target, uint16_t address) if (status != DMI_STATUS_SUCCESS) { LOG_ERROR("Failed read (NOP) from 0x%x; value=0x%" PRIx64 ", status=%d", address, value, status); - abort(); + return ~0ULL; } return value; } -static void dmi_write(struct target *target, uint16_t address, uint64_t value) +static int dmi_write(struct target *target, uint16_t address, uint64_t value) { select_dmi(target); dmi_status_t status = DMI_STATUS_BUSY; @@ -470,7 +470,7 @@ static void dmi_write(struct target *target, uint16_t address, uint64_t value) if (status != DMI_STATUS_SUCCESS) { LOG_ERROR("Failed write to 0x%x;, status=%d", address, status); - abort(); + return ERROR_FAIL; } /* The second loop isn't strictly necessary, but would ensure that the @@ -490,8 +490,10 @@ static void dmi_write(struct target *target, uint16_t address, uint64_t value) } if (status != DMI_STATUS_SUCCESS) { LOG_ERROR("failed to write (NOP) 0x%" PRIx64 " to 0x%x; status=%d", value, address, status); - abort(); + return ERROR_FAIL; } + + return ERROR_OK; } static void increase_ac_busy_delay(struct target *target) @@ -964,7 +966,7 @@ static int register_write_direct(struct target *target, unsigned number, riscv_program_csrw(&program, S0, number); } else { LOG_ERROR("Unsupported register (enum gdb_regno)(%d)", number); - abort(); + return ERROR_FAIL; } } @@ -1024,7 +1026,7 @@ static int register_read_direct(struct target *target, uint64_t *value, uint32_t riscv_program_csrr(&program, S0, number); } else { LOG_ERROR("Unsupported register (enum gdb_regno)(%d)", number); - abort(); + return ERROR_FAIL; } /* Execute program. */ @@ -1885,7 +1887,7 @@ static riscv_reg_t riscv013_get_register(struct target *target, int hid, int rid return out; } -static void riscv013_set_register(struct target *target, int hid, int rid, uint64_t value) +static int riscv013_set_register(struct target *target, int hid, int rid, uint64_t value) { LOG_DEBUG("writing 0x%" PRIx64 " to register %s on hart %d", value, gdb_regno_name(rid), hid); @@ -1893,22 +1895,28 @@ static void riscv013_set_register(struct target *target, int hid, int rid, uint6 riscv_set_current_hartid(target, hid); if (rid <= GDB_REGNO_XPR31) { - register_write_direct(target, rid, value); + return register_write_direct(target, rid, value); } else if (rid == GDB_REGNO_PC) { LOG_DEBUG("writing PC to DPC: 0x%016" PRIx64, value); register_write_direct(target, GDB_REGNO_DPC, value); uint64_t actual_value; register_read_direct(target, &actual_value, GDB_REGNO_DPC); LOG_DEBUG(" actual DPC written: 0x%016" PRIx64, actual_value); - assert(value == actual_value); + if (value != actual_value) { + LOG_ERROR("Written PC (0x%" PRIx64 ") does not match read back " + "value (0x%" PRIx64 ")", value, actual_value); + return ERROR_FAIL; + } } else if (rid == GDB_REGNO_PRIV) { uint64_t dcsr; register_read_direct(target, &dcsr, GDB_REGNO_DCSR); dcsr = set_field(dcsr, CSR_DCSR_PRV, value); - register_write_direct(target, GDB_REGNO_DCSR, dcsr); + return register_write_direct(target, GDB_REGNO_DCSR, dcsr); } else { - register_write_direct(target, rid, value); + return register_write_direct(target, rid, value); } + + return ERROR_OK; } static void riscv013_select_current_hart(struct target *target) @@ -1920,11 +1928,12 @@ static void riscv013_select_current_hart(struct target *target) dmi_write(target, DMI_DMCONTROL, dmcontrol); } -static void riscv013_halt_current_hart(struct target *target) +static int riscv013_halt_current_hart(struct target *target) { RISCV_INFO(r); LOG_DEBUG("halting hart %d", r->current_hartid); - assert(!riscv_is_halted(target)); + if (riscv_is_halted(target)) + LOG_ERROR("Hart %d is already halted!", r->current_hartid); /* Issue the halt command, and then wait for the current hart to halt. */ uint32_t dmcontrol = dmi_read(target, DMI_DMCONTROL); @@ -1941,19 +1950,21 @@ static void riscv013_halt_current_hart(struct target *target) LOG_ERROR("unable to halt hart %d", r->current_hartid); LOG_ERROR(" dmcontrol=0x%08x", dmcontrol); LOG_ERROR(" dmstatus =0x%08x", dmstatus); - abort(); + return ERROR_FAIL; } dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_HALTREQ, 0); dmi_write(target, DMI_DMCONTROL, dmcontrol); + + return ERROR_OK; } -static void riscv013_resume_current_hart(struct target *target) +static int riscv013_resume_current_hart(struct target *target) { return riscv013_step_or_resume_current_hart(target, false); } -static void riscv013_step_current_hart(struct target *target) +static int riscv013_step_current_hart(struct target *target) { return riscv013_step_or_resume_current_hart(target, true); } @@ -1998,10 +2009,10 @@ static enum riscv_halt_reason riscv013_halt_reason(struct target *target) LOG_ERROR("Unknown DCSR cause field: %x", (int)get_field(dcsr, CSR_DCSR_CAUSE)); LOG_ERROR(" dcsr=0x%016lx", (long)dcsr); - abort(); + return RISCV_HALT_UNKNOWN; } -void riscv013_write_debug_buffer(struct target *target, unsigned index, riscv_insn_t data) +int riscv013_write_debug_buffer(struct target *target, unsigned index, riscv_insn_t data) { return dmi_write(target, DMI_PROGBUF0 + index, data); } @@ -2070,17 +2081,20 @@ static void riscv013_on_step_or_resume(struct target *target, bool step) riscv_set_register(target, GDB_REGNO_DCSR, dcsr); } -static void riscv013_step_or_resume_current_hart(struct target *target, bool step) +static int riscv013_step_or_resume_current_hart(struct target *target, bool step) { RISCV_INFO(r); LOG_DEBUG("resuming hart %d (for step?=%d)", r->current_hartid, step); - assert(riscv_is_halted(target)); + if (!riscv_is_halted(target)) { + LOG_ERROR("Hart %d is not halted!", r->current_hartid); + 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) - abort(); + return ERROR_FAIL; /* Issue the resume command, and then wait for the current hart to resume. */ uint32_t dmcontrol = dmi_read(target, DMI_DMCONTROL); @@ -2097,7 +2111,7 @@ static void riscv013_step_or_resume_current_hart(struct target *target, bool ste dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 0); dmi_write(target, DMI_DMCONTROL, dmcontrol); - return; + return ERROR_OK; } uint32_t dmstatus = dmi_read(target, DMI_DMSTATUS); @@ -2109,10 +2123,10 @@ static void riscv013_step_or_resume_current_hart(struct target *target, bool ste if (step) { LOG_ERROR(" was stepping, halting"); riscv013_halt_current_hart(target); - return; + return ERROR_OK; } - abort(); + return ERROR_FAIL; } void riscv013_clear_abstract_error(struct target *target) diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index cda16f35b..009bc3a68 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -1011,6 +1011,9 @@ int riscv_openocd_poll(struct target *target) case RISCV_HALT_SINGLESTEP: target->debug_reason = DBG_REASON_SINGLESTEP; break; + case RISCV_HALT_UNKNOWN: + target->debug_reason = DBG_REASON_UNDEFINED; + break; } if (riscv_rtos_enabled(target)) { @@ -1361,8 +1364,7 @@ int riscv_halt_one_hart(struct target *target, int hartid) return ERROR_OK; } - r->halt_current_hart(target); - return ERROR_OK; + return r->halt_current_hart(target); } int riscv_resume_all_harts(struct target *target) @@ -1389,8 +1391,7 @@ int riscv_resume_one_hart(struct target *target, int hartid) } r->on_resume(target); - r->resume_current_hart(target); - return ERROR_OK; + return r->resume_current_hart(target); } int riscv_step_rtos_hart(struct target *target) @@ -1407,13 +1408,20 @@ int riscv_step_rtos_hart(struct target *target) riscv_set_current_hartid(target, hartid); LOG_DEBUG("stepping hart %d", hartid); - assert(riscv_is_halted(target)); + if (!riscv_is_halted(target)) { + LOG_ERROR("Hart isn't halted before single step!"); + return ERROR_FAIL; + } riscv_invalidate_register_cache(target); r->on_step(target); - r->step_current_hart(target); + if (r->step_current_hart(target) != ERROR_OK) + return ERROR_FAIL; riscv_invalidate_register_cache(target); r->on_halt(target); - assert(riscv_is_halted(target)); + if (!riscv_is_halted(target)) { + LOG_ERROR("Hart was not halted after single step!"); + return ERROR_FAIL; + } return ERROR_OK; } @@ -1523,13 +1531,12 @@ bool riscv_has_register(struct target *target, int hartid, int regid) return 1; } -void riscv_set_register(struct target *target, enum gdb_regno r, riscv_reg_t v) +int riscv_set_register(struct target *target, enum gdb_regno r, riscv_reg_t v) { - /* TODO: propagate errors */ return riscv_set_register_on_hart(target, riscv_current_hartid(target), r, v); } -void riscv_set_register_on_hart(struct target *target, int hartid, +int riscv_set_register_on_hart(struct target *target, int hartid, enum gdb_regno regid, uint64_t value) { RISCV_INFO(r); @@ -1562,7 +1569,10 @@ enum riscv_halt_reason riscv_halt_reason(struct target *target, int hartid) { RISCV_INFO(r); riscv_set_current_hartid(target, hartid); - assert(riscv_is_halted(target)); + if (!riscv_is_halted(target)) { + LOG_ERROR("Hart is not halted!"); + return RISCV_HALT_UNKNOWN; + } return r->halt_reason(target); } diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 71c68a2a7..dc3c6f5d1 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -30,6 +30,7 @@ enum riscv_halt_reason { RISCV_HALT_INTERRUPT, RISCV_HALT_BREAKPOINT, RISCV_HALT_SINGLESTEP, + RISCV_HALT_UNKNOWN }; typedef struct { @@ -88,18 +89,18 @@ typedef struct { /* Helper functions that target the various RISC-V debug spec * implementations. */ riscv_reg_t (*get_register)(struct target *, int hartid, int regid); - void (*set_register)(struct target *, int hartid, int regid, + int (*set_register)(struct target *, int hartid, int regid, uint64_t value); void (*select_current_hart)(struct target *); bool (*is_halted)(struct target *target); - void (*halt_current_hart)(struct target *); - void (*resume_current_hart)(struct target *target); - void (*step_current_hart)(struct target *target); + int (*halt_current_hart)(struct target *); + int (*resume_current_hart)(struct target *target); + int (*step_current_hart)(struct target *target); void (*on_halt)(struct target *target); void (*on_resume)(struct target *target); void (*on_step)(struct target *target); enum riscv_halt_reason (*halt_reason)(struct target *target); - void (*write_debug_buffer)(struct target *target, unsigned index, + int (*write_debug_buffer)(struct target *target, unsigned index, riscv_insn_t d); riscv_insn_t (*read_debug_buffer)(struct target *target, unsigned index); int (*execute_debug_buffer)(struct target *target); @@ -202,8 +203,8 @@ bool riscv_has_register(struct target *target, int hartid, int regid); /* Returns the value of the given register on the given hart. 32-bit registers * are zero extended to 64 bits. */ -void riscv_set_register(struct target *target, enum gdb_regno i, riscv_reg_t v); -void riscv_set_register_on_hart(struct target *target, int hid, enum gdb_regno rid, uint64_t v); +int riscv_set_register(struct target *target, enum gdb_regno i, riscv_reg_t v); +int riscv_set_register_on_hart(struct target *target, int hid, enum gdb_regno rid, uint64_t v); riscv_reg_t riscv_get_register(struct target *target, enum gdb_regno i); riscv_reg_t riscv_get_register_on_hart(struct target *target, int hid, enum gdb_regno rid);