[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 6/7] trace: Add event "guest_inst_after"
From: |
Lluís Vilanova |
Subject: |
Re: [Qemu-devel] [PATCH 6/7] trace: Add event "guest_inst_after" |
Date: |
Thu, 14 Sep 2017 19:23:34 +0300 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
Richard Henderson writes:
> On 09/10/2017 09:35 AM, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova <address@hidden>
>> ---
>> accel/tcg/translator.c | 23 ++++++++++++++++++-----
>> trace-events | 8 ++++++++
>> 2 files changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
>> index d66d601c89..c010aeee45 100644
>> --- a/accel/tcg/translator.c
>> +++ b/accel/tcg/translator.c
>> @@ -35,7 +35,8 @@ void translator_loop_temp_check(DisasContextBase *db)
>> void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
>> CPUState *cpu, TranslationBlock *tb)
>> {
>> - target_ulong pc_bbl;
>> + target_ulong pc_bbl, pc_insn = 0;
>> + bool translated_insn = false;
>> int max_insns;
>>
>> /* Initialize DisasContext */
>> @@ -75,10 +76,15 @@ void translator_loop(const TranslatorOps *ops,
>> DisasContextBase *db,
>> tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */
>>
>> while (true) {
>> - target_ulong pc_insn = db->pc_next;
>> TCGv_i32 insn_size_tcg = 0;
>> int insn_size_opcode_idx;
>>
>> + /* Tracing after (previous instruction) */
>> + if (db->num_insns > 0) {
>> + trace_guest_inst_after_tcg(cpu, tcg_ctx.tcg_env, pc_insn);
>> + }
> How does this differ from "guest_inst"? Why would you need two trace points?
I assume you mean how it differs from guest_inst_before. The two main ideas are:
* To be able to get a trace an execution-time event only after the instruction
or TB have finished executing successfully (i.e., there could be an
exception).
* Some values are only known *after* the instruction is translated (like the
instruction size, or other extra information we might add in the future), so
an efficient way to collect that is to trace guest_bbl_* and guest_insn_after
at translation time (to build a TB "dictionary" as some call it), and trace
guest_bbl_before at execution time (and use the detailed info above that you
got at translation time).
> Why are you placing this at the beginning of the while loop rather than the
> end?
Yeah, that'll be much clearer.
>> @@ -164,6 +172,9 @@ void translator_loop(const TranslatorOps *ops,
>> DisasContextBase *db,
>>
>> gen_set_inline_region_begin(tcg_ctx.disas.inline_label);
>>
>> + if (TRACE_GUEST_INST_AFTER_ENABLED && translated_insn) {
>> + trace_guest_inst_after_tcg(cpu, tcg_ctx.tcg_env, pc_insn);
>> + }
>> if (TRACE_GUEST_BBL_AFTER_ENABLED) {
>> trace_guest_bbl_after_tcg(cpu, tcg_ctx.tcg_env, pc_bbl);
>> }
> I think I'm finally beginning to understand what you're after with your
> inlining. But I still think this should be doable in the appropriate opcode
> generating functions.
I'm not sure we can if we want to avoid having the duplicate translation-time
events I said in a previous response (since TB can have two exit points and
we're detecting them through goto_tb).
Thanks,
Lluis
[Qemu-devel] [PATCH 6/7] trace: Add event "guest_inst_after", Lluís Vilanova, 2017/09/10
[Qemu-devel] [PATCH 7/7] trace: Add event "guest_inst_info_after", Lluís Vilanova, 2017/09/10
Re: [Qemu-devel] [PATCH 0/7] trace: Add guest code events, no-reply, 2017/09/10