Get rid of abort() calls.

Also changed a few asserts that could trigger due to broken hardware.

Fixes Issue #142.

Change-Id: Ia2b99baa82f30ebcb2fd7e4902f0e67046ce4ed2
This commit is contained in:
Tim Newsome 2017-12-27 13:04:30 -08:00
parent d7ed191457
commit 365c79c3ff
5 changed files with 77 additions and 54 deletions

View File

@ -55,7 +55,6 @@ int riscv_program_exec(struct riscv_program *p, struct target *t)
LOG_ERROR("Unable to write ebreak"); LOG_ERROR("Unable to write ebreak");
for (size_t i = 0; i < riscv_debug_buffer_size(p->target); ++i) 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]); 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; 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("Unable to insert instruction:");
LOG_ERROR(" instruction_count=%d", (int)p->instruction_count); LOG_ERROR(" instruction_count=%d", (int)p->instruction_count);
LOG_ERROR(" buffer size =%d", (int)riscv_debug_buffer_size(p->target)); LOG_ERROR(" buffer size =%d", (int)riscv_debug_buffer_size(p->target));
abort(); return ERROR_FAIL;
} }
p->debug_buffer[p->instruction_count] = i; p->debug_buffer[p->instruction_count] = i;

View File

@ -1329,12 +1329,11 @@ static riscv_reg_t get_register(struct target *target, int hartid, int regid)
return value; 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) uint64_t value)
{ {
assert(hartid == 0); assert(hartid == 0);
/* TODO: propagate errors */ return register_write(target, regid, value);
register_write(target, regid, value);
} }
static int halt(struct target *target) static int halt(struct target *target)

View File

