[Pdbg] [PATCH] libpdbg: Add indirect address translation via callback

[Alistair Popple]

On Tuesday, 6 November 2018 2:39:36 PM AEDT Amitay Isaacs wrote:
> Hi Alistair,
> 
> Is the translate logic self-contained at the level of a node?
> 
> Since we traverse from node up towards the root, at the time of
> translate() we only have local offset, but no information about the
> address space.  So there is an implicit assumption that any translation
> is local and does not depend on the overall addresses space in which
> the translation is happening.  Is that correct?

As far as I know that is correct. Note that the offsets and nodes are mostly 
software defined contructs anyway - the translation is really about creating 
mappings of HW registers to software in a "convenient" way from what I can 
tell.

- Alistair

> On Tue, 2018-10-23 at 14:35 +1100, Alistair Popple wrote:
> > Some hardware targets have more complicated addressing schemes than a
> > simple base address + offset. It may be possible to determine a
> > device-tree representation for these schemes but for the moment it is
> > more straight forward to define a callback to do the translation.
> > 
> > Signed-off-by: Alistair Popple <alistair at popple.id.au>
> > ---
> > 
> >  libpdbg/target.c | 7 ++++++-
> >  libpdbg/target.h | 7 +++++++
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libpdbg/target.c b/libpdbg/target.c
> > index fbcf792..8e8c381 100644
> > --- a/libpdbg/target.c
> > +++ b/libpdbg/target.c
> > @@ -20,8 +20,13 @@ static struct pdbg_target
> > *get_class_target_addr(struct pdbg_target *target, con
> > 
> >  {
> >  
> >  	/* Check class */
> >  	while (strcmp(target->class, name)) {
> > 
> > +
> > +		if (target->translate)
> > +			*addr = target->translate(target, *addr);
> > +		else
> > +			*addr += dt_get_address(target, 0, NULL);
> > +
> > 
> >  		/* Keep walking the tree translating addresses */
> > 
> > -		*addr += dt_get_address(target, 0, NULL);
> > 
> >  		target = target->parent;
> >  		
> >  		/* The root node doesn't have an address space so it's
> > 
> > diff --git a/libpdbg/target.h b/libpdbg/target.h
> > index c8da048..1cb6b13 100644
> > --- a/libpdbg/target.h
> > +++ b/libpdbg/target.h
> > @@ -38,6 +38,7 @@ struct pdbg_target {
> > 
> >  	char *class;
> >  	int (*probe)(struct pdbg_target *target);
> >  	void (*release)(struct pdbg_target *target);
> > 
> > +	uint64_t (*translate)(struct pdbg_target *target, uint64_t
> > addr);
> > 
> >  	int index;
> >  	enum pdbg_target_status status;
> >  	const char *dn_name;
> > 
> > @@ -57,6 +58,12 @@ struct pdbg_target_class
> > *require_target_class(const char *name);
> > 
> >  struct pdbg_target_class *get_target_class(const char *name);
> >  bool pdbg_target_is_class(struct pdbg_target *target, const char
> > 
> > *class);
> > 
> > +/* This works and should be safe because struct pdbg_target is
> > guaranteed to be
> > + * the first member of the specialised type (see the DECLARE_HW_UNIT
> > definition
> > + * below). I'm not sure how sane it is though. Probably not very but
> > it does
> > + * remove a bunch of tedious container_of() typing */
> > +#define translate_cast(x) (uint64_t (*)(struct pdbg_target *,
> > uint64_t)) (x)
> > +
> > 
> >  extern struct list_head empty_list;
> >  extern struct list_head target_classes;
> 
> Amitay.