Skip to content

Commit 2ee531a

Browse files
committed
Fix data types around batch.{c,h}
Make corrections of data types in batch.{c,h} and in related code. Some of the issues were found by activating "-Wconversion" in GCC, others by inspecting the code manually. This is an intial step towards being able to use "-Wconversion" on RISC-V target code, which will give us bit more confidence when refactoring or merging new patches. Changes made: - The value passed to jtag_add_runtest() is now `unsigned int`, not `int`. No need for `assert(idle <= INT_MAX)` anymore. - `get_delay()` can return an unsigned value. - DMI address is no larger than 32-bits per the debug spec. Changed address parameters of multiple functions from uint64_t to uint32_t. - Added few assertions around `field->num_bits` in batch.c. Signed-off-by: Jan Matyas <[email protected]>
1 parent f82c5a7 commit 2ee531a

File tree

5 files changed

+51
-40
lines changed

5 files changed

+51
-40
lines changed

src/target/riscv/batch.c

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,13 @@
1010
#include "riscv.h"
1111
#include "field_helpers.h"
1212

13+
#define DTM_DMI_MIN_ADDRESS_LENGTH 7
14+
// TODO: DTM_DMI_MAX_ADDRESS_LENGTH should be reduced to 32 (per the debug spec)
1315
#define DTM_DMI_MAX_ADDRESS_LENGTH ((1<<DTM_DTMCS_ABITS_LENGTH)-1)
16+
17+
#define DMI_SCAN_MIN_BIT_LENGTH (DTM_DMI_MIN_ADDRESS_LENGTH + DTM_DMI_DATA_LENGTH + DTM_DMI_OP_LENGTH)
1418
#define DMI_SCAN_MAX_BIT_LENGTH (DTM_DMI_MAX_ADDRESS_LENGTH + DTM_DMI_DATA_LENGTH + DTM_DMI_OP_LENGTH)
19+
1520
#define DMI_SCAN_BUF_SIZE (DIV_ROUND_UP(DMI_SCAN_MAX_BIT_LENGTH, 8))
1621

1722
/* Reserve extra room in the batch (needed for the last NOP operation) */
@@ -127,11 +132,10 @@ static void add_idle_before_batch(const struct riscv_batch *batch, size_t start_
127132
const unsigned int idle_change = new_delay - batch->last_scan_delay;
128133
LOG_TARGET_DEBUG(batch->target, "Adding %u idle cycles before the batch.",
129134
idle_change);
130-
assert(idle_change <= INT_MAX);
131135
jtag_add_runtest(idle_change, TAP_IDLE);
132136
}
133137

