From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88908 invoked by alias); 23 Feb 2018 13:52:29 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 88239 invoked by uid 89); 23 Feb 2018 13:52:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=family X-HELO: eggs.gnu.org Received: from eggs.gnu.org (HELO eggs.gnu.org) (208.118.235.92) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 23 Feb 2018 13:52:26 +0000 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1epDls-0007vo-AA for gdb-patches@sourceware.org; Fri, 23 Feb 2018 08:52:24 -0500 Received: from fencepost.gnu.org ([2001:4830:134:3::e]:42543) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1epDls-0007vb-5d; Fri, 23 Feb 2018 08:52:20 -0500 Received: from [176.228.60.248] (port=3364 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1epDlr-0005fJ-IM; Fri, 23 Feb 2018 08:52:20 -0500 Date: Fri, 23 Feb 2018 13:52:00 -0000 Message-Id: <83woz34xuj.fsf@gnu.org> From: Eli Zaretskii To: Markus Metzger CC: gdb-patches@sourceware.org In-reply-to: <1519379570-16643-2-git-send-email-markus.t.metzger@intel.com> (message from Markus Metzger on Fri, 23 Feb 2018 10:52:50 +0100) Subject: Re: [PATCH 2/2] btrace: set/show record btrace cpu Reply-to: Eli Zaretskii References: <1519379570-16643-1-git-send-email-markus.t.metzger@intel.com> <1519379570-16643-2-git-send-email-markus.t.metzger@intel.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg00343.txt.bz2 > From: Markus Metzger > Date: Fri, 23 Feb 2018 10:52:50 +0100 > > Add new set/show commands to set the processor that is used for enabling errata > workarounds when decoding branch trace. Thanks. > The general format is ": " but we also allow two special > values "auto" and "none". > > The default is "auto", which is the current behaviour of having GDB determine > the processor on which the trace was recorded. > > If that cpu is not known to the trace decoder, e.g. when using an old decoder on > a new system, decode may fail with "unknown cpu". In most cases it should > suffice to 'downgrade' decode to assume an older cpu. Unfortunately, we can't > do this automatically. This is useful information that is entirely missing from the patch to the manual. As a general rule, if you find something you need to say in the patch log message in order to describe it to this list, it's almost certain the same text should be in the manual, as the manual will be read by folks who are not unlike the readers of this list. > +* New options > + > +set record btrace cpu > +show record btrace cpu > + Controls the processor to be used for enabling errata workarounds for branch > + trace decode. This part is OK. > +@item set record btrace cpu @var{identifier} > +Set the processor to be used for enabling trace decode errata > +workarounds. I think we need to say something about just what those "errata workarounds" are, and what are they used for. > The general @var{identifier} format is a vendor > +identifier followed by a vendor-specific processor identifier. This fails to mention the colon delimiter, and in general it is better to just show the form, rather than describe it. Like this: The argument @var{identifier} identifies the @sc{cpu}, and is of the form @code{@var{vendor}:@var{family}/@var{model}@r{[}/@var{stepping}@r{]}. > +The following vendor identifiers and corresponding processor > +identifiers are currently supported: > + > +@multitable @columnfractions .1 .9 > + > +@item @code{intel} > +@tab @var{family}/@var{model}[/@var{stepping}] I think we need to tell a bit more about @var{family} and @var{model} here, and/or maybe tell the readers how to obtain that information. > +If @var{identifier} is @code{auto}, enable errata workarounds for the > +processor on which the trace was recorded. If @var{identifier} is > +@code{none}, errata workarounds are disabled. This should mention what you described in the log message above, and also tell what does "disabled" mean in practice (or maybe this will become clear when you explain what are the errata workarounds about). > +@smallexample > +(gdb) info record > +Active record target: record-btrace > +Recording format: Intel Processor Trace. > +Buffer size: 16kB. > +Failed to configure the Intel Processor Trace decoder: unknown cpu. > +(gdb) set record btrace cpu intel:6/158 > +(gdb) info record > +Active record target: record-btrace > +Recording format: Intel Processor Trace. > +Buffer size: 16kB. > +Recorded 84872 instructions in 3189 functions (0 gaps) for thread 1 (...). > +@end smallexample This is a long example, and it contains several parts. If we care about possible division of the example between pages, we should use @group..@end group around the parts that we want to be on the same page. > + add_prefix_cmd ("cpu", class_support, cmd_set_record_btrace_cpu, > + _("\ > +Set the cpu to be used for trace decode.\n\n\ > +The format is \": \" or \"none\" or \"auto\" (default). ^^ So should there be a blank after the colon, or shouldn't there be? The example in the manual says no blank. > +The default is AUTO, which uses the cpu on which the trace was recorded.\n\ ^^^^ Above you used "auto", quoted and in lower case. > +When GDB does not support that cpu, this option can be used to enable\n\ > +workarounds for a similar cpu that GDB supports.\n\n\ This trick is not in the manual; it should be, IMO.