[Pdbg] [PATCH 2/2] adu: Add arugments for block size

Alistair Popple alistair at popple.id.au
Mon Dec 10 11:49:54 AEDT 2018


Not all memory can be read with the default ADU block size of 8
bytes. Specifically cache-inhibited access to some MMIO regions such
as PCIe BAR spaces requires 4 byte accesses to avoid check stopping
the machine.

This patch adds an argument to the put/getmem commands to allow a
specific block size to be selected.

Signed-off-by: Alistair Popple <alistair at popple.id.au>
---
 libpdbg/adu.c     | 124 +++++++++++++++++++++++++++++++++++++-----------------
 libpdbg/libpdbg.h |  18 +++++---
 libpdbg/target.h  |   4 +-
 src/mem.c         |  14 +++---
 src/pdbgproxy.c   |   4 +-
 src/thread.c      |   2 +-
 6 files changed, 111 insertions(+), 55 deletions(-)

diff --git a/libpdbg/adu.c b/libpdbg/adu.c
index b67a43e..3ab0d31 100644
--- a/libpdbg/adu.c
+++ b/libpdbg/adu.c
@@ -84,20 +84,48 @@
 #define FBC_ALTD_DATA_DONE	PPC_BIT(3)
 #define FBC_ALTD_PBINIT_MISSING PPC_BIT(18)
 
