Hello Timothy Hayes,

I'd like you to do a code review. Please visit

    https://gem5-review.googlesource.com/c/public/gem5/+/30318

to review the following change.


Change subject: mem: Add HTM fields to the Packet object
......................................................................

mem: Add HTM fields to the Packet object

JIRA: https://gem5.atlassian.net/browse/GEM5-587

Change-Id: I39268825327f2387ca7e622093fdb42c24a6c82c
---
M src/mem/SConscript
A src/mem/htm.cc
A src/mem/htm.hh
M src/mem/packet.cc
M src/mem/packet.hh
5 files changed, 243 insertions(+), 6 deletions(-)



diff --git a/src/mem/SConscript b/src/mem/SConscript
index b77dbb1..0187c09 100644
--- a/src/mem/SConscript
+++ b/src/mem/SConscript
@@ -1,6 +1,6 @@
 # -*- mode:python -*-
 #
-# Copyright (c) 2018 ARM Limited
+# Copyright (c) 2018-2019 ARM Limited
 # All rights reserved
 #
 # The license below extends only to copyright in the software and shall
@@ -77,6 +77,7 @@
 Source('tport.cc')
 Source('xbar.cc')
 Source('hmc_controller.cc')
+Source('htm.cc')
 Source('serial_link.cc')
 Source('mem_delay.cc')

@@ -108,6 +109,7 @@
 DebugFlag('DRAMPower')
 DebugFlag('DRAMState')
 DebugFlag('ExternalPort')
+DebugFlag('HtmMem', 'Hardware Transactional Memory (Mem side)')
 DebugFlag('LLSC')
 DebugFlag('MMU')
 DebugFlag('MemoryAccess')
diff --git a/src/mem/htm.cc b/src/mem/htm.cc
new file mode 100644
index 0000000..dc01e6d
--- /dev/null
+++ b/src/mem/htm.cc
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2020 ARM Limited
+ * All rights reserved
+ *
+ * The license below extends only to copyright in the software and shall
+ * not be construed as granting a license to any other intellectual
+ * property including but not limited to intellectual property relating
+ * to a hardware implementation of the functionality of the software
+ * licensed hereunder.  You may use the software subject to the license
+ * terms below provided that you ensure that this notice is replicated
+ * unmodified and in its entirety in all distributions of the software,
+ * modified or unmodified, in source code or in binary form.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "mem/htm.hh"
+
+std::string
+htmFailureToStr(HtmCacheFailure rc)
+{
+    static const std::map<HtmCacheFailure, std::string> rc_to_string = {
+        { HtmCacheFailure::NO_FAIL, "NO_FAIL" },
+        { HtmCacheFailure::FAIL_SELF, "FAIL_SELF" },
+        { HtmCacheFailure::FAIL_REMOTE, "FAIL_REMOTE" },
+        { HtmCacheFailure::FAIL_OTHER, "FAIL_OTHER" }
+    };
+
+    auto it = rc_to_string.find(rc);
+    return it == rc_to_string.end() ? "Unrecognized Failure" : it->second;
+}
diff --git a/src/mem/htm.hh b/src/mem/htm.hh
new file mode 100644
index 0000000..e60632b
--- /dev/null
+++ b/src/mem/htm.hh
@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2020 ARM Limited
+ * All rights reserved
+ *
+ * The license below extends only to copyright in the software and shall
+ * not be construed as granting a license to any other intellectual
+ * property including but not limited to intellectual property relating
+ * to a hardware implementation of the functionality of the software
+ * licensed hereunder.  You may use the software subject to the license
+ * terms below provided that you ensure that this notice is replicated
+ * unmodified and in its entirety in all distributions of the software,
+ * modified or unmodified, in source code or in binary form.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __MEM_HTM_HH__
+#define __MEM_HTM_HH__
+
+#include <map>
+#include <string>
+
+enum class HtmCacheFailure
+{
+    NO_FAIL,     // no failure in cache
+    FAIL_SELF,   // failed due local cache's replacement policy
+    FAIL_REMOTE, // failed due remote invalidation
+    FAIL_OTHER,  // failed due other circumstances
+};
+
+/** Convert enum into string to be used for debug purposes */
+std::string htmFailureToStr(HtmCacheFailure rc);
+
+#endif // __MEM_HTM_HH__
diff --git a/src/mem/packet.cc b/src/mem/packet.cc
index 1c1da21..9903649 100644
--- a/src/mem/packet.cc
+++ b/src/mem/packet.cc
@@ -222,7 +222,11 @@
       InvalidateResp, "InvalidateReq" },
     /* Invalidation Response */
     { SET2(IsInvalidate, IsResponse),
-      InvalidCmd, "InvalidateResp" }
+      InvalidCmd, "InvalidateResp" },
+      // hardware transactional memory
+    { SET3(IsRead, IsRequest, NeedsResponse), HTMReqResp, "HTMReq" },
+    { SET2(IsRead, IsResponse), InvalidCmd, "HTMReqResp" },
+    { SET2(IsRead, IsRequest), InvalidCmd, "HTMAbort" },
 };

 AddrRange
