On Jan. 5, 2015, 10:48 p.m., Andreas Hansson wrote: > > Can you explain the sequence of events that result in bad behavior? If > > possible (which is not necessarily the case), it would be best to fix this > > without adding arbitrary delays. > > Andreas Hansson wrote: > The crossbar is processing a response, and as part of that forward it to > the next object, in this case the pagetable walker, and before the call to > sendTimingResp is finished, the pagetable walker sends a new request back to > the crossbar. > > This change ensures that the sendTimingResp is finished and done with > before the request is sent. > > If you do not like the idea of sending it on the next clock edge we can > also make it curTick(). > > Gabe Black wrote: > curTick would be better I think. I feel like I've run into this sort of > thing before, where I've wanted to do something inline but there were > assumptions/expectations that it be done in a different call stack. I *think* > we even had issues where we'd run out of stack when that looped back on > itself too much under certain circumstances, but it may just be that that > theoretically could happen but hadn't. In any case, it feels like there's a > generic problem to be solved here, although not necessarily as part of what > you're doing. > > Andreas Hansson wrote: > The main reason I went for clockEdge rather than curTick is that it > aligns the request sending with the CPU clock edges rather than the crossbar. > If curTick is aligned with the CPU clock then clockEdge == curTick. Only if > the response arrives in the middle of a CPU cycle will the request be delayed > to the next edge. Makes sense? > > From an realism point of view I do not see how any sensible > implementation would have a direct combinational path from receiving a > response to sending a request (not that it has stopped us in other places in > the code base). > > Gabe Black wrote: > Yep, that makes sense.
I will consider that a "Ship it" unless someone objects. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2595/#review5724 ----------------------------------------------------------- On Jan. 5, 2015, 4:35 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2595/ > ----------------------------------------------------------- > > (Updated Jan. 5, 2015, 4:35 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10643:30307bfe1e8c > --------------------------- > x86: Delay X86 table walk on receiving walker response > > This patch fixes a minor issue in the X86 page table walker where it > ended up sending new request packets to the crossbar before the > response processing was finished (recvTimingResp is directly calling > sendTimingReq). Under certain conditions this caused the crossbar to > see illegal combinations of request/response overlap, in turn causing > problems with a slightly modified crossbar implementation. > > > Diffs > ----- > > src/arch/x86/pagetable_walker.hh 9ac724889705 > src/arch/x86/pagetable_walker.cc 9ac724889705 > > Diff: http://reviews.gem5.org/r/2595/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
