From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59454) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WAmgS-0004IE-Ta for qemu-devel@nongnu.org; Tue, 04 Feb 2014 15:33:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WAmgJ-0005vL-HR for qemu-devel@nongnu.org; Tue, 04 Feb 2014 15:33:28 -0500 Received: from roura.ac.upc.es ([147.83.33.10]:51717) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WAmgI-0005uG-OG for qemu-devel@nongnu.org; Tue, 04 Feb 2014 15:33:19 -0500 From: =?utf-8?Q?Llu=C3=ADs_Vilanova?= References: <20140131160902.32741.2680.stgit@fimbulvetr.bsc.es> <52F0FFBD.8050801@twiddle.net> Date: Tue, 04 Feb 2014 21:33:16 +0100 In-Reply-To: <52F0FFBD.8050801@twiddle.net> (Richard Henderson's message of "Tue, 04 Feb 2014 06:57:01 -0800") Message-ID: <87k3davcs3.fsf@fimbulvetr.bsc.es> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 00/12] trace: [tcg] Allow tracing guest events in TCG-generated code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: qemu-devel@nongnu.org, Stefan Hajnoczi Richard Henderson writes: > On 01/31/2014 08:09 AM, Llu=C3=ADs 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 prop= ery). >>=20 >> Such events generate an extra set of tracing functions that can be calle= d during >> TCG code generation and will automatically redirect a call to the approp= riate >> backend-dependent tracing functions when the guest code is executed. >>=20 >> Files generating guest code (TCG) must include "trace-tcg.h". Files decl= aring >> per-target helpers ("${target}/helper.h") must include >> "trace/generated-helpers.h". >>=20 >> The flow of the generated routines is: >>=20 >>=20 >> [At translation time] >>=20 >> * trace_${name}_tcg(bool, TCGv) >> Declared: "trace/generated-tcg-tracers.h" >> Defined : "trace/generated-tcg-tracers.h" >>=20 >> * gen_helper_trace_${name}_tcg(bool, TCGv) >> Declared: "trace/generated-helpers.h" >> Defined : "trace/generated-helpers.h" >>=20 >> 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. >>=20 >> * gen_helper_trace_${name}_tcg_proxy(TCGi32, TCGv) >> Declared: "trace/generated-helpers.h" >> Defined : "trace/generated-helpers.h" (using helper machinery) >>=20 >> The actual TCG helper function, created using QEMU's TCG helper machiner= y. > 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 tra= ce > 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 traci= ng capabilities, but generates a "trace_foo_tcg" routine that can be called du= ring 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 t= he > 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 t= he second, but I just wanted to keep this one as simple as possible. Also, implementing any of these two last approaches would provide minimal overhea= ds on builds that have such events enabled at compile time. Thanks, Lluis --=20 "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