From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nx244.node01.secure-mailgate.com (nx244.node01.secure-mailgate.com [89.22.108.244]) by sourceware.org (Postfix) with ESMTPS id A2E1F385782D for ; Thu, 1 Apr 2021 21:57:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A2E1F385782D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=trande.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=zied.guermazi@trande.de Received: from host202.checkdomain.de ([185.137.168.148]) by node01.secure-mailgate.com with esmtps (TLSv1.2:AES128-GCM-SHA256:128) (Exim 4.92) (envelope-from ) id 1lS5JS-00HVGg-Is; Thu, 01 Apr 2021 23:57:15 +0200 X-SecureMailgate-Identity: host202.checkdomain.de Received: from [192.168.178.48] (x4dbd0141.dyn.telefonica.de [77.189.1.65]) (Authenticated sender: zied.guermazi@trande.de) by host202.checkdomain.de (Postfix) with ESMTPSA id 96CC02C073C; Thu, 1 Apr 2021 23:57:13 +0200 (CEST) X-SecureMailgate-Identity: host202.checkdomain.de Subject: Re: [PATCH v3 4/7] start/stop btrace with coresight etm and collect etm buffer on linux os To: Luis Machado , gdb-patches@sourceware.org References: <20210331025234.518688-1-zied.guermazi@trande.de> <20210331025234.518688-5-zied.guermazi@trande.de> <39782042-9993-d86c-4251-389fba89e1c0@linaro.org> From: Zied Guermazi Message-ID: Date: Thu, 1 Apr 2021 23:57:13 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <39782042-9993-d86c-4251-389fba89e1c0@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-PPP-Message-ID: <20210401215713.2809268.37337@host202.checkdomain.de> X-PPP-Vhost: trande.de X-Originating-IP: 185.137.168.148 X-SecureMailgate-Domain: host202.checkdomain.de X-SecureMailgate-Username: 185.137.168.148 Authentication-Results: secure-mailgate.com; auth=pass smtp.auth=185.137.168.148@host202.checkdomain.de X-SecureMailgate-Outgoing-Class: ham X-SecureMailgate-Outgoing-Evidence: SB/global_tokens (2.31968291242e-08) X-Recommended-Action: accept X-Filter-ID: Pt3MvcO5N4iKaDQ5O6lkdGlMVN6RH8bjRMzItlySaT/wx11L8fQa3X80dstyn5GJPUtbdvnXkggZ 3YnVId/Y5jcf0yeVQAvfjHznO7+bT5wqBcSGeVWq1E9YWZbsmhKMTlod1WpdzSJAbIL3qp5JunYf gJ55pDnOxWzxymfhhAUcEJUqtZn698k7kZCd7zwAAXDkcXwqI3EzqRI7b1h6drsao9SIRuaLqqNv +4CAXfk4PWJM2zMugzmIRtzFHL4aIHvmTkWMar5O6fTWZjnz42zs58aD+siDxcMiaATm5f1wASW+ oIwg9Aszfzu0AKyQ5J4klJb2SW3YBIP367JKMw9hqzzbauOeEde3lGnV/K3IYkWNfDPPLuCBXxjv OoEhn1lcwZJQZfMwrNqFYxEyPMW2K9b7rjx99/aQfurBJHE5Y0xQrSl8uS7UqueZ+Cf+0j6m8Z3T buDICLL8IEpevtfsPev7LNp/T0V6wcNiJWH6ZboosCGGDrdx/yGEn66R4VdJNCnCfvvZZI62RAzO krSLTkVvpgY07hnXq92QpIgWqQcrkdncBVBujWllVdXQX7semJdl8Weuy8+tlMSTQ04uitUf42Cc Hl6wE8LbH+hYFX5W98JZfM4UDzp+I2t3MybRJ2S+0+L4irozIy0W6vBmrmMhaNZpatyubonvXmlF QVRTn8m0sooRS7h6xdEgKTjECb0PwpN4olPuA0AI936c0SM84BxzaZIqKXfxGEKqa774GDzsGL98 VkV/RBCLWNiHearbSjjPs0YW4Sm/WqFQcGdCvFTuRAGl7A4EfuvLOJdNROjBFOR5AT6YXc2lmm6k GBmsfkzbk17qXdMwdItNX9A+pfW0sdzVksWgdzKv1KPcaedUtYK7db97bYwOM1dok83D0FHpk3bd 7HbB1JmyCtlKNfU+5VB9yzvTGUKdo8pzfJC/FCbMqwCJeIFsHHO0xOXvjuAr2yQK0KxU7x/ieAuY rQs5ttmJcF7JASnLYgiEbYKHJ/ADC/yb4BnnWkDcjLEI0dEZkTYJFSI3M1KxHB/etGG+jKYi/tvO WyGZRTiqekhsLIBPcSqDTZjPSTQd7aI/Gk6NGQq2doWnVT3JqEqUUR2dHxpQKlcvmQfIYkWNfDPP LuCBXxjvOoEhSPiM/k+PIuCYPw2AQTZwTLTzSml8REK0q3OdzzhjQvUVZjUFgdVrb53pIBbvvIld R7d4CWpSt/sm/gYhmNcISA== X-Report-Abuse-To: spam@node04.secure-mailgate.com X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00, BODY_8BITS, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Apr 2021 21:57:20 -0000 hi Luis thanks for your review comments. Here is the status of the updates. The changes will be published in next version of the patch set. /Zied On 01.04.21 14:16, Luis Machado wrote: > 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  >> + >> +    * 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  >>         * 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 >>     #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. [Zied] not having an active tracing session is handled in btrace_disable in btrace.c. so it is not being able to disable branch tracing. > >> +} >> + >> +/* 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 >>   +#if defined (HAVE_LIBOPENCSD_C_API) >> +#  include >> +#endif >> + >>   #if HAVE_LINUX_PERF_EVENT_H && defined(SYS_perf_event_open) >>   #include >>   #include >> @@ -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. [Zied]  done > >> +  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 [Zied] done > >> +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 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 [Zied] done > >> +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. [Zied] fixed >> + >> +  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'. > [Zied] done >> +  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. [Zied] done > >> +  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. [Zied] done > >> +  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. [Zied] done > >> +/* 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? [Zied] no. it is fine. 'p' of perf is below/ at the same column than 'T' of This > >> + 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; >> +  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? [Zied] no, it is fine. 'a' of around is below/ at the same column than 'W' of We > >> +      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 [Zied] fixed > >> +      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? [Zied], yes it is a buffer shared ring buffer between kernel space and user space. kernel writes to it. We have to set it to volatile to prevent any optimization/caching in registers > >> + >> +  /* 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.