x86,sim: Remove special handling for KVM in the clone syscall.

When a gem5 op is triggered using a KVM MMIO exit event, the PC has
already been advanced beyond the offending instruction. Normally when
a system call or gem5 op is triggered, the PC has not advanced because
the instruction hasn't actually finished executing. This means that if
a gem5 op, and by extension a system call in SE mode, want to advance
the PC to the instruction after the gem5 op, they have to check whether
they were triggered from KVM.

To avoid having to special case these sorts of situations (currently
only in the clone system call), we can have the code which dispatches to
gem5 ops from KVM adjust the next PC so that it points to what the
current PC is. That way the PC can be advanced unconditionally, and will
point to the instruction after the one that triggered the call.

To be fully consistent, we would also need to adjust the current PC.
That would be non-trivial since we'd have to figure out where the
current instruction started, and that may not even be possible to
unambiguously determine given x86's instruction structure. Then we would
also need to restore the original PC to avoid confusing KVM.

Change-Id: I9ef90b2df8e27334dedc25c59eb45757f7220eea
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38486
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>
Tested-by: kokoro <noreply+kokoro@google.com>

diff --git a/src/cpu/kvm/base.cc b/src/cpu/kvm/base.cc
index b364d4f29d471bd27c6d0266c0994d0c543d402a..aac6aaf7de664811f9a10d169d91cb521314642c 100644 (file)
--- a/src/cpu/kvm/base.cc
+++ b/src/cpu/kvm/base.cc
@@ -1091,6 +1091,15 @@ BaseKvmCPU::doMMIOAccess(Addr paddr, void *data, int size, bool write)
     pkt->dataStatic(data);
 
     if (mmio_req->isLocalAccess()) {
+        // Since the PC has already been advanced by KVM, set the next
+        // PC to the current PC. KVM doesn't use that value, and that
+        // way any gem5 op or syscall which needs to know what the next
+        // PC is will be able to get a reasonable value.
+        //
+        // We won't be able to rewind the current PC to the "correct"
+        // value without figuring out how big the current instruction
+        // is, and that's probably not worth the effort.
+        tc->setNPC(tc->instAddr());
         // We currently assume that there is no need to migrate to a
         // different event queue when doing local accesses. Currently, they
         // are only used for m5ops, so it should be a valid assumption.

diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index 491a309b43a9081fcf88df486e872c288385f3e0..79cd35a6d0914d2cdf5cf05308a42e59d4d44c8a 100644 (file)
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -1509,8 +1509,7 @@ cloneFunc(SyscallDesc *desc, ThreadContext *tc, RegVal flags, RegVal newStack,
     desc->returnInto(ctc, 0);
 
     TheISA::PCState cpc = tc->pcState();
-    if (!p->kvmInSE)
-        cpc.advance();
+    cpc.advance();
     ctc->pcState(cpc);
     ctc->activate();