Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code

[Lluís Vilanova]

Richard Henderson writes:

> On 01/31/2014 08:09 AM, Lluís Vilanova wrote:
>> Adds the base ability to specify which events in the "trace-events" file may 
>> be
>> used to trace guest activity in the TCG code (using the "tcg" event propery).
>> 
>> Such events generate an extra set of tracing functions that can be called 
>> during
>> TCG code generation and will automatically redirect a call to the appropriate
>> backend-dependent tracing functions when the guest code is executed.
>> 
>> Files generating guest code (TCG) must include "trace-tcg.h". Files declaring
>> per-target helpers ("${target}/helper.h") must include
>> "trace/generated-helpers.h".
>> 
>> The flow of the generated routines is:
>> 
>> 
>> [At translation time]
>> 
>> * trace_${name}_tcg(bool, TCGv)
>> Declared: "trace/generated-tcg-tracers.h"
>> Defined : "trace/generated-tcg-tracers.h"
>> 
>> * gen_helper_trace_${name}_tcg(bool, TCGv)
>> Declared: "trace/generated-helpers.h"
>> Defined : "trace/generated-helpers.h"
>> 
>> Automatically transforms all the arguments and allocates them into the
>> appropriate TCG temporary values (which are also freed). Provides a more
>> streamlined interface by allowing events in "trace-events" to take a mix of
>> tracing-supported types and TCG types.
>> 
>> * gen_helper_trace_${name}_tcg_proxy(TCGi32, TCGv)
>> Declared: "trace/generated-helpers.h"
>> Defined : "trace/generated-helpers.h" (using helper machinery)
>> 
>> The actual TCG helper function, created using QEMU's TCG helper machinery.

> I suppose I have no major objection to the feature, although frankly it's
> not especially exciting.  I can't really imagine ever wanting to bulk trace
> all of the helpers.  Tracing specific helpers on a target-by-target basis,
> sure.  But that can be done just as easily as adding tracing code to any
> other bit of C.

I'm not sure I understand this comment. The patch does not add helper tracing
capabilities, but generates a "trace_foo_tcg" routine that can be called during
TCG code generation, and will call "trace_foo" when that TCG code is
executed.


> If I read these patches right -- and since they're mostly python I'm not
> sure that I am -- we go through 5 layers of wrappers to get to the current
> trace_foo expansion.  Where trace_foo contains the check to see whether the
> tracepoint is actually enabled.

As a side-note, all layers should be optimized by the compiler (except for the
TCG code generation/execution separation), since they're all "static inline".


> I would strongly suggest this is backward.  One should perform the check for
> the tracepoint being enabled at translation time before emitting the call to
> the helper in the first place.

Right, the thing is that dynamically enabling/disabling such events will not
immediately show up in the trace if I do the check at translation time
(trace_foo_tcg), since QEMU will execute the cached TCG translations. I see the
following solutions to ensure traces are accurate:

* Delay the check until execution time.

* Check at translation time; flush translation cache when the tracing state of
  a TCG event changes.

* Check at translation time; use multiple translation caches, one for each
  possible tracing state of all the TCG-enabled events.

This series implements the first approach, since it's correct and much
simpler.

Other patches I did not send implement the third approach, which is quite
efficient if one is dynamically switching the tracing state while executing
mostly-cached code (e.g., profiling the accesses).

I can wait for a later series to send the third option, or even implement the
second, but I just wanted to keep this one as simple as possible. Also,
implementing any of these two last approaches would provide minimal overheads on
builds that have such events enabled at compile time.


Thanks,
  Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth