----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1904/#review4415 -----------------------------------------------------------
I get the feeling that this will completely break all the assumptions we make about a drained system not executing microcode. You need to change that before committing, otherwise all KVM-related work will break. src/cpu/o3/commit_impl.hh <http://reviews.gem5.org/r/1904/#comment4140> This breaks all the assumptions we make about a drained system. It doesn't matter if it is Idle or not, as long as it is executing microcode, it isn't drained and will break things. src/cpu/simple/atomic.cc <http://reviews.gem5.org/r/1904/#comment4137> I'm not sure if this is actually correct. Isn't it possible for a CPU to be Idle and in microcode or something else that will break the assumptions we have about drained CPUs? src/cpu/simple/atomic.cc <http://reviews.gem5.org/r/1904/#comment4138> isDrained() should always be true, independent of the state, when we reach this code. Could you move it to a separate assert to make sure we better error messages if it explodes? src/cpu/simple/timing.cc <http://reviews.gem5.org/r/1904/#comment4139> Same as for the atomic CPU. Should probably be something like this: assert(_statue == Running || _status == Idle); assert(isDrained()); - Andreas Sandberg On June 7, 2013, 3:48 p.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1904/ > ----------------------------------------------------------- > > (Updated June 7, 2013, 3:48 p.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9748:5d7be2fc04c7 > --------------------------- > cpu: some changes to switching > > 1. The fetch stage, when it overtakes from another cpu, initializes its > status variable to Running. When an o3 cpu is about to switch to another > cpu, the fetch stage checks that its status should be Idle. Now suppose > there are two processors in the system. The operating system has just > started and it is running on cpu0. Then, cpu1 would not be actually doing > anything. When trying to switch to another cpu, cpu1 gets stuck because > there is nothing going on that will move it from Running to Idle. > > I think we should have the fetchStatus be initialized to Idle state. > The o3 cpu will activate some context at point in future. When it does > that, it calls the function wakeFromQuiesce() on the fetch stage. This > function in turn changes the fetchStatus to Running. As of now, the > function only does this for thread 0. I am proposing that we > pass the thread id and do it for that particular thread. > > 2. The TimingSimpleCPU incorrectly tested its status when switching out. > > 3. The commit stage should check for the its status being Idle when > testing whether it needs to drain itself. > > 4. The atomic cpu should test its status variable before checking any > other variables for deciding on draining. > > > Diffs > ----- > > src/cpu/o3/commit_impl.hh ea26ba576891 > src/cpu/o3/cpu.cc ea26ba576891 > src/cpu/o3/fetch.hh ea26ba576891 > src/cpu/o3/fetch_impl.hh ea26ba576891 > src/cpu/simple/atomic.cc ea26ba576891 > src/cpu/simple/timing.cc ea26ba576891 > > Diff: http://reviews.gem5.org/r/1904/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
