diff --git a/doc/openocd.texi b/doc/openocd.texi index 94a8fc78a..7ff0fa3ba 100644 --- a/doc/openocd.texi +++ b/doc/openocd.texi @@ -9271,7 +9271,7 @@ various operations. The current target may be changed by using @command{targets} command with the name of the target which should become current. -@deffn {Command} {reg} [(number|name) [(value|'force')]] +@deffn {Command} {reg} [(number|name)] [value] ['force'] Access a single register by @var{number} or by its @var{name}. The target must generally be halted before access to CPU core registers is allowed. Depending on the hardware, some other @@ -9285,15 +9285,17 @@ which are also dirty (and will be written back later) are flagged as such. @emph{With number/name}: display that register's value. -Use @var{force} argument to read directly from the target, -bypassing any internal cache. +Use @var{force} argument to read directly from the target +(as opposed to OpenOCD's internal register cache). @emph{With both number/name and value}: set register's value. Writes may be held in a writeback cache internal to OpenOCD, so that setting the value marks the register as dirty instead of immediately flushing that value. Resuming CPU execution (including by single stepping) or otherwise activating the -relevant module will flush such values. +relevant module will flush such values. The use of @var{force} +causes the register value to be written to the target +immediately. Cores may have surprisingly many registers in their Debug and trace infrastructure: @@ -9310,10 +9312,13 @@ Debug and trace infrastructure: @end example @end deffn -@deffn {Command} {set_reg} dict -Set register values of the target. +@deffn {Command} {set_reg} ['-force'] dict +Set register values of the target. The new register values may be +kept in OpenOCD's cache and not written to the target immediately +(unless @var{-force} is used). @itemize +@item @option{-force} ... Force immediate write of the new register values. @item @var{dict} ... Tcl dictionary with pairs of register names and values. @end itemize @@ -9329,7 +9334,7 @@ set_reg @{pc 0 sp 0x1000@} Get register values from the target and return them as Tcl dictionary with pairs of register names and values. If option "-force" is set, the register values are read directly from the -target, bypassing any caching. +target, not from OpenOCD's internal cache. @itemize @item @var{list} ... List of register names @@ -9343,6 +9348,13 @@ get_reg @{pc sp@} @end example @end deffn +@deffn {Command} {flush_reg_cache} [-invalidate] +Flush the internal OpenOCD's register cache - write back the dirty register values to the target. +If @option{-invalidate} is set, also invalidate (forget) the OpenOCD's cached register values; +therefore the next call to get_reg is guaranteed to read the fresh register value directly +from the target. +@end deffn + @deffn {Command} {write_memory} address width data ['phys'] This function provides an efficient way to write to the target memory from a Tcl script. diff --git a/src/target/arc.c b/src/target/arc.c index 0c111d553..de3e781dd 100644 --- a/src/target/arc.c +++ b/src/target/arc.c @@ -293,6 +293,7 @@ static int arc_set_register(struct reg *reg, uint8_t *buf) static const struct reg_arch_type arc_reg_type = { .get = arc_get_register, .set = arc_set_register, + .flush = NULL, }; /* GDB register groups. For now we support only general and "empty" */ diff --git a/src/target/armv4_5.c b/src/target/armv4_5.c index a258c7fd4..8b8caae8f 100644 --- a/src/target/armv4_5.c +++ b/src/target/armv4_5.c @@ -655,6 +655,7 @@ static int armv4_5_set_core_reg(struct reg *reg, uint8_t *buf) static const struct reg_arch_type arm_reg_type = { .get = armv4_5_get_core_reg, .set = armv4_5_set_core_reg, + .flush = NULL, }; struct reg_cache *arm_build_reg_cache(struct target *target, struct arm *arm) diff --git a/src/target/armv7m.c b/src/target/armv7m.c index 440ca49d1..e5c721e22 100644 --- a/src/target/armv7m.c +++ b/src/target/armv7m.c @@ -786,6 +786,7 @@ int armv7m_arch_state(struct target *target) static const struct reg_arch_type armv7m_reg_type = { .get = armv7m_get_core_reg, .set = armv7m_set_core_reg, + .flush = NULL, }; /** Builds cache of architecturally defined registers. */ diff --git a/src/target/armv8.c b/src/target/armv8.c index 50a9f4688..9029d514b 100644 --- a/src/target/armv8.c +++ b/src/target/armv8.c @@ -1714,6 +1714,7 @@ static int armv8_set_core_reg(struct reg *reg, uint8_t *buf) static const struct reg_arch_type armv8_reg_type = { .get = armv8_get_core_reg, .set = armv8_set_core_reg, + .flush = NULL, }; static int armv8_get_core_reg32(struct reg *reg) @@ -1775,6 +1776,7 @@ static int armv8_set_core_reg32(struct reg *reg, uint8_t *buf) static const struct reg_arch_type armv8_reg32_type = { .get = armv8_get_core_reg32, .set = armv8_set_core_reg32, + .flush = NULL, }; /** Builds cache of architecturally defined registers. */ diff --git a/src/target/avr32_ap7k.c b/src/target/avr32_ap7k.c index bbbf23659..2e5aa52b5 100644 --- a/src/target/avr32_ap7k.c +++ b/src/target/avr32_ap7k.c @@ -157,6 +157,7 @@ static int avr32_set_core_reg(struct reg *reg, uint8_t *buf) static const struct reg_arch_type avr32_reg_type = { .get = avr32_get_core_reg, .set = avr32_set_core_reg, + .flush = NULL, }; static struct reg_cache *avr32_build_reg_cache(struct target *target) diff --git a/src/target/cortex_m.c b/src/target/cortex_m.c index 9314d6675..a566d47ae 100644 --- a/src/target/cortex_m.c +++ b/src/target/cortex_m.c @@ -2431,6 +2431,7 @@ static const struct dwt_reg dwt_comp[] = { static const struct reg_arch_type dwt_reg_type = { .get = cortex_m_dwt_get_reg, .set = cortex_m_dwt_set_reg, + .flush = NULL, }; static void cortex_m_dwt_addreg(struct target *t, struct reg *r, const struct dwt_reg *d) diff --git a/src/target/dsp563xx.c b/src/target/dsp563xx.c index 0547947ef..bfa7a22f0 100644 --- a/src/target/dsp563xx.c +++ b/src/target/dsp563xx.c @@ -430,6 +430,7 @@ static int dsp563xx_set_core_reg(struct reg *reg, uint8_t *buf) static const struct reg_arch_type dsp563xx_reg_type = { .get = dsp563xx_get_core_reg, .set = dsp563xx_set_core_reg, + .flush = NULL, }; static void dsp563xx_build_reg_cache(struct target *target) diff --git a/src/target/embeddedice.c b/src/target/embeddedice.c index 7aef153b5..5a0a7246e 100644 --- a/src/target/embeddedice.c +++ b/src/target/embeddedice.c @@ -152,6 +152,7 @@ static int embeddedice_get_reg(struct reg *reg) static const struct reg_arch_type eice_reg_type = { .get = embeddedice_get_reg, .set = embeddedice_set_reg_w_exec, + .flush = NULL, }; /** diff --git a/src/target/esirisc.c b/src/target/esirisc.c index fc2d20105..08f78a7d2 100644 --- a/src/target/esirisc.c +++ b/src/target/esirisc.c @@ -1419,6 +1419,7 @@ static int esirisc_set_reg(struct reg *reg, uint8_t *buf) static const struct reg_arch_type esirisc_reg_type = { .get = esirisc_get_reg, .set = esirisc_set_reg, + .flush = NULL, }; static struct reg_cache *esirisc_build_reg_cache(struct target *target) diff --git a/src/target/etb.c b/src/target/etb.c index fb3112d70..6146da3a6 100644 --- a/src/target/etb.c +++ b/src/target/etb.c @@ -108,6 +108,7 @@ static int etb_get_reg(struct reg *reg) static const struct reg_arch_type etb_reg_type = { .get = etb_get_reg, .set = etb_set_reg_w_exec, + .flush = NULL, }; struct reg_cache *etb_build_reg_cache(struct etb *etb) diff --git a/src/target/etm.c b/src/target/etm.c index d9a3cdc5e..05acbf570 100644 --- a/src/target/etm.c +++ b/src/target/etm.c @@ -215,6 +215,7 @@ static int etm_write_reg(struct reg *reg, uint32_t value); static const struct reg_arch_type etm_scan6_type = { .get = etm_get_reg, .set = etm_set_reg_w_exec, + .flush = NULL, }; /* Look up register by ID ... most ETM instances only diff --git a/src/target/lakemont.c b/src/target/lakemont.c index 0340d0d0b..73fe4840a 100644 --- a/src/target/lakemont.c +++ b/src/target/lakemont.c @@ -357,6 +357,7 @@ static const struct reg_arch_type lakemont_reg_type = { */ .get = lakemont_get_core_reg, .set = lakemont_set_core_reg, + .flush = NULL, }; struct reg_cache *lakemont_build_reg_cache(struct target *t) diff --git a/src/target/mem_ap.c b/src/target/mem_ap.c index 5b2bbb1ae..5cf6128ad 100644 --- a/src/target/mem_ap.c +++ b/src/target/mem_ap.c @@ -179,6 +179,7 @@ static int mem_ap_reg_set(struct reg *reg, uint8_t *buf) static struct reg_arch_type mem_ap_reg_arch_type = { .get = mem_ap_reg_get, .set = mem_ap_reg_set, + .flush = NULL, }; static const char *mem_ap_get_gdb_arch(const struct target *target) diff --git a/src/target/mips32.c b/src/target/mips32.c index fcb7042cb..e1f37d76a 100644 --- a/src/target/mips32.c +++ b/src/target/mips32.c @@ -496,6 +496,7 @@ int mips32_arch_state(struct target *target) static const struct reg_arch_type mips32_reg_type = { .get = mips32_get_core_reg, .set = mips32_set_core_reg, + .flush = NULL, }; struct reg_cache *mips32_build_reg_cache(struct target *target) diff --git a/src/target/mips64.c b/src/target/mips64.c index 3d193a300..9fcadc62e 100644 --- a/src/target/mips64.c +++ b/src/target/mips64.c @@ -370,6 +370,7 @@ int mips64_arch_state(struct target *target) static const struct reg_arch_type mips64_reg_type = { .get = mips64_get_core_reg, .set = mips64_set_core_reg, + .flush = NULL, }; int mips64_build_reg_cache(struct target *target) diff --git a/src/target/openrisc/or1k.c b/src/target/openrisc/or1k.c index efc076c99..a989cf6b7 100644 --- a/src/target/openrisc/or1k.c +++ b/src/target/openrisc/or1k.c @@ -494,6 +494,7 @@ static int or1k_set_core_reg(struct reg *reg, uint8_t *buf) static const struct reg_arch_type or1k_reg_type = { .get = or1k_get_core_reg, .set = or1k_set_core_reg, + .flush = NULL, }; static struct reg_cache *or1k_build_reg_cache(struct target *target) diff --git a/src/target/register.c b/src/target/register.c index e4f22f8e9..cc967d45c 100644 --- a/src/target/register.c +++ b/src/target/register.c @@ -14,6 +14,8 @@ #include "register.h" #include +#include +#include /** * @file @@ -115,12 +117,72 @@ static int register_set_dummy_core_reg(struct reg *reg, uint8_t *buf) return ERROR_OK; } +static int register_flush_dummy(struct reg *reg) +{ + reg->dirty = false; + + return ERROR_OK; +} + static const struct reg_arch_type dummy_type = { .get = register_get_dummy_core_reg, .set = register_set_dummy_core_reg, + .flush = register_flush_dummy, }; void register_init_dummy(struct reg *reg) { reg->type = &dummy_type; } + +int register_flush(const struct target *target, struct reg *reg, bool invalidate) +{ + if (!reg) { + LOG_ERROR("BUG: %s called with NULL", __func__); + return ERROR_FAIL; + } + + if (!reg->exist) { + LOG_ERROR("BUG: %s called with non-existent register", __func__); + return ERROR_FAIL; + } + + if (!reg->dirty) { + LOG_TARGET_DEBUG(target, "Register '%s' is not dirty, nothing to flush", reg->name); + if (reg->valid && invalidate) { + LOG_TARGET_DEBUG(target, "Invalidating register '%s'", reg->name); + reg->valid = false; + } + return ERROR_OK; + } + + if (!reg->type->flush) { + LOG_TARGET_ERROR(target, "Unable to flush dirty register '%s' - operation not yet supported " + "by %s implementation in OpenOCD", reg->name, target->type->name); + return ERROR_NOT_IMPLEMENTED; + } + + if (!reg->valid) { + LOG_ERROR("BUG: Register '%s' is not valid, but flush attempted", reg->name); + return ERROR_FAIL; + } + + LOG_TARGET_DEBUG(target, "Flushing register '%s'", reg->name); + + int result = reg->type->flush(reg); + if (result != ERROR_OK) { + LOG_TARGET_ERROR(target, "Failed to flush register '%s'", reg->name); + return result; + } + + if (reg->dirty) { + LOG_ERROR("BUG: Register '%s' remained dirty after flushing", reg->name); + return ERROR_FAIL; + } + if (reg->valid && invalidate) { + LOG_TARGET_DEBUG(target, "Invalidating register '%s' after flush", reg->name); + reg->valid = false; + } + + return ERROR_OK; +} diff --git a/src/target/register.h b/src/target/register.h index b9af401b4..ca9fb2317 100644 --- a/src/target/register.h +++ b/src/target/register.h @@ -151,6 +151,7 @@ struct reg_cache { struct reg_arch_type { int (*get)(struct reg *reg); int (*set)(struct reg *reg, uint8_t *buf); + int (*flush)(struct reg *reg); }; struct reg *register_get_by_number(struct reg_cache *first, @@ -163,4 +164,7 @@ void register_cache_invalidate(struct reg_cache *cache); void register_init_dummy(struct reg *reg); +/* Flushes the register. Also invalidates the cached register value if invalidate == true */ +int register_flush(const struct target *target, struct reg *reg, bool invalidate); + #endif /* OPENOCD_TARGET_REGISTER_H */ diff --git a/src/target/riscv/program.h b/src/target/riscv/program.h index 91f0dabaa..1105ae623 100644 --- a/src/target/riscv/program.h +++ b/src/target/riscv/program.h @@ -39,11 +39,10 @@ int riscv_program_init(struct riscv_program *p, struct target *t); /* Write the program to the program buffer. */ int riscv_program_write(struct riscv_program *program); -/* Executes a program, returning 0 if the program successfully executed. Note - * that this may cause registers to be saved or restored, which could result to - * calls to things like riscv013_reg_save which itself could require a - * program to execute. That's OK, just make sure this eventually terminates. - * */ +/* Executes the RISC-V Program Buffer and returns ERROR_OK if the program + * buffer got successfully executed. In case of failure, more detailed error reason + * can be found in p->execution_result. + */ int riscv_program_exec(struct riscv_program *p, struct target *t); /* A lower level interface, you shouldn't use this unless you have a reason. */ diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 23c0aa62c..f20696f4b 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -986,7 +986,7 @@ static int examine_progbuf(struct target *target) return ERROR_OK; } - if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; struct riscv_program program; @@ -1318,7 +1318,7 @@ static int fpr_read_progbuf(struct target *target, uint64_t *value, const unsigned int freg = number - GDB_REGNO_FPR0; - if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; struct riscv_program program; @@ -1348,7 +1348,7 @@ static int csr_read_progbuf(struct target *target, uint64_t *value, assert(target->state == TARGET_HALTED); assert(number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095); - if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; struct riscv_program program; @@ -1416,7 +1416,7 @@ static int fpr_write_progbuf(struct target *target, enum gdb_regno number, assert(number >= GDB_REGNO_FPR0 && number <= GDB_REGNO_FPR31); const unsigned int freg = number - GDB_REGNO_FPR0; - if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; struct riscv_program program; @@ -1446,11 +1446,11 @@ static int vtype_write_progbuf(struct target *target, riscv_reg_t value) { assert(target->state == TARGET_HALTED); - if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; if (register_write_abstract(target, GDB_REGNO_S0, value) != ERROR_OK) return ERROR_FAIL; - if (riscv013_reg_save(target, GDB_REGNO_S1) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S1) != ERROR_OK) return ERROR_FAIL; struct riscv_program program; @@ -1467,11 +1467,11 @@ static int vl_write_progbuf(struct target *target, riscv_reg_t value) { assert(target->state == TARGET_HALTED); - if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; if (register_write_abstract(target, GDB_REGNO_S0, value) != ERROR_OK) return ERROR_FAIL; - if (riscv013_reg_save(target, GDB_REGNO_S1) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S1) != ERROR_OK) return ERROR_FAIL; struct riscv_program program; @@ -1490,7 +1490,7 @@ static int csr_write_progbuf(struct target *target, enum gdb_regno number, assert(target->state == TARGET_HALTED); assert(number >= GDB_REGNO_CSR0 && number <= GDB_REGNO_CSR4095); - if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; if (register_write_abstract(target, GDB_REGNO_S0, value) != ERROR_OK) return ERROR_FAIL; @@ -2296,7 +2296,7 @@ int riscv013_get_register_buf(struct target *target, uint8_t *value, &debug_vl, &debug_vsew) != ERROR_OK) return ERROR_FAIL; - if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; unsigned int vnum = regno - GDB_REGNO_V0; @@ -2351,7 +2351,7 @@ int riscv013_set_register_buf(struct target *target, enum gdb_regno regno, &debug_vl, &debug_vsew) != ERROR_OK) return ERROR_FAIL; - if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; unsigned int vnum = regno - GDB_REGNO_V0; @@ -4278,11 +4278,11 @@ static int read_memory_progbuf_inner_fill_progbuf(struct target *target, { const bool is_repeated_read = increment == 0; - if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; - if (riscv013_reg_save(target, GDB_REGNO_S1) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S1) != ERROR_OK) return ERROR_FAIL; - if (is_repeated_read && riscv013_reg_save(target, GDB_REGNO_A0) != ERROR_OK) + if (is_repeated_read && riscv013_reg_save_gpr(target, GDB_REGNO_A0) != ERROR_OK) return ERROR_FAIL; struct riscv_program program; @@ -4376,7 +4376,7 @@ read_memory_progbuf_inner_one(struct target *target, const riscv_mem_access_args { assert(riscv_mem_access_is_read(args)); - if (riscv013_reg_save(target, GDB_REGNO_S1) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S1) != ERROR_OK) return mem_access_result(MEM_ACCESS_SKIPPED_REG_SAVE_FAILED); struct riscv_program program; @@ -4951,9 +4951,9 @@ static int write_memory_progbuf_try_to_write(struct target *target, static int write_memory_progbuf_fill_progbuf(struct target *target, uint32_t size) { - if (riscv013_reg_save(target, GDB_REGNO_S0) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S0) != ERROR_OK) return ERROR_FAIL; - if (riscv013_reg_save(target, GDB_REGNO_S1) != ERROR_OK) + if (riscv013_reg_save_gpr(target, GDB_REGNO_S1) != ERROR_OK) return ERROR_FAIL; struct riscv_program program; @@ -5058,19 +5058,8 @@ struct target_type riscv013_target = { int riscv013_get_register(struct target *target, riscv_reg_t *value, enum gdb_regno rid) { - /* It would be beneficial to move this redirection to the - * version-independent section, but there is a conflict: - * `dcsr[5]` is `dcsr.v` in current spec, but it is `dcsr.debugint` in 0.11. - */ - if (rid == GDB_REGNO_PRIV) { - uint64_t dcsr; - if (riscv_reg_get(target, &dcsr, GDB_REGNO_DCSR) != ERROR_OK) - return ERROR_FAIL; - *value = set_field(0, VIRT_PRIV_V, get_field(dcsr, CSR_DCSR_V)); - *value = set_field(*value, VIRT_PRIV_PRV, get_field(dcsr, CSR_DCSR_PRV)); - return ERROR_OK; - } - + assert(rid != GDB_REGNO_PC && "'pc' should be read through 'dpc'"); + assert(rid != GDB_REGNO_PRIV && "'priv' should be read through 'dcsr'"); LOG_TARGET_DEBUG(target, "reading register %s", riscv_reg_gdb_regno_name(target, rid)); if (dm013_select_target(target) != ERROR_OK) @@ -5087,6 +5076,8 @@ int riscv013_get_register(struct target *target, int riscv013_set_register(struct target *target, enum gdb_regno rid, riscv_reg_t value) { + assert(rid != GDB_REGNO_PC && "'pc' should be written through 'dpc'"); + assert(rid != GDB_REGNO_PRIV && "'priv' should be written through 'dcsr'"); LOG_TARGET_DEBUG(target, "writing 0x%" PRIx64 " to register %s", value, riscv_reg_gdb_regno_name(target, rid)); diff --git a/src/target/riscv/riscv-013_reg.c b/src/target/riscv/riscv-013_reg.c index b2b1a921f..bb86a52fe 100644 --- a/src/target/riscv/riscv-013_reg.c +++ b/src/target/riscv/riscv-013_reg.c @@ -13,73 +13,455 @@ #include "debug_defines.h" #include -static int riscv013_reg_get(struct reg *reg) +/* Start of register fetch/send definitions. + * - "reg_fetcher" (named "fetch_*") -- loads the value from target into reg + * cache entry. Modifies only "reg->value". + * - "reg_sender" (named "send_*") -- stores the value from "buf" to the + * target. Does not modify "reg" at all. + */ +typedef int (*reg_fetcher)(struct reg *reg); +typedef int (*reg_sender)(const struct reg *reg, const uint8_t *buf); + +/* TODO: Introduce separate "fetch_*" and "send_*" functions for register + * classes (xregs, fregs, csrs). + */ +static int fetch_riscv013_reg(struct reg *reg) +{ + riscv_reg_t value; + int res = riscv013_get_register(riscv_reg_impl_get_target(reg), + &value, reg->number); + if (res != ERROR_OK) + return res; + buf_set_u64(reg->value, 0, reg->size, value); + return res; +} + +static int send_riscv013_reg(const struct reg *reg, const uint8_t *buf) +{ + assert(reg->size <= 64); + const riscv_reg_t value = buf_get_u64(buf, 0, reg->size); + return riscv013_set_register(riscv_reg_impl_get_target(reg), + reg->number, value); +} + +/* TODO: Inline "riscv013_*_register_buf" into "vreg" accessors. + */ +static int fetch_vreg(struct reg *reg) +{ + return riscv013_get_register_buf(riscv_reg_impl_get_target(reg), + reg->value, reg->number); +} + +static int send_vreg(const struct reg *reg, const uint8_t *buf) +{ + return riscv013_set_register_buf(riscv_reg_impl_get_target(reg), + reg->number, reg->value); +} + +/* End of register fetch/send definitions. */ +/* Start of cache entry utils. */ + +static void set_cache_value(struct reg *reg, const uint8_t *buf) +{ + assert(riscv_reg_impl_is_existing(reg)); + buf_cpy(buf, reg->value, reg->size); +} + +/** + * Note: There are only 3 reachable states for an item in the register cache: + * |: state :|: "reg->valid" :|: "reg->dirty" :| + * | "invalid" | false | false | + * | "valid" | true | false | + * | "dirty" | true | true | + * + * The state "reg->valid == dirty && reg->valid == false" should be unreacheable. + * To achieve this, "reg->valid" and "reg->dirty" are not assigned directly + * outside of "mark_()" functions. + */ +static void mark_invalid(struct reg *reg) +{ + assert(riscv_reg_impl_is_existing(reg)); + reg->valid = false; + reg->dirty = false; +} + +static void mark_clean(struct reg *reg) +{ + assert(riscv_reg_impl_is_existing(reg)); + assert(riscv_reg_impl_get_target(reg)->state == TARGET_HALTED); + reg->valid = true; + reg->dirty = false; +} + +static void mark_dirty(struct reg *reg) +{ + assert(riscv_reg_impl_is_existing(reg)); + assert(riscv_reg_impl_get_target(reg)->state == TARGET_HALTED); + reg->valid = true; + reg->dirty = true; +} + +static int access_prologue(const struct reg *reg, bool write) +{ + assert(riscv_reg_impl_is_initialized(reg)); + const struct target * const target = riscv_reg_impl_get_target(reg); + if (!reg->exist) { + LOG_TARGET_DEBUG(target, "Register %s does not exist", + reg->name); + return ERROR_FAIL; + } + LOG_TARGET_DEBUG(target, "%s %s (valid=%s, dirty=%s)", + write ? "Writing" : "Reading", + reg->name, + reg->valid ? "true" : "false", + reg->dirty ? "true" : "false"); + return ERROR_OK; +} + +static void access_epilogue(const struct reg *reg, bool write) +{ + assert(riscv_reg_impl_is_existing(reg)); + if (debug_level < LOG_LVL_DEBUG) + return; + char *value_hex = buf_to_hex_str(reg->value, reg->size); + LOG_TARGET_DEBUG(riscv_reg_impl_get_target(reg), + "%s 0x%s %s %s (valid=%s, dirty=%s)", + write ? "Wrote" : "Read", + value_hex, + write ? "to" : "from", + reg->name, + reg->valid ? "true" : "false", + reg->dirty ? "true" : "false"); + free(value_hex); +} + +static int noncaching_get(struct reg *reg, reg_fetcher fetch_reg) { - struct target *target = riscv_reg_impl_get_target(reg); + assert(riscv_reg_impl_is_initialized(reg)); + int res = access_prologue(reg, /*write*/ false); + if (res != ERROR_OK) + return res; + res = fetch_reg(reg); + mark_invalid(reg); + if (res != ERROR_OK) + return res; + access_epilogue(reg, /*write*/ false); + return ERROR_OK; +} - /* TODO: Hack to deal with gdb that thinks these registers still exist. */ - if (reg->number > GDB_REGNO_XPR15 && reg->number <= GDB_REGNO_XPR31 && - riscv_supports_extension(target, 'E')) { - buf_set_u64(reg->value, 0, reg->size, 0); +static int caching_get(struct reg *reg, reg_fetcher fetch_reg) +{ + assert(riscv_reg_impl_is_initialized(reg)); + int res = access_prologue(reg, /*write*/ false); + if (res != ERROR_OK) + return res; + if (reg->valid) { + LOG_TARGET_DEBUG(riscv_reg_impl_get_target(reg), + "Reading %s from cache", reg->name); return ERROR_OK; } + res = fetch_reg(reg); + if (res != ERROR_OK) { + mark_invalid(reg); + return res; + } + if (riscv_reg_impl_get_target(reg)->state == TARGET_HALTED) + mark_clean(reg); + else + mark_invalid(reg); + access_epilogue(reg, /*write*/ false); + return ERROR_OK; +} - if (reg->number >= GDB_REGNO_V0 && reg->number <= GDB_REGNO_V31) { - if (riscv013_get_register_buf(target, reg->value, reg->number) != ERROR_OK) - return ERROR_FAIL; +static int noncaching_set(struct reg *reg, const uint8_t *buf, + reg_sender send_reg) +{ + int res = access_prologue(reg, /*write*/ true); + if (res != ERROR_OK) + return res; + res = send_reg(reg, buf); + mark_invalid(reg); + if (res != ERROR_OK) + return res; + set_cache_value(reg, buf); + access_epilogue(reg, /*write*/ true); + return ERROR_OK; +} - reg->valid = riscv_reg_impl_gdb_regno_cacheable(reg->number, /* is write? */ false); +static int caching_set(struct reg *reg, const uint8_t *buf, + reg_sender send_reg) +{ + assert(riscv_reg_impl_is_initialized(reg)); + const struct target * const target = riscv_reg_impl_get_target(reg); + if (target->state != TARGET_HALTED) { + LOG_TARGET_DEBUG(target, "Not caching the write to %s", reg->name); + return noncaching_set(reg, buf, send_reg); + } + int res = access_prologue(reg, /*write*/ true); + if (res != ERROR_OK) + return res; + if (reg->valid && buf_eq(reg->value, buf, reg->size)) { + LOG_TARGET_DEBUG(riscv_reg_impl_get_target(reg), + "Writing the same value to %s", reg->name); } else { - uint64_t value; - int result = riscv_reg_get(target, &value, reg->number); - if (result != ERROR_OK) - return result; - buf_set_u64(reg->value, 0, reg->size, value); + LOG_TARGET_DEBUG(target, "Caching the write to %s", reg->name); + set_cache_value(reg, buf); + mark_dirty(reg); + } + access_epilogue(reg, /*write*/ true); + return ERROR_OK; +} + +static int flush_reg(struct reg *reg, reg_sender send_reg) +{ + if (!reg->dirty) + return ERROR_OK; + int res = access_prologue(reg, /*write*/ true); + if (res != ERROR_OK) + return res; + assert(reg->dirty); + assert(reg->valid); + const struct target * const target = riscv_reg_impl_get_target(reg); + if (target->state != TARGET_HALTED) { + LOG_TARGET_ERROR(target, + "BUG: register %s is dirty while the target is not halted", + reg->name); + return ERROR_TARGET_NOT_HALTED; + } + res = send_reg(reg, reg->value); + if (res != ERROR_OK) { + /* Register is not marked "invalid" here (like it's done on failure in + * "caching_set") since it is "dirty", i.e. the most recent value + * is currently in the cache. */ + return res; } - char *str = buf_to_hex_str(reg->value, reg->size); - LOG_TARGET_DEBUG(target, "Read 0x%s from %s (valid=%d).", str, reg->name, - reg->valid); - free(str); + mark_clean(reg); + access_epilogue(reg, /*write*/ true); + return res; +} + +/* End of cache entry utils. */ +/* Start of "reg_arch_type" method definitions. */ + +static int riscv013_noncaching_get(struct reg *reg) +{ + return noncaching_get(reg, fetch_riscv013_reg); +} + +static int riscv013_caching_get(struct reg *reg) +{ + return caching_get(reg, fetch_riscv013_reg); +} + +static int riscv013_noncaching_set(struct reg *reg, uint8_t *buf) +{ + return noncaching_set(reg, buf, send_riscv013_reg); +} + +static int riscv013_caching_set(struct reg *reg, uint8_t *buf) +{ + return caching_set(reg, buf, send_riscv013_reg); +} + +static int riscv013_reg_flush(struct reg *reg) +{ + return flush_reg(reg, send_riscv013_reg); +} + +static int flush_noncacheable(struct reg *reg) +{ + assert(!reg->dirty && "Non-cacheable register must never get dirty"); return ERROR_OK; } -static int riscv013_reg_set(struct reg *reg, uint8_t *buf) +static int get_vreg(struct reg *reg) { - struct target *target = riscv_reg_impl_get_target(reg); + assert(reg->number >= GDB_REGNO_V0 && reg->number <= GDB_REGNO_V31); + return caching_get(reg, fetch_vreg); +} - char *str = buf_to_hex_str(buf, reg->size); - LOG_TARGET_DEBUG(target, "Write 0x%s to %s (valid=%d).", str, reg->name, - reg->valid); - free(str); +static int set_vreg(struct reg *reg, uint8_t *buf) +{ + assert(reg->number >= GDB_REGNO_V0 && reg->number <= GDB_REGNO_V31); + return caching_set(reg, buf, send_vreg); +} - /* TODO: Hack to deal with gdb that thinks these registers still exist. */ - if (reg->number > GDB_REGNO_XPR15 && reg->number <= GDB_REGNO_XPR31 && - riscv_supports_extension(target, 'E') && - buf_get_u64(buf, 0, reg->size) == 0) - return ERROR_OK; +static int flush_vreg(struct reg *reg) +{ + assert(reg->number >= GDB_REGNO_V0 && reg->number <= GDB_REGNO_V31); + return flush_reg(reg, send_vreg); +} - if (reg->number >= GDB_REGNO_V0 && reg->number <= GDB_REGNO_V31) { - if (riscv013_set_register_buf(target, reg->number, buf) != ERROR_OK) - return ERROR_FAIL; +static int get_pc(struct reg *pc) +{ + assert(pc->number == GDB_REGNO_PC); + assert(!pc->valid); + assert(!pc->dirty); + struct reg * const dpc = + riscv_reg_impl_cache_entry(riscv_reg_impl_get_target(pc), + GDB_REGNO_DPC); + int res = dpc->type->get(dpc); + if (res == ERROR_OK) { + /* "pc" is just an alias for "dpc". "dpc" just got updated, sync up "pc" */ + set_cache_value(pc, dpc->value); + } + return res; +} - memcpy(reg->value, buf, DIV_ROUND_UP(reg->size, 8)); - reg->valid = riscv_reg_impl_gdb_regno_cacheable(reg->number, /* is write? */ true); - } else { - const riscv_reg_t value = buf_get_u64(buf, 0, reg->size); - if (riscv_reg_set(target, reg->number, value) != ERROR_OK) - return ERROR_FAIL; +static int set_pc(struct reg *pc, uint8_t *buf) +{ + assert(pc->number == GDB_REGNO_PC); + assert(!pc->valid); + assert(!pc->dirty); + struct reg * const dpc = + riscv_reg_impl_cache_entry(riscv_reg_impl_get_target(pc), + GDB_REGNO_DPC); + int res = dpc->type->set(dpc, buf); + if (res == ERROR_OK) { + /* "pc" is just an alias for "dpc". "dpc" just got updated, sync up "pc" */ + set_cache_value(pc, dpc->value); } + return res; +} +static int get_priv(struct reg *priv) +{ + assert(priv->number == GDB_REGNO_PRIV); + assert(!priv->valid); + assert(!priv->dirty); + struct reg * const dcsr = + riscv_reg_impl_cache_entry(riscv_reg_impl_get_target(priv), + GDB_REGNO_DCSR); + assert(dcsr->size == 32); + int res = dcsr->type->get(dcsr); + if (res != ERROR_OK) + return res; + buf_set_u32(priv->value, VIRT_PRIV_PRV_OFFSET, VIRT_PRIV_PRV_LENGTH, + buf_get_u32(dcsr->value, CSR_DCSR_PRV_OFFSET, CSR_DCSR_PRV_LENGTH)); + buf_set_u32(priv->value, VIRT_PRIV_V_OFFSET, VIRT_PRIV_V_LENGTH, + buf_get_u32(dcsr->value, CSR_DCSR_V_OFFSET, CSR_DCSR_V_LENGTH)); return ERROR_OK; } +static int set_priv(struct reg *priv, uint8_t *priv_buf) +{ + assert(priv->number == GDB_REGNO_PRIV); + assert(!priv->valid); + assert(!priv->dirty); + struct reg * const dcsr = riscv_reg_impl_cache_entry(riscv_reg_impl_get_target(priv), + GDB_REGNO_DCSR); + int res = dcsr->type->get(dcsr); + if (res != ERROR_OK) + return res; + assert(dcsr->size == 32); + uint8_t dcsr_buf[32 / 8]; + buf_cpy(dcsr->value, dcsr_buf, dcsr->size); + buf_set_u32(dcsr_buf, CSR_DCSR_PRV_OFFSET, CSR_DCSR_PRV_LENGTH, + buf_get_u32(priv_buf, VIRT_PRIV_PRV_OFFSET, VIRT_PRIV_PRV_LENGTH)); + buf_set_u32(dcsr_buf, CSR_DCSR_V_OFFSET, CSR_DCSR_V_LENGTH, + buf_get_u32(priv_buf, VIRT_PRIV_V_OFFSET, VIRT_PRIV_V_LENGTH)); + res = dcsr->type->set(dcsr, dcsr_buf); + if (res == ERROR_OK) + set_cache_value(priv, priv_buf); + return res; +} + +/* End of "reg_arch_type" method definitions. */ + +static const struct reg_arch_type zero_type = { + .get = riscv013_caching_get, + /* "zero" is read-only, but there is nothig wrong with attempting to + * write it (e.g. for the purpose of HW testing). */ + .set = riscv013_noncaching_set, + .flush = riscv013_reg_flush, +}; + +static const struct reg_arch_type xreg_type = { + .get = riscv013_caching_get, + .set = riscv013_caching_set, + .flush = riscv013_reg_flush, +}; + +static const struct reg_arch_type freg_type = { + .get = riscv013_caching_get, + .set = riscv013_caching_set, + .flush = riscv013_reg_flush, +}; + +static const struct reg_arch_type vreg_type = { + .get = get_vreg, + .set = set_vreg, + .flush = flush_vreg +}; + +static const struct reg_arch_type pc_type = { + .get = get_pc, + .set = set_pc, + .flush = flush_noncacheable +}; + +static const struct reg_arch_type priv_type = { + .get = get_priv, + .set = set_priv, + .flush = flush_noncacheable +}; + +static const struct reg_arch_type warl_csr_type = { + .get = riscv013_caching_get, + .set = riscv013_noncaching_set, + .flush = riscv013_reg_flush +}; + +static const struct reg_arch_type noncacheable_type = { + .get = riscv013_noncaching_get, + .set = riscv013_noncaching_set, + .flush = flush_noncacheable +}; + static const struct reg_arch_type *riscv013_gdb_regno_reg_type(uint32_t regno) { - static const struct reg_arch_type riscv013_reg_type = { - .get = riscv013_reg_get, - .set = riscv013_reg_set - }; - return &riscv013_reg_type; + if (regno != GDB_REGNO_ZERO && regno <= GDB_REGNO_XPR31) + return &xreg_type; + if (regno >= GDB_REGNO_FPR0 && regno <= GDB_REGNO_FPR31) { + /* For now "freg_type" is the same as "xreg_type", but it will be + * changed soon */ + return &freg_type; + } + if (regno >= GDB_REGNO_V0 && regno <= GDB_REGNO_V31) + return &vreg_type; + switch (regno) { + case GDB_REGNO_ZERO: + return &zero_type; + case GDB_REGNO_PC: + return &pc_type; + case GDB_REGNO_PRIV: + return &priv_type; + case GDB_REGNO_VLENB: + /* "vlenb" is read-only, but there is nothig wrong with attempting to + * write it (e.g. for the purpose of HW testing). */ + case GDB_REGNO_DPC: + case GDB_REGNO_VSTART: + case GDB_REGNO_VXSAT: + case GDB_REGNO_VXRM: + case GDB_REGNO_VL: + case GDB_REGNO_VTYPE: + case GDB_REGNO_MISA: + case GDB_REGNO_DCSR: + case GDB_REGNO_DSCRATCH0: + case GDB_REGNO_MEPC: + case GDB_REGNO_SATP: + return &warl_csr_type; + /* TODO: https://github.com/riscv-collab/riscv-openocd/issues/1219 */ + case GDB_REGNO_MSTATUS: + case GDB_REGNO_MCAUSE: + /* TODO: Sdtrig register should be WARL. Note that "tdata*" and "tinfo" + * values must get invalidated when "tselect" changes. */ + case GDB_REGNO_TSELECT: + case GDB_REGNO_TDATA1: + case GDB_REGNO_TDATA2: + default: + return &noncacheable_type; + } } static int init_cache_entry(struct target *target, uint32_t regno) @@ -345,42 +727,30 @@ int riscv013_reg_examine_all(struct target *target) return ERROR_OK; } -/** - * This function is used to save the value of a register in cache. The register - * is marked as dirty, and writeback is delayed for as long as possible. - */ -int riscv013_reg_save(struct target *target, enum gdb_regno regid) +int riscv013_reg_save_gpr(struct target *target, enum gdb_regno regid) { - if (target->state != TARGET_HALTED) { - LOG_TARGET_ERROR(target, "Can't save register %s on a hart that is not halted.", - riscv_reg_gdb_regno_name(target, regid)); - return ERROR_FAIL; - } - assert(riscv_reg_impl_gdb_regno_cacheable(regid, /* is write? */ false) && - "Only cacheable registers can be saved."); + assert(regid >= GDB_REGNO_ZERO && regid <= GDB_REGNO_XPR31); + struct reg *reg = riscv_reg_impl_cache_entry(target, regid); + assert(riscv_reg_impl_is_initialized(reg)); RISCV_INFO(r); - riscv_reg_t value; - if (!target->reg_cache) { - assert(!target_was_examined(target)); - /* To create register cache it is needed to examine the target first, - * therefore during examine, any changed register needs to be saved - * and restored manually. - */ - return ERROR_OK; + if (target->state != TARGET_HALTED) { + LOG_TARGET_ERROR(target, "Can't save register %s on a hart that is not halted", + reg->name); + return ERROR_TARGET_NOT_HALTED; } - struct reg *reg = riscv_reg_impl_cache_entry(target, regid); - LOG_TARGET_DEBUG(target, "Saving %s", reg->name); - if (riscv_reg_get(target, &value, regid) != ERROR_OK) - return ERROR_FAIL; + int res = reg->type->get(reg); + if (res != ERROR_OK) + return res; assert(reg->valid && "The register is cacheable, so the cache entry must be valid now."); - /* Mark the register dirty. We assume that this function is called + /* Mark the register as dirty so that its value is written back to the target + * during the next register cache flush. We assume that this function is called * because the caller is about to mess with the underlying value of the - * register. */ + * register, and wants to make sure the value gets restored later. */ reg->dirty = true; r->last_activity = timeval_ms(); diff --git a/src/target/riscv/riscv-013_reg.h b/src/target/riscv/riscv-013_reg.h index e542a3501..d31727beb 100644 --- a/src/target/riscv/riscv-013_reg.h +++ b/src/target/riscv/riscv-013_reg.h @@ -20,13 +20,10 @@ int riscv013_reg_examine_all(struct target *target); /** - * This function is used to save the value of a register in cache. The register - * is marked as dirty, and writeback is delayed for as long as possible. + * This function is used to save the value of a GPR in cache. The register is + * marked as dirty, and writeback is delayed for as long as possible. * Generally used to save registers before program buffer execution. - * - * TODO: The interface should be restricted in such a way that only GPRs can be - * saved. */ -int riscv013_reg_save(struct target *target, enum gdb_regno regid); +int riscv013_reg_save_gpr(struct target *target, enum gdb_regno regid); #endif /* OPENOCD_TARGET_RISCV_RISCV_013_REG_H */ diff --git a/src/target/riscv/riscv_reg.c b/src/target/riscv/riscv_reg.c index 1a0d4029d..37648eea4 100644 --- a/src/target/riscv/riscv_reg.c +++ b/src/target/riscv/riscv_reg.c @@ -8,19 +8,6 @@ #include "riscv.h" #include "riscv_reg.h" #include "riscv_reg_impl.h" -/** - * TODO: Currently `reg->get/set` is implemented in terms of - * `riscv_get/set_register`. However, the intention behind - * `riscv_get/set_register` is to work with the cache, therefore it accesses - * and modifyes register cache directly. The idea is to implement - * `riscv_get/set_register` in terms of `riscv_reg_impl_cache_entry` and - * `reg->get/set`. - * Once this is done, the following includes should be removed. - */ -#include "debug_defines.h" -#include "riscv-011.h" -#include "riscv-013.h" -#include "field_helpers.h" static const char * const default_reg_names[GDB_REGNO_COUNT] = { [GDB_REGNO_ZERO] = "zero", @@ -586,6 +573,7 @@ static int resize_reg(const struct target *target, uint32_t regno, bool exist, reg->size = size; reg->exist = exist; if (reg->exist) { + assert(size > 0); reg->value = malloc(DIV_ROUND_UP(reg->size, 8)); if (!reg->value) { LOG_ERROR("Failed to allocate memory."); @@ -784,97 +772,19 @@ int riscv_reg_flush_all(struct target *target) * may become dirty in the process (e.g. S0, S1). For that reason, flush * registers in reverse order, so that GPRs are flushed last. */ + int res = ERROR_OK; for (unsigned int number = target->reg_cache->num_regs; number-- > 0; ) { struct reg *reg = riscv_reg_impl_cache_entry(target, number); - if (reg->valid && reg->dirty) { - riscv_reg_t value = buf_get_u64(reg->value, 0, reg->size); - - LOG_TARGET_DEBUG(target, "%s is dirty; write back 0x%" PRIx64, - reg->name, value); - if (riscv_reg_write(target, number, value) != ERROR_OK) - return ERROR_FAIL; - } - } - LOG_TARGET_DEBUG(target, "Flush of register cache completed"); - return ERROR_OK; -} - -/** - * This function is used internally by functions that change register values. - * If `write_through` is true, it is ensured that the value of the target's - * register is set to be equal to the `value` argument. The cached value is - * updated if the register is cacheable. - * TODO: Currently `reg->get/set` is implemented in terms of - * `riscv_get/set_register`. However, the intention behind - * `riscv_get/set_register` is to work with the cache, therefore it accesses - * and modifyes register cache directly. The idea is to implement - * `riscv_get/set_register` in terms of `riscv_reg_impl_cache_entry` and - * `reg->get/set`. - */ -static int riscv_set_or_write_register(struct target *target, - enum gdb_regno regid, riscv_reg_t value, bool write_through) -{ - RISCV_INFO(r); - assert(r); - if (r->dtm_version == DTM_DTMCS_VERSION_0_11) - return riscv011_set_register(target, regid, value); - - keep_alive(); - - if (regid == GDB_REGNO_PC) { - return riscv_set_or_write_register(target, GDB_REGNO_DPC, value, write_through); - } else if (regid == GDB_REGNO_PRIV) { - riscv_reg_t dcsr; - - if (riscv_reg_get(target, &dcsr, GDB_REGNO_DCSR) != ERROR_OK) - return ERROR_FAIL; - dcsr = set_field(dcsr, CSR_DCSR_PRV, get_field(value, VIRT_PRIV_PRV)); - dcsr = set_field(dcsr, CSR_DCSR_V, get_field(value, VIRT_PRIV_V)); - return riscv_set_or_write_register(target, GDB_REGNO_DCSR, dcsr, write_through); - } - - struct reg *reg = riscv_reg_impl_cache_entry(target, regid); - assert(riscv_reg_impl_is_initialized(reg)); - - if (!reg->exist) { - LOG_TARGET_DEBUG(target, "Register %s does not exist.", reg->name); - return ERROR_FAIL; - } - - if (target->state != TARGET_HALTED) { - LOG_TARGET_DEBUG(target, - "Target not halted, writing to target: %s <- 0x%" PRIx64, - reg->name, value); - return riscv013_set_register(target, regid, value); - } - - const bool need_to_write = !reg->valid || reg->dirty || - value != buf_get_u64(reg->value, 0, reg->size); - const bool cacheable = riscv_reg_impl_gdb_regno_cacheable(regid, need_to_write); - - if (!cacheable || (write_through && need_to_write)) { - LOG_TARGET_DEBUG(target, - "Writing to target: %s <- 0x%" PRIx64 " (cacheable=%s, valid=%s, dirty=%s)", - reg->name, value, cacheable ? "true" : "false", - reg->valid ? "true" : "false", - reg->dirty ? "true" : "false"); - if (riscv013_set_register(target, regid, value) != ERROR_OK) - return ERROR_FAIL; - - reg->dirty = false; - } else { - reg->dirty = need_to_write; + assert(reg->type->flush + && "All RISC-V registers must have the 'flush' method defined"); + if (reg->type->flush(reg) != ERROR_OK) + res = ERROR_FAIL; } - - buf_set_u64(reg->value, 0, reg->size, value); - reg->valid = cacheable; - - LOG_TARGET_DEBUG(target, - "Wrote 0x%" PRIx64 " to %s (cacheable=%s, valid=%s, dirty=%s)", - value, reg->name, cacheable ? "true" : "false", - reg->valid ? "true" : "false", - reg->dirty ? "true" : "false"); - return ERROR_OK; + if (res == ERROR_OK) + LOG_TARGET_DEBUG(target, "Flush of register cache completed successfully"); + else + LOG_TARGET_ERROR(target, "Flush of register cache failed"); + return res; } bool riscv_reg_cache_any_dirty(const struct target *target, int log_level) @@ -908,83 +818,49 @@ void riscv_reg_cache_invalidate_all(struct target *target) /** * This function is used to change the value of a register. The new value may * be cached, and may not be written until the hart is resumed. - * TODO: Currently `reg->get/set` is implemented in terms of - * `riscv_get/set_register`. However, the intention behind - * `riscv_get/set_register` is to work with the cache, therefore it accesses - * and modifyes register cache directly. The idea is to implement - * `riscv_get/set_register` in terms of `riscv_reg_impl_cache_entry` and - * `reg->get/set`. */ int riscv_reg_set(struct target *target, enum gdb_regno regid, riscv_reg_t value) { - return riscv_set_or_write_register(target, regid, value, - /* write_through */ false); + struct reg * const reg = riscv_reg_impl_cache_entry(target, regid); + assert(riscv_reg_impl_is_initialized(reg)); + assert(reg->size <= sizeof(riscv_reg_t) * CHAR_BIT); + assert(reg->size <= 64); + uint8_t buf[64 / 8]; + buf_set_u64(buf, 0, reg->size, value); + return reg->type->set(reg, buf); } /** * This function is used to change the value of a register. The new value may * be cached, but it will be written to hart immediately. - * TODO: Currently `reg->get/set` is implemented in terms of - * `riscv_get/set_register`. However, the intention behind - * `riscv_get/set_register` is to work with the cache, therefore it accesses - * and modifyes register cache directly. The idea is to implement - * `riscv_get/set_register` in terms of `riscv_reg_impl_cache_entry` and - * `reg->get/set`. */ int riscv_reg_write(struct target *target, enum gdb_regno regid, riscv_reg_t value) { - return riscv_set_or_write_register(target, regid, value, - /* write_through */ true); + int res = riscv_reg_set(target, regid, value); + if (res != ERROR_OK) + return res; + struct reg * const reg = riscv_reg_impl_cache_entry(target, regid); + return reg->type->flush(reg); } /** * This function is used to get the value of a register. If possible, the value * in cache will be updated. - * TODO: Currently `reg->get/set` is implemented in terms of - * `riscv_get/set_register`. However, the intention behind - * `riscv_get/set_register` is to work with the cache, therefore it accesses - * and modifyes register cache directly. The idea is to implement - * `riscv_get/set_register` in terms of `riscv_reg_impl_cache_entry` and - * `reg->get/set`. */ int riscv_reg_get(struct target *target, riscv_reg_t *value, enum gdb_regno regid) { - RISCV_INFO(r); - assert(r); - if (r->dtm_version == DTM_DTMCS_VERSION_0_11) - return riscv011_get_register(target, value, regid); - - keep_alive(); + assert(value); - if (regid == GDB_REGNO_PC) - return riscv_reg_get(target, value, GDB_REGNO_DPC); - - struct reg *reg = riscv_reg_impl_cache_entry(target, regid); + struct reg * const reg = riscv_reg_impl_cache_entry(target, regid); assert(riscv_reg_impl_is_initialized(reg)); - if (!reg->exist) { - LOG_TARGET_DEBUG(target, "Register %s does not exist.", reg->name); - return ERROR_FAIL; - } - - if (reg->valid) { - *value = buf_get_u64(reg->value, 0, reg->size); - LOG_TARGET_DEBUG(target, "Read %s: 0x%" PRIx64 " (cached)", reg->name, - *value); - return ERROR_OK; - } - - LOG_TARGET_DEBUG(target, "Reading %s from target", reg->name); - if (riscv013_get_register(target, value, regid) != ERROR_OK) - return ERROR_FAIL; - - buf_set_u64(reg->value, 0, reg->size, *value); - reg->valid = riscv_reg_impl_gdb_regno_cacheable(regid, /* is write? */ false) && - target->state == TARGET_HALTED; - reg->dirty = false; - - LOG_TARGET_DEBUG(target, "Read %s: 0x%" PRIx64, reg->name, *value); + assert(reg->size <= sizeof(riscv_reg_t) * CHAR_BIT); + assert(reg->size <= 64); + int res = reg->type->get(reg); + if (res != ERROR_OK) + return res; + *value = buf_get_u64(reg->value, 0, reg->size); return ERROR_OK; } diff --git a/src/target/riscv/riscv_reg_impl.h b/src/target/riscv/riscv_reg_impl.h index a86020252..fddcf3344 100644 --- a/src/target/riscv/riscv_reg_impl.h +++ b/src/target/riscv/riscv_reg_impl.h @@ -30,6 +30,16 @@ static inline bool riscv_reg_impl_is_initialized(const struct reg *reg) assert(reg->valid || !reg->dirty); return true; } + +static inline bool riscv_reg_impl_is_existing(const struct reg *reg) +{ + assert(riscv_reg_impl_is_initialized(reg)); + if (!reg->exist) + return false; + assert(reg->size > 0); + assert(reg->value); + return true; +} /** * Initialize register cache. Note, that each specific register cache entry is * not initialized by this function. @@ -171,52 +181,4 @@ int riscv_reg_impl_expose_csrs(const struct target *target); /** Hide additional CSRs, as specified by `riscv_info_t::hide_csr` list. */ void riscv_reg_impl_hide_csrs(const struct target *target); -/** - * If write is true: - * return true iff we are guaranteed that the register will contain exactly - * the value we just wrote when it's read. - * If write is false: - * return true iff we are guaranteed that the register will read the same - * value in the future as the value we just read. - */ -static inline bool riscv_reg_impl_gdb_regno_cacheable(enum gdb_regno regno, - bool is_write) -{ - if (regno == GDB_REGNO_ZERO) - return !is_write; - - /* GPRs, FPRs, vector registers are just normal data stores. */ - if (regno <= GDB_REGNO_XPR31 || - (regno >= GDB_REGNO_FPR0 && regno <= GDB_REGNO_FPR31) || - (regno >= GDB_REGNO_V0 && regno <= GDB_REGNO_V31)) - return true; - - /* Most CSRs won't change value on us, but we can't assume it about arbitrary - * CSRs. */ - switch (regno) { - case GDB_REGNO_DPC: - case GDB_REGNO_VSTART: - case GDB_REGNO_VXSAT: - case GDB_REGNO_VXRM: - case GDB_REGNO_VLENB: - case GDB_REGNO_VL: - case GDB_REGNO_VTYPE: - case GDB_REGNO_MISA: - case GDB_REGNO_DCSR: - case GDB_REGNO_DSCRATCH0: - case GDB_REGNO_MEPC: - case GDB_REGNO_SATP: - /* - * WARL registers might not contain the value we just wrote, but - * these ones won't spontaneously change their value either. * - */ - return !is_write; - - case GDB_REGNO_TSELECT: /* I think this should be above, but then it doesn't work. */ - case GDB_REGNO_TDATA1: /* Changes value when tselect is changed. */ - case GDB_REGNO_TDATA2: /* Changes value when tselect is changed. */ - default: - return false; - } -} #endif /* OPENOCD_TARGET_RISCV_RISCV_REG_IMPL_H */ diff --git a/src/target/stm8.c b/src/target/stm8.c index fb5c81f06..f7205f3fa 100644 --- a/src/target/stm8.c +++ b/src/target/stm8.c @@ -1180,6 +1180,7 @@ static int stm8_get_gdb_reg_list(struct target *target, struct reg **reg_list[], static const struct reg_arch_type stm8_reg_type = { .get = stm8_get_core_reg, .set = stm8_set_core_reg, + .flush = NULL, }; static struct reg_cache *stm8_build_reg_cache(struct target *target) diff --git a/src/target/target.c b/src/target/target.c index 56eff9242..eacd05c34 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -1470,6 +1470,86 @@ unsigned int target_data_bits(struct target *target) return 32; } +static bool target_implements_reg_flush(const struct reg_cache *cache) +{ + while (cache) { + for (unsigned int i = 0; i < cache->num_regs; i++) { + struct reg *reg = &cache->reg_list[i]; + if (reg->type->flush) + return true; + } + cache = cache->next; + } + return false; +} + +static int target_flush_all_regs_default(struct target *target, bool invalidate) +{ + struct reg_cache *cache = target->reg_cache; + + /* In targets where per-register flushing isn't implemented yet, this function + * would result in many errors (one for each dirty register). The check below is + * to prevent that, and just a single clear error message will be printed. */ + if (!target_implements_reg_flush(cache)) { + LOG_TARGET_ERROR(target, "Unable to flush register cache - operation not yet supported " + "by %s implementation in OpenOCD", target->type->name); + return ERROR_FAIL; + } + + unsigned int total_count = 0; + unsigned int dirty_count = 0; + unsigned int failed_count = 0; + while (cache) { + for (unsigned int i = 0; i < cache->num_regs; i++) { + total_count++; + struct reg *reg = &cache->reg_list[i]; + + if (!reg->exist || !reg->valid) + continue; + + if (reg->dirty) { + dirty_count++; + if (!reg->type->flush) { + LOG_TARGET_WARNING(target, "Flushing register '%s' is not implemented", reg->name); + failed_count++; + continue; + } + LOG_TARGET_DEBUG(target, "Flushing register %s", reg->name); + int result = reg->type->flush(reg); + if (result != ERROR_OK) { + LOG_TARGET_ERROR(target, "Failed to flush register '%s'", reg->name); + failed_count++; + continue; + } + if (reg->dirty) { + LOG_TARGET_ERROR(target, "BUG: Register '%s' remains dirty after flushing", reg->name); + return ERROR_FAIL; + } + } + if (invalidate) { + LOG_TARGET_DEBUG(target, "Invalidating register '%s'", reg->name); + reg->valid = false; + } + } + cache = cache->next; + } + + if (dirty_count == 0) { + LOG_TARGET_INFO(target, "No registers dirty, nothing to flush"); + return ERROR_OK; + } + + LOG_TARGET_DEBUG(target, "Found %u dirty registers out of %u registers", dirty_count, total_count); + if (failed_count > 0) { + LOG_TARGET_ERROR(target, "Failed to flush %u out of %u dirty registers", + failed_count, dirty_count); + return ERROR_FAIL; + } + LOG_TARGET_DEBUG(target, "All dirty registers flushed"); + + return ERROR_OK; +} + static int target_profiling(struct target *target, uint32_t *samples, uint32_t max_num_samples, uint32_t *num_samples, uint32_t seconds) { @@ -1536,6 +1616,9 @@ static int target_init_one(struct command_context *cmd_ctx, if (!target->type->profiling) target->type->profiling = target_profiling_default; + if (!target->type->flush_all_regs) + target->type->flush_all_regs = target_flush_all_regs_default; + return ERROR_OK; } @@ -3106,16 +3189,24 @@ COMMAND_HANDLER(handle_reg_command) /* display a register */ if ((CMD_ARGC == 1) || ((CMD_ARGC == 2) && !((CMD_ARGV[1][0] >= '0') && (CMD_ARGV[1][0] <= '9')))) { - if ((CMD_ARGC == 2) && (strcmp(CMD_ARGV[1], "force") == 0)) - reg->valid = false; + if (CMD_ARGC == 2 + && strcmp(CMD_ARGV[1], "force") == 0 + && reg->valid) { + int retval = register_flush(target, reg, true); + if (retval != ERROR_OK) { + LOG_TARGET_ERROR(target, "Failed to flush register '%s' before force-reading", reg->name); + return retval; + } + } if (!reg->valid) { int retval = reg->type->get(reg); if (retval != ERROR_OK) { - LOG_ERROR("Could not read register '%s'", reg->name); + LOG_TARGET_ERROR(target, "Could not read register '%s'", reg->name); return retval; } } + char *value = buf_to_hex_str(reg->value, reg->size); command_print(CMD, "%s (/%i): 0x%s", reg->name, (int)(reg->size), value); free(value); @@ -3123,7 +3214,14 @@ COMMAND_HANDLER(handle_reg_command) } /* set register value */ - if (CMD_ARGC == 2) { + if (CMD_ARGC >= 2 && CMD_ARGC <= 3) { + bool flush = false; + + if (CMD_ARGC >= 3 && !strcmp(CMD_ARGV[2], "force")) + flush = true; + else if (CMD_ARGC >= 3) + return ERROR_COMMAND_SYNTAX_ERROR; + uint8_t *buf = malloc(DIV_ROUND_UP(reg->size, 8)); if (!buf) { LOG_ERROR("Failed to allocate memory"); @@ -3137,17 +3235,21 @@ COMMAND_HANDLER(handle_reg_command) } retval = reg->type->set(reg, buf); + free(buf); + if (retval != ERROR_OK) { LOG_ERROR("Could not write to register '%s'", reg->name); + return retval; } else { char *value = buf_to_hex_str(reg->value, reg->size); command_print(CMD, "%s (/%i): 0x%s", reg->name, (int)(reg->size), value); free(value); } - free(buf); + if (reg->dirty && flush) + return register_flush(target, reg, false); - return retval; + return ERROR_OK; } return ERROR_COMMAND_SYNTAX_ERROR; @@ -4742,6 +4844,14 @@ static int target_jim_get_reg(Jim_Interp *interp, int argc, return JIM_ERR; } + if (force && reg->valid && reg->dirty) { + int retval = register_flush(target, reg, false); + if (retval != ERROR_OK) { + Jim_SetResultFormatted(interp, "failed to flush dirty register '%s' before force-reading", reg->name); + return retval; + } + } + if (force || !reg->valid) { int retval = reg->type->get(reg); @@ -4781,18 +4891,30 @@ static int target_jim_get_reg(Jim_Interp *interp, int argc, COMMAND_HANDLER(handle_set_reg_command) { - if (CMD_ARGC != 1) + if (CMD_ARGC < 1 || CMD_ARGC > 2) return ERROR_COMMAND_SYNTAX_ERROR; + bool flush = false; + + /* Check for the "-force" argument */ + if (CMD_ARGC == 2) { + if (!strcmp(CMD_ARGV[0], "-force")) { + flush = true; + } else { + command_print(CMD, "unknown argument '%s'", CMD_ARGV[0]); + return ERROR_COMMAND_ARGUMENT_INVALID; + } + } + int tmp; #if JIM_VERSION >= 80 - Jim_Obj **dict = Jim_DictPairs(CMD_CTX->interp, CMD_JIMTCL_ARGV[0], &tmp); + Jim_Obj **dict = Jim_DictPairs(CMD_CTX->interp, CMD_JIMTCL_ARGV[CMD_ARGC - 1], &tmp); if (!dict) return ERROR_FAIL; #else Jim_Obj **dict; - int ret = Jim_DictPairs(CMD_CTX->interp, CMD_JIMTCL_ARGV[0], &dict, &tmp); + int ret = Jim_DictPairs(CMD_CTX->interp, CMD_JIMTCL_ARGV[CMD_ARGC - 1], &dict, &tmp); if (ret != JIM_OK) return ERROR_FAIL; @@ -4833,6 +4955,14 @@ COMMAND_HANDLER(handle_set_reg_command) reg_value, reg_name); return retval; } + + if (flush && reg->valid && reg->dirty) { + retval = register_flush(target, reg, false); + if (retval != ERROR_OK) { + Jim_SetResultFormatted(CMD_CTX->interp, "Failed to flush register '%s'", reg->name); + return retval; + } + } } return ERROR_OK; @@ -5570,14 +5700,14 @@ static const struct command_registration target_instance_command_handlers[] = { .mode = COMMAND_EXEC, .jim_handler = target_jim_get_reg, .help = "Get register values from the target", - .usage = "list", + .usage = "['-force'] list", }, { .name = "set_reg", .mode = COMMAND_EXEC, .handler = handle_set_reg_command, .help = "Set target register values", - .usage = "dict", + .usage = "['-force'] dict", }, { .name = "read_memory", @@ -6495,6 +6625,24 @@ COMMAND_HANDLER(handle_test_mem_access_command) return retval; } +COMMAND_HANDLER(handle_flush_reg_cache_command) +{ + struct target *target = get_current_target(CMD_CTX); + + if (CMD_ARGC > 1) + return ERROR_COMMAND_SYNTAX_ERROR; + + bool invalidate = false; + + if (CMD_ARGC == 1) { + if (strcmp(CMD_ARGV[0], "-invalidate")) + return ERROR_COMMAND_SYNTAX_ERROR; + invalidate = true; + } + + return target->type->flush_all_regs(target, invalidate); +} + static const struct command_registration target_exec_command_handlers[] = { { .name = "fast_load_image", @@ -6532,9 +6680,9 @@ static const struct command_registration target_exec_command_handlers[] = { .name = "reg", .handler = handle_reg_command, .mode = COMMAND_EXEC, - .help = "display (reread from target with \"force\") or set a register; " - "with no arguments, displays all registers and their values", - .usage = "[(register_number|register_name) [(value|'force')]]", + .help = "display (reread from target with \"force\") or set a register " + "(write to the target immediately with \"force\"); ", + .usage = "[(register_number|register_name) [value] ['force']]", }, { .name = "poll", @@ -6708,14 +6856,21 @@ static const struct command_registration target_exec_command_handlers[] = { .mode = COMMAND_EXEC, .jim_handler = target_jim_get_reg, .help = "Get register values from the target", - .usage = "list", + .usage = "['-force'] list", }, { .name = "set_reg", .mode = COMMAND_EXEC, .handler = handle_set_reg_command, .help = "Set target register values", - .usage = "dict", + .usage = "['-force'] dict", + }, + { + .name = "flush_reg_cache", + .handler = handle_flush_reg_cache_command, + .mode = COMMAND_EXEC, + .help = "Flush register cache", + .usage = "['-invalidate']", }, { .name = "read_memory", @@ -6760,7 +6915,6 @@ static const struct command_registration target_exec_command_handlers[] = { .help = "Test the target's memory access functions", .usage = "size", }, - COMMAND_REGISTRATION_DONE }; static int target_register_user_commands(struct command_context *cmd_ctx) diff --git a/src/target/target_type.h b/src/target/target_type.h index f35a59c5f..6d17ab5a7 100644 --- a/src/target/target_type.h +++ b/src/target/target_type.h @@ -309,6 +309,10 @@ struct target_type { * will typically be 32 for 32-bit targets, and 64 for 64-bit targets. If * not implemented, it's assumed to be 32. */ unsigned int (*data_bits)(struct target *target); + + /* Flush the whole register cache. + * Also invalidate flushed register values if invalidate == true */ + int (*flush_all_regs)(struct target *target, bool invalidate); }; extern struct target_type aarch64_target; diff --git a/src/target/xscale.c b/src/target/xscale.c index 83afd5de2..21366a2ad 100644 --- a/src/target/xscale.c +++ b/src/target/xscale.c @@ -2854,6 +2854,7 @@ static int xscale_analyze_trace(struct target *target, struct command_invocation static const struct reg_arch_type xscale_reg_type = { .get = xscale_get_reg, .set = xscale_set_reg, + .flush = NULL, }; static void xscale_build_reg_cache(struct target *target) diff --git a/src/target/xtensa/xtensa.c b/src/target/xtensa/xtensa.c index 3b888fe5f..7645e3b58 100644 --- a/src/target/xtensa/xtensa.c +++ b/src/target/xtensa/xtensa.c @@ -490,6 +490,7 @@ static int xtensa_core_reg_set(struct reg *reg, uint8_t *buf) static const struct reg_arch_type xtensa_reg_type = { .get = xtensa_core_reg_get, .set = xtensa_core_reg_set, + .flush = NULL, }; /* Convert a register index that's indexed relative to windowbase, to the real address. */