[PATCH 2/8] add btrace coresight related commands

[Metzger, Markus T]

Hello Zied,

> The parameters needed for decoding the traces are more complex than the one for decoding PT traces. So an implementation for allowing the user a full flexibility in changing them will be very complex. Therefore, I did not provide a mean for changing them manually. I have added CV_ARM to the list of btrace_cpu_vendor to be able to show it. It can be removed if assessed as not needed.

If we're not supporting any CPU identification for ARM, then adding
the vendor enum seems a bit pointless.


> This patch extends the commands needed for using branch tracing
> with ARM CoreSight traces.
> Those commands are:
> set record btrace etm sink
> set record btrace etm size
> record btrace etm
> You'd want to describe them in gdb/doc/gdb.texinfo and announce the
> new commands in gdb/NEWS.
> 
> [Zied] set record btrace etm buffer-size : an error in the description.
> both commands are documented in patch 8/8

I didn't get that far.  It makes sense to document things as they get
Introduced so we're not missing anything or documenting anything
wrongly as patches get modified.


> +          printf_unfiltered ("btrace cpu is 'ARM'.\n");
> The idea behind 'set record btrace cpu' is to allow the user to overwrite
> the cpu identifier for enabling errata workarounds.  The main use-case is
> when libipt does not know the cpu so it cannot enable workarounds
> automatically.
> 
> 
> [Zied] yes, I initially went into the direction of letting the user change all parameters needed for decoding etm traces, but considering the complexity and the fact that this will be error prone, I reverted back to a minimum support by just printing that this is an ARM core. 

We might just ignore it if that's not needed for ARM.  We would want to say
in the help text that this is not used for ARM.

 
> +This format may not be available on all processors."),
> +	   &record_btrace_cmdlist);
> +  add_alias_cmd ("etm", "btrace etm", class_obscure, 1, &record_cmdlist);
> This alias wasn't mentioned in the list of new commands.
> 
> [Zied] See patch 8/8

I was referring to the commit message.

 
> diff --git a/gdbsupport/btrace-common.h b/gdbsupport/btrace-common.h
> index 26d26ec957f..7f3cabfde77 100644
> --- a/gdbsupport/btrace-common.h
> +++ b/gdbsupport/btrace-common.h
> @@ -77,7 +80,10 @@ enum btrace_cpu_vendor
>    CV_INTEL,
> 
>    /* AMD.  */
> -  CV_AMD
> +  CV_AMD,
> +
> +  /* ARM. */
> +  CV_ARM
> Does ARM also use model/family/stepping to identify their CPUs?
> 
> If not, please also extend struct btrace_cpu to make it a union
> tagged with the vendor field.
> 
> [Zied] I am just using this to stub the show commands for ARM. 

The default is 'auto'.  Wouldn't that fix, here?

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928