[Pdbg] [PATCH 6/7] libpdbg: Allow mem_read/mem_write for pba class

[Alistair Popple]

On Monday, 28 October 2019 12:51:28 PM AEDT Amitay Isaacs wrote:
> On Mon, 2019-10-28 at 12:16 +1100, Alistair Popple wrote:
> > On Thursday, 24 October 2019 12:48:46 PM AEDT Amitay Isaacs wrote:
> > > Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> > > ---
> > >  libpdbg/target.c | 28 ++++++++++++++++++++--------
> > >  1 file changed, 20 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/libpdbg/target.c b/libpdbg/target.c
> > > index 7ffc6e9..069d526 100644
> > > --- a/libpdbg/target.c
> > > +++ b/libpdbg/target.c
> > > @@ -248,12 +248,18 @@ 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;
> > > +	struct pba *pba;
> > >  	int rc = -1;
> > >  
> > > -	assert(pdbg_target_is_class(target, "mem"));
> > > -
> > > -	mem = target_to_mem(target);
> > > -	rc = mem->read(mem, addr, output, size, block_size, ci);
> > > +	if (pdbg_target_is_class(target, "mem")) {
> > > +		mem = target_to_mem(target);
> > > +		rc = mem->read(mem, addr, output, size, block_size,
> > > ci);
> > > +	} else if (pdbg_target_is_class(target, "pba")) {
> > > +		pba = target_to_pba(target);
> > > +		rc = pba->read(pba, addr, output, size, block_size,
> > > ci);
> > > +	} else {
> > > +		assert(pdbg_target_is_class(target, "mem") || 
> > pdbg_target_is_class(target, "pba"));
> > > +	}
> > 
> > A separate backend tree means that the specific access method used is
> > specified 
> > by the backend tree and therefore means we shouldn't have this kind
> > of logic 
> > in the library. If an application user wants to read memory on the
> > "/mem0" 
> > target we should use whatever access method the backend tree
> > specifies. If an 
> > application wants to use a specific access method (eg. pba) then it
> > should 
> > "somehow" look up that target and explicitly pass it in.
> 
> Yeah, but backend can (and should) support multiple methods of access
> for memory.  We don't want to create a different backend tree just for
> PBA memory access.
> 
> > 
> > Of course the "somehow" is perhaps something we need to figure out.
> > At the most 
> > basic level it could just be by using a specific path to the access
> > method, but 
> > I'm open to ideas.
> 
> Let's figure out what is the "somehow" users need to be able to do the
> right thing.  It cannot be absolute path based thing, because then each
> applicaton needs to know how to construct the device tree paths.  We
> need some "magic" where users can say I want processor 0 and memory
> access blah.

Agreed, we don't want the application to have to know about device specific 
paths. However I think it's fine to expect applications to construct system 
device tree paths.

Fore example to access memory just call something like getmem on "/mem0", and 
for pba you could even just do a getmem on "/mem-pba0" although that does kind 
of feel like an abuse of the design (because now /mem0 and /mem-pba0 would 
point to the same HW).

- Alistair

> > 
> > - Alistair
> > 
> > >  	return rc;
> > >  }
> > > @@ -261,12 +267,18 @@ int mem_read(struct pdbg_target *target,
> > > uint64_t 
> > addr, uint8_t *output, uint64_
> > >  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;
> > > +	struct pba *pba;
> > >  	int rc = -1;
> > >  
> > > -	assert(pdbg_target_is_class(target, "mem"));
> > > -
> > > -	mem = target_to_mem(target);
> > > -	rc = mem->write(mem, addr, input, size, block_size, ci);
> > > +	if (pdbg_target_is_class(target, "mem")) {
> > > +		mem = target_to_mem(target);
> > > +		rc = mem->write(mem, addr, input, size, block_size,
> > > ci);
> > > +	} else if (pdbg_target_is_class(target, "pba")) {
> > > +		pba = target_to_pba(target);
> > > +		rc = pba->write(pba, addr, input, size, block_size,
> > > ci);
> > > +	} else {
> > > +		assert(pdbg_target_is_class(target, "mem") || 
> > pdbg_target_is_class(target, "pba"));
> > > +	}
> > >  
> > >  	return rc;
> > >  }
> > > 
> > 
> > 
> > 
> 
> Amitay.
>