From 02dafc5498750d9734ba8f2a1608a846f90b71d1 Mon Sep 17 00:00:00 2001 From: Tuan Ta Date: Mon, 2 Apr 2018 15:19:40 -0400 Subject: [PATCH] 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 Reviewed-by: Jason Lowe-Power Maintainer: Brandon Potter --- src/cpu/minor/cpu.cc | 46 +++++++++++++++++++++++++++++---------- src/cpu/minor/cpu.hh | 10 +++++++++ src/cpu/minor/execute.cc | 3 ++- src/cpu/minor/fetch2.cc | 1 + src/cpu/minor/fetch2.hh | 9 ++++---- src/cpu/minor/pipeline.cc | 1 + 6 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/cpu/minor/cpu.cc b/src/cpu/minor/cpu.cc index 63efde2dc..484457bd4 100644 --- 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 4e4762390..606a401b6 100644 --- 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 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 roundRobinPriority(ThreadID priority) { diff --git a/src/cpu/minor/execute.cc b/src/cpu/minor/execute.cc index 7b76ca2e1..93c0895ac 100644 --- 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 ba898d987..09a06fc24 100644 --- 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 c66fbd8dc..2230560f1 100644 --- 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 b5659ac0d..3248d5465 100644 --- 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 -- 2.30.2