[RFC,02/12] sbefifo: Rework the memory interfaces
diff mbox series

Message ID 20190806013723.4047-3-alistair@popple.id.au
State New
Headers show
Series
  • Split backends from system description
Related show

Checks

Context Check Description
snowpatch_ozlabs/build-multiarch success Test build-multiarch on branch master
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (3a46123baa267a1bc6e03c50449a42e8d321bf86)

Commit Message

Alistair Popple Aug. 6, 2019, 1:37 a.m. UTC
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@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(-)

Comments

Amitay Isaacs Aug. 20, 2019, 3:42 a.m. UTC | #1
On Tue, 2019-08-06 at 11:37 +1000, Alistair Popple wrote:
> 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@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@0 {
> +				      compatible = "ibm,sbefifo-mem";
> +			};
>  		};
>  
>  		hmfsi@100000 {

Shouldn't we add sbefifo-mem@1 (for sbefifo@1) for completeness?


> 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@90000 {
> -	  compatible = "ibm,power9-adu";
> -	  reg = <0x0 0x90000 0x5>;
> -};
> -
>  htm@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
> 

Amitay.
Alistair Popple Aug. 20, 2019, 4:47 a.m. UTC | #2
> > 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@0 {
> > +				      compatible = "ibm,sbefifo-mem";
> > +			};
> >  		};
> >  
> >  		hmfsi@100000 {
> 
> Shouldn't we add sbefifo-mem@1 (for sbefifo@1) for completeness?

Yep, will repost a non-RFC version with that added.

- Alistair
 
> 
> > 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@90000 {
> > -	  compatible = "ibm,power9-adu";
> > -	  reg = <0x0 0x90000 0x5>;
> > -};
> > -
> >  htm@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);
> >  
> 
> Amitay.
>

Patch
diff mbox series

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@0 {
+				      compatible = "ibm,sbefifo-mem";
+			};
 		};
 
 		hmfsi@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@90000 {
-	  compatible = "ibm,power9-adu";
-	  reg = <0x0 0x90000 0x5>;
-};
-
 htm@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);