From de20c2ad5faa0bc411b86bd6db69186b8c2ac979 Mon Sep 17 00:00:00 2001 From: Evgeniy Naydanov Date: Thu, 5 Dec 2024 16:27:26 +0300 Subject: [PATCH] target/riscv: clean-up register invalidation * Registers were not invalidated if the hart became unavailable. * Improved logging in the case register invalidation involves loss of information. Change-Id: Icfb5e190dd6dcb1a97e4d314d802466cab7a01e4 Signed-off-by: Evgeniy Naydanov --- src/target/riscv/riscv-011.c | 2 +- src/target/riscv/riscv-013.c | 9 +++++++ src/target/riscv/riscv.c | 51 ++++++++++++++++++++++-------------- src/target/riscv/riscv_reg.c | 28 ++++++++++++++++++++ src/target/riscv/riscv_reg.h | 11 ++++++++ 5 files changed, 80 insertions(+), 21 deletions(-) diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index 2e89ebaf9..1a3f4b12e 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -1136,7 +1136,7 @@ static int execute_resume(struct target *target, bool step) } target->state = TARGET_RUNNING; - register_cache_invalidate(target->reg_cache); + riscv_reg_cache_invalidate_all(target); return ERROR_OK; } diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 0747842f8..1a4b038d1 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -2728,6 +2728,13 @@ static int handle_became_unavailable(struct target *target, enum riscv_hart_state previous_riscv_state) { RISCV013_INFO(info); + + if (riscv_reg_cache_any_dirty(target, LOG_LVL_WARNING)) + LOG_TARGET_WARNING(target, "Discarding values of dirty registers " + "(due to target becoming unavailable)."); + + riscv_reg_cache_invalidate_all(target); + info->dcsr_ebreak_is_set = false; return ERROR_OK; } @@ -5435,6 +5442,8 @@ static int riscv013_step_or_resume_current_hart(struct target *target, if (riscv_reg_flush_all(target) != ERROR_OK) return ERROR_FAIL; + riscv_reg_cache_invalidate_all(target); + dm013_info_t *dm = get_dm(target); /* Issue the resume command, and then wait for the current hart to resume. */ uint32_t dmcontrol = DM_DMCONTROL_DMACTIVE | DM_DMCONTROL_RESUMEREQ; diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 48adde48b..0ca921657 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -290,7 +290,6 @@ static const virt2phys_info_t sv57x4 = { static enum riscv_halt_reason riscv_halt_reason(struct target *target); static void riscv_info_init(struct target *target, struct riscv_info *r); -static void riscv_invalidate_register_cache(struct target *target); static int riscv_step_rtos_hart(struct target *target); static void riscv_sample_buf_maybe_add_timestamp(struct target *target, bool before) @@ -2485,10 +2484,15 @@ static int riscv_halt_go_all_harts(struct target *target) return ERROR_FAIL; } } else { + // Safety check: + if (riscv_reg_cache_any_dirty(target, LOG_LVL_ERROR)) + LOG_TARGET_INFO(target, "BUG: Registers should not be dirty while " + "the target is not halted!"); + + riscv_reg_cache_invalidate_all(target); + if (r->halt_go(target) != ERROR_OK) return ERROR_FAIL; - - riscv_invalidate_register_cache(target); } return ERROR_OK; @@ -2572,7 +2576,11 @@ static int riscv_assert_reset(struct target *target) struct target_type *tt = get_target_type(target); if (!tt) return ERROR_FAIL; - riscv_invalidate_register_cache(target); + + if (riscv_reg_cache_any_dirty(target, LOG_LVL_INFO)) + LOG_TARGET_INFO(target, "Discarding values of dirty registers."); + + riscv_reg_cache_invalidate_all(target); return tt->assert_reset(target); } @@ -2699,7 +2707,15 @@ static int resume_go(struct target *target, int current, static int resume_finish(struct target *target, int debug_execution) { assert(target->state == TARGET_HALTED); - register_cache_invalidate(target->reg_cache); + if (riscv_reg_cache_any_dirty(target, LOG_LVL_ERROR)) { + /* If this happens, it means there is a bug in the previous + * register-flushing algorithm: not all registers were flushed + * back to the target in preparation for the resume.*/ + LOG_TARGET_ERROR(target, + "BUG: registers should have been flushed by this point."); + } + + riscv_reg_cache_invalidate_all(target); target->state = debug_execution ? TARGET_DEBUG_RUNNING : TARGET_RUNNING; target->debug_reason = DBG_REASON_NOTHALTED; @@ -4001,7 +4017,15 @@ static int riscv_openocd_step_impl(struct target *target, int current, LOG_TARGET_ERROR(target, "Unable to step rtos hart."); } - register_cache_invalidate(target->reg_cache); + if (riscv_reg_cache_any_dirty(target, LOG_LVL_ERROR)) { + /* If this happens, it means there is a bug in the previous + * register-flushing algorithm: not all registers were flushed + * back to the target prior to single-step. */ + LOG_TARGET_ERROR(target, + "BUG: registers should have been flushed by this point."); + } + + riscv_reg_cache_invalidate_all(target); if (info->isrmask_mode == RISCV_ISRMASK_STEPONLY) if (riscv_interrupts_restore(target, current_mstatus) != ERROR_OK) { @@ -5177,7 +5201,7 @@ COMMAND_HANDLER(riscv_exec_progbuf) if (riscv_reg_flush_all(target) != ERROR_OK) return ERROR_FAIL; int error = riscv_program_exec(&prog, target); - riscv_invalidate_register_cache(target); + riscv_reg_cache_invalidate_all(target); if (error != ERROR_OK) { LOG_TARGET_ERROR(target, "exec_progbuf: Program buffer execution failed."); @@ -5731,8 +5755,6 @@ static int riscv_resume_go_all_harts(struct target *target) } else { LOG_TARGET_DEBUG(target, "Hart requested resume, but was already resumed."); } - - riscv_invalidate_register_cache(target); return ERROR_OK; } @@ -5826,17 +5848,6 @@ unsigned int riscv_vlenb(const struct target *target) return r->vlenb; } -static void riscv_invalidate_register_cache(struct target *target) -{ - /* Do not invalidate the register cache if it is not yet set up - * (e.g. when the target failed to get examined). */ - if (!target->reg_cache) - return; - - LOG_TARGET_DEBUG(target, "Invalidating register cache."); - register_cache_invalidate(target->reg_cache); -} - int riscv_get_hart_state(struct target *target, enum riscv_hart_state *state) { RISCV_INFO(r); diff --git a/src/target/riscv/riscv_reg.c b/src/target/riscv/riscv_reg.c index 4f386f47c..29d8a2b65 100644 --- a/src/target/riscv/riscv_reg.c +++ b/src/target/riscv/riscv_reg.c @@ -888,6 +888,34 @@ static int riscv_set_or_write_register(struct target *target, return ERROR_OK; } +bool riscv_reg_cache_any_dirty(const struct target *target, int log_level) +{ + bool any_dirty = false; + + if (!target->reg_cache) + return any_dirty; + + for (unsigned int number = 0; number < target->reg_cache->num_regs; ++number) { + const struct reg * const reg = riscv_reg_impl_cache_entry(target, number); + assert(riscv_reg_impl_is_initialized(reg)); + if (reg->dirty) { + log_printf_lf(log_level, __FILE__, __LINE__, __func__, + "[%s] Register %s is dirty!", target_name(target), reg->name); + any_dirty = true; + } + } + return any_dirty; +} + +void riscv_reg_cache_invalidate_all(struct target *target) +{ + if (!target->reg_cache) + return; + + LOG_TARGET_DEBUG(target, "Invalidating register cache."); + register_cache_invalidate(target->reg_cache); +} + /** * 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. diff --git a/src/target/riscv/riscv_reg.h b/src/target/riscv/riscv_reg.h index 264addd0b..dc87c559b 100644 --- a/src/target/riscv/riscv_reg.h +++ b/src/target/riscv/riscv_reg.h @@ -21,6 +21,17 @@ void riscv_reg_free_all(struct target *target); /** Write all dirty registers to the target. */ int riscv_reg_flush_all(struct target *target); +/** + * Check whether there are any dirty registers in the OpenOCD's register cache. + * In addition, all dirty registers will be reported to the log using the + * supplied "log_level". + */ +bool riscv_reg_cache_any_dirty(const struct target *target, int log_level); +/** + * Invalidate all registers - forget their cached register values. + * WARNING: If a register was dirty, its walue will be silently lost! + */ +void riscv_reg_cache_invalidate_all(struct target *target); /** * Set the register value. For cacheable registers, only the cache is updated * (write-back mode).