Jui-min Lee has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/44305 )

Change subject: systemc: Fix tlm phase ordering in Gem5ToTlmBridge
......................................................................

systemc: Fix tlm phase ordering in Gem5ToTlmBridge

Previously we only rely on timestamp to schedule all phase change events
of tlm transactions in Gem5ToTlmBridge. However, it is valid to have
END_REQ and BEGIN_RESP of a transaction to be scheduled at the same
timestamp and the gem5 scheduler will not necessary order them in a
correct way.

This unfortunately breaks the code as sending a response before
accepting a request doesn't make sense at all and will trigger an
assertion failure.

In this CL we slightly increase the priority of END_REQ event so we can
always process phase change events in a correct order.

Change-Id: Ic33a92162c8c53af3887c7b04090115a38f96866
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/44305
Reviewed-by: Gabe Black <gabe.bl...@gmail.com>
Maintainer: Gabe Black <gabe.bl...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/systemc/tlm_bridge/gem5_to_tlm.cc
1 file changed, 25 insertions(+), 4 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 b80a083..e1cc087 100644
--- a/src/systemc/tlm_bridge/gem5_to_tlm.cc
+++ b/src/systemc/tlm_bridge/gem5_to_tlm.cc
@@ -62,6 +62,7 @@

 #include "params/Gem5ToTlmBridge32.hh"
 #include "params/Gem5ToTlmBridge64.hh"
+#include "sim/eventq.hh"
 #include "sim/system.hh"
 #include "systemc/tlm_bridge/sc_ext.hh"
 #include "systemc/tlm_bridge/sc_mm.hh"
@@ -70,6 +71,24 @@
 {

 /**
+ * Helper function to help set priority of phase change events of tlm
+ * transactions. This is to workaround the uncertainty of gem5 eventq if
+ * multiple events are scheduled at the same timestamp.
+ */
+static EventBase::Priority
+getPriorityOfTlmPhase(const tlm::tlm_phase& phase)
+{
+ // In theory, for all phase change events of a specific TLM base protocol + // transaction, only tlm::END_REQ and tlm::BEGIN_RESP would be scheduled at + // the same time in the same queue. So we only need to ensure END_REQ has a
+    // higher priority (less in pri value) than BEGIN_RESP.
+    if (phase == tlm::END_REQ) {
+        return EventBase::Default_Pri - 1;
+    }
+    return EventBase::Default_Pri;
+}
+
+/**
  * Instantiate a tlm memory manager that takes care about all the
  * tlm transactions in the system.
  */
@@ -367,8 +386,9 @@
         // Accepted but is now blocking until END_REQ (exclusion rule).
         blockingRequest = trans;
         auto cb = [this, trans, phase]() { pec(*trans, phase); };
-        system->schedule(new EventFunctionWrapper(cb, "pec", true),
-                         curTick() + delay.value());
+        auto event = new EventFunctionWrapper(
+                cb, "pec", true, getPriorityOfTlmPhase(phase));
+        system->schedule(event, curTick() + delay.value());
     } else if (status == tlm::TLM_COMPLETED) {
         // Transaction is over nothing has do be done.
         sc_assert(phase == tlm::END_RESP);
@@ -442,8 +462,9 @@
     tlm::tlm_phase &phase, sc_core::sc_time &delay)
 {
     auto cb = [this, &trans, phase]() { pec(trans, phase); };
-    system->schedule(new EventFunctionWrapper(cb, "pec", true),
-                     curTick() + delay.value());
+    auto event = new EventFunctionWrapper(
+            cb, "pec", true, getPriorityOfTlmPhase(phase));
+    system->schedule(event, curTick() + delay.value());
     return tlm::TLM_ACCEPTED;
 }


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/44305
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: Ic33a92162c8c53af3887c7b04090115a38f96866
Gerrit-Change-Number: 44305
Gerrit-PatchSet: 5
Gerrit-Owner: Jui-min Lee <f...@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-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to