cpu: fix how a thread starts up in MinorCPU

When a thread is activated by another thread calling a clone system
call, the child thread's context is initialized in the middle of the
clone system call and before the context is fully initialized.
Therefore, the child thread starts fetching an unitialized PC, which
could lead to a page fault.

This patch adds a pipeline wakeup event that is scheduled later in the
cycle when the thread is activated. This event ensures that the first
fetch only happens after the thread context is fully initialized
(e.g., in case of clone syscall, it is when the parent thread copies
its context over to the child thread).

When a thread first starts or wakes up, input queue to the Fetch2 stage
needs to be drained since the execution flow is likely to change and
previously fetched instructions in the queue may no longer be in the
correct flow. This patch dumps/drains all inputs in the input queue
of a thread context in the Fetch2 stage when the associated thread wakes
up.

Change-Id: Iad970638e435858b7289cd471158cc0afdbbb0e5
Reviewed-on: https://gem5-review.googlesource.com/c/8182
Reviewed-by: Brandon Potter <Brandon.Potter@amd.com>
Reviewed-by: Jason Lowe-Power <jason@lowepower.com>
Maintainer: Brandon Potter <Brandon.Potter@amd.com>

diff --git a/src/cpu/minor/cpu.cc b/src/cpu/minor/cpu.cc
index 63efde2dce3cffd828ff1a185fd9dcaa48829af8..484457bd415405f2119047c3eae25c5ca6632cdf 100644 (file)
--- a/src/cpu/minor/cpu.cc
+++ b/src/cpu/minor/cpu.cc
@@ -49,6 +49,7 @@
 
 MinorCPU::MinorCPU(MinorCPUParams *params) :
     BaseCPU(params),