@@ -481,3 +485,73 @@
     printLabels();
     obj->print(os, verbosity, curPrefix());
 }
+
+// hardware transactional memory
+// Communicates to the core that a packet was processed by the memory
+// subsystem while running in transactional mode.
+// It may happen that the transaction has failed at the memory subsystem
+// and this needs to be communicated to the core somehow.
+// This function decorates the response packet with flags to indicate
+// such a situation has occurred.
+void
+Packet::makeHtmTransactionalReqResponse(
+    const HtmCacheFailure htm_return_code)
+{
+    assert(needsResponse());
+    assert(isRequest());
+
+    cmd = cmd.responseCommand();
+
+    setHtmTransactionFailedInCache(htm_return_code);
+
+    // responses are never express, even if the snoop that
+    // triggered them was
+    flags.clear(EXPRESS_SNOOP);
+}
+
+// Indicates that this packet/request has returned from the
+// cache hierarchy in a failed transaction. The core is
+// notified like this.
+
+void
+Packet::setHtmTransactionFailedInCache(
+    const HtmCacheFailure htm_return_code)
+{
+    if (htm_return_code != HtmCacheFailure::NO_FAIL)
+        flags.set(FAILS_TRANSACTION);
+
+    htmFailRc = htm_return_code;
+}
+
+bool
+Packet::htmTransactionFailedInCache() const
+{
+    return flags.isSet(FAILS_TRANSACTION);
+}
+
+HtmCacheFailure
+Packet::getHtmTransactionFailedInCacheRC() const
+{
+    assert(htmTransactionFailedInCache());
+    return htmFailRc;
+}
+
+void
+Packet::setHtmTransactional(uint64_t htm_uid)
+{
+    flags.set(FROM_TRANSACTION);
+    htmTransactionUid = htm_uid;
+}
+
+bool
+Packet::isHtmTransactional() const
+{
+    return flags.isSet(FROM_TRANSACTION);
+}
+
+uint64_t
+Packet::getHtmTransactionUid() const
+{
+    assert(flags.isSet(FROM_TRANSACTION));
+    return htmTransactionUid;
+}
diff --git a/src/mem/packet.hh b/src/mem/packet.hh
index 42d286a..26c21c8 100644
--- a/src/mem/packet.hh
+++ b/src/mem/packet.hh
@@ -58,6 +58,7 @@
 #include "base/logging.hh"
 #include "base/printable.hh"
 #include "base/types.hh"
+#include "mem/htm.hh"
 #include "mem/request.hh"
 #include "sim/core.hh"

@@ -130,6 +131,10 @@
         FlushReq,      //request for a cache flush
         InvalidateReq,   // request for address to be invalidated
         InvalidateResp,
+        // hardware transactional memory
+        HTMReq,
+        HTMReqResp,
+        HTMAbort,
         NUM_MEM_CMDS
     };

