Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/54204 )
Change subject: arch,sim-se: Handle syscall retry/suppression in the
syscall desc.
......................................................................
arch,sim-se: Handle syscall retry/suppression in the syscall desc.
Rather than make each ISA include boilerplate to ignore a
SyscallReturn's value when it's marked as suppressed or needing a retry,
put that code into the SyscallDesc::doSyscall method instead.
That has two benefits. First, it removes a decent amount of code
duplication which is nice from a maintenance perspective. Second, it
puts the SyscallDesc in charge of figuring out what to do once a system
call implementation finishes. That will let it schedule a retry of the
system call for instance, without worrying about what the ISA is doing
with the SyscallReturn behind its back.
Jira Issue: https://gem5.atlassian.net/browse/GEM5-1123
Change-Id: I76760cba75fd23e6e3357f6169c0140bee3f01b6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/54204
Reviewed-by: Giacomo Travaglini <giacomo.travagl...@arm.com>
Maintainer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/arch/sparc/se_workload.hh
M src/arch/x86/linux/linux.hh
M src/arch/arm/freebsd/se_workload.hh
M src/arch/riscv/se_workload.hh
M src/arch/arm/linux/se_workload.hh
M src/arch/mips/se_workload.hh
M src/sim/syscall_desc.cc
M src/sim/syscall_desc.hh
M src/arch/power/se_workload.hh
9 files changed, 32 insertions(+), 25 deletions(-)
Approvals:
Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/freebsd/se_workload.hh
b/src/arch/arm/freebsd/se_workload.hh
index 8069bd2..4ec5090 100644
--- a/src/arch/arm/freebsd/se_workload.hh
+++ b/src/arch/arm/freebsd/se_workload.hh
@@ -82,9 +82,6 @@
static void
store(ThreadContext *tc, const SyscallReturn &ret)
{
- if (ret.suppressed() || ret.needsRetry())
- return;
-
RegVal val;
if (ret.successful()) {
tc->setCCReg(ArmISA::CCREG_C, 0);
diff --git a/src/arch/arm/linux/se_workload.hh
b/src/arch/arm/linux/se_workload.hh
index b22688f..f0af647 100644
--- a/src/arch/arm/linux/se_workload.hh
+++ b/src/arch/arm/linux/se_workload.hh
@@ -74,9 +74,6 @@
static void
store(ThreadContext *tc, const SyscallReturn &ret)
{
- if (ret.suppressed() || ret.needsRetry())
- return;
-
tc->setIntReg(ArmISA::ReturnValueReg, ret.encodedValue());
if (ret.count() > 1)
tc->setIntReg(ArmISA::SyscallPseudoReturnReg, ret.value2());
diff --git a/src/arch/mips/se_workload.hh b/src/arch/mips/se_workload.hh
index 178093d..c10ceb0 100644
--- a/src/arch/mips/se_workload.hh
+++ b/src/arch/mips/se_workload.hh
@@ -77,9 +77,6 @@
static void
store(ThreadContext *tc, const SyscallReturn &ret)
{
- if (ret.suppressed() || ret.needsRetry())
- return;
-
if (ret.successful()) {
// no error
tc->setIntReg(MipsISA::SyscallSuccessReg, 0);
diff --git a/src/arch/power/se_workload.hh b/src/arch/power/se_workload.hh
index 2b62795..c351d65 100644
--- a/src/arch/power/se_workload.hh
+++ b/src/arch/power/se_workload.hh
@@ -77,9 +77,6 @@
static void
store(ThreadContext *tc, const SyscallReturn &ret)
{
- if (ret.suppressed() || ret.needsRetry())
- return;
-
PowerISA::Cr cr = tc->readIntReg(PowerISA::INTREG_CR);
if (ret.successful()) {
cr.cr0.so = 0;
diff --git a/src/arch/riscv/se_workload.hh b/src/arch/riscv/se_workload.hh
index 1f8a067..484803e 100644
--- a/src/arch/riscv/se_workload.hh
+++ b/src/arch/riscv/se_workload.hh
@@ -75,9 +75,6 @@
static void
store(ThreadContext *tc, const SyscallReturn &ret)
{
- if (ret.suppressed() || ret.needsRetry())
- return;
-
if (ret.successful()) {
// no error
tc->setIntReg(RiscvISA::ReturnValueReg, ret.returnValue());
diff --git a/src/arch/sparc/se_workload.hh b/src/arch/sparc/se_workload.hh
index 6d034f7..18988fe 100644
--- a/src/arch/sparc/se_workload.hh
+++ b/src/arch/sparc/se_workload.hh
@@ -89,9 +89,6 @@
static void
store(ThreadContext *tc, const SyscallReturn &ret)
{
- if (ret.suppressed() || ret.needsRetry())
- return;
-
// check for error condition. SPARC syscall convention is to
// indicate success/failure in reg the carry bit of the ccr
// and put the return value itself in the standard return value
reg.
diff --git a/src/arch/x86/linux/linux.hh b/src/arch/x86/linux/linux.hh
index cbf7358..928c580 100644
--- a/src/arch/x86/linux/linux.hh
+++ b/src/arch/x86/linux/linux.hh
@@ -86,9 +86,6 @@
static void
store(ThreadContext *tc, const SyscallReturn &ret)
{
- if (ret.suppressed() || ret.needsRetry())
- return;
-
tc->setIntReg(X86ISA::INTREG_RAX, ret.encodedValue());
}
};
diff --git a/src/sim/syscall_desc.cc b/src/sim/syscall_desc.cc
index 7499102..7427f41 100644
--- a/src/sim/syscall_desc.cc
+++ b/src/sim/syscall_desc.cc
@@ -44,12 +44,14 @@
SyscallReturn retval = executor(this, tc);
- if (retval.needsRetry())
+ if (retval.needsRetry()) {
DPRINTF_SYSCALL(Base, "Needs retry.\n", name());
- else if (retval.suppressed())
+ } else if (retval.suppressed()) {
DPRINTF_SYSCALL(Base, "No return value.\n", name());
- else
+ } else {
+ returnInto(tc, retval);
DPRINTF_SYSCALL(Base, "Returned %d.\n", retval.encodedValue());
+ }
}
} // namespace gem5
diff --git a/src/sim/syscall_desc.hh b/src/sim/syscall_desc.hh
index 0740632..6ded904 100644
--- a/src/sim/syscall_desc.hh
+++ b/src/sim/syscall_desc.hh
@@ -141,7 +141,7 @@
// Use invokeSimcall to gather the other arguments based on the
// given ABI and pass them to the syscall implementation.
- return invokeSimcall<ABI, SyscallReturn, Args...>(tc,
+ return invokeSimcall<ABI, false, SyscallReturn, Args...>(tc,
std::function<SyscallReturn(ThreadContext *, Args...)>(
partial));
};
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/54204
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-2
Gerrit-Change-Id: I76760cba75fd23e6e3357f6169c0140bee3f01b6
Gerrit-Change-Number: 54204
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Bobby Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
