RE: [rfc] remote, btrace: add branch tracing protocol to Qbtrace packet

["Metzger, Markus T" <markus.t.metzger@intel.com>]

> -----Original Message-----
> From: Jan Kratochvil [mailto:jan.kratochvil@redhat.com]
> Sent: Wednesday, February 27, 2013 5:48 PM


> > Eventually, I want GDB to collect all the supported btrace methods and allow
> > the user to specify one of the supported methods, i.e.
> >
> > set record btrace method bts
> > set record btrace method lbr
> 
> From the user interface point of view I expected rather:
> 	record full
> 	record btrace
> 	record lbr
> 
> It is already one level of "configuration", two levels of configuration make
> it a bit too complicated for the user IMO.  Sure the code inside GDB would be
> shared between "record-btrace" and "record-lbr" targets but that is hidden
> from the user.

That should be possible, as well.


> > I would create the respective sub-commands dynamically after querying the
> > target.  Unfortunately, we cannot remove the commands again on disconnect.
> 
> I do not think there should be dynamically created commands, it is not
> established in GDB.

I just thought it's more convenient for the user to only be offered commands
if the feature is available.  I'm OK to add those commands unconditionally.

Btw, I think target sub-commands are created dynamically when you call add_target.


> > As default, GDB will pick one of the supported methods, say, the first in the
> > qSupported response.
> 
> Do you expect more "btrace" variants than "bts" and "lbr"?

I would expect other processors to offer similar features.


> If "lbr" exists it should be enabled by default without asking the user.

We could do that until the user requests BTS tracing or full tracing. This would
mean that there would be a record target pushed implicitly. With the current
logic, this would prevent displaced stepping and would require an explicit
record stop before another record target can be pushed.

I think we should discuss the details once we start sending patches to enable
LBR tracing.


> For "bts" one has to type "record btrace" as it is a noticeable slowdown.

Agreed.


> > There will be separate entries for each supported method in the qSupported
> > response, i.e.
> >
> > ...:Qbtrace:bts+:Qbtrace:lbr+:...
> 
> colons vs. semicolons mistaken:
> 
>   ...;Qbtrace:bts+;Qbtrace:lbr+;...
> 
> 
> No preference for the qSupported details, I am OK with your proposal myself,
> "qXfer" gdbserver->gdb feature is designed that way already.

Pedro objected and rather wanted a comma-separated list, i.e.

....;Qbtrace=bts,lbr;...

I'm fine either way. For the way I proposed and what seems to be your preference,
as well, I already found code in GDB to handle it. For Pedro's preferred way, I have
not found anything that I could reuse.

I would like to get the minimal changes in with the btrace series so we don't need
to rework the protocol later on and are forced to maintain backwards compatibility.
I do not intend to implement full support for different branch trace recording
methods right now, since we currently only have one. We will add that support
when there's a need.

Pedro did not respond to my reply. I don't know how to proceed, here.

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052