arch: Use a fault to trigger system calls in SE mode.
authorGabe Black <gabeblack@google.com>
Mon, 24 Aug 2020 08:36:53 +0000 (01:36 -0700)
committerGabe Black <gabeblack@google.com>
Thu, 3 Sep 2020 10:07:15 +0000 (10:07 +0000)
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 <gabeblack@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
src/arch/mips/isa/decoder.isa
src/arch/power/isa/decoder.isa
src/arch/x86/isa/decoder/one_byte_opcodes.isa
src/arch/x86/isa/decoder/two_byte_opcodes.isa
src/sim/faults.cc
src/sim/faults.hh

index 6e19f85d81b9c20091299344e4a58519710e0d6b..d35fc1865f291f502968698b2108421cd4e416b1 100644 (file)
@@ -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<SystemCallFault>(); }});
+                        0: syscall_se({{
+                            fault = std::make_shared<SESyscallFault>();
+                        }});
+                      default: syscall({{
+                            fault = std::make_shared<SystemCallFault>();
+                        }});
                     }
                     0x7: sync({{ ; }}, IsMemBarrier);
                   0x5: break({{fault = std::make_shared<BreakpointFault>();}});
index 2e88aea7f2bdd10a1f81e7f5e12ae01ade252fbe..b7b9afffc04523b966803882995e40ca43f30e5d 100644 (file)
@@ -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<SESyscallFault>(); }});
 
     format FloatArithOp {
         59: decode A_XO {
index b1b6218be09a54f252dce3131b8e22f0dfedb902..b5f77cd31e7ce2717da506efdd2e50585969ab89 100644 (file)
                         // 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<SESyscallFault>();
+                            }});
                     }
 
                     default: Inst::INT(Ib);
index 0dec25b93034621813f7707eae6f87b4e19f19bb..5d4514405d612edc47975544a2b4851423b880fb 100644 (file)
                 }
             }
             0x05: decode FullSystemInt {
-                0: SyscallInst::syscall('xc->syscall()',
-                                        IsSyscall, IsNonSpeculative,
-                                        IsSerializeAfter);
+                0: SyscallInst::syscall({{
+                    return std::make_shared<SESyscallFault>();
+                }});
                 default: decode MODE_MODE {
                     0x0: decode MODE_SUBMODE {
                         0x0: Inst::SYSCALL_64();
             0x2: Inst::RDMSR();
             0x3: rdpmc();
             0x4: decode FullSystemInt {
-                0: SyscallInst::sysenter('xc->syscall()',
-                                         IsSyscall, IsNonSpeculative,
-                                         IsSerializeAfter);
+                0: SyscallInst::sysenter({{
+                    return std::make_shared<SESyscallFault>();
+                }});
                 default: sysenter();
             }
             0x5: sysexit();
index 9bbc11930d06094e420f8ca5a2d158dd984482e5..b6468ea4bdfad6b5891d4b9d9ed28095bf0f7b84 100644 (file)
@@ -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)
 {
index 4b5c1a52fdee180ac2e74f5a2e352531aac13491..62817f025ea085b679e11bb78e1e3ffd5e124fa8 100644 (file)
@@ -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: