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?
>> 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.
> 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.
r~