@ -31,22 +31,22 @@
#define DMI_PROGBUF1 (DMI_PROGBUF0 + 1) #define DMI_PROGBUF1 (DMI_PROGBUF0 + 1)
static void riscv013_on_step_or_resume(struct target *target, bool step); 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); static void riscv013_clear_abstract_error(struct target *target);
/* Implementations of the functions in riscv_info_t. */ /* Implementations of the functions in riscv_info_t. */
static riscv_reg_t riscv013_get_register(struct target *target, int hartid, int regid); 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_select_current_hart(struct target *target);
static void riscv013_halt_current_hart(struct target *target); static int riscv013_halt_current_hart(struct target *target);
static void riscv013_resume_current_hart(struct target *target); static int riscv013_resume_current_hart(struct target *target);
static void riscv013_step_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_halt(struct target *target);
static void riscv013_on_step(struct target *target); static void riscv013_on_step(struct target *target);
static void riscv013_on_resume(struct target *target); static void riscv013_on_resume(struct target *target);
static bool riscv013_is_halted(struct target *target); static bool riscv013_is_halted(struct target *target);
static enum riscv_halt_reason riscv013_halt_reason(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); riscv_insn_t d);
static riscv_insn_t riscv013_read_debug_buffer(struct target *target, unsigned static riscv_insn_t riscv013_read_debug_buffer(struct target *target, unsigned
index); index);
@ -418,7 +418,7 @@ static uint64_t dmi_read(struct target *target, uint16_t address)
if (status != DMI_STATUS_SUCCESS) { if (status != DMI_STATUS_SUCCESS) {
LOG_ERROR("Failed read from 0x%x; status=%d", address, status); LOG_ERROR("Failed read from 0x%x; status=%d", address, status);
abort(); return ~0ULL;
} }
/* This second loop ensures that we got the read /* 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) { if (status != DMI_STATUS_SUCCESS) {
LOG_ERROR("Failed read (NOP) from 0x%x; value=0x%" PRIx64 ", status=%d", LOG_ERROR("Failed read (NOP) from 0x%x; value=0x%" PRIx64 ", status=%d",
address, value, status); address, value, status);
abort(); return ~0ULL;
} }
return value; 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); select_dmi(target);
dmi_status_t status = DMI_STATUS_BUSY; 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) { if (status != DMI_STATUS_SUCCESS) {
LOG_ERROR("Failed write to 0x%x;, status=%d", LOG_ERROR("Failed write to 0x%x;, status=%d",
address, status); address, status);
abort(); return ERROR_FAIL;
} }
/* The second loop isn't strictly necessary, but would ensure that the /* 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) { if (status != DMI_STATUS_SUCCESS) {
LOG_ERROR("failed to write (NOP) 0x%" PRIx64 " to 0x%x; status=%d", value, address, status); 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) 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); riscv_program_csrw(&program, S0, number);
} else { } else {
LOG_ERROR("Unsupported register (enum gdb_regno)(%d)", number); 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); riscv_program_csrr(&program, S0, number);
} else { } else {
LOG_ERROR("Unsupported register (enum gdb_regno)(%d)", number); LOG_ERROR("Unsupported register (enum gdb_regno)(%d)", number);
abort(); return ERROR_FAIL;
} }
/* Execute program. */ /* Execute program. */
@ -1885,7 +1887,7 @@ static riscv_reg_t riscv013_get_register(struct target *target, int hid, int rid
return out; 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, LOG_DEBUG("writing 0x%" PRIx64 " to register %s on hart %d", value,
gdb_regno_name(rid), hid); 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); riscv_set_current_hartid(target, hid);
if (rid <= GDB_REGNO_XPR31) { if (rid <= GDB_REGNO_XPR31) {
register_write_direct(target, rid, value); return register_write_direct(target, rid, value);
} else if (rid == GDB_REGNO_PC) { } else if (rid == GDB_REGNO_PC) {
LOG_DEBUG("writing PC to DPC: 0x%016" PRIx64, value); LOG_DEBUG("writing PC to DPC: 0x%016" PRIx64, value);
register_write_direct(target, GDB_REGNO_DPC, value); register_write_direct(target, GDB_REGNO_DPC, value);
uint64_t actual_value; uint64_t actual_value;
register_read_direct(target, &actual_value, GDB_REGNO_DPC); register_read_direct(target, &actual_value, GDB_REGNO_DPC);
LOG_DEBUG(" actual DPC written: 0x%016" PRIx64, actual_value); 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) { } else if (rid == GDB_REGNO_PRIV) {
uint64_t dcsr; uint64_t dcsr;
register_read_direct(target, &dcsr, GDB_REGNO_DCSR); register_read_direct(target, &dcsr, GDB_REGNO_DCSR);
dcsr = set_field(dcsr, CSR_DCSR_PRV, value); 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 { } 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) 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); 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); RISCV_INFO(r);
LOG_DEBUG("halting hart %d", r->current_hartid); 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. */ /* Issue the halt command, and then wait for the current hart to halt. */
uint32_t dmcontrol = dmi_read(target, DMI_DMCONTROL); 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("unable to halt hart %d", r->current_hartid);
LOG_ERROR(" dmcontrol=0x%08x", dmcontrol); LOG_ERROR(" dmcontrol=0x%08x", dmcontrol);
LOG_ERROR(" dmstatus =0x%08x", dmstatus); LOG_ERROR(" dmstatus =0x%08x", dmstatus);
abort(); return ERROR_FAIL;
} }
dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_HALTREQ, 0); dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_HALTREQ, 0);
dmi_write(target, DMI_DMCONTROL, dmcontrol); 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); 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); 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("Unknown DCSR cause field: %x", (int)get_field(dcsr, CSR_DCSR_CAUSE));
LOG_ERROR(" dcsr=0x%016lx", (long)dcsr); 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); 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); 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); RISCV_INFO(r);
LOG_DEBUG("resuming hart %d (for step?=%d)", r->current_hartid, step); 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; struct riscv_program program;
riscv_program_init(&program, target); riscv_program_init(&program, target);
riscv_program_fence_i(&program); riscv_program_fence_i(&program);
if (riscv_program_exec(&program, target) != ERROR_OK) 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. */ /* Issue the resume command, and then wait for the current hart to resume. */
uint32_t dmcontrol = dmi_read(target, DMI_DMCONTROL); 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); dmcontrol = set_field(dmcontrol, DMI_DMCONTROL_RESUMEREQ, 0);
dmi_write(target, DMI_DMCONTROL, dmcontrol); dmi_write(target, DMI_DMCONTROL, dmcontrol);
return; return ERROR_OK;
} }
uint32_t dmstatus = dmi_read(target, DMI_DMSTATUS); 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) { if (step) {
LOG_ERROR(" was stepping, halting"); LOG_ERROR(" was stepping, halting");
riscv013_halt_current_hart(target); riscv013_halt_current_hart(target);
return; return ERROR_OK;
} }
abort(); return ERROR_FAIL;
} }
void riscv013_clear_abstract_error(struct target *target) void riscv013_clear_abstract_error(struct target *target)

