From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34493) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHFXq-0006OQ-Ba for qemu-devel@nongnu.org; Thu, 07 Jan 2016 13:44:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aHFXl-0007AN-5O for qemu-devel@nongnu.org; Thu, 07 Jan 2016 13:44:22 -0500 Received: from roura.ac.upc.es ([147.83.33.10]:35800) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHFXk-00079C-PW for qemu-devel@nongnu.org; Thu, 07 Jan 2016 13:44:17 -0500 From: =?utf-8?Q?Llu=C3=ADs_Vilanova?= References: <144838492534.3052.2948919558518613064.stgit@localhost> <144838497636.3052.15443880874914788795.stgit@localhost> <20160107080300.GG12971@stefanha-x1.localdomain> Date: Thu, 07 Jan 2016 19:44:13 +0100 In-Reply-To: <20160107080300.GG12971@stefanha-x1.localdomain> (Stefan Hajnoczi's message of "Thu, 7 Jan 2016 16:03:00 +0800") Message-ID: <87a8ohjkle.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 v2 09/10] trace: [tcg] Add per-vCPU tracing states for events with the 'vcpu' property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Eduardo Habkost , qemu-devel@nongnu.org, Markus Armbruster , Luiz Capitulino , Stefan Hajnoczi , Andreas =?utf-8?Q?F=C3=A4rber?= Stefan Hajnoczi writes: > On Tue, Nov 24, 2015 at 06:09:36PM +0100, Llu=C3=ADs Vilanova wrote: >> + /* Ensure 'tb_phys_idx' can encode event states as a bitmask */ >> + bool too_many_tcg_vcpu_events[ >> + TRACE_CPU_EVENT_COUNT > sizeof(unsigned int)*8 ? -1 : 0]; > There is a limit of 32 vcpu tcg events? That seems low but as long as > not too many users of this feature are merged it will work... Well, 'tb_phys_idx' and 'tb_phys_idx_req' (just above this line) are 'unsig= ned int'. They could be changed to a 64-bit index if necessary, but more than t= hat would require a more complex physical TB cache selection. Fortunately, this is going to be catched at compilation time, making it safe against crashing QEMU because of too many events of this type. >> @@ -227,6 +228,17 @@ void cpu_dump_statistics(CPUState *cpu, FILE *f, fp= rintf_function cpu_fprintf, >> void cpu_reset(CPUState *cpu) >> { >> CPUClass *klass =3D CPU_GET_CLASS(cpu); >> + TraceEvent *ev =3D NULL; >> + >> + if (!qemu_initialized) { > Is there a cleaner place to do this without introducing the > qemu_initialized global? > I guess the problem is that tracing itself is initialized before the > vcpus are set up. Is qemu_add_machine_init_done_notifier() sufficient > for this purpose? Right, tracing must be initialized early, while vCPUs do so much later. Als= o, the hook I took for initialization is also called by regular vCPU resets and hotplugs. The problem with machine_init is that it only works in full-system (softmmu) mode, so it would require a separate initialization call for the = user mode variants (e.g., linux-user). It would be much cleaner to add a trace post-initialization routine right b= efore main_loop/cpu_loop (doing the per-vCPU tracing state initialization). I'll re-check the code to see if there was any other condition that made me take 'cpu_reset' instead. Thanks, Lluis