[Pdbg] [PATCH 2/5] sbefifo: Rework the memory interfaces

Alistair Popple alistair at popple.id.au
Tue Aug 20 17:19:03 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  |  8 ++++++++
 p9-pib.dts.m4     |  5 -----
 src/mem.c         | 46 ++++++----------------------------------------
 6 files changed, 52 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 784db32..5c63512 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;
 
@@ -484,6 +496,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",
@@ -492,8 +515,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,
@@ -508,4 +529,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 2396804..67191b5 100644
--- a/libpdbg/target.c
+++ b/libpdbg/target.c
@@ -247,48 +247,26 @@ int fsi_write_mask(struct pdbg_target *fsi_dt, uint32_t addr, uint32_t data, uin
 
 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..aa4bcac 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 {
@@ -53,6 +57,10 @@
 				index = <0x1>;
 				compatible = "ibm,kernel-sbefifo";
 				device-path = "/dev/sbefifo2";
+
+				sbefifo-mem at 0 {
+				      compatible = "ibm,sbefifo-mem";
+				};
 			};
 		};
 	};
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