From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57322) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adcpy-0001zb-97 for qemu-devel@nongnu.org; Wed, 09 Mar 2016 07:03:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adcpu-0005X6-2X for qemu-devel@nongnu.org; Wed, 09 Mar 2016 07:03:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38976) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adcpt-0005Ws-KC for qemu-devel@nongnu.org; Wed, 09 Mar 2016 07:03:29 -0500 References: <20160301110732.10104.2019.stgit@PASHA-ISP> <20160301110752.10104.65295.stgit@PASHA-ISP> From: Paolo Bonzini Message-ID: <56E0110A.1000205@redhat.com> Date: Wed, 9 Mar 2016 13:03:22 +0100 MIME-Version: 1.0 In-Reply-To: <20160301110752.10104.65295.stgit@PASHA-ISP> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 3/5] replay: introduce new checkpoint for icount warp List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Dovgalyuk , qemu-devel@nongnu.org Cc: edgar.iglesias@xilinx.com, peter.maydell@linaro.org, igor.rubinov@gmail.com, mark.burton@greensocs.com, real@ispras.ru, batuzovk@ispras.ru, maria.klimushenkova@ispras.ru, stefanha@redhat.com, kwolf@redhat.com, hines@cert.org, alex.bennee@linaro.org, fred.konrad@greensocs.com On 01/03/2016 12:07, Pavel Dovgalyuk wrote: > qemu_clock_warp function is called to update virtual clock when CPU > is sleeping. This function includes replay checkpoint to make execution > deterministic in icount mode. > Record/replay module flushes async event queue at checkpoints. > Some of the events (e.g., block devices operations) include interaction > with hardware. E.g., APIC polled by block devices sets one of IRQ flags= . > Flag to be set depends on currently executed thread (CPU or iothread). > Therefore in replay mode we have to process the checkpoints in the same= thread > as they were recorded. > qemu_clock_warp function (and its checkpoint) may be called from differ= ent > thread. This patch introduces new checkpoint which distinguished warp > checkpoint calls from different threads. >=20 > Signed-off-by: Pavel Dovgalyuk I think we need two different kinds of "warp" behavior, one to start the warp timer (from the main loop and when a timer is set) and one to end it (from the CPUs). Then the need for two checkpoints is much clearer, though I suggestnaming them without a reference to TCG; something like CHECKPOINT_CLOCK_WARP_STA= RT and CHECKPOINT_CLOCK_WARP_ACCOUNT for example. The start would be where you call qemu_clock_warp(QEMU_CLOCK_VIRTUAL, fal= se): if (!use_icount) { return; } if (!runstate_is_running()) { return; } if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP_START)) { return; } /* I think calling icount_warp_rt here is unnecessary. */ if (!all_cpu_threads_idle()) { return; } if (qtest_enabled()) { /* When testing, qtest commands advance icount. */ return; } /* We want to use the earliest deadline from ALL vm_clocks */ clock =3D qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT); deadline =3D qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL); ... The end or account function, instead, would be called from tcg_exec_all() and also from icount_dummy_timer() (this is what makes the call to icount_warp_rt unnecessary above): if (!use_icount || !icount_isleep) { return; } if (!runstate_is_running()) { return; } if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP_END)) { return; } timer_del(icount_warp_timer); /* * If the CPUs have been sleeping, advance QEMU_CLOCK_VIRTUAL timer n= ow. * This ensures that the deadline for the timer is computed correctly * below. * This also makes sure that the insn counter is synchronized before * the CPU starts running, in case the CPU is woken by an event other * than the earliest QEMU_CLOCK_VIRTUAL timer. */ // ...include icount_warp_rt function here... qemu_clock_warp would only be called from qemu-timer.c, and it would be simply be if (type =3D=3D QEMU_CLOCK_VIRTUAL) { qemu_start_warp_timer(); } Separating the two boundaries of the warp this way would make the code much easier to understand, and would also make the need for a new checkpoint more obvious. Paolo