[gem5-dev] [M] Change in gem5/gem5[develop]: systemc: sync the response error between gem5 packet and tlm payload

[Yu-hsin Wang (Gerrit)]

Yu-hsin Wang has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/64591?usp=email )

Change subject: systemc: sync the response error between gem5 packet and  
tlm payload
......................................................................

systemc: sync the response error between gem5 packet and tlm payload

For now we don't return the correct error status to the upstream of the
protocol conversion bridge. This prevents from the requestor to
distinguish if the response is good or not. This change fixes the issue.

Change-Id: Iec2a388b50fb1bd4fd97ece19e9061138b0b0a1f
---
M src/systemc/tlm_bridge/gem5_to_tlm.cc
M src/systemc/tlm_bridge/tlm_to_gem5.cc
M src/systemc/tlm_bridge/tlm_to_gem5.hh
3 files changed, 59 insertions(+), 25 deletions(-)



diff --git a/src/systemc/tlm_bridge/gem5_to_tlm.cc  
b/src/systemc/tlm_bridge/gem5_to_tlm.cc
index 07e4821..cca23b4 100644
--- a/src/systemc/tlm_bridge/gem5_to_tlm.cc
+++ b/src/systemc/tlm_bridge/gem5_to_tlm.cc
@@ -194,6 +194,23 @@
     return trans;
 }

+void
+setPacketResponse(PacketPtr pkt, tlm::tlm_generic_payload &trans)
+{
+    auto resp = trans.get_response_status();
+    switch (resp) {
+      case tlm::TLM_OK_RESPONSE:
+        pkt->makeResponse();
+        break;
+      case tlm::TLM_COMMAND_ERROR_RESPONSE:
+        pkt->setFunctionalResponseStatus(false);
+        break;
+      default:
+        pkt->setBadAddress();
+        break;
+    }
+}
+
 template <unsigned int BITWIDTH>
 void
 Gem5ToTlmBridge<BITWIDTH>::pec(
@@ -225,7 +242,7 @@
         // we make a response packet before sending it back to the  
initiator
         // side gem5 module.
         if (packet->needsResponse()) {
-            packet->makeResponse();
+            setPacketResponse(packet, trans);
         }
         if (packet->isResponse()) {
             need_retry = !bridgeResponsePort.sendTimingResp(packet);
@@ -296,7 +313,7 @@
     }

     if (packet->needsResponse())
-        packet->makeResponse();
+        setPacketResponse(packet, *trans);

     trans->release();

@@ -329,7 +346,7 @@
     }

     if (packet->needsResponse())
-        packet->makeResponse();
+        setPacketResponse(packet, *trans);

     trans->release();

diff --git a/src/systemc/tlm_bridge/tlm_to_gem5.cc  
b/src/systemc/tlm_bridge/tlm_to_gem5.cc
index 7aec14f..3fd8bed 100644
--- a/src/systemc/tlm_bridge/tlm_to_gem5.cc
+++ b/src/systemc/tlm_bridge/tlm_to_gem5.cc
@@ -178,6 +178,20 @@
     return std::make_pair(pkt, true);
 }

+void
+setPayloadResponse(tlm::tlm_generic_payload &trans, PacketPtr pkt)
+{
+    if (!pkt->isError()) {
+        trans.set_response_status(tlm::TLM_OK_RESPONSE);
+        return;
+    }
+    if (pkt->isRead() || pkt->isWrite()) {
+        trans.set_response_status(tlm::TLM_COMMAND_ERROR_RESPONSE);
+        return;
+    }
+    trans.set_response_status(tlm::TLM_ADDRESS_ERROR_RESPONSE);
+}
+
 template <unsigned int BITWIDTH>
 void
 TlmToGem5Bridge<BITWIDTH>::sendEndReq(tlm::tlm_generic_payload &trans)
@@ -195,9 +209,15 @@
 TlmToGem5Bridge<BITWIDTH>::sendBeginResp(tlm::tlm_generic_payload &trans,
                                          sc_core::sc_time &delay)
 {
-    tlm::tlm_phase phase = tlm::BEGIN_RESP;
+    Gem5SystemC::Gem5Extension *extension = nullptr;
+    trans.get_extension(extension);
+    panic_if(extension == nullptr,
+             "Unexpected extension when sending BEGIN_RESP");
+    auto pkt = extension->getPacket();

-    trans.set_response_status(tlm::TLM_OK_RESPONSE);
+    setPayloadResponse(trans, pkt);
+
+    tlm::tlm_phase phase = tlm::BEGIN_RESP;

     auto status = socket->nb_transport_bw(trans, phase, delay);

@@ -252,8 +272,6 @@

     responseInProgress = false;

-    checkTransaction(trans);
-
     if (needToSendRetry) {
         bmp.sendRetryResp();
         needToSendRetry = false;
@@ -269,18 +287,6 @@

 template <unsigned int BITWIDTH>
 void
-TlmToGem5Bridge<BITWIDTH>::checkTransaction(tlm::tlm_generic_payload  
&trans)
-{
-    if (trans.is_response_error()) {
-        std::stringstream ss;
-        ss << "Transaction returned with error, response status = "
-           << trans.get_response_string();
-        SC_REPORT_ERROR("TLM-2", ss.str().c_str());
-    }
-}
-
-template <unsigned int BITWIDTH>
-void
 TlmToGem5Bridge<BITWIDTH>::invalidateDmi(const gem5::MemBackdoor &backdoor)
 {
     socket->invalidate_direct_mem_ptr(
@@ -362,10 +368,10 @@
     // clean up
     delete senderState;

+    setPayloadResponse(trans, pkt);
+
     if (pkt_created)
         destroyPacket(pkt);
-
-    trans.set_response_status(tlm::TLM_OK_RESPONSE);
 }

 template <unsigned int BITWIDTH>
@@ -437,11 +443,11 @@
     // clean up
     delete senderState;

+    setPayloadResponse(trans, pkt);
+
     if (pkt_created)
         destroyPacket(pkt);

-    trans.set_response_status(tlm::TLM_OK_RESPONSE);
-
     return backdoor != nullptr;
 }

diff --git a/src/systemc/tlm_bridge/tlm_to_gem5.hh  
b/src/systemc/tlm_bridge/tlm_to_gem5.hh
index deb332e..ca5f681 100644
--- a/src/systemc/tlm_bridge/tlm_to_gem5.hh
+++ b/src/systemc/tlm_bridge/tlm_to_gem5.hh
@@ -141,8 +141,6 @@

     void destroyPacket(gem5::PacketPtr pkt);

-    void checkTransaction(tlm::tlm_generic_payload &trans);
-
     void invalidateDmi(const gem5::MemBackdoor &backdoor);

   protected:

--
To view, visit  
https://gem5-review.googlesource.com/c/public/gem5/+/64591?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: Iec2a388b50fb1bd4fd97ece19e9061138b0b0a1f
Gerrit-Change-Number: 64591
Gerrit-PatchSet: 1
Gerrit-Owner: Yu-hsin Wang <yuhsi...@google.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org