From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34154) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YAeW9-0006sn-SQ for qemu-devel@nongnu.org; Mon, 12 Jan 2015 07:54:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YAeW6-0001Zs-Ij for qemu-devel@nongnu.org; Mon, 12 Jan 2015 07:54:49 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41295) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YAeW6-0001Zi-CK for qemu-devel@nongnu.org; Mon, 12 Jan 2015 07:54:46 -0500 Message-ID: <54B3C404.4050603@redhat.com> Date: Mon, 12 Jan 2015 13:54:28 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <20150112115944.3504.66763.stgit@PASHA-ISP> <20150112120032.3504.11086.stgit@PASHA-ISP> <54B3BF3B.7020403@redhat.com> <001301d02e64$f192c740$d4b855c0$@Dovgaluk@ispras.ru> In-Reply-To: <001301d02e64$f192c740$d4b855c0$@Dovgaluk@ispras.ru> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH v7 08/21] replay: interrupts and exceptions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Dovgaluk , 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 12/01/2015 13:40, Pavel Dovgaluk wrote: >>> >>> + if (replay_exception()) { >>> + cc->do_interrupt(cpu); >>> + cpu->exception_index = -1; >> >> I cannot see replay_exception() in the series? > > It is in the patch number 8. Oops, it is in this patch in fact. >>> > > @@ -419,21 +434,24 @@ int cpu_exec(CPUArchState *env) >>> > > cpu->exception_index = EXCP_DEBUG; >>> > > cpu_loop_exit(cpu); >> > >> > Why not for EXCP_DEBUG? > As far as I understand, EXCP_DEBUG is used for external debugger (like gdb). > Debugger is usually connected to VM during replay. > That is why we do not need to replay EXCP_DEBUG - it may appear in different > places, and it does not affect on behavior of the virtual machine. Ok. >>> > > - if (interrupt_request & CPU_INTERRUPT_HALT) { >>> > > + if ((interrupt_request & CPU_INTERRUPT_HALT) >>> > > + && replay_interrupt()) { >>> > > cpu->interrupt_request &= ~CPU_INTERRUPT_HALT; >>> > > cpu->halted = 1; >>> > > cpu->exception_index = EXCP_HLT; >>> > > cpu_loop_exit(cpu); >>> > > } >>> > > #if defined(TARGET_I386) >>> > > - if (interrupt_request & CPU_INTERRUPT_INIT) { >>> > > + if ((interrupt_request & CPU_INTERRUPT_INIT) >>> > > + && replay_interrupt()) { >>> > > cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0); >>> > > do_cpu_init(x86_cpu); >>> > > cpu->exception_index = EXCP_HALTED; >>> > > cpu_loop_exit(cpu); >>> > > } >>> > > #else >>> > > - if (interrupt_request & CPU_INTERRUPT_RESET) { >>> > > + if ((interrupt_request & CPU_INTERRUPT_RESET) >>> > > + && replay_interrupt()) { >>> > > cpu_reset(cpu); >>> > > } >>> > > #endif >> > >> > Perhaps check the replay_interrupt() outside, in an && with "if >> > (unlikely(interrupt_request))"? > You mean that I should wrap whole condition into "unlikely"? > No, I wanted to have a single check of "replay_interrupt()" and/or "replay_has_interrupt()". BTW, I think this is incorrect: > + if ((replay_mode != REPLAY_MODE_PLAY > + || replay_has_interrupt()) > + && cc->cpu_exec_interrupt(cpu, interrupt_request)) { > + replay_interrupt(); because cc->cpu_exec_interrupt() can exit with cpu_loop_exit(cpu). I guess it could be something like this: if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) { /* do nothing */ } else if (interrupt_request & CPU_INTERRUPT_HALT) { replay_interrupt(); ... cpu_loop_exit(cpu); } else if (interrupt_request & CPU_INTERRUPT_INIT) { replay_interrupt(); ... cpu_loop_exit(cpu); } else { replay_interrupt(); if (cc->cpu_exec_interrupt(cpu, interrupt_request)) { next_tb = 0; } } Paolo