Yu-hsin Wang has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/63771?usp=email )
Change subject: systemc: gem5_to_tlm bridge should reuse existed tlm payload
......................................................................
systemc: gem5_to_tlm bridge should reuse existed tlm payload
Given a data path initiated from SystemC, routed by gem5, and handled
by SystemC finally.
SystemC -> gem5 -> SystemC
The target SystemC needs to get the original transaction object.
Otherwise, it would lose the extensions in the payload.
To fix the issue, we moves the SenderState class to public for reachibility.
After that, we refactor the logic converting between payload and packet
to make sure they can use the correct instance. Finally, we fix the
potential address change during routing.
Change-Id: Ic6d24e98454f564f7dd6b43ad8c011e1ea7ea433
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/63771
Tested-by: kokoro <noreply+kok...@google.com>
Reviewed-by: Gabe Black <gabe.bl...@gmail.com>
Maintainer: Gabe Black <gabe.bl...@gmail.com>
---
M src/systemc/tlm_bridge/gem5_to_tlm.cc
M src/systemc/tlm_bridge/sc_ext.hh
M src/systemc/tlm_bridge/tlm_to_gem5.cc
M src/systemc/tlm_bridge/tlm_to_gem5.hh
4 files changed, 88 insertions(+), 67 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/systemc/tlm_bridge/gem5_to_tlm.cc
b/src/systemc/tlm_bridge/gem5_to_tlm.cc
index 37da822..6f17a8b 100644
--- a/src/systemc/tlm_bridge/gem5_to_tlm.cc
+++ b/src/systemc/tlm_bridge/gem5_to_tlm.cc
@@ -121,13 +121,29 @@
}
/**
- * Convert a gem5 packet to a TLM payload by copying all the relevant
- * information to new tlm payload.
+ * Convert a gem5 packet to TLM payload by copying all the relevant
information
+ * to new payload. If the transaction is initiated by TLM model, we would
use
+ * the original payload.
+ * The return value is the payload pointer.
*/
tlm::tlm_generic_payload *
packet2payload(PacketPtr packet)
{
- tlm::tlm_generic_payload *trans = mm.allocate();
+ tlm::tlm_generic_payload *trans = nullptr;
+ auto *tlmSenderState =
+ packet->findNextSenderState<Gem5SystemC::TlmSenderState>();
+
+ // If there is a SenderState, we can pipe through the original
transaction.
+ // Otherwise, we generate a new transaction based on the packet.
+ if (tlmSenderState != nullptr) {
+ // Sync the address which could have changed.
+ trans = &tlmSenderState->trans;
+ trans->set_address(packet->getAddr());
+ trans->acquire();
+ return trans;
+ }
+
+ trans = mm.allocate();
trans->acquire();
trans->set_address(packet->getAddr());
diff --git a/src/systemc/tlm_bridge/sc_ext.hh
b/src/systemc/tlm_bridge/sc_ext.hh
index 3f7c320..bb67676 100644
--- a/src/systemc/tlm_bridge/sc_ext.hh
+++ b/src/systemc/tlm_bridge/sc_ext.hh
@@ -44,6 +44,12 @@
namespace Gem5SystemC
{
+struct TlmSenderState : public gem5::Packet::SenderState
+{
+ tlm::tlm_generic_payload &trans;
+ TlmSenderState(tlm::tlm_generic_payload &trans) : trans(trans) {}
+};
+
class Gem5Extension: public tlm::tlm_extension<Gem5Extension>
{
public:
diff --git a/src/systemc/tlm_bridge/tlm_to_gem5.cc
b/src/systemc/tlm_bridge/tlm_to_gem5.cc
index 9f7c6eb..4cb8194 100644
--- a/src/systemc/tlm_bridge/tlm_to_gem5.cc
+++ b/src/systemc/tlm_bridge/tlm_to_gem5.cc
@@ -97,9 +97,29 @@
extraPayloadToPacketSteps.push_back(std::move(step));
}
-PacketPtr
+/**
+ * Convert a TLM payload to gem5 packet by copying all the relevant
information
+ * to new packet. If the transaction is initiated by gem5 model, we would
use
+ * the original packet.
+ * The first return value is the packet pointer.
+ * The second return value is if the packet is newly created.
+ */
+std::pair<PacketPtr, bool>
payload2packet(RequestorID _id, tlm::tlm_generic_payload &trans)
{
+ Gem5SystemC::Gem5Extension *extension = nullptr;
+ trans.get_extension(extension);
+
+ // If there is an extension, this transaction was initiated by the gem5
+ // world and we can pipe through the original packet. Otherwise, we
+ // generate a new packet based on the transaction.
+ if (extension != nullptr) {
+ auto pkt = extension->getPacket();
+ // Sync the address which could have changed.
+ pkt->setAddr(trans.get_address());
+ return std::make_pair(pkt, false);
+ }
+
MemCmd cmd;
RequestPtr req;
@@ -126,7 +146,7 @@
cmd = MemCmd::WriteReq;
break;
case tlm::TLM_IGNORE_COMMAND:
- return nullptr;
+ return std::make_pair(nullptr, false);
default:
SC_REPORT_FATAL("TlmToGem5Bridge",
"received transaction with unsupported "
@@ -150,7 +170,7 @@
step(pkt, trans);
}
- return pkt;
+ return std::make_pair(pkt, true);
}
template <unsigned int BITWIDTH>
@@ -198,21 +218,8 @@
trans.acquire();
- PacketPtr pkt = nullptr;
-
- Gem5SystemC::Gem5Extension *extension = nullptr;
- trans.get_extension(extension);
-
- // If there is an extension, this transaction was initiated by the gem5
- // world and we can pipe through the original packet. Otherwise, we
- // generate a new packet based on the transaction.
- if (extension != nullptr) {
- pkt = extension->getPacket();
- } else {
- pkt = payload2packet(_id, trans);
- }
-
- auto tlmSenderState = new TlmSenderState(trans);
+ auto [pkt, pkt_created] = payload2packet(_id, trans);
+ auto *tlmSenderState = new Gem5SystemC::TlmSenderState(trans);
pkt->pushSenderState(tlmSenderState);
// If the packet doesn't need a response, we should send BEGIN_RESP by
@@ -323,18 +330,7 @@
TlmToGem5Bridge<BITWIDTH>::b_transport(tlm::tlm_generic_payload &trans,
sc_core::sc_time &t)
{
- Gem5SystemC::Gem5Extension *extension = nullptr;
- trans.get_extension(extension);
-
- PacketPtr pkt = nullptr;
-
- // If there is an extension, this transaction was initiated by the gem5
- // world and we can pipe through the original packet.
- if (extension != nullptr) {
- pkt = extension->getPacket();
- } else {
- pkt = payload2packet(_id, trans);
- }
+ auto [pkt, pkt_created] = payload2packet(_id, trans);
MemBackdoorPtr backdoor = nullptr;
Tick ticks = bmp.sendAtomicBackdoor(pkt, backdoor);
@@ -352,7 +348,7 @@
// update time
t += delay;
- if (extension == nullptr)
+ if (pkt_created)
destroyPacket(pkt);
trans.set_response_status(tlm::TLM_OK_RESPONSE);
@@ -362,19 +358,12 @@
unsigned int
TlmToGem5Bridge<BITWIDTH>::transport_dbg(tlm::tlm_generic_payload &trans)
{
- Gem5SystemC::Gem5Extension *extension = nullptr;
- trans.get_extension(extension);
+ auto [pkt, pkt_created] = payload2packet(_id, trans);
+ if (pkt != nullptr) {
+ bmp.sendFunctional(pkt);
- // If there is an extension, this transaction was initiated by the gem5
- // world and we can pipe through the original packet.
- if (extension != nullptr) {
- bmp.sendFunctional(extension->getPacket());
- } else {
- auto pkt = payload2packet(_id, trans);
- if (pkt) {
- bmp.sendFunctional(pkt);
+ if (pkt_created)
destroyPacket(pkt);
- }
}
return trans.get_data_length();
@@ -385,19 +374,7 @@
TlmToGem5Bridge<BITWIDTH>::get_direct_mem_ptr(tlm::tlm_generic_payload
&trans,
tlm::tlm_dmi &dmi_data)
{
- Gem5SystemC::Gem5Extension *extension = nullptr;
- trans.get_extension(extension);
-
- PacketPtr pkt = nullptr;
-
- // If there is an extension, this transaction was initiated by the gem5
- // world and we can pipe through the original packet.
- if (extension != nullptr) {
- pkt = extension->getPacket();
- } else {
- pkt = payload2packet(_id, trans);
- pkt->req->setFlags(Request::NO_ACCESS);
- }
+ auto [pkt, pkt_created] = payload2packet(_id, trans);
MemBackdoorPtr backdoor = nullptr;
bmp.sendAtomicBackdoor(pkt, backdoor);
@@ -423,7 +400,7 @@
);
}
- if (extension == nullptr)
+ if (!pkt_created)
destroyPacket(pkt);
trans.set_response_status(tlm::TLM_OK_RESPONSE);
@@ -455,7 +432,8 @@
pkt->payloadDelay = 0;
pkt->headerDelay = 0;
- auto tlmSenderState =
dynamic_cast<TlmSenderState*>(pkt->popSenderState());
+ auto *tlmSenderState =
+ dynamic_cast<Gem5SystemC::TlmSenderState*>(pkt->popSenderState());
sc_assert(tlmSenderState != nullptr);
auto &trans = tlmSenderState->trans;
diff --git a/src/systemc/tlm_bridge/tlm_to_gem5.hh
b/src/systemc/tlm_bridge/tlm_to_gem5.hh
index b0fe62a..e452d8b 100644
--- a/src/systemc/tlm_bridge/tlm_to_gem5.hh
+++ b/src/systemc/tlm_bridge/tlm_to_gem5.hh
@@ -59,6 +59,7 @@
#define __SYSTEMC_TLM_BRIDGE_TLM_TO_GEM5_HH__
#include <functional>
+#include <utility>
#include "mem/port.hh"
#include "params/TlmToGem5BridgeBase.hh"
@@ -78,7 +79,7 @@
void addPayloadToPacketConversionStep(PayloadToPacketConversionStep step);
-gem5::PacketPtr payload2packet(gem5::RequestorID _id,
+std::pair<gem5::PacketPtr, bool> payload2packet(gem5::RequestorID _id,
tlm::tlm_generic_payload &trans);
class TlmToGem5BridgeBase : public sc_core::sc_module
@@ -91,12 +92,6 @@
class TlmToGem5Bridge : public TlmToGem5BridgeBase
{
private:
- struct TlmSenderState : public gem5::Packet::SenderState
- {
- tlm::tlm_generic_payload &trans;
- TlmSenderState(tlm::tlm_generic_payload &trans) : trans(trans) {}
- };
-
class BridgeRequestPort : public gem5::RequestPort
{
protected:
--
To view, visit
https://gem5-review.googlesource.com/c/public/gem5/+/63771?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ic6d24e98454f564f7dd6b43ad8c011e1ea7ea433
Gerrit-Change-Number: 63771
Gerrit-PatchSet: 7
Gerrit-Owner: Yu-hsin Wang <yuhsi...@google.com>
Gerrit-Reviewer: Earl Ou <shunhsin...@google.com>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Gabe Black <gabebl...@google.com>
Gerrit-Reviewer: Jui-min Lee <f...@google.com>
Gerrit-Reviewer: Yu-hsin Wang <yuhsi...@google.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-CC: Philip Metzler <cpm...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
