From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes Date: Wed, 11 Jan 2012 17:09:28 -0200 Message-ID: <20120111190927.GA13298@amt.cnet> References: <4EFBC973.1040905@tu-ilmenau.de> <4EFC3B17.1040601@redhat.com> <4F09001D.1050701@tu-ilmenau.de> <4F096E26.4090201@redhat.com> <4F0C0EBB.3090506@tu-ilmenau.de> <4F0C1369.9070607@redhat.com> <4F0C2C4E.3000703@tu-ilmenau.de> <4F0C3044.7050307@redhat.com> <4F0C33A0.6080509@tu-ilmenau.de> <4F0C4AA9.6000203@tu-ilmenau.de> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org To: Stephan =?iso-8859-1?Q?B=E4rwolf?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:19799 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932901Ab2AKTZd (ORCPT ); Wed, 11 Jan 2012 14:25:33 -0500 Content-Disposition: inline In-Reply-To: <4F0C4AA9.6000203@tu-ilmenau.de> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Jan 10, 2012 at 03:26:49PM +0100, Stephan B=E4rwolf wrote: > >From 2168285ffb30716f30e129c3ce98ce42d19c4d4e Mon Sep 17 00:00:00 20= 01 > From: Stephan Baerwolf > Date: Sun, 8 Jan 2012 02:03:47 +0000 > Subject: [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in > protected modes >=20 > On hosts without this patch, 32bit guests will crash > (and 64bit guests may behave in a wrong way) for > example by simply executing following nasm-demo-application: >=20 > [bits 32] > global _start > SECTION .text > _start: syscall >=20 > (I tested it with winxp and linux - both always crashed) >=20 > Disassembly of section .text: >=20 > 00000000 <_start>: > 0: 0f 05 syscall >=20 > The reason seems a missing "invalid opcode"-trap (int6) for the > syscall opcode "0f05", which is not available on Intel CPUs > within non-longmodes, as also on some AMD CPUs within legacy-mode. > (depending on CPU vendor, MSR_EFER and cpuid) >=20 > Because previous mentioned OSs may not engage corresponding > syscall target-registers (STAR, LSTAR, CSTAR), they remain > NULL and (non trapping) syscalls are leading to multiple > faults and finally crashs. >=20 > Depending on the architecture (AMD or Intel) pretended by > guests, various checks according to vendor's documentation > are implemented to overcome the current issue and behave > like the CPUs physical counterparts. >=20 > (Therefore using Intel's "Intel 64 and IA-32 Architecture Software > Developers Manual" http://www.intel.com/content/dam/doc/manual/ > 64-ia-32-architectures-software-developer-manual-325462.pdf > and AMD's "AMD64 Architecture Programmer's Manual Volume 3: > General-Purpose and System Instructions" > http://support.amd.com/us/Processor_TechDocs/APM_V3_24594.pdf ) >=20 > Screenshots of an i686 testing VM (CORE i5 host) before > and after applying this patch are available under: >=20 > http://matrixstorm.com/software/linux/kvm/20111229/before.jpg > http://matrixstorm.com/software/linux/kvm/20111229/after.jpg >=20 > Signed-off-by: Stephan Baerwolf > --- > arch/x86/include/asm/kvm_emulate.h | 15 ++++++ > arch/x86/kvm/emulate.c | 92 > ++++++++++++++++++++++++++++++++++- > 2 files changed, 104 insertions(+), 3 deletions(-) >=20 > diff --git a/arch/x86/include/asm/kvm_emulate.h > b/arch/x86/include/asm/kvm_emulate.h > index b172bf4..5b68c23 100644 > --- a/arch/x86/include/asm/kvm_emulate.h > +++ b/arch/x86/include/asm/kvm_emulate.h > @@ -301,6 +301,21 @@ struct x86_emulate_ctxt { > #define X86EMUL_MODE_PROT (X86EMUL_MODE_PROT16|X86EMUL_MODE_PROT= 32| \ > X86EMUL_MODE_PROT64) > =20 > +/* CPUID vendors */ > +#define X86EMUL_CPUID_VENDOR_AuthenticAMD_ebx 0x68747541 > +#define X86EMUL_CPUID_VENDOR_AuthenticAMD_ecx 0x444d4163 > +#define X86EMUL_CPUID_VENDOR_AuthenticAMD_edx 0x69746e65 > + > +#define X86EMUL_CPUID_VENDOR_AMDisbetter_ebx 0x69444d41 > +#define X86EMUL_CPUID_VENDOR_AMDisbetter_ecx 0x21726574 > +#define X86EMUL_CPUID_VENDOR_AMDisbetter_edx 0x74656273 > + > +#define X86EMUL_CPUID_VENDOR_GenuineIntel_ebx 0x756e6547 > +#define X86EMUL_CPUID_VENDOR_GenuineIntel_ecx 0x6c65746e > +#define X86EMUL_CPUID_VENDOR_GenuineIntel_edx 0x49656e69 > + > + > + > enum x86_intercept_stage { > X86_ICTP_NONE =3D 0, /* Allow zero-init to not match anything = */ > X86_ICPT_PRE_EXCEPT, > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index f1e3be1..3357411 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -1877,6 +1877,94 @@ setup_syscalls_segments(struct x86_emulate_ctx= t > *ctxt, > ss->p =3D 1; > } > =20 > +static bool em_syscall_isenabled(struct x86_emulate_ctxt *ctxt) > +{ > + struct x86_emulate_ops *ops =3D ctxt->ops; > + u64 efer =3D 0; > + > + /* syscall is not available in real mode = */ > + if ((ctxt->mode =3D=3D X86EMUL_MODE_REAL) || > + (ctxt->mode =3D=3D X86EMUL_MODE_VM86)) > + return false; > + > + ops->get_msr(ctxt, MSR_EFER, &efer); > + /* check - if guestOS is aware of syscall (0x0f05) = */ > + if ((efer & EFER_SCE) =3D=3D 0) { > + return false; > + } else { > + /* ok, at this point it becomes vendor-specific = */ > + /* so first get us an cpuid = */ > + bool vendor; > + u32 eax, ebx, ecx, edx; > + > + /* getting the cpu-vendor = */ > + eax =3D 0x00000000; > + ecx =3D 0x00000000; > + if (likely(ops->get_cpuid)) > + vendor =3D ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx); > + else vendor =3D false; > + > + if (likely(vendor)) { > + > + /* AMD AuthenticAMD / AMDisbetter! = */ > + if (((ebx=3D=3DX86EMUL_CPUID_VENDOR_AuthenticAMD_ebx) && > + (ecx=3D=3DX86EMUL_CPUID_VENDOR_AuthenticAMD_ecx) && > + (edx=3D=3DX86EMUL_CPUID_VENDOR_AuthenticAMD_edx)) || > + ((ebx=3D=3DX86EMUL_CPUID_VENDOR_AMDisbetter_ebx) && > + (ecx=3D=3DX86EMUL_CPUID_VENDOR_AMDisbetter_ecx) && > + (edx=3D=3DX86EMUL_CPUID_VENDOR_AMDisbetter_edx))) { > + > + /* if cpu is not in longmode... = */ > + /* ...check edx bit11 of cpuid 0x80000001 = */ > + if (ctxt->mode !=3D X86EMUL_MODE_PROT64) { > + eax =3D 0x80000001; > + ecx =3D 0x00000000; > + vendor =3D ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &e= dx); > + if (likely(vendor)) { > + if (unlikely(((edx >> 11) & 0x1) =3D=3D 0)) > + return false; > + } else { > + return false; /* assuming there is no bit 11 = */ > + } > + } > + goto __em_syscall_enabled_noprotest; > + } /* end "AMD" */ > + > + > + /* Intel (GenuineIntel) = */ > + /* remarks: Intel CPUs only support "syscall" in 64bit longm= ode */ > + /* Also an 64bit guest within a 32bit compat-app ru= nning*/ > + /* will #UD !! = */ > + /* While this behaviour can be fixed (by emulating)= into*/ > + /* an AMD response - CPUs of AMD can't behave like = Intel*/ > + /* because without an hardware-raised #UD there is = no */ > + /* call in em.-mode (see x86_emulate_instruction(..= =2E))! */ > + /* TODO: make AMD-behaviour configurable = */ > + if ((ebx=3D=3DX86EMUL_CPUID_VENDOR_GenuineIntel_ebx) && > + (ecx=3D=3DX86EMUL_CPUID_VENDOR_GenuineIntel_ecx) && > + (edx=3D=3DX86EMUL_CPUID_VENDOR_GenuineIntel_edx)) { > + if (ctxt->mode !=3D X86EMUL_MODE_PROT64) > + return false; > + goto __em_syscall_enabled_noprotest; > + } /* end "Intel" */ > + > + } /* end vendor is true */ > + > + } /* end MSR_EFER check */ > + > + /* default: */ > + /* this code wasn't able to process vendor */ > + /* so apply Intels stricter rules... */ > + pr_err_ratelimited("kvm: %i: unknown vcpu vendor - assuming > intel\n", > + current->tgid); > + > + if (ctxt->mode =3D=3D X86EMUL_MODE_PROT64) goto > __em_syscall_enabled_noprotest; > + return false; > + > +__em_syscall_enabled_noprotest: > + return true; > +} > + > static int em_syscall(struct x86_emulate_ctxt *ctxt) > { > struct x86_emulate_ops *ops =3D ctxt->ops; > @@ -1885,9 +1973,7 @@ static int em_syscall(struct x86_emulate_ctxt *= ctxt) > u16 cs_sel, ss_sel; > u64 efer =3D 0; > =20 > - /* syscall is not available in real mode */ > - if (ctxt->mode =3D=3D X86EMUL_MODE_REAL || > - ctxt->mode =3D=3D X86EMUL_MODE_VM86) > + if (!(em_syscall_isenabled(ctxt))) > return emulate_ud(ctxt); > =20 > ops->get_msr(ctxt, MSR_EFER, &efer); Stephan,=20 I think only the 2-line check to inject UD if EFER_SCE is not set is missing in em_syscall. The guest is responsible for setting a proper MSR_STAR for EIP only if it enables SCE_EFER. Can you please test & resend that patch? Thanks.