From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nx202.node01.secure-mailgate.com (nx202.node01.secure-mailgate.com [89.22.108.202]) by sourceware.org (Postfix) with ESMTPS id 32D94385701F for ; Fri, 30 Apr 2021 01:42:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 32D94385701F 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 1lcIB4-001ePu-At; Fri, 30 Apr 2021 03:42:48 +0200 X-SecureMailgate-Identity: zied.guermazi@trande.de;host202.checkdomain.de Received: from [192.168.178.48] (x4db5918e.dyn.telefonica.de [77.181.145.142]) (Authenticated sender: zied.guermazi@trande.de) by host202.checkdomain.de (Postfix) with ESMTPSA id 9EB742C089B; Fri, 30 Apr 2021 03:42:42 +0200 (CEST) X-SecureMailgate-Identity: zied.guermazi@trande.de;host202.checkdomain.de From: Zied Guermazi Subject: Re: [PATCH v5 2/7] add btrace coresight related commands To: "Metzger, Markus T" , "gdb-patches@sourceware.org" References: <20210422140921.175221-1-zied.guermazi@trande.de> <20210422140921.175221-3-zied.guermazi@trande.de> Message-ID: <9818150c-eac6-0044-e471-70ed9a8ae45f@trande.de> Date: Fri, 30 Apr 2021 03:42:41 +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: Content-Language: en-US X-PPP-Message-ID: <20210430014243.4056630.54623@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 (0.00590371463744) X-Recommended-Action: accept X-Filter-ID: Pt3MvcO5N4iKaDQ5O6lkdGlMVN6RH8bjRMzItlySaT8fqGb7Az7xKLBS5NTAnBexPUtbdvnXkggZ 3YnVId/Y5jcf0yeVQAvfjHznO7+bT5xepKJyYLZISh0ScmnJxtOQL++T0fzDkcbraJCnPSgcQrqW ghmgrvbuYBMbgPFqhEjKF590ee3aGyyzch48U9RYPumU89c8OaxcWLmp7ynpEgdonvoEbZH8AWDS ZIEQa1LasE0yYSwoSJtDBJTXEF44+qKA9WNimSzzMEEnsLmJnD3bQmEqUddg4pDcHxi62gxzSOZO aYOlkYc6lCBjGxXRtrdIeJGUu23VykQIijZdTE7fgWvyBrZ2+6kCnP/WBDJy8oFzlZDzSFMZMCEJ KqX3DU5uCP5D/CUxYBzL65HHiH6YFrhxVw5i30CZMe0+JKoB15eMIME6iZU0/33rc4pTBldNoc4/ ugpbufbubK+x4vA/QHsSr98zh4ylHHSOvF4EizIlMC06U8aFKFbkglvNo7UbuWzwvM5eRjzlfUYD MWFgOK8gW+hazh84oBeMtGGs+REr5it4yuwWmXN5FmKajBUnmlmmmmIH8Dg80WcRp+fC5fGvBaRW nWe6WFte+h1HBQQL7n9+OTZ37LZfI/sW30YZMgUX6Z4ThlOFxTu4LUsF4GHuFWly2YtZO0AGnGQ3 T2YxzxB8woiuExubdcsml481x58OeqpgxEwDRkEDKwPt8rNcco5Lt6ku63DkksyWUQhsiRV7ToHt 3HSONnNjcxw3qqhc+N6cuEg4XWh5Fqb0FldYqp+kcqm+1aM0u7RGQPqAhPdEtGO1frzSxSH3UAJu OhPt8k0439dzdDl8WugZ8kRW3yj6wqwjQ36QzRPL7Rp7sTyiVa8HG50AexxrzznCpn9MRUzzcEJz OzmIv9M+m4WpRRDP6YzwkAPgQJbZw2WK4NzRiZTDVGmt15BzNCflaPsIqsBW58vnzO6f85EiPfuL SYcjdfe84YYON4oVu6y/RUg6Y3rEGP/GV8NgctgzcDoFd+96Xw4QUNtTnZZhiw7af43YPLSeyfeI qT7OqoqMIvCpZajrvxuS+2n0bmim7U+d9AZiaR2WxIbRcfP4wHlqBE95R7lrKCg7FGkTm3FqRDxU q50C08gBiOmGBiFMXZsNOH+1R01VtZLf6H6YFrhxVw5i30CZMe0+JKqWmKu/Exva/4bO4tpXcmDO vZQjnSGPGpsSE/xM+RnyGGWi2HOzq/csyFjkk1RUX5M= X-Report-Abuse-To: spam@node04.secure-mailgate.com X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00, FOREIGN_BODY1, GIT_PATCH_0, HTML_MESSAGE, JMQ_SPF_NEUTRAL, 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 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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: Fri, 30 Apr 2021 01:42:53 -0000 hi Markus, thanks for your review, here is the status of the rework as it will be published in next version of the patchset. On 27.04.21 19:31, Metzger, Markus T wrote: > Hello Zied, > >> 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 > This patch looks good to me. I'll skip it in future reviews. Please point > out updates explicitly. > > See a few nits below. > > >> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo >> index d13cbda2c11..48627d5d71a 100644 >> --- a/gdb/doc/gdb.texinfo >> +++ b/gdb/doc/gdb.texinfo >> @@ -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. > Is this 4MB or rather 4GB? [Zied] I didn't dig into the value set if unlimited is used. I used the same wording as in pt and bts since the same processing was done. >> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c >> index b7b3c91f85d..83768694193 100644 >> --- a/gdb/record-btrace.c >> +++ b/gdb/record-btrace.c >> @@ -2675,7 +2699,7 @@ record_btrace_target::stop (ptid_t ptid) >> tp->btrace.flags |= BTHR_STOP; >> } >> } >> - } >> +} > That looks odd. What changed? [Zied] while visiting the function to check if it needs any rework for etm support, noticed the leading space and fixed it. >> @@ -3233,4 +3342,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; >> } > Doesn't look like this macro is used anywhere else so I don't quite see the > point of it. [Zied] the macro was defined based on another review, it was suggested to replace "magic numbers" by constants describing its meaning. >> 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 >> @@ -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; > Should this be const char *? Who would free the memory used to > store those sink names? [Zied] sink can change if the user decide t use a different sink at the next run. this can be done through the command "record etm sink" I assume that add_setshow_string_cmd will free the old value, is my assumption true here? > Regards, > Markus. > Intel Deutschland GmbH > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0,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 > -- *Zied Guermazi* founder Trande UG Leuschnerstraße 2 69469 Weinheim/Germany Mobile: +491722645127 mailto:zied.guermazi@trande.de *Trande UG* Leuschnerstraße 2, D-69469 Weinheim; Telefon: +491722645127 Sitz der Gesellschaft: Weinheim- Registergericht: AG Mannheim HRB 736209 - Geschäftsführung: Zied Guermazi *Confidentiality Note* This message is intended only for the use of the named recipient(s) and may contain confidential and/or privileged information. If you are not the intended recipient, please contact the sender and delete the message. Any unauthorized use of the information contained in this message is prohibited.