134-
static int get_delay(const struct riscv_batch *batch, size_t scan_idx,
138+
static unsigned int get_delay(const struct riscv_batch *batch, size_t scan_idx,
135139
const struct riscv_scan_delays *delays, bool resets_delays,
136140
size_t reset_delays_after)
137141
{
@@ -142,7 +146,6 @@ static int get_delay(const struct riscv_batch *batch, size_t scan_idx,
142146
const enum riscv_scan_delay_class delay_class =
143147
batch->delay_classes[scan_idx];
144148
const unsigned int delay = riscv_scan_get_delay(delays, delay_class);
145-
assert(delay <= INT_MAX);
146149
return delays_were_reset ? 0 : delay;
147150
}
148151

@@ -199,9 +202,11 @@ static void log_batch(const struct riscv_batch *batch, size_t start_idx,
199202
return;
200203

201204
const unsigned int scan_bits = batch->fields->num_bits;
202-
assert(scan_bits == (unsigned int)riscv_get_dmi_scan_length(batch->target));
203-
const unsigned int abits = scan_bits - DTM_DMI_OP_LENGTH
204-
- DTM_DMI_DATA_LENGTH;
205+
assert(scan_bits == riscv_get_dmi_scan_length(batch->target));
206+
assert(scan_bits >= DMI_SCAN_MIN_BIT_LENGTH);
207+
assert(scan_bits <= DMI_SCAN_MAX_BIT_LENGTH);
208+
const unsigned int abits = (unsigned int)(scan_bits - DTM_DMI_OP_LENGTH
209+
- DTM_DMI_DATA_LENGTH);
205210

206211
/* Determine the "op" and "address" of the scan that preceded the first
207212
* executed scan.
@@ -211,7 +216,7 @@ static void log_batch(const struct riscv_batch *batch, size_t start_idx,
211216
* would be a more robust solution.
212217
*/
213218
bool last_scan_was_read = false;
214-
uint32_t last_scan_address = -1 /* to silence maybe-uninitialized */;
219+
uint32_t last_scan_address = (uint32_t)(-1) /* to silence maybe-uninitialized */;
215220
if (start_idx > 0) {
216221
const struct scan_field * const field = &batch->fields[start_idx - 1];
217222
assert(field->out_value);
@@ -224,7 +229,7 @@ static void log_batch(const struct riscv_batch *batch, size_t start_idx,
224229
/* Decode and log every executed scan */
225230
for (size_t i = start_idx; i < batch->used_scans; ++i) {
226231
static const char * const op_string[] = {"-", "r", "w", "?"};
227-
const int delay = get_delay(batch, i, delays, resets_delays,
232+
const unsigned int delay = get_delay(batch, i, delays, resets_delays,
228233
reset_delays_after);
229234
const struct scan_field * const field = &batch->fields[i];
230235

@@ -247,15 +252,15 @@ static void log_batch(const struct riscv_batch *batch, size_t start_idx,
247252
DTM_DMI_ADDRESS_OFFSET, abits);
248253

249254
LOG_DEBUG("%db %s %08" PRIx32 " @%02" PRIx32
250-
" -> %s %08" PRIx32 " @%02" PRIx32 "; %di",
255+
" -> %s %08" PRIx32 " @%02" PRIx32 "; %ui",
251256
field->num_bits, op_string[out_op], out_data, out_address,
252257
status_string[in_op], in_data, in_address, delay);
253258

254259
if (last_scan_was_read && in_op == DTM_DMI_OP_SUCCESS)
255260
log_dmi_decoded(batch, /*write*/ false,
256261
last_scan_address, in_data);
257262
} else {
258-
LOG_DEBUG("%db %s %08" PRIx32 " @%02" PRIx32 " -> ?; %di",
263+
LOG_DEBUG("%db %s %08" PRIx32 " @%02" PRIx32 " -> ?; %ui",
259264
field->num_bits, op_string[out_op], out_data, out_address,
260265
delay);
261266
}
@@ -321,12 +326,14 @@ int riscv_batch_run_from(struct riscv_batch *batch, size_t start_idx,
321326
return ERROR_OK;
322327
}
323328

324-
void riscv_batch_add_dmi_write(struct riscv_batch *batch, uint64_t address, uint32_t data,
329+
void riscv_batch_add_dmi_write(struct riscv_batch *batch, uint32_t address, uint32_t data,
325330
bool read_back, enum riscv_scan_delay_class delay_class)
326331
{
327332
assert(batch->used_scans < batch->allocated_scans);
328333
struct scan_field *field = batch->fields + batch->used_scans;
329334
field->num_bits = riscv_get_dmi_scan_length(batch->target);
335+
assert(field->num_bits >= DMI_SCAN_MIN_BIT_LENGTH);
336+
assert(field->num_bits <= DMI_SCAN_MAX_BIT_LENGTH);
330337
field->out_value = (void *)(batch->data_out + batch->used_scans * DMI_SCAN_BUF_SIZE);
331338
riscv_fill_dmi_write(batch->target, (char *)field->out_value, address, data);
332339
if (read_back) {
@@ -340,12 +347,14 @@ void riscv_batch_add_dmi_write(struct riscv_batch *batch, uint64_t address, uint
340347
batch->used_scans++;
341348
}
342349

343-
size_t riscv_batch_add_dmi_read(struct riscv_batch *batch, uint64_t address,
350+
size_t riscv_batch_add_dmi_read(struct riscv_batch *batch, uint32_t address,
344351
enum riscv_scan_delay_class delay_class)
345352
{
346353
assert(batch->used_scans < batch->allocated_scans);
347354
struct scan_field *field = batch->fields + batch->used_scans;
348355
field->num_bits = riscv_get_dmi_scan_length(batch->target);
356+
assert(field->num_bits >= DMI_SCAN_MIN_BIT_LENGTH);
357+
assert(field->num_bits <= DMI_SCAN_MAX_BIT_LENGTH);
349358
field->out_value = (void *)(batch->data_out + batch->used_scans * DMI_SCAN_BUF_SIZE);
350359
field->in_value = (void *)(batch->data_in + batch->used_scans * DMI_SCAN_BUF_SIZE);
351360
riscv_fill_dmi_read(batch->target, (char *)field->out_value, address);
@@ -383,6 +392,8 @@ void riscv_batch_add_nop(struct riscv_batch *batch)
383392
assert(batch->used_scans < batch->allocated_scans);
384393
struct scan_field *field = batch->fields + batch->used_scans;
385394
field->num_bits = riscv_get_dmi_scan_length(batch->target);
395+
assert(field->num_bits >= DMI_SCAN_MIN_BIT_LENGTH);
396+
assert(field->num_bits <= DMI_SCAN_MAX_BIT_LENGTH);
386397
field->out_value = (void *)(batch->data_out + batch->used_scans * DMI_SCAN_BUF_SIZE);
387398
field->in_value = (void *)(batch->data_in + batch->used_scans * DMI_SCAN_BUF_SIZE);
388399
riscv_fill_dm_nop(batch->target, (char *)field->out_value);

src/target/riscv/batch.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,11 +190,11 @@ int riscv_batch_run_from(struct riscv_batch *batch, size_t start_idx,
190190
size_t riscv_batch_finished_scans(const struct riscv_batch *batch);
191191

192192
/* Adds a DM register write to this batch. */
193-
void riscv_batch_add_dmi_write(struct riscv_batch *batch, uint64_t address, uint32_t data,
193+
void riscv_batch_add_dmi_write(struct riscv_batch *batch, uint32_t address, uint32_t data,
194194
bool read_back, enum riscv_scan_delay_class delay_class);
195195

196196
static inline void
197-
riscv_batch_add_dm_write(struct riscv_batch *batch, uint64_t address, uint32_t data,
197+
riscv_batch_add_dm_write(struct riscv_batch *batch, uint32_t address, uint32_t data,
198198
bool read_back, enum riscv_scan_delay_class delay_type)
199199
{
200200
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
205205
/* DM register reads must be handled in two parts: the first one schedules a read and
206206
* provides a key, the second one actually obtains the result of the read -
207207
* status (op) and the actual data. */
208-
size_t riscv_batch_add_dmi_read(struct riscv_batch *batch, uint64_t address,
208+
size_t riscv_batch_add_dmi_read(struct riscv_batch *batch, uint32_t address,
209209
enum riscv_scan_delay_class delay_class);
210210

211211
static inline size_t
212-
riscv_batch_add_dm_read(struct riscv_batch *batch, uint64_t address,
212+
riscv_batch_add_dm_read(struct riscv_batch *batch, uint32_t address,
213213
enum riscv_scan_delay_class delay_type)
214214
{
215215
return riscv_batch_add_dmi_read(batch,

src/target/riscv/riscv-013.c

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ static riscv_insn_t riscv013_read_progbuf(struct target *target, unsigned int
5656
index);
5757
static int riscv013_invalidate_cached_progbuf(struct target *target);
5858
static int riscv013_execute_progbuf(struct target *target, uint32_t *cmderr);
59-
static void riscv013_fill_dmi_write(struct target *target, char *buf, uint64_t a, uint32_t d);
60-
static void riscv013_fill_dmi_read(struct target *target, char *buf, uint64_t a);
61-
static int riscv013_get_dmi_scan_length(struct target *target);
59+
static void riscv013_fill_dmi_write(struct target *target, char *buf, uint32_t a, uint32_t d);
60+
static void riscv013_fill_dmi_read(struct target *target, char *buf, uint32_t a);
61+
static unsigned int riscv013_get_dmi_scan_length(struct target *target);
6262
static void riscv013_fill_dm_nop(struct target *target, char *buf);
6363
static unsigned int register_size(struct target *target, enum gdb_regno number);
6464
static int register_read_direct(struct target *target, riscv_reg_t *value,
@@ -5390,31 +5390,31 @@ static int riscv013_execute_progbuf(struct target *target, uint32_t *cmderr)
53905390
return riscv013_execute_abstract_command(target, run_program, cmderr);
53915391
}
53925392

5393-
static void riscv013_fill_dmi_write(struct target *target, char *buf, uint64_t a, uint32_t d)
5393+
static void riscv013_fill_dmi_write(struct target *target, char *buf, uint32_t a, uint32_t d)
53945394
{
53955395
RISCV013_INFO(info);
5396-
buf_set_u64((unsigned char *)buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_WRITE);
5397-
buf_set_u64((unsigned char *)buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, d);
5398-
buf_set_u64((unsigned char *)buf, DTM_DMI_ADDRESS_OFFSET, info->abits, a);
5396+
buf_set_u32((unsigned char *)buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_WRITE);
5397+
buf_set_u32((unsigned char *)buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, d);
5398+
buf_set_u32((unsigned char *)buf, DTM_DMI_ADDRESS_OFFSET, info->abits, a);
53995399
}
54005400

5401-
static void riscv013_fill_dmi_read(struct target *target, char *buf, uint64_t a)
5401+
static void riscv013_fill_dmi_read(struct target *target, char *buf, uint32_t a)
54025402
{
54035403
RISCV013_INFO(info);
5404-
buf_set_u64((unsigned char *)buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_READ);
5405-
buf_set_u64((unsigned char *)buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, 0);
5406-
buf_set_u64((unsigned char *)buf, DTM_DMI_ADDRESS_OFFSET, info->abits, a);
5404+
buf_set_u32((unsigned char *)buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_READ);
5405+
buf_set_u32((unsigned char *)buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, 0);
5406+
buf_set_u32((unsigned char *)buf, DTM_DMI_ADDRESS_OFFSET, info->abits, a);
54075407
}
54085408

54095409
static void riscv013_fill_dm_nop(struct target *target, char *buf)
54105410
{
54115411
RISCV013_INFO(info);
5412-
buf_set_u64((unsigned char *)buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_NOP);
5413-
buf_set_u64((unsigned char *)buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, 0);
5414-
buf_set_u64((unsigned char *)buf, DTM_DMI_ADDRESS_OFFSET, info->abits, 0);
5412+
buf_set_u32((unsigned char *)buf, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH, DMI_OP_NOP);
5413+
buf_set_u32((unsigned char *)buf, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH, 0);
5414+
buf_set_u32((unsigned char *)buf, DTM_DMI_ADDRESS_OFFSET, info->abits, 0);
54155415
}
54165416

5417-
static int riscv013_get_dmi_scan_length(struct target *target)
5417+
static unsigned int riscv013_get_dmi_scan_length(struct target *target)
54185418
{
54195419
RISCV013_INFO(info);
54205420
return info->abits + DTM_DMI_DATA_LENGTH + DTM_DMI_OP_LENGTH;

src/target/riscv/riscv.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5890,13 +5890,13 @@ int riscv_execute_progbuf(struct target *target, uint32_t *cmderr)
58905890
return r->execute_progbuf(target, cmderr);
58915891
}
58925892

5893-
void riscv_fill_dmi_write(struct target *target, char *buf, uint64_t a, uint32_t d)
5893+
void riscv_fill_dmi_write(struct target *target, char *buf, uint32_t a, uint32_t d)
58945894
{
58955895
RISCV_INFO(r);
58965896
r->fill_dmi_write(target, buf, a, d);
58975897
}
58985898

5899-
void riscv_fill_dmi_read(struct target *target, char *buf, uint64_t a)
5899+
void riscv_fill_dmi_read(struct target *target, char *buf, uint32_t a)
59005900
{
59015901
RISCV_INFO(r);
59025902
r->fill_dmi_read(target, buf, a);
@@ -5908,7 +5908,7 @@ void riscv_fill_dm_nop(struct target *target, char *buf)
59085908
r->fill_dm_nop(target, buf);
59095909
}
59105910

5911-
int riscv_get_dmi_scan_length(struct target *target)
5911+
unsigned int riscv_get_dmi_scan_length(struct target *target)
59125912
{
59135913
RISCV_INFO(r);
59145914
return r->get_dmi_scan_length(target);

src/target/riscv/riscv.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,9 @@ struct riscv_info {
275275
riscv_insn_t (*read_progbuf)(struct target *target, unsigned int index);
276276
int (*execute_progbuf)(struct target *target, uint32_t *cmderr);
277277
int (*invalidate_cached_progbuf)(struct target *target);
278-
int (*get_dmi_scan_length)(struct target *target);
279-
void (*fill_dmi_write)(struct target *target, char *buf, uint64_t a, uint32_t d);
280-
void (*fill_dmi_read)(struct target *target, char *buf, uint64_t a);
278+
unsigned int (*get_dmi_scan_length)(struct target *target);
279+
void (*fill_dmi_write)(struct target *target, char *buf, uint32_t a, uint32_t d);
280+
void (*fill_dmi_read)(struct target *target, char *buf, uint32_t a);
281281
void (*fill_dm_nop)(struct target *target, char *buf);
282282

283283
int (*authdata_read)(struct target *target, uint32_t *value, unsigned int index);
@@ -463,9 +463,9 @@ int riscv_write_progbuf(struct target *target, int index, riscv_insn_t insn);
463463
int riscv_execute_progbuf(struct target *target, uint32_t *cmderr);
464464

465465
void riscv_fill_dm_nop(struct target *target, char *buf);
466-
void riscv_fill_dmi_write(struct target *target, char *buf, uint64_t a, uint32_t d);
467-
void riscv_fill_dmi_read(struct target *target, char *buf, uint64_t a);
468-
int riscv_get_dmi_scan_length(struct target *target);
466+
void riscv_fill_dmi_write(struct target *target, char *buf, uint32_t a, uint32_t d);
467+
void riscv_fill_dmi_read(struct target *target, char *buf, uint32_t a);
468+
unsigned int riscv_get_dmi_scan_length(struct target *target);
469469

470470
uint32_t riscv_get_dmi_address(const struct target *target, uint32_t dm_address);
471471

0 commit comments

Comments
 (0)