----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1826/#review4247 -----------------------------------------------------------
Overall this looks like it's going in the right direction. I have some concerns and issue we should resolve, but overall it's a nice addition to the current functionality. src/mem/cache/cache_impl.hh <http://reviews.gem5.org/r/1826/#comment4032> I am a little concerned here as I am not sure what will happen first: the bank being marked free or the retry sent? Ultimately I would suggest to complicate things a little bit further and set a flag here (mustSendRespRetry or similar) and then in the event that unblocks the bank (is there one?) send out the retry once everything else is done. src/mem/coherent_bus.hh <http://reviews.gem5.org/r/1826/#comment4030> I'm not thrilled at how it's done at the moment, but I can see the need for something similar. I have quite some bus patches in the pipeline, I'll try and get them on the reviewboard early next week. src/mem/coherent_bus.cc <http://reviews.gem5.org/r/1826/#comment4031> As above, in general this looks like the right way forward. I have some major reshuffling of this in the pipeline, making the buses properly multi-layered and accounting for their own time, so I'd like to wait with adding this functionality until that is done if that's ok. - Andreas Hansson On April 17, 2013, 11:19 a.m., Xiangyu Dong wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1826/ > ----------------------------------------------------------- > > (Updated April 17, 2013, 11:19 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9639:2aae67c49743 > --------------------------- > mem: add retry mechanism for cache fills in classic cache model > The changeset 6122d201ff80 modeled the cache bank and blocks the cache access > if the target bank is busy. However, due to the lack of retry mechanism at > the > cache master port, that changeset cannot properly blocks the cache traffic > that > is towards CPU. > This patch modifies the CoherentBus model and adds a flow control scheme to > the > RespLayer. With this modification, cache fill operations can now be properly > modeled. > @todo The modification to CoherentBus is a little hacky. e.g. The recvRetry > functions for Req and Resp are not symmetric > @todo Might also need to modify noncoherent bus > @todo There is no write buffer entries for cache fill operations. An > incremental patch will be needed to model the cache fill buffer > > > Diffs > ----- > > src/mem/coherent_bus.hh 6d4158ff7b82 > src/mem/coherent_bus.cc 6d4158ff7b82 > src/mem/cache/cache_impl.hh 6d4158ff7b82 > src/mem/cache/base.hh 6d4158ff7b82 > > Diff: http://reviews.gem5.org/r/1826/diff/ > > > Testing > ------- > > > Thanks, > > Xiangyu Dong > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
