diff --git a/src/target/riscv/batch.c b/src/target/riscv/batch.c index 4de81a0de..bffad15da 100644 --- a/src/target/riscv/batch.c +++ b/src/target/riscv/batch.c @@ -10,13 +10,24 @@ #include "riscv.h" #include "field_helpers.h" +// TODO: DTM_DMI_MAX_ADDRESS_LENGTH should be reduced to 32 (per the debug spec) #define DTM_DMI_MAX_ADDRESS_LENGTH ((1< 0); + assert(abits <= DTM_DMI_MAX_ADDRESS_LENGTH); + + return abits + DTM_DMI_DATA_LENGTH + DTM_DMI_OP_LENGTH; +} + struct riscv_batch *riscv_batch_alloc(struct target *target, size_t scans) { scans += BATCH_RESERVED_SCANS; @@ -127,11 +138,10 @@ static void add_idle_before_batch(const struct riscv_batch *batch, size_t start_ const unsigned int idle_change = new_delay - batch->last_scan_delay; LOG_TARGET_DEBUG(batch->target, "Adding %u idle cycles before the batch.", idle_change); - assert(idle_change <= INT_MAX); jtag_add_runtest(idle_change, TAP_IDLE); } -static int get_delay(const struct riscv_batch *batch, size_t scan_idx, +static unsigned int get_delay(const struct riscv_batch *batch, size_t scan_idx, const struct riscv_scan_delays *delays, bool resets_delays, size_t reset_delays_after) { @@ -142,7 +152,6 @@ static int get_delay(const struct riscv_batch *batch, size_t scan_idx, const enum riscv_scan_delay_class delay_class = batch->delay_classes[scan_idx]; const unsigned int delay = riscv_scan_get_delay(delays, delay_class); - assert(delay <= INT_MAX); return delays_were_reset ? 0 : delay; } @@ -198,10 +207,7 @@ static void log_batch(const struct riscv_batch *batch, size_t start_idx, if (debug_level < LOG_LVL_DEBUG) return; - const unsigned int scan_bits = batch->fields->num_bits; - assert(scan_bits == (unsigned int)riscv_get_dmi_scan_length(batch->target)); - const unsigned int abits = scan_bits - DTM_DMI_OP_LENGTH - - DTM_DMI_DATA_LENGTH; + const unsigned int abits = riscv_get_dmi_address_bits(batch->target); /* Determine the "op" and "address" of the scan that preceded the first * executed scan. @@ -211,7 +217,7 @@ static void log_batch(const struct riscv_batch *batch, size_t start_idx, * would be a more robust solution. */ bool last_scan_was_read = false; - uint32_t last_scan_address = -1 /* to silence maybe-uninitialized */; + uint32_t last_scan_address = (uint32_t)(-1) /* to silence maybe-uninitialized */; if (start_idx > 0) { const struct scan_field * const field = &batch->fields[start_idx - 1]; assert(field->out_value); @@ -224,7 +230,7 @@ static void log_batch(const struct riscv_batch *batch, size_t start_idx, /* Decode and log every executed scan */ for (size_t i = start_idx; i < batch->used_scans; ++i) { static const char * const op_string[] = {"-", "r", "w", "?"}; - const int delay = get_delay(batch, i, delays, resets_delays, + const unsigned int delay = get_delay(batch, i, delays, resets_delays, reset_delays_after); const struct scan_field * const field = &batch->fields[i]; @@ -247,7 +253,7 @@ static void log_batch(const struct riscv_batch *batch, size_t start_idx, DTM_DMI_ADDRESS_OFFSET, abits); LOG_DEBUG("%db %s %08" PRIx32 " @%02" PRIx32 - " -> %s %08" PRIx32 " @%02" PRIx32 "; %di", + " -> %s %08" PRIx32 " @%02" PRIx32 "; %ui", field->num_bits, op_string[out_op], out_data, out_address, status_string[in_op], in_data, in_address, delay); @@ -255,7 +261,7 @@ static void log_batch(const struct riscv_batch *batch, size_t start_idx, log_dmi_decoded(batch, /*write*/ false, last_scan_address, in_data); } else { - LOG_DEBUG("%db %s %08" PRIx32 " @%02" PRIx32 " -> ?; %di", + LOG_DEBUG("%db %s %08" PRIx32 " @%02" PRIx32 " -> ?; %ui", field->num_bits, op_string[out_op], out_data, out_address, delay); } @@ -321,35 +327,56 @@ int riscv_batch_run_from(struct riscv_batch *batch, size_t start_idx, return ERROR_OK; } -void riscv_batch_add_dmi_write(struct riscv_batch *batch, uint64_t address, uint32_t data, +void riscv_batch_add_dmi_write(struct riscv_batch *batch, uint32_t address, uint32_t data, bool read_back, enum riscv_scan_delay_class delay_class) { + // TODO: Check that the bit width of "address" is no more than dtmcs.abits, + // otherwise return an error (during batch creation or when the batch is executed). + assert(batch->used_scans < batch->allocated_scans); struct scan_field *field = batch->fields + batch->used_scans; - field->num_bits = riscv_get_dmi_scan_length(batch->target); - field->out_value = (void *)(batch->data_out + batch->used_scans * DMI_SCAN_BUF_SIZE); - riscv_fill_dmi_write(batch->target, (char *)field->out_value, address, data); + + field->num_bits = get_dmi_scan_length(batch->target); + assert(field->num_bits <= DMI_SCAN_MAX_BIT_LENGTH); + + uint8_t *out_value = batch->data_out + batch->used_scans * DMI_SCAN_BUF_SIZE; + uint8_t *in_value = batch->data_in + batch->used_scans * DMI_SCAN_BUF_SIZE; + + field->out_value = out_value; + riscv_fill_dmi_write(batch->target, out_value, address, data); + if (read_back) { - field->in_value = (void *)(batch->data_in + batch->used_scans * DMI_SCAN_BUF_SIZE); - riscv_fill_dm_nop(batch->target, (char *)field->in_value); + field->in_value = in_value; + riscv_fill_dm_nop(batch->target, in_value); } else { field->in_value = NULL; } + batch->delay_classes[batch->used_scans] = delay_class; batch->last_scan = RISCV_SCAN_TYPE_WRITE; batch->used_scans++; } -size_t riscv_batch_add_dmi_read(struct riscv_batch *batch, uint64_t address, +size_t riscv_batch_add_dmi_read(struct riscv_batch *batch, uint32_t address, enum riscv_scan_delay_class delay_class) { + // TODO: Check that the bit width of "address" is no more than dtmcs.abits, + // otherwise return an error (during batch creation or when the batch is executed). + assert(batch->used_scans < batch->allocated_scans); struct scan_field *field = batch->fields + batch->used_scans; - field->num_bits = riscv_get_dmi_scan_length(batch->target); - field->out_value = (void *)(batch->data_out + batch->used_scans * DMI_SCAN_BUF_SIZE); - field->in_value = (void *)(batch->data_in + batch->used_scans * DMI_SCAN_BUF_SIZE); - riscv_fill_dmi_read(batch->target, (char *)field->out_value, address); - riscv_fill_dm_nop(batch->target, (char *)field->in_value); + + field->num_bits = get_dmi_scan_length(batch->target); + assert(field->num_bits <= DMI_SCAN_MAX_BIT_LENGTH); + + uint8_t *out_value = batch->data_out + batch->used_scans * DMI_SCAN_BUF_SIZE; + uint8_t *in_value = batch->data_in + batch->used_scans * DMI_SCAN_BUF_SIZE; + + field->out_value = out_value; + field->in_value = in_value; + riscv_fill_dmi_read(batch->target, out_value, address); + riscv_fill_dm_nop(batch->target, in_value); + batch->delay_classes[batch->used_scans] = delay_class; batch->last_scan = RISCV_SCAN_TYPE_READ; batch->used_scans++; @@ -382,11 +409,18 @@ void riscv_batch_add_nop(struct riscv_batch *batch) { assert(batch->used_scans < batch->allocated_scans); struct scan_field *field = batch->fields + batch->used_scans; - field->num_bits = riscv_get_dmi_scan_length(batch->target); - field->out_value = (void *)(batch->data_out + batch->used_scans * DMI_SCAN_BUF_SIZE); - field->in_value = (void *)(batch->data_in + batch->used_scans * DMI_SCAN_BUF_SIZE); - riscv_fill_dm_nop(batch->target, (char *)field->out_value); - riscv_fill_dm_nop(batch->target, (char *)field->in_value); + + field->num_bits = get_dmi_scan_length(batch->target); + assert(field->num_bits <= DMI_SCAN_MAX_BIT_LENGTH); + + uint8_t *out_value = batch->data_out + batch->used_scans * DMI_SCAN_BUF_SIZE; + uint8_t *in_value = batch->data_in + batch->used_scans * DMI_SCAN_BUF_SIZE; + + field->out_value = out_value; + field->in_value = in_value; + riscv_fill_dm_nop(batch->target, out_value); + riscv_fill_dm_nop(batch->target, in_value); + /* DMI NOP never triggers any debug module operation, * so the shortest (base) delay can be used. */ batch->delay_classes[batch->used_scans] = RISCV_DELAY_BASE; diff --git a/src/target/riscv/batch.h b/src/target/riscv/batch.h index 67b7ecc9d..5d8b57234 100644 --- a/src/target/riscv/batch.h +++ b/src/target/riscv/batch.h @@ -190,11 +190,11 @@ int riscv_batch_run_from(struct riscv_batch *batch, size_t start_idx, size_t riscv_batch_finished_scans(const struct riscv_batch *batch); /* Adds a DM register write to this batch. */ -void riscv_batch_add_dmi_write(struct riscv_batch *batch, uint64_t address, uint32_t data, +void riscv_batch_add_dmi_write(struct riscv_batch *batch, uint32_t address, uint32_t data, bool read_back, enum riscv_scan_delay_class delay_class); static inline void -riscv_batch_add_dm_write(struct riscv_batch *batch, uint64_t address, uint32_t data, +riscv_batch_add_dm_write(struct riscv_batch *batch, uint32_t address, uint32_t data, bool read_back, enum riscv_scan_delay_class delay_type) { return riscv_batch_add_dmi_write(batch, @@ -205,11 +205,11 @@ riscv_batch_add_dm_write(struct riscv_batch *batch, uint64_t address, uint32_t d /* DM register reads must be handled in two parts: the first one schedules a read and * provides a key, the second one actually obtains the result of the read - * status (op) and the actual data. */ -size_t riscv_batch_add_dmi_read(struct riscv_batch *batch, uint64_t address, +size_t riscv_batch_add_dmi_read(struct riscv_batch *batch, uint32_t address, enum riscv_scan_delay_class delay_class); static inline size_t -riscv_batch_add_dm_read(struct riscv_batch *batch, uint64_t address, +riscv_batch_add_dm_read(struct riscv_batch *batch, uint32_t address, enum riscv_scan_delay_class delay_type) { return riscv_batch_add_dmi_read(batch, diff --git a/src/target/riscv/riscv-013.c b/src/target/riscv/riscv-013.c index c399de7e4..7052ce4be 100644 --- a/src/target/riscv/riscv-013.c +++ b/src/target/riscv/riscv-013.c @@ -56,10 +56,10 @@ static riscv_insn_t riscv013_read_progbuf(struct target *target, unsigned int index); static int riscv013_invalidate_cached_progbuf(struct target *target); static int riscv013_execute_progbuf(struct target *target, uint32_t *cmderr); -static void riscv013_fill_dmi_write(struct target *target, char *buf, uint64_t a, uint32_t d); -static void riscv013_fill_dmi_read(struct target *target, char *buf, uint64_t a); -static int riscv013_get_dmi_scan_length(struct target *target); -static void riscv013_fill_dm_nop(struct target *target, char *buf); +static void riscv013_fill_dmi_write(const struct target *target, uint8_t *buf, uint32_t a, uint32_t d); +static void riscv013_fill_dmi_read(const struct target *target, uint8_t *buf, uint32_t a); +static unsigned int riscv013_get_dmi_address_bits(const struct target *target); +static void riscv013_fill_dm_nop(const struct target *target, uint8_t *buf); static unsigned int register_size(struct target *target, enum gdb_regno number); static int register_read_direct(struct target *target, riscv_reg_t *value, enum gdb_regno number); @@ -1941,6 +1941,29 @@ static int examine(struct target *target) info->abits = get_field(dtmcontrol, DTM_DTMCS_ABITS); info->dtmcs_idle = get_field(dtmcontrol, DTM_DTMCS_IDLE); + if (info->abits > RISCV013_DTMCS_ABITS_MAX) { + /* Max. address width given by the debug specification is exceeded */ + LOG_TARGET_ERROR(target, "The target's debug bus (DMI) address width exceeds " + "the maximum:"); + LOG_TARGET_ERROR(target, " found dtmcs.abits = %d; maximum is abits = %d.", + info->abits, RISCV013_DTMCS_ABITS_MAX); + return ERROR_FAIL; + } + + if (info->abits < RISCV013_DTMCS_ABITS_MIN) { + /* The requirement for minimum DMI address width of 7 bits is part of + * the RISC-V Debug spec since Jan-20-2017 (commit 03df6ee7). However, + * implementations exist that implement narrower DMI address. For example + * Spike as of Q1/2025 uses dmi.abits = 6. + * + * For that reason, warn the user but continue. + */ + LOG_TARGET_WARNING(target, "The target's debug bus (DMI) address width is " + "lower than the minimum:"); + LOG_TARGET_WARNING(target, " found dtmcs.abits = %d; minimum is abits = %d.", + info->abits, RISCV013_DTMCS_ABITS_MIN); + } + if (!check_dbgbase_exists(target)) { LOG_TARGET_ERROR(target, "Could not find debug module with DMI base address (dbgbase) = 0x%x", target->dbgbase); return ERROR_FAIL; @@ -2771,7 +2794,7 @@ static int init_target(struct command_context *cmd_ctx, generic_info->fill_dmi_write = &riscv013_fill_dmi_write; generic_info->fill_dmi_read = &riscv013_fill_dmi_read; generic_info->fill_dm_nop = &riscv013_fill_dm_nop; - generic_info->get_dmi_scan_length = &riscv013_get_dmi_scan_length; + generic_info->get_dmi_address_bits = &riscv013_get_dmi_address_bits; generic_info->authdata_read = &riscv013_authdata_read; generic_info->authdata_write = &riscv013_authdata_write; generic_info->dmi_read = &dmi_read; @@ -5390,34 +5413,34 @@ static int riscv013_execute_progbuf(struct target *target, uint32_t *cmderr) return riscv013_execute_abstract_command(target, run_program, cmderr); } -static void riscv013_fill_dmi_write(struct target *target, char *buf, uint64_t a, uint32_t d) +static void riscv013_fill_dmi_write(const struct target *target, uint8_t *buf, uint32_t a, uint32_t d) { RISCV013_INFO(info); - buf_set_u64((unsigned char *)buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_WRITE); - buf_set_u64((unsigned char *)buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, d); - buf_set_u64((unsigned char *)buf, DTM_DMI_ADDRESS_OFFSET, info->abits, a); + buf_set_u32(buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_WRITE); + buf_set_u32(buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, d); + buf_set_u32(buf, DTM_DMI_ADDRESS_OFFSET, info->abits, a); } -static void riscv013_fill_dmi_read(struct target *target, char *buf, uint64_t a) +static void riscv013_fill_dmi_read(const struct target *target, uint8_t *buf, uint32_t a) { RISCV013_INFO(info); - buf_set_u64((unsigned char *)buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_READ); - buf_set_u64((unsigned char *)buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, 0); - buf_set_u64((unsigned char *)buf, DTM_DMI_ADDRESS_OFFSET, info->abits, a); + buf_set_u32(buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_READ); + buf_set_u32(buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, 0); + buf_set_u32(buf, DTM_DMI_ADDRESS_OFFSET, info->abits, a); } -static void riscv013_fill_dm_nop(struct target *target, char *buf) +static void riscv013_fill_dm_nop(const struct target *target, uint8_t *buf) { RISCV013_INFO(info); - buf_set_u64((unsigned char *)buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_NOP); - buf_set_u64((unsigned char *)buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, 0); - buf_set_u64((unsigned char *)buf, DTM_DMI_ADDRESS_OFFSET, info->abits, 0); + buf_set_u32(buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_NOP); + buf_set_u32(buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, 0); + buf_set_u32(buf, DTM_DMI_ADDRESS_OFFSET, info->abits, 0); } -static int riscv013_get_dmi_scan_length(struct target *target) +static unsigned int riscv013_get_dmi_address_bits(const struct target *target) { RISCV013_INFO(info); - return info->abits + DTM_DMI_DATA_LENGTH + DTM_DMI_OP_LENGTH; + return info->abits; } /* Helper Functions. */ diff --git a/src/target/riscv/riscv.c b/src/target/riscv/riscv.c index 0ca921657..2743eebe9 100644 --- a/src/target/riscv/riscv.c +++ b/src/target/riscv/riscv.c @@ -5890,28 +5890,28 @@ int riscv_execute_progbuf(struct target *target, uint32_t *cmderr) return r->execute_progbuf(target, cmderr); } -void riscv_fill_dmi_write(struct target *target, char *buf, uint64_t a, uint32_t d) +void riscv_fill_dmi_write(const struct target *target, uint8_t *buf, uint32_t a, uint32_t d) { RISCV_INFO(r); r->fill_dmi_write(target, buf, a, d); } -void riscv_fill_dmi_read(struct target *target, char *buf, uint64_t a) +void riscv_fill_dmi_read(const struct target *target, uint8_t *buf, uint32_t a) { RISCV_INFO(r); r->fill_dmi_read(target, buf, a); } -void riscv_fill_dm_nop(struct target *target, char *buf) +void riscv_fill_dm_nop(const struct target *target, uint8_t *buf) { RISCV_INFO(r); r->fill_dm_nop(target, buf); } -int riscv_get_dmi_scan_length(struct target *target) +unsigned int riscv_get_dmi_address_bits(const struct target *target) { RISCV_INFO(r); - return r->get_dmi_scan_length(target); + return r->get_dmi_address_bits(target); } static int check_if_trigger_exists(struct target *target, unsigned int index) diff --git a/src/target/riscv/riscv.h b/src/target/riscv/riscv.h index 482d0fd71..bc9f32f3a 100644 --- a/src/target/riscv/riscv.h +++ b/src/target/riscv/riscv.h @@ -125,6 +125,9 @@ typedef struct { #define DTM_DTMCS_VERSION_UNKNOWN ((unsigned int)-1) #define RISCV_TINFO_VERSION_UNKNOWN (-1) +#define RISCV013_DTMCS_ABITS_MIN 7 +#define RISCV013_DTMCS_ABITS_MAX 32 + struct reg_name_table { unsigned int num_entries; char **reg_names; @@ -275,10 +278,10 @@ struct riscv_info { riscv_insn_t (*read_progbuf)(struct target *target, unsigned int index); int (*execute_progbuf)(struct target *target, uint32_t *cmderr); int (*invalidate_cached_progbuf)(struct target *target); - int (*get_dmi_scan_length)(struct target *target); - void (*fill_dmi_write)(struct target *target, char *buf, uint64_t a, uint32_t d); - void (*fill_dmi_read)(struct target *target, char *buf, uint64_t a); - void (*fill_dm_nop)(struct target *target, char *buf); + unsigned int (*get_dmi_address_bits)(const struct target *target); + void (*fill_dmi_write)(const struct target *target, uint8_t *buf, uint32_t a, uint32_t d); + void (*fill_dmi_read)(const struct target *target, uint8_t *buf, uint32_t a); + void (*fill_dm_nop)(const struct target *target, uint8_t *buf); int (*authdata_read)(struct target *target, uint32_t *value, unsigned int index); int (*authdata_write)(struct target *target, uint32_t value, unsigned int index); @@ -462,10 +465,10 @@ riscv_insn_t riscv_read_progbuf(struct target *target, int index); int riscv_write_progbuf(struct target *target, int index, riscv_insn_t insn); int riscv_execute_progbuf(struct target *target, uint32_t *cmderr); -void riscv_fill_dm_nop(struct target *target, char *buf); -void riscv_fill_dmi_write(struct target *target, char *buf, uint64_t a, uint32_t d); -void riscv_fill_dmi_read(struct target *target, char *buf, uint64_t a); -int riscv_get_dmi_scan_length(struct target *target); +void riscv_fill_dm_nop(const struct target *target, uint8_t *buf); +void riscv_fill_dmi_write(const struct target *target, uint8_t *buf, uint32_t a, uint32_t d); +void riscv_fill_dmi_read(const struct target *target, uint8_t *buf, uint32_t a); +unsigned int riscv_get_dmi_address_bits(const struct target *target); uint32_t riscv_get_dmi_address(const struct target *target, uint32_t dm_address);