Re: [PATCH v6 3/7] start/stop btrace with coresight etm and parse etm buffer. nat independant

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

On 5/31/21 6:33 PM, Zied Guermazi wrote:
> This patch extend branch tracing by adding the functions needed
> to collect parameters for decoding ETM traces and decoding them.
> 
> gdb/ChangeLog
> 
> 	* arch/arm.h: Add defines for exception numbers as encoded
> 	in the ETM traces.
> 	* btrace.c (ftrace_new_function): log new function.
> 	(ftrace_remove_last_insn): New.
> 	(cs_etm_get_etmv3_config): New.
> 	(cs_etm_get_etmv4_config): New.
> 	(cs_etm_get_isa_flag): New.
> 	(cs_etm_get_instruction_class): New.
> 	(cs_etm_update_btrace_with_inst_range): New.
> 	(cs_etm_update_btrace_with_exception): New.
> 	(cs_etm_update_btrace_with_trace_on): New.
> 	(cs_etm_trace_element_callback): New.
> 	(cs_etm_free_decoder): New.
> 	(cs_etm_create_decoder): New.
> 	(btrace_etm_readmem_callback): New.
> 	(cs_etm_add_mem_access_callback): New.
> 	(cs_etm_process_data_block): New.
> 	(btrace_print_all): New.
> 	(btrace_compute_ftrace_etm): New.
> 	(btrace_compute_ftrace_1): add handling of CoreSight traces.
> 	(btrace_enable): add error message if ETM unavailable.
> 	(btrace_stitch_trace): add handling of CoreSight traces.
> 	(maint_info_btrace_cmd): add handling of CoreSight trace format.
> 	* btrace.h (btrace_insn_flag): add ARM ISA flags.
> 	* record-btrace.c (cs_etm_reconstruct_cpsr_iset_state): New.
> 	(record_btrace_target::fetch_registers): fetch cpsr register
> 	from insn->flags when applicable.
> 
> gdbsupport/ChangeLog
> 
> 	* btrace-common.h (cs_etmv3_trace_params): New
> 	(cs_etmv4_trace_params): New.
> 	(cs_etm_trace_params): New.
> 	(cs_etm_decoder_params): New
> 	(btrace_data_etm_config): New.
> 	(btrace_data_etm): New.
> 	(btrace_data): add a btrace_data_etm.
> 	* btrace-common.cc (btrace_data::fini): handle CoreSight traces.
> 	(btrace_data::empty): handle CoreSight traces.
> 	(btrace_data_append): handle CoreSight traces.
> ---
>   gdb/arch/arm.h              |  33 ++
>   gdb/btrace.c                | 618 ++++++++++++++++++++++++++++++++++++
>   gdb/btrace.h                |  16 +-
>   gdb/record-btrace.c         |  57 +++-
>   gdbsupport/btrace-common.cc |  39 +++
>   gdbsupport/btrace-common.h  | 106 +++++++
>   6 files changed, 864 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
> index fa589fd0582..4d520fe906a 100644
> --- a/gdb/arch/arm.h
> +++ b/gdb/arch/arm.h
> @@ -150,6 +150,39 @@ enum arm_m_profile_type {
>   #define BranchDest(addr,instr) \
>     ((CORE_ADDR) (((unsigned long) (addr)) + 8 + (sbits (instr, 0, 23) << 2)))
>   
> +/* Exception numbers as encoded in ETMv3.4 for ARMv7-M processors.
> +   See IHI0014Q_etm_architecture_spec table 7.11.  */
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_USAGE_FAULT    		0x009
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_NMI     			0x00a
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_SVC     			0x00b
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_DEBUG_MONITOR  		0x00c
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_MEM_USAGE      		0x00d
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_PEND_SV 			0x00e
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_SYS_TICK			0x00f
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_PROCESSOR_RESET		0x011
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_HARD_FAULT     		0x013
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_BUS_FAULT      		0x015
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_IRQ(n) \
> +  (n == 0 ? 0x008 : n < 8 ? n : n + 0x010)
> +#define CS_ETMV3_4_CORTEX_M_EXCEPTION_IS_IRQ(n) \
> +  ((n > 0x000 && n < 0x009) || (n > 0x017 && n < 0x200))
> +
> +/* Exception numbers as encoded in ETMv3.4 for non ARMv7-M processors.
> +   See IHI0014Q_etm_architecture_spec table 7.12.  */
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_HALTING_DEBUG		0x01
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_SECURE_MONITOR_CALL  	0x02
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_ENTRY_TO_HYP_MODE    	0x03
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_ASYNCHRONOUS_DATA_ABORT      0x04
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_JAZELLE_THUMBEE_CHECK	0x05
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_PROCESSOR_RESET      	0x08
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_UNDEFINED_INSTRUCTION	0x09
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_SUPERVISOR_CALL      	0x0a
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_PREFETCH_ABORT_SW_BKPT	0x0b
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_SYNCHRONOUS_DATA_ABORT_SW_WP 0x0c
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_GENERIC      		0x0d
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_IRQ   			0x0e
> +#define CS_ETMV3_4_CORTEX_A_R_EXCEPTION_FIQ   			0x0f
> +
>   /* Forward declaration.  */
>   struct regcache;
>   
> diff --git a/gdb/btrace.c b/gdb/btrace.c
> index 5e689c11d4b..2676389b63e 100644
> --- a/gdb/btrace.c
> +++ b/gdb/btrace.c
> @@ -35,6 +35,7 @@
>   #include "gdbcmd.h"
>   #include "cli/cli-utils.h"
>   #include "gdbarch.h"
> +#include "arch/arm.h"
>   
>   /* For maintenance commands.  */
>   #include "record-btrace.h"
> @@ -671,6 +672,38 @@ ftrace_update_insns (struct btrace_function *bfun, const btrace_insn &insn)
>       ftrace_debug (bfun, "update insn");
>   }
>   
> +#if defined (HAVE_LIBOPENCSD_C_API)
> +/* Remove last instruction from BFUN's list.
> +   This function is not generic and does not undo functions chaining.
> +   When adding an instruction after using it, the caller must ensure
> +   that the instruction produces the same chaining.
> +   An example of good case, is when the same removed instruction
> +   is added later.  */
> +
> +static void
> +ftrace_remove_last_insn (struct btrace_thread_info *btinfo)
> +{
> +  /* If we didn't have a function, we return.  */
> +  if (btinfo->functions.empty ())
> +    return;
> +
> +  struct btrace_function *bfun = &btinfo->functions.back ();
> +  /* If we had a gap before, we return.  */
> +  if (bfun->errcode != 0)
> +    return;
> +
> +  if (!bfun->insn.empty ())
> +    bfun->insn.pop_back ();
> +  else
> +    {
> +      /* A valid function must have at least one instruction.  */
> +      internal_error (__FILE__, __LINE__,
> +		       _("Attempt to remove last instruction"
> +			 "from an empty function"));
> +    }
> +}
> +#endif /* defined (HAVE_LIBOPENCSD_C_API) */
> +
>   /* Classify the instruction at PC.  */
>   
>   static enum btrace_insn_class
> @@ -1505,6 +1538,574 @@ btrace_compute_ftrace_pt (struct thread_info *tp,
>   
>   #endif /* defined (HAVE_LIBIPT)  */
>   
> +#if defined (HAVE_LIBOPENCSD_C_API)
> +
> +/* This structure holds the status and the context for decoding ETM traces.
> +   It is also used in the ETM trace element callback to get the context.  */
> +struct cs_etm_decoder
> +{
> +  /* The tree representing CoreSight architecture in the SoC.  */
> +  dcd_tree_handle_t dcd_tree;
> +
> +  /* Callback function to allow the decoder to access program memory.  */
> +  Fn_MemAcc_CB mem_access;
> +
> +  /* thread_info of traced thread.  */
> +  struct thread_info *t_info;
> +
> +  /* Returned value of previously processed block.  */
> +  ocsd_datapath_resp_t prev_return;
> +
> +  /* ARM architecture version of associated core.  */
> +  ocsd_arch_version_t arch_version;
> +
> +  /* List of gaps in the execution record.  */
> +  std::vector<unsigned int> &gaps;
> +};
> +
> +/* Fills a ocsd_etmv3_cfg from a cs_etm_trace_params.  */
> +
> +static void
> +cs_etm_get_etmv3_config (const struct cs_etm_trace_params *params,
> +			  ocsd_etmv3_cfg *config)
> +{
> +  config->reg_idr = params->etmv3.reg_idr;
> +  config->reg_ctrl = params->etmv3.reg_ctrl;
> +  config->reg_ccer = params->etmv3.reg_ccer;
> +  config->reg_trc_id = params->etmv3.reg_trc_id;
> +  config->arch_ver = (ocsd_arch_version_t)params->arch_ver;
> +  config->core_prof = (ocsd_core_profile_t)params->core_profile;
> +}
> +
> +/* Fills a ocsd_etmv4_cfg from a cs_etm_trace_params.  */
> +
> +static void
> +cs_etm_get_etmv4_config (const struct cs_etm_trace_params *params,
> +			  ocsd_etmv4_cfg *config)
> +{
> +  config->reg_configr = params->etmv4.reg_configr;
> +  config->reg_traceidr = params->etmv4.reg_traceidr;
> +  config->reg_idr0 = params->etmv4.reg_idr0;
> +  config->reg_idr1 = params->etmv4.reg_idr1;
> +  config->reg_idr2 = params->etmv4.reg_idr2;
> +  config->reg_idr8 = params->etmv4.reg_idr8;
> +  config->reg_idr9 = 0;
> +  config->reg_idr10 = 0;
> +  config->reg_idr11 = 0;
> +  config->reg_idr12 = 0;
> +  config->reg_idr13 = 0;
> +  config->arch_ver = (ocsd_arch_version_t)params->arch_ver;
> +  config->core_prof = (ocsd_core_profile_t)params->core_profile;
> +}
> +
> +/* Set the instruction flag to track ISA mode of ARM processor.  */
> +
> +static btrace_insn_flags
> +cs_etm_get_isa_flag (const ocsd_generic_trace_elem *elem)
> +{
> +  switch (elem->isa)
> +    {
> +    case ocsd_isa_arm:
> +      return BTRACE_INSN_FLAG_ISA_ARM;
> +
> +    case ocsd_isa_thumb2:
> +      return BTRACE_INSN_FLAG_ISA_THUMB2;
> +
> +    case ocsd_isa_aarch64:
> +      return BTRACE_INSN_FLAG_ISA_AARCH64;
> +
> +    case ocsd_isa_tee:
> +      return BTRACE_INSN_FLAG_ISA_TEE;
> +
> +    case ocsd_isa_jazelle:
> +      return BTRACE_INSN_FLAG_ISA_JAZELLE;
> +
> +    case ocsd_isa_custom:
> +      return BTRACE_INSN_FLAG_ISA_CUSTOM;
> +
> +    case ocsd_isa_unknown:
> +      return BTRACE_INSN_FLAG_ISA_UNKNOWN;
> +
> +    default:
> +      internal_error (__FILE__, __LINE__,
> +		       _("Undefined elem->isa value returned by OpenCsd."));
> +    }
> +}
> +
> +/* Returns the instruction class.  */
> +
> +static enum btrace_insn_class
> +cs_etm_get_instruction_class (const ocsd_generic_trace_elem *elem)
> +{
> +  switch (elem->last_i_type)
> +    {
> +    case OCSD_INSTR_BR:
> +    case OCSD_INSTR_BR_INDIRECT:
> +      switch (elem->last_i_subtype)
> +	{
> +	case OCSD_S_INSTR_V8_RET:
> +	case OCSD_S_INSTR_V8_ERET:
> +	case OCSD_S_INSTR_V7_IMPLIED_RET:
> +	  return BTRACE_INSN_RETURN;
> +
> +	case OCSD_S_INSTR_BR_LINK:
> +	  return BTRACE_INSN_CALL;
> +
> +	case OCSD_S_INSTR_NONE:
> +	  return BTRACE_INSN_JUMP;
> +	}
> +      return BTRACE_INSN_OTHER;
> +
> +    case OCSD_INSTR_ISB:
> +    case OCSD_INSTR_DSB_DMB:
> +    case OCSD_INSTR_WFI_WFE:
> +    case OCSD_INSTR_OTHER:
> +    default:
> +      return BTRACE_INSN_OTHER;
> +    }
> +}
> +
> +/* Update btrace in the case of an instruction range.  */
> +
> +static void
> +cs_etm_update_btrace_with_inst_range (const struct cs_etm_decoder *etm_decoder,
> +				       const ocsd_generic_trace_elem *elem)
> +{
> +  gdb_assert (elem->elem_type == OCSD_GEN_TRC_ELEM_INSTR_RANGE);
> +
> +  struct thread_info *tp = etm_decoder->t_info;
> +  struct btrace_thread_info *btinfo = &tp->btrace;
> +  struct gdbarch *gdbarch = target_gdbarch ();
> +  struct btrace_insn insn;
> +
> +  insn.iclass = BTRACE_INSN_OTHER;
> +  insn.pc = elem->st_addr;
> +  for (int i = 0; i< elem->num_instr_range; i++)
> +    {
> +      try
> +	{
> +	  insn.size = gdb_insn_length (gdbarch, insn.pc);
> +	}
> +      catch (const gdb_exception_error &err)
> +	{
> +	  error (_("Failed to get the size of the instruction."));
> +	}
> +
> +      struct btrace_function *bfun = ftrace_update_function (btinfo, insn.pc);
> +      if (etm_decoder->arch_version == ARCH_V7)
> +	insn.flags = cs_etm_get_isa_flag (elem);
> +
> +      if (i == elem->num_instr_range -1)
> +	insn.iclass = cs_etm_get_instruction_class (elem);
> +
> +      ftrace_update_insns (bfun, insn);
> +      insn.pc = insn.pc + insn.size;
> +    }
> +}
> +
> +/* Update btrace in the case of an exception.  */
> +
> +static void
> +cs_etm_update_btrace_with_exception (const struct cs_etm_decoder *etm_decoder,
> +				      const ocsd_generic_trace_elem *elem)
> +{
> +  gdb_assert (elem->elem_type == OCSD_GEN_TRC_ELEM_EXCEPTION);
> +
> +  struct thread_info *tp = etm_decoder->t_info;
> +  struct btrace_thread_info *btinfo = &tp->btrace;
> +
> +  /* Handle the implementation of breakpoints in gdb for arm (v7) architecture
> +     using undefined instructions.  */
> +  if (etm_decoder->arch_version == ARCH_V7)
> +    {
> +      if (elem->exception_number
> +	   == CS_ETMV3_4_CORTEX_A_R_EXCEPTION_UNDEFINED_INSTRUCTION)
> +	{
> +	  DEBUG ("handle breakpoints implementation in gdb for ARMv7");
> +	  ftrace_remove_last_insn (btinfo);
> +	}
> +    }
> +}
> +
> +/* Update btrace in the case of a trace on.  */
> +
> +static void
> +cs_etm_update_btrace_with_trace_on (const struct cs_etm_decoder *etm_decoder,
> +				     const ocsd_generic_trace_elem *elem)
> +{
> +  gdb_assert (elem->elem_type == OCSD_GEN_TRC_ELEM_TRACE_ON);
> +
> +  if (elem->trace_on_reason != TRACE_ON_NORMAL)
> +    {
> +      struct thread_info *tp = etm_decoder->t_info;
> +      struct btrace_thread_info *btinfo = &tp->btrace;
> +      ftrace_new_gap (btinfo, elem->trace_on_reason, etm_decoder->gaps);
> +    }
> +}
> +
> +/* Callback function when a ocsd_generic_trace_elem is emitted.  */
> +
> +static ocsd_datapath_resp_t
> +cs_etm_trace_element_callback (const void *context,
> +				const ocsd_trc_index_t index,
> +				const uint8_t trace_chan_id,
> +				const ocsd_generic_trace_elem *elem)
> +{
> +  if (record_debug != 0)
> +    {
> +      char str_buffer[128];
> +      if (ocsd_gen_elem_str (elem, str_buffer, 128) == OCSD_OK)
> +	DEBUG ("ETM trace_element: index= %s, channel= 0x%x, %s",
> +	       pulongest (index), trace_chan_id, str_buffer);
> +    }
> +
> +  const struct cs_etm_decoder *etm_decoder = (struct cs_etm_decoder *)context;
> +  gdb_assert (etm_decoder->t_info != nullptr);
> +
> +  switch (elem->elem_type)
> +    {
> +    case OCSD_GEN_TRC_ELEM_TRACE_ON:
> +      cs_etm_update_btrace_with_trace_on (etm_decoder, elem);
> +      break;
> +
> +    case OCSD_GEN_TRC_ELEM_INSTR_RANGE:
> +      cs_etm_update_btrace_with_inst_range (etm_decoder, elem);
> +      break;
> +
> +    case OCSD_GEN_TRC_ELEM_EXCEPTION:
> +      cs_etm_update_btrace_with_exception (etm_decoder, elem);
> +      break;
> +
> +    /* Not yet handled types, but may be supported in the future.  */
> +    case OCSD_GEN_TRC_ELEM_UNKNOWN:
> +    case OCSD_GEN_TRC_ELEM_EO_TRACE:
> +    case OCSD_GEN_TRC_ELEM_NO_SYNC:
> +    case OCSD_GEN_TRC_ELEM_EXCEPTION_RET:
> +    case OCSD_GEN_TRC_ELEM_TIMESTAMP:
> +    case OCSD_GEN_TRC_ELEM_PE_CONTEXT:
> +    case OCSD_GEN_TRC_ELEM_ADDR_NACC:
> +    case OCSD_GEN_TRC_ELEM_CYCLE_COUNT:
> +    case OCSD_GEN_TRC_ELEM_ADDR_UNKNOWN:
> +    case OCSD_GEN_TRC_ELEM_EVENT:
> +    case OCSD_GEN_TRC_ELEM_SWTRACE:
> +    case OCSD_GEN_TRC_ELEM_CUSTOM:
> +    default:
> +      break;
> +    }
> +  return OCSD_RESP_CONT;
> +}
> +
> +/* Callback to print error log.  */
> +
> +static void cs_etm_print_error_log (const void *p_context, const char *psz_msg_str, const int str_len)
> +{
> +    char string_buffer[128];
> +    memcpy (string_buffer, psz_msg_str, std::min(str_len, 127));
> +    if (str_len >127)

Space before operator and literal.

> +      string_buffer[127] = 0;
> +    else
> +      string_buffer[str_len] = 0;
> +    warning (_("Processing ETM traces failed with error: %s"), psz_msg_str);
> +}
> +
> +/* Create a cs_etm_decoder and initialize it.  */
> +
> +static ocsd_err_t
> +cs_etm_create_decoder (struct cs_etm_trace_params *t_params,
> +			struct cs_etm_decoder *decoder)
> +{
> +  ocsd_err_t errcode;
> +  const char *decoder_name;
> +  ocsd_etmv3_cfg config_etmv3;
> +  ocsd_etmv4_cfg config_etmv4;
> +  void *trace_config;
> +  switch (t_params->protocol)
> +    {
> +    case OCSD_PROTOCOL_ETMV3:
> +    case OCSD_PROTOCOL_PTM:
> +      cs_etm_get_etmv3_config (t_params, &config_etmv3);
> +      decoder_name = (t_params->protocol == OCSD_PROTOCOL_ETMV3)
> +		      ? OCSD_BUILTIN_DCD_ETMV3 : OCSD_BUILTIN_DCD_PTM;
> +      trace_config = &config_etmv3;
> +      decoder->arch_version = ARCH_V7;
> +      break;
> +
> +    case OCSD_PROTOCOL_ETMV4I:
> +      cs_etm_get_etmv4_config (t_params, &config_etmv4);
> +      decoder_name = OCSD_BUILTIN_DCD_ETMV4I;
> +      trace_config = &config_etmv4;
> +      decoder->arch_version = ARCH_V8;
> +      break;
> +
> +    default:
> +      decoder->arch_version = ARCH_UNKNOWN;
> +      warning (_("cs_etm_create_decoder: Unknown architecture version"));

It might be useful to print the hex value of the architecture version, 
in case we run into this and want to figure out what the value is.

> +      return OCSD_ERR_NOT_INIT;
> +
> +  }
> +  uint8_t csid;
> +  errcode = ocsd_dt_create_decoder (decoder->dcd_tree, decoder_name,
> +			      OCSD_CREATE_FLG_FULL_DECODER,
> +			      trace_config, &csid);
> +  if (errcode != OCSD_OK)
> +    {
> +      warning (_("ocsd_dt_create_decoder failed with error: %d"), errcode);
> +      return errcode;
> +    }
> +
> +  errcode = ocsd_dt_set_gen_elem_outfn (decoder->dcd_tree,
> +					 cs_etm_trace_element_callback,
> +					 decoder);
> +  if (errcode != OCSD_OK)
> +    {
> +      warning (_("ocsd_dt_set_gen_elem_outfn failed failed with error: %d"),
> +		   errcode);
> +      return errcode;
> +    }
> +
> +  errcode = ocsd_def_errlog_init (OCSD_ERR_SEV_ERROR, 1);
> +  if (errcode != OCSD_OK)
> +    {
> +      warning (_("ocsd_def_errlog_init failed failed with error: %d"),
> +		   errcode);
> +      return errcode;
> +    }
> +
> +  /* Initialize error printer.  */
> +  errcode = ocsd_def_errlog_config_output (C_API_MSGLOGOUT_FLG_NONE, nullptr);
> +  if (errcode != OCSD_OK)
> +    {
> +      warning (_("ocsd_def_errlog_init failed failed with error: %d"),
> +		   errcode);

In the error message, this should be "ocsd_def_errlog_config" instead of 
"ocsd_def_errlog_init".

> +      return errcode;
> +    }
> +
> +  errcode = ocsd_def_errlog_set_strprint_cb (decoder->dcd_tree, nullptr,
> +					      cs_etm_print_error_log);
> +  if (errcode != OCSD_OK)
> +    {
> +      warning (_("ocsd_def_errlog_set_strprint_cb failed failed with error: %d"),
> +		   errcode);
> +      return errcode;
> +    }
> +
> +  decoder->prev_return = OCSD_RESP_CONT;
> +  return OCSD_OK;
> +}
> +
> +/* Free a cs_etm_decoder.  */
> +
> +static void
> +cs_etm_free_decoder (struct cs_etm_decoder *decoder)
> +{
> +  if (decoder == nullptr)
> +    return;
> +
> +  ocsd_destroy_dcd_tree (decoder->dcd_tree);
> +  decoder->dcd_tree = nullptr;
> +  decoder->t_info = nullptr;
> +  xfree (decoder);
> +}
> +
> +/* Allocate a cs_etm_decoder and initialize it.  */
> +
> +static struct cs_etm_decoder *
> +cs_etm_alloc_decoder (struct thread_info *tp, int cpu_count,
> +		      const struct cs_etm_decoder_params * d_params,
> +		      std::vector<cs_etm_trace_params> * t_params)
> +{
> +  ocsd_dcd_tree_src_t src_type = OCSD_TRC_SRC_SINGLE;
> +  uint32_t deformatterCfgFlags = 0;
> +
> +  gdb_assert (d_params != nullptr);
> +
> +  if (d_params->formatted)
> +    src_type = OCSD_TRC_SRC_FRAME_FORMATTED;
> +  if (d_params->frame_aligned)
> +    deformatterCfgFlags |= OCSD_DFRMTR_FRAME_MEM_ALIGN;
> +  if (d_params->fsyncs)
> +    deformatterCfgFlags |= OCSD_DFRMTR_HAS_FSYNCS;
> +  if (d_params->hsyncs)
> +    deformatterCfgFlags |= OCSD_DFRMTR_HAS_HSYNCS;
> +  if (d_params->reset_on_4x_sync)
> +    deformatterCfgFlags |= OCSD_DFRMTR_RESET_ON_4X_FSYNC;
> +
> +  dcd_tree_handle_t dcdtree_handle;
> +  dcdtree_handle = ocsd_create_dcd_tree (src_type, deformatterCfgFlags);
> +
> +  if (dcdtree_handle == C_API_INVALID_TREE_HANDLE)
> +    {
> +      DEBUG ("ocsd_create_dcd_tree failed");
> +      return nullptr;
> +    }
> +  struct cs_etm_decoder *decoder;
> +
> +  decoder = (struct cs_etm_decoder*) xmalloc (sizeof (struct cs_etm_decoder));
> +  decoder->dcd_tree = dcdtree_handle;
> +
> +  for (int i = 0; i < cpu_count; i++)
> +    {
> +      ocsd_err_t errcode = cs_etm_create_decoder (&(t_params->at (i)), decoder);
> +      if (errcode != OCSD_OK)
> +	{
> +	  cs_etm_free_decoder (decoder);
> +	  return nullptr;
> +	}
> +    }
> +
> +  decoder->t_info = tp;
> +  return decoder;
> +}
> +
> +/* A callback function to allow the trace decoder to read the inferior's
> +   memory.
> +   Returns the number of bytes actually read, or 0 for access error  */

Nit. Period after the last sentence.

> +
> +static uint32_t
> +btrace_etm_readmem_callback (const void *p_context, const ocsd_vaddr_t address,
> +			      const ocsd_mem_space_acc_t mem_space,
> +			      const uint32_t reqBytes, uint8_t *byteBuffer)
> +{
> +  int result = (int) reqBytes;
> +  try
> +  {
> +      int errcode = target_read_code ((CORE_ADDR) address, byteBuffer, reqBytes);
> +      if (errcode != 0)
> +	result = 0;
> +  }
> +  catch (const gdb_exception_error &error)
> +  {
> +      result = 0;
> +  }
> +
> +  return result;
> +}
> +
> +/* Add memory access callback to the decoder.  */
> +
> +static ocsd_err_t
> +cs_etm_add_mem_access_callback (struct cs_etm_decoder *decoder,
> +				 CORE_ADDR start, CORE_ADDR end,
> +				 Fn_MemAcc_CB p_cb_func)
> +{
> +  ocsd_err_t error;
> +  error = ocsd_dt_add_callback_mem_acc (decoder->dcd_tree,
> +					 (ocsd_vaddr_t) start,
> +					 (ocsd_vaddr_t) end,
> +					 OCSD_MEM_SPACE_ANY,
> +					 p_cb_func, decoder);
> +  if (error == OCSD_OK)
> +    decoder->mem_access = p_cb_func;
> +
> +  return error;
> +}
> +
> +/* Process an etm traces data block.
> +   In case of an error it resets the decoder and tries to process further.  */
> +
> +static void
> +cs_etm_process_data_block (struct btrace_thread_info *btinfo,
> +			   struct cs_etm_decoder *decoder,
> +			   uint64_t index, const uint8_t *buf,
> +			   size_t len, size_t *consumed)
> +{
> +  ocsd_datapath_resp_t data_path_return = OCSD_RESP_CONT;
> +  size_t processed = 0;
> +  uint32_t count;
> +
> +  while (processed < len)
> +    {
> +      if (OCSD_DATA_RESP_IS_CONT (data_path_return))
> +	{
> +	  data_path_return = ocsd_dt_process_data (decoder->dcd_tree,
> +					OCSD_OP_DATA,
> +					index + processed, len - processed,
> +					&buf[processed], &count);
> +	  processed += count;
> +	
> +	}
> +      else if (OCSD_DATA_RESP_IS_WAIT (data_path_return))
> +	{
> +	  data_path_return = ocsd_dt_process_data (decoder->dcd_tree,
> +					OCSD_OP_FLUSH,
> +					0, 0, nullptr, nullptr);
> +	}
> +      else
> +	{
> +	  warning (_("error %d in ocsd_dt_process_data after processing %zu"),
> +		      data_path_return, processed);
> +	  ftrace_new_gap (btinfo, data_path_return, decoder->gaps);
> +	  data_path_return = ocsd_dt_process_data (decoder->dcd_tree,
> +					OCSD_OP_RESET,
> +					0, 0, nullptr, nullptr);
> +	  //todo: shall we increase processed? by 1, or 4 do we need to manually align to 16 bytes?

I'm not sure about the above. If you want to leave a hint for future 
developers (or for yourself), maybe you could elaborate a bit more on 
this. And use TODO, uppercase.

> +	}
> +      DEBUG_FTRACE ("ocsd_dt_process_data returned with error code %d."
> +			 " Consumed = 0x%zx",
> +			 data_path_return,
> +			 processed);
> +    }
> +  *consumed = processed;
> +}
> +
> +/* Print all functions in a btrace.  */
> +
> +static void
> +btrace_print_all (struct btrace_thread_info *btinfo)
> +{
> +  for (const btrace_function &function : btinfo->functions)
> +    ftrace_debug (&function, "");
> +}
> +
> +static void
> +btrace_compute_ftrace_etm (struct thread_info *tp,
> +			   const struct btrace_data_etm *btrace,
> +			   std::vector<unsigned int> &gaps)
> +{
> +  DEBUG_FTRACE ("btrace->size is 0x%x for thread %s",
> +		(unsigned int)(btrace->size), print_thread_id (tp));
> +  if (btrace->size == 0)
> +    return;
> +
> +  struct btrace_thread_info *btinfo = &tp->btrace;
> +  if (btinfo->functions.empty ())
> +    btinfo->level = 0;
> +
> +  struct cs_etm_decoder *decoder
> +   = cs_etm_alloc_decoder (tp, btrace->config.cpu_count,
> +			    &(btrace->config.etm_decoder_params),
> +			    btrace->config.etm_trace_params);
> +  if (decoder == nullptr)
> +    error (_("Failed to allocate ARM CoreSight ETM Trace decoder."));
> +
> +  ocsd_err_t ocsd_error
> +   = cs_etm_add_mem_access_callback (decoder,
> +				      (CORE_ADDR) 0x0L, (CORE_ADDR) -1L,
> +				      btrace_etm_readmem_callback);
> +  if (ocsd_error != OCSD_OK)
> +    error (_("Failed to add CoreSight Trace decoder memory access callback."));
> +
> +  size_t consumed;
> +  cs_etm_process_data_block (btinfo, decoder, 0, btrace->data, btrace->size,
> +				 &consumed);
> +  DEBUG_FTRACE ("CoreSight Trace processed. Consumed 0x%zx", consumed);
> +
> +  ftrace_compute_global_level_offset (btinfo);
> +  btrace_add_pc (tp);
> +  btrace_print_all (btinfo);
> +  cs_etm_free_decoder (decoder);
> +}
> +#else /*    defined (HAVE_LIBOPENCSD_C_API)    */
> +
> +static void
> +btrace_compute_ftrace_etm (struct thread_info *tp,
> +			   const struct btrace_data_etm *btrace,
> +			   std::vector<unsigned int> &gaps)
> +{
> +  internal_error (__FILE__, __LINE__, _("Unexpected branch trace format."));
> +}
> +#endif /*    defined (HAVE_LIBOPENCSD_C_API)    */
> +
>   /* Compute the function branch trace from a block branch trace BTRACE for
>      a thread given by BTINFO.  If CPU is not NULL, overwrite the cpu in the
>      branch trace configuration.  This is currently only used for the PT
> @@ -1534,6 +2135,10 @@ btrace_compute_ftrace_1 (struct thread_info *tp,
>   
>         btrace_compute_ftrace_pt (tp, &btrace->variant.pt, gaps);
>         return;
> +
> +    case BTRACE_FORMAT_ETM:
> +      btrace_compute_ftrace_etm (tp, &btrace->variant.etm, gaps);
> +      return;
>       }
>   
>     internal_error (__FILE__, __LINE__, _("Unknown branch trace format."));
> @@ -1602,6 +2207,10 @@ btrace_enable (struct thread_info *tp, const struct btrace_config *conf)
>     if (conf->format == BTRACE_FORMAT_PT)
>       error (_("Intel Processor Trace support was disabled at compile time."));
>   #endif /* !defined (HAVE_LIBIPT) */
> +#if !defined (HAVE_LIBOPENCSD_C_API)
> +  if (conf->format == BTRACE_FORMAT_ETM)
> +    error (_("ARM CoreSight Trace support was disabled at compile time."));
> +#endif /* !defined (HAVE_LIBOPENCSD_C_API) */
>   
>     DEBUG ("enable thread %s (%s)", print_thread_id (tp),
>   	 target_pid_to_str (tp->ptid).c_str ());
> @@ -1794,6 +2403,10 @@ btrace_stitch_trace (struct btrace_data *btrace, struct thread_info *tp)
>       case BTRACE_FORMAT_PT:
>         /* Delta reads are not supported.  */
>         return -1;
> +
> +    case BTRACE_FORMAT_ETM:
> +      /* Delta reads are not supported.  */
> +      return -1;
>       }
>   
>     internal_error (__FILE__, __LINE__, _("Unknown branch trace format."));
> @@ -3416,6 +4029,11 @@ maint_info_btrace_cmd (const char *args, int from_tty)
>         }
>         break;
>   #endif /* defined (HAVE_LIBIPT)  */
> +#if defined (HAVE_LIBOPENCSD_C_API)
> +    case BTRACE_FORMAT_ETM:
> +      printf_unfiltered (_("Version: %s.\n"), ocsd_get_version_str ());
> +      break;
> +#endif /* defined (HAVE_LIBOPENCSD_C_API) */
>       }
>   }
>   
> diff --git a/gdb/btrace.h b/gdb/btrace.h
> index 8f6ce32103d..92e14b6aadf 100644
> --- a/gdb/btrace.h
> +++ b/gdb/btrace.h
> @@ -34,6 +34,12 @@
>   #  include <intel-pt.h>
>   #endif
>   
> +#if defined (HAVE_LIBOPENCSD_C_API)
> +#  include <opencsd/c_api/opencsd_c_api.h>
> +#  include <opencsd/etmv4/trc_pkt_types_etmv4.h>
> +#  include <opencsd/ocsd_if_types.h>
> +#endif
> +
>   #include <vector>
>   
>   struct thread_info;
> @@ -59,7 +65,15 @@ enum btrace_insn_class
>   enum btrace_insn_flag
>   {
>     /* The instruction has been executed speculatively.  */
> -  BTRACE_INSN_FLAG_SPECULATIVE = (1 << 0)
> +  BTRACE_INSN_FLAG_SPECULATIVE =	(1 << 0),
> +  BTRACE_INSN_FLAG_ISA_ARM =		(1 << 24),
> +  BTRACE_INSN_FLAG_ISA_THUMB2 =	(2 << 24),
> +  BTRACE_INSN_FLAG_ISA_AARCH64 =	(3 << 24),
> +  BTRACE_INSN_FLAG_ISA_TEE =		(4 << 24),
> +  BTRACE_INSN_FLAG_ISA_JAZELLE =	(5 << 24),
> +  BTRACE_INSN_FLAG_ISA_CUSTOM =	(6 << 24),
> +  BTRACE_INSN_FLAG_ISA_UNKNOWN =	(7 << 24),
> +  BTRACE_INSN_FLAG_ISA_MASK =		(15 << 24)

Indentation looks off for the last line above.

>   };
>   DEF_ENUM_FLAGS_TYPE (enum btrace_insn_flag, btrace_insn_flags);
>   
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index 16ffb76272b..57b9c8ec487 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -44,6 +44,7 @@
>   #include "cli/cli-style.h"
>   #include "async-event.h"
>   #include <forward_list>
> +#include "arch/arm.h"
>   
>   static const target_info record_btrace_target_info = {
>     "record-btrace",
> @@ -1557,6 +1558,38 @@ record_btrace_target::remove_breakpoint (struct gdbarch *gdbarch,
>     return ret;
>   }
>   
> +/* Reconstruct the instruction set state bits of CPSR register
> +   according to instruction flags.
> +   See Table A2-1 in DDI0406B_arm_architecture_reference_manual
> +   for more details.  */
> +
> +static unsigned int
> +cs_etm_reconstruct_cpsr_iset_state (const struct btrace_insn *insn)
> +{
> +  switch (insn->flags & BTRACE_INSN_FLAG_ISA_MASK)
> +    {
> +    case BTRACE_INSN_FLAG_ISA_ARM:
> +      /* ARM state: J and T bits are not set.  */
> +      return 0;
> +
> +    case BTRACE_INSN_FLAG_ISA_THUMB2:
> +      /* THUMB state: J bit is not set, T bit is set.  */
> +      return 0x20;
> +
> +    case BTRACE_INSN_FLAG_ISA_TEE:
> +      /* THUMB EE state: J and T bits are set.  */
> +      return 0x1000020;
> +
> +    case BTRACE_INSN_FLAG_ISA_JAZELLE:
> +      /* JAZELLE state: J bit is set, T bit is not set.  */
> +      return 0x1000000;
> +
> +    default:
> +      /* Default is ARM mode.  */
> +      return 0;
> +    }
> +}
> +
>   /* The fetch_registers method of target record-btrace.  */
>   
>   void
> @@ -1581,16 +1614,32 @@ record_btrace_target::fetch_registers (struct regcache *regcache, int regno)
>         pcreg = gdbarch_pc_regnum (gdbarch);
>         if (pcreg < 0)
>   	return;
> -
> -      /* We can only provide the PC register.  */
> -      if (regno >= 0 && regno != pcreg)
> +      /* We can only provide the PC or CPSR registers here.  */
> +      if (regno >= 0 && !(regno == pcreg || regno == ARM_PS_REGNUM))
>   	return;
>   
>         insn = btrace_insn_get (replay);
> -      gdb_assert (insn != NULL);
> +      gdb_assert (insn != nullptr);
>   
> +      if ((regno < 0) || (regno == pcreg))
> +	{
>   	  regcache->raw_supply (regno, &insn->pc);
>   	}
> +      if ((regno < 0) || (regno == ARM_PS_REGNUM))
> +	{
> +	  /*  Provide CPSR register in the case of an armv7 target.  */
> +	  const struct target_desc *tdesc = gdbarch_target_desc (gdbarch);
> +
> +	  const char *tdesc_name = tdesc_architecture_name (tdesc);
> +	  if (strcmp (tdesc_name, "arm") == 0)
> +	    {
> +	      int cpsr;
> +	      cpsr = cs_etm_reconstruct_cpsr_iset_state (insn);

Put the declaration and initialization of cpsr in the same line.

> +	      regcache->raw_supply (regno, &cpsr);
> +	    }
> +	}
> +      return;
> +    }
>     else
>       this->beneath ()->fetch_registers (regcache, regno);
>   }
> diff --git a/gdbsupport/btrace-common.cc b/gdbsupport/btrace-common.cc
> index 79ff21de44b..6005b0d15e4 100644
> --- a/gdbsupport/btrace-common.cc
> +++ b/gdbsupport/btrace-common.cc
> @@ -86,6 +86,12 @@ btrace_data::fini ()
>       case BTRACE_FORMAT_PT:
>         xfree (variant.pt.data);
>         return;
> +
> +    case BTRACE_FORMAT_ETM:
> +      delete variant.etm.config.etm_trace_params;
> +      variant.etm.config.etm_trace_params = nullptr;
> +      xfree (variant.etm.data);
> +      return;
>       }
>   
>     internal_error (__FILE__, __LINE__, _("Unkown branch trace format."));
> @@ -106,6 +112,9 @@ btrace_data::empty () const
>   
>       case BTRACE_FORMAT_PT:
>         return (variant.pt.size == 0);
> +
> +    case BTRACE_FORMAT_ETM:
> +      return (variant.etm.size == 0);
>       }
>   
>     internal_error (__FILE__, __LINE__, _("Unkown branch trace format."));
> @@ -191,6 +200,36 @@ btrace_data_append (struct btrace_data *dst,
>   	  }
>   	}
>         return 0;
> +
> +    case BTRACE_FORMAT_ETM:
> +      switch (dst->format)
> +	{
> +	default:
> +	  return -1;
> +
> +	case BTRACE_FORMAT_NONE:
> +	  dst->format = BTRACE_FORMAT_ETM;
> +	  dst->variant.etm.data = nullptr;
> +	  dst->variant.etm.size = 0;
> +
> +	/* fall-through.  */
> +	case BTRACE_FORMAT_ETM:
> +	  {
> +	    size_t size = src->variant.etm.size + dst->variant.etm.size;
> +	    gdb_byte *data = (gdb_byte *) xmalloc (size);
> +
> +	    if (dst->variant.etm.size > 0)
> +	      memcpy (data, dst->variant.etm.data, dst->variant.etm.size);
> +	    memcpy (data + dst->variant.etm.size, src->variant.etm.data,
> +		    src->variant.etm.size);
> +
> +	    xfree (dst->variant.etm.data);
> +
> +	    dst->variant.etm.data = data;
> +	    dst->variant.etm.size = size;
> +	  }
> +	}
> +      return 0;
>       }
>   
>     internal_error (__FILE__, __LINE__, _("Unkown branch trace format."));
> diff --git a/gdbsupport/btrace-common.h b/gdbsupport/btrace-common.h
> index 153b977723a..ee05ecb8b10 100644
> --- a/gdbsupport/btrace-common.h
> +++ b/gdbsupport/btrace-common.h
> @@ -187,6 +187,109 @@ struct btrace_data_pt
>     size_t size;
>   };
>   
> +/* Parameters needed for decoding ETMv3 traces.
> +   See open source coresight trace decoder library (opencsd)
> +   for further details.  */
> +struct cs_etmv3_trace_params
> +{
> +  /* ETMv3 Main Control Register.  */
> +  uint32_t reg_ctrl;
> +
> +  /* ETMv3 Trace ID Register.  */
> +  uint32_t reg_trc_id;
> +
> +  /* ETMv3 Configuration Code Extension Register.  */
> +  uint32_t reg_ccer;
> +
> +  /* ETMv3 ID Register.  */
> +  uint32_t reg_idr;
> +};
> +
> +/* Parameters needed for decoding ETMv4 traces.
> +   See open source coresight trace decoder library (opencsd)
> +   for further details.  */
> +struct cs_etmv4_trace_params
> +{
> +  /* ETMv4 ID Register 0.  */
> +  uint32_t reg_idr0;
> +
> +  /* ETMv4 ID Register 1.  */
> +  uint32_t reg_idr1;
> +
> +  /* ETMv4 ID Register 2.  */
> +  uint32_t reg_idr2;
> +
> +  /* ETMv4 ID Register 8.  */
> +  uint32_t reg_idr8;
> +
> +  /* ETMv4 Config Register.  */
> +  uint32_t reg_configr;
> +
> +  /* ETMv4 Trace ID Register.  */
> +  uint32_t reg_traceidr;
> +};
> +
> +/* Parameters of trace source.  */
> +struct cs_etm_trace_params
> +{
> +  /* Architecture version of trace source.  */
> +  int arch_ver;
> +  /* Core profile of the trace source.  */
> +  int core_profile;
> +  /* Traces protocol.  */
> +  int protocol;
> +  union {
> +    struct cs_etmv3_trace_params etmv3;
> +    struct cs_etmv4_trace_params etmv4;
> +  };
> +};
> +
> +/* Parameters of trace sink/decoder.  */
> +struct cs_etm_decoder_params
> +{
> +  uint8_t formatted:1,     /* Formatted input, or single source input.  */
> +  fsyncs	   :1,     /* Formatted data has fsyncs.  */
> +  hsyncs	   :1,     /* Formatted data has hsyncs.  */
> +  frame_aligned    :1,     /* Formatted frames are memory aligned.  */
> +  reset_on_4x_sync :1,     /* Reset decoders at 4x consecutive fsyncs.  */
> +  __res	    :3;     /* Reserved, not used.  */
> +};

