diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/snoop_filter.cc --- a/src/mem/snoop_filter.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/snoop_filter.cc Tue Dec 22 16:23:08 2015 +0000 @@ -108,7 +108,7 @@ lookupLatency); if (cpkt->needsResponse()) { - if (!cpkt->memInhibitAsserted()) { + if (!cpkt->cacheResponding()) { // Max one request per address per port panic_if(sf_item.requested & req_port, "double request :( " \ "SF value %x.%x\n", sf_item.requested, sf_item.holder); @@ -208,7 +208,7 @@ // snoops so this was never an aissue. Now that Writebacks generate snoops // we need to special case for Writebacks. assert(cpkt->isWriteback() || cpkt->req->isUncacheable() || - (cpkt->isInvalidate() == cpkt->needsExclusive())); + (cpkt->isInvalidate() == cpkt->needsWritable())); if (cpkt->isInvalidate() && !sf_item.requested) { // Early clear of the holder, if no other request is currently going on // @todo: This should possibly be updated even though we do not filter @@ -233,7 +233,7 @@ cpkt->cmdString()); assert(cpkt->isResponse()); - assert(cpkt->memInhibitAsserted()); + assert(cpkt->cacheResponding()); // Ultimately we should check if the packet came from an // allocating source, not just if the port is snooping @@ -258,10 +258,10 @@ "the original request\n", sf_item.requested, sf_item.holder); // Update the residency of the cache line. - if (cpkt->needsExclusive() || !cpkt->sharedAsserted()) { - DPRINTF(SnoopFilter, "%s: dropping %x because needs: %i shared: %i "\ + if (cpkt->needsWritable() || cpkt->passWritable()) { + DPRINTF(SnoopFilter, "%s: dropping %x because needs: %i writable: %i "\ "SF val: %x.%x\n", __func__, rsp_mask, - cpkt->needsExclusive(), cpkt->sharedAsserted(), + cpkt->needsWritable(), cpkt->passWritable(), sf_item.requested, sf_item.holder); sf_item.holder &= ~rsp_mask; @@ -287,7 +287,7 @@ cpkt->cmdString()); assert(cpkt->isResponse()); - assert(cpkt->memInhibitAsserted()); + assert(cpkt->cacheResponding()); Addr line_addr = cpkt->getBlockAddr(linesize); auto sf_it = cachedLocations.find(line_addr); @@ -305,7 +305,7 @@ // Remote (to this snoop filter) snoops update the filter // already when they arrive from below, because we may not see // any response. - if (cpkt->needsExclusive()) { + if (cpkt->needsWritable()) { // If the request to this snoop response hit an in-flight // transaction, // the holder was not reset -> no assertion & do that here, now! @@ -345,7 +345,7 @@ // Update the residency of the cache line. Here we assume that the // line has been zapped in all caches that are not the responder. - if (cpkt->needsExclusive() || !cpkt->sharedAsserted()) + if (cpkt->needsWritable() || cpkt->passWritable()) sf_item.holder = 0; sf_item.holder |= slave_mask; sf_item.requested &= ~slave_mask; diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/tport.cc --- a/src/mem/tport.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/tport.cc Tue Dec 22 16:23:08 2015 +0000 @@ -63,9 +63,11 @@ SimpleTimingPort::recvTimingReq(PacketPtr pkt) { // the SimpleTimingPort should not be used anywhere where there is - // a need to deal with inhibited packets - if (pkt->memInhibitAsserted()) - panic("SimpleTimingPort should never see an inhibited request\n"); + // a need to deal with snoop responses and their flow control + // requirements + if (pkt->cacheResponding()) + panic("SimpleTimingPort should never see packets with the " + "cacheResponding flag set\n"); bool needsResponse = pkt->needsResponse(); Tick latency = recvAtomic(pkt); diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/packet.cc --- a/src/mem/packet.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/packet.cc Tue Dec 22 16:23:08 2015 +0000 @@ -80,10 +80,10 @@ { SET4(IsRead, IsResponse, HasData, IsInvalidate), InvalidCmd, "ReadRespWithInvalidate" }, /* WriteReq */ - { SET5(IsWrite, NeedsExclusive, IsRequest, NeedsResponse, HasData), + { SET5(IsWrite, NeedsWritable, IsRequest, NeedsResponse, HasData), WriteResp, "WriteReq" }, /* WriteResp */ - { SET3(IsWrite, NeedsExclusive, IsResponse), InvalidCmd, "WriteResp" }, + { SET3(IsWrite, NeedsWritable, IsResponse), InvalidCmd, "WriteResp" }, /* WritebackDirty */ { SET4(IsWrite, IsRequest, IsEviction, HasData), InvalidCmd, "WritebackDirty" }, @@ -107,34 +107,34 @@ { SET4(IsRead, IsResponse, IsHWPrefetch, HasData), InvalidCmd, "HardPFResp" }, /* WriteLineReq */ - { SET5(IsWrite, NeedsExclusive, IsRequest, NeedsResponse, HasData), + { SET5(IsWrite, NeedsWritable, IsRequest, NeedsResponse, HasData), WriteResp, "WriteLineReq" }, /* UpgradeReq */ - { SET5(IsInvalidate, NeedsExclusive, IsUpgrade, IsRequest, NeedsResponse), + { SET5(IsInvalidate, NeedsWritable, IsUpgrade, IsRequest, NeedsResponse), UpgradeResp, "UpgradeReq" }, /* SCUpgradeReq: response could be UpgradeResp or UpgradeFailResp */ - { SET6(IsInvalidate, NeedsExclusive, IsUpgrade, IsLlsc, + { SET6(IsInvalidate, NeedsWritable, IsUpgrade, IsLlsc, IsRequest, NeedsResponse), UpgradeResp, "SCUpgradeReq" }, /* UpgradeResp */ - { SET3(NeedsExclusive, IsUpgrade, IsResponse), + { SET3(NeedsWritable, IsUpgrade, IsResponse), InvalidCmd, "UpgradeResp" }, /* SCUpgradeFailReq: generates UpgradeFailResp but still gets the data */ - { SET6(IsRead, NeedsExclusive, IsInvalidate, + { SET6(IsRead, NeedsWritable, IsInvalidate, IsLlsc, IsRequest, NeedsResponse), UpgradeFailResp, "SCUpgradeFailReq" }, /* UpgradeFailResp - Behaves like a ReadExReq, but notifies an SC * that it has failed, acquires line as Dirty*/ - { SET4(IsRead, NeedsExclusive, IsResponse, HasData), + { SET4(IsRead, NeedsWritable, IsResponse, HasData), InvalidCmd, "UpgradeFailResp" }, /* ReadExReq - Read issues by a cache, always cache-line aligned, * and the response is guaranteed to be writeable (exclusive or * even modified) */ - { SET5(IsRead, NeedsExclusive, IsInvalidate, IsRequest, NeedsResponse), + { SET5(IsRead, NeedsWritable, IsInvalidate, IsRequest, NeedsResponse), ReadExResp, "ReadExReq" }, /* ReadExResp - Response matching a read exclusive, as we check * the need for exclusive also on responses */ - { SET4(IsRead, NeedsExclusive, IsResponse, HasData), + { SET4(IsRead, NeedsWritable, IsResponse, HasData), InvalidCmd, "ReadExResp" }, /* ReadCleanReq - Read issued by a cache, always cache-line * aligned, and the response is guaranteed to not contain dirty data @@ -149,21 +149,21 @@ { SET4(IsRead, IsLlsc, IsRequest, NeedsResponse), ReadResp, "LoadLockedReq" }, /* StoreCondReq */ - { SET6(IsWrite, NeedsExclusive, IsLlsc, + { SET6(IsWrite, NeedsWritable, IsLlsc, IsRequest, NeedsResponse, HasData), StoreCondResp, "StoreCondReq" }, /* StoreCondFailReq: generates failing StoreCondResp */ - { SET6(IsWrite, NeedsExclusive, IsLlsc, + { SET6(IsWrite, NeedsWritable, IsLlsc, IsRequest, NeedsResponse, HasData), StoreCondResp, "StoreCondFailReq" }, /* StoreCondResp */ - { SET4(IsWrite, NeedsExclusive, IsLlsc, IsResponse), + { SET4(IsWrite, NeedsWritable, IsLlsc, IsResponse), InvalidCmd, "StoreCondResp" }, /* SwapReq -- for Swap ldstub type operations */ - { SET6(IsRead, IsWrite, NeedsExclusive, IsRequest, HasData, NeedsResponse), + { SET6(IsRead, IsWrite, NeedsWritable, IsRequest, HasData, NeedsResponse), SwapResp, "SwapReq" }, /* SwapResp -- for Swap ldstub type operations */ - { SET5(IsRead, IsWrite, NeedsExclusive, IsResponse, HasData), + { SET5(IsRead, IsWrite, NeedsWritable, IsResponse, HasData), InvalidCmd, "SwapResp" }, /* IntReq -- for interrupts */ { SET4(IsWrite, IsRequest, NeedsResponse, HasData), @@ -185,12 +185,12 @@ /* PrintReq */ { SET2(IsRequest, IsPrint), InvalidCmd, "PrintReq" }, /* Flush Request */ - { SET3(IsRequest, IsFlush, NeedsExclusive), InvalidCmd, "FlushReq" }, + { SET3(IsRequest, IsFlush, NeedsWritable), InvalidCmd, "FlushReq" }, /* Invalidation Request */ - { SET4(IsInvalidate, IsRequest, NeedsExclusive, NeedsResponse), + { SET4(IsInvalidate, IsRequest, NeedsWritable, NeedsResponse), InvalidateResp, "InvalidateReq" }, /* Invalidation Response */ - { SET3(IsInvalidate, IsResponse, NeedsExclusive), + { SET3(IsInvalidate, IsResponse, NeedsWritable), InvalidCmd, "InvalidateResp" } }; diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/ruby/system/DMASequencer.cc --- a/src/mem/ruby/system/DMASequencer.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/ruby/system/DMASequencer.cc Tue Dec 22 16:23:08 2015 +0000 @@ -89,8 +89,9 @@ pkt->getAddr(), id); DMASequencer *seq = static_cast(&owner); - if (pkt->memInhibitAsserted()) - panic("DMASequencer should never see an inhibited request\n"); + if (pkt->cacheResponding()) + panic("DMASequencer should never see a request with the " + "cacheResponding flag set\n"); assert(isPhysMemAddress(pkt->getAddr())); assert(getOffset(pkt->getAddr()) + pkt->getSize() <= diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/ruby/system/RubyPort.cc --- a/src/mem/ruby/system/RubyPort.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/ruby/system/RubyPort.cc Tue Dec 22 16:23:08 2015 +0000 @@ -231,8 +231,9 @@ pkt->getAddr(), id); RubyPort *ruby_port = static_cast(&owner); - if (pkt->memInhibitAsserted()) - panic("RubyPort should never see an inhibited request\n"); + if (pkt->cacheResponding()) + panic("RubyPort should never see request with the " + "cacheResponding flag set\n"); // Check for pio requests and directly send them to the dedicated // pio port. diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/serial_link.cc --- a/src/mem/serial_link.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/serial_link.cc Tue Dec 22 16:23:08 2015 +0000 @@ -185,7 +185,7 @@ } else if ( !retryReq ) { // look at the response queue if we expect to see a response bool expects_response = pkt->needsResponse() && - !pkt->memInhibitAsserted(); + !pkt->cacheResponding(); if (expects_response) { if (respQueueFull()) { DPRINTF(SerialLink, "Response queue full\n"); diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/simple_mem.cc --- a/src/mem/simple_mem.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/simple_mem.cc Tue Dec 22 16:23:08 2015 +0000 @@ -73,7 +73,7 @@ SimpleMemory::recvAtomic(PacketPtr pkt) { access(pkt); - return pkt->memInhibitAsserted() ? 0 : getLatency(); + return pkt->cacheResponding() ? 0 : getLatency(); } void @@ -97,8 +97,8 @@ bool SimpleMemory::recvTimingReq(PacketPtr pkt) { - // sink inhibited packets without further action - if (pkt->memInhibitAsserted()) { + // if a cache is responding, sink the packet without further action + if (pkt->cacheResponding()) { pendingDelete.reset(pkt); return true; } diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/packet.hh --- a/src/mem/packet.hh Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/packet.hh Tue Dec 22 16:23:08 2015 +0000 @@ -139,7 +139,7 @@ IsWrite, //!< Data flows from requester to responder IsUpgrade, IsInvalidate, - NeedsExclusive, //!< Requires exclusive copy to complete in-cache + NeedsWritable, //!< Requires writable copy to complete in-cache IsRequest, //!< Issued by requester IsResponse, //!< Issue by responder NeedsResponse, //!< Requester needs response from target @@ -189,7 +189,7 @@ bool isUpgrade() const { return testCmdAttrib(IsUpgrade); } bool isRequest() const { return testCmdAttrib(IsRequest); } bool isResponse() const { return testCmdAttrib(IsResponse); } - bool needsExclusive() const { return testCmdAttrib(NeedsExclusive); } + bool needsWritable() const { return testCmdAttrib(NeedsWritable); } bool needsResponse() const { return testCmdAttrib(NeedsResponse); } bool isInvalidate() const { return testCmdAttrib(IsInvalidate); } bool isEviction() const { return testCmdAttrib(IsEviction); } @@ -252,15 +252,33 @@ // Flags to transfer across when copying a packet COPY_FLAGS = 0x0000000F, - SHARED = 0x00000001, + // Does this packet pass the writable flag between caches or + // not. The flag is negated such that a packet by default + // passes writable, and allocates a block either as Modified + // (if a cache is responding) or Exclusive (when the response + // comes from a memory). See further details in the + // explanation of the snoop flag accessor methods. + PASS_NON_WRITABLE = 0x00000001, + // Special control flags /// Special timing-mode atomic snoop for multi-level coherence. EXPRESS_SNOOP = 0x00000002, - /// Does supplier have exclusive copy? - /// Useful for multi-level coherence. - SUPPLY_EXCLUSIVE = 0x00000004, - // Snoop response flags - MEM_INHIBIT = 0x00000008, + + /// Allow a responding cache to inform the cache hierarchy + /// that it had a writable copy before responding. If a cache + /// has the block in Owned state (dirty, but not writable), it + /// will respond without setting this flag, but still pass the + /// block as writable. The memory system reacts to this by + /// sending out invalidations to all other branches in the + /// cache hierarchy, invalidating their copies. + RESPONDER_HAD_WRITABLE = 0x00000004, + + // Snoop co-ordination flag to indicate that a cache is + // responding to a snoop. The caches ever only respond when + // they have a dirty line (Modified or Owned), and so + // implicitly this flag also passes dirty. This flag is also + // used by the memories to supress a response. + CACHE_RESPONDING = 0x00000008, /// Are the 'addr' and 'size' fields valid? VALID_ADDR = 0x00000100, @@ -495,7 +513,7 @@ bool isUpgrade() const { return cmd.isUpgrade(); } bool isRequest() const { return cmd.isRequest(); } bool isResponse() const { return cmd.isResponse(); } - bool needsExclusive() const { return cmd.needsExclusive(); } + bool needsWritable() const { return cmd.needsWritable(); } bool needsResponse() const { return cmd.needsResponse(); } bool isInvalidate() const { return cmd.isInvalidate(); } bool isEviction() const { return cmd.isEviction(); } @@ -506,22 +524,94 @@ bool isPrint() const { return cmd.isPrint(); } bool isFlush() const { return cmd.isFlush(); } - // Snoop flags - void assertMemInhibit() + //@{ + /// Snoop flags + /** + * Set the cacheResponding flag. This is used by the caches to + * signal another cache that they are responding to a request, + * always doing so with a dirty line, thus having the line in + * either Modified or Owned state. Note that on snoop hits we + * always pass the line as Modified (and never Owned), as in the + * case of an Owned line we proceed to invalidate all other + * copies. On a cache fill, we check passWritable first, ignoring + * the cacheResponding flag if passWritable is not set, and + * depending on these two flags, a line is consequently allocated + * as: + * + * passWritable cacheResponding state + * false false Shared + * false true Shared + * true false Exclusive + * true true Modified + */ + void setCacheResponding() { assert(isRequest()); - assert(!flags.isSet(MEM_INHIBIT)); - flags.set(MEM_INHIBIT); + assert(!flags.isSet(CACHE_RESPONDING)); + flags.set(CACHE_RESPONDING); } - bool memInhibitAsserted() const { return flags.isSet(MEM_INHIBIT); } - void assertShared() { flags.set(SHARED); } - bool sharedAsserted() const { return flags.isSet(SHARED); } + bool cacheResponding() const { return flags.isSet(CACHE_RESPONDING); } + /** + * The passWritable flag is used by the caches in combination with + * the cacheResponding flag, as clarified above. If the + * passWritable flag is set, the packet is passing writable. By + * negating the flag, a response from a memory passes the line as + * writable by default. + * + * The passWritable flag is also used by upstream caches to inform + * a downstream cache that they have the block (by calling + * setPassNonWritable on snoop request packets that hit in + * upstream cachs tags or MSHRs). If the snoop packet is not + * passing writable, a downstream cache is prevented from passing + * a dirty line upwards if it was not explicitly asked for a + * writable copy. See Cache::satisfyCpuSideRequest. + * + * The passWritable flag is also used on writebacks, in + * combination with the WritbackClean or WritebackDirty commands, + * to pass the block downstream either as: + * + * command passWritable state + * WritebackDirty true Modified + * WritebackDirty false Owned + * WritebackClean true Exclusive + * WritebackClean false Shared + */ + void setPassNonWritable() { flags.set(PASS_NON_WRITABLE); } + bool passWritable() const { return !flags.isSet(PASS_NON_WRITABLE); } + //@} - // Special control flags + /** + * The express snoop flag is used for two purposes. Firtly, it is + * used to bypass flow control for normal (non-snoop) requests + * going downstream in the memory system. In cases where a cache + * is responding to a snoop from another cache (it had a dirty + * line), but the line as not writable (and there are possibly + * other copies), the express snoop flag is set by the downstream + * cache to invalidate all other copies in zero time. Secondly, + * the express snoop flag is also set to be able to distinguish + * snoop packets that came from a downstream cache, rather than + * snoop packets from neighbouring caches. + */ void setExpressSnoop() { flags.set(EXPRESS_SNOOP); } bool isExpressSnoop() const { return flags.isSet(EXPRESS_SNOOP); } - void setSupplyExclusive() { flags.set(SUPPLY_EXCLUSIVE); } - bool isSupplyExclusive() const { return flags.isSet(SUPPLY_EXCLUSIVE); } + + /** + * On providing a snoop response (which only happens for Modified + * or Owned lines), make sure that we can transform an Owned + * response to a Modified one. If this flag is not set, the + * responding cache had the line in the Owned state, and there are + * possibly other Shared copies in the memory system. A downstream + * cache helps in orchestrating the invalidation of these copies + * by sending out the appropriate express snoops. + */ + void setResponderHadWritable() + { + assert(cacheResponding()); + flags.set(RESPONDER_HAD_WRITABLE); + } + bool responderHadWritable() const + { return flags.isSet(RESPONDER_HAD_WRITABLE); } + void setSuppressFuncError() { flags.set(SUPPRESS_FUNC_ERROR); } bool suppressFuncError() const { return flags.isSet(SUPPRESS_FUNC_ERROR); } void setBlockCached() { flags.set(BLOCK_CACHED); } diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/dram_ctrl.cc --- a/src/mem/dram_ctrl.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/dram_ctrl.cc Tue Dec 22 16:23:08 2015 +0000 @@ -277,7 +277,7 @@ access(pkt); Tick latency = 0; - if (!pkt->memInhibitAsserted() && pkt->hasData()) { + if (!pkt->cacheResponding() && pkt->hasData()) { // this value is not supposed to be accurate, just enough to // keep things going, mimic a closed page latency = tRP + tRCD + tCL; @@ -590,8 +590,8 @@ DPRINTF(DRAM, "recvTimingReq: request %s addr %lld size %d\n", pkt->cmdString(), pkt->getAddr(), pkt->getSize()); - // sink inhibited packets without further action - if (pkt->memInhibitAsserted()) { + // if a cache is responding, sink the packet without further action + if (pkt->cacheResponding()) { pendingDelete.reset(pkt); return true; } diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/dramsim2.cc --- a/src/mem/dramsim2.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/dramsim2.cc Tue Dec 22 16:23:08 2015 +0000 @@ -155,7 +155,7 @@ access(pkt); // 50 ns is just an arbitrary value at this point - return pkt->memInhibitAsserted() ? 0 : 50000; + return pkt->cacheResponding() ? 0 : 50000; } void @@ -175,8 +175,8 @@ bool DRAMSim2::recvTimingReq(PacketPtr pkt) { - // sink inhibited packets without further action - if (pkt->memInhibitAsserted()) { + // if a cache is responding, sink the packet without further action + if (pkt->cacheResponding()) { pendingDelete.reset(pkt); return true; } diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/hmc_controller.cc --- a/src/mem/hmc_controller.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/hmc_controller.cc Tue Dec 22 16:23:08 2015 +0000 @@ -81,15 +81,12 @@ // before forwarding the packet (and possibly altering it), // remember if we are expecting a response const bool expect_response = pkt->needsResponse() && - !pkt->memInhibitAsserted(); + !pkt->cacheResponding(); // since it is a normal request, attempt to send the packet bool success = masterPorts[master_port_id]->sendTimingReq(pkt); if (!success) { - // inhibited packets should never be forced to retry - assert(!pkt->memInhibitAsserted()); - DPRINTF(HMCController, "recvTimingReq: src %s %s 0x%x RETRY\n", src_port->name(), pkt->cmdString(), pkt->getAddr()); diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/mem_checker_monitor.cc --- a/src/mem/mem_checker_monitor.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/mem_checker_monitor.cc Tue Dec 22 16:23:08 2015 +0000 @@ -156,7 +156,7 @@ bool is_write = pkt->isWrite(); unsigned size = pkt->getSize(); Addr addr = pkt->getAddr(); - bool expects_response = pkt->needsResponse() && !pkt->memInhibitAsserted(); + bool expects_response = pkt->needsResponse() && !pkt->cacheResponding(); std::unique_ptr pkt_data; MemCheckerMonitorSenderState* state = NULL; @@ -170,15 +170,14 @@ // If a cache miss is served by a cache, a monitor near the memory // would see a request which needs a response, but this response - // would be inhibited and not come back from the memory. Therefore + // would not come back from the memory. Therefore // we additionally have to check the inhibit flag. if (expects_response && (is_read || is_write)) { state = new MemCheckerMonitorSenderState(0); pkt->pushSenderState(state); } - // Attempt to send the packet (always succeeds for inhibited - // packets) + // Attempt to send the packet bool successful = masterPort.sendTimingReq(pkt); // If not successful, restore the sender state @@ -227,7 +226,7 @@ } } else if (successful) { DPRINTF(MemCheckerMonitor, - "Forwarded inhibited request: addr = %#llx\n", addr); + "Forwarded request without response: addr = %#llx\n", addr); } return successful; diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/noncoherent_xbar.cc --- a/src/mem/noncoherent_xbar.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/noncoherent_xbar.cc Tue Dec 22 16:23:08 2015 +0000 @@ -142,15 +142,12 @@ // before forwarding the packet (and possibly altering it), // remember if we are expecting a response const bool expect_response = pkt->needsResponse() && - !pkt->memInhibitAsserted(); + !pkt->cacheResponding(); // since it is a normal request, attempt to send the packet bool success = masterPorts[master_port_id]->sendTimingReq(pkt); if (!success) { - // inhibited packets should never be forced to retry - assert(!pkt->memInhibitAsserted()); - DPRINTF(NoncoherentXBar, "recvTimingReq: src %s %s 0x%x RETRY\n", src_port->name(), pkt->cmdString(), pkt->getAddr()); diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/cache/mshr.hh --- a/src/mem/cache/mshr.hh Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/cache/mshr.hh Tue Dec 22 16:23:08 2015 +0000 @@ -80,8 +80,8 @@ /** Flag set by downstream caches */ bool downstreamPending; - /** Will we have a dirty copy after this request? */ - bool pendingDirty; + /** Will we have a writable copy after this request? */ + bool pendingWritable; /** Did we snoop an invalidate while waiting for data? */ bool postInvalidate; @@ -118,12 +118,12 @@ class TargetList : public std::list { public: - bool needsExclusive; + bool needsWritable; bool hasUpgrade; TargetList(); - void resetFlags() { needsExclusive = hasUpgrade = false; } - bool isReset() const { return !needsExclusive && !hasUpgrade; } + void resetFlags() { needsWritable = hasUpgrade = false; } + bool isReset() const { return !needsWritable && !hasUpgrade; } void add(PacketPtr pkt, Tick readyTime, Counter order, Target::Source source, bool markPending); void replaceUpgrades(); @@ -169,11 +169,11 @@ * flags are accessed improperly. */ - /** True if we need to get an exclusive copy of the block. */ - bool needsExclusive() const { return targets.needsExclusive; } + /** True if we need to get a writable copy of the block. */ + bool needsWritable() const { return targets.needsWritable; } - bool isPendingDirty() const { - assert(inService); return pendingDirty; + bool isPendingWritable() const { + assert(inService); return pendingWritable; } bool hasPostInvalidate() const { @@ -223,7 +223,7 @@ void allocate(Addr blk_addr, unsigned blk_size, PacketPtr pkt, Tick when_ready, Counter _order, bool alloc_on_fill); - bool markInService(bool pending_dirty_resp); + bool markInService(bool pending_modified_resp); void clearDownstreamPending(); @@ -284,7 +284,7 @@ bool promoteDeferredTargets(); - void promoteExclusive(); + void promoteWritable(); bool checkFunctional(PacketPtr pkt); diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/cache/mshr.cc --- a/src/mem/cache/mshr.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/cache/mshr.cc Tue Dec 22 16:23:08 2015 +0000 @@ -62,7 +62,7 @@ using namespace std; MSHR::MSHR() : readyTime(0), _isUncacheable(false), downstreamPending(false), - pendingDirty(false), + pendingWritable(false), postInvalidate(false), postDowngrade(false), queue(NULL), order(0), blkAddr(0), blkSize(0), isSecure(false), inService(false), @@ -73,7 +73,7 @@ MSHR::TargetList::TargetList() - : needsExclusive(false), hasUpgrade(false) + : needsWritable(false), hasUpgrade(false) {} @@ -82,8 +82,8 @@ Counter order, Target::Source source, bool markPending) { if (source != Target::FromSnoop) { - if (pkt->needsExclusive()) { - needsExclusive = true; + if (pkt->needsWritable()) { + needsWritable = true; } // StoreCondReq is effectively an upgrade if it's in an MSHR @@ -238,7 +238,7 @@ } bool -MSHR::markInService(bool pending_dirty_resp) +MSHR::markInService(bool pending_modified_resp) { assert(!inService); if (isForwardNoResponse()) { @@ -250,7 +250,7 @@ } inService = true; - pendingDirty = targets.needsExclusive || pending_dirty_resp; + pendingWritable = targets.needsWritable || pending_modified_resp; postInvalidate = postDowngrade = false; if (!downstreamPending) { @@ -297,14 +297,14 @@ // - there are other targets already deferred // - there's a pending invalidate to be applied after the response // comes back (but before this target is processed) - // - this target requires an exclusive block and either we're not - // getting an exclusive block back or we have already snooped - // another read request that will downgrade our exclusive block - // to shared + // - this target requires a writable block and either we're not + // getting a writable block back or we have already snooped + // another read request that will downgrade our writable block + // to non-writable (Shared or Owned) if (inService && (!deferredTargets.empty() || hasPostInvalidate() || - (pkt->needsExclusive() && - (!isPendingDirty() || hasPostDowngrade() || isForward)))) { + (pkt->needsWritable() && + (!isPendingWritable() || hasPostDowngrade() || isForward)))) { // need to put on deferred list if (hasPostInvalidate()) replaceUpgrade(pkt); @@ -324,12 +324,12 @@ DPRINTF(Cache, "%s for %s addr %#llx size %d\n", __func__, pkt->cmdString(), pkt->getAddr(), pkt->getSize()); - // when we snoop packets that need exclusive, they should + // when we snoop packets that need writable, they should // effectively act as invalidations, however, this assumes that we // never snoop writes as they are currently not marked as // invalidations - panic_if(pkt->needsExclusive() != pkt->isInvalidate(), - "%s got snoop %s to addr %#llx that needs exclusive, " + panic_if(pkt->needsWritable() != pkt->isInvalidate(), + "%s got snoop %s to addr %#llx that needs writable, " "but is not invalidate", name(), pkt->cmdString(), pkt->getAddr()); @@ -347,7 +347,7 @@ // That is, even though the upper-level cache got out on its // local bus first, some other invalidating transaction // reached the global bus before the upgrade did. - if (pkt->needsExclusive()) { + if (pkt->needsWritable()) { targets.replaceUpgrades(); deferredTargets.replaceUpgrades(); } @@ -357,7 +357,7 @@ // From here on down, the request issued by this MSHR logically // precedes the request we're snooping. - if (pkt->needsExclusive()) { + if (pkt->needsWritable()) { // snooped request still precedes the re-request we'll have to // issue for deferred targets, if any... deferredTargets.replaceUpgrades(); @@ -370,17 +370,18 @@ return true; } - if (isPendingDirty() || pkt->isInvalidate()) { + if (isPendingWritable() || pkt->isInvalidate()) { // We need to save and replay the packet in two cases: - // 1. We're awaiting an exclusive copy, so ownership is pending, - // and we need to deal with the snoop after we receive data. + // 1. We're awaiting a writable copy (Modified or Exclusive), + // so this MSHR is the orgering point, and we need to respond + // after we receive data. // 2. It's an invalidation (e.g., UpgradeReq), and we need // to forward the snoop up the hierarchy after the current // transaction completes. // Start by determining if we will eventually respond or not, // matching the conditions checked in Cache::handleSnoop - bool will_respond = isPendingDirty() && pkt->needsResponse() && + bool will_respond = isPendingWritable() && pkt->needsResponse() && pkt->cmd != MemCmd::InvalidateReq; // The packet we are snooping may be deleted by the time we @@ -396,29 +397,41 @@ PacketPtr cp_pkt = will_respond ? new Packet(pkt, true, true) : new Packet(new Request(*pkt->req), pkt->cmd); - if (isPendingDirty()) { - // The new packet will need to get the response from the - // MSHR already queued up here - pkt->assertMemInhibit(); + if (isPendingWritable()) { + // the packet will get a dirty line (actually this is a + // bit dubious, as we may get the line in the Exclusive + // state, but we have no way of tracking this at the + // moment), and by not setting the shared flag we are + // transferring the line as writable, thus in the Modified + // state + pkt->setCacheResponding(); + + // inform the cache hierarchy that this cache had the line + // in the Modified state, even if the response is passed + // as Shared (and thus non-writable) + pkt->setResponderHadWritable(); + // in the case of an uncacheable request there is no need - // to set the exclusive flag, but since the recipient does - // not care there is no harm in doing so - pkt->setSupplyExclusive(); + // to set the responderHadWritable flag, but since the + // recipient does not care there is no harm in doing so } targets.add(cp_pkt, curTick(), _order, Target::FromSnoop, - downstreamPending && targets.needsExclusive); + downstreamPending && targets.needsWritable); - if (pkt->needsExclusive()) { + if (pkt->needsWritable()) { // This transaction will take away our pending copy postInvalidate = true; } } - if (!pkt->needsExclusive() && !pkt->req->isUncacheable()) { + if (!pkt->needsWritable() && !pkt->req->isUncacheable()) { // This transaction will get a read-shared copy, downgrading - // our copy if we had an exclusive one + // our copy if we had a writable one postDowngrade = true; - pkt->assertShared(); + // make sure that any downstream cache does not respond with a + // writable (and dirty) copy even if it has one, unless it was + // explicitly asked for one + pkt->setPassNonWritable(); } return true; @@ -447,20 +460,19 @@ void -MSHR::promoteExclusive() +MSHR::promoteWritable() { - if (deferredTargets.needsExclusive && + if (deferredTargets.needsWritable && !(hasPostInvalidate() || hasPostDowngrade())) { - // We got an exclusive response, but we have deferred targets - // which are waiting to request an exclusive copy (not because + // We got a writable response, but we have deferred targets + // which are waiting to request a writable copy (not because // of a pending invalidate). This can happen if the original - // request was for a read-only (non-exclusive) block, but we - // got an exclusive copy anyway because of the E part of the - // MOESI/MESI protocol. Since we got the exclusive copy - // there's no need to defer the targets, so move them up to - // the regular target list. - assert(!targets.needsExclusive); - targets.needsExclusive = true; + // request was for a read-only block, but we got an writable + // response anyway. Since we got the writable copy there's no + // need to defer the targets, so move them up to the regular + // target list. + assert(!targets.needsWritable); + targets.needsWritable = true; // if any of the deferred targets were upper-level cache // requests marked downstreamPending, need to clear that assert(!downstreamPending); // not pending here anymore @@ -497,7 +509,7 @@ isForward ? "Forward" : "", allocOnFill ? "AllocOnFill" : "", isForwardNoResponse() ? "ForwNoResp" : "", - needsExclusive() ? "Excl" : "", + needsWritable() ? "Wrtbl" : "", _isUncacheable ? "Unc" : "", inService ? "InSvc" : "", downstreamPending ? "DwnPend" : "", diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/cache/mshr_queue.hh --- a/src/mem/cache/mshr_queue.hh Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/cache/mshr_queue.hh Tue Dec 22 16:23:08 2015 +0000 @@ -188,10 +188,10 @@ * readyList or deallocates the MSHR if it does not expect a response. * * @param mshr The MSHR to mark in service. - * @param pending_dirty_resp Whether we expect a dirty response - * from another cache + * @param pending_modified_resp Whether we expect a modified response + * from another cache */ - void markInService(MSHR *mshr, bool pending_dirty_resp); + void markInService(MSHR *mshr, bool pending_modified_resp); /** * Mark an in service entry as pending, used to resend a request. diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/cache/mshr_queue.cc --- a/src/mem/cache/mshr_queue.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/cache/mshr_queue.cc Tue Dec 22 16:23:08 2015 +0000 @@ -200,9 +200,9 @@ } void -MSHRQueue::markInService(MSHR *mshr, bool pending_dirty_resp) +MSHRQueue::markInService(MSHR *mshr, bool pending_modified_resp) { - if (mshr->markInService(pending_dirty_resp)) { + if (mshr->markInService(pending_modified_resp)) { deallocate(mshr); } else { readyList.erase(mshr->readyIter); diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/coherent_xbar.cc --- a/src/mem/coherent_xbar.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/coherent_xbar.cc Tue Dec 22 16:23:08 2015 +0000 @@ -145,10 +145,10 @@ // remember if the packet is an express snoop bool is_express_snoop = pkt->isExpressSnoop(); - bool is_inhibited = pkt->memInhibitAsserted(); + bool cache_responding = pkt->cacheResponding(); // for normal requests, going downstream, the express snoop flag - // and the inhibited flag should always be the same - assert(is_express_snoop == is_inhibited); + // and the cache responding flag should always be the same + assert(is_express_snoop == cache_responding); // determine the destination based on the address PortID master_port_id = findPort(pkt->getAddr()); @@ -236,10 +236,12 @@ return true; } - // remember if the packet will generate a snoop response - const bool expect_snoop_resp = !is_inhibited && pkt->memInhibitAsserted(); + // remember if the packet will generate a snoop response by + // checking if a cache set the cacheResponding flag during the + // snooping above + const bool expect_snoop_resp = !cache_responding && pkt->cacheResponding(); const bool expect_response = pkt->needsResponse() && - !pkt->memInhibitAsserted(); + !pkt->cacheResponding(); // since it is a normal request, attempt to send the packet bool success = masterPorts[master_port_id]->sendTimingReq(pkt); @@ -251,10 +253,8 @@ // check if we were successful in sending the packet onwards if (!success) { - // express snoops and inhibited packets should never be forced - // to retry + // express snoops should never be forced to retry assert(!is_express_snoop); - assert(!pkt->memInhibitAsserted()); // restore the header delay pkt->headerDelay = old_header_delay; @@ -386,8 +386,9 @@ // @todo Assess the choice of latency further calcPacketTiming(pkt, forwardLatency * clockPeriod()); - // remeber if the packet is inhibited so we can see if it changes - const bool is_inhibited = pkt->memInhibitAsserted(); + // remeber if a cache has already committed to responding so we + // can see if it changes during the snooping + const bool cache_responding = pkt->cacheResponding(); assert(pkt->snoopDelay == 0); @@ -414,7 +415,7 @@ pkt->snoopDelay = 0; // if we can expect a response, remember how to route it - if (!is_inhibited && pkt->memInhibitAsserted()) { + if (!cache_responding && pkt->cacheResponding()) { assert(routeTo.find(pkt->req) == routeTo.end()); routeTo[pkt->req] = master_port_id; } @@ -760,7 +761,7 @@ // response from snoop agent assert(pkt->cmd != orig_cmd); - assert(pkt->memInhibitAsserted()); + assert(pkt->cacheResponding()); // should only happen once assert(snoop_response_cmd == MemCmd::InvalidCmd); // save response state diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/comm_monitor.cc --- a/src/mem/comm_monitor.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/comm_monitor.cc Tue Dec 22 16:23:08 2015 +0000 @@ -144,18 +144,17 @@ const bool is_read = pkt->isRead(); const bool is_write = pkt->isWrite(); const bool expects_response( - pkt->needsResponse() && !pkt->memInhibitAsserted()); + pkt->needsResponse() && !pkt->cacheResponding()); // If a cache miss is served by a cache, a monitor near the memory // would see a request which needs a response, but this response - // would be inhibited and not come back from the memory. Therefore - // we additionally have to check the inhibit flag. + // would not come back from the memory. Therefore we additionally + // have to check the cacheResponding flag if (expects_response && !stats.disableLatencyHists) { pkt->pushSenderState(new CommMonitorSenderState(curTick())); } - // Attempt to send the packet (always succeeds for inhibited - // packets) + // Attempt to send the packet bool successful = masterPort.sendTimingReq(pkt); // If not successful, restore the sender state diff -r 07706eb5faf4 -r e4679f18edb9 src/cpu/o3/cpu.cc --- a/src/cpu/o3/cpu.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/cpu/o3/cpu.cc Tue Dec 22 16:23:08 2015 +0000 @@ -92,9 +92,9 @@ FullO3CPU::IcachePort::recvTimingResp(PacketPtr pkt) { DPRINTF(O3CPU, "Fetch unit received timing\n"); - // We shouldn't ever get a cacheable block in ownership state + // We shouldn't ever get a cacheable block in Modified state assert(pkt->req->isUncacheable() || - !(pkt->memInhibitAsserted() && !pkt->sharedAsserted())); + !(pkt->cacheResponding() && pkt->passWritable())); fetch->processCacheCompletion(pkt); return true; diff -r 07706eb5faf4 -r e4679f18edb9 src/dev/dma_device.cc --- a/src/dev/dma_device.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/dev/dma_device.cc Tue Dec 22 16:23:08 2015 +0000 @@ -105,9 +105,9 @@ bool DmaPort::recvTimingResp(PacketPtr pkt) { - // We shouldn't ever get a cacheable block in ownership state + // We shouldn't ever get a cacheable block in Modified state assert(pkt->req->isUncacheable() || - !(pkt->memInhibitAsserted() && !pkt->sharedAsserted())); + !(pkt->cacheResponding() && pkt->passWritable())); handleResp(pkt); diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/abstract_mem.cc --- a/src/mem/abstract_mem.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/abstract_mem.cc Tue Dec 22 16:23:08 2015 +0000 @@ -323,8 +323,8 @@ void AbstractMemory::access(PacketPtr pkt) { - if (pkt->memInhibitAsserted()) { - DPRINTF(MemoryAccess, "mem inhibited on 0x%x: not responding\n", + if (pkt->cacheResponding()) { + DPRINTF(MemoryAccess, "Cache responding to %#llx: not responding\n", pkt->getAddr()); return; } diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/cache/cache.cc --- a/src/mem/cache/cache.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/cache/cache.cc Tue Dec 22 16:23:08 2015 +0000 @@ -157,7 +157,7 @@ // can satisfy a following ReadEx anyway since we can rely on the // Read requester(s) to have buffered the ReadEx snoop and to // invalidate their blocks after receiving them. - // assert(!pkt->needsExclusive() || blk->isWritable()); + // assert(!pkt->needsWritable() || blk->isWritable()); assert(pkt->getOffset(blkSize) + pkt->getSize() <= blkSize); // Check RMW operations first since both isRead() and @@ -165,15 +165,19 @@ if (pkt->cmd == MemCmd::SwapReq) { cmpAndSwap(blk, pkt); } else if (pkt->isWrite()) { + // we have the block in a writable state and can go ahead, + // note that the line may be also be considered writable in + // downstream caches along the path to memory, but always + // Exclusive, and never Modified assert(blk->isWritable()); - // Write or WriteLine at the first cache with block in Exclusive + // Write or WriteLine at the first cache with block in writable state if (blk->checkWrite(pkt)) { pkt->writeDataToBlock(blk->data, blkSize); } - // Always mark the line as dirty even if we are a failed - // StoreCond so we supply data to any snoops that have - // appended themselves to this cache before knowing the store - // will fail. + // Always mark the line as dirty (and thus transition to the + // Modified state) even if we are a failed StoreCond so we + // supply data to any snoops that have appended themselves to + // this cache before knowing the store will fail. blk->status |= BlkDirty; DPRINTF(Cache, "%s for %s addr %#llx size %d (write)\n", __func__, pkt->cmdString(), pkt->getAddr(), pkt->getSize()); @@ -193,79 +197,80 @@ assert(pkt->getSize() == blkSize); // special handling for coherent block requests from // upper-level caches - if (pkt->needsExclusive()) { + if (pkt->needsWritable()) { // sanity check assert(pkt->cmd == MemCmd::ReadExReq || pkt->cmd == MemCmd::SCUpgradeFailReq); // if we have a dirty copy, make sure the recipient - // keeps it marked dirty + // keeps it marked dirty (in the modified state) if (blk->isDirty()) { - pkt->assertMemInhibit(); + pkt->setCacheResponding(); } // on ReadExReq we give up our copy unconditionally, // even if this cache is mostly inclusive, we may want // to revisit this invalidateBlock(blk); } else if (blk->isWritable() && !pending_downgrade && - !pkt->sharedAsserted() && + pkt->passWritable() && pkt->cmd != MemCmd::ReadCleanReq) { - // we can give the requester an exclusive copy (by not - // asserting shared line) on a read request if: - // - we have an exclusive copy at this level (& below) + // we can give the requester a writable copy (by not + // setting pass non-writable) on a read request if: + // - we have a writable copy at this level (& below) // - we don't have a pending snoop from below // signaling another read request // - no other cache above has a copy (otherwise it - // would have asseretd shared line on request) - // - we are not satisfying an instruction fetch (this - // prevents dirty data in the i-cache) - + // would have set the non-writable flag when + // snooping the packet) + // - the read has explicitly asked for a clean + // copy of the line if (blk->isDirty()) { // special considerations if we're owner: if (!deferred_response) { - // if we are responding immediately and can - // signal that we're transferring ownership - // (inhibit set) along with exclusivity - // (shared not set), do so - pkt->assertMemInhibit(); + // respond with the line in Modified state + // (cacheResponding set, passWritable set) + pkt->setCacheResponding(); - // if this cache is mostly inclusive, we keep - // the block as writable (exclusive), and pass - // it upwards as writable and dirty - // (modified), hence we have multiple caches - // considering the same block writable, - // something that we get away with due to the - // fact that: 1) this cache has been - // considered the ordering points and - // responded to all snoops up till now, and 2) - // we always snoop upwards before consulting - // the local cache, both on a normal request - // (snooping done by the crossbar), and on a - // snoop - blk->status &= ~BlkDirty; + if (clusivity == Enums::mostly_excl) { + // if this cache is mostly exclusive with + // respect to the cache above, drop the + // block, no need to first unset the dirty + // bit + invalidateBlock(blk); + } else { + // if this cache is mostly inclusive, we + // keep the block in the Exclusive state, + // and pass it upwards as Modified + // (writable and dirty), hence we have + // multiple caches, all on the same path + // towards memory, all considering the + // same block writable, but only one + // considering it Modified - // if this cache is mostly exclusive with - // respect to the cache above, drop the block - if (clusivity == Enums::mostly_excl) { - invalidateBlock(blk); + // we get away with multiple caches (on + // the same path to memory) considering + // the block writeable as we always enter + // the cache hierarchy through a cache, + // and first snoop upwards in all other + // branches + blk->status &= ~BlkDirty; } } else { // if we're responding after our own miss, // there's a window where the recipient didn't // know it was getting ownership and may not // have responded to snoops correctly, so we - // can't pass off ownership *or* exclusivity - pkt->assertShared(); + // have to respond with a shared line + pkt->setPassNonWritable(); } } } else { // otherwise only respond with a shared copy - pkt->assertShared(); + pkt->setPassNonWritable(); } } } else { - // Upgrade or Invalidate, since we have it Exclusively (E or - // M), we ack then invalidate. + // Upgrade or Invalidate assert(pkt->isUpgrade() || pkt->isInvalidate()); // for invalidations we could be looking at the temp block @@ -285,9 +290,9 @@ void -Cache::markInService(MSHR *mshr, bool pending_dirty_resp) +Cache::markInService(MSHR *mshr, bool pending_modified_resp) { - markInServiceInternal(mshr, pending_dirty_resp); + markInServiceInternal(mshr, pending_modified_resp); } ///////////////////////////////////////////////////// @@ -420,9 +425,10 @@ if (pkt->cmd == MemCmd::WritebackDirty) { blk->status |= BlkDirty; } - // if shared is not asserted we got the writeback in modified - // state, if it is asserted we are in the owned state - if (!pkt->sharedAsserted()) { + // if the packet is passing writable we got the writeback in + // Modified or Exclusive state, if not we are in the Owned or + // Shared state + if (pkt->passWritable()) { blk->status |= BlkWritable; } // nothing else to do; writeback doesn't expect response @@ -445,8 +451,7 @@ // go to next level. return false; } else if ((blk != NULL) && - (pkt->needsExclusive() ? blk->isWritable() - : blk->isReadable())) { + (pkt->needsWritable() ? blk->isWritable() : blk->isReadable())) { // OK to satisfy access incHitCount(pkt); satisfyCpuSideRequest(pkt, blk); @@ -454,7 +459,7 @@ } // Can't satisfy access normally... either no block (blk == NULL) - // or have block but need exclusive & only have shared. + // or have block but need writable incMissCount(pkt); @@ -607,18 +612,32 @@ promoteWholeLineWrites(pkt); - if (pkt->memInhibitAsserted()) { + if (pkt->cacheResponding()) { // a cache above us (but not where the packet came from) is - // responding to the request - DPRINTF(Cache, "mem inhibited on addr %#llx (%s): not responding\n", + // responding to the request, in other words it has the line + // in Modified or Owned state + DPRINTF(Cache, "Cache above responding to %#llx (%s): " + "not responding\n", pkt->getAddr(), pkt->isSecure() ? "s" : "ns"); - // if the packet needs exclusive, and the cache that has - // promised to respond (setting the inhibit flag) is not - // providing exclusive (it is in O vs M state), we know that - // there may be other shared copies in the system; go out and - // invalidate them all - if (pkt->needsExclusive() && !pkt->isSupplyExclusive()) { + // if the packet needs the block to be writable, and the cache + // that has promised to respond (setting the cache responding + // flag) is not providing writable (it is in Owned rather than + // the Modified state), we know that there may be other Shared + // copies in the system; go out and invalidate them all + if (pkt->needsWritable() && !pkt->responderHadWritable()) { + // an upstream cache that had the line in Owned state + // (dirty, but not writable), is responding and thus + // transferring the dirty line from one branch of the + // cache hierarchy to another + + // send out an express snoop and invalidate all other + // copies (snooping a packet that needs writable is the + // same as an invalidation), thus turning the Owned line + // into a Modified line, note that we don't invalidate the + // block in the current cache or any other cache on the + // path to memory + // create a downstream express snoop with cleared packet // flags, there is no need to allocate any data as the // packet is merely used to co-ordinate state transitions @@ -629,11 +648,12 @@ snoop_pkt->headerDelay = snoop_pkt->payloadDelay = 0; // make this an instantaneous express snoop, and let the - // other caches in the system know that the packet is - // inhibited, because we have found the authorative copy - // (O) that will supply the right data + // other caches in the system know that the another cache + // is responding, because we have found the authorative + // copy (Modified or Owned) that will supply the right + // data snoop_pkt->setExpressSnoop(); - snoop_pkt->assertMemInhibit(); + snoop_pkt->setCacheResponding(); // this express snoop travels towards the memory, and at // every crossbar it is snooped upwards thus reaching @@ -642,17 +662,20 @@ // express snoops always succeed assert(success); - // main memory will delete the packet + // main memory will delete the snoop packet } - // queue for deletion, as the sending cache is still relying - // on the packet + // queue for deletion, as opposed to immediate deletion, as + // the sending cache is still relying on the packet pendingDelete.reset(pkt); - // no need to take any action in this particular cache as the - // caches along the path to memory are allowed to keep lines - // in a shared state, and a cache above us already committed - // to responding + // no need to take any action in this particular cache as an + // upstram cache has already committed to responding, and + // either the packet does not need writable (and we can let + // the cache that set the cache responding flag pass on the + // line without any need for intervention), or if the packet + // needs writable it is provided, or we have already sent out + // any express snoops in the section above return true; } @@ -872,7 +895,7 @@ // internally, and have a sufficiently weak memory // model, this is probably unnecessary, but at some // point it must have seemed like we needed it... - assert(pkt->needsExclusive()); + assert(pkt->needsWritable()); assert(!blk->isWritable()); blk->status &= ~BlkReadable; } @@ -900,7 +923,7 @@ // See comment in cache.hh. PacketPtr Cache::getBusPacket(PacketPtr cpu_pkt, CacheBlk *blk, - bool needsExclusive) const + bool needsWritable) const { bool blkValid = blk && blk->isValid(); @@ -931,9 +954,9 @@ // which will clobber the owned copy. const bool useUpgrades = true; if (blkValid && useUpgrades) { - // only reason to be here is that blk is shared - // (read-only) and we need exclusive - assert(needsExclusive); + // only reason to be here is that blk is read only and we need + // it to be writable + assert(needsWritable); assert(!blk->isWritable()); cmd = cpu_pkt->isLLSC() ? MemCmd::SCUpgradeReq : MemCmd::UpgradeReq; } else if (cpu_pkt->cmd == MemCmd::SCUpgradeFailReq || @@ -945,24 +968,26 @@ cmd = MemCmd::SCUpgradeFailReq; } else if (cpu_pkt->cmd == MemCmd::WriteLineReq) { // forward as invalidate to all other caches, this gives us - // the line in exclusive state, and invalidates all other + // the line in Exclusive state, and invalidates all other // copies cmd = MemCmd::InvalidateReq; } else { // block is invalid - cmd = needsExclusive ? MemCmd::ReadExReq : + cmd = needsWritable ? MemCmd::ReadExReq : (isReadOnly ? MemCmd::ReadCleanReq : MemCmd::ReadSharedReq); } PacketPtr pkt = new Packet(cpu_pkt->req, cmd, blkSize); - // if there are sharers in the upper levels, pass that info downstream - if (cpu_pkt->sharedAsserted()) { + // if there are upstream caches that have already marked the + // packet as not passing writable, pass that info downstream + if (!cpu_pkt->passWritable()) { // note that cpu_pkt may have spent a considerable time in the // MSHR queue and that the information could possibly be out // of date, however, there is no harm in conservatively - // assuming the block is shared - pkt->assertShared(); - DPRINTF(Cache, "%s passing shared from %s to %s addr %#llx size %d\n", + // assuming the block is not allowed to pass writable + pkt->setPassNonWritable(); + DPRINTF(Cache, "%s passing non-writable from %s to %s addr %#llx " + "size %d\n", __func__, cpu_pkt->cmdString(), pkt->cmdString(), pkt->getAddr(), pkt->getSize()); } @@ -992,7 +1017,7 @@ promoteWholeLineWrites(pkt); - if (pkt->memInhibitAsserted()) { + if (pkt->cacheResponding()) { // have to invalidate ourselves and any lower caches even if // upper cache will be responding if (pkt->isInvalidate()) { @@ -1000,19 +1025,21 @@ if (blk && blk->isValid()) { tags->invalidate(blk); blk->invalidate(); - DPRINTF(Cache, "rcvd mem-inhibited %s on %#llx (%s):" + DPRINTF(Cache, "Other cache responding to %s on %#llx (%s):" " invalidating\n", pkt->cmdString(), pkt->getAddr(), pkt->isSecure() ? "s" : "ns"); } if (!last_level_cache) { - DPRINTF(Cache, "forwarding mem-inhibited %s on %#llx (%s)\n", + DPRINTF(Cache, "Other cache responding to %s on %#llx (%s):" + " forwarding\n", pkt->cmdString(), pkt->getAddr(), pkt->isSecure() ? "s" : "ns"); lat += ticksToCycles(memSidePort->sendAtomic(pkt)); } } else { - DPRINTF(Cache, "rcvd mem-inhibited %s on %#llx: not responding\n", + DPRINTF(Cache, "Other cache responding to %s on %#llx: " + "not responding\n", pkt->cmdString(), pkt->getAddr()); } @@ -1034,7 +1061,7 @@ if (!satisfied) { // MISS - PacketPtr bus_pkt = getBusPacket(pkt, blk, pkt->needsExclusive()); + PacketPtr bus_pkt = getBusPacket(pkt, blk, pkt->needsWritable()); bool is_forward = (bus_pkt == NULL); @@ -1181,11 +1208,12 @@ && pkt->checkFunctional(&cbpw, blk_addr, is_secure, blkSize, blk->data); - // data we have is dirty if marked as such or if valid & ownership - // pending due to outstanding UpgradeReq + // data we have is dirty if marked as such or if we have an + // in-service MSHR that is pending a writable line, i.e. in the + // Exclusive or Modified state bool have_dirty = have_data && (blk->isDirty() || - (mshr && mshr->inService && mshr->isPendingDirty())); + (mshr && mshr->inService && mshr->isPendingWritable())); bool done = have_dirty || cpuSidePort->checkFunctional(pkt) @@ -1281,9 +1309,9 @@ miss_latency; } - // upgrade deferred targets if we got exclusive - if (!pkt->sharedAsserted()) { - mshr->promoteExclusive(); + // upgrade deferred targets if the response is passing writable + if (pkt->passWritable()) { + mshr->promoteWritable(); } bool is_fill = !mshr->isForward && @@ -1335,9 +1363,9 @@ // from above. if (tgt_pkt->cmd == MemCmd::WriteLineReq) { assert(!is_error); - // we got the block in exclusive state, so promote any - // deferred targets if possible - mshr->promoteExclusive(); + // we got the block in a writable state, so promote + // any deferred targets if possible + mshr->promoteWritable(); // NB: we use the original packet here and not the response! blk = handleFill(tgt_pkt, blk, writebacks, mshr->allocOnFill); assert(blk != NULL); @@ -1539,7 +1567,7 @@ blk->status &= ~BlkWritable; } else { // we are in the owned state, tell the receiver - pkt->assertShared(); + pkt->setPassNonWritable(); } // make sure the block is not marked dirty @@ -1652,7 +1680,7 @@ // must be an outstanding upgrade request // on a block we're about to replace... assert(!blk->isWritable() || blk->isDirty()); - assert(repl_mshr->needsExclusive()); + assert(repl_mshr->needsWritable()); // too hard to replace block with transient state // allocation failed, block not inserted return NULL; @@ -1753,27 +1781,29 @@ // marked as writable as part of the fill, and then later marked // dirty as part of satisfyCpuSideRequest if (pkt->cmd == MemCmd::WriteLineReq) { - assert(!pkt->sharedAsserted()); + assert(pkt->passWritable()); // at the moment other caches do not respond to the // invalidation requests corresponding to a whole-line write - assert(!pkt->memInhibitAsserted()); + assert(!pkt->cacheResponding()); } - if (!pkt->sharedAsserted()) { - // we could get non-shared responses from memory (rather than - // a cache) even in a read-only cache, note that we set this - // bit even for a read-only cache as we use it to represent - // the exclusive state + // here we deal with setting the appropriate state of the line, + // and we start by looking at the writable flag, and ignore that + // it is a cache responding with dirty if the line is not passed + // as writable, thus the line is never allocated as Owned (dirty + // but not writable), and always ends up being either Shared, + // Exclusive or Modified + if (pkt->passWritable()) { + // we could get a writable line from memory (rather than a + // cache) even in a read-only cache, note that we set this bit + // even for a read-only cache, possibly revisit this decision blk->status |= BlkWritable; - // If we got this via cache-to-cache transfer (i.e., from a - // cache that was an owner) and took away that owner's copy, - // then we need to write it back. Normally this happens - // anyway as a side effect of getting a copy to write it, but - // there are cases (such as failed store conditionals or - // compare-and-swaps) where we'll demand an exclusive copy but - // end up not writing it. - if (pkt->memInhibitAsserted()) { + // check if we got this via cache-to-cache transfer (i.e., from a + // cache that had the block in Modified or Owned state) + if (pkt->cacheResponding()) { + // we got the block in Modified state, and invalidated the + // owners copy blk->status |= BlkDirty; chatty_assert(!isReadOnly, "Should never see dirty snoop response " @@ -1827,7 +1857,7 @@ pkt = new Packet(req_pkt, false, req_pkt->isRead()); assert(req_pkt->req->isUncacheable() || req_pkt->isInvalidate() || - pkt->sharedAsserted()); + !pkt->passWritable()); pkt->makeTimingResponse(); if (pkt->isRead()) { pkt->setDataFromBlock(blk_data, blkSize); @@ -1835,11 +1865,11 @@ if (pkt->cmd == MemCmd::ReadResp && pending_inval) { // Assume we defer a response to a read from a far-away cache // A, then later defer a ReadExcl from a cache B on the same - // bus as us. We'll assert MemInhibit in both cases, but in - // the latter case MemInhibit will keep the invalidation from - // reaching cache A. This special response tells cache A that - // it gets the block to satisfy its read, but must immediately - // invalidate it. + // bus as us. We'll assert cacheResponding in both cases, but + // in the latter case cacheResponding will keep the + // invalidation from reaching cache A. This special response + // tells cache A that it gets the block to satisfy its read, + // but must immediately invalidate it. pkt->cmd = MemCmd::ReadRespWithInvalidate; } // Here we consider forward_time, paying for just forward latency and @@ -1870,7 +1900,7 @@ // responds in atomic mode, so remember a few things about the // original packet up front bool invalidate = pkt->isInvalidate(); - bool M5_VAR_USED needs_exclusive = pkt->needsExclusive(); + bool M5_VAR_USED needs_writable = pkt->needsWritable(); uint32_t snoop_delay = 0; @@ -1878,7 +1908,7 @@ // first propagate snoop upward to see if anyone above us wants to // handle it. save & restore packet src since it will get // rewritten to be relative to cpu-side bus (if any) - bool alreadyResponded = pkt->memInhibitAsserted(); + bool alreadyResponded = pkt->cacheResponding(); if (is_timing) { // copy the packet so that we can clear any flags before // forwarding it upwards, we also allocate data (passing @@ -1896,13 +1926,15 @@ // cache snoop_delay += snoopPkt.headerDelay; - if (snoopPkt.memInhibitAsserted()) { + if (snoopPkt.cacheResponding()) { // cache-to-cache response from some upper cache assert(!alreadyResponded); - pkt->assertMemInhibit(); + pkt->setCacheResponding(); } - if (snoopPkt.sharedAsserted()) { - pkt->assertShared(); + // upstream cache has the block, or has an outstanding + // MSHR, pass the flag on + if (!snoopPkt.passWritable()) { + pkt->setPassNonWritable(); } // If this request is a prefetch or clean evict and an upper level // signals block present, make sure to propagate the block @@ -1912,7 +1944,7 @@ } } else { cpuSidePort->sendAtomicSnoop(pkt); - if (!alreadyResponded && pkt->memInhibitAsserted()) { + if (!alreadyResponded && pkt->cacheResponding()) { // cache-to-cache response from some upper cache: // forward response to original requester assert(pkt->isResponse()); @@ -1941,7 +1973,7 @@ // invalidation itself is taken care of below. bool respond = blk->isDirty() && pkt->needsResponse() && pkt->cmd != MemCmd::InvalidateReq; - bool have_exclusive = blk->isWritable(); + bool have_writable = blk->isWritable(); // Invalidate any prefetch's from below that would strip write permissions // MemCmd::HardPFReq is only observed by upstream caches. After missing @@ -1955,31 +1987,40 @@ } if (!pkt->req->isUncacheable() && pkt->isRead() && !invalidate) { - // reading non-exclusive shared data, note that we retain - // the block in owned state if it is dirty, with the response - // taken care of below, and otherwhise simply downgrade to - // shared - assert(!needs_exclusive); - pkt->assertShared(); + // reading without requiring the line in a writable state, + // note that we retain the block as Owned if it is Modified + // (dirty data), with the response taken care of below, and + // otherwhise simply downgrade from Exclusive to Shared (or + // remain in Shared) + assert(!needs_writable); + pkt->setPassNonWritable(); blk->status &= ~BlkWritable; } if (respond) { // prevent anyone else from responding, cache as well as // memory, and also prevent any memory from even seeing the - // request (with current inhibited semantics), note that this - // applies both to reads and writes and that for writes it - // works thanks to the fact that we still have dirty data and - // will write it back at a later point - assert(!pkt->memInhibitAsserted()); - pkt->assertMemInhibit(); - if (have_exclusive) { + // request + pkt->setCacheResponding(); + if (have_writable) { + // inform the cache hierarchy that this cache had the line + // in the Modified state, even if the response is passed + // as Shared (and thus non-writable) + pkt->setResponderHadWritable(); + // in the case of an uncacheable request there is no point - // in setting the exclusive flag, but since the recipient - // does not care there is no harm in doing so, in any case - // it is just a hint - pkt->setSupplyExclusive(); + // in setting the responderHadWritable flag, but since the + // recipient does not care there is no harm in doing so + } else { + // we only had the line as Owned, and should normally set + // the non-writable flag here, but instead we pass the + // line as writable (in the Modified state), and rely on + // any neighbouring caches to treat the snoop of a + // transaction that needs writable as an invalidate, and + // send out express snoops downstream once this packet + // hits the next cache } + if (is_timing) { doTimingSupplyResponse(pkt, blk->data, is_deferred, pending_inval); } else { @@ -2090,14 +2131,14 @@ } if (wb_pkt->cmd == MemCmd::WritebackDirty) { - assert(!pkt->memInhibitAsserted()); - pkt->assertMemInhibit(); - if (!pkt->needsExclusive()) { - pkt->assertShared(); - // the writeback is no longer passing exclusivity (the - // receiving cache should consider the block owned - // rather than modified) - wb_pkt->assertShared(); + // the packet we are responding to is now the dirty line + pkt->setCacheResponding(); + if (!pkt->needsWritable()) { + pkt->setPassNonWritable(); + // the writeback is no longer passing writeable (the + // receiving cache should consider the block Owned + // rather than Modified) + wb_pkt->setPassNonWritable(); } else { // if we're not asserting the shared line, we need to // invalidate our copy. we'll do that below as long as @@ -2114,16 +2155,16 @@ // The cache technically holds the block until the // corresponding message reaches the crossbar // below. Therefore when a snoop encounters a CleanEvict - // or WritebackClean message we must set assertShared + // or WritebackClean message we must set setPassNonWritable // (just like when it encounters a Writeback) to avoid the // snoop filter prematurely clearing the holder bit in the // crossbar below - if (!pkt->needsExclusive()) { - pkt->assertShared(); - // the writeback is no longer passing exclusivity (the - // receiving cache should consider the block owned - // rather than modified) - wb_pkt->assertShared(); + if (!pkt->needsWritable()) { + pkt->setPassNonWritable(); + // the writeback is no longer passing writeable (the + // receiving cache should consider the block Owned + // rather than Modified) + wb_pkt->setPassNonWritable(); } else { assert(pkt->isInvalidate()); } @@ -2280,7 +2321,7 @@ snoop_pkt.senderState = NULL; cpuSidePort->sendTimingSnoopReq(&snoop_pkt); // Writeback/CleanEvict snoops do not generate a snoop response. - assert(!(snoop_pkt.memInhibitAsserted())); + assert(!(snoop_pkt.cacheResponding())); return snoop_pkt.isBlockCached(); } else { cpuSidePort->sendAtomicSnoop(pkt); @@ -2327,18 +2368,22 @@ // the MSHRs and when it was selected to be sent or if the // prefetch was squashed by an upper cache. - // It is important to check memInhibitAsserted before - // prefetchSquashed. If another cache has asserted MEM_INGIBIT, it - // will be sending a response which will arrive at the MSHR - // allocated ofr this request. Checking the prefetchSquash first - // may result in the MSHR being prematurely deallocated. - - if (snoop_pkt.memInhibitAsserted()) { + // It is important to check cacheResponding before + // prefetchSquashed. If another cache has committed to + // responding, it will be sending a dirty response which will + // arrive at the MSHR allocated ofr this request. Checking the + // prefetchSquash first may result in the MSHR being + // prematurely deallocated. + if (snoop_pkt.cacheResponding()) { auto M5_VAR_USED r = outstandingSnoop.insert(snoop_pkt.req); assert(r.second); - // If we are getting a non-shared response it is dirty - bool pending_dirty_resp = !snoop_pkt.sharedAsserted(); - markInService(mshr, pending_dirty_resp); + + // the snoop response is always dirty, and if we are + // getting a writable response it will be allocated as + // Modified + bool pending_modified_resp = snoop_pkt.passWritable(); + markInService(mshr, pending_modified_resp); + DPRINTF(Cache, "Upward snoop of prefetch for addr" " %#x (%s) hit\n", tgt_pkt->getAddr(), tgt_pkt->isSecure()? "s": "ns"); @@ -2364,7 +2409,7 @@ assert(tags->findBlock(mshr->blkAddr, mshr->isSecure) == NULL); pkt = tgt_pkt; } else { - pkt = getBusPacket(tgt_pkt, blk, mshr->needsExclusive()); + pkt = getBusPacket(tgt_pkt, blk, mshr->needsWritable()); mshr->isForward = (pkt == NULL); @@ -2454,10 +2499,11 @@ bool success = false; - // always let inhibited requests through, even if blocked, - // ultimately we should check if this is an express snoop, but at - // the moment that flag is only set in the cache itself - if (pkt->memInhibitAsserted()) { + // always let packets through if an upstream cache has committed + // to responding, even if blocked (we should technically look at + // the isExpressSnoop flag, but it is set by the cache itself, and + // consequently we have to rely on the cacheResponding flag) + if (pkt->cacheResponding()) { // do not change the current retry state bool M5_VAR_USED bypass_success = cache->recvTimingReq(pkt); assert(bypass_success); @@ -2597,18 +2643,16 @@ // it gets retried } else { // As part of the call to sendTimingReq the packet is - // forwarded to all neighbouring caches (and any - // caches above them) as a snoop. The packet is also - // sent to any potential cache below as the - // interconnect is not allowed to buffer the - // packet. Thus at this point we know if any of the - // neighbouring, or the downstream cache is - // responding, and if so, if it is with a dirty line - // or not. - bool pending_dirty_resp = !pkt->sharedAsserted() && - pkt->memInhibitAsserted(); + // forwarded to all neighbouring caches (and any caches + // above them) as a snoop. Thus at this point we know if + // any of the neighbouring caches are responding, and if + // so, we know it is dirty, and we can determine if it is + // being passed as Modified, making our MSHR the ordering + // point + bool pending_modified_resp = pkt->passWritable() && + pkt->cacheResponding(); - cache.markInService(mshr, pending_dirty_resp); + cache.markInService(mshr, pending_modified_resp); } } diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/addr_mapper.cc --- a/src/mem/addr_mapper.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/addr_mapper.cc Tue Dec 22 16:23:08 2015 +0000 @@ -116,16 +116,15 @@ { Addr orig_addr = pkt->getAddr(); bool needsResponse = pkt->needsResponse(); - bool memInhibitAsserted = pkt->memInhibitAsserted(); + bool cacheResponding = pkt->cacheResponding(); - if (needsResponse && !memInhibitAsserted) { + if (needsResponse && !cacheResponding) { pkt->pushSenderState(new AddrMapperSenderState(orig_addr)); } pkt->setAddr(remapAddr(orig_addr)); - // Attempt to send the packet (always succeeds for inhibited - // packets) + // Attempt to send the packet bool successful = masterPort.sendTimingReq(pkt); // If not successful, restore the address and sender state diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/bridge.cc --- a/src/mem/bridge.cc Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/bridge.cc Tue Dec 22 16:23:08 2015 +0000 @@ -154,9 +154,9 @@ DPRINTF(Bridge, "recvTimingReq: %s addr 0x%x\n", pkt->cmdString(), pkt->getAddr()); - // sink inhibited packets without further action, also discard any - // packet that is not a read or a write - if (pkt->memInhibitAsserted() || + // if a cache is responding, sink the packet without further + // action, also discard any packet that is not a read or a write + if (pkt->cacheResponding() || !(pkt->isWrite() || pkt->isRead())) { assert(!pkt->needsResponse()); pendingDelete.reset(pkt); diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/cache/base.hh --- a/src/mem/cache/base.hh Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/cache/base.hh Tue Dec 22 16:23:08 2015 +0000 @@ -224,11 +224,11 @@ return mshr; } - void markInServiceInternal(MSHR *mshr, bool pending_dirty_resp) + void markInServiceInternal(MSHR *mshr, bool pending_modified_resp) { MSHRQueue *mq = mshr->queue; bool wasFull = mq->isFull(); - mq->markInService(mshr, pending_dirty_resp); + mq->markInService(mshr, pending_modified_resp); if (wasFull && !mq->isFull()) { clearBlocked((BlockedCause)mq->index); } diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/cache/blk.hh --- a/src/mem/cache/blk.hh Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/cache/blk.hh Tue Dec 22 16:23:08 2015 +0000 @@ -281,7 +281,7 @@ /** * Pretty-print a tag, and interpret state bits to readable form - * including mapping to a MOESI stat. + * including mapping to a MOESI state. * * @return string with basic state information */ @@ -299,6 +299,12 @@ * E 1 0 1 * S 0 0 1 * I 0 0 0 + * + * Note that only one cache ever has a block in Modified or + * Owned state, but that multiple caches on the same path to + * memory can have a block in the Exclusive state (despite the + * name), and that multiple caches on different paths can have + * a block in the Shared state. **/ unsigned state = isWritable() << 2 | isDirty() << 1 | isValid(); char s = '?'; diff -r 07706eb5faf4 -r e4679f18edb9 src/mem/cache/cache.hh --- a/src/mem/cache/cache.hh Tue Dec 22 16:23:07 2015 +0000 +++ b/src/mem/cache/cache.hh Tue Dec 22 16:23:08 2015 +0000 @@ -472,13 +472,14 @@ PacketPtr getTimingPacket(); /** - * Marks a request as in service (sent on the bus). This can have - * side effect since storage for no response commands is - * deallocated once they are successfully sent. Also remember if - * we are expecting a dirty response from another cache, - * effectively making this MSHR the ordering point. + * Marks a request as in service (sent downstream in the memory + * system). This can have side effect since storage for no + * response commands is deallocated once they are successfully + * sent. Also remember if we are expecting a Modified (dirty and + * writable) response from another cache, effectively making this + * MSHR the ordering point. */ - void markInService(MSHR *mshr, bool pending_dirty_resp); + void markInService(MSHR *mshr, bool pending_modified_resp); /** * Return whether there are any outstanding misses.