[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/7] trace: Add event "guest_bbl_before"
From: |
Lluís Vilanova |
Subject: |
Re: [Qemu-devel] [PATCH 1/7] trace: Add event "guest_bbl_before" |
Date: |
Thu, 14 Sep 2017 17:21:20 +0300 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
Richard Henderson writes:
> On 09/10/2017 09:15 AM, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova <address@hidden>
>> ---
>> 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?
In case the translation hits a breakpoint. I do not want the BBL/TB to appear
once before breaking, and then again after continuing.
>> +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.
I'm not sure I follow. The ENV/CPUState is automatically added to the print
format (due to the "vcpu" property). The two format strings are necessary due to
the "tcg" property (translation-time format and execution-time format, since
TCGv arguments are not passed to the tracing function at translation time).
I'm not sure if this addresses your issue.
> 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".
Yes, I got carried away by the naming used by some binary analysis tools like
pin. I will change it to TB and that will probably make it clearer.
Thanks,
Lluis