From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 2/2] add sysenter/syscall emulation for 32bit compat mode Date: Sun, 31 May 2009 11:59:19 +0300 Message-ID: <4A2246E7.7050102@redhat.com> References: <1243504592-5112-1-git-send-email-andre.przywara@amd.com> <1243504592-5112-2-git-send-email-andre.przywara@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Amit Shah , Christoph Egger To: Andre Przywara Return-path: Received: from mx2.redhat.com ([66.187.237.31]:49348 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752991AbZEaI7V (ORCPT ); Sun, 31 May 2009 04:59:21 -0400 In-Reply-To: <1243504592-5112-2-git-send-email-andre.przywara@amd.com> Sender: kvm-owner@vger.kernel.org List-ID: Andre Przywara wrote: > sysenter/sysexit are not supported on AMD's 32bit compat mode, whereas > syscall is not supported on Intel's 32bit compat mode. To allow cross > vendor migration we emulate the missing instructions by setting up the > processor state according to the other call. > The sysenter code was originally sketched by Amit Shah, it was completed, > debugged, syscall added and made-to-work by Christoph Egger and polished > up by Andre Przywara. > Please note that sysret does not need to be emulated, because it will be > exectued in 64bit mode and returning to 32bit compat mode works on Intel. > ++vcpu->stat.insn_emulation; > if (r) { > diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c > index 22c765d..41b78fa 100644 > --- a/arch/x86/kvm/x86_emulate.c > +++ b/arch/x86/kvm/x86_emulate.c > @@ -32,6 +32,8 @@ > #include > #include > > +#include "mmu.h" > + > I think this is unneeded? > @@ -1985,10 +1992,114 @@ twobyte_insn: > goto cannot_emulate; > } > break; > + case 0x05: { /* syscall */ > + unsigned long cr0 = ctxt->vcpu->arch.cr0; > + struct kvm_segment cs, ss; > + > + memset(&cs, 0, sizeof(struct kvm_segment)); > + memset(&ss, 0, sizeof(struct kvm_segment)); > + > + /* inject #UD if > + * 1. we are in real mode > + * 2. protected mode is not enabled > + * 3. LOCK prefix is used > + */ > + if ((ctxt->mode == X86EMUL_MODE_REAL) > + || (!(cr0 & X86_CR0_PE)) > + || (c->lock_prefix)) { > + /* we don't need to inject #UD here, because > + * when emulate_instruction() returns something else > + * than EMULATE_DONE, then svm.c:ud_interception() > + * will do that for us. > + */ > + goto cannot_emulate; > I prefer explicit injection, relying on the caller is tricky and may change. > + case 0x07: /* sysret */ > Since we don't emulate sysret, it should be dropped here. > + cs.limit = 0xffffffff; > + ss.base = 0; > + ss.limit = 0xffffffff; > Once is enough. Please move the code out of the switch and into separate functions. -- error compiling committee.c: too many arguments to function