Matthew Poremba has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/19568 )

Change subject: arch-x86: Don't free PTW state with inflight requests
......................................................................

arch-x86: Don't free PTW state with inflight requests

If a page table walk is squashed, the walker state is being deleted
in the squash code. If there are in flight requests, the deleted
walker state values may be clobbered, leading to undefined behavior.
This adds a squashed boolean to the walker state which is set if a
walk is squashed while requests are still in flight. When packets
for the in flight request return, we check if the walk was squashed
and return that the walk is complete once the number of in flight
requests reaches zero. The walker state is then freed by the PTW.

Change-Id: I57a64b1548b83a8a9e8441fc9d6f33e9842df2b3
---
M src/arch/x86/pagetable_walker.cc
M src/arch/x86/pagetable_walker.hh
2 files changed, 29 insertions(+), 3 deletions(-)



diff --git a/src/arch/x86/pagetable_walker.cc b/src/arch/x86/pagetable_walker.cc
index 932eb8e..86f140f 100644
--- a/src/arch/x86/pagetable_walker.cc
+++ b/src/arch/x86/pagetable_walker.cc
@@ -205,8 +205,14 @@
             std::make_shared<UnimpFault>("Squashed Inst"),
             currState->req, currState->tc, currState->mode);

-        // delete the current request
-        delete currState;
+        // delete the current request if there are no inflight packets.
+        // if there is something in flight, delete when the packets are
+        // received and inflight is zero.
+        if (currState->numInflight() == 0) {
+            delete currState;
+        } else {
+            currState->squash();
+        }

         // check the next translation request, if it exists
         if (currStates.size())
@@ -597,6 +603,11 @@
     assert(inflight);
     assert(state == Waiting);
     inflight--;
+    if (squashed) {
+        // if were were squashed, return true once inflight is zero and
+        // this WalkerState will be freed there.
+        return (inflight == 0);
+    }
     if (pkt->isRead()) {
         // should not have a pending read it we also had one outstanding
         assert(!read);
@@ -678,6 +689,12 @@
     }
 }

+unsigned
+Walker::WalkerState::numInflight() const
+{
+    return inflight;
+}
+
 bool
 Walker::WalkerState::isRetrying()
 {
@@ -697,6 +714,12 @@
 }

 void
+Walker::WalkerState::squash()
+{
+    squashed = true;
+}
+
+void
 Walker::WalkerState::retry()
 {
     retrying = false;
diff --git a/src/arch/x86/pagetable_walker.hh b/src/arch/x86/pagetable_walker.hh
index 88b8147..73e8924 100644
--- a/src/arch/x86/pagetable_walker.hh
+++ b/src/arch/x86/pagetable_walker.hh
@@ -111,6 +111,7 @@
             bool timing;
             bool retrying;
             bool started;
+            bool squashed;
           public:
WalkerState(Walker * _walker, BaseTLB::Translation *_translation, const RequestPtr &_req, bool _isFunctional = false) :
@@ -118,7 +119,7 @@
                 nextState(Ready), inflight(0),
                 translation(_translation),
                 functional(_isFunctional), timing(false),
-                retrying(false), started(false)
+                retrying(false), started(false), squashed(false)
             {
             }
             void initState(ThreadContext * _tc, BaseTLB::Mode _mode,
@@ -126,10 +127,12 @@
             Fault startWalk();
             Fault startFunctional(Addr &addr, unsigned &logBytes);
             bool recvPacket(PacketPtr pkt);
+            unsigned numInflight() const;
             bool isRetrying();
             bool wasStarted();
             bool isTiming();
             void retry();
+            void squash();
             std::string name() const {return walker->name();}

           private:

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/19568
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: I57a64b1548b83a8a9e8441fc9d6f33e9842df2b3
Gerrit-Change-Number: 19568
Gerrit-PatchSet: 1
Gerrit-Owner: Matthew Poremba <matthew.pore...@amd.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to