[Pdbg] [PATCH] libpdbg: ensure thread state validity, only print requested targets

[Alistair Popple]

On Thursday, 9 August 2018 5:53:14 PM AEST Nicholas Piggin wrote:
> On Thu, 09 Aug 2018 16:46:09 +1000
> Alistair Popple <alistair at popple.id.au> wrote:
> 
> > Thanks Nick,
> > 
> > On Thursday, 9 August 2018 2:28:45 PM AEST Nicholas Piggin wrote:
> > > Thread state is not queried from the target each time, but cached.
> > > This patch adds a valid flag in the thread state cache to ensure
> > > this is being probed proprly before use.  
> > 
> > I really need to add some comments/documentation but there is already a
> > pdbg_target_status() function which is supposed to indicate if a particular
> > target has been initialised or not (and therefore if thread_status() is
> > initialised).
> > 
> > So rather than adding a target specific valid flag I think it would be best to
> > just check that. There is also an implicit (ie. undocumented :-) rule that
> > target functions other than target_probe() should only be called on targets with
> > status == PDBG_TARGET_ENABLED.
> 
> Oh it's not the target status field but the thread_state one (which is
> of course target specific), so I think it's okay. Using the target
> status maybe works, but we pass around that thread_state structure
> without the target in some places, also could target status be used
> with things like ramming that wants to check status of non-target
> siblings? (not that I've added the valid check there, but I should)

Not quite sure I follow, but what I meant is we should assume:

1) If pdbg_target_status(target) == PDBG_TARGET_ENABLED then
   thread_status(target) will always return a valid thread_state.

2) If pdbg_target_status(target) != PDBG_TARGET_ENABLED then we shouldn't be
   calling thread_status(target) as the target hasn't been initialised and
   doesn't even exist so it can't have a status.

Therefore I didn't quite understand why we needed a seperate bit tracking
thread_state validity as it should always be valid or non-existant.

For checking status of all thread siblings on a given core you could do
something like:

pdbg_for_each_target("thread", core, sibling_thread) {
	assert(pdbg_target_probe(sibling_thread) == PDBG_TARGET_ENABLED);
	thread_state = thread_status(sibling_thread);
}

This is effectively the check we do in p9_ram_setup() today, although that code
is a little old and needs some cleanup to make it look more like the above which
is what I'm doing now.

Thanks again,

Alistair

> Thanks,
> Nick
>