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
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/19568
Reviewed-by: Jason Lowe-Power <jason@lowepower.com>
Maintainer: Jason Lowe-Power <jason@lowepower.com>
Tested-by: kokoro <noreply+kokoro@google.com>

2 files changed, 29 insertions, 3 deletions

diff --git a/src/arch/x86/pagetable_walker.cc b/src/arch/x86/pagetable_walker.cc
index 932eb8eef..86f140fdc 100644
--- a/src/arch/x86/pagetable_walker.cc
+++ b/src/arch/x86/pagetable_walker.cc
@@ -205,8 +205,14 @@ Walker::startWalkWrapper()
             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 @@ Walker::WalkerState::recvPacket(PacketPtr pkt)
     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 @@ Walker::WalkerState::sendPackets()
     }
 }
 
+unsigned
+Walker::WalkerState::numInflight() const
+{
+    return inflight;
+}
+
 bool
 Walker::WalkerState::isRetrying()
 {
@@ -697,6 +714,12 @@ Walker::WalkerState::wasStarted()
 }
 
 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 88b8147cf..73e892471 100644
--- a/src/arch/x86/pagetable_walker.hh
+++ b/src/arch/x86/pagetable_walker.hh
@@ -111,6 +111,7 @@ namespace X86ISA
             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 @@ namespace X86ISA
                 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 @@ namespace X86ISA
             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: