| Message ID | 20181023033528.31256-7-alistair@popple.id.au |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libpdbg: Add indirect address translation via callback | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | master/apply_patch Patch failed to apply |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
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? 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@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; > > -- > 2.11.0 > Amitay.
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@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.
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;
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@popple.id.au> --- libpdbg/target.c | 7 ++++++- libpdbg/target.h | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-)