Ciro Santilli has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/29777 )
Change subject: sim-se: don't wake up SE futex syscalls on ARM events
......................................................................
sim-se: don't wake up SE futex syscalls on ARM events
Before this commit:
* SEV events were not waking neither WFE (wrong) nor futex WAIT (correct)
* locked memory events (LLSC) due to LDXR and STXR were waking up both
WFE (correct) and futex WAIT (wrong)
This commit fixes all wrong behaviours mentioned above.
The fact that LLSC events were waking up futexes leads to deadlocks,
as shown in the test case described at:
https://gem5.atlassian.net/browse/GEM5-537
because threads woken up by SVE are not removed from the waiter list
for the futex address they are sleeping on.
A previous fix atttempt was done at:
1531b56d605d47252dc0620bb3e755b7cf84df97
in which only sleeping threads are woken up. But that is not sufficient,
because the futex sleeping thread that was being wrongly woken up on SEV
can start to sleep on a second futex. The deadlock chain of events looks
like this for thread1:
- sleep on futex1, waiter list for futex1 now contains thread1
- woken up by SEV from another thread, not removed from waiter list
(wrong behaviour made impossible by this patch)
- sleep on futex2
- another thread wakes up thread1 via futex1, since it was still found in
that waiter list and is in sleeping state
As a result, some other thread that was sleeping on futex1 misses the
wakeup which thread1 received instead, and we have deadlock.
JIRA: https://gem5.atlassian.net/browse/GEM5-622
Change-Id: Icec5e30b041f53e5aa3b6e0d291e77bc0e865984
---
M src/arch/arm/locked_mem.hh
M src/cpu/base.hh
M src/sim/futex_map.cc
M src/sim/futex_map.hh
4 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/src/arch/arm/locked_mem.hh b/src/arch/arm/locked_mem.hh
index ad52da4..a01b838 100644
--- a/src/arch/arm/locked_mem.hh
+++ b/src/arch/arm/locked_mem.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012-2013,2017 ARM Limited
+ * Copyright (c) 2012-2013,2017, 2020 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -50,6 +50,7 @@
#include "arch/arm/miscregs.hh"
#include "arch/arm/isa_traits.hh"
+#include "arch/arm/utility.hh"
#include "debug/LLSC.hh"
#include "mem/packet.hh"
#include "mem/request.hh"
@@ -80,8 +81,8 @@
xc->getCpuPtr()->name());
xc->setMiscReg(MISCREG_LOCKFLAG, false);
// Implement ARMv8 WFE/SEV semantics
+ sendEvent(xc);
xc->setMiscReg(MISCREG_SEV_MAILBOX, true);
- xc->getCpuPtr()->wakeup(xc->threadId());
}
}
@@ -155,8 +156,8 @@
DPRINTF(LLSC,"Clearing lock and signaling sev\n");
xc->setMiscReg(MISCREG_LOCKFLAG, false);
// Implement ARMv8 WFE/SEV semantics
+ sendEvent(xc);
xc->setMiscReg(MISCREG_SEV_MAILBOX, true);
- xc->getCpuPtr()->wakeup(xc->threadId());
}
} // namespace ArmISA
diff --git a/src/cpu/base.hh b/src/cpu/base.hh
index a33d1bd..0ad8cad 100644
--- a/src/cpu/base.hh
+++ b/src/cpu/base.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2011-2013, 2017 ARM Limited
+ * Copyright (c) 2011-2013, 2017, 2020 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -234,7 +234,10 @@
postInterrupt(ThreadID tid, int int_num, int index)
{
interrupts[tid]->post(int_num, index);
- if (FullSystem)
+ // Only wake up syscall emulation if it is not waiting on a futex.
+ // This is to model the fact that instructions such as ARM SEV
+ // should wake up a WFE sleep, but not a futex syscall WAIT.
+ if (FullSystem |
| !system->futexMap.is_waiting(threadContexts[tid]))
wakeup(tid);
}
diff --git a/src/sim/futex_map.cc b/src/sim/futex_map.cc
index f7dde9c..7dabaf2 100644
--- a/src/sim/futex_map.cc
+++ b/src/sim/futex_map.cc
@@ -82,11 +82,10 @@
// must only count threads that were actually
// woken up by this syscall.
auto& tc = waiterList.front().tc;
- if (tc->status() == ThreadContext::Suspended) {
- tc->activate();
- woken_up++;
- }
+ tc->activate();
+ woken_up++;
waiterList.pop_front();
+ waitingTcs.erase(tc);
}
if (waiterList.empty())
@@ -108,6 +107,7 @@
} else {
it->second.push_back(WaiterState(tc, bitmask));
}
+ waitingTcs.emplace(tc);
/** Suspend the thread context */
tc->suspend();
@@ -133,6 +133,7 @@
if (waiter.checkMask(bitmask)) {
waiter.tc->activate();
iter = waiterList.erase(iter);
+ waitingTcs.erase(waiter.tc);
woken_up++;
} else {
++iter;
@@ -188,3 +189,9 @@
return woken_up + requeued;
}
+
+bool
+FutexMap::is_waiting(ThreadContext *tc)
+{
+ return waitingTcs.find(tc) != waitingTcs.end();
+}
diff --git a/src/sim/futex_map.hh b/src/sim/futex_map.hh
index 025d324..e6cb27a 100644
--- a/src/sim/futex_map.hh
+++ b/src/sim/futex_map.hh
@@ -30,6 +30,7 @@
#define __FUTEX_MAP_HH__
#include <unordered_map>
+#include <unordered_set>
#include <cpu/thread_context.hh>
@@ -116,6 +117,16 @@
* requeued.
*/
int requeue(Addr addr1, uint64_t tgid, int count, int count2, Addr
addr2);
+
+ /**
+ * Determine if the given thread context is currently waiting on a
+ * futex wait operation.
+ */
+ bool is_waiting(ThreadContext *tc);
+
+ private:
+
+ std::unordered_set<ThreadContext *> waitingTcs;
};
#endif // __FUTEX_MAP_HH__
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/29777
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: Icec5e30b041f53e5aa3b6e0d291e77bc0e865984
Gerrit-Change-Number: 29777
Gerrit-PatchSet: 1
Gerrit-Owner: Ciro Santilli <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
