----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2691/#review6124 -----------------------------------------------------------
I vote for the non-goto verison. Is there really no way we can achieve this without the extra packets going back and forth? It seems the shortcircuiting we do with packets we should be able to do without them if we tapped in one level below the recvTimingReq/recvTimingResp. src/mem/cache/cache_impl.hh (line 527) <http://reviews.gem5.org/r/2691/#comment5327> later on we test pkt->isLockedRMW and then read or write. Could we not do the same here rather and avoid looking at the actual command, i.e. pkt->isLockedRMW() && pkt->isWrite() src/mem/cache/cache_impl.hh (line 621) <http://reviews.gem5.org/r/2691/#comment5328> status should be 0 even, or could it still have the dirty flag set? the readable flag is essentially valid, so here we invent a new state Readable = 0 Dirty = ? Writeable = 0 Still the block is "valid". This scares me a bit. Partly because the binary encoding of cache states is not that easy to follow, and partly because we now have MOESI + some new state. src/mem/cache/cache_impl.hh (line 638) <http://reviews.gem5.org/r/2691/#comment5329> Why is this MSHR needed? It was faked, was it not? src/mem/cache/cache_impl.hh (line 639) <http://reviews.gem5.org/r/2691/#comment5330> Is it safe to call this here and ignore the return value? It would be nice if we do not assume that the response is always accepted. Perhaps schedule an event and put it in a queue rather? src/mem/cache/cache_impl.hh (line 1296) <http://reviews.gem5.org/r/2691/#comment5331> potentially still dirty, or always came from "Exclusive", i.e. Readable | Writeable? src/mem/packet.hh (line 107) <http://reviews.gem5.org/r/2691/#comment5326> Do we need the actual LockedRMW... or could if just be a flag on the existing read and write req/resp? - Andreas Hansson On May 6, 2015, 10:59 p.m., Steve Reinhardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2691/ > ----------------------------------------------------------- > > (Updated May 6, 2015, 10:59 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10745:9b84d1b570e3 > --------------------------- > mem: implement x86 locked accesses in timing-mode classic cache > > Add LockedRMW(Read|Write)(Req|Resp) commands. In timing mode, > use a combination of clearing permission bits and leaving > an MSHR in place to prevent accesses & snoops from touching > a locked block between the read and write parts of an locked > RMW sequence. > > > Diffs > ----- > > src/mem/cache/cache_impl.hh fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/cache/mshr.hh fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/packet.hh fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/packet.cc fbdaa08aaa426b9f4660c366f934ccb670d954ec > > Diff: http://reviews.gem5.org/r/2691/diff/ > > > Testing > ------- > > > Thanks, > > Steve Reinhardt > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
