* [PATCH 0/4] Vendor specific instructions and the emulator
@ 2011-01-04 13:14 Avi Kivity
2011-01-04 13:14 ` [PATCH 1/4] KVM: Fix x86_decode_insn() return code check Avi Kivity
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Avi Kivity @ 2011-01-04 13:14 UTC (permalink / raw)
To: Marcelo Tosatti, kvm
Currently we have some ad-hoc code in x86.c to restrict #UD emulation to
expected instructions (that is, vendor specific instructions). This patchset
replaces the ad-hoc code with proper emulator support using decode tables.
Avi Kivity (4):
KVM: Fix x86_decode_insn() return code check
KVM: Simplify exit path on decode failure
KVM: x86 emulator: vendor specific instructions
KVM: Drop ad-hoc vendor specific instruction restriction
arch/x86/include/asm/kvm_emulate.h | 1 +
arch/x86/kvm/emulate.c | 12 +++++++++---
arch/x86/kvm/x86.c | 35 +++++------------------------------
3 files changed, 15 insertions(+), 33 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/4] KVM: Fix x86_decode_insn() return code check 2011-01-04 13:14 [PATCH 0/4] Vendor specific instructions and the emulator Avi Kivity @ 2011-01-04 13:14 ` Avi Kivity 2011-01-13 13:54 ` Avi Kivity 2011-01-04 13:14 ` [PATCH 2/4] KVM: Simplify exit path on decode failure Avi Kivity ` (3 subsequent siblings) 4 siblings, 1 reply; 9+ messages in thread From: Avi Kivity @ 2011-01-04 13:14 UTC (permalink / raw) To: Marcelo Tosatti, kvm x86_decode_insn() doesn't return X86EMUL_* values, it returns EMULATION_* codes. Adjust the check. Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/kvm/x86.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index fa708c9..b20499d 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4391,7 +4391,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, vcpu->arch.emulate_ctxt.perm_ok = false; r = x86_decode_insn(&vcpu->arch.emulate_ctxt, insn, insn_len); - if (r == X86EMUL_PROPAGATE_FAULT) + if (r != EMULATION_OK) goto done; trace_kvm_emulate_insn_start(vcpu); -- 1.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] KVM: Fix x86_decode_insn() return code check 2011-01-04 13:14 ` [PATCH 1/4] KVM: Fix x86_decode_insn() return code check Avi Kivity @ 2011-01-13 13:54 ` Avi Kivity 0 siblings, 0 replies; 9+ messages in thread From: Avi Kivity @ 2011-01-13 13:54 UTC (permalink / raw) To: Marcelo Tosatti, kvm On 01/04/2011 03:14 PM, Avi Kivity wrote: > x86_decode_insn() doesn't return X86EMUL_* values, it returns > EMULATION_* codes. Adjust the check. > > Signed-off-by: Avi Kivity<avi@redhat.com> > --- > arch/x86/kvm/x86.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index fa708c9..b20499d 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4391,7 +4391,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, > vcpu->arch.emulate_ctxt.perm_ok = false; > > r = x86_decode_insn(&vcpu->arch.emulate_ctxt, insn, insn_len); > - if (r == X86EMUL_PROPAGATE_FAULT) > + if (r != EMULATION_OK) > goto done; > > trace_kvm_emulate_insn_start(vcpu); Strangely, this patch causes a failure in Windows XP. Dropping until submitter investigates and fixes. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/4] KVM: Simplify exit path on decode failure 2011-01-04 13:14 [PATCH 0/4] Vendor specific instructions and the emulator Avi Kivity 2011-01-04 13:14 ` [PATCH 1/4] KVM: Fix x86_decode_insn() return code check Avi Kivity @ 2011-01-04 13:14 ` Avi Kivity 2011-01-10 17:17 ` Jan Kiszka 2011-01-04 13:14 ` [PATCH 3/4] KVM: x86 emulator: vendor specific instructions Avi Kivity ` (2 subsequent siblings) 4 siblings, 1 reply; 9+ messages in thread From: Avi Kivity @ 2011-01-04 13:14 UTC (permalink / raw) To: Marcelo Tosatti, kvm 'goto done' leads to a maze of checks and actions, all pointless. Bail out immediately if we can't decode. Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/kvm/x86.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index b20499d..b085ac3 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4392,7 +4392,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, r = x86_decode_insn(&vcpu->arch.emulate_ctxt, insn, insn_len); if (r != EMULATION_OK) - goto done; + return EMULATE_FAIL; trace_kvm_emulate_insn_start(vcpu); -- 1.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] KVM: Simplify exit path on decode failure 2011-01-04 13:14 ` [PATCH 2/4] KVM: Simplify exit path on decode failure Avi Kivity @ 2011-01-10 17:17 ` Jan Kiszka 2011-01-11 9:40 ` Avi Kivity 0 siblings, 1 reply; 9+ messages in thread From: Jan Kiszka @ 2011-01-10 17:17 UTC (permalink / raw) To: Avi Kivity; +Cc: Marcelo Tosatti, kvm Am 04.01.2011 14:14, Avi Kivity wrote: > 'goto done' leads to a maze of checks and actions, all pointless. Bail > out immediately if we can't decode. > > Signed-off-by: Avi Kivity <avi@redhat.com> > --- > arch/x86/kvm/x86.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index b20499d..b085ac3 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4392,7 +4392,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, > > r = x86_decode_insn(&vcpu->arch.emulate_ctxt, insn, insn_len); > if (r != EMULATION_OK) > - goto done; > + return EMULATE_FAIL; This leaves 'done' unused behind, and the compiler mourns. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/4] KVM: Simplify exit path on decode failure 2011-01-10 17:17 ` Jan Kiszka @ 2011-01-11 9:40 ` Avi Kivity 0 siblings, 0 replies; 9+ messages in thread From: Avi Kivity @ 2011-01-11 9:40 UTC (permalink / raw) To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm On 01/10/2011 07:17 PM, Jan Kiszka wrote: > Am 04.01.2011 14:14, Avi Kivity wrote: > > 'goto done' leads to a maze of checks and actions, all pointless. Bail > > out immediately if we can't decode. > > > > Signed-off-by: Avi Kivity<avi@redhat.com> > > --- > > arch/x86/kvm/x86.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index b20499d..b085ac3 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -4392,7 +4392,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, > > > > r = x86_decode_insn(&vcpu->arch.emulate_ctxt, insn, insn_len); > > if (r != EMULATION_OK) > > - goto done; > > + return EMULATE_FAIL; > > This leaves 'done' unused behind, and the compiler mourns. It's already in 'next'. Marcelo can you amend it? -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/4] KVM: x86 emulator: vendor specific instructions 2011-01-04 13:14 [PATCH 0/4] Vendor specific instructions and the emulator Avi Kivity 2011-01-04 13:14 ` [PATCH 1/4] KVM: Fix x86_decode_insn() return code check Avi Kivity 2011-01-04 13:14 ` [PATCH 2/4] KVM: Simplify exit path on decode failure Avi Kivity @ 2011-01-04 13:14 ` Avi Kivity 2011-01-04 13:14 ` [PATCH 4/4] KVM: Drop ad-hoc vendor specific instruction restriction Avi Kivity 2011-01-07 9:49 ` [PATCH 0/4] Vendor specific instructions and the emulator Marcelo Tosatti 4 siblings, 0 replies; 9+ messages in thread From: Avi Kivity @ 2011-01-04 13:14 UTC (permalink / raw) To: Marcelo Tosatti, kvm Mark some instructions as vendor specific, and allow the caller to request emulation only of vendor specific instructions. This is useful in some circumstances (responding to a #UD fault). Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/include/asm/kvm_emulate.h | 1 + arch/x86/kvm/emulate.c | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h index 8e37deb..50ebc32 100644 --- a/arch/x86/include/asm/kvm_emulate.h +++ b/arch/x86/include/asm/kvm_emulate.h @@ -239,6 +239,7 @@ struct x86_emulate_ctxt { int interruptibility; bool perm_ok; /* do not check permissions if true */ + bool only_vendor_specific_insn; bool have_exception; struct x86_exception exception; diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 02a0041..ad46239 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -76,6 +76,7 @@ #define Group (1<<14) /* Bits 3:5 of modrm byte extend opcode */ #define GroupDual (1<<15) /* Alternate decoding of mod == 3 */ /* Misc flags */ +#define VendorSpecific (1<<22) /* Vendor specific instruction */ #define NoAccess (1<<23) /* Don't access memory (lea/invlpg/verr etc) */ #define Op3264 (1<<24) /* Operand is 64b in long mode, 32b otherwise */ #define Undefined (1<<25) /* No Such Instruction */ @@ -2365,7 +2366,8 @@ static struct group_dual group7 = { { D(SrcMem16 | ModRM | Mov | Priv), D(SrcMem | ModRM | ByteOp | Priv | NoAccess), }, { - D(SrcNone | ModRM | Priv), N, N, D(SrcNone | ModRM | Priv), + D(SrcNone | ModRM | Priv | VendorSpecific), N, + N, D(SrcNone | ModRM | Priv | VendorSpecific), D(SrcNone | ModRM | DstMem | Mov), N, D(SrcMem16 | ModRM | Mov | Priv), N, } }; @@ -2489,7 +2491,7 @@ static struct opcode opcode_table[256] = { static struct opcode twobyte_table[256] = { /* 0x00 - 0x0F */ N, GD(0, &group7), N, N, - N, D(ImplicitOps), D(ImplicitOps | Priv), N, + N, D(ImplicitOps | VendorSpecific), D(ImplicitOps | Priv), N, D(ImplicitOps | Priv), D(ImplicitOps | Priv), N, N, N, D(ImplicitOps | ModRM), N, N, /* 0x10 - 0x1F */ @@ -2502,7 +2504,8 @@ static struct opcode twobyte_table[256] = { /* 0x30 - 0x3F */ D(ImplicitOps | Priv), I(ImplicitOps, em_rdtsc), D(ImplicitOps | Priv), N, - D(ImplicitOps), D(ImplicitOps | Priv), N, N, + D(ImplicitOps | VendorSpecific), D(ImplicitOps | Priv | VendorSpecific), + N, N, N, N, N, N, N, N, N, N, /* 0x40 - 0x4F */ X16(D(DstReg | SrcMem | ModRM | Mov)), @@ -2741,6 +2744,9 @@ done_prefixes: if (c->d == 0 || (c->d & Undefined)) return -1; + if (!(c->d & VendorSpecific) && ctxt->only_vendor_specific_insn) + return -1; + if (mode == X86EMUL_MODE_PROT64 && (c->d & Stack)) c->op_bytes = 8; -- 1.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] KVM: Drop ad-hoc vendor specific instruction restriction 2011-01-04 13:14 [PATCH 0/4] Vendor specific instructions and the emulator Avi Kivity ` (2 preceding siblings ...) 2011-01-04 13:14 ` [PATCH 3/4] KVM: x86 emulator: vendor specific instructions Avi Kivity @ 2011-01-04 13:14 ` Avi Kivity 2011-01-07 9:49 ` [PATCH 0/4] Vendor specific instructions and the emulator Marcelo Tosatti 4 siblings, 0 replies; 9+ messages in thread From: Avi Kivity @ 2011-01-04 13:14 UTC (permalink / raw) To: Marcelo Tosatti, kvm Use the new support in the emulator, and drop the ad-hoc code in x86.c. Signed-off-by: Avi Kivity <avi@redhat.com> --- arch/x86/kvm/x86.c | 31 +++---------------------------- 1 files changed, 3 insertions(+), 28 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index b085ac3..8652643 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4390,39 +4390,14 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, vcpu->arch.emulate_ctxt.have_exception = false; vcpu->arch.emulate_ctxt.perm_ok = false; + vcpu->arch.emulate_ctxt.only_vendor_specific_insn + = emulation_type & EMULTYPE_TRAP_UD; + r = x86_decode_insn(&vcpu->arch.emulate_ctxt, insn, insn_len); if (r != EMULATION_OK) return EMULATE_FAIL; trace_kvm_emulate_insn_start(vcpu); - - /* Only allow emulation of specific instructions on #UD - * (namely VMMCALL, sysenter, sysexit, syscall)*/ - if (emulation_type & EMULTYPE_TRAP_UD) { - if (!c->twobyte) - return EMULATE_FAIL; - switch (c->b) { - case 0x01: /* VMMCALL */ - if (c->modrm_mod != 3 || c->modrm_rm != 1) - return EMULATE_FAIL; - break; - case 0x34: /* sysenter */ - case 0x35: /* sysexit */ - if (c->modrm_mod != 0 || c->modrm_rm != 0) - return EMULATE_FAIL; - break; - case 0x05: /* syscall */ - if (c->modrm_mod != 0 || c->modrm_rm != 0) - return EMULATE_FAIL; - break; - default: - return EMULATE_FAIL; - } - - if (!(c->modrm_reg == 0 || c->modrm_reg == 3)) - return EMULATE_FAIL; - } - ++vcpu->stat.insn_emulation; if (r) { if (reexecute_instruction(vcpu, cr2)) -- 1.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/4] Vendor specific instructions and the emulator 2011-01-04 13:14 [PATCH 0/4] Vendor specific instructions and the emulator Avi Kivity ` (3 preceding siblings ...) 2011-01-04 13:14 ` [PATCH 4/4] KVM: Drop ad-hoc vendor specific instruction restriction Avi Kivity @ 2011-01-07 9:49 ` Marcelo Tosatti 4 siblings, 0 replies; 9+ messages in thread From: Marcelo Tosatti @ 2011-01-07 9:49 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm On Tue, Jan 04, 2011 at 03:14:40PM +0200, Avi Kivity wrote: > Currently we have some ad-hoc code in x86.c to restrict #UD emulation to > expected instructions (that is, vendor specific instructions). This patchset > replaces the ad-hoc code with proper emulator support using decode tables. > > Avi Kivity (4): > KVM: Fix x86_decode_insn() return code check > KVM: Simplify exit path on decode failure > KVM: x86 emulator: vendor specific instructions > KVM: Drop ad-hoc vendor specific instruction restriction > > arch/x86/include/asm/kvm_emulate.h | 1 + > arch/x86/kvm/emulate.c | 12 +++++++++--- > arch/x86/kvm/x86.c | 35 +++++------------------------------ > 3 files changed, 15 insertions(+), 33 deletions(-) Applied, thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-01-13 13:54 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-01-04 13:14 [PATCH 0/4] Vendor specific instructions and the emulator Avi Kivity 2011-01-04 13:14 ` [PATCH 1/4] KVM: Fix x86_decode_insn() return code check Avi Kivity 2011-01-13 13:54 ` Avi Kivity 2011-01-04 13:14 ` [PATCH 2/4] KVM: Simplify exit path on decode failure Avi Kivity 2011-01-10 17:17 ` Jan Kiszka 2011-01-11 9:40 ` Avi Kivity 2011-01-04 13:14 ` [PATCH 3/4] KVM: x86 emulator: vendor specific instructions Avi Kivity 2011-01-04 13:14 ` [PATCH 4/4] KVM: Drop ad-hoc vendor specific instruction restriction Avi Kivity 2011-01-07 9:49 ` [PATCH 0/4] Vendor specific instructions and the emulator Marcelo Tosatti
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.