+/* There are more general implementations of this with a loop and more
+ * performant implementations using GCC builtins which aren't
+ * portable. Given we only need a limited domain this is quick, easy
+ * and portable. */
+uint8_t blog2(uint8_t x)
+{
+	switch(x) {
+	case 1:
+		return 0;
+	case 2:
+		return 1;
+	case 4:
+		return 2;
+	case 8:
+		return 3;
+	case 16:
+		return 4;
+	case 32:
+		return 5;
+	case 64:
+		return 6;
+	default:
+		assert(0);
+	}
+}
+
 int adu_getmem(struct pdbg_target *adu_target, uint64_t start_addr,
-	       uint8_t *output, uint64_t size)
+	       uint8_t *output, uint64_t size, uint8_t block_size)
 {
-	return __adu_getmem(adu_target, start_addr, output, size, false);
+	return __adu_getmem(adu_target, start_addr, output,
+			    size, false, block_size);
 }
 
 int adu_getmem_ci(struct pdbg_target *adu_target, uint64_t start_addr,
-		  uint8_t *output, uint64_t size)
+		  uint8_t *output, uint64_t size, uint8_t block_size)
 {
-	return __adu_getmem(adu_target, start_addr, output, size, true);
+	return __adu_getmem(adu_target, start_addr, output,
+			    size, true, block_size);
 }
 
 int __adu_getmem(struct pdbg_target *adu_target, uint64_t start_addr,
-		 uint8_t *output, uint64_t size, bool ci)
+		 uint8_t *output, uint64_t size, bool ci, uint8_t block_size)
 {
 	struct adu *adu;
 	uint8_t *output0;
@@ -110,32 +138,33 @@ int __adu_getmem(struct pdbg_target *adu_target, uint64_t start_addr,
 	output0 = output;
 
 	/* Align start address to 8-byte boundary */
-	addr0 = 8 * (start_addr / 8);
+	addr0 = block_size * (start_addr / block_size);
 
 	/* We read data in 8-byte aligned chunks */
-	for (addr = addr0; addr < start_addr + size; addr += 8) {
+	for (addr = addr0; addr < start_addr + size; addr += block_size) {
 		uint64_t data;
 
-		if (adu->getmem(adu, addr, &data, ci))
+		if (adu->getmem(adu, addr, &data, ci, block_size))
 			return -1;
 
-		/* ADU returns data in big-endian form in the register */
+		/* ADU returns data in big-endian form in the register. */
 		data = __builtin_bswap64(data);
+		data >>= (addr & 0x7ull)*8;
 
 		if (addr < start_addr) {
 			size_t offset = start_addr - addr;
-			size_t n = (size <= 8-offset ? size : 8-offset);
+			size_t n = (size <= block_size-offset ? size : block_size-offset);
 
 			memcpy(output, ((uint8_t *) &data) + offset, n);
 			output += n;
-		} else if (addr + 8 > start_addr + size) {
+		} else if (addr + block_size > start_addr + size) {
 			uint64_t offset = start_addr + size - addr;
 
 			memcpy(output, &data, offset);
 			output += offset;
 		} else {
-			memcpy(output, &data, 8);
-			output += 8;
+			memcpy(output, &data, block_size);
+			output += block_size;
 		}
 
 		pdbg_progress_tick(output - output0, size);
@@ -147,19 +176,19 @@ int __adu_getmem(struct pdbg_target *adu_target, uint64_t start_addr,
 }
 
 int adu_putmem(struct pdbg_target *adu_target, uint64_t start_addr,
-	       uint8_t *output, uint64_t size)
+	       uint8_t *output, uint64_t size, uint8_t block_size)
 {
-	return __adu_putmem(adu_target, start_addr, output, size, false);
+	return __adu_putmem(adu_target, start_addr, output, size, false, block_size);
 }
 
 int adu_putmem_ci(struct pdbg_target *adu_target, uint64_t start_addr,
-		  uint8_t *output, uint64_t size)
+		  uint8_t *output, uint64_t size, uint8_t block_size)
 {
-	return __adu_putmem(adu_target, start_addr, output, size, true);
+	return __adu_putmem(adu_target, start_addr, output, size, true, block_size);
 }
 
 int __adu_putmem(struct pdbg_target *adu_target, uint64_t start_addr,
-		 uint8_t *input, uint64_t size, bool ci)
+		 uint8_t *input, uint64_t size, bool ci, uint8_t block_size)
 {
 	struct adu *adu;
 	int rc = 0, tsize;
@@ -169,7 +198,7 @@ int __adu_putmem(struct pdbg_target *adu_target, uint64_t start_addr,
 	adu = target_to_adu(adu_target);
 	end_addr = start_addr + size;
 	for (addr = start_addr; addr < end_addr; addr += tsize, input += tsize) {
-		if ((addr % 8) || (addr + 8 > end_addr)) {
+		if ((addr % block_size) || (addr + block_size > end_addr)) {
 			/* If the address is not 64-bit aligned we
 			 * copy in a byte at a time until it is. */
 			tsize = 1;
@@ -177,12 +206,13 @@ int __adu_putmem(struct pdbg_target *adu_target, uint64_t start_addr,
 			/* Copy the input data in with correct alignment */
 			data = ((uint64_t) *input) << 8*(8 - (addr % 8) - 1);
 		} else {
-			tsize = 8;
-			memcpy(&data, input, sizeof(data));
+			tsize = block_size;
+			memcpy(&data, input, block_size);
 			data = __builtin_bswap64(data);
+			data >>= (addr & 7ull)*8;
 		}
 
-		adu->putmem(adu, addr, data, tsize, ci);
+		adu->putmem(adu, addr, data, tsize, ci, block_size);
 		pdbg_progress_tick(addr - start_addr, size);
 	}
 
@@ -234,7 +264,8 @@ static int adu_reset(struct adu *adu)
 	return 0;
 }
 
-static int p8_adu_getmem(struct adu *adu, uint64_t addr, uint64_t *data, int ci)
+static int p8_adu_getmem(struct adu *adu, uint64_t addr, uint64_t *data,
+			 int ci, uint8_t block_size)
 {
 	uint64_t ctrl_reg, cmd_reg, val;
 	int rc = 0;
@@ -242,12 +273,15 @@ static int p8_adu_getmem(struct adu *adu, uint64_t addr, uint64_t *data, int ci)
 	CHECK_ERR(adu_lock(adu));
 
 	ctrl_reg = P8_TTYPE_TREAD;
-	if (ci)
+	if (ci) {
 		/* Do cache inhibited access */
 		ctrl_reg = SETFIELD(P8_FBC_ALTD_TTYPE, ctrl_reg, P8_TTYPE_CI_PARTIAL_READ);
-	else
+		block_size = (blog2(block_size) + 1) << 1;
+	} else {
 		ctrl_reg = SETFIELD(P8_FBC_ALTD_TTYPE, ctrl_reg, P8_TTYPE_DMA_PARTIAL_READ);
-	ctrl_reg = SETFIELD(P8_FBC_ALTD_TSIZE, ctrl_reg, 8);
+		block_size = 0;
+	}
+	ctrl_reg = SETFIELD(P8_FBC_ALTD_TSIZE, ctrl_reg, block_size);
 
 	CHECK_ERR_GOTO(out, rc = pib_read(&adu->target, P8_ALTD_CMD_REG, &cmd_reg));
 	cmd_reg |= FBC_ALTD_START_OP;
@@ -292,19 +326,23 @@ out:
 
 }
 
-int p8_adu_putmem(struct adu *adu, uint64_t addr, uint64_t data, int size, int ci)
+int p8_adu_putmem(struct adu *adu, uint64_t addr, uint64_t data, int size,
+		  int ci, uint8_t block_size)
 {
 	int rc = 0;
 	uint64_t cmd_reg, ctrl_reg, val;
 	CHECK_ERR(adu_lock(adu));
 
 	ctrl_reg = P8_TTYPE_TWRITE;
-	if (ci)
+	if (ci) {
 		/* Do cache inhibited access */
 		ctrl_reg = SETFIELD(P8_FBC_ALTD_TTYPE, ctrl_reg, P8_TTYPE_CI_PARTIAL_WRITE);
-	else
+		block_size = (blog2(block_size) + 1) << 1;
+	} else {
 		ctrl_reg = SETFIELD(P8_FBC_ALTD_TTYPE, ctrl_reg, P8_TTYPE_DMA_PARTIAL_WRITE);
-	ctrl_reg = SETFIELD(P8_FBC_ALTD_TSIZE, ctrl_reg, size);
+		block_size <<= 1;
+	}
+	ctrl_reg = SETFIELD(P8_FBC_ALTD_TSIZE, ctrl_reg, block_size);
 
 	CHECK_ERR_GOTO(out, rc = pib_read(&adu->target, P8_ALTD_CMD_REG, &cmd_reg));
 	cmd_reg |= FBC_ALTD_START_OP;
@@ -349,19 +387,23 @@ out:
 	return rc;
 }
 
-static int p9_adu_getmem(struct adu *adu, uint64_t addr, uint64_t *data, int ci)
+static int p9_adu_getmem(struct adu *adu, uint64_t addr, uint64_t *data,
+			 int ci, uint8_t block_size)
 {
 	uint64_t ctrl_reg, cmd_reg, val;
 
 	cmd_reg = P9_TTYPE_TREAD;
-	if (ci)
+	if (ci) {
 		/* Do cache inhibited access */
 		cmd_reg = SETFIELD(P9_FBC_ALTD_TTYPE, cmd_reg, P9_TTYPE_CI_PARTIAL_READ);
-	else
+		block_size = (blog2(block_size) + 1) << 1;
+	} else {
 		cmd_reg = SETFIELD(P9_FBC_ALTD_TTYPE, cmd_reg, P9_TTYPE_DMA_PARTIAL_READ);
+		block_size = 0;
+	}
 
 	/* For a read size is apparently always 0 */
-	cmd_reg = SETFIELD(P9_FBC_ALTD_TSIZE, cmd_reg, 0);
+	cmd_reg = SETFIELD(P9_FBC_ALTD_TSIZE, cmd_reg, block_size);
  	cmd_reg |= FBC_ALTD_START_OP;
 	cmd_reg = SETFIELD(FBC_ALTD_SCOPE, cmd_reg, SCOPE_REMOTE);
 	cmd_reg = SETFIELD(FBC_ALTD_DROP_PRIORITY, cmd_reg, DROP_PRIORITY_LOW);
@@ -370,6 +412,8 @@ retry:
 	/* Clear status bits */
 	CHECK_ERR(adu_reset(adu));
 
+	CHECK_ERR(pib_write(&adu->target, P9_ALTD_DATA_REG, 0));
+
 	/* Set the address */
 	ctrl_reg = SETFIELD(P9_FBC_ALTD_ADDRESS, 0ULL, addr);
 	CHECK_ERR(pib_write(&adu->target, P9_ALTD_CONTROL_REG, ctrl_reg));
@@ -400,7 +444,8 @@ retry:
 	return 0;
 }
 
-static int p9_adu_putmem(struct adu *adu, uint64_t addr, uint64_t data, int size, int ci)
+static int p9_adu_putmem(struct adu *adu, uint64_t addr, uint64_t data, int size,
+			 int ci, uint8_t block_size)
 {
 	uint64_t ctrl_reg, cmd_reg, val;
 
@@ -409,12 +454,15 @@ static int p9_adu_putmem(struct adu *adu, uint64_t addr, uint64_t data, int size
 	size <<= 1;
 
 	cmd_reg = P9_TTYPE_TWRITE;
-	if (ci)
+	if (ci) {
 		/* Do cache inhibited access */
 		cmd_reg = SETFIELD(P9_FBC_ALTD_TTYPE, cmd_reg, P9_TTYPE_CI_PARTIAL_WRITE);
-	else
+		block_size = (blog2(block_size) + 1) << 1;
+	} else {
 		cmd_reg = SETFIELD(P9_FBC_ALTD_TTYPE, cmd_reg, P9_TTYPE_DMA_PARTIAL_WRITE);
-	cmd_reg = SETFIELD(P9_FBC_ALTD_TSIZE, cmd_reg, size);
+		block_size <<= 1;
+	}
+	cmd_reg = SETFIELD(P9_FBC_ALTD_TSIZE, cmd_reg, block_size);
  	cmd_reg |= FBC_ALTD_START_OP;
 	cmd_reg = SETFIELD(FBC_ALTD_SCOPE, cmd_reg, SCOPE_REMOTE);
 	cmd_reg = SETFIELD(FBC_ALTD_DROP_PRIORITY, cmd_reg, DROP_PRIORITY_LOW);
diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
index 1977c75..0ba8508 100644
--- a/libpdbg/libpdbg.h
+++ b/libpdbg/libpdbg.h
@@ -178,12 +178,18 @@ int htm_status(struct pdbg_target *target);
 int htm_dump(struct pdbg_target *target, char *filename);
 int htm_record(struct pdbg_target *target, char *filename);
 
-int adu_getmem(struct pdbg_target *target, uint64_t addr, uint8_t *ouput, uint64_t size);
-int adu_putmem(struct pdbg_target *target, uint64_t addr, uint8_t *input, uint64_t size);
-int adu_getmem_ci(struct pdbg_target *target, uint64_t addr, uint8_t *ouput, uint64_t size);
-int adu_putmem_ci(struct pdbg_target *target, uint64_t addr, uint8_t *input, uint64_t size);
-int __adu_getmem(struct pdbg_target *target, uint64_t addr, uint8_t *ouput, uint64_t size, bool ci);
-int __adu_putmem(struct pdbg_target *target, uint64_t addr, uint8_t *input, uint64_t size, bool ci);
+int adu_getmem(struct pdbg_target *target, uint64_t addr,
+	       uint8_t *ouput, uint64_t size, uint8_t block_size);
+int adu_putmem(struct pdbg_target *target, uint64_t addr,
+	       uint8_t *input, uint64_t size, uint8_t block_size);
+int adu_getmem_ci(struct pdbg_target *target, uint64_t addr,
+		  uint8_t *ouput, uint64_t size, uint8_t block_size);
+int adu_putmem_ci(struct pdbg_target *target, uint64_t addr,
+		  uint8_t *input, uint64_t size, uint8_t block_size);
+int __adu_getmem(struct pdbg_target *target, uint64_t addr, uint8_t *ouput,
+		 uint64_t size, bool ci, uint8_t block_size);
+int __adu_putmem(struct pdbg_target *target, uint64_t addr, uint8_t *input,
+		 uint64_t size, bool ci, uint8_t block_size);
 
 int opb_read(struct pdbg_target *target, uint32_t addr, uint32_t *data);
 int opb_write(struct pdbg_target *target, uint32_t addr, uint32_t data);
diff --git a/libpdbg/target.h b/libpdbg/target.h
index d1a6aec..96d0c73 100644
--- a/libpdbg/target.h
+++ b/libpdbg/target.h
@@ -101,8 +101,8 @@ struct htm {
 
 struct adu {
 	struct pdbg_target target;
-	int (*getmem)(struct adu *, uint64_t, uint64_t *, int);
-	int (*putmem)(struct adu *, uint64_t, uint64_t, int, int);
+	int (*getmem)(struct adu *, uint64_t, uint64_t *, int, uint8_t);
+	int (*putmem)(struct adu *, uint64_t, uint64_t, int, int, uint8_t);
 };
 #define target_to_adu(x) container_of(x, struct adu, target)
 
diff --git a/src/mem.c b/src/mem.c
index ce099c2..b578f95 100644
--- a/src/mem.c
+++ b/src/mem.c
@@ -39,8 +39,10 @@ struct mem_flags {
 };
 
 #define MEM_CI_FLAG ("--ci", ci, parse_flag_noarg, false)
+#define BLOCK_SIZE (parse_number8_pow2, "8")
 
-static int getmem(uint64_t addr, uint64_t size, struct mem_flags flags)
+static int getmem(uint64_t addr, uint64_t size,
+		  uint8_t block_size, struct mem_flags flags)
 {
 	struct pdbg_target *target;
 	uint8_t *buf;
@@ -59,7 +61,7 @@ static int getmem(uint64_t addr, uint64_t size, struct mem_flags flags)
 
 		pdbg_set_progress_tick(progress_tick);
 		progress_init();
-		if (!__adu_getmem(target, addr, buf, size, flags.ci)) {
+		if (!__adu_getmem(target, addr, buf, size, flags.ci, block_size)) {
 			if (write(STDOUT_FILENO, buf, size) < 0)
 				PR_ERROR("Unable to write stdout.\n");
 			else
@@ -74,10 +76,10 @@ static int getmem(uint64_t addr, uint64_t size, struct mem_flags flags)
 	return rc;
 
 }
-OPTCMD_DEFINE_CMD_WITH_FLAGS(getmem, getmem, (ADDRESS, DATA),
+OPTCMD_DEFINE_CMD_WITH_FLAGS(getmem, getmem, (ADDRESS, DATA, BLOCK_SIZE),
 			     mem_flags, (MEM_CI_FLAG));
 
-static int putmem(uint64_t addr, struct mem_flags flags)
+static int putmem(uint64_t addr, uint8_t block_size, struct mem_flags flags)
 {
 	uint8_t *buf;
 	int read_size, rc = 0;
@@ -98,7 +100,7 @@ static int putmem(uint64_t addr, struct mem_flags flags)
 		if (read_size <= 0)
 			break;
 
-		if (__adu_putmem(adu_target, addr, buf, read_size, flags.ci)) {
+		if (__adu_putmem(adu_target, addr, buf, read_size, flags.ci, block_size)) {
 			rc = 0;
 			printf("Unable to write memory.\n");
 			break;
@@ -111,5 +113,5 @@ static int putmem(uint64_t addr, struct mem_flags flags)
 	free(buf);
 	return rc;
 }
-OPTCMD_DEFINE_CMD_WITH_FLAGS(putmem, putmem, (ADDRESS),
+OPTCMD_DEFINE_CMD_WITH_FLAGS(putmem, putmem, (ADDRESS, BLOCK_SIZE),
 			     mem_flags, (MEM_CI_FLAG));
diff --git a/src/pdbgproxy.c b/src/pdbgproxy.c
index 697e58e..64c1ae4 100644
--- a/src/pdbgproxy.c
+++ b/src/pdbgproxy.c
@@ -233,7 +233,7 @@ static void get_mem(uint64_t *stack, void *priv)
 
 	linear_map = get_real_addr(addr);
 	if (linear_map != -1UL) {
-		if (adu_getmem(adu, linear_map, (uint8_t *) data, len)) {
+		if (adu_getmem(adu, linear_map, (uint8_t *) data, len, 8)) {
 			PR_ERROR("Unable to read memory\n");
 			err = 1;
 		}
@@ -315,7 +315,7 @@ static void put_mem(uint64_t *stack, void *priv)
 
 	PR_INFO("put_mem 0x%016" PRIx64 " = 0x%016" PRIx64 "\n", addr, stack[2]);
 
-	if (adu_putmem(adu, addr, data, len)) {
+	if (adu_putmem(adu, addr, data, len, 8)) {
 		PR_ERROR("Unable to write memory\n");
 		err = 3;
 	}
diff --git a/src/thread.c b/src/thread.c
index 4401384..02dd1c2 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -97,7 +97,7 @@ static bool is_real_address(struct thread_regs *regs, uint64_t addr)
 
 static int load8(struct pdbg_target *target, uint64_t addr, uint64_t *value)
 {
-	if (adu_getmem(target, addr, (uint8_t *)value, 8)) {
+	if (adu_getmem(target, addr, (uint8_t *)value, 8, 8)) {
 		pdbg_log(PDBG_ERROR, "Unable to read memory address=%016" PRIx64 ".\n", addr);
 		return 0;
 	}
-- 
2.11.0



More information about the Pdbg mailing list