[PATCH v5 4/7] start/stop btrace with coresight etm and collect etm buffer on linux os
Metzger, Markus T
markus.t.metzger@intel.com
Tue Apr 27 17:31:50 GMT 2021
Hello Zied,
>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.
>diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
>index 324f7ef0407..dabf6a14c29 100644
>--- a/gdb/nat/linux-btrace.c
>+++ b/gdb/nat/linux-btrace.c
>@@ -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;
This is an independent improvement. You can send this in a separate
patch, if you want.
We should check the return value of sysconf (), though, and fall back
to PAGE_SIZE on errors; maybe with a one-time warning that gives error
details.
>@@ -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,
Same here.
>+ if (conf->sink != nullptr)
>+ {
>+ if (strcmp (conf->sink, "default")!=0)
Spaces around !=.
>+ /* 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));
All that perf_event setup code looks very similar to PT. Could we share
all that?
>@@ -726,8 +885,22 @@ linux_disable_bts (struct btrace_tinfo_bts *tinfo)
> static enum btrace_error
> linux_disable_pt (struct btrace_tinfo_pt *tinfo)
> {
>- munmap((void *) tinfo->pt.mem, tinfo->pt.size);
>- munmap((void *) tinfo->header, PAGE_SIZE);
>+ long page_size = sysconf (_SC_PAGESIZE);
>+ munmap ((void *) tinfo->pt.mem, tinfo->pt.size);
>+ munmap ((void *) tinfo->header, page_size);
Maybe it makes sense to store the page size we used for the mmap so
we don't need to look it up again - and can be sure we use the same value.
>@@ -898,6 +1075,194 @@ 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;
Please declare on first use.
>+
>+ fseek (file.get (), 0, SEEK_END);
>+ length = ftell (file.get ());
>+ fseek (file.get (), 0, SEEK_SET);
>+
>+ char *buffer;
Same here.
>+
>+ buffer = (char *) xmalloc (length+1);
>+
>+ length = fread (buffer, 1, length, file.get ());
>+ buffer[length]='\0';
>+ while (--length)
Please use explicit comparisons against zero.
>+ {
>+ if ((buffer[length] == ',') || (buffer[length] == '-'))
>+ {
>+ length++;
>+ break;
>+ }
>+ }
>+
>+ int cpu_count;
>+
>+ if (sscanf (&buffer[length], "%d", &cpu_count) < 1)
Please move the sscanf () call outside of the if expression.
>+ error (_("Failed to get cpu count in %s: %s."),
>+ buffer, safe_strerror (errno));
>+
>+ cpu_count ++;
You may want to add a comment that the kernel starts enumerating
cpus at zero.
>+ return (cpu_count);
>+}
>+
>+/* Check if the ETM is an etmv4. */
>+
>+static bool
>+cs_etm_is_etmv4 (int cpu)
>+{
>+ char filename[PATH_MAX];
>+ snprintf (filename, PATH_MAX,
>+ "/sys/bus/event_source/devices/cs_etm/cpu%d/trcidr/trcidr0", cpu);
>+ errno = 0;
>+ gdb_file_up file = gdb_fopen_cloexec (filename, "r");
>+ if (file.get () == nullptr)
>+ return false;
>+
>+ return true;
Wouldn't other (future) versions re-use the infrastructure?
>+}
>+
>+/* Get etm configuration register from sys filesystem. */
You use the term sysfs below. Let's use it here, as well.
>+#define CORESIGHT_ETM_PMU_SEED 0x10
>+
>+/* Calculate trace_id for this cpu
>+ to be kept aligned with coresight-pmu.h. */
Can we include that header?
>+/* 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)
No need for comparing against bools.
>+ else
>+ {
>+ etm_trace_params->arch_ver = ARCH_V7;
>+ etm_trace_params->core_profile = profile_CortexA;
I don't think that's safe to assume that not v4 automatically means v3.
Can we check for v3 explicitly?
>+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->cpu_count = get_cpu_count ();
>+ conf->etm_trace_params = new std::vector<cs_etm_trace_params>;
>+ for (int i = 0; i < conf->cpu_count; i++)
>+ {
>+ fill_etm_trace_params (&etm_trace_params,i);
Can this throw? We just allocated memory.
>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;
>+
>+ /* The trace perf event buffer. */
>+ struct perf_event_buffer etm;
>+};
Maybe we can share that, too.
regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
More information about the Gdb-patches
mailing list