Giacomo Travaglini has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/50147 )

Change subject: cpu: Fix TME for dyn_o3_cpu
......................................................................

cpu: Fix TME for dyn_o3_cpu

Commit c417b76 changed the behaviour of addRequest(),
but did not update documentation or the HTM-related logic that used it.

Updates documentation for addRequest() in light of c417b76,
refactors request class to be idiomatic and use assigned byteEnable,
made HTM cmds pass in a correct byteEnable.

Change-Id: I7aa8c127df896e81caf915fbfea93e7b4bcc53b7
---
M src/cpu/o3/dyn_inst.cc
M src/cpu/o3/lsq.cc
M src/cpu/o3/lsq.hh
3 files changed, 27 insertions(+), 26 deletions(-)



diff --git a/src/cpu/o3/dyn_inst.cc b/src/cpu/o3/dyn_inst.cc
index 1bd37cc..edbaa04 100644
--- a/src/cpu/o3/dyn_inst.cc
+++ b/src/cpu/o3/dyn_inst.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2011 ARM Limited
+ * Copyright (c) 2010-2011, 2021 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -301,9 +301,11 @@
 Fault
 DynInst::initiateHtmCmd(Request::Flags flags)
 {
+    const unsigned int size = 8;
     return cpu->pushRequest(
             dynamic_cast<DynInstPtr::PtrType>(this),
-            /* ld */ true, nullptr, 8, 0x0ul, flags, nullptr, nullptr);
+            /* ld */ true, nullptr, size, 0x0ul, flags, nullptr, nullptr,
+            std::vector<bool>(size, true));
 }

 Fault
diff --git a/src/cpu/o3/lsq.cc b/src/cpu/o3/lsq.cc
index 359b54d..110c58a 100644
--- a/src/cpu/o3/lsq.cc
+++ b/src/cpu/o3/lsq.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011-2012, 2014, 2017-2019 ARM Limited
+ * Copyright (c) 2011-2012, 2014, 2017-2019, 2021 ARM Limited
  * Copyright (c) 2013 Advanced Micro Devices, Inc.
  * All rights reserved
  *
@@ -1374,6 +1374,16 @@
     SingleDataRequest(port, inst, true, 0x0lu, 8, flags_,
         nullptr, nullptr, nullptr)
 {
+}
+
+void
+LSQ::HtmCmdRequest::initiateTranslation()
+{
+    // Special commands are implemented as loads to avoid significant
+    // changes to the cpu and memory interfaces
+    // The virtual and physical address uses a dummy value of 0x00
+    // Address translation does not really occur thus the code below
+
     assert(_requests.size() == 0);

     addRequest(_addr, _size, _byteEnable);
@@ -1390,34 +1400,23 @@
         _inst->memReqFlags = _requests.back()->getFlags();
         _inst->savedReq = this;

-        setState(State::Translation);
+        flags.set(Flag::TranslationStarted);
+        flags.set(Flag::TranslationFinished);
+
+        _inst->translationStarted(true);
+        _inst->translationCompleted(true);
+
+        setState(State::Request);
     } else {
-        panic("unexpected behaviour");
+        panic("unexpected behaviour in initiateTranslation()");
     }
 }

 void
-LSQ::HtmCmdRequest::initiateTranslation()
-{
-    // Transaction commands are implemented as loads to avoid significant
-    // changes to the cpu and memory interfaces
-    // The virtual and physical address uses a dummy value of 0x00
-    // Address translation does not really occur thus the code below
-
-    flags.set(Flag::TranslationStarted);
-    flags.set(Flag::TranslationFinished);
-
-    _inst->translationStarted(true);
-    _inst->translationCompleted(true);
-
-    setState(State::Request);
-}
-
-void
 LSQ::HtmCmdRequest::finish(const Fault &fault, const RequestPtr &req,
         gem5::ThreadContext* tc, BaseMMU::Mode mode)
 {
-    panic("unexpected behaviour");
+    panic("unexpected behaviour - finish()");
 }

 Fault
diff --git a/src/cpu/o3/lsq.hh b/src/cpu/o3/lsq.hh
index e5e519a..f843e64 100644
--- a/src/cpu/o3/lsq.hh
+++ b/src/cpu/o3/lsq.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011-2012, 2014, 2018-2019 ARM Limited
+ * Copyright (c) 2011-2012, 2014, 2018-2019, 2021 ARM Limited
  * Copyright (c) 2013 Advanced Micro Devices, Inc.
  * All rights reserved
  *
@@ -362,8 +362,8 @@
         /** Helper function used to add a (sub)request, given its address
          * `addr`, size `size` and byte-enable mask `byteEnable`.
          *
-         * The request is only added if the mask is empty or if there is at
-         * least an active element in it.
+         * The request is only added if there is at least one active
+         * element in the mask.
          */
         void addRequest(Addr addr, unsigned size,
                 const std::vector<bool>& byte_enable);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/50147
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: I7aa8c127df896e81caf915fbfea93e7b4bcc53b7
Gerrit-Change-Number: 50147
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-MessageType: newchange
_______________________________________________
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