+    pipelineStartupEvent([this]{ wakeupPipeline(); }, name()),
     threadPolicy(params->threadPolicy)
 {
     /* This is only written for one thread at the moment */
@@ -279,20 +280,43 @@ MinorCPU::takeOverFrom(BaseCPU *old_cpu)
 void
 MinorCPU::activateContext(ThreadID thread_id)
 {
-    DPRINTF(MinorCPU, "ActivateContext thread: %d\n", thread_id);
+    /* Remember to wake up this thread_id by scheduling the
+     * pipelineStartup event.
+     * We can't wakeupFetch the thread right away because its context may
+     * not have been fully initialized. For example, in the case of clone
+     * syscall, this activateContext function is called in the middle of
+     * the syscall and before the new thread context is initialized.
+     * If we start fetching right away, the new thread will fetch from an
+     * invalid address (i.e., pc is not initialized yet), which could lead
+     * to a page fault. Instead, we remember which threads to wake up and
+     * schedule an event to wake all them up after their contexts are
+     * fully initialized */
+    readyThreads.push_back(thread_id);
+    if (!pipelineStartupEvent.scheduled())
+        schedule(pipelineStartupEvent, clockEdge(Cycles(0)));
+}
 
-    /* Do some cycle accounting.  lastStopped is reset to stop the
-     *  wakeup call on the pipeline from adding the quiesce period
-     *  to BaseCPU::numCycles */
-    stats.quiesceCycles += pipeline->cyclesSinceLastStopped();
-    pipeline->resetLastStopped();
+void
+MinorCPU::wakeupPipeline()
+{
+    for (auto thread_id : readyThreads) {
+        DPRINTF(MinorCPU, "ActivateContext thread: %d\n", thread_id);
 
-    /* Wake up the thread, wakeup the pipeline tick */
-    threads[thread_id]->activate();
-    wakeupOnEvent(Minor::Pipeline::CPUStageId);
-    pipeline->wakeupFetch(thread_id);
+        /* Do some cycle accounting.  lastStopped is reset to stop the
+         *  wakeup call on the pipeline from adding the quiesce period
+         *  to BaseCPU::numCycles */
+        stats.quiesceCycles += pipeline->cyclesSinceLastStopped();
+        pipeline->resetLastStopped();
+
+        /* Wake up the thread, wakeup the pipeline tick */
+        threads[thread_id]->activate();
+        wakeupOnEvent(Minor::Pipeline::CPUStageId);
+
+        pipeline->wakeupFetch(thread_id);
+        BaseCPU::activateContext(thread_id);
+    }
 
-    BaseCPU::activateContext(thread_id);
+    readyThreads.clear();
 }
 
 void

diff --git a/src/cpu/minor/cpu.hh b/src/cpu/minor/cpu.hh
index 4e4762390034fde5cbc11b274d3386437b295030..606a401b67dcb1bf2c7cc9a87d1a2c12831394fd 100644 (file)
--- a/src/cpu/minor/cpu.hh
+++ b/src/cpu/minor/cpu.hh
@@ -83,6 +83,13 @@ class MinorCPU : public BaseCPU
      *  Elements of pipeline call TheISA to implement the model. */
     Minor::Pipeline *pipeline;
 
+    /** An event that wakes up the pipeline when a thread context is
+     * activated */
+    EventFunctionWrapper pipelineStartupEvent;
+
+    /** List of threads that are ready to wake up and run */
+    std::vector<ThreadID> readyThreads;
+
   public:
     /** Activity recording for pipeline.  This belongs to Pipeline but
      *  stages will access it through the CPU as the MinorCPU object
@@ -165,6 +172,9 @@ class MinorCPU : public BaseCPU
     void activateContext(ThreadID thread_id) override;
     void suspendContext(ThreadID thread_id) override;
 
+    /** Wake up ready-to-run threads */
+    void wakeupPipeline();
+
     /** Thread scheduling utility functions */
     std::vector<ThreadID> roundRobinPriority(ThreadID priority)
     {

diff --git a/src/cpu/minor/execute.cc b/src/cpu/minor/execute.cc
index 7b76ca2e1b504be1a932ccb6bfb6892336ef46de..93c0895ac94dedd5181dca103a54e2fd40f4615e 100644 (file)
--- a/src/cpu/minor/execute.cc
+++ b/src/cpu/minor/execute.cc
@@ -1054,7 +1054,8 @@ Execute::commit(ThreadID thread_id, bool only_commit_microops, bool discard,
         !branch.isStreamChange() && /* No real branch */
         fault == NoFault && /* No faults */
         completed_inst && /* Still finding instructions to execute */
-        num_insts_committed != commitLimit /* Not reached commit limit */
+        num_insts_committed != commitLimit && /* Not reached commit limit */
+        cpu.getContext(thread_id)->status() != ThreadContext::Suspended
         )
     {
         if (only_commit_microops) {

diff --git a/src/cpu/minor/fetch2.cc b/src/cpu/minor/fetch2.cc
index ba898d987f3c98c72405156cde33159bba66ac97..09a06fc240ac44c08f1423dc02b2566d9f15ee5a 100644 (file)
--- a/src/cpu/minor/fetch2.cc
+++ b/src/cpu/minor/fetch2.cc
@@ -120,6 +120,7 @@ Fetch2::dumpAllInput(ThreadID tid)
         popInput(tid);
 
     fetchInfo[tid].inputIndex = 0;
+    fetchInfo[tid].havePC = false;
 }
 
 void

diff --git a/src/cpu/minor/fetch2.hh b/src/cpu/minor/fetch2.hh
index c66fbd8dc434c87f7d6928064b6c63dce966f396..2230560f164335bfe1218ff635403e92a645bb64 100644 (file)
--- a/src/cpu/minor/fetch2.hh
+++ b/src/cpu/minor/fetch2.hh
@@ -172,6 +172,11 @@ class Fetch2 : public Named
     Stats::Scalar loadInstructions;
     Stats::Scalar storeInstructions;
 
+  public:
+    /** Dump the whole contents of the input buffer.  Useful after a
+     *  prediction changes control flow */
+    void dumpAllInput(ThreadID tid);
+
   protected:
     /** Get a piece of data to work on from the inputBuffer, or 0 if there
      *  is no data. */
@@ -180,10 +185,6 @@ class Fetch2 : public Named
     /** Pop an element off the input buffer, if there are any */
     void popInput(ThreadID tid);
 
-    /** Dump the whole contents of the input buffer.  Useful after a
-     *  prediction changes control flow */
-    void dumpAllInput(ThreadID tid);
-
     /** Update local branch prediction structures from feedback from
      *  Execute. */
     void updateBranchPrediction(const BranchData &branch);

diff --git a/src/cpu/minor/pipeline.cc b/src/cpu/minor/pipeline.cc
index b5659ac0da356d6849b3151e5502285c9477f7a8..3248d54657e376688ff165ec07162cb5380e01d0 100644 (file)
--- a/src/cpu/minor/pipeline.cc
+++ b/src/cpu/minor/pipeline.cc
@@ -199,6 +199,7 @@ void
 Pipeline::wakeupFetch(ThreadID tid)
 {
     fetch1.wakeupFetch(tid);
+    fetch2.dumpAllInput(tid);
 }
 
 bool