This is an automated email from Gerrit. "Evgeniy Naydanov <[email protected]>" just uploaded a new patch set to Gerrit, which you can find at https://review.openocd.org/c/openocd/+/9668
-- gerrit commit 756b8d7e838111d62413f602876ec7dc397018f2 Author: Evgeniy Naydanov <[email protected]> Date: Thu Mar 12 16:39:08 2026 +0300 target/riscv: DTM-version-specific examine The patch build the framework for eliminating the "fake" `target_type`s ("riscv011_target" and "riscv013_target") by replacing them with `riscv_version_specific_type`. Applying this approach to `target_type`s `init`, `deinit` and `examine` allows to eliminate the extra read of `dtmcs` (`dtmcontrol` on 0.11) and drops the `cmd_ctx` from `riscv_info`, since the signature of the methods can be changed. Change-Id: If0d861c050e9bf9487318e8d23ecaf3e42cd7b56 Signed-off-by: Evgeniy Naydanov <[email protected]> diff --git a/src/target/riscv/riscv-011.c b/src/target/riscv/riscv-011.c index 8fd3792bf1..bb623e830d 100644 --- a/src/target/riscv/riscv-011.c +++ b/src/target/riscv/riscv-011.c @@ -1430,9 +1430,9 @@ static int halt(struct target *target) return ERROR_OK; } -static void deinit_target(struct target *target) +static void riscv011_deinit_version_specific(struct target *target) { - LOG_DEBUG("riscv_deinit_target()"); + LOG_LARGET_DEBUG(target, ""); struct riscv_info *info = target->arch_info; if (!info) return; @@ -1490,26 +1490,14 @@ static int step(struct target *target, bool current, target_addr_t address, return ERROR_OK; } -static int examine(struct target *target) +static int riscv011_examine_impl(struct target *target, uint32_t dtmcontrol) { - /* Don't need to select dbus, since the first thing we do is read dtmcontrol. */ - uint32_t dtmcontrol; - if (dtmcs_scan(target->tap, 0, &dtmcontrol) != ERROR_OK || dtmcontrol == 0) { - LOG_ERROR("Could not scan dtmcontrol. Check JTAG connectivity/board power."); - return ERROR_FAIL; - } - + assert(get_field32(dtmcontrol, DTMCONTROL_VERSION) == 0); LOG_DEBUG("dtmcontrol=0x%x", dtmcontrol); LOG_DEBUG(" addrbits=%d", get_field32(dtmcontrol, DTMCONTROL_ADDRBITS)); LOG_DEBUG(" version=%d", get_field32(dtmcontrol, DTMCONTROL_VERSION)); LOG_DEBUG(" idle=%d", get_field32(dtmcontrol, DTMCONTROL_IDLE)); - if (get_field(dtmcontrol, DTMCONTROL_VERSION) != 0) { - LOG_ERROR("Unsupported DTM version %d. (dtmcontrol=0x%x)", - get_field32(dtmcontrol, DTMCONTROL_VERSION), dtmcontrol); - return ERROR_FAIL; - } - RISCV_INFO(r); riscv011_info_t *info = get_info(target); @@ -2428,8 +2416,7 @@ static unsigned int riscv011_get_progbufsize(const struct target *target) return 0; } -static int init_target(struct command_context *cmd_ctx, - struct target *target) +static int riscv011_init_version_specific(struct target *target) { LOG_DEBUG("init"); RISCV_INFO(generic_info); @@ -2453,11 +2440,6 @@ static int init_target(struct command_context *cmd_ctx, struct target_type riscv011_target = { .name = "riscv", - - .init_target = init_target, - .deinit_target = deinit_target, - .examine = examine, - /* poll current target status */ .poll = riscv011_poll, @@ -2468,3 +2450,9 @@ struct target_type riscv011_target = { .assert_reset = assert_reset, .deassert_reset = deassert_reset, }; + +const struct riscv_version_specific_type riscv_dtm_v0_11_type = { + .init = riscv011_init_version_specific, + .deinit = riscv011_deinit_version_specific, + .examine_impl = riscv011_examine_impl, +}; diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index 1b47d5e77f..fa30fe2172 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -255,7 +255,7 @@ typedef struct { * necessary. */ bool dcsr_ebreak_is_set; - /* This hart was placed into a halt group in examine(). */ + /* This hart was placed into a halt group during examination. */ bool haltgroup_supported; } riscv013_info_t; @@ -1336,7 +1336,7 @@ static int scratch_write64(struct target *target, scratch_mem_t *scratch, static unsigned int register_size(struct target *target, enum gdb_regno number) { /* If reg_cache hasn't been initialized yet, make a guess. We need this for - * when this function is called during examine(). */ + * when this function is called during examination. */ if (target->reg_cache) return target->reg_cache->reg_list[number].size; else @@ -1771,16 +1771,19 @@ static int halt_set_dcsr_ebreak(struct target *target) /*** OpenOCD target functions. ***/ -static void deinit_target(struct target *target) +static void riscv013_deinit_version_specific(struct target *target) { - LOG_TARGET_DEBUG(target, "Deinitializing target."); + LOG_TARGET_DEBUG(target, "Deinitializing verison-specific info"); + struct riscv_info *info = target->arch_info; if (!info) return; riscv013_info_t *vsinfo = info->version_specific; - if (vsinfo) - ac_cache_free(&vsinfo->ac_not_supported_cache); + if (!vsinfo) + return; + + ac_cache_free(&vsinfo->ac_not_supported_cache); riscv013_dm_free(target); @@ -1989,36 +1992,22 @@ static int examine_dm(struct target *target) return ERROR_OK; } -static int examine(struct target *target) +static int riscv013_examine_impl(struct target *target, uint32_t dtmcs) { + assert(get_field32(dtmcs, DTM_DTMCS_VERSION) == DTM_DTMCS_VERSION_1_0); + LOG_DEBUG_REG(target, DTM_DTMCS, dtmcs); + LOG_TARGET_DEBUG(target, "dbgbase=0x%x", target->dbgbase); + /* We reset target state in case if something goes wrong during examine: * DTM/DM scans could fail or hart may fail to halt. */ target->state = TARGET_UNKNOWN; target->debug_reason = DBG_REASON_UNDEFINED; - /* Don't need to select dbus, since the first thing we do is read dtmcontrol. */ - LOG_TARGET_DEBUG(target, "dbgbase=0x%x", target->dbgbase); - - uint32_t dtmcontrol; - if (dtmcs_scan(target->tap, 0, &dtmcontrol) != ERROR_OK || dtmcontrol == 0) { - LOG_TARGET_ERROR(target, "Could not scan dtmcontrol. Check JTAG connectivity/board power."); - return ERROR_FAIL; - } - - LOG_TARGET_DEBUG(target, "dtmcontrol=0x%x", dtmcontrol); - LOG_DEBUG_REG(target, DTM_DTMCS, dtmcontrol); - - if (get_field(dtmcontrol, DTM_DTMCS_VERSION) != 1) { - LOG_TARGET_ERROR(target, "Unsupported DTM version %" PRIu32 ". (dtmcontrol=0x%" PRIx32 ")", - get_field32(dtmcontrol, DTM_DTMCS_VERSION), dtmcontrol); - return ERROR_FAIL; - } - riscv013_info_t *info = get_info(target); info->index = target->coreid; - info->abits = get_field(dtmcontrol, DTM_DTMCS_ABITS); - info->dtmcs_idle = get_field(dtmcontrol, DTM_DTMCS_IDLE); + info->abits = get_field(dtmcs, DTM_DTMCS_ABITS); + info->dtmcs_idle = get_field(dtmcs, DTM_DTMCS_IDLE); if (info->abits > RISCV013_DTMCS_ABITS_MAX) { /* Max. address width given by the debug specification is exceeded */ @@ -2849,8 +2838,7 @@ static int tick(struct target *target) return ERROR_OK; } -static int init_target(struct command_context *cmd_ctx, - struct target *target) +static int riscv013_init_version_specific(struct target *target) { LOG_TARGET_DEBUG(target, "Init."); RISCV_INFO(generic_info); @@ -3024,6 +3012,12 @@ static int deassert_reset(struct target *target) return ERROR_OK; } +const struct riscv_version_specific_type riscv_dtm_v1_0_type = { + .init = riscv013_init_version_specific, + .deinit = riscv013_deinit_version_specific, + .examine_impl = riscv013_examine_impl, +}; + static int execute_autofence(struct target *target) { if (dm013_select_target(target) != ERROR_OK) @@ -5078,10 +5072,6 @@ static unsigned int riscv013_get_progbufsize(const struct target *target) struct target_type riscv013_target = { .name = "riscv", - .init_target = init_target, - .deinit_target = deinit_target, - .examine = examine, - .poll = &riscv_openocd_poll, .halt = &riscv_halt, .step = &riscv_openocd_step, diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 1e2b9caa35..41e5eac266 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -461,18 +461,16 @@ static struct target_type *get_target_type(struct target *target) } RISCV_INFO(info); - switch (info->dtm_version) { - case DTM_DTMCS_VERSION_0_11: - return &riscv011_target; - case DTM_DTMCS_VERSION_1_0: - return &riscv013_target; - default: - /* TODO: once we have proper support for non-examined targets - * we should have an assert here */ - LOG_TARGET_ERROR(target, "Unsupported DTM version: %d", - info->dtm_version); + if (!info->version_specific_type) { + LOG_TARGET_ERROR(target, "DTM version is not examined"); return NULL; } + if (info->version_specific_type == &riscv_dtm_v0_11_type) + return &riscv011_target; + if (info->version_specific_type == &riscv_dtm_v1_0_type) + return &riscv013_target; + assert(false); + return NULL; } static struct riscv_private_config *alloc_default_riscv_private_config(void) @@ -665,7 +663,6 @@ static int riscv_init_target(struct command_context *cmd_ctx, { LOG_TARGET_DEBUG(target, "riscv_init_target()"); RISCV_INFO(info); - info->cmd_ctx = cmd_ctx; info->reset_delays_wait = -1; select_dtmcontrol.num_bits = target->tap->ir_length; @@ -720,22 +717,21 @@ static void riscv_deinit_target(struct target *target) free(target->private_config); struct riscv_info *info = target->arch_info; - struct target_type *tt = get_target_type(target); - if (!tt) - LOG_TARGET_ERROR(target, "Could not identify target type."); if (riscv_reg_flush_all(target) != ERROR_OK) LOG_TARGET_ERROR(target, "Failed to flush registers. Ignoring this error."); - if (tt && info && info->version_specific) - tt->deinit_target(target); - riscv_reg_free_all(target); free_wp_triggers_cache(target); if (!info) return; + if (info->version_specific_type) { + assert(info->version_specific_type->deinit); + info->version_specific_type->deinit(target); + } + free(info->reserved_triggers); range_list_t *entry, *tmp; @@ -2469,6 +2465,39 @@ static int old_or_new_riscv_step(struct target *target, bool current, handle_breakpoints, true /* handle callbacks*/); } +static const struct riscv_version_specific_type *dtm_version_to_type(uint32_t dtm_version) +{ + switch (dtm_version) { + case DTM_DTMCS_VERSION_0_11: + return &riscv_dtm_v0_11_type; + case DTM_DTMCS_VERSION_1_0: + return &riscv_dtm_v1_0_type; + } + return NULL; +} + +static int init_dtm_version_specific_type(struct target *target, uint32_t dtm_version) +{ + LOG_TARGET_DEBUG(target, "DTM version=0x%" PRIu32, dtm_version); + struct riscv_info *r = riscv_info(target); + assert(!r->version_specific_type); + r->version_specific_type = dtm_version_to_type(dtm_version); + if (r->version_specific_type) + return ERROR_OK; + LOG_TARGET_ERROR(target, "Unsupported DTM version: %" PRIu32 ".", dtm_version); + return ERROR_FAIL; +} + +static int check_version_specific_type(const struct target *target, + uint32_t dtm_version) +{ + const struct riscv_info *r = riscv_info(target); + if (r->version_specific_type == dtm_version_to_type(dtm_version)) + return ERROR_OK; + LOG_TARGET_ERROR(target, "DTM version mismatch. Check target connection"); + return ERROR_FAIL; +} + static int riscv_examine(struct target *target) { LOG_TARGET_DEBUG(target, "Starting examination"); @@ -2477,44 +2506,29 @@ static int riscv_examine(struct target *target) return ERROR_OK; } - /* Don't need to select dbus, since the first thing we do is read dtmcontrol. */ - - RISCV_INFO(info); uint32_t dtmcontrol; if (dtmcs_scan(target->tap, 0, &dtmcontrol) != ERROR_OK || dtmcontrol == 0) { LOG_TARGET_ERROR(target, "Could not read dtmcontrol. Check JTAG connectivity/board power."); return ERROR_FAIL; } LOG_TARGET_DEBUG(target, "dtmcontrol=0x%" PRIx32, dtmcontrol); - uint32_t dtm_version = get_field(dtmcontrol, DTMCONTROL_VERSION); - LOG_TARGET_DEBUG(target, "version=0x%" PRIx32, dtm_version); - - struct target_type *tt; - if (info->dtm_version == DTM_DTMCS_VERSION_UNKNOWN) { - info->dtm_version = dtm_version; - tt = get_target_type(target); - if (!tt) { - info->dtm_version = DTM_DTMCS_VERSION_UNKNOWN; - return ERROR_FAIL; - } - int retval = tt->init_target(info->cmd_ctx, target); - if (retval != ERROR_OK) { - info->dtm_version = DTM_DTMCS_VERSION_UNKNOWN; - return retval; - } - } else { - if (info->dtm_version != dtm_version) { - // REVISIT: could we deinit_target, change version and init_target again? - LOG_TARGET_ERROR(target, "dtmcs.version changed to 0x%" PRIx32, dtm_version); - return ERROR_FAIL; - } - tt = get_target_type(target); - if (!tt) - return ERROR_FAIL; - } + const struct riscv_info *r = riscv_info(target); + assert(r); + uint32_t dtm_version = get_field32(dtmcontrol, DTMCONTROL_VERSION); + int res = r->version_specific_type ? + check_version_specific_type(target, dtm_version) : + init_dtm_version_specific_type(target, dtm_version); + if (res != ERROR_OK) + return res; + assert(r->version_specific_type); + assert(r->version_specific_type->init); + res = r->version_specific_type->init(target); + if (res != ERROR_OK) + return res; - return tt->examine(target); + assert(r->version_specific_type->examine_impl); + return r->version_specific_type->examine_impl(target, dtmcontrol); } static int oldriscv_poll(struct target *target) @@ -5438,7 +5452,7 @@ COMMAND_HANDLER(riscv_exec_progbuf) } RISCV_INFO(r); - if (r->dtm_version != DTM_DTMCS_VERSION_1_0) { + if (r->version_specific_type != &riscv_dtm_v1_0_type) { LOG_TARGET_ERROR(target, "exec_progbuf: Program buffer is " "only supported on v0.13 or v1.0 targets."); return ERROR_FAIL; @@ -5982,7 +5996,6 @@ static void riscv_info_init(struct target *target, struct riscv_info *r) r->common_magic = RISCV_COMMON_MAGIC; - r->dtm_version = DTM_DTMCS_VERSION_UNKNOWN; r->version_specific = NULL; memset(r->trigger_unique_id, 0xff, sizeof(r->trigger_unique_id)); diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 2a0a9b95f0..1651c5dd98 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -164,13 +164,19 @@ riscv_mem_access_is_write(const struct riscv_mem_access_args args) return !args.read_buffer && args.write_buffer; } +struct riscv_version_specific_type { + int (*init)(struct target *target); + void (*deinit)(struct target *target); + int (*examine_impl)(struct target *target, uint32_t dtmcs); +}; + +extern const struct riscv_version_specific_type riscv_dtm_v0_11_type; +extern const struct riscv_version_specific_type riscv_dtm_v1_0_type; struct riscv_info { unsigned int common_magic; - unsigned int dtm_version; - - struct command_context *cmd_ctx; + const struct riscv_version_specific_type *version_specific_type; void *version_specific; struct reg_name_table custom_register_names; diff --git a/src/target/riscv/riscv_reg.c b/src/target/riscv/riscv_reg.c index ba1bc2a858..059f4d79e2 100644 --- a/src/target/riscv/riscv_reg.c +++ b/src/target/riscv/riscv_reg.c @@ -816,7 +816,7 @@ static int riscv_set_or_write_register(struct target *target, { RISCV_INFO(r); assert(r); - if (r->dtm_version == DTM_DTMCS_VERSION_0_11) + if (r->version_specific_type == &riscv_dtm_v0_11_type) return riscv011_set_register(target, regid, value); keep_alive(); @@ -954,7 +954,7 @@ int riscv_reg_get(struct target *target, riscv_reg_t *value, { RISCV_INFO(r); assert(r); - if (r->dtm_version == DTM_DTMCS_VERSION_0_11) + if (r->version_specific_type == &riscv_dtm_v0_11_type) return riscv011_get_register(target, value, regid); keep_alive(); --
