diff options
| -rw-r--r-- | src/mem/cache/cache_impl.hh | 38 | ||||
| -rw-r--r-- | src/mem/cache/mshr.cc | 6 | ||||
| -rw-r--r-- | src/mem/packet.hh | 52 |
3 files changed, 73 insertions, 23 deletions
diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh index 296f31ebd..a161f8085 100644 --- a/src/mem/cache/cache_impl.hh +++ b/src/mem/cache/cache_impl.hh @@ -487,17 +487,24 @@ Cache<TagStore>::recvTimingReq(PacketPtr pkt) // that have shared copies. Not necessary if we know that // supplier had exclusive copy to begin with. if (pkt->needsExclusive() && !pkt->isSupplyExclusive()) { - Packet *snoopPkt = new Packet(pkt, true); // clear flags + // 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 + Packet *snoop_pkt = new Packet(pkt, true, false); + // also reset the bus time that the original packet has // not yet paid for - snoopPkt->firstWordDelay = snoopPkt->lastWordDelay = 0; - snoopPkt->setExpressSnoop(); - snoopPkt->assertMemInhibit(); - bool M5_VAR_USED success = memSidePort->sendTimingReq(snoopPkt); - // the packet is marked inhibited and will thus bypass any - // flow control + snoop_pkt->firstWordDelay = snoop_pkt->lastWordDelay = 0; + + // make this an instantaneous express snoop, and let the + // other caches in the system know that the packet is + // inhibited + snoop_pkt->setExpressSnoop(); + snoop_pkt->assertMemInhibit(); + bool M5_VAR_USED success = memSidePort->sendTimingReq(snoop_pkt); + // express snoops always succeed assert(success); - // main memory will delete snoopPkt + // main memory will delete the packet } // since we're the official target but we aren't responding, // delete the packet now. @@ -1570,12 +1577,19 @@ doTimingSupplyResponse(PacketPtr req_pkt, const uint8_t *blk_data, { // sanity check assert(req_pkt->isRequest()); + assert(req_pkt->needsResponse()); DPRINTF(Cache, "%s for %s address %x size %d\n", __func__, req_pkt->cmdString(), req_pkt->getAddr(), req_pkt->getSize()); // timing-mode snoop responses require a new packet, unless we // already made a copy... - PacketPtr pkt = already_copied ? req_pkt : new Packet(req_pkt); + PacketPtr pkt = req_pkt; + if (!already_copied) + // do not clear flags, and allocate space for data if the + // packet needs it (the only packets that carry data are read + // responses) + pkt = new Packet(req_pkt, false, req_pkt->isRead()); + assert(req_pkt->isInvalidate() || pkt->sharedAsserted()); pkt->makeTimingResponse(); // @todo Make someone pay for this @@ -1624,7 +1638,7 @@ Cache<TagStore>::handleSnoop(PacketPtr pkt, BlkType *blk, // rewritten to be relative to cpu-side bus (if any) bool alreadyResponded = pkt->memInhibitAsserted(); if (is_timing) { - Packet snoopPkt(pkt, true); // clear flags + Packet snoopPkt(pkt, true, false); // clear flags, no allocation snoopPkt.setExpressSnoop(); snoopPkt.pushSenderState(new ForwardResponseRecord(pkt->getSrc())); // the snoop packet does not need to wait any additional @@ -1996,7 +2010,7 @@ Cache<TagStore>::getTimingPacket() // at the moment. Without this check we could get a stale // copy from memory that might get used in place of the // dirty one. - Packet snoop_pkt(tgt_pkt, true); + Packet snoop_pkt(tgt_pkt, true, false); snoop_pkt.setExpressSnoop(); snoop_pkt.senderState = mshr; cpuSidePort->sendTimingSnoopReq(&snoop_pkt); @@ -2032,7 +2046,7 @@ Cache<TagStore>::getTimingPacket() // not a cache block request, but a response is expected // make copy of current packet to forward, keep current // copy for response handling - pkt = new Packet(tgt_pkt); + pkt = new Packet(tgt_pkt, false, true); if (pkt->isWrite()) { pkt->setData(tgt_pkt->getConstPtr<uint8_t>()); } diff --git a/src/mem/cache/mshr.cc b/src/mem/cache/mshr.cc index bc46ed267..e4b62e230 100644 --- a/src/mem/cache/mshr.cc +++ b/src/mem/cache/mshr.cc @@ -366,7 +366,11 @@ MSHR::handleSnoop(PacketPtr pkt, Counter _order) // Actual target device (typ. a memory) will delete the // packet on reception, so we need to save a copy here. - PacketPtr cp_pkt = new Packet(pkt, true); + + // Clear flags and also allocate new data as the original + // packet data storage may have been deleted by the time we + // get to send this packet. + PacketPtr cp_pkt = new Packet(pkt, true, true); targets.add(cp_pkt, curTick(), _order, Target::FromSnoop, downstreamPending && targets.needsExclusive); diff --git a/src/mem/packet.hh b/src/mem/packet.hh index d0b210975..dab1b1b95 100644 --- a/src/mem/packet.hh +++ b/src/mem/packet.hh @@ -652,9 +652,9 @@ class Packet : public Printable * less than that of the original packet. In this case the new * packet should allocate its own data. */ - Packet(PacketPtr pkt, bool clearFlags = false) + Packet(PacketPtr pkt, bool clear_flags, bool alloc_data) : cmd(pkt->cmd), req(pkt->req), - data(pkt->flags.isSet(STATIC_DATA) ? pkt->data : NULL), + data(nullptr), addr(pkt->addr), _isSecure(pkt->_isSecure), size(pkt->size), src(pkt->src), dest(pkt->dest), bytesValidStart(pkt->bytesValidStart), @@ -663,16 +663,26 @@ class Packet : public Printable lastWordDelay(pkt->lastWordDelay), senderState(pkt->senderState) { - if (!clearFlags) + if (!clear_flags) flags.set(pkt->flags & COPY_FLAGS); flags.set(pkt->flags & (VALID_ADDR|VALID_SIZE)); - flags.set(pkt->flags & STATIC_DATA); - // if we did not copy the static data pointer, allocate data - // dynamically instead - if (!data) - allocate(); + // should we allocate space for data, or not, the express + // snoops do not need to carry any data as they only serve to + // co-ordinate state changes + if (alloc_data) { + // even if asked to allocate data, if the original packet + // holds static data, then the sender will not be doing + // any memcpy on receiving the response, thus we simply + // carry the pointer forward + if (pkt->flags.isSet(STATIC_DATA)) { + data = pkt->data; + flags.set(STATIC_DATA); + } else { + allocate(); + } + } } /** @@ -789,7 +799,14 @@ class Packet : public Printable /** * Set the data pointer to the following value that should not be - * freed. + * freed. Static data allows us to do a single memcpy even if + * multiple packets are required to get from source to destination + * and back. In essence the pointer is set calling dataStatic on + * the original packet, and whenever this packet is copied and + * forwarded the same pointer is passed on. When a packet + * eventually reaches the destination holding the data, it is + * copied once into the location originally set. On the way back + * to the source, no copies are necessary. */ template <typename T> void @@ -819,7 +836,15 @@ class Packet : public Printable /** * Set the data pointer to a value that should have delete [] - * called on it. + * called on it. Dynamic data is local to this packet, and as the + * packet travels from source to destination, forwarded packets + * will allocate their own data. When a packet reaches the final + * destination it will populate the dynamic data of that specific + * packet, and on the way back towards the source, memcpy will be + * invoked in every step where a new packet was created e.g. in + * the caches. Ultimately when the response reaches the source a + * final memcpy is needed to extract the data from the packet + * before it is deallocated. */ template <typename T> void @@ -867,7 +892,14 @@ class Packet : public Printable void setData(const uint8_t *p) { + // we should never be copying data onto itself, which means we + // must idenfity packets with static data, as they carry the + // same pointer from source to destination and back + assert(p != getPtr<uint8_t>() || flags.isSet(STATIC_DATA)); + if (p != getPtr<uint8_t>()) + // for packet with allocated dynamic data, we copy data from + // one to the other, e.g. a forwarded response to a response std::memcpy(getPtr<uint8_t>(), p, getSize()); } |