sim: Remove ISA specific KVM handling of the return from clone.
authorGabe Black <gabeblack@google.com>
Thu, 28 Nov 2019 07:07:34 +0000 (23:07 -0800)
committerGabe Black <gabeblack@google.com>
Thu, 12 Mar 2020 00:43:28 +0000 (00:43 +0000)
When the new thread context ctc is created, it should have a copy of
all the state in the original tc, including the original PC. This code
used to specially handle the KVM case by explicitly making this new
context return from the system call immediately by jumping right to
RCX which (assuming a particular instruction was used) is where user
mode should resume.

The first problem with this approach as far as I can tell is that the
CPU will still be in CPL0, ie supervisor mode, and will not have been
forced back into CPL3, ie user mode. This may not have any immediately
visible effect, but may down the line.

Second, this seems unnecessary. The non-special case code will advance
the PC beyond the instruction which triggered the system call. Then
once the new thread starts executing again, it will execute sysret and
return to rcx naturally, just like the original thread will.

The only observed difference is that when executing a gem5 instruction,
the IP is set to the currently executing instruction, and so to avoid
the new context from re-executing the system call, the PC needs to be
advanced. When calling in from KVM, the instruction has already been
"completed", and so the IP should *not* be advanced.

Also note that when reading the PCState object in KVM, it doesn't
figure out where the next instruction is and so NPC is just one
ExtMachInst sized blob later on. Advancing the PC will just move to
an address 8 bytes later, which is very unlikely to be what you want.

Jira Issue: https://gem5.atlassian.net/browse/GEM5-187

Change-Id: I0d97f66e64ce39b13d6700dcf3d7da88d6fe0048
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23199
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Gabe Black <gabeblack@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
src/sim/syscall_emul.hh

index 44bb147511b88153ea280dedfa2eaac42ba1e47d..2fd5fa7e56444efcf551793258fee0f008200695 100644 (file)
@@ -1551,17 +1551,10 @@ cloneFunc(SyscallDesc *desc, int callnum, ThreadContext *tc,
     ctc->setIntReg(TheISA::SyscallPseudoReturnReg, 1);
 #endif
 
-    if (p->kvmInSE) {
-#if THE_ISA == X86_ISA
-        ctc->pcState(tc->readIntReg(TheISA::INTREG_RCX));
-#else
-        panic("KVM CPU model is not supported for this ISA");
-#endif
-    } else {
-        TheISA::PCState cpc = tc->pcState();
+    TheISA::PCState cpc = tc->pcState();
+    if (!p->kvmInSE)
         cpc.advance();
-        ctc->pcState(cpc);
-    }
+    ctc->pcState(cpc);
     ctc->activate();
 
     return cp->pid();