Re: [Qemu-devel] [RFC PATCH v8 09/21] replay: interrupts and exceptions

[Pavel Dovgaluk]

 From: Paolo Bonzini [mailto:pbonz...@redhat.com]
 On 22/01/2015 09:52, Pavel Dovgalyuk wrote:
  +if (replay_mode == REPLAY_MODE_RECORD) {
  +replay_save_instructions();
  +replay_put_event(EVENT_EXCEPTION);
  +return true;
 
 Missing mutex lock/unlock.

I think not. We just have to write number of already executed instructions.
This number is not linked to exception event. They could be read in replay while
processing some other event.
 
  @@ -1294,6 +1295,9 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int 
  interrupt_request)
   if (interrupt_request  CPU_INTERRUPT_POLL) {
   cs-interrupt_request = ~CPU_INTERRUPT_POLL;
   apic_poll_irq(cpu-apic_state);
  +if (replay_mode != REPLAY_MODE_NONE) {
  +return true;
  +}
   }
   #endif
 
 Can you explain this?  It probably needs a comment.

Each function call should process one interrupt at once, because it corresponds 
to single event in the log.

Pavel Dovgalyuk

Re: [Qemu-devel] [RFC PATCH v8 09/21] replay: interrupts and exceptions

[Paolo Bonzini]

On 02/02/2015 14:50, Pavel Dovgaluk wrote:
 I think not. We just have to write number of already executed instructions.
 This number is not linked to exception event. They could be read in replay 
 while
 processing some other event.
  

I was referring to replay_put_event(EVENT_EXCEPTION) only.

In principle you could run EVENT_EXCEPTION concurrently with other event
writes, for example:

+replay_mutex_lock();
+replay_put_event(EVENT_CLOCK + kind);
+replay_put_qword(clock);
+replay_mutex_unlock();

and get

EVENT_CLOCK + kind (1 byte)
EVENT_EXCEPTION (1 byte)
clock (8 bytes)

in the replay stream.

I know it's really unlikely, perhaps even impossible in the current QEMU
architecture that is dominated by the big QEMU lock.  But the right
thing to do is to lock the mutex around all event writes.  Even if the
write itself is atomic, it could happen in the middle of another write
if you do not lock the mutex.

Paolo

Re: [Qemu-devel] [RFC PATCH v8 09/21] replay: interrupts and exceptions

[Paolo Bonzini]

On 22/01/2015 09:52, Pavel Dovgalyuk wrote:
 +if (replay_mode == REPLAY_MODE_RECORD) {
 +replay_save_instructions();
 +replay_put_event(EVENT_EXCEPTION);
 +return true;

Missing mutex lock/unlock.

 +} else if (replay_mode == REPLAY_MODE_PLAY) {
 +bool res = false;
 +replay_exec_instructions();
 +replay_mutex_lock();
 +if (skip_async_events(EVENT_EXCEPTION)) {
 +replay_has_unread_data = 0;
 +res = true;
 +}
 +replay_mutex_unlock();
 +return res;
 +}

bool res;
replay_exec_instructions();
res = replay_has_exception();
if (res) {
replay_has_unread_data = 0;
}
return res;

Same for replay_interrupt().

Perhaps worth factoring out two functions replay_cpu_event and
replay_has_cpu_event?  You choose.

 
 @@ -1294,6 +1295,9 @@ bool x86_cpu_exec_interrupt(CPUState *cs, int 
 interrupt_request)
  if (interrupt_request  CPU_INTERRUPT_POLL) {
  cs-interrupt_request = ~CPU_INTERRUPT_POLL;
  apic_poll_irq(cpu-apic_state);
 +if (replay_mode != REPLAY_MODE_NONE) {
 +return true;
 +}
  }
  #endif

Can you explain this?  It probably needs a comment.

Paolo

[Qemu-devel] [RFC PATCH v8 09/21] replay: interrupts and exceptions

[Pavel Dovgalyuk]

This patch includes modifications of common cpu files. All interrupts and
exceptions occured during recording are written into the replay log.
These events allow correct replaying the execution by kicking cpu thread
when one of these events is found in the log.

Signed-off-by: Pavel Dovgalyuk pavel.dovga...@ispras.ru
---
 cpu-exec.c   |   48 +---
 replay/replay-internal.h |6 +++-
 replay/replay.c  |   69 ++
 replay/replay.h  |   17 +++
 target-i386/seg_helper.c |4 +++
 5 files changed, 132 insertions(+), 12 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 99a0993..16d1faa 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -24,6 +24,7 @@
 #include qemu/atomic.h
 #include sysemu/qtest.h
 #include qemu/timer.h
+#include replay/replay.h
 
 /* -icount align implementation. */
 
@@ -338,22 +339,25 @@ int cpu_exec(CPUArchState *env)
 /* This must be volatile so it is not trashed by longjmp() */
 volatile bool have_tb_lock = false;
 
+/* replay_interrupt may need current_cpu */
+current_cpu = cpu;
+
 if (cpu-halted) {
 #ifdef TARGET_I386
-if (cpu-interrupt_request  CPU_INTERRUPT_POLL) {
+if ((cpu-interrupt_request  CPU_INTERRUPT_POLL)
+ replay_interrupt()) {
 apic_poll_irq(x86_cpu-apic_state);
 cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
 }
 #endif
 if (!cpu_has_work(cpu)) {
+current_cpu = NULL;
 return EXCP_HALTED;
 }
 
 cpu-halted = 0;
 }
 
-current_cpu = cpu;
-
 /* As long as current_cpu is null, up to the assignment just above,
  * requests by other threads to exit the execution loop are expected to
  * be issued using the exit_request global. We must make sure that our
@@ -400,10 +404,21 @@ int cpu_exec(CPUArchState *env)
 cpu-exception_index = -1;
 break;
 #else
-cc-do_interrupt(cpu);
-cpu-exception_index = -1;
+if (replay_exception()) {
+cc-do_interrupt(cpu);
+cpu-exception_index = -1;
+} else if (!replay_has_interrupt()) {
+/* give a chance to iothread in replay mode */
+ret = EXCP_INTERRUPT;
+break;
+}
 #endif
 }
+} else if (replay_has_exception()
+cpu-icount_decr.u16.low + cpu-icount_extra == 0) {
+/* try to cause an exception pending in the log */
+cpu_exec_nocache(env, 1, tb_find_fast(env), true);
+break;
 }
 
 next_tb = 0; /* force lookup of first TB */
@@ -419,30 +434,40 @@ int cpu_exec(CPUArchState *env)
 cpu-exception_index = EXCP_DEBUG;
 cpu_loop_exit(cpu);
 }
-if (interrupt_request  CPU_INTERRUPT_HALT) {
+if (replay_mode == REPLAY_MODE_PLAY
+ !replay_has_interrupt()) {
+/* Do nothing */
+} else 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) {
+else 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) {
+else if (interrupt_request  CPU_INTERRUPT_RESET) {
+replay_interrupt();
 cpu_reset(cpu);
+cpu_loop_exit(cpu);
 }
 #endif
 /* The target hook has 3 exit conditions:
False when the interrupt isn't processed,
True when it is, and we should restart on a new TB,
and via longjmp via cpu_loop_exit.  */
-if (cc-cpu_exec_interrupt(cpu, interrupt_request)) {
-next_tb = 0;
+else {
+replay_interrupt();
+if (cc-cpu_exec_interrupt(cpu, interrupt_request)) {
+next_tb = 0;
+}
 }