View File

@ -1011,6 +1011,9 @@ int riscv_openocd_poll(struct target *target)
case RISCV_HALT_SINGLESTEP: case RISCV_HALT_SINGLESTEP:
target->debug_reason = DBG_REASON_SINGLESTEP; target->debug_reason = DBG_REASON_SINGLESTEP;
break; break;
case RISCV_HALT_UNKNOWN:
target->debug_reason = DBG_REASON_UNDEFINED;
break;
} }
if (riscv_rtos_enabled(target)) { if (riscv_rtos_enabled(target)) {
@ -1361,8 +1364,7 @@ int riscv_halt_one_hart(struct target *target, int hartid)
return ERROR_OK; return ERROR_OK;
} }
r->halt_current_hart(target); return r->halt_current_hart(target);
return ERROR_OK;
} }
int riscv_resume_all_harts(struct target *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->on_resume(target);
r->resume_current_hart(target); return r->resume_current_hart(target);
return ERROR_OK;
} }
int riscv_step_rtos_hart(struct target *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); riscv_set_current_hartid(target, hartid);
LOG_DEBUG("stepping hart %d", 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); riscv_invalidate_register_cache(target);
r->on_step(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); riscv_invalidate_register_cache(target);
r->on_halt(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; return ERROR_OK;
} }
@ -1523,13 +1531,12 @@ bool riscv_has_register(struct target *target, int hartid, int regid)
return 1; 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); 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) enum gdb_regno regid, uint64_t value)
{ {
RISCV_INFO(r); RISCV_INFO(r);
@ -1562,7 +1569,10 @@ enum riscv_halt_reason riscv_halt_reason(struct target *target, int hartid)
{ {
RISCV_INFO(r); RISCV_INFO(r);
riscv_set_current_hartid(target, hartid); 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); return r->halt_reason(target);
} }

View File

@ -30,6 +30,7 @@ enum riscv_halt_reason {
RISCV_HALT_INTERRUPT, RISCV_HALT_INTERRUPT,
RISCV_HALT_BREAKPOINT, RISCV_HALT_BREAKPOINT,
RISCV_HALT_SINGLESTEP, RISCV_HALT_SINGLESTEP,
RISCV_HALT_UNKNOWN
}; };
typedef struct { typedef struct {
@ -88,18 +89,18 @@ typedef struct {
/* Helper functions that target the various RISC-V debug spec /* Helper functions that target the various RISC-V debug spec
* implementations. */ * implementations. */
riscv_reg_t (*get_register)(struct target *, int hartid, int regid); 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); uint64_t value);
void (*select_current_hart)(struct target *); void (*select_current_hart)(struct target *);
bool (*is_halted)(struct target *target); bool (*is_halted)(struct target *target);
void (*halt_current_hart)(struct target *); int (*halt_current_hart)(struct target *);
void (*resume_current_hart)(struct target *target); int (*resume_current_hart)(struct target *target);
void (*step_current_hart)(struct target *target); int (*step_current_hart)(struct target *target);
void (*on_halt)(struct target *target); void (*on_halt)(struct target *target);
void (*on_resume)(struct target *target); void (*on_resume)(struct target *target);
void (*on_step)(struct target *target); void (*on_step)(struct target *target);
enum riscv_halt_reason (*halt_reason)(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 d);
riscv_insn_t (*read_debug_buffer)(struct target *target, unsigned index); riscv_insn_t (*read_debug_buffer)(struct target *target, unsigned index);
int (*execute_debug_buffer)(struct target *target); 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 /* Returns the value of the given register on the given hart. 32-bit registers
* are zero extended to 64 bits. */ * are zero extended to 64 bits. */
void riscv_set_register(struct target *target, enum gdb_regno i, riscv_reg_t v); int 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_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(struct target *target, enum gdb_regno i);
riscv_reg_t riscv_get_register_on_hart(struct target *target, int hid, enum gdb_regno rid); riscv_reg_t riscv_get_register_on_hart(struct target *target, int hid, enum gdb_regno rid);