From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:51049)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <richard.henderson@linaro.org>) id 1dsB0O-0003wG-22
	for qemu-devel@nongnu.org; Wed, 13 Sep 2017 12:59:17 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <richard.henderson@linaro.org>) id 1dsB0L-00041f-Hx
	for qemu-devel@nongnu.org; Wed, 13 Sep 2017 12:59:16 -0400
Received: from mail-pg0-x22b.google.com ([2607:f8b0:400e:c05::22b]:47704)
	by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)
	(Exim 4.71) (envelope-from <richard.henderson@linaro.org>)
	id 1dsB0L-000415-7N
	for qemu-devel@nongnu.org; Wed, 13 Sep 2017 12:59:13 -0400
Received: by mail-pg0-x22b.google.com with SMTP id d8so1455150pgt.4
	for <qemu-devel@nongnu.org>; Wed, 13 Sep 2017 09:59:13 -0700 (PDT)
References: <150505986682.19604.11937392314067517230.stgit@frigg.lan>
	<150506010962.19604.7773376130922107594.stgit@frigg.lan>
From: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <c4f6b95c-e548-cd78-e3fa-345d534f273f@linaro.org>
Date: Wed, 13 Sep 2017 09:59:09 -0700
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
Subject: Re: [Qemu-devel] [PATCH 1/7] trace: Add event "guest_bbl_before"
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: =?UTF-8?Q?Llu=c3=ads_Vilanova?= <vilanova@ac.upc.edu>, qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>, Stefan Hajnoczi <stefanha@redhat.com>, Peter Crosthwaite <crosthwaite.peter@gmail.com>

On 09/10/2017 09:15 AM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  accel/tcg/translator.c |    6 ++++++
>  trace-events           |   11 +++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index afa3af478a..91b3b0da32 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -13,6 +13,7 @@
>  #include "cpu.h"
>  #include "tcg/tcg.h"
>  #include "tcg/tcg-op.h"
> +#include "trace-tcg.h"
>  #include "exec/exec-all.h"
>  #include "exec/gen-icount.h"
>  #include "exec/log.h"
> @@ -91,6 +92,11 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
>              }
>          }
>  
> +        /* Tracing before */
> +        if (db->num_insns == 1) {
> +            trace_guest_bbl_before_tcg(cpu, tcg_ctx.tcg_env, db->pc_first);
> +        }

Why not place this before the loop, so that you don't
have to check num_insns == 1?

> +vcpu tcg guest_bbl_before(uint64_t vaddr) "vaddr=0x%016"PRIx64, "vaddr=0x%016"PRIx64

You're really going to print both ENV and PC tagged with "vaddr"?
That just seems confusing.

Also, terminology.  A "basic block" ("bb" by preference, not "bbl"), has a
specific meaning (https://en.wikipedia.org/wiki/Basic_block).  What we're
generating here is a TranslationBlock (which may consist of many basic blocks),
and oft contracted within the source as "tb".


r~