From: David B. <dbr...@us...> - 2010-04-04 10:15:35
|
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "Main OpenOCD repository". The branch, master has been updated via 876bf9bf4cd5fc2640d91811fb69c0d36f9e2c18 (commit) via 88fcb5a9ef971e54166de7cd16a3b0be20113b82 (commit) from d37a10da52bc8e6df4e3df2edd02ddbc29fe3bc4 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- commit 876bf9bf4cd5fc2640d91811fb69c0d36f9e2c18 Author: David Brownell <dbr...@us...> Date: Sun Apr 4 00:42:05 2010 -0700 target: are we running algorithm code? Fixing one bug can easily uncover another .... in this case, making sure that we properly invalidate some cached NOR state when resuming arbitrary target code turned up an issue when the code wasn't quite arbitrary (and we couldn't know that, but some parts of OpenOCD assumed the cache would not be invalidated. Specifically: some flash drivers (like CFI) update that state in loops with downloaded algorithms, thus invalidating the state as it's probed. + Add a new target state flag, to record whether the target is running downloaded algorithm code. + Use that flag to add a special case: "trust" downloaded algorithms not to corrupt that cached state, bypassing cache invalidation. Also update some of the documentation to stipulate that this flavor of trustworthiness is now *required* ... not just a fortuitous acident. Signed-off-by: David Brownell <dbr...@us...> diff --git a/src/target/target.c b/src/target/target.c index 75c41d3..286933f 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -478,9 +478,14 @@ int target_resume(struct target *target, int current, uint32_t address, int hand * themselves. We want flash drivers and infrastructure to * be able to rely on (non-invalidated) cached state. * + * For now we require that algorithms provided by OpenOCD are + * used only by code which properly maintains that cached state. + * state + * * REVISIT do the same for NAND ; maybe other flash flavors too... */ - nor_resume(target); + if (!target->running_alg) + nor_resume(target); return retval; } @@ -659,10 +664,12 @@ int target_run_algorithm(struct target *target, goto done; } + target->running_alg = true; retval = target->type->run_algorithm(target, num_mem_params, mem_params, num_reg_params, reg_param, entry_point, exit_point, timeout_ms, arch_info); + target->running_alg = false; done: return retval; diff --git a/src/target/target.h b/src/target/target.h index 7400b7e..562724b 100644 --- a/src/target/target.h +++ b/src/target/target.h @@ -117,6 +117,14 @@ struct target */ bool examined; + /** true iff the target is currently running a downloaded + * "algorithm" instetad of arbitrary user code. OpenOCD code + * invoking algorithms is trusted to maintain correctness of + * any cached state (e.g. for flash status), which arbitrary + * code will have no reason to know about. + */ + bool running_alg; + struct target_event_action *event_action; int reset_halt; /* attempt resetting the CPU into the halted mode? */ commit 88fcb5a9ef971e54166de7cd16a3b0be20113b82 Author: David Brownell <dbr...@us...> Date: Sun Apr 4 00:38:39 2010 -0700 simplify and unconfuse target_run_algorithm() For some reason there are *two* schemes for interposing logic into the run_algorithm() code path... One is a standard procedural wapper around the target method invocation. the other (superfluous) one hacked the method table by splicing a second procedural wrapper into the method table. Remove it: * Rename its slightly-more-featureful wrapper so it becomes the standard procedural wrapper, leaving its added logic (where it should have been in the first place. Also add a paranoia check, to report targets that don't support algorithms without traversing a NULL pointer, and tweak its code structure a bit so it's easier to modify. * Get rid of the superfluous/conusing method table hacks. This is a net simplification, making it simpler to analyse what's going on, and then interpose logic . ... by ensuring there's only one natural place for it to live. ------------ Signed-off-by: David Brownell <dbr...@us...> diff --git a/src/target/target.c b/src/target/target.c index 868241e..75c41d3 100644 --- a/src/target/target.c +++ b/src/target/target.c @@ -629,16 +629,46 @@ static int target_soft_reset_halt_imp(struct target *target) return target->type->soft_reset_halt_imp(target); } -static int target_run_algorithm_imp(struct target *target, int num_mem_params, struct mem_param *mem_params, int num_reg_params, struct reg_param *reg_param, uint32_t entry_point, uint32_t exit_point, int timeout_ms, void *arch_info) +/** + * Downloads a target-specific native code algorithm to the target, + * and executes it. * Note that some targets may need to set up, enable, + * and tear down a breakpoint (hard or * soft) to detect algorithm + * termination, while others may support lower overhead schemes where + * soft breakpoints embedded in the algorithm automatically terminate the + * algorithm. + * + * @param target used to run the algorithm + * @param arch_info target-specific description of the algorithm. + */ +int target_run_algorithm(struct target *target, + int num_mem_params, struct mem_param *mem_params, + int num_reg_params, struct reg_param *reg_param, + uint32_t entry_point, uint32_t exit_point, + int timeout_ms, void *arch_info) { + int retval = ERROR_FAIL; + if (!target_was_examined(target)) { LOG_ERROR("Target not examined yet"); - return ERROR_FAIL; + goto done; + } + if (target->type->run_algorithm) { + LOG_ERROR("Target type '%s' does not support %s", + target_type_name(target), __func__); + goto done; } - return target->type->run_algorithm_imp(target, num_mem_params, mem_params, num_reg_params, reg_param, entry_point, exit_point, timeout_ms, arch_info); + + retval = target->type->run_algorithm(target, + num_mem_params, mem_params, + num_reg_params, reg_param, + entry_point, exit_point, timeout_ms, arch_info); + +done: + return retval; } + int target_read_memory(struct target *target, uint32_t address, uint32_t size, uint32_t count, uint8_t *buffer) { @@ -711,17 +741,6 @@ int target_step(struct target *target, } -int target_run_algorithm(struct target *target, - int num_mem_params, struct mem_param *mem_params, - int num_reg_params, struct reg_param *reg_param, - uint32_t entry_point, uint32_t exit_point, - int timeout_ms, void *arch_info) -{ - return target->type->run_algorithm(target, - num_mem_params, mem_params, num_reg_params, reg_param, - entry_point, exit_point, timeout_ms, arch_info); -} - /** * Reset the @c examined flag for the given target. * Pure paranoia -- targets are zeroed on allocation. @@ -785,9 +804,6 @@ static int target_init_one(struct command_context *cmd_ctx, type->soft_reset_halt_imp = target->type->soft_reset_halt; type->soft_reset_halt = target_soft_reset_halt_imp; - type->run_algorithm_imp = target->type->run_algorithm; - type->run_algorithm = target_run_algorithm_imp; - /* Sanity-check MMU support ... stub in what we must, to help * implement it in stages, but warn if we need to do so. */ diff --git a/src/target/target_type.h b/src/target/target_type.h index 70eb962..d3db8b5 100644 --- a/src/target/target_type.h +++ b/src/target/target_type.h @@ -146,8 +146,6 @@ struct target_type */ int (*remove_watchpoint)(struct target *target, struct watchpoint *watchpoint); - /* target algorithm support */ - int (*run_algorithm_imp)(struct target *target, int num_mem_params, struct mem_param *mem_params, int num_reg_params, struct reg_param *reg_param, uint32_t entry_point, uint32_t exit_point, int timeout_ms, void *arch_info); /** * Target algorithm support. Do @b not call this method directly, * use target_run_algorithm() instead. ----------------------------------------------------------------------- Summary of changes: src/target/target.c | 59 ++++++++++++++++++++++++++++++++-------------- src/target/target.h | 8 ++++++ src/target/target_type.h | 2 - 3 files changed, 49 insertions(+), 20 deletions(-) hooks/post-receive -- Main OpenOCD repository |