On Fri, Jun 28, 2019 at 02:16:41PM +0200, Richard Henderson wrote:
> On 6/28/19 1:39 PM, Luc Michel wrote:
> > Add a TCG trace at the begining of a translation block recording the
> > first and last (past-the-end) PC values.
> >
> > Signed-off-by: Luc Michel <luc.mic...@greensocs.com>
> > ---
> > This can be used to trace the execution of the guest quite efficiently.
> > It will report each time a TB is entered (using the tb_enter_exec
> > trace). The traces arguments give the PC start and past-the-end values.
> > It has very little to no performance impact since the trace is actually
> > emitted in the generated code only when it is enabled at run time.
> >
> > It works already quite well on its own to trace guest execution. However
> > it does not handle the case where a TB is exited in the middle of
> > execution. I'm not sure how to properly trace that. A trace could be
> > added when `cpu_loop_exit()' is called to report the current PC, but in
> > most cases the interesting value (the PC of the instruction that
> > caused the exit) is already lost at this stage.
> >
> > I'm not sure there is a generic (i.e. not target specific) way of
> > recovering the last PC executed when cpu_loop_exit() is called. Do you
> > think of a better way?
> >
> > Thanks to the Xilinx's QEMU team who sponsored this work.
> > ---
> > accel/tcg/translator.c | 24 ++++++++++++++++++++++++
> > trace-events | 3 +++
> > 2 files changed, 27 insertions(+)
> >
> > diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> > index 9226a348a3..c55377aa18 100644
> > --- a/accel/tcg/translator.c
> > +++ b/accel/tcg/translator.c
> > @@ -14,10 +14,11 @@
> > #include "tcg/tcg-op.h"
> > #include "exec/exec-all.h"
> > #include "exec/gen-icount.h"
> > #include "exec/log.h"
> > #include "exec/translator.h"
> > +#include "trace-tcg.h"
> >
> > /* Pairs with tcg_clear_temp_count.
> > To be called by #TranslatorOps.{translate_insn,tb_stop} if
> > (1) the target is sufficiently clean to support reporting,
> > (2) as and when all temporaries are known to be consumed.
> > @@ -28,14 +29,31 @@ void translator_loop_temp_check(DisasContextBase *db)
> > qemu_log("warning: TCG temporary leaks before "
> > TARGET_FMT_lx "\n", db->pc_next);
> > }
> > }
> >
> > +static TCGOp *gen_trace_tb_enter(TranslationBlock *tb)
> > +{
> > + TCGOp *last_pc_op;
> > +
> > + TCGv pc_end = tcg_temp_new();
> > +
> > + /* The last PC value is not known yet */
> > + tcg_gen_movi_tl(pc_end, 0xdeadbeef);
> > + last_pc_op = tcg_last_op();
>
> TL is a target-specific type that does not necessarily correspond to uint64_t,
> as you assume in the print message. More importantly, on a 32-bit host with a
> 64-bit guest, this movi will generate *two* ops...
>
> > + /* Fixup the last PC value in the tb_enter trace now that we know it */
> > + tcg_set_insn_param(trace_pc_end, 1, db->pc_next);
>
> ... which means that this operation does not do what you think it does. It
> will only set one (unknown) half of the _i64 temporary.
>
> Moreover, this isn't quite as zero overhead as I would like, because the
> pc_end
> temporary is always created, even if the trace_tb condition is not satisfied
> and so it (eventually) gets removed as unused.
>
> I'm not quite sure what you're after with pc_end anyway. As you note within
> the cover, you can't reliably use it for anything. If you remove that, then
> you've also removed all of the other problems with this patch.
>
Hi,
One of the use cases is to be able to collect code-coverage from these traces.
In that case we're going to need a reliable pc_end. We may need to recover it
from outside of TCG in some cases though...
Cheers,
Edgar
