Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

target/riscv: clean-up register invalidation #1181

Merged
merged 1 commit into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/target/riscv/riscv-011.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
9 changes: 9 additions & 0 deletions src/target/riscv/riscv-013.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -5435,6 +5442,8 @@ static int riscv013_step_or_resume_current_hart(struct target *target,
if (riscv_reg_flush_all(target) != ERROR_OK)
JanMatCodasip marked this conversation as resolved.
Show resolved Hide resolved
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;
Expand Down
51 changes: 31 additions & 20 deletions src/target/riscv/riscv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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,
JanMatCodasip marked this conversation as resolved.
Show resolved Hide resolved
"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;
Expand Down Expand Up @@ -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.");
JanMatCodasip marked this conversation as resolved.
Show resolved Hide resolved
}

riscv_reg_cache_invalidate_all(target);

if (info->isrmask_mode == RISCV_ISRMASK_STEPONLY)
if (riscv_interrupts_restore(target, current_mstatus) != ERROR_OK) {
Expand Down Expand Up @@ -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.");
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down
28 changes: 28 additions & 0 deletions src/target/riscv/riscv_reg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions src/target/riscv/riscv_reg.h
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
Loading