From 7aa9591151b10036d77e19d098638129ca88c174 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Mon, 24 Aug 2020 01:36:53 -0700 Subject: [PATCH] arch: Use a fault to trigger system calls in SE mode. When the system call happens during the execution of the system call instruction, it can be ambiguous what state takes precedence, the state update from the instruction or the system call. These may be tracked differently and found in an unpredictable order in, for example, the O3 CPU. An instruction can avoid updating any state explicitly, but implicitly updated state (specifically the PC) will always update, whether the instruction wants it to or not. If the system call can be deferred by using a Fault object, then it's no longer ambiguous. The PC update will be discarded, and the system call can set the PC however it likes. Because there is no implicit PC update, the PC needs to be walked forward, either to what it would have been anyway, or to what the system call set in NPC. In addition, because of the existing semantics around handling Faults, the instruction no longer needs to be marked as serializing, non-speculative, etc. The "normal", aka architectural, aka FS version of the system call instructions don't return a Fault artificially. Change-Id: I72011a16a89332b1dcfb01c79f2f0d75c55ab773 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/33281 Maintainer: Gabe Black Tested-by: kokoro Reviewed-by: Andreas Sandberg --- src/arch/mips/isa/decoder.isa | 9 ++++++--- src/arch/power/isa/decoder.isa | 3 +-- src/arch/x86/isa/decoder/one_byte_opcodes.isa | 6 +++--- src/arch/x86/isa/decoder/two_byte_opcodes.isa | 12 ++++++------ src/sim/faults.cc | 10 ++++++++++ src/sim/faults.hh | 9 +++++++++ 6 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/arch/mips/isa/decoder.isa b/src/arch/mips/isa/decoder.isa index 6e19f85d8..d35fc1865 100644 --- a/src/arch/mips/isa/decoder.isa +++ b/src/arch/mips/isa/decoder.isa @@ -159,9 +159,12 @@ decode OPCODE_HI default Unknown::unknown() { 0x2: movz({{ Rd = (Rt == 0) ? Rs : Rd; }}); 0x3: movn({{ Rd = (Rt != 0) ? Rs : Rd; }}); 0x4: decode FullSystemInt { - 0: syscall_se({{ xc->syscall(); }}, - IsSerializeAfter, IsNonSpeculative); - default: syscall({{ fault = std::make_shared(); }}); + 0: syscall_se({{ + fault = std::make_shared(); + }}); + default: syscall({{ + fault = std::make_shared(); + }}); } 0x7: sync({{ ; }}, IsMemBarrier); 0x5: break({{fault = std::make_shared();}}); diff --git a/src/arch/power/isa/decoder.isa b/src/arch/power/isa/decoder.isa index 2e88aea7f..b7b9afffc 100644 --- a/src/arch/power/isa/decoder.isa +++ b/src/arch/power/isa/decoder.isa @@ -515,8 +515,7 @@ decode OPCODE default Unknown::unknown() { 55: stfdu({{ Mem_df = Fs; }}); } - 17: IntOp::sc({{ xc->syscall(); }}, - [ IsSyscall, IsNonSpeculative, IsSerializeAfter ]); + 17: IntOp::sc({{ return std::make_shared(); }}); format FloatArithOp { 59: decode A_XO { diff --git a/src/arch/x86/isa/decoder/one_byte_opcodes.isa b/src/arch/x86/isa/decoder/one_byte_opcodes.isa index b1b6218be..b5f77cd31 100644 --- a/src/arch/x86/isa/decoder/one_byte_opcodes.isa +++ b/src/arch/x86/isa/decoder/one_byte_opcodes.isa @@ -398,9 +398,9 @@ // will sign extend it, and there's no easy way to // specify only checking the first byte. 0xffffffffffffff80: - SyscallInst::int80('xc->syscall()', - IsSyscall, IsNonSpeculative, - IsSerializeAfter); + SyscallInst::int80({{ + return std::make_shared(); + }}); } default: Inst::INT(Ib); diff --git a/src/arch/x86/isa/decoder/two_byte_opcodes.isa b/src/arch/x86/isa/decoder/two_byte_opcodes.isa index 0dec25b93..5d4514405 100644 --- a/src/arch/x86/isa/decoder/two_byte_opcodes.isa +++ b/src/arch/x86/isa/decoder/two_byte_opcodes.isa @@ -237,9 +237,9 @@ } } 0x05: decode FullSystemInt { - 0: SyscallInst::syscall('xc->syscall()', - IsSyscall, IsNonSpeculative, - IsSerializeAfter); + 0: SyscallInst::syscall({{ + return std::make_shared(); + }}); default: decode MODE_MODE { 0x0: decode MODE_SUBMODE { 0x0: Inst::SYSCALL_64(); @@ -431,9 +431,9 @@ 0x2: Inst::RDMSR(); 0x3: rdpmc(); 0x4: decode FullSystemInt { - 0: SyscallInst::sysenter('xc->syscall()', - IsSyscall, IsNonSpeculative, - IsSerializeAfter); + 0: SyscallInst::sysenter({{ + return std::make_shared(); + }}); default: sysenter(); } 0x5: sysexit(); diff --git a/src/sim/faults.cc b/src/sim/faults.cc index 9bbc11930..b6468ea4b 100644 --- a/src/sim/faults.cc +++ b/src/sim/faults.cc @@ -50,6 +50,16 @@ UnimpFault::invoke(ThreadContext *tc, const StaticInstPtr &inst) panic("Unimpfault: %s", panicStr.c_str()); } +void +SESyscallFault::invoke(ThreadContext *tc, const StaticInstPtr &inst) +{ + tc->syscall(); + // Move the PC forward since that doesn't happen automatically. + TheISA::PCState pc = tc->pcState(); + inst->advancePC(pc); + tc->pcState(pc); +} + void ReExec::invoke(ThreadContext *tc, const StaticInstPtr &inst) { diff --git a/src/sim/faults.hh b/src/sim/faults.hh index 4b5c1a52f..62817f025 100644 --- a/src/sim/faults.hh +++ b/src/sim/faults.hh @@ -64,6 +64,15 @@ class UnimpFault : public FaultBase StaticInst::nullStaticInstPtr) override; }; +// A fault to trigger a system call in SE mode. +class SESyscallFault : public FaultBase +{ + const char *name() const override { return "syscall_fault"; } + + void invoke(ThreadContext *tc, const StaticInstPtr &inst= + StaticInst::nullStaticInstPtr) override; +}; + class ReExec : public FaultBase { public: -- 2.30.2