@@ -258,7 +263,7 @@

     enum : FlagsType {
         // Flags to transfer across when copying a packet
-        COPY_FLAGS             = 0x0000003F,
+        COPY_FLAGS             = 0x000000FF,

         // Flags that are used to create reponse packets
         RESPONDER_FLAGS        = 0x00000009,
@@ -288,6 +293,14 @@
         // operations
         SATISFIED              = 0x00000020,

+        // hardware transactional memory
+        // Indicates that this packet/request has returned from the
+        // cache hierarchy in a failed transaction. The core is
+        // notified like this.
+        FAILS_TRANSACTION      = 0x00000040,
+
+        FROM_TRANSACTION       = 0x00000080,
+
         /// Are the 'addr' and 'size' fields valid?
         VALID_ADDR             = 0x00000100,
         VALID_SIZE             = 0x00000200,
@@ -350,6 +363,10 @@
     // Quality of Service priority value
     uint8_t _qosValue;

+    // hardware transactional memory
+    HtmCacheFailure htmFailRc;
+    uint64_t htmTransactionUid;
+
   public:

     /**
@@ -792,14 +809,22 @@
     Packet(const RequestPtr &_req, MemCmd _cmd)
         :  cmd(_cmd), id((PacketId)_req.get()), req(_req),
            data(nullptr), addr(0), _isSecure(false), size(0),
-           _qosValue(0), headerDelay(0), snoopDelay(0),
+           _qosValue(0),
+           htmFailRc(HtmCacheFailure::NO_FAIL),
+           htmTransactionUid(0),
+           headerDelay(0), snoopDelay(0),
            payloadDelay(0), senderState(NULL)
     {
+        flags.clear();
         if (req->hasPaddr()) {
             addr = req->getPaddr();
             flags.set(VALID_ADDR);
             _isSecure = req->isSecure();
         }
+        // hardware transactional memory
+        if (req->isHTMCmd()) {
+            flags.set(VALID_ADDR);
+        }
         if (req->hasSize()) {
             size = req->getSize();
             flags.set(VALID_SIZE);
@@ -814,9 +839,13 @@
Packet(const RequestPtr &_req, MemCmd _cmd, int _blkSize, PacketId _id = 0)
         :  cmd(_cmd), id(_id ? _id : (PacketId)_req.get()), req(_req),
            data(nullptr), addr(0), _isSecure(false),
-           _qosValue(0), headerDelay(0),
+           _qosValue(0),
+           htmFailRc(HtmCacheFailure::NO_FAIL),
+           htmTransactionUid(0),
+           headerDelay(0),
            snoopDelay(0), payloadDelay(0), senderState(NULL)
     {
+        flags.clear();
         if (req->hasPaddr()) {
             addr = req->getPaddr() & ~(_blkSize - 1);
             flags.set(VALID_ADDR);
@@ -839,6 +868,8 @@
            addr(pkt->addr), _isSecure(pkt->_isSecure), size(pkt->size),
            bytesValid(pkt->bytesValid),
            _qosValue(pkt->qosValue()),
+           htmFailRc(HtmCacheFailure::NO_FAIL),
+           htmTransactionUid(0),
            headerDelay(pkt->headerDelay),
            snoopDelay(0),
            payloadDelay(pkt->payloadDelay),
@@ -849,6 +880,15 @@

         flags.set(pkt->flags & (VALID_ADDR|VALID_SIZE));

+        if (pkt->isHtmTransactional())
+            setHtmTransactional(pkt->getHtmTransactionUid());
+
+        if (pkt->htmTransactionFailedInCache()) {
+            setHtmTransactionFailedInCache(
+                pkt->getHtmTransactionFailedInCacheRC()
+            );
+        }
+
         // 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
@@ -872,7 +912,12 @@
     static MemCmd
     makeReadCmd(const RequestPtr &req)
     {
-        if (req->isLLSC())
+        if (req->isHTMCmd()) {
+            if (req->isHTMAbort())
+                return MemCmd::HTMAbort;
+            else
+                return MemCmd::HTMReq;
+        } else if (req->isLLSC())
             return MemCmd::LoadLockedReq;
         else if (req->isPrefetchEx())
             return MemCmd::SoftPFExReq;
@@ -1345,6 +1390,15 @@
      * @return string with the request's type and start<->end addresses
      */
     std::string print() const;
+
+    // hardware transactional memory
+    void makeHtmTransactionalReqResponse(const HtmCacheFailure ret_code);
+    void setHtmTransactional(uint64_t val);
+    bool isHtmTransactional() const;
+    uint64_t getHtmTransactionUid() const;
+    void setHtmTransactionFailedInCache(const HtmCacheFailure ret_code);
+    bool htmTransactionFailedInCache() const;
+    HtmCacheFailure getHtmTransactionFailedInCacheRC() const;
 };

 #endif //__MEM_PACKET_HH

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30318
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: I39268825327f2387ca7e622093fdb42c24a6c82c
Gerrit-Change-Number: 30318
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Timothy Hayes <timothy.ha...@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