Matthew Poremba has submitted this change. (
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
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/19568
Reviewed-by: Jason Lowe-Power
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/arch/x86/pagetable_walker.cc
M src/arch/x86/pagetable_walker.hh
2 files changed, 29 insertions(+), 3 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
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("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: 2
Gerrit-Owner: Matthew Poremba
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Matthew Poremba
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev
