> On June 1, 2015, 5:14 p.m., Brad Beckmann wrote: > > src/mem/protocol/MESI_Two_Level-L1cache.sm, line 1227 > > <http://reviews.gem5.org/r/2842/diff/1/?file=45368#file45368line1227> > > > > Why introduce a new action here? It seems like the simplier way to > > handle this situation is insert the forward_eviction_to_cpu action after > > the hx_load_hit callback rather than introduce a new action and additional > > complexity in the sequencer.
Calling hx_load_hit and then forward_eviction_to_cpu would always have to be seen by the LSQ in that order. To guarantee this is never an issue, both should be sent as one. The main reason being that if the ReadResp arrives after the InvalidateReq in the LSQ, the ReExec fault is lost in the LSQ. See comment in LSQ::recvTimingResp. From what I can see, this order is currently not guaranteed (and I'm not sure what the cost of guaranteeing it would be). In RubyPort, the InvalidateReq is *not* sent with a schedTimingResp, yet any response via hitCallback is. This means, that regardless of the calling (action) order, the InvalidateReq always arrives before the ReadResp, and the fault is always lost in the LSQ. There are likely other potential fixes (fix LSQ not to depend on order, guarantee order from Ruby side), but this one seems the least intrusive and most robust. - Marco ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2842/#review6447 ----------------------------------------------------------- On May 21, 2015, 7:27 p.m., Marco Elver wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2842/ > ----------------------------------------------------------- > > (Updated May 21, 2015, 7:27 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10839:2978f9ed5e7a > --------------------------- > ruby: Add ReadRespWithInvalidate support, fixes MESI consistency bug > > A sunk Inv in the IS state needs to be propagated to the LQ eventually. Thus > far, Ruby has no support for properly propagating an invalidate along with a > ReadResp. > > This patch adds support for ReadRespWithInvalidate upon a readCallback, so > that the LQ can properly deal with such a request. > > Note that we cannot just do a forward_eviction in the IS,Inv transition, as > other outstanding loads in the LQ after the one for which we get the Inv, may > be satisfied (with old values) inbetween the Inv and IS_I,Data transitions. > > The scenarios are the same as for the problem fixed in revision 10575 (which > addressed the LSQ side of things). > > > Diffs > ----- > > src/mem/protocol/MESI_Two_Level-L1cache.sm ecbab2522757 > src/mem/protocol/RubySlicc_Types.sm ecbab2522757 > src/mem/ruby/system/Sequencer.hh ecbab2522757 > src/mem/ruby/system/Sequencer.cc ecbab2522757 > > Diff: http://reviews.gem5.org/r/2842/diff/ > > > Testing > ------- > > Tester no longer finds bug. Note this was done with an (older) version of > gem5 that was still happy with Ruby+O3CPU. A quick run with ruby-tester > yielded no obvious issues. > > > *NOTE:* I'm not entirely happy with the "Hack?" code. If someone can possibly > suggest how we can add a nicer way to add support for ReadRespWithInvalidate > to Ruby, then that'd probably be better than what this patch does. > > > Thanks, > > Marco Elver > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
