Fixes and cleanup in riscv batch and related functions

Fixes:

- Data types of address & data parameters in riscv_batch_add_*()
  and riscv*_fill_dm*() changed to uint64_t and uint32_t.

- Corrected the comparison in riscv_batch_full().

- Corrected assertions in riscv_batch_get_dmi_read_op()
  and riscv_batch_get_dmi_read_data().

Cleanup:

- Simplified calloc() fail handling in riscv_batch_alloc().

- Added explicit NULL assignments in riscv_batch_alloc()
  for clarity and readability. Don't rely on calloc().

- Removed suffix `_u64` from riscv_*_fill_dm*() since it
  does not have any meaning.

- Renamed *dmi_write_u64_bits() to *get_dmi_scan_length()
  which better describes its purpose.

Change-Id: Id70e5968528d64b2ee5476f1c00e08459a1e291d
Signed-off-by: Jan Matyas <jan.matyas@codasip.com>

src/target/riscv/batch.c

@@ -13,58 +13,62 @@
 #define DMI_SCAN_MAX_BIT_LENGTH (DTM_DMI_MAX_ADDRESS_LENGTH + DTM_DMI_DATA_LENGTH + DTM_DMI_OP_LENGTH)
 #define DMI_SCAN_BUF_SIZE (DIV_ROUND_UP(DMI_SCAN_MAX_BIT_LENGTH, 8))
 
+#define BATCH_RESERVED_SCANS 4
+
 static void dump_field(int idle, const struct scan_field *field);
 
 struct riscv_batch *riscv_batch_alloc(struct target *target, size_t scans, size_t idle)
 {
-	scans += 4;
+	scans += BATCH_RESERVED_SCANS;
 	struct riscv_batch *out = calloc(1, sizeof(*out));
-	if (!out)
-		goto error0;
+	if (!out) {
+		LOG_ERROR("Failed to allocate struct riscv_batch");
+		return NULL;
+	}
+
 	out->target = target;
 	out->allocated_scans = scans;
 	out->idle_count = idle;
-	out->data_out = malloc(sizeof(*out->data_out) * (scans) * DMI_SCAN_BUF_SIZE);
+	out->last_scan = RISCV_SCAN_TYPE_INVALID;
+
+	out->data_out = NULL;
+	out->data_in = NULL;
+	out->fields = NULL;
+	out->bscan_ctxt = NULL;
+	out->read_keys = NULL;
+
+	out->data_out = malloc(sizeof(*out->data_out) * scans * DMI_SCAN_BUF_SIZE);
 	if (!out->data_out) {
 		LOG_ERROR("Failed to allocate data_out in RISC-V batch.");
-		goto error1;
+		goto alloc_error;
 	};
-	out->data_in = malloc(sizeof(*out->data_in) * (scans) * DMI_SCAN_BUF_SIZE);
+	out->data_in = malloc(sizeof(*out->data_in) * scans * DMI_SCAN_BUF_SIZE);
 	if (!out->data_in) {
 		LOG_ERROR("Failed to allocate data_in in RISC-V batch.");
-		goto error2;
+		goto alloc_error;
 	}
-	out->fields = malloc(sizeof(*out->fields) * (scans));
+	out->fields = malloc(sizeof(*out->fields) * scans);
 	if (!out->fields) {
 		LOG_ERROR("Failed to allocate fields in RISC-V batch.");
-		goto error3;
+		goto alloc_error;
 	}
 	if (bscan_tunnel_ir_width != 0) {
-		out->bscan_ctxt = malloc(sizeof(*out->bscan_ctxt) * (scans));
+		out->bscan_ctxt = malloc(sizeof(*out->bscan_ctxt) * scans);
 		if (!out->bscan_ctxt) {
 			LOG_ERROR("Failed to allocate bscan_ctxt in RISC-V batch.");
-			goto error4;
+			goto alloc_error;
 		}
 	}
-	out->last_scan = RISCV_SCAN_TYPE_INVALID;
-	out->read_keys = malloc(sizeof(*out->read_keys) * (scans));
+	out->read_keys = malloc(sizeof(*out->read_keys) * scans);
 	if (!out->read_keys) {
 		LOG_ERROR("Failed to allocate read_keys in RISC-V batch.");
-		goto error5;
+		goto alloc_error;
 	}
+
 	return out;
 
-error5:
-	free(out->bscan_ctxt);
-error4:
-	free(out->fields);
-error3:
-	free(out->data_in);
-error2:
-	free(out->data_out);
-error1:
-	free(out);
-error0:
+alloc_error:
+	riscv_batch_free(out);
 	return NULL;
 }
 
