Hi Gabe,
On Sun, Mar 21, 2010 at 5:22 PM, Gabe Black <gbl...@eecs.umich.edu> wrote:
> It's been a while since I've looked at this, but I want to make sure I
> remember to respond so I don't want to wait until I have a chance to
> re-research all this. Isn't the problem that initiateAcc ends up calling
> completeAcc mid-function, that cleans up after the access, then
> initateAcc gets control back and tries to finish initiating the access?
Yes and no... you're right that that's how we viewed it before (so
your memory is good), and that's one way of looking at it, and it's
not incorrect. I'm saying that if we look at the problem differently
I think there's an easier solution.
> The problem to me seems to be that initiateAcc doesn't know whether it's
> just letting the rest of the CPU know something needs to happen later,
> or if it's actually causing that thing to happen right away. The other
> part, completeAcc, doesn't know if it's happening in the middle of
> initiateAcc, or if its in a callback. If it's in initiateAcc there are
> things it (or maybe initiateAcc, is that what you're saying?) shouldn't
> touch. If it's in a callback, it's likely the last thing to handle the
> instruction/translation/packet/whatever, so it needs to be sure to clean
> up everything. These seem like contradictory responsibilities.
You've got the right idea there... basically initiateAcc() actually
initiates the acc when it calls read() or write(), and it can't count
on the translation being complete when read()/write() returns, but
conversely it also shouldn't count on the translation *not* being
complete when read()/write() returns. So there really shouldn't be
any code after read()/write() that should care one way or the other.
If we get rid of the code that does, then we'll solve the problem
without adding any complexity to the control flow.
As a specific example, the one thing that appears to give us trouble
in initiateAcc() is this:
fault = xc->write((uint%(mem_acc_size)d_t&)Mem, EA,
memAccessFlags, NULL);
if (traceData) { traceData->setData(Mem); }
...and the root problem is that if the access completes in write(),
traceData may not be valid any more. The ironic thing here is that
(1) the value of Mem doesn't depend on write(), so the line could be
moved before the call; and (2) traceData->setData() is also called in
write() itself, so this whole line is redundant. So we can fix this
specific problem by either (1) moving the traceData line above write()
or (2) preferably, deleting the line entirely.
Similarly, read()/write() initiate the translation when they call
translateTiming(), and there shouldn't be any code in read()/write()
after that point that depends on the translation having completed *or
not*. Interestingly there is code (the req->isUncacheable() check,
and a setData() call in read()) that seems like it requires the
translation to have completed, which is just as incorrect. That code
should be moved somewhere else (like finishTranslation()) where it can
be guaranteed that the translation has finished.
> What I
> had been advocating before (if I remember right) was to disambiguate
> what the circumstances each function can expect by breaking up the call
> chains so that the functions don't end up nested inside each other
> unpredictably. Like I said, though, I'm making sure I respond rather
> than getting my facts straight, so please let me know if I'm missing
> your point (likely) or misunderstanding something.
That would work, but this approach is simpler since it doesn't depend
on figuring out what circumstances you're in.
Steve
_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
