[Pdbg] [PATCH 5/7] mem: Pass target class as argument to _getmem/_putmem

[Alistair Popple]

On Monday, 28 October 2019 12:46:38 PM AEDT Amitay Isaacs wrote:
> On Mon, 2019-10-28 at 12:07 +1100, Alistair Popple wrote:
> > On Thursday, 24 October 2019 12:48:45 PM AEDT Amitay Isaacs wrote:
> > > This will allow different mem class (e.g. pba) to be supported.
> > > 
> > > Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> > > ---
> > >  src/mem.c | 20 ++++++++++----------
> > >  1 file changed, 10 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/src/mem.c b/src/mem.c
> > > index 2fe5918..b225047 100644
> > > --- a/src/mem.c
> > > +++ b/src/mem.c
> > > @@ -81,7 +81,7 @@ static uint8_t *read_stdin(size_t *size)
> > >  	return buf;
> > >  }
> > >  
> > > -static int _getmem(uint64_t addr, uint64_t size, uint8_t
> > > block_size, bool 
> > ci, bool raw)
> > > +static int _getmem(const char *classname, uint64_t addr, uint64_t
> > > size, 
> > uint8_t block_size, bool ci, bool raw)
> > >  {
> > >  	struct pdbg_target *target;
> > >  	uint8_t *buf;
> > > @@ -95,7 +95,7 @@ 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("mem", target) {
> > > +	pdbg_for_each_class_target(classname, target) {
> > 
> > I plan to merge this on top of the backend patches you posted
> > previously. The 
> > intent is that the environment (ie. the backend tree) would configure
> > a default 
> > access method for memory, so by default I think this should still be
> > operating 
> > on the "mem" class so that it picks up the correct virtual node.
> > 
> > If a user (ie. the pdbg application) knows that it wants to use a
> > particular 
> > access method (eg. pba) then I think it should find that specific
> > target and 
> > pass that in to the library as the target instead. Although perhaps
> > we need 
> > some way nice way of finding those? The class is supposed to
> > represent the 
> > actual hardware type being accessed, rather than the access method
> > itself.
> 
> Yeah, that's the real issue.  We don't have a nice way of
> differentiating between two memory class instances.  We might need to
> invent something else like "class", which supports that distinction.

Yep, in some ways I was intentionally ignoring this issue so we could make 
progress on the backend series. I have merged those and will push it so we can 
start figruing out how best to do this as it seems the immediate requirement 
for pba access has disappeared.

- Alistair

> > 
> > - Alistair
> > 
> > >  		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> > >  			continue;
> > >  
> > > @@ -129,21 +129,21 @@ static int _getmem(uint64_t addr, uint64_t
> > > size, 
> > uint8_t block_size, bool ci, bo
> > >  static int getmem(uint64_t addr, uint64_t size, struct mem_flags
> > > flags)
> > >  {
> > >  	if (flags.ci)
> > > -		return _getmem(addr, size, 8, true, flags.raw);
> > > +		return _getmem("mem", addr, size, 8, true, flags.raw);
> > >  	else
> > > -		return _getmem(addr, size, 0, false, flags.raw);
> > > +		return _getmem("mem", addr, size, 0, false, flags.raw);
> > >  }
> > >  OPTCMD_DEFINE_CMD_WITH_FLAGS(getmem, getmem, (ADDRESS, DATA),
> > >  			     mem_flags, (MEM_CI_FLAG, MEM_RAW_FLAG));
> > >  
> > >  static int getmemio(uint64_t addr, uint64_t size, uint8_t
> > > block_size, 
> > struct mem_io_flags flags)
> > >  {
> > > -	return _getmem(addr, size, block_size, true, flags.raw);
> > > +	return _getmem("mem", addr, size, block_size, true, flags.raw);
> > >  }
> > >  OPTCMD_DEFINE_CMD_WITH_FLAGS(getmemio, getmemio, (ADDRESS, DATA, 
> > BLOCK_SIZE),
> > >  			     mem_io_flags, (MEM_RAW_FLAG));
> > >  
> > > -static int _putmem(uint64_t addr, uint8_t block_size, bool ci)
> > > +static int _putmem(const char *classname, uint64_t addr, uint8_t 
> > block_size, bool ci)
> > >  {
> > >  	uint8_t *buf;
> > >  	size_t buflen;
> > > @@ -153,7 +153,7 @@ static int _putmem(uint64_t addr, uint8_t
> > > block_size, 
> > bool ci)
> > >  	buf = read_stdin(&buflen);
> > >  	assert(buf);
> > >  
> > > -	pdbg_for_each_class_target("mem", target) {
> > > +	pdbg_for_each_class_target(classname, target) {
> > >  		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> > >  			continue;
> > >  
> > > @@ -181,14 +181,14 @@ static int _putmem(uint64_t addr, uint8_t
> > > block_size, 
> > bool ci)
> > >  static int putmem(uint64_t addr, struct mem_flags flags)
> > >  {
> > >  	if (flags.ci)
> > > -		return _putmem(addr, 8, true);
> > > +		return _putmem("mem", addr, 8, true);
> > >  	else
> > > -		return _putmem(addr, 0, false);
> > > +		return _putmem("mem", addr, 0, false);
> > >  }
> > >  OPTCMD_DEFINE_CMD_WITH_FLAGS(putmem, putmem, (ADDRESS),
> > > mem_flags, 
> > (MEM_CI_FLAG));
> > >  
> > >  static int putmemio(uint64_t addr, uint8_t block_size)
> > >  {
> > > -	return _putmem(addr, block_size, true);
> > > +	return _putmem("mem", addr, block_size, true);
> > >  }
> > >  OPTCMD_DEFINE_CMD_WITH_ARGS(putmemio, putmemio, (ADDRESS,
> > > BLOCK_SIZE));
> > > 
> > 
> > 
> > 
> 
> Amitay.
>