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

[Richard Henderson <rth@twiddle.net>]

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.

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.

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.


r~