Steve Reinhardt wrote:
> 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.
I'm not an expert on this code, but I think it's preventing excessively
restarting the CPU if you do a switchover or resume from a checkpoint.
>
> 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.
So halted would be basically powered off and uninitialized and suspended
would be ready and waiting for something to do? In that case, I'd want
to initialize the unused CPUs to suspended and make the "halt" microop
use suspended because the CPUs couldn't be started otherwise. If I'm
just using it wrong I'll stop doing that :)
>
> 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).
No problem.
Gabe
_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
