From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59697) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X9aXj-0003n8-II for qemu-devel@nongnu.org; Tue, 22 Jul 2014 09:55:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X9aXe-0004Kz-FU for qemu-devel@nongnu.org; Tue, 22 Jul 2014 09:55:47 -0400 Received: from zimbra3.corp.accelance.fr ([2001:4080:204::2:8]:53597) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X9aXe-0004Kd-1k for qemu-devel@nongnu.org; Tue, 22 Jul 2014 09:55:42 -0400 Date: Tue, 22 Jul 2014 15:55:37 +0200 (CEST) From: Sebastian Tanase Message-ID: <1857580641.20043988.1406037337014.JavaMail.root@openwide.fr> In-Reply-To: <53CE3ABE.3000100@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC PATCH V4 6/6] monitor: Add drift info to 'info jit' List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, peter maydell , aliguori@amazon.com, wenchaoqemu@gmail.com, quintela@redhat.com, qemu-devel@nongnu.org, mst@redhat.com, stefanha@redhat.com, armbru@redhat.com, lcapitulino@redhat.com, michael@walle.cc, camille begue , alex@alex.org.uk, crobinso@redhat.com, afaerber@suse.de, rth@twiddle.net ----- Mail original ----- > De: "Paolo Bonzini" > =C3=80: "Sebastian Tanase" > Cc: aliguori@amazon.com, afaerber@suse.de, rth@twiddle.net, "peter maydel= l" , > michael@walle.cc, alex@alex.org.uk, stefanha@redhat.com, lcapitulino@redh= at.com, crobinso@redhat.com, > armbru@redhat.com, wenchaoqemu@gmail.com, quintela@redhat.com, kwolf@redh= at.com, mst@redhat.com, "camille begue" > , qemu-devel@nongnu.org > Envoy=C3=A9: Mardi 22 Juillet 2014 12:19:42 > Objet: Re: [RFC PATCH V4 6/6] monitor: Add drift info to 'info jit' >=20 > Il 22/07/2014 11:58, Sebastian Tanase ha scritto: > >=20 > > -timers_state.cpu_clock_offset contains the offset between the real > > and virtual clocks. > > However, when using the value of the virtual clock > > (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)), > > qemu_icount_bias already includes this offset because, on ARM, > > qemu_clock_warp (which > > then calls icount_warp_rt) is called for the first time in > > tcg_exec_all, making > > qemu_icount_bias take the value of > > qemu_clock_get_ns(QEMU_CLOCK_REALTIME) >=20 > Does this means that QEMU_CLOCK_VIRTUAL counts up from > qemu_clock_get_ns(QEMU_CLOCK_REALTIME) rather than 0? This would be > a > bug, and it would be good to fix it (by initializing > vm_clock_warp_start > to -1). Yes, QEMU_CLOCK_VIRTUAL counts up from qemu_clock_get_ns(QEMU_CLOCK_REALTIM= E) on ARM (I have only tested with the versatilepb and vexpress boards). >=20 > > A solution to not compute the initial offset in qemu_icount_bias > > would be to initialize > > vm_clock_warp_start to -1. The result will be that > > qemu_icount_bias will start counting when > > the vcpu goes from active to inactive. At that time, > > vm_clock_warp_start will already store the realtime clock > > value and a timer on the real clock will be set to expire at clock > > + deadline, making qemu_icount_bias increment > > by deadline. >=20 > That would be correct. >=20 > > A consequence of initializing vm_clock_warp_start to -1 is also > > the fact that we'll skip the first check for a virtual expired > > timer. As I mentioned above, in ARM case, > > it's not dangerous because there are no timers active the first > > time we perform this check. However, this > > is just a potential scenario and I cannot guarantee that on other > > target architectures there won't be > > an expired timer pending the first time we check. >=20 > Like a timer that expired in the past? That would be caught in > tcg_cpu_exec, when it initializes the decrementer to > qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL). >=20 > > So, do you think it is worth taking this solution into account or > > it will cause more harm than good? >=20 > Yes, even more so. If we add workarounds to complicated code, it > will > become even more complicated. >=20 > Paolo >=20 I'll make a small patch that initializes vm_clock_warp_start to -1. Sebastian