cpu: Make sure that a drained atomic CPU isn't executing ucode

Currently, the atomic CPU can be in the middle of a microcode sequence
when it is drained. This leads to two problems:

 * When switching to a hardware virtualized CPU, we obviously can't
   execute gem5 microcode.

 * Since curMacroStaticInst is populated when executing microcode,
   repeated switching between CPUs executing microcode leads to
   incorrect execution.

After applying this patch, the CPU will be on a proper instruction
boundary, which means that it is safe to switch to any CPU model
(including hardware virtualized ones). This changeset fixes a bug
where the multiple switches to the same atomic CPU sometimes corrupts
the target state because of dangling pointers to the currently
executing microinstruction.

Note: This changeset moves tick event descheduling from switchOut() to
drain(), which makes timing consistent between just draining a system
and draining /and/ switching between two atomic CPUs. This makes
debugging quite a lot easier (execution traces get the same timing),
but the latency of the last instruction before a drain will not be
accounted for correctly (it will always be 1 cycle).

Note 2: This changeset removes so_state variable, the locked variable,
and the tickEvent from checkpoints since none of them contain state
that needs to be preserved across checkpoints. The so_state is made
redundant because we don't use the drain state variable anymore, the
lock variable should never be set when the system is drained, and the
tick event isn't scheduled.

1 files changed, 60 insertions, 47 deletions

diff --git a/src/cpu/simple/atomic.cc b/src/cpu/simple/atomic.cc
index 5348d24d8..036abdcdb 100644
--- a/src/cpu/simple/atomic.cc
+++ b/src/cpu/simple/atomic.cc
@@ -47,6 +47,7 @@
 #include "config/the_isa.hh"
 #include "cpu/simple/atomic.hh"
 #include "cpu/exetrace.hh"
+#include "debug/Drain.hh"
 #include "debug/ExecFaulting.hh"
 #include "debug/SimpleCPU.hh"
 #include "mem/packet.hh"
@@ -111,6 +112,7 @@ AtomicSimpleCPU::AtomicSimpleCPU(AtomicSimpleCPUParams *p)
     : BaseSimpleCPU(p), tickEvent(this), width(p->width), locked(false),
       simulate_data_stalls(p->simulate_data_stalls),
       simulate_inst_stalls(p->simulate_inst_stalls),
+      drain_manager(NULL),
       icachePort(name() + ".icache_port", this),
       dcachePort(name() + ".dcache_port", this),
       fastmem(p->fastmem)
@@ -126,37 +128,30 @@ AtomicSimpleCPU::~AtomicSimpleCPU()
     }
 }
 
-void
-AtomicSimpleCPU::serialize(ostream &os)
-{
-    Drainable::State so_state(getDrainState());
-    SERIALIZE_ENUM(so_state);
-    SERIALIZE_SCALAR(locked);
-    BaseSimpleCPU::serialize(os);
-    nameOut(os, csprintf("%s.tickEvent", name()));
-    tickEvent.serialize(os);
-}
-
-void
-AtomicSimpleCPU::unserialize(Checkpoint *cp, const string &section)
-{
-    Drainable::State so_state;
-    UNSERIALIZE_ENUM(so_state);
-    UNSERIALIZE_SCALAR(locked);
-    BaseSimpleCPU::unserialize(cp, section);
-    tickEvent.unserialize(cp, csprintf("%s.tickEvent", section));
-}
-
 unsigned int
-AtomicSimpleCPU::drain(DrainManager *drain_manager)
+AtomicSimpleCPU::drain(DrainManager *dm)
 {
-    setDrainState(Drainable::Drained);
-    return 0;
+    assert(!drain_manager);
+    if (_status == SwitchedOut)
+        return 0;
+
+    if (!isDrained()) {
+        DPRINTF(Drain, "Requesting drain: %s\n", pcState());
+        drain_manager = dm;
+        return 1;
+    } else {
+        if (tickEvent.scheduled())
+            deschedule(tickEvent);
+
+        DPRINTF(Drain, "Not executing microcode, no need to drain.\n");
+        return 0;
+    }
 }
 
 void
 AtomicSimpleCPU::drainResume()
 {
+    assert(!drain_manager);
     if (_status == Idle || _status == SwitchedOut)
         return;
 
@@ -166,23 +161,41 @@ AtomicSimpleCPU::drainResume()
               "'atomic' mode.\n");
     }
 
-    setDrainState(Drainable::Running);
-    if (thread->status() == ThreadContext::Active) {
-        if (!tickEvent.scheduled())
-            schedule(tickEvent, nextCycle());
-    }
+    assert(!tickEvent.scheduled());
+    if (thread->status() == ThreadContext::Active)
+        schedule(tickEvent, nextCycle());
+
     system->totalNumInsts = 0;
 }
 
+bool
+AtomicSimpleCPU::tryCompleteDrain()
+{
+    if (!drain_manager)
+        return false;
+
+    DPRINTF(Drain, "tryCompleteDrain: %s\n", pcState());
+    if (!isDrained())
+        return false;
+
+    DPRINTF(Drain, "CPU done draining, processing drain event\n");
+    drain_manager->signalDrainDone();
+    drain_manager = NULL;
+
+    return true;
+}
+
+
 void
 AtomicSimpleCPU::switchOut()
 {
     BaseSimpleCPU::switchOut();
 
+    assert(!tickEvent.scheduled());
     assert(_status == BaseSimpleCPU::Running || _status == Idle);
-    _status = SwitchedOut;
+    assert(isDrained());
 
-    tickEvent.squash();
+    _status = SwitchedOut;
 }
 
 
@@ -191,24 +204,19 @@ AtomicSimpleCPU::takeOverFrom(BaseCPU *oldCPU)
 {
     BaseSimpleCPU::takeOverFrom(oldCPU);
 
+    // The tick event should have been descheduled by drain()
     assert(!tickEvent.scheduled());
 
-    // if any of this CPU's ThreadContexts are active, mark the CPU as
-    // running and schedule its tick event.
-    ThreadID size = threadContexts.size();
-    for (ThreadID i = 0; i < size; ++i) {
-        ThreadContext *tc = threadContexts[i];
-        if (tc->status() == ThreadContext::Active &&
-            _status != BaseSimpleCPU::Running) {
-            _status = BaseSimpleCPU::Running;
-            schedule(tickEvent, nextCycle());
-            break;
-        }
-    }
-    if (_status != BaseSimpleCPU::Running) {
+    assert(!threadContexts.empty());
+    if (threadContexts.size() > 1)
+        fatal("The atomic CPU only supports one thread.\n");
+
+    // If the ThreadContext is active, mark the CPU as running.
+    if (thread->status() == ThreadContext::Active)
+        _status = BaseSimpleCPU::Running;
+    else
         _status = Idle;
-    }
-    assert(threadContexts.size() == 1);
+
     ifetch_req.setThreadContext(_cpuId, 0); // Add thread ID if we add MT
     data_read_req.setThreadContext(_cpuId, 0); // Add thread ID here too
     data_write_req.setThreadContext(_cpuId, 0); // Add thread ID here too
@@ -464,8 +472,10 @@ AtomicSimpleCPU::tick()
 
         checkPcEventQueue();
         // We must have just got suspended by a PC event
-        if (_status == Idle)
+        if (_status == Idle) {
+            tryCompleteDrain();
             return;
+        }
 
         Fault fault = NoFault;
 
@@ -549,6 +559,9 @@ AtomicSimpleCPU::tick()
             advancePC(fault);
     }
 
+    if (tryCompleteDrain())
+        return;
+
     // instruction takes at least one cycle
     if (latency < clockPeriod())
         latency = clockPeriod();