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>

diff --git a/src/arch/x86/pagetable_walker.cc b/src/arch/x86/pagetable_walker.cc
index 932eb8eefbe15a56dbeff52f0dd6c7d8b9905bc7..86f140fdc67b1066629c07ee8e6914975b94a954 100644 (file)
--- 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()
 {
@@ -696,6 +713,12 @@ Walker::WalkerState::wasStarted()
     return started;
 }
 
+void
+Walker::WalkerState::squash()
+{
+    squashed = true;
+}
+
 void
 Walker::WalkerState::retry()
 {

diff --git a/src/arch/x86/pagetable_walker.hh b/src/arch/x86/pagetable_walker.hh
index 88b8147cf947fecea2daa155eac6e6aa58090c03..73e892471ff7bf6f78cda09ae05a0c61d0503b64 100644 (file)
--- 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: