[Pdbg] [PATCH 5/7] libpdbg: Add "nonexistant" pdbg target status.
Cyril Bur
cyrilbur at gmail.com
Fri Mar 16 14:21:16 AEDT 2018
On Fri, 2018-03-16 at 13:32 +1100, Alistair Popple wrote:
> On Wednesday, 7 March 2018 4:49:04 PM AEDT Cyril Bur wrote:
> > When libpdbg runs though the device tree and calls probe() on all the
> > targets it marks any node (and all its children) which fails to probe
> > as "disabled".
> >
> > When pdbg recieves arguments to specifiy a pib, core or thread (-p -c
> > -t) it runs though the device tree and marks nodes which do not match
> > the specified -p -c or -t as "disabled".
> >
> > These two scenarios are related - that is we don't want to be operating
> > on any disabled nodes. However, they are not the same.
> >
> > Consider HTM wanting to know if the machine has powersave disabled. A
> > simple way would be to loop through all the threads and if any of them
> > are not in "active" state then powersave must be enabled. In this
> > scenario HTM want to be able to get the status of every thread on the
> > machine regardless of -p -c -t flags but if the machine isn't fully
> > speced out and has cores which didn't probe, HTM wants to know to
> > ignore those threads.
>
> It's not clear to me how this will work. At the moment all the threads should
> get marked as disabled anyway unless they're selected on the command line, so
> they will never get set to "nonexistant". Or is the idea that you always pass
> "-a" for HTM to work around our terrible UI anyway?
>
Hm commit message wasn't clear. So I need to be able to distinguish
between cores and threads that physically don't exist on a system and
those which were 'deselected' by way of -p -c -t flags.
Currently, in a for_each_thread() there isn't a way to distinguish
between a thread that exists but was 'deselected' by way of -p -c -t
(and therefore can be acted on) and one attached to a core which failed
to probe (and therefore doesn't physically exist) and so cannot be
acted on.
HTM code wants to be able to get the status of every single thread in
the system regardless of -p -c -t flags (that is, it would like to
pretend -a was passed) but not attempt to get the status of a non
existent thread (this ends badly).
For extra clarity, currently if -a is passed the number of disabled
threads should equal the number of (with patch applied) "nonexistant"
threads.
With this patch if -p -c -t is passed the number of disabled threads
will simply be the number of threads in the difference between -a and
-p -c -t. The number of "nonexistant" will be the same as the paragraph
above.
> > This patch introduces a "nonexistant" pdbg target status which is set
> > during libpdbg probe() time. Later pdbg can mark nodes to targets that
> > do 'exist' but it would like to avoid as "disabled".
> >
> > Signed-off-by: Cyril Bur <cyrilbur at gmail.com>
> > ---
> > libpdbg/libpdbg.c | 2 ++
> > libpdbg/libpdbg.h | 2 +-
> > libpdbg/target.c | 10 +++++-----
> > src/main.c | 6 +++---
> > src/main.h | 3 ++-
> > 5 files changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c
> > index 6194048..7f8a935 100644
> > --- a/libpdbg/libpdbg.c
> > +++ b/libpdbg/libpdbg.c
> > @@ -69,6 +69,8 @@ enum pdbg_target_status pdbg_target_status(struct pdbg_target *target)
> > return PDBG_TARGET_DISABLED;
> > else if (!strcmp(p->prop, "hidden"))
> > return PDBG_TARGET_HIDDEN;
> > + else if (!strcmp(p->prop, "nonexistant"))
> > + return PDBG_TARGET_NONEXISTANT;
> > else
> > assert(0);
> > }
> > diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
> > index 8dec016..d6dbcf0 100644
> > --- a/libpdbg/libpdbg.h
> > +++ b/libpdbg/libpdbg.h
> > @@ -9,7 +9,7 @@ struct pdbg_taget *pdbg_root_target;
> > /* loops/iterators */
> > struct pdbg_target *__pdbg_next_target(const char *klass, struct pdbg_target *parent, struct pdbg_target *last);
> > struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent, struct pdbg_target *last);
> > -enum pdbg_target_status {PDBG_TARGET_ENABLED, PDBG_TARGET_DISABLED, PDBG_TARGET_HIDDEN};
> > +enum pdbg_target_status {PDBG_TARGET_ENABLED, PDBG_TARGET_DISABLED, PDBG_TARGET_HIDDEN, PDBG_TARGET_NONEXISTANT};
> >
> > #define pdbg_for_each_target(class, parent, target) \
> > for (target = __pdbg_next_target(class, parent, NULL); \
> > diff --git a/libpdbg/target.c b/libpdbg/target.c
> > index 1eb85bb..790cc6e 100644
> > --- a/libpdbg/target.c
> > +++ b/libpdbg/target.c
> > @@ -300,7 +300,7 @@ void pdbg_targets_init(void *fdt)
> > }
> >
> > /* Disable a node and all it's children */
> > -static void disable_node(struct dt_node *dn)
> > +static void deprobe(struct dt_node *dn)
> > {
> > struct dt_node *next;
> > struct dt_property *p;
> > @@ -309,9 +309,9 @@ static void disable_node(struct dt_node *dn)
> > if (p)
> > dt_del_property(dn, p);
> >
> > - dt_add_property_string(dn, "status", "disabled");
> > + dt_add_property_string(dn, "status", "nonexistant");
> > dt_for_each_child(dn, next)
> > - disable_node(next);
> > + deprobe(next);
> > }
> >
> > static void _target_probe(struct dt_node *dn)
> > @@ -326,13 +326,13 @@ static void _target_probe(struct dt_node *dn)
> > }
> >
> > p = dt_find_property(dn, "status");
> > - if ((p && !strcmp(p->prop, "disabled")) || (dn->target->probe && (rc = dn->target->probe(dn->target)))) {
> > + if ((p && (!strcmp(p->prop, "disabled") || !strcmp(p->prop, "nonexistant"))) || (dn->target->probe && (rc = dn->target->probe(dn->target)))) {
> > if (rc)
> > PR_DEBUG("not found\n");
> > else
> > PR_DEBUG("disabled\n");
> >
> > - disable_node(dn);
> > + deprobe(dn);
> > } else {
> > PR_DEBUG("success\n");
> > }
> > diff --git a/src/main.c b/src/main.c
> > index 121e4ad..ae29bfa 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -269,7 +269,7 @@ int for_each_child_target(char *class, struct pdbg_target *parent,
> > index = pdbg_target_index(target);
> > assert(index != -1);
> > status = pdbg_target_status(target);
> > - if (status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_HIDDEN)
> > + if (status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_HIDDEN || status == PDBG_TARGET_NONEXISTANT)
> > continue;
> >
> > rc += cb(target, index, arg1, arg2);
> > @@ -289,7 +289,7 @@ int for_each_target(char *class, int (*cb)(struct pdbg_target *, uint32_t, uint6
> > index = pdbg_target_index(target);
> > assert(index != -1);
> > status = pdbg_target_status(target);
> > - if (status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_HIDDEN)
> > + if (status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_HIDDEN || status == PDBG_TARGET_NONEXISTANT)
> > continue;
> >
> > rc += cb(target, index, arg1, arg2);
> > @@ -420,7 +420,7 @@ void print_target(struct pdbg_target *target, int level)
> > enum pdbg_target_status status;
> >
> > status = pdbg_target_status(target);
> > - if (status == PDBG_TARGET_DISABLED)
> > + if (status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_NONEXISTANT)
> > return;
> >
> > if (status == PDBG_TARGET_ENABLED) {
> > diff --git a/src/main.h b/src/main.h
> > index 71599ee..f9ea77c 100644
> > --- a/src/main.h
> > +++ b/src/main.h
> > @@ -21,7 +21,8 @@ enum backend { FSI, I2C, KERNEL, FAKE, HOST };
> >
> > static inline bool target_is_disabled(struct pdbg_target *target)
> > {
> > - return pdbg_target_status(target) == PDBG_TARGET_DISABLED;
> > + return pdbg_target_status(target) == PDBG_TARGET_DISABLED ||
> > + pdbg_target_status(target) == PDBG_TARGET_NONEXISTANT;
> > }
> >
> > /* Returns the sum of return codes. This can be used to count how many targets the callback was run on. */
> >
>
>
More information about the Pdbg
mailing list