diff mbox

[RFC,v8,09/21] replay: interrupts and exceptions

Message ID 20150122085221.5276.24836.stgit@PASHA-ISP.def.inno
State New
Headers show

Commit Message

Pavel Dovgalyuk Jan. 22, 2015, 8:52 a.m. UTC
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.dovgaluk@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(-)

Comments

Paolo Bonzini Jan. 29, 2015, 9:44 a.m. UTC | #1
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
Pavel Dovgalyuk Feb. 2, 2015, 1:50 p.m. UTC | #2
> From: Paolo Bonzini [mailto:pbonzini@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
Paolo Bonzini Feb. 2, 2015, 2:18 p.m. UTC | #3
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
diff mbox

Patch

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;
+                        }
                     }
                     /* Don't use the cached interrupt_request value,
                        do_interrupt may have updated the EXITTB flag. */
@@ -453,7 +478,8 @@  int cpu_exec(CPUArchState *env)
                         next_tb = 0;
                     }
                 }
-                if (unlikely(cpu->exit_request)) {
+                if (unlikely(cpu->exit_request
+                             || replay_has_interrupt())) {
                     cpu->exit_request = 0;
                     cpu->exception_index = EXCP_INTERRUPT;
                     cpu_loop_exit(cpu);
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 4d661a1..64cc3b2 100755
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -16,7 +16,11 @@ 
 
 enum ReplayEvents {
     /* for instruction event */
-    EVENT_INSTRUCTION
+    EVENT_INSTRUCTION,
+    /* for software interrupt */
+    EVENT_INTERRUPT,
+    /* for emulated exceptions */
+    EVENT_EXCEPTION
 };
 
 typedef struct ReplayState {
diff --git a/replay/replay.c b/replay/replay.c
index d6f5c4b..307ac4b 100755
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -82,3 +82,72 @@  void replay_exec_instructions(void)
         }
     }
 }
+
+bool replay_exception(void)
+{
+    if (replay_mode == REPLAY_MODE_RECORD) {
+        replay_save_instructions();
+        replay_put_event(EVENT_EXCEPTION);
+        return true;
+    } 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;
+    }
+
+    return true;
+}
+
+bool replay_has_exception(void)
+{
+    bool res = false;
+    if (replay_mode == REPLAY_MODE_PLAY) {
+        replay_exec_instructions();
+        replay_mutex_lock();
+        if (skip_async_events(EVENT_EXCEPTION)) {
+            res = true;
+        }
+        replay_mutex_unlock();
+    }
+
+    return res;
+}
+
+bool replay_interrupt(void)
+{
+    if (replay_mode == REPLAY_MODE_RECORD) {
+        replay_save_instructions();
+        replay_put_event(EVENT_INTERRUPT);
+        return true;
+    } else if (replay_mode == REPLAY_MODE_PLAY) {
+        bool res = false;
+        replay_exec_instructions();
+        replay_mutex_lock();
+        if (skip_async_events(EVENT_INTERRUPT)) {
+            replay_has_unread_data = 0;
+            res = true;
+        }
+        replay_mutex_unlock();
+        return res;
+    }
+
+    return true;
+}
+
+bool replay_has_interrupt(void)
+{
+    bool res = false;
+    if (replay_mode == REPLAY_MODE_PLAY) {
+        replay_exec_instructions();
+        replay_mutex_lock();
+        res = skip_async_events(EVENT_INTERRUPT);
+        replay_mutex_unlock();
+    }
+    return res;
+}
diff --git a/replay/replay.h b/replay/replay.h
index e425dea..eb3b0ff 100755
--- a/replay/replay.h
+++ b/replay/replay.h
@@ -27,4 +27,21 @@  int replay_get_instructions(void);
 /*! Updates instructions counter in replay mode. */
 void replay_exec_instructions(void);
 
+/* Interrupts and exceptions */
+
+/*! Called by exception handler to write or read
+    exception processing events. */
+bool replay_exception(void);
+/*! Used to determine that exception is pending.
+    Does not proceed to the next event in the log. */
+bool replay_has_exception(void);
+/*! Called by interrupt handlers to write or read
+    interrupt processing events.
+    \return true if interrupt should be processed */
+bool replay_interrupt(void);
+/*! Tries to read interrupt event from the file.
+    Returns true, when interrupt request is pending */
+bool replay_has_interrupt(void);
+
+
 #endif
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index fa374d0..9c00e6f 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -22,6 +22,7 @@ 
 #include "qemu/log.h"
 #include "exec/helper-proto.h"
 #include "exec/cpu_ldst.h"
+#include "replay/replay.h"
 
 //#define DEBUG_PCALL
 
@@ -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
     if (interrupt_request & CPU_INTERRUPT_SIPI) {