I can see where this patch solves the immediate problem, but it's not clear
it's not just a band-aid. The real question is what is the code supposed to
be doing?
The larger context in FullO3CPU<Impl>::init() is this:
for (int tid=0; tid < number_of_threads; tid++) {
#if FULL_SYSTEM
ThreadContext *src_tc = threadContexts[tid];
#else
ThreadContext *src_tc = thread[tid]->getTC();
#endif
// Threads start in the Suspended State
if (src_tc->status() != ThreadContext::Suspended) {
continue;
}
#if FULL_SYSTEM
TheISA::initCPU(src_tc, src_tc->contextId());
#endif
}
So the idea is that we only call initCPU() if the thread is in the Suspended
state (I believe there's a more straightforward way to code that!), which
the comment implies is because we only want to call initCPU() when the
thread is just starting up. There's no indication of why we would ever call
FullO3CPU<Impl>::init() when the thread isn't just starting up though (i.e.,
did this check ever fail in practice?). Does it have something to do with
creating an O3CPU but not starting it up right away because we're starting
in SimpleCPU instead and are going to do a switchover later? This condition
isn't present in any of the SimpleCPU models.
As far as wakeup, I think there should be at least a warning and maybe an
assertion if you wakeup a Halted cpu. My thought is that a Halted CPU
doesn't have a PC to run from, so it doesn't make sense just to wake it up.
If there is a PC for it to run from it should be in Suspended. If it's
assigned a PC but you don't want it to start running right away, it should
transition from Halted to Suspended at that time. This model may not work
cleanly with starting up a CPU via an interrupt, and I'm willing to rethink
it if that's the case, but let's do it deliberately and not ad hoc.
Sorry for not catching this before I committed... I thought I had run all of
the long regressions, but somehow I missed this one (or missed the fact that
it failed).
Steve
On Thu, Apr 16, 2009 at 10:37 PM, Gabe Black <gbl...@eecs.umich.edu> wrote:
> The patch below seems to fix it. I think there may be a similar issue
> with the in order model. There's a bug in the wakeup function of I
> believe all the CPU models where it only wakes up from Suspended and
> just returns if you're Halted. I have a patch in my queue that fixes it
> for the simple CPU, but a more general fix would be appropriate, I
> think. I'll leave committing these to you guys since you know your code
> best.
>
> Gabe
>
> diff -r be16ad28822f src/cpu/o3/cpu.cc
> --- a/src/cpu/o3/cpu.cc Wed Apr 15 13:18:24 2009 -0700
> +++ b/src/cpu/o3/cpu.cc Thu Apr 16 22:32:49 2009 -0700
> @@ -575,7 +575,7 @@
> ThreadContext *src_tc = thread[tid]->getTC();
> #endif
> // Threads start in the Suspended State
> - if (src_tc->status() != ThreadContext::Suspended) {
> + if (src_tc->status() != ThreadContext::Halted) {
> continue;
> }
>
>
_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
