[{"id":1768026,"web_url":"http://patchwork.ozlabs.org/comment/1768026/","msgid":"","list_archive_url":null,"date":"2017-09-13T16:59:09","subject":"Re: [Qemu-devel] [PATCH 1/7] trace: Add event \"guest_bbl_before\"","submitter":{"id":72104,"url":"http://patchwork.ozlabs.org/api/people/72104/","name":"Richard Henderson","email":"richard.henderson@linaro.org"},"content":"On 09/10/2017 09:15 AM, Lluís Vilanova wrote:\n> Signed-off-by: Lluís Vilanova \n> ---\n> accel/tcg/translator.c | 6 ++++++\n> trace-events | 11 +++++++++++\n> 2 files changed, 17 insertions(+)\n> \n> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c\n> index afa3af478a..91b3b0da32 100644\n> --- a/accel/tcg/translator.c\n> +++ b/accel/tcg/translator.c\n> @@ -13,6 +13,7 @@\n> #include \"cpu.h\"\n> #include \"tcg/tcg.h\"\n> #include \"tcg/tcg-op.h\"\n> +#include \"trace-tcg.h\"\n> #include \"exec/exec-all.h\"\n> #include \"exec/gen-icount.h\"\n> #include \"exec/log.h\"\n> @@ -91,6 +92,11 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,\n> }\n> }\n> \n> + /* Tracing before */\n> + if (db->num_insns == 1) {\n> + trace_guest_bbl_before_tcg(cpu, tcg_ctx.tcg_env, db->pc_first);\n> + }\n\nWhy not place this before the loop, so that you don't\nhave to check num_insns == 1?\n\n> +vcpu tcg guest_bbl_before(uint64_t vaddr) \"vaddr=0x%016\"PRIx64, \"vaddr=0x%016\"PRIx64\n\nYou're really going to print both ENV and PC tagged with \"vaddr\"?\nThat just seems confusing.\n\nAlso, terminology. A \"basic block\" (\"bb\" by preference, not \"bbl\"), has a\nspecific meaning (https://en.wikipedia.org/wiki/Basic_block). What we're\ngenerating here is a TranslationBlock (which may consist of many basic blocks),\nand oft contracted within the source as \"tb\".\n\n\nr~","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"JDmfLNmt\"; dkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsnwD69h8z9s9Y\n\tfor ;\n\tThu, 14 Sep 2017 03:00:12 +1000 (AEST)","from localhost ([::1]:43672 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dsB1H-0004RO-1L\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Sep 2017 13:00:11 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:51049)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dsB0O-0003wG-22\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 12:59:17 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dsB0L-00041f-Hx\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 12:59:16 -0400","from mail-pg0-x22b.google.com ([2607:f8b0:400e:c05::22b]:47704)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from )\n\tid 1dsB0L-000415-7N\n\tfor qemu-devel@nongnu.org; Wed, 13 Sep 2017 12:59:13 -0400","by mail-pg0-x22b.google.com with SMTP id d8so1455150pgt.4\n\tfor ; Wed, 13 Sep 2017 09:59:13 -0700 (PDT)","from bigtime.twiddle.net (97-126-103-167.tukw.qwest.net.\n\t[97.126.103.167]) by smtp.gmail.com with ESMTPSA id\n\ts4sm12598578pgr.32.2017.09.13.09.59.10\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 13 Sep 2017 09:59:11 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=vZ81GyPfLJorjiG0YNFGXmh0nTNM4Ps/tIEAPCKivIQ=;\n\tb=JDmfLNmtokBzjNjYgC4g43Xe6lpmzQ7EKsVcP0w/1PTO28c8kDlBouBpzqzHdYaBsu\n\te7EfDIr09STQGcbquNXNaQpr6b6ewPx9jFlsSCA15qDF2805P2U98eiHISjtAREF/HPt\n\tVFXqez6GegI9mYZW5c49iaUhM+kd5LljLDDoc=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=vZ81GyPfLJorjiG0YNFGXmh0nTNM4Ps/tIEAPCKivIQ=;\n\tb=JQAYxn6wYQBIaG6bMbuNjwjVkXstI0lULmBfVWpuy5TDh0DHyKdJkBjfrnPn6e9AM4\n\tU9A1pZEnx0XmwOwHJ1Bqlb14PYbf6ogmUA4+ByZVVxHw+uqk8KdSDZfeyzff25Lq9EZE\n\t0etIC2/IzY2UxBNw9VLU0Re7j91gjy568M1z9bQ/inmQYDja+Sj9DlXPQvfEwk64VmuE\n\tCOFuN6sut0+eXI5vkQGp+a3gRsxRABgA3+3AQvpx1gwUDSZwa0T0FTyBaWdv6/VWbBFx\n\tMP1gcG6g+BzT5F7K7NtysRQ9rQckuVyXOtFBhB1IvrWNMu+L9D5jLb1+ifODG6wcGJER\n\tGCng==","X-Gm-Message-State":"AHPjjUhQxNZ47jiUI/ZJjBXNQTGpmcr1YzXBANh27TMEtRQnFO7EX7CE\n\tKMbwJSn+pB8u5cJD","X-Google-Smtp-Source":"ADKCNb6IjqpQfcpdBeewsfFieOX5/SZbM+XzVfWAmlX+JPHaKlbUWCPGUXDwXN6IVD/pZHWgLrAZEw==","X-Received":"by 10.98.81.2 with SMTP id f2mr18836276pfb.190.1505321952237;\n\tWed, 13 Sep 2017 09:59:12 -0700 (PDT)","To":"=?utf-8?q?Llu=C3=ADs_Vilanova?= ,\n\tqemu-devel@nongnu.org","References":"<150505986682.19604.11937392314067517230.stgit@frigg.lan>\n\t<150506010962.19604.7773376130922107594.stgit@frigg.lan>","From":"Richard Henderson ","Message-ID":"","Date":"Wed, 13 Sep 2017 09:59:09 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<150506010962.19604.7773376130922107594.stgit@frigg.lan>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2607:f8b0:400e:c05::22b","Subject":"Re: [Qemu-devel] [PATCH 1/7] trace: Add event \"guest_bbl_before\"","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Paolo Bonzini ,\n\tPeter Crosthwaite ,\n\tStefan Hajnoczi ,\n\tRichard Henderson ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1768630,"web_url":"http://patchwork.ozlabs.org/comment/1768630/","msgid":"<8760clqs2n.fsf@frigg.lan>","list_archive_url":null,"date":"2017-09-14T14:21:20","subject":"Re: [Qemu-devel] [PATCH 1/7] trace: Add event \"guest_bbl_before\"","submitter":{"id":9099,"url":"http://patchwork.ozlabs.org/api/people/9099/","name":"Lluís Vilanova","email":"vilanova@ac.upc.edu"},"content":"Richard Henderson writes:\n\n> On 09/10/2017 09:15 AM, Lluís Vilanova wrote:\n>> Signed-off-by: Lluís Vilanova \n>> ---\n>> accel/tcg/translator.c | 6 ++++++\n>> trace-events | 11 +++++++++++\n>> 2 files changed, 17 insertions(+)\n>> \n>> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c\n>> index afa3af478a..91b3b0da32 100644\n>> --- a/accel/tcg/translator.c\n>> +++ b/accel/tcg/translator.c\n>> @@ -13,6 +13,7 @@\n>> #include \"cpu.h\"\n>> #include \"tcg/tcg.h\"\n>> #include \"tcg/tcg-op.h\"\n>> +#include \"trace-tcg.h\"\n>> #include \"exec/exec-all.h\"\n>> #include \"exec/gen-icount.h\"\n>> #include \"exec/log.h\"\n>> @@ -91,6 +92,11 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,\n>> }\n>> }\n>> \n>> + /* Tracing before */\n>> + if (db->num_insns == 1) {\n>> + trace_guest_bbl_before_tcg(cpu, tcg_ctx.tcg_env, db->pc_first);\n>> + }\n\n> Why not place this before the loop, so that you don't\n> have to check num_insns == 1?\n\nIn case the translation hits a breakpoint. I do not want the BBL/TB to appear\nonce before breaking, and then again after continuing.\n\n\n>> +vcpu tcg guest_bbl_before(uint64_t vaddr) \"vaddr=0x%016\"PRIx64, \"vaddr=0x%016\"PRIx64\n\n> You're really going to print both ENV and PC tagged with \"vaddr\"?\n> That just seems confusing.\n\nI'm not sure I follow. The ENV/CPUState is automatically added to the print\nformat (due to the \"vcpu\" property). The two format strings are necessary due to\nthe \"tcg\" property (translation-time format and execution-time format, since\nTCGv arguments are not passed to the tracing function at translation time).\n\nI'm not sure if this addresses your issue.\n\n\n> Also, terminology. A \"basic block\" (\"bb\" by preference, not \"bbl\"), has a\n> specific meaning (https://en.wikipedia.org/wiki/Basic_block). What we're\n> generating here is a TranslationBlock (which may consist of many basic blocks),\n> and oft contracted within the source as \"tb\".\n\nYes, I got carried away by the naming used by some binary analysis tools like\npin. I will change it to TB and that will probably make it clearer.\n\n\nThanks,\n Lluis","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtLMk1DGcz9s4s\n\tfor ;\n\tFri, 15 Sep 2017 00:22:24 +1000 (AEST)","from localhost ([::1]:48087 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dsV24-0002GH-0p\n\tfor incoming@patchwork.ozlabs.org; Thu, 14 Sep 2017 10:22:20 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:35506)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dsV1L-00025h-Nt\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 10:21:41 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dsV1I-0004h5-GB\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 10:21:35 -0400","from roura.ac.upc.edu ([147.83.33.10]:44608 helo=roura.ac.upc.es)\n\tby eggs.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dsV1I-0004eu-3R\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 10:21:32 -0400","from correu-1.ac.upc.es (correu-1.ac.upc.es [147.83.30.91])\n\tby roura.ac.upc.es (8.13.8/8.13.8) with ESMTP id v8EELRMs018358;\n\tThu, 14 Sep 2017 16:21:27 +0200","from localhost (unknown [132.68.50.201])\n\tby correu-1.ac.upc.es (Postfix) with ESMTPSA id DC7B61C7;\n\tThu, 14 Sep 2017 16:21:21 +0200 (CEST)"],"From":"=?utf-8?q?Llu=C3=ADs_Vilanova?= ","To":"Richard Henderson ","References":"<150505986682.19604.11937392314067517230.stgit@frigg.lan>\n\t<150506010962.19604.7773376130922107594.stgit@frigg.lan>\n\t","Mail-Followup-To":"Richard Henderson ,\n\tqemu-devel@nongnu.org, Paolo Bonzini , Richard\n\tHenderson , Stefan Hajnoczi ,\n\tPeter Crosthwaite ","Date":"Thu, 14 Sep 2017 17:21:20 +0300","In-Reply-To":" (Richard\n\tHenderson's message of \"Wed, 13 Sep 2017 09:59:09 -0700\")","Message-ID":"<8760clqs2n.fsf@frigg.lan>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.6.x [fuzzy]","X-Received-From":"147.83.33.10","Subject":"Re: [Qemu-devel] [PATCH 1/7] trace: Add event \"guest_bbl_before\"","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Paolo Bonzini ,\n\tPeter Crosthwaite ,\n\tqemu-devel@nongnu.org, Stefan Hajnoczi ,\n\tRichard Henderson ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]