From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52971) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGmAW-0004Hh-Vd for qemu-devel@nongnu.org; Thu, 29 Jan 2015 05:17:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGmAS-0000h8-9A for qemu-devel@nongnu.org; Thu, 29 Jan 2015 05:17:48 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58675) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGmAS-0000h1-1c for qemu-devel@nongnu.org; Thu, 29 Jan 2015 05:17:44 -0500 Message-ID: <54C9FE31.4090404@redhat.com> Date: Thu, 29 Jan 2015 10:32:33 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <20150122085127.5276.53895.stgit@PASHA-ISP.def.inno> <20150122085215.5276.8878.stgit@PASHA-ISP.def.inno> In-Reply-To: <20150122085215.5276.8878.stgit@PASHA-ISP.def.inno> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH v8 08/21] cpu: replay instructions sequence List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Dovgalyuk , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, alex.bennee@linaro.org, mark.burton@greensocs.com, real@ispras.ru, batuzovk@ispras.ru, maria.klimushenkova@ispras.ru, afaerber@suse.de, fred.konrad@greensocs.com On 22/01/2015 09:52, Pavel Dovgalyuk wrote: > This patch adds calls to replay functions into the icount setup block. > In record mode number of executed instructions is written to the log. > In replay mode number of istructions to execute is taken from the replay log. > > Signed-off-by: Pavel Dovgalyuk > --- > cpu-exec.c | 1 + > cpus.c | 28 ++++++++++++++++++---------- > replay/replay.c | 24 ++++++++++++++++++++++++ > replay/replay.h | 4 ++++ > 4 files changed, 47 insertions(+), 10 deletions(-) > > diff --git a/cpu-exec.c b/cpu-exec.c > index 49f01f5..99a0993 100644 > --- a/cpu-exec.c > +++ b/cpu-exec.c > @@ -531,6 +531,7 @@ int cpu_exec(CPUArchState *env) > } > cpu->exception_index = EXCP_INTERRUPT; > next_tb = 0; > + qemu_notify_event(); Why is this needed? > cpu_loop_exit(cpu); > } > break; > diff --git a/cpus.c b/cpus.c > index c513275..8787277 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -41,6 +41,7 @@ > #include "qemu/seqlock.h" > #include "qapi-event.h" > #include "hw/nmi.h" > +#include "replay/replay.h" > > #ifndef _WIN32 > #include "qemu/compatfd.h" > @@ -1342,18 +1343,22 @@ static int tcg_cpu_exec(CPUArchState *env) > + cpu->icount_extra); > cpu->icount_decr.u16.low = 0; > cpu->icount_extra = 0; > - deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL); > + if (replay_mode != REPLAY_MODE_PLAY) { > + deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL); > > - /* Maintain prior (possibly buggy) behaviour where if no deadline > - * was set (as there is no QEMU_CLOCK_VIRTUAL timer) or it is more than > - * INT32_MAX nanoseconds ahead, we still use INT32_MAX > - * nanoseconds. > - */ > - if ((deadline < 0) || (deadline > INT32_MAX)) { > - deadline = INT32_MAX; > - } > + /* Maintain prior (possibly buggy) behaviour where if no deadline > + * was set (as there is no QEMU_CLOCK_VIRTUAL timer) or it is more than > + * INT32_MAX nanoseconds ahead, we still use INT32_MAX > + * nanoseconds. > + */ > + if ((deadline < 0) || (deadline > INT32_MAX)) { > + deadline = INT32_MAX; > + } > > - count = qemu_icount_round(deadline); > + count = qemu_icount_round(deadline); > + } else { > + count = replay_get_instructions(); > + } Please extract the "if" to a separate function tcg_get_icount_limit(). > timers_state.qemu_icount += count; > decr = (count > 0xffff) ? 0xffff : count; > count -= decr; > @@ -1371,6 +1376,9 @@ static int tcg_cpu_exec(CPUArchState *env) > + cpu->icount_extra); > cpu->icount_decr.u32 = 0; > cpu->icount_extra = 0; > + if (replay_mode == REPLAY_MODE_PLAY) { > + replay_exec_instructions(); replay_account_executed_instructions() > + } > } > return ret; > } > diff --git a/replay/replay.c b/replay/replay.c > index a43bbbc..d6f5c4b 100755 > --- a/replay/replay.c > +++ b/replay/replay.c > @@ -58,3 +58,27 @@ uint64_t replay_get_current_step(void) > { > return cpu_get_icount_raw(); > } > + > +int replay_get_instructions(void) > +{ > + int res = 0; > + replay_mutex_lock(); > + if (skip_async_events(EVENT_INSTRUCTION)) { > + res = replay_state.instructions_count; > + } > + replay_mutex_unlock(); > + return res; > +} > + > +void replay_exec_instructions(void) > +{ > + if (replay_state.instructions_count > 0) { > + int count = (int)(replay_get_current_step() > + - replay_state.current_step); > + replay_state.instructions_count -= count; > + replay_state.current_step += count; > + if (replay_state.instructions_count == 0 && count != 0) { If replay_state.instructions_count is now zero, count must be nonzero (because replay_state.instructions_count was > 0) before. > + replay_has_unread_data = 0; > + } Can replay_state.instructions_count be < count at all? If not, and if replay_state.instructions_count is zero, then count must also be zero. If so, I suggest rewriting as int count = (int)(replay_get_current_step() - replay_state.current_step); assert(replay_state.instructions_count >= count); replay_state.instructions_count -= count; replay_state.current_step += count; if (replay_state.instructions_count == 0) { replay_has_unread_data = 0; } Paolo > + } > +} > diff --git a/replay/replay.h b/replay/replay.h > index a03c748..e425dea 100755 > --- a/replay/replay.h > +++ b/replay/replay.h > @@ -22,5 +22,9 @@ extern ReplayMode replay_mode; > > /*! Returns number of executed instructions. */ > uint64_t replay_get_current_step(void); > +/*! Returns number of instructions to execute in replay mode. */ > +int replay_get_instructions(void); > +/*! Updates instructions counter in replay mode. */ > +void replay_exec_instructions(void); > > #endif >