Re: [PATCH v3 4/7] start/stop btrace with coresight etm and collect etm buffer on linux os

[Luis Machado <luis.machado@linaro.org>]

On 3/30/21 11:52 PM, Zied Guermazi wrote:
> 
> This patch implement the lower layer for starting ad stopping
> ARM CoreSight tracing on linux targets for arm and aarch64
> 
> gdb/ChangeLog
> 
> 	* nat/linux-btrace.h (btrace_tinfo_etm): New.
> 	(btrace_target_info): add etm.
> 	* nat/linux-btrace.c (linux_enable_bts): get page size from sysconf.
> 	(linux_enable_pt): get page size from sysconf.
> 	(perf_event_etm_event_type): New.
> 	(perf_event_etm_event_sink): New.
> 	(linux_enable_etm): New.
> 	(linux_enable_btrace): add enabling etm traces.
> 	(linux_disable_bts): get page size from sysconf.
> 	(linux_disable_pt): get page size from sysconf.
> 	(linux_disable_etm): New.
> 	(linux_disable_btrace): add disabling etm traces.
> 	(get_cpu_count): New.
> 	(cs_etm_is_etmv4): New.
> 	(cs_etm_get_register): New.
> 	(coresight_get_trace_id): New.
> 	(fill_etm_trace_params): New.
> 	(linux_fill_btrace_etm_config): New.
> 	(linux_read_etm): New.
> 	(linux_read_btrace): add reading etm traces.
> 	* arm-linux-nat.c (arm_linux_nat_target::enable_btrace): New.
> 	(arm_linux_nat_target::disable_btrace): New.
> 	(arm_linux_nat_target::teardown_btrace): New.
> 	(arm_linux_nat_target::read_btrace): New.
> 	(arm_linux_nat_target::btrace_conf): New.
> 	* aarch64-linux-nat.c (aarch64_linux_nat_target::enable_btrace): New.
> 	(aarch64_linux_nat_target::disable_btrace): New.
> 	(aarch64_linux_nat_target::teardown_btrace): New.
> 	(aarch64_linux_nat_target::read_btrace): New.
> 	(aarch64_linux_nat_target::btrace_conf): New.
> 
> ---
>   gdb/ChangeLog           |  34 ++++
>   gdb/aarch64-linux-nat.c |  68 +++++++
>   gdb/arm-linux-nat.c     |  68 +++++++
>   gdb/nat/linux-btrace.c  | 400 ++++++++++++++++++++++++++++++++++++++--
>   gdb/nat/linux-btrace.h  |  19 ++
>   5 files changed, 577 insertions(+), 12 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 4453a3e8185..d22ec8b5976 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,37 @@
> +2021-02-25  Zied Guermazi  <zied.guermazi@trande.de>
> +
> +	* nat/linux-btrace.h (btrace_tinfo_etm): New.
> +	(btrace_target_info): add etm.
> +	* nat/linux-btrace.c (perf_event_read_available): New.
> +	(linux_enable_bts): get page size from sysconf.
> +	(linux_enable_pt): get page size from sysconf.
> +	(perf_event_etm_event_type): New.
> +	(perf_event_etm_event_sink): New.
> +	(linux_enable_etm): New.
> +	(linux_enable_btrace): add enabling etm traces.
> +	(linux_disable_bts): get page size from sysconf.
> +	(linux_disable_pt): get page size from sysconf.
> +	(linux_disable_etm): New.
> +	(linux_disable_btrace): add disabling etm traces.
> +	(get_cpu_count): New.
> +	(cs_etm_is_etmv4): New.
> +	(cs_etm_get_register): New.
> +	(coresight_get_trace_id): New.
> +	(fill_etm_trace_params): New.
> +	(linux_fill_btrace_etm_config): New.
> +	(linux_read_etm): New.
> +	(linux_read_btrace): add reading etm traces.
> +	* arm-linux-nat.c (arm_linux_nat_target::enable_btrace): New.
> +	(arm_linux_nat_target::disable_btrace): New.
> +	(arm_linux_nat_target::teardown_btrace): New.
> +	(arm_linux_nat_target::read_btrace): New.
> +	(arm_linux_nat_target::btrace_conf): New.
> +	* aarch64-linux-nat.c (aarch64_linux_nat_target::enable_btrace): New.
> +	(aarch64_linux_nat_target::disable_btrace): New.
> +	(aarch64_linux_nat_target::teardown_btrace): New.
> +	(aarch64_linux_nat_target::read_btrace): New.
> +	(aarch64_linux_nat_target::btrace_conf): New.
> +
>   2021-02-25  Zied Guermazi  <zied.guermazi@trande.de>
>   
>   	* btrace.c (ftrace_remove_last_insn): New.
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 0cae91b569c..f459fdf6f0e 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -35,6 +35,7 @@
>   #include "nat/aarch64-linux.h"
>   #include "nat/aarch64-linux-hw-point.h"
>   #include "nat/aarch64-sve-linux-ptrace.h"
> +#include "nat/linux-btrace.h"
>   
>   #include "elf/external.h"
>   #include "elf/common.h"
> @@ -84,6 +85,17 @@ class aarch64_linux_nat_target final : public linux_nat_target
>     /* Override the GNU/Linux post attach hook.  */
>     void post_attach (int pid) override;
>   
> +  /* Override branch tracing.  */
> +  struct btrace_target_info *enable_btrace (ptid_t ptid,
> +				const struct btrace_config *conf) override;
> +  void disable_btrace (struct btrace_target_info *tinfo) override;
> +  void teardown_btrace (struct btrace_target_info *tinfo) override;
> +  enum btrace_error read_btrace (struct btrace_data *data,
> +				  struct btrace_target_info *btinfo,
> +				  enum btrace_read_type type) override;
> +  const struct btrace_config *btrace_conf (const struct btrace_target_info *)
> +				override;
> +
>     /* These three defer to common nat/ code.  */
>     void low_new_thread (struct lwp_info *lp) override
>     { aarch64_linux_new_thread (lp); }
> @@ -632,6 +644,62 @@ aarch64_linux_nat_target::post_attach (int pid)
>     linux_nat_target::post_attach (pid);
>   }
>   
> +/* Enable branch tracing.  */
> +
> +struct btrace_target_info *
> +aarch64_linux_nat_target::enable_btrace (ptid_t ptid,
> +					  const struct btrace_config *conf)
> +{
> +  struct btrace_target_info *tinfo = nullptr;
> +  try
> +    {
> +      tinfo = linux_enable_btrace (ptid, conf);
> +    }
> +  catch (const gdb_exception_error &exception)
> +    {
> +      error (_("Could not enable branch tracing for %s: %s"),
> +	     target_pid_to_str (ptid).c_str (), exception.what ());
> +    }
> +
> +  return tinfo;
> +}
> +
> +/* Disable branch tracing.  */
> +
> +void
> +aarch64_linux_nat_target::disable_btrace (struct btrace_target_info *tinfo)
> +{
> +  enum btrace_error errcode = linux_disable_btrace (tinfo);
> +
> +  if (errcode != BTRACE_ERR_NONE)
> +    error (_("Could not disable branch tracing."));
> +}
> +
> +/* Teardown branch tracing.  */
> +
> +void
> +aarch64_linux_nat_target::teardown_btrace (struct btrace_target_info *tinfo)
> +{
> +  /* Ignore errors.  */
> +  linux_disable_btrace (tinfo);
> +}
> +
> +enum btrace_error
> +aarch64_linux_nat_target::read_btrace (struct btrace_data *data,
> +				       struct btrace_target_info *btinfo,
> +				       enum btrace_read_type type)
> +{
> +  return linux_read_btrace (data, btinfo, type);
> +}
> +
> +/* See to_btrace_conf in target.h.  */
> +
> +const struct btrace_config *
> +aarch64_linux_nat_target::btrace_conf (const struct btrace_target_info *btinfo)
> +{
> +  return linux_btrace_conf (btinfo);
> +}
> +
>   /* Implement the "read_description" target_ops method.  */
>   
>   const struct target_desc *
> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
> index 662dade0a12..8a8d4b65776 100644
> --- a/gdb/arm-linux-nat.c
> +++ b/gdb/arm-linux-nat.c
> @@ -39,6 +39,7 @@
>   #include <sys/procfs.h>
>   
>   #include "nat/linux-ptrace.h"
> +#include "nat/linux-btrace.h"
>   #include "linux-tdep.h"
>   
>   /* Prototypes for supply_gregset etc.  */
> @@ -95,6 +96,17 @@ class arm_linux_nat_target final : public linux_nat_target
>   
>     const struct target_desc *read_description () override;
>   
> +  /* Override branch tracing.  */
> +  struct btrace_target_info *enable_btrace (ptid_t ptid,
> +				const struct btrace_config *conf) override;
> +  void disable_btrace (struct btrace_target_info *tinfo) override;
> +  void teardown_btrace (struct btrace_target_info *tinfo) override;
> +  enum btrace_error read_btrace (struct btrace_data *data,
> +				  struct btrace_target_info *btinfo,
> +				  enum btrace_read_type type) override;
> +  const struct btrace_config *btrace_conf (const struct btrace_target_info *)
> +				override;
> +
>     /* Override linux_nat_target low methods.  */
>   
>     /* Handle thread creation and exit.  */
> @@ -1190,6 +1202,62 @@ arm_linux_nat_target::watchpoint_addr_within_range (CORE_ADDR addr,
>     return start <= addr && start + length - 1 >= addr;
>   }
>   
> +/* Enable branch tracing.  */
> +
> +struct btrace_target_info *
> +arm_linux_nat_target::enable_btrace (ptid_t ptid,
> +				     const struct btrace_config *conf)
> +{
> +  struct btrace_target_info *tinfo = nullptr;
> +  try
> +    {
> +      tinfo = linux_enable_btrace (ptid, conf);
> +    }
> +  catch (const gdb_exception_error &exception)
> +    {
> +      error (_("Could not enable branch tracing for %s: %s"),
> +	     target_pid_to_str (ptid).c_str (), exception.what ());
> +    }
> +
> +  return tinfo;
> +}
> +
> +/* Disable branch tracing.  */
> +
> +void
> +arm_linux_nat_target::disable_btrace (struct btrace_target_info *tinfo)
> +{
> +  enum btrace_error errcode = linux_disable_btrace (tinfo);
> +
> +  if (errcode != BTRACE_ERR_NONE)
> +    error (_("Could not disable branch tracing."));

Is this error due to not being able to disable branch tracing or not 
having an active tracing session to disable? If the latter, you may want 
to rephrase the error message.

> +}
> +
> +/* Teardown branch tracing.  */
> +
> +void
> +arm_linux_nat_target::teardown_btrace (struct btrace_target_info *tinfo)
> +{
> +  /* Ignore errors.  */
> +  linux_disable_btrace (tinfo);
> +}
> +
> +enum btrace_error
> +arm_linux_nat_target::read_btrace (struct btrace_data *data,
> +				   struct btrace_target_info *btinfo,
> +				   enum btrace_read_type type)
> +{
> +  return linux_read_btrace (data, btinfo, type);
> +}
> +
> +/* See to_btrace_conf in target.h.  */
> +
> +const struct btrace_config *
> +arm_linux_nat_target::btrace_conf (const struct btrace_target_info *btinfo)
> +{
> +  return linux_btrace_conf (btinfo);
> +}
> +
>   /* Handle thread creation.  We need to copy the breakpoints and watchpoints
>      in the parent thread to the child thread.  */
>   void
> diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
> index 324f7ef0407..11e2dd96397 100644
> --- a/gdb/nat/linux-btrace.c
> +++ b/gdb/nat/linux-btrace.c
> @@ -32,6 +32,10 @@
>   
>   #include <sys/syscall.h>
>   
> +#if defined (HAVE_LIBOPENCSD_C_API)
> +#  include <opencsd/ocsd_if_types.h>
> +#endif
> +
>   #if HAVE_LINUX_PERF_EVENT_H && defined(SYS_perf_event_open)
>   #include <unistd.h>
>   #include <sys/mman.h>
> @@ -483,10 +487,11 @@ linux_enable_bts (ptid_t ptid, const struct btrace_config_bts *conf)
>     scoped_fd fd (syscall (SYS_perf_event_open, &bts->attr, pid, -1, -1, 0));
>     if (fd.get () < 0)
>       diagnose_perf_event_open_fail ();
> +  long page_size = sysconf (_SC_PAGESIZE);
>   
>     /* Convert the requested size in bytes to pages (rounding up).  */
> -  pages = ((size_t) conf->size / PAGE_SIZE
> -	   + ((conf->size % PAGE_SIZE) == 0 ? 0 : 1));
> +  pages = ((size_t) conf->size / page_size
> +	   + ((conf->size % page_size) == 0 ? 0 : 1));
>     /* We need at least one page.  */
>     if (pages == 0)
>       pages = 1;
> @@ -505,17 +510,17 @@ linux_enable_bts (ptid_t ptid, const struct btrace_config_bts *conf)
>         size_t length;
>         __u64 data_size;
>   
> -      data_size = (__u64) pages * PAGE_SIZE;
> +      data_size = (__u64) pages * page_size;
>   
>         /* Don't ask for more than we can represent in the configuration.  */
>         if ((__u64) UINT_MAX < data_size)
>   	continue;
>   
>         size = (size_t) data_size;
> -      length = size + PAGE_SIZE;
> +      length = size + page_size;
>   
>         /* Check for overflows.  */
> -      if ((__u64) length != data_size + PAGE_SIZE)
> +      if ((__u64) length != data_size + page_size)
>   	continue;
>   
>         errno = 0;
> @@ -530,7 +535,7 @@ linux_enable_bts (ptid_t ptid, const struct btrace_config_bts *conf)
>   
>     struct perf_event_mmap_page *header = (struct perf_event_mmap_page *)
>       data.get ();
> -  data_offset = PAGE_SIZE;
> +  data_offset = page_size;
>   
>   #if defined (PERF_ATTR_SIZE_VER5)
>     if (offsetof (struct perf_event_mmap_page, data_size) <= header->size)
> @@ -613,7 +618,8 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
>       diagnose_perf_event_open_fail ();
>   
>     /* Allocate the configuration page. */
> -  scoped_mmap data (nullptr, PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,
> +  long page_size = sysconf (_SC_PAGESIZE);
> +  scoped_mmap data (nullptr, page_size, PROT_READ | PROT_WRITE, MAP_SHARED,
>   		    fd.get (), 0);
>     if (data.get () == MAP_FAILED)
>       error (_("Failed to map trace user page: %s."), safe_strerror (errno));
> @@ -624,8 +630,8 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
>     header->aux_offset = header->data_offset + header->data_size;
>   
>     /* Convert the requested size in bytes to pages (rounding up).  */
> -  pages = ((size_t) conf->size / PAGE_SIZE
> -	   + ((conf->size % PAGE_SIZE) == 0 ? 0 : 1));
> +  pages = ((size_t) conf->size / page_size
> +	   + ((conf->size % page_size) == 0 ? 0 : 1));
>     /* We need at least one page.  */
>     if (pages == 0)
>       pages = 1;
> @@ -644,7 +650,7 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
>         size_t length;
>         __u64 data_size;
>   
> -      data_size = (__u64) pages * PAGE_SIZE;
> +      data_size = (__u64) pages * page_size;
>   
>         /* Don't ask for more than we can represent in the configuration.  */
>         if ((__u64) UINT_MAX < data_size)
> @@ -689,6 +695,154 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
>   
>   #endif /* !defined (PERF_ATTR_SIZE_VER5) */
>   
> +/* Determine the etm event type.  */
> +
> +static int
> +perf_event_etm_event_type ()
> +{
> +  static const char filename[] = "/sys/bus/event_source/devices/cs_etm/type";
> +
> +  errno = 0;
> +  gdb_file_up file = gdb_fopen_cloexec (filename, "r");
> +  if (file.get () == nullptr)
> +    error (_("Failed to open %s: %s."), filename, safe_strerror (errno));
> +
> +  int type, found = fscanf (file.get (), "%d", &type);
> +  if (found != 1)
> +    error (_("Failed to read the ETM event type from %s."), filename);
> +
> +  return type;
> +}
> +
> +/* Get the sink hash to use in the event attributes.  */
> +
> +static unsigned int
> +perf_event_etm_event_sink (const struct btrace_config_etm *conf)
> +{
> +  char filename[PATH_MAX];
> +
> +  sprintf (filename, "/sys/bus/event_source/devices/cs_etm/sinks/%s",
> +	   conf->sink);

Instead of hardcoding the filename buffer size, you could use 
string_printf or snprintf here.

> +  errno = 0;
> +  gdb_file_up file = gdb_fopen_cloexec (filename, "r");
> +  if (file.get () == nullptr)
> +    error (_("Failed to open %s: %s."), filename, safe_strerror (errno));
> +
> +  unsigned int sink;
> +  int  found = fscanf (file.get (), "0x%x", &sink);
> +  if (found != 1)
> +    error (_("Failed to read the ETM sink from %s."), filename);
> +
> +  return sink;
> +}
> +
> +/* Enable arm CoreSight ETM tracing.  */
> +

arm -> ARM

> +static struct btrace_target_info *
> +linux_enable_etm (ptid_t ptid, const struct btrace_config_etm *conf)
> +{
> +  struct btrace_tinfo_etm *etm;
> +  size_t pages;
> +  int pid, pg;
> +
> +  pid = ptid.lwp ();
> +  if (pid == 0)
> +    pid = ptid.pid ();
> +
> +  gdb::unique_xmalloc_ptr<btrace_target_info> tinfo
> +    (XCNEW (btrace_target_info));
> +  tinfo->ptid = ptid;
> +
> +  tinfo->conf.format = BTRACE_FORMAT_ETM;
> +  etm = &tinfo->variant.etm;
> +
> +  etm->attr.type = perf_event_etm_event_type ();
> +  etm->attr.size = sizeof (etm->attr);
> +
> +  etm->attr.sample_type = PERF_SAMPLE_CPU;
> +  etm->attr.read_format = PERF_FORMAT_ID;
> +  etm->attr.sample_id_all = 1;
> +  etm->attr.enable_on_exec = 1;
> +  etm->attr.exclude_kernel = 1;
> +  etm->attr.exclude_hv = 1;
> +  etm->attr.exclude_idle = 1;
> +  if (conf->sink != nullptr)
> +    {
> +      if (strcmp (conf->sink, "default")!=0)
> +	etm->attr.config2 = perf_event_etm_event_sink (conf);
> +    }
> +
> +  errno = 0;
> +  scoped_fd fd (syscall (SYS_perf_event_open, &etm->attr, pid, -1, -1, 0));
> +  if (fd.get () < 0)
> +    diagnose_perf_event_open_fail ();
> +
> +  /* Allocate the configuration page.  */
> +  long page_size = sysconf (_SC_PAGESIZE);
> +  scoped_mmap data (nullptr, page_size, PROT_READ | PROT_WRITE, MAP_SHARED,
> +		    fd.get (), 0);
> +  if (data.get () == MAP_FAILED)
> +    error (_("Failed to map trace user page: %s."), safe_strerror (errno));
> +
> +  struct perf_event_mmap_page *header = (struct perf_event_mmap_page *)
> +    data.get ();
> +
> +  header->aux_offset = header->data_offset + header->data_size;
> +  /* Convert the requested size in bytes to pages (rounding up).  */
> +  pages = ((size_t) conf->size / page_size
> +	   + ((conf->size % page_size) == 0 ? 0 : 1));
> +  /* We need at least one page.  */
> +  if (pages == 0)
> +    pages = 1;
> +
> +  /* The buffer size can be requested in powers of two pages.  Adjust PAGES
> +     to the next power of two.  */
> +  for (pg = 0; pages != ((size_t) 1 << pg); ++pg)
> +    if ((pages & ((size_t) 1 << pg)) != 0)
> +      pages += ((size_t) 1 << pg);
> +
> +  /* We try to allocate the requested size.
> +     If that fails, try to get as much as we can.  */
> +  scoped_mmap aux;
> +  for (; pages > 0; pages >>= 1)
> +    {
> +      size_t length;
> +      __u64 data_size;
> +      data_size = (__u64) pages * page_size;
> +
> +      /* Don't ask for more than we can represent in the configuration.  */
> +      if ((__u64) UINT_MAX < data_size)
> +	continue;
> +
> +      length = (size_t) data_size;
> +
> +      /* Check for overflows.  */
> +      if ((__u64) length != data_size)
> +	continue;
> +
> +      header->aux_size = data_size;
> +
> +      errno = 0;
> +      aux.reset (nullptr, length, PROT_READ, MAP_SHARED, fd.get (),
> +		 header->aux_offset);
> +      if (aux.get () != MAP_FAILED)
> +	break;
> +    }
> +  if (pages == 0)
> +    error (_("Failed to map trace buffer: %s."), safe_strerror (errno));
> +
> +  etm->etm.size = aux.size ();
> +  etm->etm.mem = (const uint8_t *) aux.release ();
> +  etm->etm.data_head = &header->aux_head;
> +  etm->etm.last_head = header->aux_tail;
> +  etm->header = (struct perf_event_mmap_page *) data.release ();
> +  gdb_assert (etm->header == header);
> +  etm->file = fd.release ();
> +
> +  tinfo->conf.etm.size = (unsigned int) etm->etm.size;
> +  return tinfo.release ();
> +}
> +
>   /* See linux-btrace.h.  */
>   
>   struct btrace_target_info *
> @@ -707,6 +861,10 @@ linux_enable_btrace (ptid_t ptid, const struct btrace_config *conf)
>   
>       case BTRACE_FORMAT_PT:
>         return linux_enable_pt (ptid, &conf->pt);
> +
> +    case BTRACE_FORMAT_ETM:
> +      return linux_enable_etm (ptid, &conf->etm);
> +
>       }
>   }
>   
> @@ -715,7 +873,8 @@ linux_enable_btrace (ptid_t ptid, const struct btrace_config *conf)
>   static enum btrace_error
>   linux_disable_bts (struct btrace_tinfo_bts *tinfo)
>   {
> -  munmap((void *) tinfo->header, tinfo->bts.size + PAGE_SIZE);
> +  long page_size = sysconf (_SC_PAGESIZE);
> +  munmap((void *) tinfo->header, tinfo->bts.size + page_size);
>     close (tinfo->file);
>   
>     return BTRACE_ERR_NONE;
> @@ -726,8 +885,22 @@ linux_disable_bts (struct btrace_tinfo_bts *tinfo)
>   static enum btrace_error
>   linux_disable_pt (struct btrace_tinfo_pt *tinfo)
>   {
> +  long page_size = sysconf (_SC_PAGESIZE);
>     munmap((void *) tinfo->pt.mem, tinfo->pt.size);
> -  munmap((void *) tinfo->header, PAGE_SIZE);
> +  munmap((void *) tinfo->header, page_size);
> +  close (tinfo->file);
> +
> +  return BTRACE_ERR_NONE;
> +}
> +
> +/* Disable arm CoreSight ETM tracing.  */
> +

arm -> ARM

> +static enum btrace_error
> +linux_disable_etm (struct btrace_tinfo_etm *tinfo)
> +{
> +  long page_size = sysconf (_SC_PAGESIZE);
> +  munmap ((void *) tinfo->etm.mem, tinfo->etm.size);
> +  munmap ((void *) tinfo->header, page_size);
>     close (tinfo->file);
>   
>     return BTRACE_ERR_NONE;
> @@ -753,6 +926,10 @@ linux_disable_btrace (struct btrace_target_info *tinfo)
>       case BTRACE_FORMAT_PT:
>         errcode = linux_disable_pt (&tinfo->variant.pt);
>         break;
> +
> +    case BTRACE_FORMAT_ETM:
> +      errcode = linux_disable_etm (&tinfo->variant.etm);
> +      break;
>       }
>   
>     if (errcode == BTRACE_ERR_NONE)
> @@ -898,6 +1075,197 @@ linux_read_pt (struct btrace_data_pt *btrace,
>     internal_error (__FILE__, __LINE__, _("Unknown btrace read type."));
>   }
>   
> +/* Return the number of CPUs that are present.  */
> +
> +static int
> +get_cpu_count (void)
> +{
> +  static const char filename[] = "/sys/devices/system/cpu/present";
> +
> +  gdb_file_up file = gdb_fopen_cloexec (filename, "r");
> +  if (file.get () == nullptr)
> +    error (_("Failed to open %s: %s."), filename, safe_strerror (errno));
> +
> +  int length;
> +
> +  fseek (file.get (), 0, SEEK_END);
> +  length = ftell (file.get ());
> +  fseek (file.get (), 0, SEEK_SET);
> +
> +  char * buffer;

Spurious space between * and buffer.
> +
> +  buffer = (char*) xmalloc (length+1);

space between char and *.

> +
> +  length = fread (buffer, 1, length, file.get ());
> +  buffer[length]='\0';

Space between buffer[length] and =, and between = and '\0'.

> +  while (--length)
> +    {
> +      if ((buffer[length] == ',') || (buffer[length] == '-'))
> +	{
> +	  length++;
> +	  break;
> +	}
> +    }
> +
> +  int cpu_count;
> +
> +  if (sscanf (&buffer[length], "%d", &cpu_count) < 1)
> +    error (_("Failed to get cpu count in %s: %s."),
> +	     buffer, safe_strerror (errno));
> +
> +  cpu_count ++;
> +  return (cpu_count);
> +}
> +
> +/* Check if the ETM is an etmv4.  */
> +
> +static bool
> +cs_etm_is_etmv4 (int cpu)
> +{
> +  char filename[PATH_MAX];
> +  sprintf (filename,
> +	  "/sys/bus/event_source/devices/cs_etm/cpu%d/trcidr/trcidr0",
> +	  cpu);

Same suggestion about not relying on a fixed-size buffer and using 
string_printf or snprintf.

> +  errno = 0;
> +  gdb_file_up file = gdb_fopen_cloexec (filename, "r");
> +  if (file.get () == nullptr)
> +    return false;
> +
> +  return true;
> +}
> +
> +/* Get etm configuration register from sys filesystem.  */
> +
> +static uint32_t
> +cs_etm_get_register (int cpu, const char *path)
> +{
> +  char filename[PATH_MAX];
> +
> +  /* Get coresight register from sysfs.  */
> +  sprintf (filename,
> +	  "/sys/bus/event_source/devices/cs_etm/cpu%d/%s",
> +	  cpu, path);

Same here.

> +  errno = 0;
> +  gdb_file_up file = gdb_fopen_cloexec (filename, "r");
> +  if (file.get () == nullptr)
> +    error (_("Failed to open %s: %s."), filename, safe_strerror (errno));
> +
> +  uint32_t val = 0;
> +
> +  int  found = fscanf (file.get (), "0x%x", &val);
> +  if (found != 1)
> +    error (_("Failed to read coresight register from %s."), filename);
> +  return val;
> +}
> +
> +#define CORESIGHT_ETM_PMU_SEED  0x10
> +
> +/* Calculate trace_id for this cpu
> +   to be kept aligned with coresight-pmu.h.  */
> +
> +static inline int
> +coresight_get_trace_id (int cpu)
> +{
> +  return (CORESIGHT_ETM_PMU_SEED + (cpu * 2));
> +}
> +
> +

Spurious newline.

> +/* PTMs ETMIDR[11:8] set to b0011.  */
> +#define ETMIDR_PTM_VERSION 0x00000300
> +
> +/* Collect and fill etm trace parameter.  */
> +
> +static void
> +fill_etm_trace_params (struct cs_etm_trace_params *etm_trace_params, int cpu)
> +{
> +  if (cs_etm_is_etmv4 (cpu) == true)
> +    {
> +      etm_trace_params->arch_ver = ARCH_V8;
> +      etm_trace_params->core_profile = profile_CortexA;
> +      etm_trace_params->protocol = OCSD_PROTOCOL_ETMV4I;
> +      /* This is the parameter passed in etm->attr.config in the call to
> +	 perf_event_open remapped according to linux/coresight-pmu.h.  */

Is the indentation of the second line of this comment block off?

> +      etm_trace_params->etmv4.reg_configr = 0;
> +      etm_trace_params->etmv4.reg_idr0
> +	 = cs_etm_get_register (cpu, "trcidr/trcidr0");
> +      etm_trace_params->etmv4.reg_idr1
> +	 = cs_etm_get_register (cpu, "trcidr/trcidr1");
> +      etm_trace_params->etmv4.reg_idr2
> +	 = cs_etm_get_register (cpu, "trcidr/trcidr2");
> +      etm_trace_params->etmv4.reg_idr8
> +	 = cs_etm_get_register (cpu, "trcidr/trcidr8");
> +      etm_trace_params->etmv4.reg_traceidr = coresight_get_trace_id (cpu);
> +    }
> +  else
> +    {
> +      etm_trace_params->arch_ver = ARCH_V7;
> +      etm_trace_params->core_profile = profile_CortexA;
> +      etm_trace_params->protocol
> +	= (etm_trace_params->etmv3.reg_idr & ETMIDR_PTM_VERSION)
> +	  == ETMIDR_PTM_VERSION ? OCSD_PROTOCOL_PTM : OCSD_PROTOCOL_ETMV3;
> +      etm_trace_params->etmv3.reg_ccer
> +	 = cs_etm_get_register (cpu, "mgmt/etmccer");
> +      etm_trace_params->etmv3.reg_ctrl
> +	 = cs_etm_get_register (cpu, "mgmt/etmcr");
> +      etm_trace_params->etmv3.reg_idr
> +	 = cs_etm_get_register (cpu, "mgmt/etmidr");
> +      etm_trace_params->etmv3.reg_trc_id
> +	 = cs_etm_get_register (cpu, "traceid");
> +    }
> +}
> +
> +static void
> +linux_fill_btrace_etm_config (struct btrace_target_info *tinfo,
> +			       struct btrace_data_etm_config *conf)
> +{
> +
> +  cs_etm_trace_params etm_trace_params;
> +  conf->num_cpu = get_cpu_count ();
> +  conf->etm_trace_params = new std::vector<cs_etm_trace_params>;
> +  for (int i = 0; i < conf->num_cpu; i++)
> +    {
> +      fill_etm_trace_params (&etm_trace_params,i);
> +      conf->etm_trace_params->push_back (etm_trace_params);
> +    }
> +
> +  conf->etm_decoder_params.formatted = 1;
> +  conf->etm_decoder_params.fsyncs = 0;
> +  conf->etm_decoder_params.hsyncs = 0;
> +  conf->etm_decoder_params.frame_aligned = 0;
> +  conf->etm_decoder_params.reset_on_4x_sync = 1;
> +}
> +
> +static enum btrace_error
> +linux_read_etm (struct btrace_data_etm *btrace,
> +		struct btrace_target_info *tinfo,
> +		enum btrace_read_type type)
> +{
> +  struct perf_event_buffer *etm;
> +  etm = &tinfo->variant.etm.etm;
> +
> +  linux_fill_btrace_etm_config (tinfo, &btrace->config);
> +
> +  switch (type)
> +    {
> +    case BTRACE_READ_DELTA:
> +      /* We don't support delta reads.  The data head (i.e. aux_head) wraps
> +	 around to stay inside the aux buffer.  */

Indentation of the second line of the comment block off?

> +      return BTRACE_ERR_NOT_SUPPORTED;
> +
> +    case BTRACE_READ_NEW:
> +      if (!perf_event_new_data (etm))
> +	return BTRACE_ERR_NONE;
> +
> +      /* Fall through.  */
> +    case BTRACE_READ_ALL:
> +      perf_event_read_all (etm, &(btrace->data),&(btrace->size));
> +      return BTRACE_ERR_NONE;
> +    }
> +
> +  internal_error (__FILE__, __LINE__, _("Unkown btrace read type."));
> +}
> +
> +
>   /* See linux-btrace.h.  */
>   
>   enum btrace_error
> @@ -924,6 +1292,14 @@ linux_read_btrace (struct btrace_data *btrace,
>         btrace->variant.pt.size = 0;
>   
>         return linux_read_pt (&btrace->variant.pt, tinfo, type);
> +
> +    case BTRACE_FORMAT_ETM:
> +      /* We read btrace in arm CoreSight ETM Trace format.  */

arm -> ARM

> +      btrace->format = BTRACE_FORMAT_ETM;
> +      btrace->variant.etm.data = NULL;
> +      btrace->variant.etm.size = 0;
> +
> +      return linux_read_etm (&btrace->variant.etm, tinfo, type);
>       }
>   
>     internal_error (__FILE__, __LINE__, _("Unkown branch trace format."));
> diff --git a/gdb/nat/linux-btrace.h b/gdb/nat/linux-btrace.h
> index 607182da144..3038d2f45a0 100644
> --- a/gdb/nat/linux-btrace.h
> +++ b/gdb/nat/linux-btrace.h
> @@ -78,6 +78,22 @@ struct btrace_tinfo_pt
>     /* The trace perf event buffer.  */
>     struct perf_event_buffer pt;
>   };
> +
> +/* Branch trace target information for ARM CoreSight ETM Trace.  */
> +struct btrace_tinfo_etm
> +{
> +  /* The Linux perf_event configuration for collecting the branch trace.  */
> +  struct perf_event_attr attr;
> +
> +  /* The perf event file.  */
> +  int file;
> +
> +  /* The perf event configuration page.  */
> +  volatile struct perf_event_mmap_page *header;

I'm not too familiar with the interface, but does this need to be 
volatile? Is this something changing this in the background?

> +
> +  /* The trace perf event buffer.  */
> +  struct perf_event_buffer etm;
> +};
>   #endif /* HAVE_LINUX_PERF_EVENT_H */
>   
>   /* Branch trace target information per thread.  */
> @@ -98,6 +114,9 @@ struct btrace_target_info
>   
>       /* CONF.FORMAT == BTRACE_FORMAT_PT.  */
>       struct btrace_tinfo_pt pt;
> +
> +    /* CONF.FORMAT == BTRACE_FORMAT_ETM.  */
> +    struct btrace_tinfo_etm etm;
>     } variant;
>   #endif /* HAVE_LINUX_PERF_EVENT_H */
>   };
> 

I'm addressing mostly formatting issues for now, as they tend to get in 
the way of proper code review. And I'm not too familiar with the branch 
tracing code.