From eb4a5c15cec0768e6c003c20dfae9ea34e192400 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Wed, 27 Nov 2019 23:07:34 -0800 Subject: [PATCH] sim: Remove ISA specific KVM handling of the return from clone. 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 Maintainer: Gabe Black Tested-by: kokoro --- src/sim/syscall_emul.hh | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh index 44bb14751..2fd5fa7e5 100644 --- a/src/sim/syscall_emul.hh +++ b/src/sim/syscall_emul.hh @@ -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(); -- 2.30.2