@@ -80,7 +84,7 @@ void riscv_batch_free(struct riscv_batch *batch)
 
 bool riscv_batch_full(struct riscv_batch *batch)
 {
-	return batch->used_scans > (batch->allocated_scans - 4);
+	return batch->used_scans >= (batch->allocated_scans - BATCH_RESERVED_SCANS);
 }
 
 int riscv_batch_run(struct riscv_batch *batch)
@@ -125,33 +129,33 @@ int riscv_batch_run(struct riscv_batch *batch)
 	return ERROR_OK;
 }
 
-void riscv_batch_add_dm_write(struct riscv_batch *batch, unsigned int address, uint64_t data,
+void riscv_batch_add_dm_write(struct riscv_batch *batch, uint64_t address, uint32_t data,
 	bool read_back)
 {
 	assert(batch->used_scans < batch->allocated_scans);
 	struct scan_field *field = batch->fields + batch->used_scans;
-	field->num_bits = riscv_dmi_write_u64_bits(batch->target);
+	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_dm_write_u64(batch->target, (char *)field->out_value, address, data);
+	riscv_fill_dm_write(batch->target, (char *)field->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_u64(batch->target, (char *)field->in_value);
+		riscv_fill_dm_nop(batch->target, (char *)field->in_value);
 	} else {
 		field->in_value = NULL;
 	}
 	batch->last_scan = RISCV_SCAN_TYPE_WRITE;
 	batch->used_scans++;
 }
 
-size_t riscv_batch_add_dm_read(struct riscv_batch *batch, unsigned int address)
+size_t riscv_batch_add_dm_read(struct riscv_batch *batch, uint64_t address)
 {
 	assert(batch->used_scans < batch->allocated_scans);
 	struct scan_field *field = batch->fields + batch->used_scans;
-	field->num_bits = riscv_dmi_write_u64_bits(batch->target);
+	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_read_u64(batch->target, (char *)field->out_value, address);
-	riscv_fill_dm_nop_u64(batch->target, (char *)field->in_value);
+	riscv_fill_dm_read(batch->target, (char *)field->out_value, address);
+	riscv_fill_dm_nop(batch->target, (char *)field->in_value);
 	batch->last_scan = RISCV_SCAN_TYPE_READ;
 	batch->used_scans++;
 
@@ -163,17 +167,17 @@ unsigned int riscv_batch_get_dmi_read_op(const struct riscv_batch *batch, size_t
 {
 	assert(key < batch->read_keys_used);
 	size_t index = batch->read_keys[key];
-	assert(index <= batch->used_scans);
+	assert(index < batch->used_scans);
 	uint8_t *base = batch->data_in + DMI_SCAN_BUF_SIZE * index;
 	/* extract "op" field from the DMI read result */
-	return (unsigned)buf_get_u32(base, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH);
+	return (unsigned int)buf_get_u32(base, DTM_DMI_OP_OFFSET, DTM_DMI_OP_LENGTH);
 }
 
 uint32_t riscv_batch_get_dmi_read_data(const struct riscv_batch *batch, size_t key)
 {
 	assert(key < batch->read_keys_used);
 	size_t index = batch->read_keys[key];
-	assert(index <= batch->used_scans);
+	assert(index < batch->used_scans);
 	uint8_t *base = batch->data_in + DMI_SCAN_BUF_SIZE * index;
 	/* extract "data" field from the DMI read result */
 	return buf_get_u32(base, DTM_DMI_DATA_OFFSET, DTM_DMI_DATA_LENGTH);
@@ -183,11 +187,11 @@ 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_dmi_write_u64_bits(batch->target);
+	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_u64(batch->target, (char *)field->out_value);
-	riscv_fill_dm_nop_u64(batch->target, (char *)field->in_value);
+	riscv_fill_dm_nop(batch->target, (char *)field->out_value);
+	riscv_fill_dm_nop(batch->target, (char *)field->in_value);
 	batch->last_scan = RISCV_SCAN_TYPE_NOP;
 	batch->used_scans++;
 }
@@ -226,13 +230,16 @@ static void dump_field(int idle, const struct scan_field *field)
 
 size_t riscv_batch_available_scans(struct riscv_batch *batch)
 {
-	return batch->allocated_scans - batch->used_scans - 4;
+	assert(batch->allocated_scans >= (batch->used_scans + BATCH_RESERVED_SCANS));
+	return batch->allocated_scans - batch->used_scans - BATCH_RESERVED_SCANS;
 }
 
 bool riscv_batch_dmi_busy_encountered(const struct riscv_batch *batch)
 {
-	if (!batch->used_scans)
+	if (batch->used_scans == 0)
+		/* Empty batch */
 		return false;
+
 	assert(batch->last_scan == RISCV_SCAN_TYPE_NOP);
 	const struct scan_field *field = batch->fields + batch->used_scans - 1;
 	const uint64_t in = buf_get_u64(field->in_value, 0, field->num_bits);

src/target/riscv/batch.h

@@ -59,13 +59,13 @@ bool riscv_batch_full(struct riscv_batch *batch);
 int riscv_batch_run(struct riscv_batch *batch);
 
 /* Adds a DM register write to this batch. */
-void riscv_batch_add_dm_write(struct riscv_batch *batch, unsigned int address, uint64_t data,
+void riscv_batch_add_dm_write(struct riscv_batch *batch, uint64_t address, uint32_t data,
 	bool read_back);
 
 /* 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_dm_read(struct riscv_batch *batch, unsigned int address);
+size_t riscv_batch_add_dm_read(struct riscv_batch *batch, uint64_t address);
 unsigned int riscv_batch_get_dmi_read_op(const struct riscv_batch *batch, size_t key);
 uint32_t riscv_batch_get_dmi_read_data(const struct riscv_batch *batch, size_t key);
 

src/target/riscv/riscv-013.c

@@ -55,13 +55,13 @@ 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_u64(struct target *target, char *buf, int a, uint64_t d);
-static void riscv013_fill_dmi_read_u64(struct target *target, char *buf, int a);
-static int riscv013_dmi_write_u64_bits(struct target *target);
-static void riscv013_fill_dmi_nop_u64(struct target *target, char *buf);
-static void riscv013_fill_dm_write_u64(struct target *target, char *buf, int a, uint64_t d);
-static void riscv013_fill_dm_read_u64(struct target *target, char *buf, int a);
-static void riscv013_fill_dm_nop_u64(struct target *target, char *buf);
+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 void riscv013_fill_dmi_nop(struct target *target, char *buf);
+static int riscv013_get_dmi_scan_length(struct target *target);
+static void riscv013_fill_dm_write(struct target *target, char *buf, uint64_t a, uint32_t d);
+static void riscv013_fill_dm_read(struct target *target, char *buf, uint64_t a);
+static void riscv013_fill_dm_nop(struct target *target, char *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);
@@ -2767,10 +2767,10 @@ static int init_target(struct command_context *cmd_ctx,
 	generic_info->write_progbuf = &riscv013_write_progbuf;
 	generic_info->execute_progbuf = &riscv013_execute_progbuf;
 	generic_info->invalidate_cached_progbuf = &riscv013_invalidate_cached_progbuf;
-	generic_info->fill_dm_write_u64 = &riscv013_fill_dm_write_u64;
-	generic_info->fill_dm_read_u64 = &riscv013_fill_dm_read_u64;
-	generic_info->fill_dm_nop_u64 = &riscv013_fill_dm_nop_u64;
-	generic_info->dmi_write_u64_bits = &riscv013_dmi_write_u64_bits;
+	generic_info->fill_dm_write = &riscv013_fill_dm_write;
+	generic_info->fill_dm_read = &riscv013_fill_dm_read;
+	generic_info->fill_dm_nop = &riscv013_fill_dm_nop;
+	generic_info->get_dmi_scan_length = &riscv013_get_dmi_scan_length;
 	generic_info->authdata_read = &riscv013_authdata_read;
 	generic_info->authdata_write = &riscv013_authdata_write;
 	generic_info->dmi_read = &dmi_read;
@@ -5169,55 +5169,55 @@ static int riscv013_execute_progbuf(struct target *target, uint32_t *cmderr)
 	return execute_abstract_command(target, run_program, cmderr);
 }
 
-static void riscv013_fill_dmi_write_u64(struct target *target, char *buf, int a, uint64_t d)
+static void riscv013_fill_dmi_write(struct target *target, char *buf, uint64_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);
 }
 
-static void riscv013_fill_dmi_read_u64(struct target *target, char *buf, int a)
+static void riscv013_fill_dmi_read(struct target *target, char *buf, uint64_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);
 }
 
-static void riscv013_fill_dmi_nop_u64(struct target *target, char *buf)
+static void riscv013_fill_dmi_nop(struct target *target, char *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);
 }
 
-static int riscv013_dmi_write_u64_bits(struct target *target)
+static int riscv013_get_dmi_scan_length(struct target *target)
 {
 	RISCV013_INFO(info);
 	return info->abits + DTM_DMI_DATA_LENGTH + DTM_DMI_OP_LENGTH;
 }
 
-void riscv013_fill_dm_write_u64(struct target *target, char *buf, int a, uint64_t d)
+void riscv013_fill_dm_write(struct target *target, char *buf, uint64_t a, uint32_t d)
 {
 	dm013_info_t *dm = get_dm(target);
 	if (!dm)
 		return;
-	riscv013_fill_dmi_write_u64(target, buf, a + dm->base, d);
+	riscv013_fill_dmi_write(target, buf, a + dm->base, d);
 }
 
-void riscv013_fill_dm_read_u64(struct target *target, char *buf, int a)
+void riscv013_fill_dm_read(struct target *target, char *buf, uint64_t a)
 {
 	dm013_info_t *dm = get_dm(target);
 	if (!dm)
 		return;
-	riscv013_fill_dmi_read_u64(target, buf, a + dm->base);
+	riscv013_fill_dmi_read(target, buf, a + dm->base);
 }
 
-void riscv013_fill_dm_nop_u64(struct target *target, char *buf)
+void riscv013_fill_dm_nop(struct target *target, char *buf)
 {
-	riscv013_fill_dmi_nop_u64(target, buf);
+	riscv013_fill_dmi_nop(target, buf);
 }
 
 static int maybe_execute_fence_i(struct target *target)

src/target/riscv/riscv.c

@@ -5354,28 +5354,28 @@ int riscv_execute_progbuf(struct target *target, uint32_t *cmderr)
 	return r->execute_progbuf(target, cmderr);
 }
 
-void riscv_fill_dm_write_u64(struct target *target, char *buf, int a, uint64_t d)
+void riscv_fill_dm_write(struct target *target, char *buf, uint64_t a, uint32_t d)
 {
 	RISCV_INFO(r);
-	r->fill_dm_write_u64(target, buf, a, d);
+	r->fill_dm_write(target, buf, a, d);
 }
 
-void riscv_fill_dm_read_u64(struct target *target, char *buf, int a)
+void riscv_fill_dm_read(struct target *target, char *buf, uint64_t a)
 {
 	RISCV_INFO(r);
-	r->fill_dm_read_u64(target, buf, a);
+	r->fill_dm_read(target, buf, a);
 }
 
-void riscv_fill_dm_nop_u64(struct target *target, char *buf)
+void riscv_fill_dm_nop(struct target *target, char *buf)
 {
 	RISCV_INFO(r);
-	r->fill_dm_nop_u64(target, buf);
+	r->fill_dm_nop(target, buf);
 }
 
-int riscv_dmi_write_u64_bits(struct target *target)
+int riscv_get_dmi_scan_length(struct target *target)
 {
 	RISCV_INFO(r);
-	return r->dmi_write_u64_bits(target);
+	return r->get_dmi_scan_length(target);
 }
 
 static int check_if_trigger_exists(struct target *target, unsigned int index)

src/target/riscv/riscv.h

@@ -231,10 +231,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 (*dmi_write_u64_bits)(struct target *target);
-	void (*fill_dm_write_u64)(struct target *target, char *buf, int a, uint64_t d);
-	void (*fill_dm_read_u64)(struct target *target, char *buf, int a);
-	void (*fill_dm_nop_u64)(struct target *target, char *buf);
+	int (*get_dmi_scan_length)(struct target *target);
+	void (*fill_dm_write)(struct target *target, char *buf, uint64_t a, uint32_t d);
+	void (*fill_dm_read)(struct target *target, char *buf, uint64_t a);
+	void (*fill_dm_nop)(struct target *target, char *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);
@@ -429,10 +429,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_u64(struct target *target, char *buf);
-void riscv_fill_dm_write_u64(struct target *target, char *buf, int a, uint64_t d);
-void riscv_fill_dm_read_u64(struct target *target, char *buf, int a);
-int riscv_dmi_write_u64_bits(struct target *target);
+void riscv_fill_dm_nop(struct target *target, char *buf);
+void riscv_fill_dm_write(struct target *target, char *buf, uint64_t a, uint32_t d);
+void riscv_fill_dm_read(struct target *target, char *buf, uint64_t a);
+int riscv_get_dmi_scan_length(struct target *target);
 
 int riscv_enumerate_triggers(struct target *target);