[PATCH v3 0/3] Vendor specific instructions and the emulator

[PATCH v3 0/3] Vendor specific instructions and the emulator

[Avi Kivity]

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 (3):
  KVM: Drop bogus x86_decode_insn() error check
    v2: replaces v1's first two patches
  KVM: x86 emulator: vendor specific instructions
  KVM: Drop ad-hoc vendor specific instruction restriction
    v3: preserve early-exit on #UD (reexecute_instruction() isn't reliable
        since we don't have a valid cr2)

 arch/x86/include/asm/kvm_emulate.h |    1 +
 arch/x86/kvm/emulate.c             |   12 +++++++++---
 arch/x86/kvm/x86.c                 |   36 +++++-------------------------------
 3 files changed, 15 insertions(+), 34 deletions(-)

[PATCH v3 1/3] KVM: Drop bogus x86_decode_insn() error check

[Avi Kivity]

x86_decode_insn() doesn't return X86EMUL_* values, so the check
for X86EMUL_PROPOGATE_FAULT will always fail.  There is a proper
check later on, so there is no need for a replacement for this
code.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/x86.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a7f65aa..803604f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4388,8 +4388,6 @@ 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)
-			goto done;
 
 		trace_kvm_emulate_insn_start(vcpu);
 
@@ -4449,7 +4447,6 @@ restart:
 		return handle_emulation_failure(vcpu);
 	}
 
-done:
 	if (vcpu->arch.emulate_ctxt.have_exception) {
 		inject_emulated_exception(vcpu);
 		r = EMULATE_DONE;
-- 
1.7.1

[PATCH v3 2/3] KVM: x86 emulator: vendor specific instructions

[Avi Kivity]

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

[PATCH v3 3/3] KVM: Drop ad-hoc vendor specific instruction restriction

[Avi Kivity]

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 |   33 +++++----------------------------
 1 files changed, 5 insertions(+), 28 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 803604f..bdbe2e8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4387,39 +4387,16 @@ 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);
 
 		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 (emulation_type & EMULTYPE_TRAP_UD)
+				return EMULATE_FAIL;
 			if (reexecute_instruction(vcpu, cr2))
 				return EMULATE_DONE;
 			if (emulation_type & EMULTYPE_SKIP)
-- 
1.7.1

Re: [PATCH v3 0/3] Vendor specific instructions and the emulator

[Marcelo Tosatti]

On Tue, Feb 01, 2011 at 04:32:01PM +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 (3):
>   KVM: Drop bogus x86_decode_insn() error check
>     v2: replaces v1's first two patches
>   KVM: x86 emulator: vendor specific instructions
>   KVM: Drop ad-hoc vendor specific instruction restriction
>     v3: preserve early-exit on #UD (reexecute_instruction() isn't reliable
>         since we don't have a valid cr2)
> 
>  arch/x86/include/asm/kvm_emulate.h |    1 +
>  arch/x86/kvm/emulate.c             |   12 +++++++++---
>  arch/x86/kvm/x86.c                 |   36 +++++-------------------------------
>  3 files changed, 15 insertions(+), 34 deletions(-)

Applied, thanks.