[Pdbg] [RFC 02/12] sbefifo: Rework the memory interfaces

Alistair Popple alistair at popple.id.au
Tue Aug 6 11:37:13 AEST 2019


The SBEFIFO unit introduced it's own implementation specific interface
for accessing memory. Instead it should conform to the existing memory
access interfaces. This patch splits the SBEFIFO memory access into
it's own hardware unit so that it can conform with the existing
interfaces.

Signed-off-by: Alistair Popple <alistair at popple.id.au>
---
 libpdbg/hwunit.h  |  2 --
 libpdbg/sbefifo.c | 38 ++++++++++++++++++++++++++++++--------
 libpdbg/target.c  | 38 ++++++++------------------------------
 p9-kernel.dts.m4  |  4 ++++
 p9-pib.dts.m4     |  5 -----
 src/mem.c         | 46 ++++++----------------------------------------
 6 files changed, 48 insertions(+), 85 deletions(-)

diff --git a/libpdbg/hwunit.h b/libpdbg/hwunit.h
index 2618941..a359af0 100644
--- a/libpdbg/hwunit.h
+++ b/libpdbg/hwunit.h
@@ -68,8 +68,6 @@ struct mem {
 struct sbefifo {
 	struct pdbg_target target;
 	int (*istep)(struct sbefifo *, uint32_t major, uint32_t minor);
-	int (*mem_read)(struct sbefifo *, uint64_t, uint8_t *, uint64_t, bool);
-	int (*mem_write)(struct sbefifo *, uint64_t, uint8_t *, uint64_t, bool);
 	int (*thread_start)(struct sbefifo *, uint32_t core_id, uint32_t thread_id);
 	int (*thread_stop)(struct sbefifo *, uint32_t core_id, uint32_t thread_id);
 	int (*thread_step)(struct sbefifo *, uint32_t core_id, uint32_t thread_id);
diff --git a/libpdbg/sbefifo.c b/libpdbg/sbefifo.c
index a2442cd..b7f52a1 100644
--- a/libpdbg/sbefifo.c
+++ b/libpdbg/sbefifo.c
@@ -244,9 +244,9 @@ static int sbefifo_op_istep(struct sbefifo *sbefifo,
 	return 0;
 }
 
-static int sbefifo_op_getmem(struct sbefifo *sbefifo,
+static int sbefifo_op_getmem(struct mem *sbefifo,
 			     uint64_t addr, uint8_t *data, uint64_t size,
-			     bool ci)
+			     uint8_t block_size, bool ci)
 {
 	uint8_t *out;
 	uint64_t start_addr, end_addr;
@@ -257,6 +257,11 @@ static int sbefifo_op_getmem(struct sbefifo *sbefifo,
 
 	align = ci ? 8 : 128;
 
+	if (block_size && block_size != 8) {
+		PR_ERROR("sbefifo: Only 8 byte block sizes are supported\n");
+		return -1;
+	};
+
 	start_addr = addr & (~(uint64_t)(align-1));
 	end_addr = (addr + size + (align-1)) & (~(uint64_t)(align-1));
 
@@ -285,7 +290,8 @@ static int sbefifo_op_getmem(struct sbefifo *sbefifo,
 	msg[5] = htobe32(len);
 
 	out_len = len + 4;
-	rc = sbefifo_op(sbefifo, msg, sizeof(msg), cmd, &out, &out_len, &status);
+	rc = sbefifo_op(target_to_sbefifo(sbefifo->target.parent), msg, sizeof(msg), cmd,
+			&out, &out_len, &status);
 	if (rc)
 		return rc;
 
@@ -304,9 +310,9 @@ static int sbefifo_op_getmem(struct sbefifo *sbefifo,
 	return 0;
 }
 
-static int sbefifo_op_putmem(struct sbefifo *sbefifo,
+static int sbefifo_op_putmem(struct mem *sbefifo,
 			     uint64_t addr, uint8_t *data, uint64_t size,
-			     bool ci)
+			     uint8_t block_size, bool ci)
 {
 	uint8_t *out;
 	uint32_t *msg;
@@ -316,6 +322,11 @@ static int sbefifo_op_putmem(struct sbefifo *sbefifo,
 
 	align = ci ? 8 : 128;
 
+	if (block_size && block_size != 8) {
+		PR_ERROR("sbefifo: Only 8 byte block sizes are supported\n");
+		return -1;
+	};
+
 	if (addr & (align-1)) {
 		PR_ERROR("sbefifo: Address must be aligned to %d bytes\n", align);
 		return -1;
@@ -353,7 +364,8 @@ static int sbefifo_op_putmem(struct sbefifo *sbefifo,
 	memcpy(&msg[6], data, len);
 
 	out_len = 4;
-	rc = sbefifo_op(sbefifo, msg, msg_len, cmd, &out, &out_len, &status);
+	rc = sbefifo_op(target_to_sbefifo(sbefifo->target.parent), msg, msg_len, cmd,
+			&out, &out_len, &status);
 	if (rc)
 		return rc;
 
@@ -479,6 +491,17 @@ static int sbefifo_probe(struct pdbg_target *target)
 	return 0;
 }
 
+struct mem sbefifo_mem = {
+	.target = {
+		.name = "SBE FIFO Chip-op based memory access",
+		.compatible = "ibm,sbefifo-mem",
+		.class = "mem",
+	},
+	.read = sbefifo_op_getmem,
+	.write = sbefifo_op_putmem,
+};
+DECLARE_HW_UNIT(sbefifo_mem);
+
 struct sbefifo kernel_sbefifo = {
 	.target = {
 		.name =	"Kernel based FSI SBE FIFO",
@@ -487,8 +510,6 @@ struct sbefifo kernel_sbefifo = {
 		.probe = sbefifo_probe,
 	},
 	.istep = sbefifo_op_istep,
-	.mem_read = sbefifo_op_getmem,
-	.mem_write = sbefifo_op_putmem,
 	.thread_start = sbefifo_op_thread_start,
 	.thread_stop = sbefifo_op_thread_stop,
 	.thread_step = sbefifo_op_thread_step,
@@ -503,4 +524,5 @@ __attribute__((constructor))
 static void register_sbefifo(void)
 {
 	pdbg_hwunit_register(&kernel_sbefifo_hw_unit);
+	pdbg_hwunit_register(&sbefifo_mem_hw_unit);
 }
diff --git a/libpdbg/target.c b/libpdbg/target.c
index 8880bf3..61353bc 100644
--- a/libpdbg/target.c
+++ b/libpdbg/target.c
@@ -219,48 +219,26 @@ int fsi_write(struct pdbg_target *fsi_dt, uint32_t addr, uint32_t data)
 
 int mem_read(struct pdbg_target *target, uint64_t addr, uint8_t *output, uint64_t size, uint8_t block_size, bool ci)
 {
+	struct mem *mem;
 	int rc = -1;
 
-	assert(pdbg_target_is_class(target, "sbefifo") ||
-	       pdbg_target_is_class(target, "mem"));
+	assert(pdbg_target_is_class(target, "mem"));
 
-	if (pdbg_target_is_class(target, "sbefifo")) {
-		struct sbefifo *sbefifo;
-
-		sbefifo = target_to_sbefifo(target);
-		rc = sbefifo->mem_read(sbefifo, addr, output, size, ci);
-	}
-
-	if (pdbg_target_is_class(target, "mem")) {
-		struct mem *mem;
-
-		mem = target_to_mem(target);
-		rc = mem->read(mem, addr, output, size, block_size, ci);
-	}
+	mem = target_to_mem(target);
+	rc = mem->read(mem, addr, output, size, block_size, ci);
 
 	return rc;
 }
 
 int mem_write(struct pdbg_target *target, uint64_t addr, uint8_t *input, uint64_t size, uint8_t block_size, bool ci)
 {
+	struct mem *mem;
 	int rc = -1;
 
-	assert(pdbg_target_is_class(target, "sbefifo") ||
-	       pdbg_target_is_class(target, "mem"));
+	assert(pdbg_target_is_class(target, "mem"));
 
-	if (pdbg_target_is_class(target, "sbefifo")) {
-		struct sbefifo *sbefifo;
-
-		sbefifo = target_to_sbefifo(target);
-		rc = sbefifo->mem_write(sbefifo, addr, input, size, ci);
-	}
-
-	if (pdbg_target_is_class(target, "mem")) {
-		struct mem *mem;
-
-		mem = target_to_mem(target);
-		rc = mem->write(mem, addr, input, size, block_size, ci);
-	}
+	mem = target_to_mem(target);
+	rc = mem->write(mem, addr, input, size, block_size, ci);
 
 	return rc;
 }
diff --git a/p9-kernel.dts.m4 b/p9-kernel.dts.m4
index 9ab46b8..30cde95 100644
--- a/p9-kernel.dts.m4
+++ b/p9-kernel.dts.m4
@@ -28,6 +28,10 @@
 			index = <0x0>;
 			compatible = "ibm,kernel-sbefifo";
 			device-path = "/dev/sbefifo1";
+
+			sbefifo-mem at 0 {
+				      compatible = "ibm,sbefifo-mem";
+			};
 		};
 
 		hmfsi at 100000 {
diff --git a/p9-pib.dts.m4 b/p9-pib.dts.m4
index 3e312e5..3a99157 100644
--- a/p9-pib.dts.m4
+++ b/p9-pib.dts.m4
@@ -36,11 +36,6 @@ reg = <0x0 HEX(CHIPLET_BASE($1)) 0xfffff>;
 }')dnl
 
 
-adu at 90000 {
-	  compatible = "ibm,power9-adu";
-	  reg = <0x0 0x90000 0x5>;
-};
-
 htm at 5012880 {
 	compatible = "ibm,power9-nhtm";
 	reg = <0x0 0x5012880 0x40>;
diff --git a/src/mem.c b/src/mem.c
index cacd394..53d33b8 100644
--- a/src/mem.c
+++ b/src/mem.c
@@ -95,23 +95,6 @@ static int _getmem(uint64_t addr, uint64_t size, uint8_t block_size, bool ci, bo
 	buf = malloc(size);
 	assert(buf);
 
-	pdbg_for_each_class_target("sbefifo", target) {
-		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
-			continue;
-
-		pdbg_set_progress_tick(progress_tick);
-		progress_init();
-		rc = mem_read(target, addr, buf, size, block_size, ci);
-		progress_end();
-		if (rc) {
-			PR_ERROR("Unable to read memory using sbefifo\n");
-			continue;
-		}
-
-		count++;
-		goto done;
-	}
-
 	pdbg_for_each_class_target("mem", target) {
 		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
 			continue;
@@ -121,15 +104,15 @@ static int _getmem(uint64_t addr, uint64_t size, uint8_t block_size, bool ci, bo
 		rc = mem_read(target, addr, buf, size, block_size, ci);
 		progress_end();
 		if (rc) {
-			PR_ERROR("Unable to read memory using adu\n");
+			PR_ERROR("Unable to read memory from %s\n",
+				 pdbg_target_path(target));
 			continue;
 		}
 
 		count++;
-		goto done;
+		break;
 	}
 
-done:
 	if (count > 0) {
 		uint64_t i;
 		bool printable = true;
@@ -188,23 +171,6 @@ static int _putmem(uint64_t addr, uint8_t block_size, bool ci)
 	buf = read_stdin(&buflen);
 	assert(buf);
 
-	pdbg_for_each_class_target("sbefifo", target) {
-		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
-			continue;
-
-		pdbg_set_progress_tick(progress_tick);
-		progress_init();
-		rc = mem_write(target, addr, buf, buflen, block_size, ci);
-		progress_end();
-		if (rc) {
-			printf("Unable to write memory using sbefifo\n");
-			continue;
-		}
-
-		count++;
-		goto done;
-	}
-
 	pdbg_for_each_class_target("mem", target) {
 		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
 			continue;
@@ -214,15 +180,15 @@ static int _putmem(uint64_t addr, uint8_t block_size, bool ci)
 		rc = mem_write(target, addr, buf, buflen, block_size, ci);
 		progress_end();
 		if (rc) {
-			printf("Unable to write memory using adu\n");
+			printf("Unable to write memory using %s\n",
+			       pdbg_target_path(target));
 			continue;
 		}
 
 		count++;
-		goto done;
+		break;
 	}
 
-done:
 	if (count > 0)
 		printf("Wrote %zu bytes starting at 0x%016" PRIx64 "\n", buflen, addr);
 
-- 
2.20.1



More information about the Pdbg mailing list