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? 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. 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.
Gabe Steve Reinhardt wrote: > Thanks to Brad's prompting, I took another look at this tonight. > > I think the fundamental problem is that the transition to > split-transaction translation didn't move all the operations that > depend on the translation completing into the translation callback or > something that was guaranteed to run after the translation callback. > The req->isUncacheable() test that Brad moved around in his recent > patch (which I commented on just a few minutes ago) is one example of > that. Another case is that we're calling traceData->setData() in > read() even though the translation may not be done yet (how does that > work? are we calling setData() again when the real data comes back, > so we haven't noticed that this is broken?). The other traceData > examples (like setAddr() or setData() in write()) are more like > anti-dependences; there's no direct dependence on the translation > finishing, but there is a dependence on it not deleting the traceData > object. > > So in other words the problem isn't that the finishTranslation() or > completeAcc() calls are happening too soon, it's that the code that > runs after the translation is initiated in read()/write() (or in > initiateAcc() from which it is called) should not be making > assumptions about whether finishTranslation() or completeAcc() has or > has not been called. I think it's pretty easy to fix this, in some > cases by moving independent code up above where the translation is > initiated, and in other cases by moving dependent code into > finishTranslation(). > > Interestingly, there are a number of cases where we trace the store > data twice, once in the CPU's write() function and again in the ISA's > initiateAcc() function. I propose to just get rid of the latter. > > As I mentioned in the other email, I'm tempted to just get rid of the > recordEvent() calls, unless someone actually uses these. I see that > Nate created the basic recordEvent() infrastructure in 2004, and right > now it's only uncached accesses and faults that get recorded, which > seems > very ad hoc and irregular. > > I'll try and make these changes and test them as soon as I can. I > thought I'd send this note out first though in case anyone has > feedback before I get started. > > Steve > _______________________________________________ > m5-dev mailing list > m5-dev@m5sim.org > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
