sim,cpu: Move the call to initCPU into System.

The call to initCPU was moved into initState in the base CPU class
since it should only really be called when starting a simulation
fresh. Otherwise checkpointed state will be loaded over the state of
the CPU anyway, so there's no reason to set up anything else.

Unfortunately that made it possible for the System level initialization
and the CPU initialization to happen out of order, effectively letting
initCPU clobber the state the System might have set up to prepare for
executing a kernel for instance.

To work around that issue, the call was moved to init which would
necessarily happen before initState, restoring the original ordering.

This change moves the change *back* into initState, but of the System
class instead of the CPU class. This makes it possible to guarantee
that OS initialization happens after initCPU since that's also done
by System subclasses, and they control when they call initCPU of the
base class.

This also slightly simmplifies when initCPU is called since we
shouldn't need to check whether a context is switched out or not. If
it's registered with the System object, then it should be in a
currently swapped in CPU.

This also puts the initCPU and startupCPU calls right next to each
other. A future change will take advantage of that and merge the
calls together.

Also, because there are already ISA specific subclasses of System
which already have specialized versions of initState, we should be
able to move the code in initCPU and startupCPU directly into those
subclasses. That will give those subclasses more flexibilty if, for
instance, they want all CPUs to start running in the BIOS like they
would on a real system, or if they want only the BSP to be active
as if the BIOS had already paused the APs before passing control to
a bootloader or OS.

This will also remove another two TheISA:: style functions, reducing
the number of global dependencies on a single ISA.

Change-Id: Ic56924660a5b575a07844a198f69a0e7fa212b52
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/24903
Reviewed-by: Gabe Black <gabeblack@google.com>
Maintainer: Gabe Black <gabeblack@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>

3 files changed, 5 insertions, 8 deletions

diff --git a/src/arch/null/utility.hh b/src/arch/null/utility.hh
index cf65ef5cd..d92e55221 100644
--- a/src/arch/null/utility.hh
+++ b/src/arch/null/utility.hh
@@ -48,6 +48,7 @@ namespace NullISA {
 inline uint64_t getArgument(ThreadContext *tc, int &number, uint16_t size,
                             bool fp) { return 0; }
 
+inline void initCPU(ThreadContext *tc, int cpuId) {}
 inline void startupCPU(ThreadContext *tc, int cpuId) {}
 
 }
diff --git a/src/cpu/base.cc b/src/cpu/base.cc
index 80726ac2b..ac0c7ac5b 100644
--- a/src/cpu/base.cc
+++ b/src/cpu/base.cc
@@ -318,12 +318,6 @@ BaseCPU::init()
 
         verifyMemoryMode();
     }
-
-    //These calls eventually need to be moved to initState
-    if (FullSystem && !params()->switched_out) {
-        for (auto *tc: threadContexts)
-            TheISA::initCPU(tc, tc->contextId());
-    }
 }
 
 void
diff --git a/src/sim/system.cc b/src/sim/system.cc
index 739b42278..e59c40477 100644
--- a/src/sim/system.cc
+++ b/src/sim/system.cc
@@ -349,8 +349,10 @@ void
 System::initState()
 {
     if (FullSystem) {
-        for (int i = 0; i < threadContexts.size(); i++)
-            TheISA::startupCPU(threadContexts[i], i);
+        for (auto *tc: threadContexts) {
+            TheISA::initCPU(tc, tc->contextId());
+            TheISA::startupCPU(tc, tc->contextId());
+        }
         // Moved from the constructor to here since it relies on the
         // address map being resolved in the interconnect
         /**