Merge pull request #1167 from fk-sc/fk-sc/rwargs

target/riscv: pass memory access info in struct, move write_memory pointer

src/target/riscv/riscv-011.c

@@ -1972,9 +1972,16 @@ static int deassert_reset(struct target *target)
 		return wait_for_state(target, TARGET_RUNNING);
 }
 
-static int read_memory(struct target *target, target_addr_t address,
+static int read_memory(struct target *target, const riscv_mem_access_args_t args)
-		uint32_t size, uint32_t count, uint8_t *buffer, uint32_t increment)
 {
+	assert(riscv_mem_access_is_read(args));
+
+	const target_addr_t address = args.address;
+	const uint32_t size = args.size;
+	const uint32_t count = args.count;
+	const uint32_t increment = args.increment;
+	uint8_t * const buffer = args.read_buffer;
+
 	if (increment != size) {
 		LOG_ERROR("read_memory with custom increment not implemented");
 		return ERROR_NOT_IMPLEMENTED;
@@ -2142,9 +2149,20 @@ static int setup_write_memory(struct target *target, uint32_t size)
 	return ERROR_OK;
 }
 
-static int write_memory(struct target *target, target_addr_t address,
+static int write_memory(struct target *target, const riscv_mem_access_args_t args)
-		uint32_t size, uint32_t count, const uint8_t *buffer)
 {
+	assert(riscv_mem_access_is_write(args));
+
+	if (args.increment != args.size) {
+		LOG_TARGET_ERROR(target, "Write increment size has to be equal to element size");
+		return ERROR_NOT_IMPLEMENTED;
+	}
+
+	const target_addr_t address = args.address;
+	const uint32_t size = args.size;
+	const uint32_t count = args.count;
+	const uint8_t * const buffer = args.write_buffer;
+
 	riscv011_info_t *info = get_info(target);
 	jtag_add_ir_scan(target->tap, &select_dbus, TAP_IDLE);
 
@@ -2371,7 +2389,9 @@ static int init_target(struct command_context *cmd_ctx,
 {
 	LOG_DEBUG("init");
 	RISCV_INFO(generic_info);
+	/* TODO: replace read and write with single access function*/
 	generic_info->read_memory = read_memory;
+	generic_info->write_memory = write_memory;
 	generic_info->authdata_read = &riscv011_authdata_read;
 	generic_info->authdata_write = &riscv011_authdata_write;
 	generic_info->print_info = &riscv011_print_info;
@@ -2406,7 +2426,5 @@ struct target_type riscv011_target = {
 	.assert_reset = assert_reset,
 	.deassert_reset = deassert_reset,
 
-	.write_memory = write_memory,
-
 	.arch_state = arch_state,
 };

src/target/riscv/riscv.c

@@ -2945,8 +2945,14 @@ static int riscv_address_translate(struct target *target,
 
 		uint8_t buffer[8];
 		assert(info->pte_shift <= 3);
-		int retval = r->read_memory(target, pte_address,
+		const riscv_mem_access_args_t args = {
-				4, (1 << info->pte_shift) / 4, buffer, 4);
+			.address = pte_address,
+			.read_buffer = buffer,
+			.size = 4,
+			.increment = 4,
+			.count = (1 << info->pte_shift) / 4,
+		};
+		int retval = r->read_memory(target, args);
 		if (retval != ERROR_OK)
 			return ERROR_FAIL;
 
@@ -3182,29 +3188,40 @@ static int check_virt_memory_access(struct target *target, target_addr_t address
 static int riscv_read_phys_memory(struct target *target, target_addr_t phys_address,
 			uint32_t size, uint32_t count, uint8_t *buffer)
 {
+	const riscv_mem_access_args_t args = {
+		.address = phys_address,
+		.read_buffer = buffer,
+		.size = size,
+		.count = count,
+		.increment = size,
+	};
 	RISCV_INFO(r);
-	return r->read_memory(target, phys_address, size, count, buffer, size);
+	return r->read_memory(target, args);
 }
 
 static int riscv_write_phys_memory(struct target *target, target_addr_t phys_address,
 			uint32_t size, uint32_t count, const uint8_t *buffer)
 {
-	struct target_type *tt = get_target_type(target);
+	const riscv_mem_access_args_t args = {
-	if (!tt)
+		.address = phys_address,
-		return ERROR_FAIL;
+		.write_buffer = buffer,
-	return tt->write_memory(target, phys_address, size, count, buffer);
+		.size = size,
+		.count = count,
+		.increment = size,
+	};
+
+	RISCV_INFO(r);
+	return r->write_memory(target, args);
 }
 
-static int riscv_rw_memory(struct target *target, target_addr_t address, uint32_t size,
+static int riscv_rw_memory(struct target *target, const riscv_mem_access_args_t args)
-		uint32_t count, uint8_t *read_buffer, const uint8_t *write_buffer)
 {
-	/* Exactly one of the buffers must be set, the other must be NULL */
+	assert(riscv_mem_access_is_valid(args));
-	assert(!!read_buffer != !!write_buffer);
 
-	const bool is_write = write_buffer ? true : false;
+	const bool is_write = riscv_mem_access_is_write(args);
-	if (count == 0) {
+	if (args.count == 0) {
 		LOG_TARGET_WARNING(target, "0-length %s 0x%" TARGET_PRIxADDR,
-				is_write ? "write to" : "read from", address);
+				is_write ? "write to" : "read from", args.address);
 		return ERROR_OK;
 	}
 
@@ -3214,25 +3231,23 @@ static int riscv_rw_memory(struct target *target, target_addr_t address, uint32_
 		return result;
 
 	RISCV_INFO(r);
-	struct target_type *tt = get_target_type(target);
-	if (!tt)
-		return ERROR_FAIL;
-
 	if (!mmu_enabled) {
 		if (is_write)
-			return tt->write_memory(target, address, size, count, write_buffer);
+			return r->write_memory(target, args);
 		else
-			return r->read_memory(target, address, size, count, read_buffer, size);
+			return r->read_memory(target, args);
 	}
 
-	result = check_virt_memory_access(target, address, size, count, is_write);
+	result = check_virt_memory_access(target, args.address,
+			args.size, args.count, is_write);
 	if (result != ERROR_OK)
 		return result;
 
 	uint32_t current_count = 0;
-	while (current_count < count) {
+	target_addr_t current_address = args.address;
+	while (current_count < args.count) {
 		target_addr_t physical_addr;
-		result = target->type->virt2phys(target, address, &physical_addr);
+		result = target->type->virt2phys(target, current_address, &physical_addr);
 		if (result != ERROR_OK) {
 			LOG_TARGET_ERROR(target, "Address translation failed.");
 			return result;
@@ -3241,16 +3256,25 @@ static int riscv_rw_memory(struct target *target, target_addr_t address, uint32_
 		/* TODO: For simplicity, this algorithm assumes the worst case - the smallest possible page size,
 		 * which is  4 KiB. The algorithm can be improved to detect the real page size, and allow to use larger
 		 * memory transfers and avoid extra unnecessary virt2phys address translations. */
-		uint32_t chunk_count = MIN(count - current_count, (RISCV_PGSIZE - RISCV_PGOFFSET(address)) / size);
+		uint32_t chunk_count = MIN(args.count - current_count,
-		if (is_write)
+				(RISCV_PGSIZE - RISCV_PGOFFSET(current_address))
-			result = tt->write_memory(target, physical_addr, size, chunk_count, write_buffer + current_count * size);
+				/ args.size);
-		else
+
-			result = r->read_memory(target, physical_addr, size, chunk_count, read_buffer + current_count * size, size);
+		riscv_mem_access_args_t current_access = args;
+		current_access.address = physical_addr;
+		current_access.count = chunk_count;
+		if (is_write) {
+			current_access.write_buffer += current_count * args.size;
+			result = r->write_memory(target, current_access);
+		} else {
+			current_access.read_buffer += current_count * args.size;
+			result = r->read_memory(target, current_access);
+		}
 		if (result != ERROR_OK)
 			return result;
 
 		current_count += chunk_count;
-		address += chunk_count * size;
+		current_address += chunk_count * args.size;
 	}
 	return ERROR_OK;
 }
@@ -3258,13 +3282,29 @@ static int riscv_rw_memory(struct target *target, target_addr_t address, uint32_
 static int riscv_read_memory(struct target *target, target_addr_t address,
 		uint32_t size, uint32_t count, uint8_t *buffer)
 {
-	return riscv_rw_memory(target, address, size, count, buffer, NULL);
+	const riscv_mem_access_args_t args = {
+		.address = address,
+		.read_buffer = buffer,
+		.size = size,
+		.count = count,
+		.increment = size,
+	};
+
+	return riscv_rw_memory(target, args);
 }
 
 static int riscv_write_memory(struct target *target, target_addr_t address,
 		uint32_t size, uint32_t count, const uint8_t *buffer)
 {
-	return riscv_rw_memory(target, address, size, count, NULL, buffer);
+	const riscv_mem_access_args_t args = {
+		.address = address,
+		.write_buffer = buffer,
+		.size = size,
+		.count = count,
+		.increment = size,
+	};
+
+	return riscv_rw_memory(target, args);
 }
 
 static const char *riscv_get_gdb_arch(const struct target *target)
@@ -4910,7 +4950,14 @@ COMMAND_HANDLER(handle_repeat_read)
 		LOG_ERROR("malloc failed");
 		return ERROR_FAIL;
 	}
-	int result = r->read_memory(target, address, size, count, buffer, 0);
+	const riscv_mem_access_args_t args = {
+		.address = address,
+		.read_buffer = buffer,
+		.size = size,
+		.count = count,
+		.increment = 0,
+	};
+	int result = r->read_memory(target, args);
 	if (result == ERROR_OK) {
 		target_handle_md_output(cmd, target, address, size, count, buffer,
 			false);

src/target/riscv/riscv.h

@@ -130,6 +130,38 @@ struct reg_name_table {
 	char **reg_names;
 };
 
+typedef struct riscv_mem_access_args {
+	target_addr_t address;
+
+	const uint8_t *write_buffer;
+	uint8_t *read_buffer;
+
+	uint32_t size;
+	uint32_t count;
+	uint32_t increment;
+} riscv_mem_access_args_t;
+
+static inline bool
+riscv_mem_access_is_valid(const riscv_mem_access_args_t args)
+{
+	return !args.read_buffer != !args.write_buffer;
+}
+
+static inline bool
+riscv_mem_access_is_read(const riscv_mem_access_args_t args)
+{
+	assert(riscv_mem_access_is_valid(args));
+	return !args.write_buffer && args.read_buffer;
+}
+
+static inline bool
+riscv_mem_access_is_write(const riscv_mem_access_args_t args)
+{
+	assert(riscv_mem_access_is_valid(args));
+	return !args.read_buffer && args.write_buffer;
+}
+
+
 struct riscv_info {
 	unsigned int common_magic;
 
@@ -268,8 +300,8 @@ struct riscv_info {
 						 riscv_sample_config_t *config,
 						 int64_t until_ms);
 
-	int (*read_memory)(struct target *target, target_addr_t address,
+	int (*read_memory)(struct target *target, const riscv_mem_access_args_t args);
-			uint32_t size, uint32_t count, uint8_t *buffer, uint32_t increment);
+	int (*write_memory)(struct target *target, const riscv_mem_access_args_t args);
 
 	unsigned int (*data_bits)(struct target *target);