From dc328d00ebe798f0b0ee8903aca4256bb128dc6f Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Wed, 29 Jan 2020 03:22:05 -0800 Subject: [PATCH] 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 Maintainer: Gabe Black Tested-by: kokoro --- src/arch/null/utility.hh | 1 + src/cpu/base.cc | 6 ------ src/sim/system.cc | 6 ++++-- 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 /** -- 2.30.2