* [PATCH 0/2] KVM guest-kernel panics double fault [not found] ` <4F0C33A0.6080509@tu-ilmenau.de> @ 2012-01-10 14:26 ` Stephan Bärwolf 2012-01-10 14:26 ` [PATCH 1/2] KVM: extend "struct x86_emulate_ops" with "get_cpuid" Stephan Bärwolf 2012-01-10 14:26 ` [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes Stephan Bärwolf 2 siblings, 0 replies; 16+ messages in thread From: Stephan Bärwolf @ 2012-01-10 14:26 UTC (permalink / raw) To: kvm From 2168285ffb30716f30e129c3ce98ce42d19c4d4e Mon Sep 17 00:00:00 2001 From: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> Date: Tue, 10 Jan 2012 14:13:22 +0100 Subject: [PATCH 0/2] KVM guest-kernel panics double fault regarding: https://lkml.org/lkml/2011/12/28/170 On tested computers (Intel Core i5-2520M, Intel Xeon X5560 and AMD Opteron 6174 [plus some misc.]), 32bit kvm guests (tested with winxp and linux-3.1) crash during execute of "syscall" (opcode 0f05). (double fault due to zeroed call of empty STAR-registers?) 64bit Intel guests behave in 32bit protected compat like AMD and not like Intel. (which would have to #UD ...) While the crash is bad (esp. for admins using VMs to isolate), because every unpriv. user can execute 0f05 - the misbehaviour with GenuineIntel-cpuid is just a blemish. Best regards, Stephan Bärwolf Stephan Baerwolf (2): KVM: extend "struct x86_emulate_ops" with "get_cpuid" KVM: fix missing "illegal instruction"-trap in protected modes arch/x86/include/asm/kvm_emulate.h | 19 +++++++ arch/x86/kvm/emulate.c | 92 ++++++++++++++++++++++++++++++++++- arch/x86/kvm/x86.c | 21 ++++++++ 3 files changed, 129 insertions(+), 3 deletions(-) -- 1.7.3.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] KVM: extend "struct x86_emulate_ops" with "get_cpuid" [not found] ` <4F0C33A0.6080509@tu-ilmenau.de> 2012-01-10 14:26 ` [PATCH 0/2] KVM guest-kernel panics double fault Stephan Bärwolf @ 2012-01-10 14:26 ` Stephan Bärwolf 2012-01-10 14:26 ` [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes Stephan Bärwolf 2 siblings, 0 replies; 16+ messages in thread From: Stephan Bärwolf @ 2012-01-10 14:26 UTC (permalink / raw) To: kvm >From c603d51a02539c89dec05fd54de336b282b1cc12 Mon Sep 17 00:00:00 2001 From: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> Date: Sun, 8 Jan 2012 23:25:59 +0000 Subject: [PATCH 1/2] KVM: extend "struct x86_emulate_ops" with "get_cpuid" In order to be able to proceed checks on CPU-specific properties within the emulator, function "get_cpuid" is introduced. With "get_cpuid" it is possible to virtually call the guests "cpuid"-opcode without changing the VM's context. Signed-off-by: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> --- arch/x86/include/asm/kvm_emulate.h | 4 ++++ arch/x86/kvm/x86.c | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 0 deletions(-) diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h index a026507..b172bf4 100644 --- a/arch/x86/include/asm/kvm_emulate.h +++ b/arch/x86/include/asm/kvm_emulate.h @@ -189,6 +189,10 @@ struct x86_emulate_ops { int (*intercept)(struct x86_emulate_ctxt *ctxt, struct x86_instruction_info *info, enum x86_intercept_stage stage); + + /* retrieve ctxt's vcpu's cpuid */ + bool (*get_cpuid)(struct x86_emulate_ctxt *ctxt, + u32 *eax, u32 *ebx, u32 *ecx, u32 *edx); }; typedef u32 __attribute__((vector_size(16))) sse128_t; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 4c938da..6181783 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4655,6 +4655,26 @@ static int emulator_intercept(struct x86_emulate_ctxt *ctxt, return kvm_x86_ops->check_intercept(emul_to_vcpu(ctxt), info, stage); } +static bool emulator_get_cpuid(struct x86_emulate_ctxt *ctxt, + u32 *eax, u32 *ebx, u32 *ecx, u32 *edx) +{ + struct kvm_cpuid_entry2 *cpuid = NULL; + + if ((ctxt) && (eax) && (ecx)) { + cpuid = kvm_find_cpuid_entry(emul_to_vcpu(ctxt), (*eax), (*ecx)); + } + + if (cpuid) { + (*eax)=cpuid->eax; + (*ecx)=cpuid->ecx; + if (ebx) (*ebx)=cpuid->ebx; + if (edx) (*edx)=cpuid->edx; + return true; + } + + return false; +} + static struct x86_emulate_ops emulate_ops = { .read_std = kvm_read_guest_virt_system, .write_std = kvm_write_guest_virt_system, @@ -4685,6 +4705,7 @@ static struct x86_emulate_ops emulate_ops = { .get_fpu = emulator_get_fpu, .put_fpu = emulator_put_fpu, .intercept = emulator_intercept, + .get_cpuid = emulator_get_cpuid, }; static void cache_all_regs(struct kvm_vcpu *vcpu) -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes [not found] ` <4F0C33A0.6080509@tu-ilmenau.de> 2012-01-10 14:26 ` [PATCH 0/2] KVM guest-kernel panics double fault Stephan Bärwolf 2012-01-10 14:26 ` [PATCH 1/2] KVM: extend "struct x86_emulate_ops" with "get_cpuid" Stephan Bärwolf @ 2012-01-10 14:26 ` Stephan Bärwolf 2012-01-11 19:09 ` Marcelo Tosatti 2 siblings, 1 reply; 16+ messages in thread From: Stephan Bärwolf @ 2012-01-10 14:26 UTC (permalink / raw) To: kvm >From 2168285ffb30716f30e129c3ce98ce42d19c4d4e Mon Sep 17 00:00:00 2001 From: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> 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@tu-ilmenau.de> --- 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); -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes 2012-01-10 14:26 ` [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes Stephan Bärwolf @ 2012-01-11 19:09 ` Marcelo Tosatti 2012-01-11 20:01 ` Stephan Bärwolf 0 siblings, 1 reply; 16+ messages in thread From: Marcelo Tosatti @ 2012-01-11 19:09 UTC (permalink / raw) To: Stephan Bärwolf; +Cc: kvm 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@tu-ilmenau.de> > 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@tu-ilmenau.de> > --- > 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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes 2012-01-11 19:09 ` Marcelo Tosatti @ 2012-01-11 20:01 ` Stephan Bärwolf 2012-01-11 21:21 ` Marcelo Tosatti 0 siblings, 1 reply; 16+ messages in thread From: Stephan Bärwolf @ 2012-01-11 20:01 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm 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@tu-ilmenau.de> >> 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@tu-ilmenau.de> >> --- >> 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. (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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes 2012-01-11 20:01 ` Stephan Bärwolf @ 2012-01-11 21:21 ` Marcelo Tosatti 2012-01-11 22:19 ` Stephan Bärwolf 0 siblings, 1 reply; 16+ messages in thread From: Marcelo Tosatti @ 2012-01-11 21:21 UTC (permalink / raw) To: Stephan Bärwolf; +Cc: kvm 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@tu-ilmenau.de> > >> 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@tu-ilmenau.de> > >> --- > >> 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); ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes 2012-01-11 21:21 ` Marcelo Tosatti @ 2012-01-11 22:19 ` Stephan Bärwolf 2012-01-12 10:47 ` Marcelo Tosatti 0 siblings, 1 reply; 16+ messages in thread From: Stephan Bärwolf @ 2012-01-11 22:19 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm On 01/11/12 22:21, Marcelo Tosatti wrote: > 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@tu-ilmenau.de> >>>> 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@tu-ilmenau.de> >>>> --- >>>> 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); > Hi Marcelo, thank you for your fast response. Yes, this would be more elegant, ->but<- not exact at least on Intel. (http://www.intel.com/content/dam/doc/manual/64-ia-32-architectures-software-developer-manual-325462.pdf) 4-586 Vol. 2B or PDF-page 1804 "[...] (* Not in 64-Bit Mode or SYSCALL/SYSRET not enabled in IA32_EFER *) [...]" We have to check somehow cpu-vendor to get info what cpu should be emulated... ...I'll prepare a patch by tomorrow reordering things and compacting checks on AMD's branch. (obviously cpuid 0x80000001 edx bit 11 needs not to be checked, as far as the pseudocode tells... ) regards Stephan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes 2012-01-11 22:19 ` Stephan Bärwolf @ 2012-01-12 10:47 ` Marcelo Tosatti 2012-01-12 15:43 ` [PATCH 0/2] KVM guest-kernel panics double fault Stephan Bärwolf ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Marcelo Tosatti @ 2012-01-12 10:47 UTC (permalink / raw) To: Stephan Bärwolf; +Cc: kvm On Wed, Jan 11, 2012 at 11:19:50PM +0100, Stephan Bärwolf wrote: > On 01/11/12 22:21, Marcelo Tosatti wrote: > > 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@tu-ilmenau.de> > >>>> 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@tu-ilmenau.de> > >>>> --- > >>>> 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); > > > Hi Marcelo, > thank you for your fast response. > > Yes, this would be more elegant, ->but<- not exact at least on Intel. > (http://www.intel.com/content/dam/doc/manual/64-ia-32-architectures-software-developer-manual-325462.pdf) > 4-586 Vol. 2B or PDF-page 1804 > "[...] (* Not in 64-Bit Mode or SYSCALL/SYSRET not enabled in IA32_EFER > *) [...]" > > We have to check somehow cpu-vendor to get info what cpu should be > emulated... > ...I'll prepare a patch by tomorrow reordering things and compacting > checks on AMD's branch. > (obviously cpuid 0x80000001 edx bit 11 needs not to be checked, as far > as the pseudocode tells... ) Right. Please have the EFER check as i suggested (its cleaner), and then only this check for Intel should suffice: + /* 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) Please mind Documentation/CodingStyle. Thanks. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/2] KVM guest-kernel panics double fault 2012-01-12 10:47 ` Marcelo Tosatti @ 2012-01-12 15:43 ` Stephan Bärwolf 2012-01-12 15:56 ` Jan Kiszka 2012-01-12 15:43 ` [PATCH 1/2] KVM: extend "struct x86_emulate_ops" with "get_cpuid" Stephan Bärwolf 2012-01-12 15:43 ` [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes Stephan Bärwolf 2 siblings, 1 reply; 16+ messages in thread From: Stephan Bärwolf @ 2012-01-12 15:43 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm [-- Attachment #1: Type: text/plain, Size: 1248 bytes --] >From d62ca9897e9970d777aec1d399318b0df44489bd Mon Sep 17 00:00:00 2001 From: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> Date: Thu, 12 Jan 2012 16:32:46 +0100 Subject: [PATCH 0/2] KVM guest-kernel panics double fault regarding: https://lkml.org/lkml/2011/12/28/170 On tested computers (Intel Core i5-2520M, Intel Xeon X5560 and AMD Opteron 6174 [plus some misc.]), 32bit kvm guests (tested with winxp and linux-3.1) crash during execute of "syscall" (opcode 0f05). (double fault due to zeroed call of empty STAR-registers?) 64bit Intel guests behave in 32bit protected compat like AMD and not like Intel. (which would have to #UD ...) While the crash is bad (esp. for admins using VMs to isolate), because every unpriv. user can execute 0f05 - the misbehaviour with GenuineIntel-cpuid is just a blemish. Best regards, Stephan Bärwolf Stephan Baerwolf (2): KVM: extend "struct x86_emulate_ops" with "get_cpuid" KVM: fix missing "illegal instruction"-trap in protected modes arch/x86/include/asm/kvm_emulate.h | 19 ++++++++ arch/x86/kvm/emulate.c | 80 ++++++++++++++++++++++++++++++++++- arch/x86/kvm/x86.c | 21 +++++++++ 3 files changed, 117 insertions(+), 3 deletions(-) -- 1.7.3.4 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0000-cover-letter.patch --] [-- Type: text/x-patch; name="0000-cover-letter.patch", Size: 1246 bytes --] >From d62ca9897e9970d777aec1d399318b0df44489bd Mon Sep 17 00:00:00 2001 From: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> Date: Thu, 12 Jan 2012 16:32:46 +0100 Subject: [PATCH 0/2] KVM guest-kernel panics double fault regarding: https://lkml.org/lkml/2011/12/28/170 On tested computers (Intel Core i5-2520M, Intel Xeon X5560 and AMD Opteron 6174 [plus some misc.]), 32bit kvm guests (tested with winxp and linux-3.1) crash during execute of "syscall" (opcode 0f05). (double fault due to zeroed call of empty STAR-registers?) 64bit Intel guests behave in 32bit protected compat like AMD and not like Intel. (which would have to #UD ...) While the crash is bad (esp. for admins using VMs to isolate), because every unpriv. user can execute 0f05 - the misbehaviour with GenuineIntel-cpuid is just a blemish. Best regards, Stephan Bärwolf Stephan Baerwolf (2): KVM: extend "struct x86_emulate_ops" with "get_cpuid" KVM: fix missing "illegal instruction"-trap in protected modes arch/x86/include/asm/kvm_emulate.h | 19 ++++++++ arch/x86/kvm/emulate.c | 80 ++++++++++++++++++++++++++++++++++- arch/x86/kvm/x86.c | 21 +++++++++ 3 files changed, 117 insertions(+), 3 deletions(-) -- 1.7.3.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] KVM guest-kernel panics double fault 2012-01-12 15:43 ` [PATCH 0/2] KVM guest-kernel panics double fault Stephan Bärwolf @ 2012-01-12 15:56 ` Jan Kiszka 2012-01-13 10:13 ` Marcelo Tosatti 0 siblings, 1 reply; 16+ messages in thread From: Jan Kiszka @ 2012-01-12 15:56 UTC (permalink / raw) To: stephan.baerwolf; +Cc: Marcelo Tosatti, kvm On 2012-01-12 16:43, Stephan Bärwolf wrote: > From d62ca9897e9970d777aec1d399318b0df44489bd Mon Sep 17 00:00:00 2001 > From: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> > Date: Thu, 12 Jan 2012 16:32:46 +0100 > Subject: [PATCH 0/2] KVM guest-kernel panics double fault > > regarding: https://lkml.org/lkml/2011/12/28/170 > > On tested computers (Intel Core i5-2520M, Intel Xeon X5560 > and AMD Opteron 6174 [plus some misc.]), 32bit kvm guests > (tested with winxp and linux-3.1) crash during execute of > "syscall" (opcode 0f05). (double fault due to zeroed call > of empty STAR-registers?) > > 64bit Intel guests behave in 32bit protected compat like > AMD and not like Intel. (which would have to #UD ...) > > While the crash is bad (esp. for admins using VMs to isolate), > because every unpriv. user can execute 0f05 - the misbehaviour > with GenuineIntel-cpuid is just a blemish. > > Best regards, > Stephan Bärwolf > > > Stephan Baerwolf (2): > KVM: extend "struct x86_emulate_ops" with "get_cpuid" > KVM: fix missing "illegal instruction"-trap in protected modes > > arch/x86/include/asm/kvm_emulate.h | 19 ++++++++ > arch/x86/kvm/emulate.c | 80 > ++++++++++++++++++++++++++++++++++- > arch/x86/kvm/x86.c | 21 +++++++++ > 3 files changed, 117 insertions(+), 3 deletions(-) > linux/scripts/checkpatch.pl can help identifying remaining style violations. Moreover, when sending your patches via Thunderbird, you first need to make sure that you have its line wrapping under control. "Toggle Word Wrap" helps me for single patches. But for series, I prefer (scripted) git format-patch/send-email. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] KVM guest-kernel panics double fault 2012-01-12 15:56 ` Jan Kiszka @ 2012-01-13 10:13 ` Marcelo Tosatti 2012-01-15 19:44 ` Stephan Bärwolf 0 siblings, 1 reply; 16+ messages in thread From: Marcelo Tosatti @ 2012-01-13 10:13 UTC (permalink / raw) To: Jan Kiszka, stephan.baerwolf; +Cc: kvm On Thu, Jan 12, 2012 at 04:56:33PM +0100, Jan Kiszka wrote: > > Stephan Baerwolf (2): > > KVM: extend "struct x86_emulate_ops" with "get_cpuid" > > KVM: fix missing "illegal instruction"-trap in protected modes > > > > arch/x86/include/asm/kvm_emulate.h | 19 ++++++++ > > arch/x86/kvm/emulate.c | 80 > > ++++++++++++++++++++++++++++++++++- > > arch/x86/kvm/x86.c | 21 +++++++++ > > 3 files changed, 117 insertions(+), 3 deletions(-) > > > > linux/scripts/checkpatch.pl can help identifying remaining style violations. > > Moreover, when sending your patches via Thunderbird, you first need to > make sure that you have its line wrapping under control. "Toggle Word > Wrap" helps me for single patches. But for series, I prefer (scripted) > git format-patch/send-email. Applied a cleanup up version of these patches. Thanks Stephan. http://git.kernel.org/?p=virt/kvm/kvm.git;a=commit;h=0769c5de24621141c953fbe1f943582d37cb4244 http://git.kernel.org/?p=virt/kvm/kvm.git;a=commit;h=e28ba7bb020f07193bc000453c8775e9d2c0dda7 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] KVM guest-kernel panics double fault 2012-01-13 10:13 ` Marcelo Tosatti @ 2012-01-15 19:44 ` Stephan Bärwolf 2012-01-16 9:58 ` Marcelo Tosatti 0 siblings, 1 reply; 16+ messages in thread From: Stephan Bärwolf @ 2012-01-15 19:44 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: Jan Kiszka, kvm Thank you for applying this, Marcelo. I fear we (or me after I agreed) did some mistake by erasing the additional cpuid 0x80000001 checks. In contradiction to only AMD it MUST also apply on Intel-CPUs. Documentation "http://www.intel.com/content/dam/doc/manual/64-ia-32-architectures-software-developer-manual-325462.pdf" Vol. 2A 3-207 (PDF-page 811) first block of table. (in addition AMD's doku "http://support.amd.com/us/Processor_TechDocs/APM_V3_24594.pdf" page 376 (PDF-page 408) table "Exceptions" on the bottom) Not all CPUs might have a syscall op at all (even in longmode) - they informing about that via cpuid (But MSR_EFER may be still set). (You can force it externally in qemu-kvm-emulation via "-cpu host,-syscall" ...) So an (guest) operating-system might not install *STAR-registers and crash again on such vcpus, right? I'll prepare some patch, asap. So everybody can judge this... regards, Stephan On 01/13/12 11:13, Marcelo Tosatti wrote: > On Thu, Jan 12, 2012 at 04:56:33PM +0100, Jan Kiszka wrote: >>> Stephan Baerwolf (2): >>> KVM: extend "struct x86_emulate_ops" with "get_cpuid" >>> KVM: fix missing "illegal instruction"-trap in protected modes >>> >>> arch/x86/include/asm/kvm_emulate.h | 19 ++++++++ >>> arch/x86/kvm/emulate.c | 80 >>> ++++++++++++++++++++++++++++++++++- >>> arch/x86/kvm/x86.c | 21 +++++++++ >>> 3 files changed, 117 insertions(+), 3 deletions(-) >>> >> linux/scripts/checkpatch.pl can help identifying remaining style violations. >> >> Moreover, when sending your patches via Thunderbird, you first need to >> make sure that you have its line wrapping under control. "Toggle Word >> Wrap" helps me for single patches. But for series, I prefer (scripted) >> git format-patch/send-email. > Applied a cleanup up version of these patches. Thanks Stephan. > > http://git.kernel.org/?p=virt/kvm/kvm.git;a=commit;h=0769c5de24621141c953fbe1f943582d37cb4244 > > http://git.kernel.org/?p=virt/kvm/kvm.git;a=commit;h=e28ba7bb020f07193bc000453c8775e9d2c0dda7 > > > -- Dipl.-Inf. Stephan Bärwolf Ilmenau University of Technology, Integrated Communication Systems Group Phone: +49 (0)3677 69 4130 Email: stephan.baerwolf@tu-ilmenau.de, Web: http://www.tu-ilmenau.de/iks ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] KVM guest-kernel panics double fault 2012-01-15 19:44 ` Stephan Bärwolf @ 2012-01-16 9:58 ` Marcelo Tosatti 2012-01-16 11:24 ` Stephan Bärwolf 0 siblings, 1 reply; 16+ messages in thread From: Marcelo Tosatti @ 2012-01-16 9:58 UTC (permalink / raw) To: Stephan Bärwolf; +Cc: Jan Kiszka, kvm On Sun, Jan 15, 2012 at 08:44:50PM +0100, Stephan Bärwolf wrote: > Thank you for applying this, Marcelo. > > I fear we (or me after I agreed) did some mistake by erasing the additional > cpuid 0x80000001 checks. > In contradiction to only AMD it MUST also apply on Intel-CPUs. > > Documentation > "http://www.intel.com/content/dam/doc/manual/64-ia-32-architectures-software-developer-manual-325462.pdf" > Vol. 2A 3-207 (PDF-page 811) first block of table. > (in addition AMD's doku "http://support.amd.com/us/Processor_TechDocs/APM_V3_24594.pdf" > page 376 (PDF-page 408) table "Exceptions" on the bottom) > > Not all CPUs might have a syscall op at all (even in longmode) - they informing about that > via cpuid (But MSR_EFER may be still set). > (You can force it externally in qemu-kvm-emulation via "-cpu host,-syscall" ...) > So an (guest) operating-system might not install *STAR-registers and crash again on such vcpus, right? No because if the operating system does not install the STAR MSRs, it will not set SCE bit in MSR_EFER (and your patch handles that situation). ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] KVM guest-kernel panics double fault 2012-01-16 9:58 ` Marcelo Tosatti @ 2012-01-16 11:24 ` Stephan Bärwolf 0 siblings, 0 replies; 16+ messages in thread From: Stephan Bärwolf @ 2012-01-16 11:24 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: Jan Kiszka, kvm Okay, thx to hear a second opinion. Then everything is ok, have a nice day. regards Stephan On 01/16/12 10:58, Marcelo Tosatti wrote: > On Sun, Jan 15, 2012 at 08:44:50PM +0100, Stephan Bärwolf wrote: >> Thank you for applying this, Marcelo. >> >> I fear we (or me after I agreed) did some mistake by erasing the additional >> cpuid 0x80000001 checks. >> In contradiction to only AMD it MUST also apply on Intel-CPUs. >> >> Documentation >> "http://www.intel.com/content/dam/doc/manual/64-ia-32-architectures-software-developer-manual-325462.pdf" >> Vol. 2A 3-207 (PDF-page 811) first block of table. >> (in addition AMD's doku "http://support.amd.com/us/Processor_TechDocs/APM_V3_24594.pdf" >> page 376 (PDF-page 408) table "Exceptions" on the bottom) >> >> Not all CPUs might have a syscall op at all (even in longmode) - they informing about that >> via cpuid (But MSR_EFER may be still set). >> (You can force it externally in qemu-kvm-emulation via "-cpu host,-syscall" ...) >> So an (guest) operating-system might not install *STAR-registers and crash again on such vcpus, right? > No because if the operating system does not install the STAR MSRs, it > will not set SCE bit in MSR_EFER (and your patch handles that > situation). > > -- Dipl.-Inf. Stephan Bärwolf Ilmenau University of Technology, Integrated Communication Systems Group Phone: +49 (0)3677 69 4130 Email: stephan.baerwolf@tu-ilmenau.de, Web: http://www.tu-ilmenau.de/iks ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] KVM: extend "struct x86_emulate_ops" with "get_cpuid" 2012-01-12 10:47 ` Marcelo Tosatti 2012-01-12 15:43 ` [PATCH 0/2] KVM guest-kernel panics double fault Stephan Bärwolf @ 2012-01-12 15:43 ` Stephan Bärwolf 2012-01-12 15:43 ` [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes Stephan Bärwolf 2 siblings, 0 replies; 16+ messages in thread From: Stephan Bärwolf @ 2012-01-12 15:43 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm [-- Attachment #1: Type: text/plain, Size: 2625 bytes --] >From a8f796f81979094b81cb74535632786ce1ccf9bb Mon Sep 17 00:00:00 2001 From: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> Date: Sun, 8 Jan 2012 23:25:59 +0000 Subject: [PATCH 1/2] KVM: extend "struct x86_emulate_ops" with "get_cpuid" In order to be able to proceed checks on CPU-specific properties within the emulator, function "get_cpuid" is introduced. With "get_cpuid" it is possible to virtually call the guests "cpuid"-opcode without changing the VM's context. Cc: <stable@vger.kernel.org> Signed-off-by: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> --- arch/x86/include/asm/kvm_emulate.h | 4 ++++ arch/x86/kvm/x86.c | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 0 deletions(-) diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h index a026507..b172bf4 100644 --- a/arch/x86/include/asm/kvm_emulate.h +++ b/arch/x86/include/asm/kvm_emulate.h @@ -189,6 +189,10 @@ struct x86_emulate_ops { int (*intercept)(struct x86_emulate_ctxt *ctxt, struct x86_instruction_info *info, enum x86_intercept_stage stage); + + /* retrieve ctxt's vcpu's cpuid */ + bool (*get_cpuid)(struct x86_emulate_ctxt *ctxt, + u32 *eax, u32 *ebx, u32 *ecx, u32 *edx); }; typedef u32 __attribute__((vector_size(16))) sse128_t; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 4c938da..6181783 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4655,6 +4655,26 @@ static int emulator_intercept(struct x86_emulate_ctxt *ctxt, return kvm_x86_ops->check_intercept(emul_to_vcpu(ctxt), info, stage); } +static bool emulator_get_cpuid(struct x86_emulate_ctxt *ctxt, + u32 *eax, u32 *ebx, u32 *ecx, u32 *edx) +{ + struct kvm_cpuid_entry2 *cpuid = NULL; + + if ((ctxt) && (eax) && (ecx)) { + cpuid = kvm_find_cpuid_entry(emul_to_vcpu(ctxt), (*eax), (*ecx)); + } + + if (cpuid) { + (*eax)=cpuid->eax; + (*ecx)=cpuid->ecx; + if (ebx) (*ebx)=cpuid->ebx; + if (edx) (*edx)=cpuid->edx; + return true; + } + + return false; +} + static struct x86_emulate_ops emulate_ops = { .read_std = kvm_read_guest_virt_system, .write_std = kvm_write_guest_virt_system, @@ -4685,6 +4705,7 @@ static struct x86_emulate_ops emulate_ops = { .get_fpu = emulator_get_fpu, .put_fpu = emulator_put_fpu, .intercept = emulator_intercept, + .get_cpuid = emulator_get_cpuid, }; static void cache_all_regs(struct kvm_vcpu *vcpu) -- 1.7.3.4 [-- Attachment #2: 0001-KVM-extend-struct-x86_emulate_ops-with-get_cpuid.patch --] [-- Type: text/x-patch, Size: 2538 bytes --] >From a8f796f81979094b81cb74535632786ce1ccf9bb Mon Sep 17 00:00:00 2001 From: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> Date: Sun, 8 Jan 2012 23:25:59 +0000 Subject: [PATCH 1/2] KVM: extend "struct x86_emulate_ops" with "get_cpuid" In order to be able to proceed checks on CPU-specific properties within the emulator, function "get_cpuid" is introduced. With "get_cpuid" it is possible to virtually call the guests "cpuid"-opcode without changing the VM's context. Cc: <stable@vger.kernel.org> Signed-off-by: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> --- arch/x86/include/asm/kvm_emulate.h | 4 ++++ arch/x86/kvm/x86.c | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 0 deletions(-) diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h index a026507..b172bf4 100644 --- a/arch/x86/include/asm/kvm_emulate.h +++ b/arch/x86/include/asm/kvm_emulate.h @@ -189,6 +189,10 @@ struct x86_emulate_ops { int (*intercept)(struct x86_emulate_ctxt *ctxt, struct x86_instruction_info *info, enum x86_intercept_stage stage); + + /* retrieve ctxt's vcpu's cpuid */ + bool (*get_cpuid)(struct x86_emulate_ctxt *ctxt, + u32 *eax, u32 *ebx, u32 *ecx, u32 *edx); }; typedef u32 __attribute__((vector_size(16))) sse128_t; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 4c938da..6181783 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4655,6 +4655,26 @@ static int emulator_intercept(struct x86_emulate_ctxt *ctxt, return kvm_x86_ops->check_intercept(emul_to_vcpu(ctxt), info, stage); } +static bool emulator_get_cpuid(struct x86_emulate_ctxt *ctxt, + u32 *eax, u32 *ebx, u32 *ecx, u32 *edx) +{ + struct kvm_cpuid_entry2 *cpuid = NULL; + + if ((ctxt) && (eax) && (ecx)) { + cpuid = kvm_find_cpuid_entry(emul_to_vcpu(ctxt), (*eax), (*ecx)); + } + + if (cpuid) { + (*eax)=cpuid->eax; + (*ecx)=cpuid->ecx; + if (ebx) (*ebx)=cpuid->ebx; + if (edx) (*edx)=cpuid->edx; + return true; + } + + return false; +} + static struct x86_emulate_ops emulate_ops = { .read_std = kvm_read_guest_virt_system, .write_std = kvm_write_guest_virt_system, @@ -4685,6 +4705,7 @@ static struct x86_emulate_ops emulate_ops = { .get_fpu = emulator_get_fpu, .put_fpu = emulator_put_fpu, .intercept = emulator_intercept, + .get_cpuid = emulator_get_cpuid, }; static void cache_all_regs(struct kvm_vcpu *vcpu) -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes 2012-01-12 10:47 ` Marcelo Tosatti 2012-01-12 15:43 ` [PATCH 0/2] KVM guest-kernel panics double fault Stephan Bärwolf 2012-01-12 15:43 ` [PATCH 1/2] KVM: extend "struct x86_emulate_ops" with "get_cpuid" Stephan Bärwolf @ 2012-01-12 15:43 ` Stephan Bärwolf 2 siblings, 0 replies; 16+ messages in thread From: Stephan Bärwolf @ 2012-01-12 15:43 UTC (permalink / raw) To: Marcelo Tosatti; +Cc: kvm [-- Attachment #1: Type: text/plain, Size: 7054 bytes --] >From d62ca9897e9970d777aec1d399318b0df44489bd Mon Sep 17 00:00:00 2001 From: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> 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 Cc: <stable@vger.kernel.org> Signed-off-by: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> --- arch/x86/include/asm/kvm_emulate.h | 15 +++++++ arch/x86/kvm/emulate.c | 80 ++++++++++++++++++++++++++++++++++- 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h index b172bf4..f0da4d9 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_AMDisbetterI_ebx 0x69444d41 +#define X86EMUL_CPUID_VENDOR_AMDisbetterI_ecx 0x21726574 +#define X86EMUL_CPUID_VENDOR_AMDisbetterI_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..29140ae 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -1877,6 +1877,82 @@ 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; + + /* NO check for lock-prefix here - done in x86_emulate_instruction()*/ + + ops->get_msr(ctxt, MSR_EFER, &efer); + + /* we assume EFER_SCE is unset in REAL and VM86 so with this we */ + /* should speedup the #UD case */ + if ((efer & EFER_SCE) == 0) + return false; + + /* syscall should always be enabled in longmode - so only become */ + /* vendorspecific (cpuid) if other modes are active... */ + if (ctxt->mode != X86EMUL_MODE_PROT64) { + bool vendor; + u32 eax, ebx, ecx, edx; + + /* syscall is not available in real mode - #UD without cpuid... */ + if ((ctxt->mode == X86EMUL_MODE_REAL) || + (ctxt->mode == X86EMUL_MODE_VM86)) + return false; + + /* 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)) { + + /* Intel ("GenuineIntel") */ + /* remark: Intel CPUs only support "syscall" in 64bit longmode */ + /* Also an 64bit guest with 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) */ + if ((ebx==X86EMUL_CPUID_VENDOR_GenuineIntel_ebx) && + (ecx==X86EMUL_CPUID_VENDOR_GenuineIntel_ecx) && + (edx==X86EMUL_CPUID_VENDOR_GenuineIntel_edx)) + return false; + + /* end "Intel" */ + + + /* 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_AMDisbetterI_ebx) && + (ecx==X86EMUL_CPUID_VENDOR_AMDisbetterI_ecx) && + (edx==X86EMUL_CPUID_VENDOR_AMDisbetterI_edx))) { + + goto __em_syscall_enabled_noprotest; + } /* end "AMD" */ + + } /* end vendor is true */ + + + /* default: (not Intel, not AMD ...) */ + /* 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); + return false; + } /* end "not longmode" */ + +__em_syscall_enabled_noprotest: + return true; +} + static int em_syscall(struct x86_emulate_ctxt *ctxt) { struct x86_emulate_ops *ops = ctxt->ops; @@ -1885,9 +1961,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); -- 1.7.3.4 [-- Attachment #2: 0002-KVM-fix-missing-illegal-instruction-trap-in-protecte.patch --] [-- Type: text/x-patch, Size: 6781 bytes --] >From d62ca9897e9970d777aec1d399318b0df44489bd Mon Sep 17 00:00:00 2001 From: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> 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 Cc: <stable@vger.kernel.org> Signed-off-by: Stephan Baerwolf <stephan.baerwolf@tu-ilmenau.de> --- arch/x86/include/asm/kvm_emulate.h | 15 +++++++ arch/x86/kvm/emulate.c | 80 ++++++++++++++++++++++++++++++++++- 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h index b172bf4..f0da4d9 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_AMDisbetterI_ebx 0x69444d41 +#define X86EMUL_CPUID_VENDOR_AMDisbetterI_ecx 0x21726574 +#define X86EMUL_CPUID_VENDOR_AMDisbetterI_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..29140ae 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -1877,6 +1877,82 @@ 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; + + /* NO check for lock-prefix here - done in x86_emulate_instruction()*/ + + ops->get_msr(ctxt, MSR_EFER, &efer); + + /* we assume EFER_SCE is unset in REAL and VM86 so with this we */ + /* should speedup the #UD case */ + if ((efer & EFER_SCE) == 0) + return false; + + /* syscall should always be enabled in longmode - so only become */ + /* vendorspecific (cpuid) if other modes are active... */ + if (ctxt->mode != X86EMUL_MODE_PROT64) { + bool vendor; + u32 eax, ebx, ecx, edx; + + /* syscall is not available in real mode - #UD without cpuid... */ + if ((ctxt->mode == X86EMUL_MODE_REAL) || + (ctxt->mode == X86EMUL_MODE_VM86)) + return false; + + /* 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)) { + + /* Intel ("GenuineIntel") */ + /* remark: Intel CPUs only support "syscall" in 64bit longmode */ + /* Also an 64bit guest with 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) */ + if ((ebx==X86EMUL_CPUID_VENDOR_GenuineIntel_ebx) && + (ecx==X86EMUL_CPUID_VENDOR_GenuineIntel_ecx) && + (edx==X86EMUL_CPUID_VENDOR_GenuineIntel_edx)) + return false; + + /* end "Intel" */ + + + /* 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_AMDisbetterI_ebx) && + (ecx==X86EMUL_CPUID_VENDOR_AMDisbetterI_ecx) && + (edx==X86EMUL_CPUID_VENDOR_AMDisbetterI_edx))) { + + goto __em_syscall_enabled_noprotest; + } /* end "AMD" */ + + } /* end vendor is true */ + + + /* default: (not Intel, not AMD ...) */ + /* 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); + return false; + } /* end "not longmode" */ + +__em_syscall_enabled_noprotest: + return true; +} + static int em_syscall(struct x86_emulate_ctxt *ctxt) { struct x86_emulate_ops *ops = ctxt->ops; @@ -1885,9 +1961,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); -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-01-16 11:26 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <4EFBC973.1040905@tu-ilmenau.de>
[not found] ` <4EFC3B17.1040601@redhat.com>
[not found] ` <4F09001D.1050701@tu-ilmenau.de>
[not found] ` <4F096E26.4090201@redhat.com>
[not found] ` <4F0C0EBB.3090506@tu-ilmenau.de>
[not found] ` <4F0C1369.9070607@redhat.com>
[not found] ` <4F0C2C4E.3000703@tu-ilmenau.de>
[not found] ` <4F0C3044.7050307@redhat.com>
[not found] ` <4F0C33A0.6080509@tu-ilmenau.de>
2012-01-10 14:26 ` [PATCH 0/2] KVM guest-kernel panics double fault Stephan Bärwolf
2012-01-10 14:26 ` [PATCH 1/2] KVM: extend "struct x86_emulate_ops" with "get_cpuid" Stephan Bärwolf
2012-01-10 14:26 ` [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes Stephan Bärwolf
2012-01-11 19:09 ` Marcelo Tosatti
2012-01-11 20:01 ` Stephan Bärwolf
2012-01-11 21:21 ` Marcelo Tosatti
2012-01-11 22:19 ` Stephan Bärwolf
2012-01-12 10:47 ` Marcelo Tosatti
2012-01-12 15:43 ` [PATCH 0/2] KVM guest-kernel panics double fault Stephan Bärwolf
2012-01-12 15:56 ` Jan Kiszka
2012-01-13 10:13 ` Marcelo Tosatti
2012-01-15 19:44 ` Stephan Bärwolf
2012-01-16 9:58 ` Marcelo Tosatti
2012-01-16 11:24 ` Stephan Bärwolf
2012-01-12 15:43 ` [PATCH 1/2] KVM: extend "struct x86_emulate_ops" with "get_cpuid" Stephan Bärwolf
2012-01-12 15:43 ` [PATCH 2/2] KVM: fix missing "illegal instruction"-trap in protected modes Stephan Bärwolf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).