I'm not sure the struct format above matches GDB's style. We usually 
declare each field individually for structs. I'll leave this to other 
maintainers.

> +
> +/* Configuration information to go with the etm trace data.  */
> +struct btrace_data_etm_config
> +{
> +  /* Count of the CPUs (trace sources).  */
> +  int    cpu_count;
> +  /* List of traces sources parameters.  */
> +  std::vector<struct cs_etm_trace_params> *etm_trace_params;
> +  /* Trace sink parameters.  */
> +  struct cs_etm_decoder_params etm_decoder_params;
> +};
> +
> +/* Branch trace in ARM Processor Trace format.  */
> +struct btrace_data_etm
> +{
> +  /* Some configuration information to go with the data.  */
> +  struct btrace_data_etm_config config;
> +
> +  /* The trace data.  */
> +  gdb_byte *data;
> +
> +  /* The size of DATA in bytes.  */
> +  size_t size;
> +
> +  /* Trace id for this thread.
> +     On a linux system, trace_id is assigned per cpu. The kernel copies
> +     the traces of each thread in a dedicated ring buffer. By this,
> +     traces belonging to different threads are de-multiplexed.
> +     On an RTOS system, especially when routing the traces outside of the SoC,
> +     the OS has no other mean for de-multiplexing the traces than
> +     the trace_id. The hardware (ETM IP) reserves 7 bits for the trace_id.
> +     On linux system trace id is not needed, set it to 0xFF to ignore it
> +     during parsing.  */
> +  uint8_t trace_id;
> +};
> +
>   /* The branch trace data.  */
>   struct btrace_data
>   {
> @@ -224,6 +327,9 @@ struct btrace_data
>   
>       /* Format == BTRACE_FORMAT_PT.  */
>       struct btrace_data_pt pt;
> +
> +    /* Format == BTRACE_FORMAT_ETM.  */
> +    struct btrace_data_etm etm;
>     } variant;
>   
>   private:
> 

Looks OK to me otherwise.