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

[Amitay Isaacs]

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.

> 
> - 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.
-- 

If you don't like what you're getting, check out what you're giving.