On Wed, Jan 11, 2012 at 09:01:10PM +0100, Stephan Bärwolf wrote: > On 01/11/12 20:09, Marcelo Tosatti wrote: > > On Tue, Jan 10, 2012 at 03:26:49PM +0100, Stephan Bärwolf wrote: > >> >From 2168285ffb30716f30e129c3ce98ce42d19c4d4e Mon Sep 17 00:00:00 2001 > >> From: Stephan Baerwolf <stephan.baerwolf@xxxxxxxxxxxxx> > >> Date: Sun, 8 Jan 2012 02:03:47 +0000 > >> Subject: [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in > >> protected modes > >> > >> 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: > >> > >> [bits 32] > >> global _start > >> SECTION .text > >> _start: syscall > >> > >> (I tested it with winxp and linux - both always crashed) > >> > >> Disassembly of section .text: > >> > >> 00000000 <_start>: > >> 0: 0f 05 syscall > >> > >> 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) > >> > >> 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. > >> > >> 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. > >> > >> (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 ) > >> > >> Screenshots of an i686 testing VM (CORE i5 host) before > >> and after applying this patch are available under: > >> > >> http://matrixstorm.com/software/linux/kvm/20111229/before.jpg > >> http://matrixstorm.com/software/linux/kvm/20111229/after.jpg > >> > >> Signed-off-by: Stephan Baerwolf <stephan.baerwolf@xxxxxxxxxxxxx> > >> --- > >> arch/x86/include/asm/kvm_emulate.h | 15 ++++++ > >> arch/x86/kvm/emulate.c | 92 > >> ++++++++++++++++++++++++++++++++++- > >> 2 files changed, 104 insertions(+), 3 deletions(-) > >> > >> 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_PROT32| \ > >> X86EMUL_MODE_PROT64) > >> > >> +/* 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 = 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_ctxt > >> *ctxt, > >> ss->p = 1; > >> } > >> > >> +static bool em_syscall_isenabled(struct x86_emulate_ctxt *ctxt) > >> +{ > >> + struct x86_emulate_ops *ops = ctxt->ops; > >> + u64 efer = 0; > >> + > >> + /* syscall is not available in real mode */ > >> + if ((ctxt->mode == X86EMUL_MODE_REAL) || > >> + (ctxt->mode == X86EMUL_MODE_VM86)) > >> + return false; > >> + > >> + ops->get_msr(ctxt, MSR_EFER, &efer); > >> + /* check - if guestOS is aware of syscall (0x0f05) */ > >> + if ((efer & EFER_SCE) == 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 = 0x00000000; > >> + ecx = 0x00000000; > >> + if (likely(ops->get_cpuid)) > >> + vendor = ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx); > >> + else vendor = false; > >> + > >> + if (likely(vendor)) { > >> + > >> + /* AMD AuthenticAMD / AMDisbetter! */ > >> + if (((ebx==X86EMUL_CPUID_VENDOR_AuthenticAMD_ebx) && > >> + (ecx==X86EMUL_CPUID_VENDOR_AuthenticAMD_ecx) && > >> + (edx==X86EMUL_CPUID_VENDOR_AuthenticAMD_edx)) || > >> + ((ebx==X86EMUL_CPUID_VENDOR_AMDisbetter_ebx) && > >> + (ecx==X86EMUL_CPUID_VENDOR_AMDisbetter_ecx) && > >> + (edx==X86EMUL_CPUID_VENDOR_AMDisbetter_edx))) { > >> + > >> + /* if cpu is not in longmode... */ > >> + /* ...check edx bit11 of cpuid 0x80000001 */ > >> + if (ctxt->mode != X86EMUL_MODE_PROT64) { > >> + eax = 0x80000001; > >> + ecx = 0x00000000; > >> + vendor = ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx); > >> + if (likely(vendor)) { > >> + if (unlikely(((edx >> 11) & 0x1) == 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 longmode */ > >> + /* Also an 64bit guest within a 32bit compat-app running*/ > >> + /* 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(...))! */ > >> + /* TODO: make AMD-behaviour configurable */ > >> + if ((ebx==X86EMUL_CPUID_VENDOR_GenuineIntel_ebx) && > >> + (ecx==X86EMUL_CPUID_VENDOR_GenuineIntel_ecx) && > >> + (edx==X86EMUL_CPUID_VENDOR_GenuineIntel_edx)) { > >> + if (ctxt->mode != 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 == 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 = ctxt->ops; > >> @@ -1885,9 +1973,7 @@ static int em_syscall(struct x86_emulate_ctxt *ctxt) > >> u16 cs_sel, ss_sel; > >> u64 efer = 0; > >> > >> - /* syscall is not available in real mode */ > >> - if (ctxt->mode == X86EMUL_MODE_REAL || > >> - ctxt->mode == X86EMUL_MODE_VM86) > >> + if (!(em_syscall_isenabled(ctxt))) > >> return emulate_ud(ctxt); > >> > >> ops->get_msr(ctxt, MSR_EFER, &efer); > > Stephan, > > > > 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. > > > > > > > This wouln't work correctly on 64bit longmode guest > running an app in 32bit compat. The AMD pseudo-code, in volume 3 "General-Purpose and System Instructions", says: SYSCALL_START: IF (MSR_EFER.SCE = 0) // Check if syscall/sysret are enabled. EXCEPTION [#UD] IF (LONG_MODE) SYSCALL_LONG_MODE ELSE // (LEGACY_MODE) SYSCALL_LEGACY_MODE > (Even on AMD in some special conditions.) > The reason is, in 64Bit compat the MSR_EFER_SCE is still > enabled but syscall is not always available by cpu in such modes. > (I also compared always with a physical computer's behaviour.) > See Intel or even AMD-doku > (http://support.amd.com/us/Processor_TechDocs/APM_V3_24594.pdf) page 376. > > Of course in 64bit it should NOT crash (and indeed I haven't > observed it, yet) but if I have cpuid GenuineIntel it should behave > like Intel cpu and not like some special AMD. > (Otherwise I could overwrite the vendor and then the patch will > emulate AMD...) > > ??What could be a good speedup: Reordering the EFER check and the > real-mode checks > (I would have to check doku, but EFER is 0 in realmode, isn't it?), then > checking if (not)longmode and then: > > I think, the patch ->only<- (and only!?) becomes a little bit slower > exactly > in this situation (32bit compat under 64bit long) because in every other > case? > the first ifs (testing the MSR_EFER and the mode) should decide to #UD... > (At all it should be faster at the end, then it is now ?) > > ...I'll prepare a patch for what I mean... > > I am looking forward to your response, > > regards Stephan How about this: diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 05a562b..4e8e534 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -1907,6 +1907,9 @@ static int em_syscall(struct x86_emulate_ctxt *ctxt) ops->get_msr(ctxt, MSR_EFER, &efer); setup_syscalls_segments(ctxt, &cs, &ss); + if (!(efer & EFER_SCE)) + return emulate_ud(ctxt); + ops->get_msr(ctxt, MSR_STAR, &msr_data); msr_data >>= 32; cs_sel = (u16)(msr_data & 0xfffc); -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
