From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1036.google.com (mail-pj1-x1036.google.com [IPv6:2607:f8b0:4864:20::1036]) by sourceware.org (Postfix) with ESMTPS id 8E27F383301D for ; Wed, 30 Jun 2021 12:26:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8E27F383301D Received: by mail-pj1-x1036.google.com with SMTP id 22-20020a17090a0c16b0290164a5354ad0so4152839pjs.2 for ; Wed, 30 Jun 2021 05:26:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=b1ovOlFh0bQzO9sq38A7P9D+vI8+QhZnn4jKM3HavPk=; b=N66AUi8Ktie3uqvKCaVlKr8ARhGqPBmbQyPqpsGf1JvqJNlLJtaq7fdE5XUYy5AQ8a NWOcCBH0/0cbefBpV8ONig7gr0bEdpwY/1Bj3nnPHVORbUB4Y0kLqTKdGw0w/30fats9 N3dKny/c6gWfCbhCiRUDDjaAfuO/L8qW6l+yNgmWt4zqisW9F8O6QYNocdPQ00RzO6u0 LKuAkWto5tP8wPF6BrGjK3y/9YmAWh+mTuiAyKhsT/pO0VPtnzECbzqUU3j8KMhy3Hts d7rZIxh6bL6rctBhB8vCsqEuOTX9xdhbNI9DUNYvd2pGuEwPnfRdZ5EhpswLs7ZhcQvE FDYw== X-Gm-Message-State: AOAM533s1yIgKOGPJBb9XfDBaq2FOHtK9O8VP8bNurqQWmKxGB0juGTK UwknqNg5toIsemxo4nXu1vjjtqI5+XHzpg== X-Google-Smtp-Source: ABdhPJz+ipxCPV8oLco2VfrMtU9LRuxpssaK2hv5v9RmJVBXs9znWbPJXExHBF28ykaJkkHDchedfA== X-Received: by 2002:a17:90a:1da3:: with SMTP id v32mr4318730pjv.192.1625056016367; Wed, 30 Jun 2021 05:26:56 -0700 (PDT) Received: from ?IPv6:2804:7f0:4841:1e6a:9c39:8c62:a64a:d289? ([2804:7f0:4841:1e6a:9c39:8c62:a64a:d289]) by smtp.gmail.com with ESMTPSA id p29sm21771223pfq.55.2021.06.30.05.26.54 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 30 Jun 2021 05:26:55 -0700 (PDT) Subject: Re: [PATCH v6 2/7] add btrace coresight related commands To: Zied Guermazi , gdb-patches@sourceware.org, markus.t.metzger@intel.com References: <20210531213307.275079-1-zied.guermazi@trande.de> <20210531213307.275079-3-zied.guermazi@trande.de> From: Luis Machado Message-ID: Date: Wed, 30 Jun 2021 09:26:52 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210531213307.275079-3-zied.guermazi@trande.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, 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: Wed, 30 Jun 2021 12:27:00 -0000 On 5/31/21 6:33 PM, Zied Guermazi wrote: > This patch extends the commands needed for using branch tracing > with ARM CoreSight traces. > Those commands are: > set record btrace etm sink > set record btrace etm buffer-size > record btrace etm > > gdb/ChangeLog > > * NEWS: list new commands for extending btrace > to support using ARM CoreSight Traces. > * record-btrace.c (record_btrace_print_etm_conf): New. > (record_btrace_print_conf): handle BTRACE_FORMAT_ETM. > (cmd_record_btrace_etm_start): New. > (cmd_record_btrace_start): handle starting ETM tracing. > (cmd_show_record_btrace_cpu): extend for ARM cpus. > (show_record_etm_buffer_size_value): New. > (_initialize_record_btrace): add commands for ETM traces. > (record_start): add starting ETM traces. > > gdb/doc/ChangeLog > > * gdb.texinfo (Process Record and Replay): Document extending > GDB btrace commands to support using ARM CoreSight traces. > > gdbsupport/ChangeLog > > * btrace-common.h (btrace_format): add BTRACE_FORMAT_ETM > to the enum. > (btrace_config_etm): new struct. > (btrace_config): add btrace_config_etm etm. > * btrace-common.cc (btrace_format_string): add BTRACE_FORMAT_ETM. > (btrace_format_short_string): add BTRACE_FORMAT_ETM. > --- > gdb/NEWS | 16 +++++ > gdb/doc/gdb.texinfo | 51 +++++++++++++++- > gdb/record-btrace.c | 117 +++++++++++++++++++++++++++++++++++- > gdb/record.c | 2 + > gdbsupport/btrace-common.cc | 6 ++ > gdbsupport/btrace-common.h | 22 ++++++- > 6 files changed, 210 insertions(+), 4 deletions(-) > > diff --git a/gdb/NEWS b/gdb/NEWS > index ab678acec8b..02035901f6e 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -2,6 +2,7 @@ > (Organized release by release) > > *** Changes since GDB 10 > +* Record btrace now supports using ARM CoreSight ETM traces. Spurious white-space above. > > * GDB now supports general memory tagging functionality if the underlying > architecture supports the proper primitives and hooks. Currently this is > @@ -77,6 +78,21 @@ > > * New commands > > +record btrace etm > +record etm > + Start branch trace recording using ARM CoreSight trace format (ETM). > + > +set|show record btrace etm buffer-size > + Set and show the size of the ring buffer used for branch tracing in > + ETM format. > + The obtained size may differ from the requested size. Use "info > + record" to see the obtained buffer size. > + > +set|show record btrace etm sink > + Set and show the trace sink used for branch tracing in > + ETM format. > + Use "default" to reset it to default sink. > + > set debug event-loop > show debug event-loop > Control the display of debug output about GDB's event loop. > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 90d827a50e7..b9b09ff97f2 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -7479,15 +7479,19 @@ For architecture environments that support process record and replay, > @kindex record btrace > @kindex record btrace bts > @kindex record btrace pt > +@kindex record btrace etm > @kindex record bts > @kindex record pt > +@kindex record etm > @kindex rec > @kindex rec full > @kindex rec btrace > @kindex rec btrace bts > @kindex rec btrace pt > +@kindex rec btrace etm > @kindex rec bts > @kindex rec pt > +@kindex rec etm > @item record @var{method} > This command starts the process record and replay target. The > recording method can be specified as parameter. Without a parameter > @@ -7501,7 +7505,7 @@ replay implementation. This method allows replaying and reverse > execution. > > @item btrace @var{format} > -Hardware-supported instruction recording, supported on Intel > +Hardware-supported instruction recording, supported on Intel and ARM > processors. This method does not record data. Further, the data is > collected in a ring buffer so old data will be overwritten when the > buffer is full. It allows limited reverse execution. Variables and > @@ -7535,6 +7539,13 @@ Decoding the recorded execution trace, on the other hand, is more > expensive than decoding @acronym{BTS} trace. This is mostly due to the > increased number of instructions to process. You should increase the > buffer-size with care. > + > +@item etm > +@cindex ARM CoreSight Trace > +Use the @dfn{ARM CoreSight Trace} recording format. In this > +format, @acronym{ETM, Extended Trace Macrocell} stores the processor > +execution trace in a compressed form that is afterwards > +decoded by @value{GDBN}. > @end table > > Not all recording formats may be available on all processors. > @@ -7682,6 +7693,12 @@ and to read-write memory. Beware that the accessed memory corresponds > to the live target and not necessarily to the current replay > position. > > +@item set record btrace etm sink @var{sink} > +Set ARM CoreSight ETM sink to collect traces. > +On @sc{gnu}/Linux systems, possible values for @var{sink} are the name of the > +files in the directory @file{/sys/bus/event_source/devices/cs_etm/sinks/}. > +Use the value @kbd{default} to reset it to default sink. > + > @item set record btrace cpu @var{identifier} > Set the processor to be used for enabling workarounds for processor > errata when decoding the trace. > @@ -7745,6 +7762,9 @@ Recorded 84872 instructions in 3189 functions (0 gaps) for thread 1 (...). > @item show record btrace replay-memory-access > Show the current setting of @code{replay-memory-access}. > > +@item show record btrace etm sink > +Show ARM CoreSight ETM sink. > + > @item show record btrace cpu > Show the processor to be used for enabling trace decode errata > workarounds. > @@ -7796,6 +7816,29 @@ also need longer to process the branch trace data before it can be used. > Show the current setting of the requested ring buffer size for branch > tracing in Intel Processor Trace format. > > +@kindex set record btrace etm > +@item set record btrace etm buffer-size @var{size} > +@itemx set record btrace etm buffer-size unlimited > +Set the requested ring buffer size for branch tracing in ARM > +CoreSight ETM Trace format. Default is 16KB. > + > +If @var{size} is a positive number, then @value{GDBN} will try to > +allocate a buffer of at least @var{size} bytes for each new thread > +that uses the btrace recording method and the ARM CoreSight ETM > +format. The actually obtained buffer size may differ from the > +requested @var{size}. Use the @code{info record} command to see the > +actual buffer size for each thread. > + > +If @var{limit} is @code{unlimited} or zero, @value{GDBN} will try to > +allocate a buffer of 4MB. > + > +Bigger buffers mean longer traces. On the other hand, @value{GDBN} will > +also need longer to process the branch trace data before it can be used. > + > +@item show record btrace etm buffer-size @var{size} > +Show the current setting of the requested ring buffer size for branch > +tracing in ARM CoreSight ETM Trace format. > + > @kindex info record > @item info record > Show various statistics about the recording depending on the recording > @@ -7847,6 +7890,12 @@ For the @code{pt} recording format, it also shows: > @item > Size of the perf ring buffer. > @end itemize > + > +For the @code{etm} recording format, it also shows: > +@itemize @bullet > +@item > +Size of the perf ring buffer. > +@end itemize > @end table > > @kindex record delete > diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c > index 00affb85d22..16ffb76272b 100644 > --- a/gdb/record-btrace.c > +++ b/gdb/record-btrace.c > @@ -201,6 +201,10 @@ static struct cmd_list_element *show_record_btrace_bts_cmdlist; > static struct cmd_list_element *set_record_btrace_pt_cmdlist; > static struct cmd_list_element *show_record_btrace_pt_cmdlist; > > +/* Command lists for "set/show record btrace etm". */ > +static struct cmd_list_element *set_record_btrace_etm_cmdlist; > +static struct cmd_list_element *show_record_btrace_etm_cmdlist; > + > /* Command list for "set record btrace cpu". */ > static struct cmd_list_element *set_record_btrace_cpu_cmdlist; > > @@ -526,6 +530,22 @@ record_btrace_print_pt_conf (const struct btrace_config_pt *conf) > } > } > > +/* Print an ARM Processor Trace configuration. */ > + > +static void > +record_btrace_print_etm_conf (const struct btrace_config_etm *conf) > +{ > + unsigned int size; > + > + size = conf->size; > + if (size > 0) > + { > + const char *suffix; > + suffix = record_btrace_adjust_size (&size); > + printf_unfiltered (_("Buffer size: %u%s.\n"), size, suffix); > + } > +} > + > /* Print a branch tracing configuration. */ > > static void > @@ -546,6 +566,10 @@ record_btrace_print_conf (const struct btrace_config *conf) > case BTRACE_FORMAT_PT: > record_btrace_print_pt_conf (&conf->pt); > return; > + > + case BTRACE_FORMAT_ETM: > + record_btrace_print_etm_conf (&conf->etm); > + return; > } > > internal_error (__FILE__, __LINE__, _("Unknown branch trace format.")); > @@ -2676,7 +2700,7 @@ record_btrace_target::stop (ptid_t ptid) > tp->btrace.flags |= BTHR_STOP; > } > } > - } > +} > I keep seeing the above change. It looks spurious. > /* The can_execute_reverse method of target record-btrace. */ > > @@ -2937,6 +2961,27 @@ cmd_record_btrace_pt_start (const char *args, int from_tty) > } > } > > +/* Start recording in ARM CoreSight ETM Trace format. */ > + > +static void > +cmd_record_btrace_etm_start (const char *args, int from_tty) > +{ > + if (args != nullptr && *args != 0) > + error (_("Invalid argument.")); > + > + record_btrace_conf.format = BTRACE_FORMAT_ETM; > + > + try > + { > + execute_command ("target record-btrace", from_tty); > + } > + catch (const gdb_exception &exception) > + { > + record_btrace_conf.format = BTRACE_FORMAT_NONE; > + throw; > + } > +} > + > /* Alias for "target record". */ > > static void > @@ -2951,10 +2996,18 @@ cmd_record_btrace_start (const char *args, int from_tty) > { > execute_command ("target record-btrace", from_tty); > } > - catch (const gdb_exception &exception) > + catch (const gdb_exception &exception_pt) > { > record_btrace_conf.format = BTRACE_FORMAT_BTS; > > + try > + { > + execute_command ("target record-btrace", from_tty); > + } > + catch (const gdb_exception &exception_bts) > + { > + record_btrace_conf.format = BTRACE_FORMAT_ETM; > + > try > { > execute_command ("target record-btrace", from_tty); > @@ -2965,6 +3018,7 @@ cmd_record_btrace_start (const char *args, int from_tty) > throw; > } > } > + } > } > > /* The "show record btrace replay-memory-access" command. */ > @@ -3103,6 +3157,17 @@ show_record_pt_buffer_size_value (struct ui_file *file, int from_tty, > value); > } > > +/* The "record etm buffer-size" show value function. */ > + > +static void > +show_record_etm_buffer_size_value (struct ui_file *file, int from_tty, > + struct cmd_list_element *c, > + const char *value) > +{ > + fprintf_filtered (file, _("The record/replay etm buffer size is %s.\n"), > + value); > +} > + > /* Initialize btrace commands. */ > > void _initialize_record_btrace (); > @@ -3170,6 +3235,15 @@ When set to \"none\", errata workarounds are disabled."), > 1, > &set_record_btrace_cmdlist); > > + > + cmd_list_element *record_btrace_etm_cmd > + = add_cmd ("etm", class_obscure, cmd_record_btrace_etm_start, > + _("\ > +Start branch trace recording in ARM CoreSight ETM Trace format.\n\n\ > +This format may not be available on all processors."), > + &record_btrace_cmdlist); > + add_alias_cmd ("etm", record_btrace_etm_cmd, class_obscure, 1, &record_cmdlist); > + > add_cmd ("auto", class_support, cmd_set_record_btrace_cpu_auto, _("\ > Automatically determine the cpu to be used for trace decode."), > &set_record_btrace_cpu_cmdlist); > @@ -3231,6 +3305,43 @@ to see the actual buffer size."), NULL, show_record_pt_buffer_size_value, > &set_record_btrace_pt_cmdlist, > &show_record_btrace_pt_cmdlist); > > + add_basic_prefix_cmd ("etm", class_support, > + _("Set record btrace etm options."), > + &set_record_btrace_etm_cmdlist, > + 0, > + &set_record_btrace_cmdlist); > + > + add_show_prefix_cmd ("etm", class_support, > + _("Show record btrace etm options."), > + &show_record_btrace_etm_cmdlist, > + 0, > + &show_record_btrace_cmdlist); > + > + add_setshow_uinteger_cmd ("buffer-size", no_class, > + &record_btrace_conf.etm.size, > + _("Set the record/replay etm buffer size."), > + _("Show the record/replay etm buffer size."), _("\ > +Bigger buffers allow longer recording but also take more time to process \ > +the recorded execution.\n\ > +The actual buffer size may differ from the requested size. Use \"info record\"\ > + to see the actual buffer size."), NULL, show_record_etm_buffer_size_value, This is inconsistent with what the documentation says: "Bigger buffers mean longer traces. On the other hand, @value{GDBN} will also need longer to process the branch trace data before it can be used." How about using the same text so things are consistent across the codebase? > + &set_record_btrace_etm_cmdlist, > + &show_record_btrace_etm_cmdlist); > + > + add_setshow_string_cmd ("sink", no_class, > + &record_btrace_conf.etm.sink, > + _("Set the record/replay ETM sink device."), > + _("Show the record/replay ETM sink device."), > + _("\ > +Sink device is the device that intercepts ETM traces and collects or routes \ > +them out of the System on Chip.\n\ > +The list of available sinks on linux targets corresponds to the files in \ > +the directory \"/sys/bus/event_source/devices/cs_etm/sinks/\".\n\ > +value \"default\" reset it to default sink"), > + NULL, NULL, > + &set_record_btrace_etm_cmdlist, > + &show_record_btrace_etm_cmdlist); > + > add_target (record_btrace_target_info, record_btrace_target_open); > > bfcache = htab_create_alloc (50, bfcache_hash, bfcache_eq, NULL, > @@ -3238,4 +3349,6 @@ to see the actual buffer size."), NULL, show_record_pt_buffer_size_value, > > record_btrace_conf.bts.size = 64 * 1024; > record_btrace_conf.pt.size = 16 * 1024; > +#define DEFAULT_ETM_BUFFER_SIZE (8 * 1024) > + record_btrace_conf.etm.size = DEFAULT_ETM_BUFFER_SIZE; > } > diff --git a/gdb/record.c b/gdb/record.c > index 6968c30d930..32492fb53c4 100644 > --- a/gdb/record.c > +++ b/gdb/record.c > @@ -118,6 +118,8 @@ record_start (const char *method, const char *format, int from_tty) > execute_command_to_string ("record btrace bts", from_tty, false); > else if (strcmp (format, "pt") == 0) > execute_command_to_string ("record btrace pt", from_tty, false); > + else if (strcmp (format, "etm") == 0) > + execute_command_to_string ("record btrace etm", from_tty, false); > else > error (_("Invalid format.")); > } > diff --git a/gdbsupport/btrace-common.cc b/gdbsupport/btrace-common.cc > index 4f9ef855e74..79ff21de44b 100644 > --- a/gdbsupport/btrace-common.cc > +++ b/gdbsupport/btrace-common.cc > @@ -36,6 +36,9 @@ btrace_format_string (enum btrace_format format) > > case BTRACE_FORMAT_PT: > return _("Intel Processor Trace"); > + > + case BTRACE_FORMAT_ETM: > + return _("ARM Processor CoreSight ETM Trace"); > } > > internal_error (__FILE__, __LINE__, _("Unknown branch trace format")); > @@ -56,6 +59,9 @@ btrace_format_short_string (enum btrace_format format) > > case BTRACE_FORMAT_PT: > return "pt"; > + > + case BTRACE_FORMAT_ETM: > + return "etm"; > } > > internal_error (__FILE__, __LINE__, _("Unknown branch trace format")); > diff --git a/gdbsupport/btrace-common.h b/gdbsupport/btrace-common.h > index 26d26ec957f..153b977723a 100644 > --- a/gdbsupport/btrace-common.h > +++ b/gdbsupport/btrace-common.h > @@ -63,7 +63,10 @@ enum btrace_format > BTRACE_FORMAT_BTS, > > /* Branch trace is in Intel Processor Trace format. */ > - BTRACE_FORMAT_PT > + BTRACE_FORMAT_PT, > + > + /* Branch trace is ARM CoreSight ETM format. */ > + BTRACE_FORMAT_ETM > }; > > /* An enumeration of cpu vendors. */ > @@ -119,6 +122,20 @@ struct btrace_config_pt > unsigned int size; > }; > > +/* An ARM CoreSight ETM Trace configuration. */ > + > +struct btrace_config_etm > +{ > + /* The size of the branch trace buffer in bytes. > + > + This is unsigned int and not size_t since it is registered as > + control variable for "set record btrace etm buffer-size". */ > + unsigned int size; > + > + /* The sink used to collect the traces. */ > + char *sink; > +}; > + > /* A branch tracing configuration. > > This describes the requested configuration as well as the actually > @@ -136,6 +153,9 @@ struct btrace_config > > /* The Intel Processor Trace format configuration. */ > struct btrace_config_pt pt; > + > + /* The ARM CoreSight ETM Trace configuration. */ > + struct btrace_config_etm etm; > }; > > /* Branch trace in BTS format. */ > Other than those nits, I have no further comments on this.