[Pdbg] [PATCH 8/9] main: Convert gdbserver command to path based targeting

[Amitay Isaacs]

On Thu, 2018-11-22 at 13:54 +1100, Rashmica Gupta wrote:
> On Tue, 2018-11-20 at 15:19 +1100, Amitay Isaacs wrote:
> > Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> > ---
> >  src/pdbgproxy.c | 72 +++++++++++++++++++++++----------------------
> > ----
> >  1 file changed, 33 insertions(+), 39 deletions(-)
> > 
> > diff --git a/src/pdbgproxy.c b/src/pdbgproxy.c
> > 
> 
> ...
> 
> >  static int gdbserver(uint16_t port)
> >  {
> > -	struct pdbg_target *target = NULL;
> > +	struct pdbg_target *target, *adu, *thread = NULL;
> >  	uint64_t msr;
> >  	int rc;
> >  
> > -	for_each_class_target("thread", target) {
> > -		if (!target_selected(target))
> > +	for_each_path_target_class("thread", target) {
> > +		if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> >  			continue;
> > -		if (pdbg_target_probe(target) == PDBG_TARGET_ENABLED)
> > -			break;
> > +
> > +		if (!thread) {
> > +			thread = target;
> > +		} else {
> > +			fprintf(stderr, "GDB server cannot be run on
> > multiple threads at once.\n");
> 
> Nice
> 
> 
> > +			return 0;
> > +		}
> >  	}
> > -	if (!target->class)
> > -		return -1;
> 
> This check was to return if the user specified a target that didn't
> exist (rather than segfault). Can you add a check for that here?
> Maybe:
> 
> if (!thread)
> 	return -1;
> 
> Would also be good to add in a useful message like "didn't find a
> valid
> thread".

Good catch.  Will add the check.

> 
> Otherwise, this looks good to me :)
> 
> 
> > -	assert(!strcmp(target->class, "thread"));
> > +
> > +	//
> >  	// Temporary until I can get this working a bit smoother on p9
> > -	if (strcmp(target->compatible, "ibm,power8-thread")) {
> > +	if (strcmp(thread->compatible, "ibm,power8-thread")) {
> >  		PR_ERROR("GDBSERVER is only tested on POWER8\n");
> >  		return -1;
> >  	}
> >  
> >  	/* Check endianess in MSR */
> > -	rc = ram_getmsr(target, &msr);
> > +	rc = ram_getmsr(thread, &msr);
> >  	if (rc) {
> >  		PR_ERROR("Couldn't read the MSR. Are all threads on
> > this chiplet quiesced?\n");
> >  		return 1;
> >  	}
> >  	littleendian = 0x01 & msr;
> >  
> > -	gdbserver_start(target, port);
> > +	/* Select ADU target */
> > +	pdbg_for_each_class_target("adu", adu) {
> > +		if (pdbg_target_probe(adu) == PDBG_TARGET_ENABLED)
> > +			break;
> > +	}
> > +
> > +	if (!adu) {
> > +		PR_ERROR("No ADU found\n");
> > +		return 0;
> > +	}
> > +
> > +	gdbserver_start(thread, adu, port);
> >  	return 0;
> >  }
> >  #else
> > -- 
> > 2.19.1
> > 

Amitay.
-- 

You're currently going through a difficult transition period called "Life."