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

[Lluís Vilanova]

Richard Henderson writes:

> On 02/04/2014 12:33 PM, Lluís Vilanova wrote:
>> 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.

> Ah, see, I told you I was probably reading the patches wrong.  With all the
> helpers.h changes, I thought this was somehow related to tracing the existing
> helpers.  But the existance of trace_foo_tcg is triggered by the trace-events 
> file?

Yes, defining "tcg foo" in the "trace-events" file will generate the functions
described in this series.


>>> 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).

> How often do events change state?  My guess is exceedingly rarely.
> And by "rare" I mean something well under once per minute.  ;-)

> At which point, option 2 would be the best bet, I think.

Right. For the 3rd option I was also thinking about having per-vCPU tracing
states (in the case of guest events), so that you can trace separate events
depending on the vCPU.

Which reminds me, I should add a vCPU pointer to the "guest_vmem" event, since
otherwise you cannot differentiate accesses among different vCPUs.


>> 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.

> Fair enough.


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