[PATCH 0/13] KVM: Make the instruction emulator aware of Nested Virtualization v3

[PATCH 0/13] KVM: Make the instruction emulator aware of Nested Virtualization v3

[Joerg Roedel]

Hi,

this is version 3 of the instruction intercept check framework and svm
code.  This version addresses the review comments of Avi and Gleb. There
were no other changes beside that since v2.

Regards,

	Joerg

Diffstat:

 arch/x86/include/asm/kvm_emulate.h |   85 ++++++++++++
 arch/x86/include/asm/kvm_host.h    |    7 +
 arch/x86/kvm/emulate.c             |  148 +++++++++++++++++----
 arch/x86/kvm/svm.c                 |  265 +++++++++++++++++++++++++++++-------
 arch/x86/kvm/vmx.c                 |    9 ++
 arch/x86/kvm/x86.c                 |   12 ++
 6 files changed, 448 insertions(+), 78 deletions(-)

Shortlog:

Avi Kivity (2):
      KVM: x86 emulator: add framework for instruction intercepts
      KVM: x86 emulator: add SVM intercepts

Joerg Roedel (11):
      KVM: X86: Don't write-back cpu-state on X86EMUL_INTERCEPTED
      KVM: X86: Add x86 callback for intercept check
      KVM: SVM: Add intercept check for emulated cr accesses
      KVM: SVM: Add intercept check for accessing dr registers
      KVM: SVM: Add intercept checks for descriptor table accesses
      KVM: SVM: Add intercept checks for SVM instructions
      KVM: SVM: Add intercept checks for remaining group7 instructions
      KVM: SVM: Add intercept checks for remaining twobyte instructions
      KVM: SVM: Add intercept checks for one-byte instructions
      KVM: SVM: Add checks for IO instructions
      KVM: SVM: Remove nested sel_cr0_write handling code

[PATCH 01/13] KVM: x86 emulator: add framework for instruction intercepts

[Joerg Roedel]

From: Avi Kivity <avi@redhat.com>

When running in guest mode, certain instructions can be intercepted by
hardware.  This also holds for nested guests running on emulated
virtualization hardware, in particular instructions emulated by kvm
itself.

This patch adds a framework for intercepting instructions.  If an
instruction is marked for interception, and if we're running in guest
mode, a callback is called to check whether an intercept is needed or
not.  The callback is called at three points in time: immediately after
beginning execution, after checking privilge exceptions, and after
checking memory exception.  This suits the different interception points
defined for different instructions and for the various virtualization
instruction sets.

In addition, a new X86EMUL_INTERCEPT is defined, which any callback or
memory access may define, allowing the more complicated intercepts to be
implemented in existing callbacks.

Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_emulate.h |   20 ++++++++++++++++++++
 arch/x86/kvm/emulate.c             |   26 ++++++++++++++++++++++++++
 arch/x86/kvm/x86.c                 |    9 +++++++++
 3 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 0f52135..92e251d 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -14,6 +14,8 @@
 #include <asm/desc_defs.h>
 
 struct x86_emulate_ctxt;
+enum x86_intercept;
+enum x86_intercept_stage;
 
 struct x86_exception {
 	u8 vector;
@@ -62,6 +64,7 @@ struct x86_exception {
 #define X86EMUL_RETRY_INSTR     3 /* retry the instruction for some reason */
 #define X86EMUL_CMPXCHG_FAILED  4 /* cmpxchg did not see expected value */
 #define X86EMUL_IO_NEEDED       5 /* IO is needed to complete emulation */
+#define X86EMUL_INTERCEPTED     6 /* Intercepted by nested VMCB/VMCS */
 
 struct x86_emulate_ops {
 	/*
@@ -158,6 +161,9 @@ struct x86_emulate_ops {
 	int (*set_dr)(int dr, unsigned long value, struct kvm_vcpu *vcpu);
 	int (*set_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 data);
 	int (*get_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata);
+	int (*intercept)(struct x86_emulate_ctxt *ctxt,
+			 enum x86_intercept intercept,
+			 enum x86_intercept_stage stage);
 };
 
 /* Type, address-of, and value of an instruction's operand. */
@@ -197,6 +203,7 @@ struct read_cache {
 struct decode_cache {
 	u8 twobyte;
 	u8 b;
+	u8 intercept;
 	u8 lock_prefix;
 	u8 rep_prefix;
 	u8 op_bytes;
@@ -238,6 +245,7 @@ struct x86_emulate_ctxt {
 	/* interruptibility state, as a result of execution of STI or MOV SS */
 	int interruptibility;
 
+	bool guest_mode; /* guest running a nested guest */
 	bool perm_ok; /* do not check permissions if true */
 	bool only_vendor_specific_insn;
 
@@ -259,6 +267,18 @@ struct x86_emulate_ctxt {
 #define X86EMUL_MODE_PROT32   4	/* 32-bit protected mode. */
 #define X86EMUL_MODE_PROT64   8	/* 64-bit (long) mode.    */
 
+enum x86_intercept_stage {
+	X86_ICPT_PRE_EXCEPT,
+	X86_ICPT_POST_EXCEPT,
+	X86_ICPT_POST_MEMACCESS,
+};
+
+enum x86_intercept {
+	x86_intercept_none,
+
+	nr_x86_intercepts
+};
+
 /* Host execution mode. */
 #if defined(CONFIG_X86_32)
 #define X86EMUL_MODE_HOST X86EMUL_MODE_PROT32
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 14c5ad5..1c574ae 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -102,6 +102,7 @@
 
 struct opcode {
 	u32 flags;
+	u8 intercept;
 	union {
 		int (*execute)(struct x86_emulate_ctxt *ctxt);
 		struct opcode *group;
@@ -2326,10 +2327,13 @@ static int em_mov(struct x86_emulate_ctxt *ctxt)
 }
 
 #define D(_y) { .flags = (_y) }
+#define DI(_y, _i) { .flags = (_y), .intercept = x86_intercept_##_i }
 #define N    D(0)
 #define G(_f, _g) { .flags = ((_f) | Group), .u.group = (_g) }
 #define GD(_f, _g) { .flags = ((_f) | Group | GroupDual), .u.gdual = (_g) }
 #define I(_f, _e) { .flags = (_f), .u.execute = (_e) }
+#define II(_f, _e, _i) \
+	{ .flags = (_f), .u.execute = (_e), .intercept = x86_intercept_##_i }
 
 #define D2bv(_f)      D((_f) | ByteOp), D(_f)
 #define I2bv(_f, _e)  I((_f) | ByteOp, _e), I(_f, _e)
@@ -2745,6 +2749,7 @@ done_prefixes:
 	}
 
 	c->execute = opcode.u.execute;
+	c->intercept = opcode.intercept;
 
 	/* Unrecognised? */
 	if (c->d == 0 || (c->d & Undefined))
@@ -2979,12 +2984,26 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 		goto done;
 	}
 
+	if (unlikely(ctxt->guest_mode) && c->intercept) {
+		rc = ops->intercept(ctxt, c->intercept,
+				    X86_ICPT_PRE_EXCEPT);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
+	}
+
 	/* Privileged instruction can be executed only in CPL=0 */
 	if ((c->d & Priv) && ops->cpl(ctxt->vcpu)) {
 		rc = emulate_gp(ctxt, 0);
 		goto done;
 	}
 
+	if (unlikely(ctxt->guest_mode) && c->intercept) {
+		rc = ops->intercept(ctxt, c->intercept,
+				    X86_ICPT_POST_EXCEPT);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
+	}
+
 	if (c->rep_prefix && (c->d & String)) {
 		/* All REP prefixes have the same first termination condition */
 		if (address_mask(c, c->regs[VCPU_REGS_RCX]) == 0) {
@@ -3023,6 +3042,13 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 
 special_insn:
 
+	if (unlikely(ctxt->guest_mode) && c->intercept) {
+		rc = ops->intercept(ctxt, c->intercept,
+				    X86_ICPT_POST_MEMACCESS);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
+	}
+
 	if (c->execute) {
 		rc = c->execute(ctxt);
 		if (rc != X86EMUL_CONTINUE)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index bfd7763..829ea7c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4242,6 +4242,13 @@ static void emulator_set_segment_selector(u16 sel, int seg,
 	kvm_set_segment(vcpu, &kvm_seg, seg);
 }
 
+static int emulator_intercept(struct x86_emulate_ctxt *ctxt,
+			      enum x86_intercept intercept,
+			      enum x86_intercept_stage stage)
+{
+	return X86EMUL_CONTINUE;
+}
+
 static struct x86_emulate_ops emulate_ops = {
 	.read_std            = kvm_read_guest_virt_system,
 	.write_std           = kvm_write_guest_virt_system,
@@ -4265,6 +4272,7 @@ static struct x86_emulate_ops emulate_ops = {
 	.set_dr              = emulator_set_dr,
 	.set_msr             = kvm_set_msr,
 	.get_msr             = kvm_get_msr,
+	.intercept           = emulator_intercept,
 };
 
 static void cache_all_regs(struct kvm_vcpu *vcpu)
@@ -4319,6 +4327,7 @@ static void init_emulate_ctxt(struct kvm_vcpu *vcpu)
 		? X86EMUL_MODE_VM86 : cs_l
 		? X86EMUL_MODE_PROT64 :	cs_db
 		? X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16;
+	vcpu->arch.emulate_ctxt.guest_mode = is_guest_mode(vcpu);
 	memset(c, 0, sizeof(struct decode_cache));
 	memcpy(c->regs, vcpu->arch.regs, sizeof c->regs);
 }
-- 
1.7.1

[PATCH 02/13] KVM: x86 emulator: add SVM intercepts

[Joerg Roedel]

From: Avi Kivity <avi@redhat.com>

Add intercept codes for instructions defined by SVM as
interceptable.

Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_emulate.h |   35 +++++++++++++++++++++++++++++++++++
 arch/x86/kvm/emulate.c             |   24 +++++++++++++-----------
 2 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 92e251d..436207e 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -275,6 +275,41 @@ enum x86_intercept_stage {
 
 enum x86_intercept {
 	x86_intercept_none,
+	x86_intercept_lmsw,
+	x86_intercept_smsw,
+	x86_intercept_lidt,
+	x86_intercept_sidt,
+	x86_intercept_lgdt,
+	x86_intercept_sgdt,
+	x86_intercept_lldt,
+	x86_intercept_sldt,
+	x86_intercept_ltr,
+	x86_intercept_str,
+	x86_intercept_rdtsc,
+	x86_intercept_rdpmc,
+	x86_intercept_pushf,
+	x86_intercept_popf,
+	x86_intercept_cpuid,
+	x86_intercept_rsm,
+	x86_intercept_iret,
+	x86_intercept_intn,
+	x86_intercept_invd,
+	x86_intercept_pause,
+	x86_intercept_hlt,
+	x86_intercept_invlpg,
+	x86_intercept_invlpga,
+	x86_intercept_vmrun,
+	x86_intercept_vmload,
+	x86_intercept_vmsave,
+	x86_intercept_vmmcall,
+	x86_intercept_stgi,
+	x86_intercept_clgi,
+	x86_intercept_skinit,
+	x86_intercept_rdtscp,
+	x86_intercept_icebp,
+	x86_intercept_wbinvd,
+	x86_intercept_monitor,
+	x86_intercept_mwait,
 
 	nr_x86_intercepts
 };
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 1c574ae..cee6fc1 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2371,15 +2371,15 @@ static struct opcode group5[] = {
 };
 
 static struct group_dual group7 = { {
-	N, N, D(ModRM | SrcMem | Priv), D(ModRM | SrcMem | Priv),
-	D(SrcNone | ModRM | DstMem | Mov), N,
-	D(SrcMem16 | ModRM | Mov | Priv),
-	D(SrcMem | ModRM | ByteOp | Priv | NoAccess),
+	N, N, DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
+	DI(SrcNone | ModRM | DstMem | Mov, smsw), N,
+	DI(SrcMem16 | ModRM | Mov | Priv, lmsw),
+	DI(SrcMem | ModRM | ByteOp | Priv | NoAccess, invlpg),
 }, {
 	D(SrcNone | ModRM | Priv | VendorSpecific), N,
 	N, D(SrcNone | ModRM | Priv | VendorSpecific),
-	D(SrcNone | ModRM | DstMem | Mov), N,
-	D(SrcMem16 | ModRM | Mov | Priv), N,
+	DI(SrcNone | ModRM | DstMem | Mov, smsw), N,
+	DI(SrcMem16 | ModRM | Mov | Priv, lmsw), N,
 } };
 
 static struct opcode group8[] = {
@@ -2454,7 +2454,7 @@ static struct opcode opcode_table[256] = {
 	/* 0x98 - 0x9F */
 	D(DstAcc | SrcNone), I(ImplicitOps | SrcAcc, em_cwd),
 	I(SrcImmFAddr | No64, em_call_far), N,
-	D(ImplicitOps | Stack), D(ImplicitOps | Stack), N, N,
+	DI(ImplicitOps | Stack, pushf), DI(ImplicitOps | Stack, popf), N, N,
 	/* 0xA0 - 0xA7 */
 	I2bv(DstAcc | SrcMem | Mov | MemAbs, em_mov),
 	I2bv(DstMem | SrcAcc | Mov | MemAbs, em_mov),
@@ -2477,7 +2477,8 @@ static struct opcode opcode_table[256] = {
 	G(ByteOp, group11), G(0, group11),
 	/* 0xC8 - 0xCF */
 	N, N, N, D(ImplicitOps | Stack),
-	D(ImplicitOps), D(SrcImmByte), D(ImplicitOps | No64), D(ImplicitOps),
+	D(ImplicitOps), DI(SrcImmByte, intn),
+	D(ImplicitOps | No64), DI(ImplicitOps, iret),
 	/* 0xD0 - 0xD7 */
 	D2bv(DstMem | SrcOne | ModRM), D2bv(DstMem | ModRM),
 	N, N, N, N,
@@ -2492,7 +2493,8 @@ static struct opcode opcode_table[256] = {
 	D2bv(SrcNone | DstAcc),	D2bv(SrcAcc | ImplicitOps),
 	/* 0xF0 - 0xF7 */
 	N, N, N, N,
-	D(ImplicitOps | Priv), D(ImplicitOps), G(ByteOp, group3), G(0, group3),
+	DI(ImplicitOps | Priv, hlt), D(ImplicitOps),
+	G(ByteOp, group3), G(0, group3),
 	/* 0xF8 - 0xFF */
 	D(ImplicitOps), D(ImplicitOps), D(ImplicitOps), D(ImplicitOps),
 	D(ImplicitOps), D(ImplicitOps), G(0, group4), G(0, group5),
@@ -2502,7 +2504,7 @@ static struct opcode twobyte_table[256] = {
 	/* 0x00 - 0x0F */
 	N, GD(0, &group7), N, N,
 	N, D(ImplicitOps | VendorSpecific), D(ImplicitOps | Priv), N,
-	D(ImplicitOps | Priv), D(ImplicitOps | Priv), N, N,
+	DI(ImplicitOps | Priv, invd), DI(ImplicitOps | Priv, wbinvd), N, N,
 	N, D(ImplicitOps | ModRM), N, N,
 	/* 0x10 - 0x1F */
 	N, N, N, N, N, N, N, N, D(ImplicitOps | ModRM), N, N, N, N, N, N, N,
@@ -2512,7 +2514,7 @@ static struct opcode twobyte_table[256] = {
 	N, N, N, N,
 	N, N, N, N, N, N, N, N,
 	/* 0x30 - 0x3F */
-	D(ImplicitOps | Priv), I(ImplicitOps, em_rdtsc),
+	D(ImplicitOps | Priv), II(ImplicitOps, em_rdtsc, rdtsc),
 	D(ImplicitOps | Priv), N,
 	D(ImplicitOps | VendorSpecific), D(ImplicitOps | Priv | VendorSpecific),
 	N, N,
-- 
1.7.1

[PATCH 03/13] KVM: X86: Don't write-back cpu-state on X86EMUL_INTERCEPTED

[Joerg Roedel]

This patch prevents the changed CPU state to be written back
when the emulator detected that the instruction was
intercepted by the guest.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_emulate.h |    1 +
 arch/x86/kvm/emulate.c             |    3 +++
 arch/x86/kvm/x86.c                 |    3 +++
 3 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 436207e..366de63 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -325,6 +325,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len);
 #define EMULATION_FAILED -1
 #define EMULATION_OK 0
 #define EMULATION_RESTART 1
+#define EMULATION_INTERCEPTED 2
 int x86_emulate_insn(struct x86_emulate_ctxt *ctxt);
 int emulator_task_switch(struct x86_emulate_ctxt *ctxt,
 			 u16 tss_selector, int reason,
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index cee6fc1..b05e50d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3455,6 +3455,9 @@ writeback:
 done:
 	if (rc == X86EMUL_PROPAGATE_FAULT)
 		ctxt->have_exception = true;
+	if (rc == X86EMUL_INTERCEPTED)
+		return EMULATION_INTERCEPTED;
+
 	return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED : EMULATION_OK;
 
 twobyte_insn:
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 829ea7c..292b158 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4459,6 +4459,9 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
 restart:
 	r = x86_emulate_insn(&vcpu->arch.emulate_ctxt);
 
+	if (r == EMULATION_INTERCEPTED)
+		return EMULATE_DONE;
+
 	if (r == EMULATION_FAILED) {
 		if (reexecute_instruction(vcpu, cr2))
 			return EMULATE_DONE;
-- 
1.7.1

[PATCH 04/13] KVM: X86: Add x86 callback for intercept check

[Joerg Roedel]

This patch adds a callback into kvm_x86_ops so that svm and
vmx code can do intercept checks on emulated instructions.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_emulate.h |   22 ++++++++++++++++++++--
 arch/x86/include/asm/kvm_host.h    |    7 +++++++
 arch/x86/kvm/emulate.c             |   32 ++++++++++++++++++++++++++------
 arch/x86/kvm/svm.c                 |    9 +++++++++
 arch/x86/kvm/vmx.c                 |    9 +++++++++
 arch/x86/kvm/x86.c                 |    6 +++---
 6 files changed, 74 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 366de63..99ea5b4 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -26,6 +26,24 @@ struct x86_exception {
 };
 
 /*
+ * This struct is used to carry enough information from the instruction
+ * decoder to main KVM so that a decision can be made whether the
+ * instruction needs to be intercepted or not.
+ */
+struct x86_instruction_info {
+	u8  intercept;          /* which intercept                      */
+	u8  rep_prefix;         /* rep prefix?                          */
+	u8  modrm_mod;		/* mod part of modrm			*/
+	u8  modrm_reg;          /* index of register used               */
+	u8  modrm_rm;		/* rm part of modrm			*/
+	u64 src_val;            /* value of source operand              */
+	u8  src_bytes;          /* size of source operand               */
+	u8  dst_bytes;          /* size of destination operand          */
+	u8  ad_bytes;           /* size of src/dst address              */
+	u64 next_rip;           /* rip following the instruction        */
+};
+
+/*
  * x86_emulate_ops:
  *
  * These operations represent the instruction emulator's interface to memory.
@@ -161,8 +179,8 @@ struct x86_emulate_ops {
 	int (*set_dr)(int dr, unsigned long value, struct kvm_vcpu *vcpu);
 	int (*set_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 data);
 	int (*get_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata);
-	int (*intercept)(struct x86_emulate_ctxt *ctxt,
-			 enum x86_intercept intercept,
+	int (*intercept)(struct kvm_vcpu *vcpu,
+			 struct x86_instruction_info *info,
 			 enum x86_intercept_stage stage);
 };
 
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 35f81b1..4ef32ac 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -504,6 +504,8 @@ struct kvm_vcpu_stat {
 	u32 nmi_injections;
 };
 
+struct x86_instruction_info;
+
 struct kvm_x86_ops {
 	int (*cpu_has_kvm_support)(void);          /* __init */
 	int (*disabled_by_bios)(void);             /* __init */
@@ -591,6 +593,11 @@ struct kvm_x86_ops {
 	void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
 
 	void (*get_exit_info)(struct kvm_vcpu *vcpu, u64 *info1, u64 *info2);
+
+	int (*check_intercept)(struct kvm_vcpu *vcpu,
+			       struct x86_instruction_info *info,
+			       enum x86_intercept_stage stage);
+
 	const struct trace_print_flags *exit_reasons_str;
 };
 
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index b05e50d..46dd5bf 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -396,6 +396,26 @@ struct group_dual {
 	(_eip) += (_size);						\
 })
 
+static int emulator_check_intercept(struct x86_emulate_ctxt *ctxt,
+				    enum x86_intercept intercept,
+				    enum x86_intercept_stage stage)
+{
+	struct x86_instruction_info info = {
+		.intercept  = intercept,
+		.rep_prefix = ctxt->decode.rep_prefix,
+		.modrm_mod  = ctxt->decode.modrm_mod,
+		.modrm_reg  = ctxt->decode.modrm_reg,
+		.modrm_rm   = ctxt->decode.modrm_rm,
+		.src_val    = ctxt->decode.src.val64,
+		.src_bytes  = ctxt->decode.src.bytes,
+		.dst_bytes  = ctxt->decode.dst.bytes,
+		.ad_bytes   = ctxt->decode.ad_bytes,
+		.next_rip   = ctxt->eip,
+	};
+
+	return ctxt->ops->intercept(ctxt->vcpu, &info, stage);
+}
+
 static inline unsigned long ad_mask(struct decode_cache *c)
 {
 	return (1UL << (c->ad_bytes << 3)) - 1;
@@ -2987,8 +3007,8 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 	}
 
 	if (unlikely(ctxt->guest_mode) && c->intercept) {
-		rc = ops->intercept(ctxt, c->intercept,
-				    X86_ICPT_PRE_EXCEPT);
+		rc = emulator_check_intercept(ctxt, c->intercept,
+					      X86_ICPT_PRE_EXCEPT);
 		if (rc != X86EMUL_CONTINUE)
 			goto done;
 	}
@@ -3000,8 +3020,8 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 	}
 
 	if (unlikely(ctxt->guest_mode) && c->intercept) {
-		rc = ops->intercept(ctxt, c->intercept,
-				    X86_ICPT_POST_EXCEPT);
+		rc = emulator_check_intercept(ctxt, c->intercept,
+					      X86_ICPT_POST_EXCEPT);
 		if (rc != X86EMUL_CONTINUE)
 			goto done;
 	}
@@ -3045,8 +3065,8 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 special_insn:
 
 	if (unlikely(ctxt->guest_mode) && c->intercept) {
-		rc = ops->intercept(ctxt, c->intercept,
-				    X86_ICPT_POST_MEMACCESS);
+		rc = emulator_check_intercept(ctxt, c->intercept,
+					      X86_ICPT_POST_MEMACCESS);
 		if (rc != X86EMUL_CONTINUE)
 			goto done;
 	}
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index cb43e98..798ebe6 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3868,6 +3868,13 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
 	update_cr0_intercept(svm);
 }
 
+static int svm_check_intercept(struct kvm_vcpu *vcpu,
+			       struct x86_instruction_info *info,
+			       enum x86_intercept_stage stage)
+{
+	return X86EMUL_CONTINUE;
+}
+
 static struct kvm_x86_ops svm_x86_ops = {
 	.cpu_has_kvm_support = has_svm,
 	.disabled_by_bios = is_disabled,
@@ -3953,6 +3960,8 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.adjust_tsc_offset = svm_adjust_tsc_offset,
 
 	.set_tdp_cr3 = set_tdp_cr3,
+
+	.check_intercept = svm_check_intercept,
 };
 
 static int __init svm_init(void)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2b99ae7..3dfefe3 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4409,6 +4409,13 @@ static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
 {
 }
 
+static int vmx_check_intercept(struct kvm_vcpu *vcpu,
+			       struct x86_instruction_info *info,
+			       enum x86_intercept_stage stage)
+{
+	return X86EMUL_CONTINUE;
+}
+
 static struct kvm_x86_ops vmx_x86_ops = {
 	.cpu_has_kvm_support = cpu_has_kvm_support,
 	.disabled_by_bios = vmx_disabled_by_bios,
@@ -4494,6 +4501,8 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.adjust_tsc_offset = vmx_adjust_tsc_offset,
 
 	.set_tdp_cr3 = vmx_set_cr3,
+
+	.check_intercept = vmx_check_intercept,
 };
 
 static int __init vmx_init(void)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 292b158..2b43606 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4242,11 +4242,11 @@ static void emulator_set_segment_selector(u16 sel, int seg,
 	kvm_set_segment(vcpu, &kvm_seg, seg);
 }
 
-static int emulator_intercept(struct x86_emulate_ctxt *ctxt,
-			      enum x86_intercept intercept,
+static int emulator_intercept(struct kvm_vcpu *vcpu,
+			      struct x86_instruction_info *info,
 			      enum x86_intercept_stage stage)
 {
-	return X86EMUL_CONTINUE;
+	return kvm_x86_ops->check_intercept(vcpu, info, stage);
 }
 
 static struct x86_emulate_ops emulate_ops = {
-- 
1.7.1

[PATCH 05/13] KVM: SVM: Add intercept check for emulated cr accesses

[Joerg Roedel]

This patch adds all necessary intercept checks for
instructions that access the crX registers.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_emulate.h |    3 +
 arch/x86/kvm/emulate.c             |    8 ++-
 arch/x86/kvm/svm.c                 |   81 +++++++++++++++++++++++++++++++++++-
 3 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 99ea5b4..7223096 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -293,6 +293,9 @@ enum x86_intercept_stage {
 
 enum x86_intercept {
 	x86_intercept_none,
+	x86_intercept_cr_read,
+	x86_intercept_cr_write,
+	x86_intercept_clts,
 	x86_intercept_lmsw,
 	x86_intercept_smsw,
 	x86_intercept_lidt,
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 46dd5bf..d605dcb 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2523,14 +2523,16 @@ static struct opcode opcode_table[256] = {
 static struct opcode twobyte_table[256] = {
 	/* 0x00 - 0x0F */
 	N, GD(0, &group7), N, N,
-	N, D(ImplicitOps | VendorSpecific), D(ImplicitOps | Priv), N,
+	N, D(ImplicitOps | VendorSpecific), DI(ImplicitOps | Priv, clts), N,
 	DI(ImplicitOps | Priv, invd), DI(ImplicitOps | Priv, wbinvd), N, N,
 	N, D(ImplicitOps | ModRM), N, N,
 	/* 0x10 - 0x1F */
 	N, N, N, N, N, N, N, N, D(ImplicitOps | ModRM), N, N, N, N, N, N, N,
 	/* 0x20 - 0x2F */
-	D(ModRM | DstMem | Priv | Op3264), D(ModRM | DstMem | Priv | Op3264),
-	D(ModRM | SrcMem | Priv | Op3264), D(ModRM | SrcMem | Priv | Op3264),
+	DI(ModRM | DstMem | Priv | Op3264, cr_read),
+	D(ModRM | DstMem | Priv | Op3264),
+	DI(ModRM | SrcMem | Priv | Op3264, cr_write),
+	D(ModRM | SrcMem | Priv | Op3264),
 	N, N, N, N,
 	N, N, N, N, N, N, N, N,
 	/* 0x30 - 0x3F */
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 798ebe6..ff4ed36 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3868,11 +3868,90 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
 	update_cr0_intercept(svm);
 }
 
+#define POST_EX(exit) { .exit_code = (exit), \
+			.stage = X86_ICPT_POST_EXCEPT, \
+			.valid = true }
+
+static struct __x86_intercept {
+	u32 exit_code;
+	enum x86_intercept_stage stage;
+	bool valid;
+} x86_intercept_map[] = {
+	[x86_intercept_cr_read]		= POST_EX(SVM_EXIT_READ_CR0),
+	[x86_intercept_cr_write]	= POST_EX(SVM_EXIT_WRITE_CR0),
+	[x86_intercept_clts]		= POST_EX(SVM_EXIT_WRITE_CR0),
+	[x86_intercept_lmsw]		= POST_EX(SVM_EXIT_WRITE_CR0),
+	[x86_intercept_smsw]		= POST_EX(SVM_EXIT_READ_CR0),
+};
+
+#undef POST_EX
+
 static int svm_check_intercept(struct kvm_vcpu *vcpu,
 			       struct x86_instruction_info *info,
 			       enum x86_intercept_stage stage)
 {
-	return X86EMUL_CONTINUE;
+	struct vcpu_svm *svm = to_svm(vcpu);
+	int vmexit, ret = X86EMUL_CONTINUE;
+	struct __x86_intercept icpt_info;
+	struct vmcb *vmcb = svm->vmcb;
+
+	if (info->intercept >= ARRAY_SIZE(x86_intercept_map))
+		goto out;
+
+	icpt_info = x86_intercept_map[info->intercept];
+
+	if (!icpt_info.valid || stage != icpt_info.stage)
+		goto out;
+
+	switch (icpt_info.exit_code) {
+	case SVM_EXIT_READ_CR0:
+		if (info->intercept == x86_intercept_cr_read)
+			icpt_info.exit_code += info->modrm_reg;
+		break;
+	case SVM_EXIT_WRITE_CR0: {
+		unsigned long cr0, val;
+		u64 intercept;
+
+		if (info->intercept == x86_intercept_cr_write)
+			icpt_info.exit_code += info->modrm_reg;
+
+		if (icpt_info.exit_code != SVM_EXIT_WRITE_CR0)
+			break;
+
+		intercept = svm->nested.intercept;
+
+		if (!(intercept & (1ULL << INTERCEPT_SELECTIVE_CR0)))
+			break;
+
+		cr0 = vcpu->arch.cr0 & ~SVM_CR0_SELECTIVE_MASK;
+		val = info->src_val  & ~SVM_CR0_SELECTIVE_MASK;
+
+		if (info->intercept == x86_intercept_lmsw) {
+			cr0 &= 0xfUL;
+			val &= 0xfUL;
+			/* lmsw can't clear PE - catch this here */
+			if (cr0 & X86_CR0_PE)
+				val |= X86_CR0_PE;
+		}
+
+		if (cr0 ^ val)
+			icpt_info.exit_code = SVM_EXIT_CR0_SEL_WRITE;
+
+		break;
+	}
+	default:
+		break;
+	}
+
+	vmcb->control.next_rip  = info->next_rip;
+	vmcb->control.exit_code = icpt_info.exit_code;
+	vmexit = nested_svm_exit_handled(svm);
+
+	ret = (vmexit == NESTED_EXIT_DONE) ? X86EMUL_INTERCEPTED
+					   : X86EMUL_CONTINUE;
+
+out:
+	return ret;
 }
 
 static struct kvm_x86_ops svm_x86_ops = {
-- 
1.7.1

[PATCH 06/13] KVM: SVM: Add intercept check for accessing dr registers

[Joerg Roedel]

This patch adds the intercept checks for instruction
accessing the debug registers.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_emulate.h |    2 ++
 arch/x86/kvm/emulate.c             |    4 ++--
 arch/x86/kvm/svm.c                 |    6 ++++++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 7223096..cdc9026 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -298,6 +298,8 @@ enum x86_intercept {
 	x86_intercept_clts,
 	x86_intercept_lmsw,
 	x86_intercept_smsw,
+	x86_intercept_dr_read,
+	x86_intercept_dr_write,
 	x86_intercept_lidt,
 	x86_intercept_sidt,
 	x86_intercept_lgdt,
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index d605dcb..e589bcc 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2530,9 +2530,9 @@ static struct opcode twobyte_table[256] = {
 	N, N, N, N, N, N, N, N, D(ImplicitOps | ModRM), N, N, N, N, N, N, N,
 	/* 0x20 - 0x2F */
 	DI(ModRM | DstMem | Priv | Op3264, cr_read),
-	D(ModRM | DstMem | Priv | Op3264),
+	DI(ModRM | DstMem | Priv | Op3264, dr_read),
 	DI(ModRM | SrcMem | Priv | Op3264, cr_write),
-	D(ModRM | SrcMem | Priv | Op3264),
+	DI(ModRM | SrcMem | Priv | Op3264, dr_write),
 	N, N, N, N,
 	N, N, N, N, N, N, N, N,
 	/* 0x30 - 0x3F */
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ff4ed36..381b038 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3882,6 +3882,8 @@ static struct __x86_intercept {
 	[x86_intercept_clts]		= POST_EX(SVM_EXIT_WRITE_CR0),
 	[x86_intercept_lmsw]		= POST_EX(SVM_EXIT_WRITE_CR0),
 	[x86_intercept_smsw]		= POST_EX(SVM_EXIT_READ_CR0),
+	[x86_intercept_dr_read]		= POST_EX(SVM_EXIT_READ_DR0),
+	[x86_intercept_dr_write]	= POST_EX(SVM_EXIT_WRITE_DR0),
 };
 
 #undef POST_EX
@@ -3939,6 +3941,10 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
 
 		break;
 	}
+	case SVM_EXIT_READ_DR0:
+	case SVM_EXIT_WRITE_DR0:
+		icpt_info.exit_code += info->modrm_reg;
+		break;
 	default:
 		break;
 	}
-- 
1.7.1

[PATCH 07/13] KVM: SVM: Add intercept checks for descriptor table accesses

[Joerg Roedel]

This patch add intercept checks into the KVM instruction
emulator to check for the 8 instructions that access the
descriptor table addresses.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/emulate.c |   13 +++++++++++--
 arch/x86/kvm/svm.c     |   13 +++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index e589bcc..ec2e9ad 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2390,8 +2390,17 @@ static struct opcode group5[] = {
 	D(SrcMem | ModRM | Stack), N,
 };
 
+static struct opcode group6[] = {
+	DI(ModRM,	 sldt),
+	DI(ModRM,	 str),
+	DI(ModRM | Priv, lldt),
+	DI(ModRM | Priv, ltr),
+	N, N, N, N,
+};
+
 static struct group_dual group7 = { {
-	N, N, DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
+	DI(ModRM | DstMem | Priv, sgdt), DI(ModRM | DstMem | Priv, sidt),
+	DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
 	DI(SrcNone | ModRM | DstMem | Mov, smsw), N,
 	DI(SrcMem16 | ModRM | Mov | Priv, lmsw),
 	DI(SrcMem | ModRM | ByteOp | Priv | NoAccess, invlpg),
@@ -2522,7 +2531,7 @@ static struct opcode opcode_table[256] = {
 
 static struct opcode twobyte_table[256] = {
 	/* 0x00 - 0x0F */
-	N, GD(0, &group7), N, N,
+	G(0, group6), GD(0, &group7), N, N,
 	N, D(ImplicitOps | VendorSpecific), DI(ImplicitOps | Priv, clts), N,
 	DI(ImplicitOps | Priv, invd), DI(ImplicitOps | Priv, wbinvd), N, N,
 	N, D(ImplicitOps | ModRM), N, N,
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 381b038..485a09f 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3871,6 +3871,10 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
 #define POST_EX(exit) { .exit_code = (exit), \
 			.stage = X86_ICPT_POST_EXCEPT, \
 			.valid = true }
+#define POST_MEM(exit) { .exit_code = (exit), \
+			 .stage = X86_ICPT_POST_MEMACCESS, \
+			 .valid = true }
+
 
 static struct __x86_intercept {
 	u32 exit_code;
@@ -3884,9 +3888,18 @@ static struct __x86_intercept {
 	[x86_intercept_smsw]		= POST_EX(SVM_EXIT_READ_CR0),
 	[x86_intercept_dr_read]		= POST_EX(SVM_EXIT_READ_DR0),
 	[x86_intercept_dr_write]	= POST_EX(SVM_EXIT_WRITE_DR0),
+	[x86_intercept_sldt]		= POST_MEM(SVM_EXIT_LDTR_READ),
+	[x86_intercept_str]		= POST_MEM(SVM_EXIT_TR_READ),
+	[x86_intercept_lldt]		= POST_MEM(SVM_EXIT_LDTR_WRITE),
+	[x86_intercept_ltr]		= POST_MEM(SVM_EXIT_TR_WRITE),
+	[x86_intercept_sgdt]		= POST_MEM(SVM_EXIT_GDTR_READ),
+	[x86_intercept_sidt]		= POST_MEM(SVM_EXIT_IDTR_READ),
+	[x86_intercept_lgdt]		= POST_MEM(SVM_EXIT_GDTR_WRITE),
+	[x86_intercept_lidt]		= POST_MEM(SVM_EXIT_IDTR_WRITE),
 };
 
 #undef POST_EX
+#undef POST_MEM
 
 static int svm_check_intercept(struct kvm_vcpu *vcpu,
 			       struct x86_instruction_info *info,
-- 
1.7.1

Re: [PATCH 07/13] KVM: SVM: Add intercept checks for descriptor table accesses

[Avi Kivity]

On 03/28/2011 12:46 PM, Joerg Roedel wrote:
> This patch add intercept checks into the KVM instruction
> emulator to check for the 8 instructions that access the
> descriptor table addresses.
>
> +static struct opcode group6[] = {
> +	DI(ModRM,	 sldt),
> +	DI(ModRM,	 str),
> +	DI(ModRM | Priv, lldt),
> +	DI(ModRM | Priv, ltr),
> +	N, N, N, N,
> +};
> +
>   static struct group_dual group7 = { {
> -	N, N, DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
> +	DI(ModRM | DstMem | Priv, sgdt), DI(ModRM | DstMem | Priv, sidt),
> +	DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),

| Mov, to avoid RMW for SIDT, for example.  Also need to indicate the 
operand size correctly.

> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 381b038..485a09f 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3871,6 +3871,10 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
>   #define POST_EX(exit) { .exit_code = (exit), \
>   			.stage = X86_ICPT_POST_EXCEPT, \
>   			.valid = true }
> +#define POST_MEM(exit) { .exit_code = (exit), \
> +			 .stage = X86_ICPT_POST_MEMACCESS, \
> +			 .valid = true }
> +
>
>   static struct __x86_intercept {
>   	u32 exit_code;
> @@ -3884,9 +3888,18 @@ static struct __x86_intercept {
>   	[x86_intercept_smsw]		= POST_EX(SVM_EXIT_READ_CR0),
>   	[x86_intercept_dr_read]		= POST_EX(SVM_EXIT_READ_DR0),
>   	[x86_intercept_dr_write]	= POST_EX(SVM_EXIT_WRITE_DR0),
> +	[x86_intercept_sldt]		= POST_MEM(SVM_EXIT_LDTR_READ),
> +	[x86_intercept_str]		= POST_MEM(SVM_EXIT_TR_READ),
> +	[x86_intercept_lldt]		= POST_MEM(SVM_EXIT_LDTR_WRITE),
> +	[x86_intercept_ltr]		= POST_MEM(SVM_EXIT_TR_WRITE),
> +	[x86_intercept_sgdt]		= POST_MEM(SVM_EXIT_GDTR_READ),
> +	[x86_intercept_sidt]		= POST_MEM(SVM_EXIT_IDTR_READ),
> +	[x86_intercept_lgdt]		= POST_MEM(SVM_EXIT_GDTR_WRITE),
> +	[x86_intercept_lidt]		= POST_MEM(SVM_EXIT_IDTR_WRITE),
>   };

Spec says POST_EX()?

>
>   #undef POST_EX
> +#undef POST_MEM
>
>   static int svm_check_intercept(struct kvm_vcpu *vcpu,
>   			       struct x86_instruction_info *info,


-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 07/13] KVM: SVM: Add intercept checks for descriptor table accesses

[Roedel, Joerg]

On Mon, Mar 28, 2011 at 08:35:54AM -0400, Avi Kivity wrote:
> On 03/28/2011 12:46 PM, Joerg Roedel wrote:
> > This patch add intercept checks into the KVM instruction
> > emulator to check for the 8 instructions that access the
> > descriptor table addresses.
> >
> > +static struct opcode group6[] = {
> > +	DI(ModRM,	 sldt),
> > +	DI(ModRM,	 str),
> > +	DI(ModRM | Priv, lldt),
> > +	DI(ModRM | Priv, ltr),
> > +	N, N, N, N,
> > +};
> > +
> >   static struct group_dual group7 = { {
> > -	N, N, DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
> > +	DI(ModRM | DstMem | Priv, sgdt), DI(ModRM | DstMem | Priv, sidt),
> > +	DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
> 
> | Mov, to avoid RMW for SIDT, for example.  Also need to indicate the 
> operand size correctly.
> 
> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> > index 381b038..485a09f 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
> > @@ -3871,6 +3871,10 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
> >   #define POST_EX(exit) { .exit_code = (exit), \
> >   			.stage = X86_ICPT_POST_EXCEPT, \
> >   			.valid = true }
> > +#define POST_MEM(exit) { .exit_code = (exit), \
> > +			 .stage = X86_ICPT_POST_MEMACCESS, \
> > +			 .valid = true }
> > +
> >
> >   static struct __x86_intercept {
> >   	u32 exit_code;
> > @@ -3884,9 +3888,18 @@ static struct __x86_intercept {
> >   	[x86_intercept_smsw]		= POST_EX(SVM_EXIT_READ_CR0),
> >   	[x86_intercept_dr_read]		= POST_EX(SVM_EXIT_READ_DR0),
> >   	[x86_intercept_dr_write]	= POST_EX(SVM_EXIT_WRITE_DR0),
> > +	[x86_intercept_sldt]		= POST_MEM(SVM_EXIT_LDTR_READ),
> > +	[x86_intercept_str]		= POST_MEM(SVM_EXIT_TR_READ),
> > +	[x86_intercept_lldt]		= POST_MEM(SVM_EXIT_LDTR_WRITE),
> > +	[x86_intercept_ltr]		= POST_MEM(SVM_EXIT_TR_WRITE),
> > +	[x86_intercept_sgdt]		= POST_MEM(SVM_EXIT_GDTR_READ),
> > +	[x86_intercept_sidt]		= POST_MEM(SVM_EXIT_IDTR_READ),
> > +	[x86_intercept_lgdt]		= POST_MEM(SVM_EXIT_GDTR_WRITE),
> > +	[x86_intercept_lidt]		= POST_MEM(SVM_EXIT_IDTR_WRITE),
> >   };
> 
> Spec says POST_EX()?

Well, not entirely clear. Spec says that #GP takes precedence before the
intercept and the intruction reference says the #GP fires if the
supplied address is not within segment limits or the segment itself is
not valid, which, in my interpretation, made them POST_MEM.

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 07/13] KVM: SVM: Add intercept checks for descriptor table accesses

[Avi Kivity]

On 03/28/2011 03:56 PM, Roedel, Joerg wrote:
> On Mon, Mar 28, 2011 at 08:35:54AM -0400, Avi Kivity wrote:
> >  On 03/28/2011 12:46 PM, Joerg Roedel wrote:
> >  >  This patch add intercept checks into the KVM instruction
> >  >  emulator to check for the 8 instructions that access the
> >  >  descriptor table addresses.
> >  >
> >  >  +static struct opcode group6[] = {
> >  >  +	DI(ModRM,	 sldt),
> >  >  +	DI(ModRM,	 str),
> >  >  +	DI(ModRM | Priv, lldt),
> >  >  +	DI(ModRM | Priv, ltr),
> >  >  +	N, N, N, N,
> >  >  +};
> >  >  +
> >  >    static struct group_dual group7 = { {
> >  >  -	N, N, DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
> >  >  +	DI(ModRM | DstMem | Priv, sgdt), DI(ModRM | DstMem | Priv, sidt),
> >  >  +	DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
> >
> >  | Mov, to avoid RMW for SIDT, for example.  Also need to indicate the
> >  operand size correctly.
> >
> >  >  diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> >  >  index 381b038..485a09f 100644
> >  >  --- a/arch/x86/kvm/svm.c
> >  >  +++ b/arch/x86/kvm/svm.c
> >  >  @@ -3871,6 +3871,10 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
> >  >    #define POST_EX(exit) { .exit_code = (exit), \
> >  >    			.stage = X86_ICPT_POST_EXCEPT, \
> >  >    			.valid = true }
> >  >  +#define POST_MEM(exit) { .exit_code = (exit), \
> >  >  +			 .stage = X86_ICPT_POST_MEMACCESS, \
> >  >  +			 .valid = true }
> >  >  +
> >  >
> >  >    static struct __x86_intercept {
> >  >    	u32 exit_code;
> >  >  @@ -3884,9 +3888,18 @@ static struct __x86_intercept {
> >  >    	[x86_intercept_smsw]		= POST_EX(SVM_EXIT_READ_CR0),
> >  >    	[x86_intercept_dr_read]		= POST_EX(SVM_EXIT_READ_DR0),
> >  >    	[x86_intercept_dr_write]	= POST_EX(SVM_EXIT_WRITE_DR0),
> >  >  +	[x86_intercept_sldt]		= POST_MEM(SVM_EXIT_LDTR_READ),
> >  >  +	[x86_intercept_str]		= POST_MEM(SVM_EXIT_TR_READ),
> >  >  +	[x86_intercept_lldt]		= POST_MEM(SVM_EXIT_LDTR_WRITE),
> >  >  +	[x86_intercept_ltr]		= POST_MEM(SVM_EXIT_TR_WRITE),
> >  >  +	[x86_intercept_sgdt]		= POST_MEM(SVM_EXIT_GDTR_READ),
> >  >  +	[x86_intercept_sidt]		= POST_MEM(SVM_EXIT_IDTR_READ),
> >  >  +	[x86_intercept_lgdt]		= POST_MEM(SVM_EXIT_GDTR_WRITE),
> >  >  +	[x86_intercept_lidt]		= POST_MEM(SVM_EXIT_IDTR_WRITE),
> >  >    };
> >
> >  Spec says POST_EX()?
>
> Well, not entirely clear. Spec says that #GP takes precedence before the
> intercept and the intruction reference says the #GP fires if the
> supplied address is not within segment limits or the segment itself is
> not valid, which, in my interpretation, made them POST_MEM.

My interpretation would be that a #GP(0) due to CPL check takes 
precedence over the intercept, which takes precedence over a 
#GP(selector) due to the actual memory reference.

-- 
error compiling committee.c: too many arguments to function

[PATCH 08/13] KVM: SVM: Add intercept checks for SVM instructions

[Joerg Roedel]

This patch adds the necessary code changes in the
instruction emulator and the extensions to svm.c to
implement intercept checks for the svm instructions.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/emulate.c |   23 ++++++++++++++++++++++-
 arch/x86/kvm/svm.c     |    8 ++++++++
 2 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index ec2e9ad..8b8f63d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -75,6 +75,8 @@
 #define Stack       (1<<13)     /* Stack instruction (push/pop) */
 #define Group       (1<<14)     /* Bits 3:5 of modrm byte extend opcode */
 #define GroupDual   (1<<15)     /* Alternate decoding of mod == 3 */
+#define RMExt       (1<<16)     /* Opcode extension in ModRM r/m if mod == 3 */
+
 /* Misc flags */
 #define VendorSpecific (1<<22) /* Vendor specific instruction */
 #define NoAccess    (1<<23) /* Don't access memory (lea/invlpg/verr etc) */
@@ -2349,6 +2351,7 @@ static int em_mov(struct x86_emulate_ctxt *ctxt)
 #define D(_y) { .flags = (_y) }
 #define DI(_y, _i) { .flags = (_y), .intercept = x86_intercept_##_i }
 #define N    D(0)
+#define EXT(_f, _e) { .flags = ((_f) | RMExt), .u.group = (_e) }
 #define G(_f, _g) { .flags = ((_f) | Group), .u.group = (_g) }
 #define GD(_f, _g) { .flags = ((_f) | Group | GroupDual), .u.gdual = (_g) }
 #define I(_f, _e) { .flags = (_f), .u.execute = (_e) }
@@ -2363,6 +2366,17 @@ static int em_mov(struct x86_emulate_ctxt *ctxt)
 		D2bv(((_f) & ~Lock) | DstAcc | SrcImm)
 
 
+static struct opcode group7_rm3[] = {
+	DI(SrcNone | ModRM | Priv, vmrun),
+	DI(SrcNone | ModRM | Priv, vmmcall),
+	DI(SrcNone | ModRM | Priv, vmload),
+	DI(SrcNone | ModRM | Priv, vmsave),
+	DI(SrcNone | ModRM | Priv, stgi),
+	DI(SrcNone | ModRM | Priv, clgi),
+	DI(SrcNone | ModRM | Priv, skinit),
+	DI(SrcNone | ModRM | Priv, invlpga),
+};
+
 static struct opcode group1[] = {
 	X7(D(Lock)), N
 };
@@ -2406,7 +2420,7 @@ static struct group_dual group7 = { {
 	DI(SrcMem | ModRM | ByteOp | Priv | NoAccess, invlpg),
 }, {
 	D(SrcNone | ModRM | Priv | VendorSpecific), N,
-	N, D(SrcNone | ModRM | Priv | VendorSpecific),
+	N, EXT(0, group7_rm3),
 	DI(SrcNone | ModRM | DstMem | Mov, smsw), N,
 	DI(SrcMem16 | ModRM | Mov | Priv, lmsw), N,
 } };
@@ -2601,6 +2615,7 @@ static struct opcode twobyte_table[256] = {
 #undef G
 #undef GD
 #undef I
+#undef EXT
 
 #undef D2bv
 #undef I2bv
@@ -2778,6 +2793,12 @@ done_prefixes:
 			opcode = g_mod3[goffset];
 		else
 			opcode = g_mod012[goffset];
+
+		if (opcode.flags & RMExt) {
+			goffset = c->modrm & 7;
+			opcode = opcode.u.group[goffset];
+		}
+
 		c->d |= opcode.flags;
 	}
 
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 485a09f..9b22f5f 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3896,6 +3896,14 @@ static struct __x86_intercept {
 	[x86_intercept_sidt]		= POST_MEM(SVM_EXIT_IDTR_READ),
 	[x86_intercept_lgdt]		= POST_MEM(SVM_EXIT_GDTR_WRITE),
 	[x86_intercept_lidt]		= POST_MEM(SVM_EXIT_IDTR_WRITE),
+	[x86_intercept_vmrun]		= POST_EX(SVM_EXIT_VMRUN),
+	[x86_intercept_vmmcall]		= POST_EX(SVM_EXIT_VMMCALL),
+	[x86_intercept_vmload]		= POST_EX(SVM_EXIT_VMLOAD),
+	[x86_intercept_vmsave]		= POST_EX(SVM_EXIT_VMSAVE),
+	[x86_intercept_stgi]		= POST_EX(SVM_EXIT_STGI),
+	[x86_intercept_clgi]		= POST_EX(SVM_EXIT_CLGI),
+	[x86_intercept_skinit]		= POST_EX(SVM_EXIT_SKINIT),
+	[x86_intercept_invlpga]		= POST_EX(SVM_EXIT_INVLPGA),
 };
 
 #undef POST_EX
-- 
1.7.1

Re: [PATCH 08/13] KVM: SVM: Add intercept checks for SVM instructions

[Avi Kivity]

On 03/28/2011 12:46 PM, Joerg Roedel wrote:
> This patch adds the necessary code changes in the
> instruction emulator and the extensions to svm.c to
> implement intercept checks for the svm instructions.
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 485a09f..9b22f5f 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3896,6 +3896,14 @@ static struct __x86_intercept {
>   	[x86_intercept_sidt]		= POST_MEM(SVM_EXIT_IDTR_READ),
>   	[x86_intercept_lgdt]		= POST_MEM(SVM_EXIT_GDTR_WRITE),
>   	[x86_intercept_lidt]		= POST_MEM(SVM_EXIT_IDTR_WRITE),
> +	[x86_intercept_vmrun]		= POST_EX(SVM_EXIT_VMRUN),
> +	[x86_intercept_vmmcall]		= POST_EX(SVM_EXIT_VMMCALL),

VMMCALL is not privileged, is it?

> +	[x86_intercept_vmload]		= POST_EX(SVM_EXIT_VMLOAD),
> +	[x86_intercept_vmsave]		= POST_EX(SVM_EXIT_VMSAVE),
> +	[x86_intercept_stgi]		= POST_EX(SVM_EXIT_STGI),
> +	[x86_intercept_clgi]		= POST_EX(SVM_EXIT_CLGI),
> +	[x86_intercept_skinit]		= POST_EX(SVM_EXIT_SKINIT),
> +	[x86_intercept_invlpga]		= POST_EX(SVM_EXIT_INVLPGA),
>   };
>

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 08/13] KVM: SVM: Add intercept checks for SVM instructions

[Roedel, Joerg]

On Mon, Mar 28, 2011 at 08:08:21AM -0400, Avi Kivity wrote:
> On 03/28/2011 12:46 PM, Joerg Roedel wrote:
> > This patch adds the necessary code changes in the
> > instruction emulator and the extensions to svm.c to
> > implement intercept checks for the svm instructions.
> >
> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> > index 485a09f..9b22f5f 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
> > @@ -3896,6 +3896,14 @@ static struct __x86_intercept {
> >   	[x86_intercept_sidt]		= POST_MEM(SVM_EXIT_IDTR_READ),
> >   	[x86_intercept_lgdt]		= POST_MEM(SVM_EXIT_GDTR_WRITE),
> >   	[x86_intercept_lidt]		= POST_MEM(SVM_EXIT_IDTR_WRITE),
> > +	[x86_intercept_vmrun]		= POST_EX(SVM_EXIT_VMRUN),
> > +	[x86_intercept_vmmcall]		= POST_EX(SVM_EXIT_VMMCALL),
> 
> VMMCALL is not privileged, is it?

Right, thanks. Here is the updated patch.


>From aeaa38bf81c9386aeff4368c2b23ec95c0d08e4c Mon Sep 17 00:00:00 2001
From: Joerg Roedel <joerg.roedel@amd.com>
Date: Fri, 11 Mar 2011 11:53:39 +0100
Subject: [PATCH 08/13] KVM: SVM: Add intercept checks for SVM instructions

This patch adds the necessary code changes in the
instruction emulator and the extensions to svm.c to
implement intercept checks for the svm instructions.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/emulate.c |   23 ++++++++++++++++++++++-
 arch/x86/kvm/svm.c     |    8 ++++++++
 2 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index ec2e9ad..8b8f63d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -75,6 +75,8 @@
 #define Stack       (1<<13)     /* Stack instruction (push/pop) */
 #define Group       (1<<14)     /* Bits 3:5 of modrm byte extend opcode */
 #define GroupDual   (1<<15)     /* Alternate decoding of mod == 3 */
+#define RMExt       (1<<16)     /* Opcode extension in ModRM r/m if mod == 3 */
+
 /* Misc flags */
 #define VendorSpecific (1<<22) /* Vendor specific instruction */
 #define NoAccess    (1<<23) /* Don't access memory (lea/invlpg/verr etc) */
@@ -2349,6 +2351,7 @@ static int em_mov(struct x86_emulate_ctxt *ctxt)
 #define D(_y) { .flags = (_y) }
 #define DI(_y, _i) { .flags = (_y), .intercept = x86_intercept_##_i }
 #define N    D(0)
+#define EXT(_f, _e) { .flags = ((_f) | RMExt), .u.group = (_e) }
 #define G(_f, _g) { .flags = ((_f) | Group), .u.group = (_g) }
 #define GD(_f, _g) { .flags = ((_f) | Group | GroupDual), .u.gdual = (_g) }
 #define I(_f, _e) { .flags = (_f), .u.execute = (_e) }
@@ -2363,6 +2366,17 @@ static int em_mov(struct x86_emulate_ctxt *ctxt)
 		D2bv(((_f) & ~Lock) | DstAcc | SrcImm)
 
 
+static struct opcode group7_rm3[] = {
+	DI(SrcNone | ModRM | Priv, vmrun),
+	DI(SrcNone | ModRM       , vmmcall),
+	DI(SrcNone | ModRM | Priv, vmload),
+	DI(SrcNone | ModRM | Priv, vmsave),
+	DI(SrcNone | ModRM | Priv, stgi),
+	DI(SrcNone | ModRM | Priv, clgi),
+	DI(SrcNone | ModRM | Priv, skinit),
+	DI(SrcNone | ModRM | Priv, invlpga),
+};
+
 static struct opcode group1[] = {
 	X7(D(Lock)), N
 };
@@ -2406,7 +2420,7 @@ static struct group_dual group7 = { {
 	DI(SrcMem | ModRM | ByteOp | Priv | NoAccess, invlpg),
 }, {
 	D(SrcNone | ModRM | Priv | VendorSpecific), N,
-	N, D(SrcNone | ModRM | Priv | VendorSpecific),
+	N, EXT(0, group7_rm3),
 	DI(SrcNone | ModRM | DstMem | Mov, smsw), N,
 	DI(SrcMem16 | ModRM | Mov | Priv, lmsw), N,
 } };
@@ -2601,6 +2615,7 @@ static struct opcode twobyte_table[256] = {
 #undef G
 #undef GD
 #undef I
+#undef EXT
 
 #undef D2bv
 #undef I2bv
@@ -2778,6 +2793,12 @@ done_prefixes:
 			opcode = g_mod3[goffset];
 		else
 			opcode = g_mod012[goffset];
+
+		if (opcode.flags & RMExt) {
+			goffset = c->modrm & 7;
+			opcode = opcode.u.group[goffset];
+		}
+
 		c->d |= opcode.flags;
 	}
 
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 485a09f..9b22f5f 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3896,6 +3896,14 @@ static struct __x86_intercept {
 	[x86_intercept_sidt]		= POST_MEM(SVM_EXIT_IDTR_READ),
 	[x86_intercept_lgdt]		= POST_MEM(SVM_EXIT_GDTR_WRITE),
 	[x86_intercept_lidt]		= POST_MEM(SVM_EXIT_IDTR_WRITE),
+	[x86_intercept_vmrun]		= POST_EX(SVM_EXIT_VMRUN),
+	[x86_intercept_vmmcall]		= POST_EX(SVM_EXIT_VMMCALL),
+	[x86_intercept_vmload]		= POST_EX(SVM_EXIT_VMLOAD),
+	[x86_intercept_vmsave]		= POST_EX(SVM_EXIT_VMSAVE),
+	[x86_intercept_stgi]		= POST_EX(SVM_EXIT_STGI),
+	[x86_intercept_clgi]		= POST_EX(SVM_EXIT_CLGI),
+	[x86_intercept_skinit]		= POST_EX(SVM_EXIT_SKINIT),
+	[x86_intercept_invlpga]		= POST_EX(SVM_EXIT_INVLPGA),
 };
 
 #undef POST_EX
-- 
1.7.1

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

[PATCH 09/13] KVM: SVM: Add intercept checks for remaining group7 instructions

[Joerg Roedel]

This patch implements the emulator intercept checks for the
RDTSCP, MONITOR, and MWAIT instructions.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/emulate.c |   15 +++++++++++++--
 arch/x86/kvm/svm.c     |    3 +++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 8b8f63d..e0eed4c 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2366,6 +2366,12 @@ static int em_mov(struct x86_emulate_ctxt *ctxt)
 		D2bv(((_f) & ~Lock) | DstAcc | SrcImm)
 
 
+static struct opcode group7_rm1[] = {
+	DI(SrcNone | ModRM | Priv, monitor),
+	DI(SrcNone | ModRM | Priv, mwait),
+	N, N, N, N, N, N,
+};
+
 static struct opcode group7_rm3[] = {
 	DI(SrcNone | ModRM | Priv, vmrun),
 	DI(SrcNone | ModRM | Priv, vmmcall),
@@ -2377,6 +2383,11 @@ static struct opcode group7_rm3[] = {
 	DI(SrcNone | ModRM | Priv, invlpga),
 };
 
+static struct opcode group7_rm7[] = {
+	N,
+	DI(SrcNone | ModRM, rdtscp),
+	N, N, N, N, N, N,
+};
 static struct opcode group1[] = {
 	X7(D(Lock)), N
 };
@@ -2419,10 +2430,10 @@ static struct group_dual group7 = { {
 	DI(SrcMem16 | ModRM | Mov | Priv, lmsw),
 	DI(SrcMem | ModRM | ByteOp | Priv | NoAccess, invlpg),
 }, {
-	D(SrcNone | ModRM | Priv | VendorSpecific), N,
+	D(SrcNone | ModRM | Priv | VendorSpecific), EXT(0, group7_rm1),
 	N, EXT(0, group7_rm3),
 	DI(SrcNone | ModRM | DstMem | Mov, smsw), N,
-	DI(SrcMem16 | ModRM | Mov | Priv, lmsw), N,
+	DI(SrcMem16 | ModRM | Mov | Priv, lmsw), EXT(0, group7_rm7),
 } };
 
 static struct opcode group8[] = {
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 9b22f5f..2853625 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3904,6 +3904,9 @@ static struct __x86_intercept {
 	[x86_intercept_clgi]		= POST_EX(SVM_EXIT_CLGI),
 	[x86_intercept_skinit]		= POST_EX(SVM_EXIT_SKINIT),
 	[x86_intercept_invlpga]		= POST_EX(SVM_EXIT_INVLPGA),
+	[x86_intercept_rdtscp]		= POST_EX(SVM_EXIT_RDTSCP),
+	[x86_intercept_monitor]		= POST_MEM(SVM_EXIT_MONITOR),
+	[x86_intercept_mwait]		= POST_EX(SVM_EXIT_MWAIT),
 };
 
 #undef POST_EX
-- 
1.7.1

Re: [PATCH 09/13] KVM: SVM: Add intercept checks for remaining group7 instructions

[Avi Kivity]

On 03/28/2011 12:46 PM, Joerg Roedel wrote:
> This patch implements the emulator intercept checks for the
> RDTSCP, MONITOR, and MWAIT instructions.
>
> Signed-off-by: Joerg Roedel<joerg.roedel@amd.com>
> ---
>   arch/x86/kvm/emulate.c |   15 +++++++++++++--
>   arch/x86/kvm/svm.c     |    3 +++
>   2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 8b8f63d..e0eed4c 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -2366,6 +2366,12 @@ static int em_mov(struct x86_emulate_ctxt *ctxt)
>   		D2bv(((_f)&  ~Lock) | DstAcc | SrcImm)
>
>
> +static struct opcode group7_rm1[] = {
> +	DI(SrcNone | ModRM | Priv, monitor),
> +	DI(SrcNone | ModRM | Priv, mwait),

Note these are unprivileged if magic msr c0010015 is tweaked.  I don't 
think we support the enabled setting.

> +	N, N, N, N, N, N,
> +};
> +
>   static struct opcode group7_rm3[] = {
>   	DI(SrcNone | ModRM | Priv, vmrun),
>   	DI(SrcNone | ModRM | Priv, vmmcall),
> @@ -2377,6 +2383,11 @@ static struct opcode group7_rm3[] = {
>   	DI(SrcNone | ModRM | Priv, invlpga),
>   };
>
> +static struct opcode group7_rm7[] = {
> +	N,
> +	DI(SrcNone | ModRM, rdtscp),

Need to check cr4.tsd before the intercept.

> +	N, N, N, N, N, N,
> +};
>   static struct opcode group1[] = {
>   	X7(D(Lock)), N
>   };
> @@ -2419,10 +2430,10 @@ static struct group_dual group7 = { {
>   	DI(SrcMem16 | ModRM | Mov | Priv, lmsw),
>   	DI(SrcMem | ModRM | ByteOp | Priv | NoAccess, invlpg),
>   }, {
> -	D(SrcNone | ModRM | Priv | VendorSpecific), N,
> +	D(SrcNone | ModRM | Priv | VendorSpecific), EXT(0, group7_rm1),
>   	N, EXT(0, group7_rm3),
>   	DI(SrcNone | ModRM | DstMem | Mov, smsw), N,
> -	DI(SrcMem16 | ModRM | Mov | Priv, lmsw), N,
> +	DI(SrcMem16 | ModRM | Mov | Priv, lmsw), EXT(0, group7_rm7),
>   } };
>
>   static struct opcode group8[] = {
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 9b22f5f..2853625 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3904,6 +3904,9 @@ static struct __x86_intercept {
>   	[x86_intercept_clgi]		= POST_EX(SVM_EXIT_CLGI),
>   	[x86_intercept_skinit]		= POST_EX(SVM_EXIT_SKINIT),
>   	[x86_intercept_invlpga]		= POST_EX(SVM_EXIT_INVLPGA),
> +	[x86_intercept_rdtscp]		= POST_EX(SVM_EXIT_RDTSCP),
> +	[x86_intercept_monitor]		= POST_MEM(SVM_EXIT_MONITOR),
> +	[x86_intercept_mwait]		= POST_EX(SVM_EXIT_MWAIT),
>   };
>
>   #undef POST_EX


-- 
error compiling committee.c: too many arguments to function

[PATCH 10/13] KVM: SVM: Add intercept checks for remaining twobyte instructions

[Joerg Roedel]

This patch adds intercepts checks for the remaining twobyte
instructions to the KVM instruction emulator.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_emulate.h |    2 ++
 arch/x86/kvm/emulate.c             |    8 ++++----
 arch/x86/kvm/svm.c                 |   19 +++++++++++++++++++
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index cdc9026..db0bbc2 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -333,6 +333,8 @@ enum x86_intercept {
 	x86_intercept_wbinvd,
 	x86_intercept_monitor,
 	x86_intercept_mwait,
+	x86_intercept_rdmsr,
+	x86_intercept_wrmsr,
 
 	nr_x86_intercepts
 };
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index e0eed4c..4a5881d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2570,8 +2570,8 @@ static struct opcode twobyte_table[256] = {
 	N, N, N, N,
 	N, N, N, N, N, N, N, N,
 	/* 0x30 - 0x3F */
-	D(ImplicitOps | Priv), II(ImplicitOps, em_rdtsc, rdtsc),
-	D(ImplicitOps | Priv), N,
+	DI(ImplicitOps | Priv, wrmsr), II(ImplicitOps, em_rdtsc, rdtsc),
+	DI(ImplicitOps | Priv, rdmsr), DI(ImplicitOps | Priv, rdpmc),
 	D(ImplicitOps | VendorSpecific), D(ImplicitOps | Priv | VendorSpecific),
 	N, N,
 	N, N, N, N, N, N, N, N,
@@ -2589,12 +2589,12 @@ static struct opcode twobyte_table[256] = {
 	X16(D(ByteOp | DstMem | SrcNone | ModRM| Mov)),
 	/* 0xA0 - 0xA7 */
 	D(ImplicitOps | Stack), D(ImplicitOps | Stack),
-	N, D(DstMem | SrcReg | ModRM | BitOp),
+	DI(ImplicitOps, cpuid), D(DstMem | SrcReg | ModRM | BitOp),
 	D(DstMem | SrcReg | Src2ImmByte | ModRM),
 	D(DstMem | SrcReg | Src2CL | ModRM), N, N,
 	/* 0xA8 - 0xAF */
 	D(ImplicitOps | Stack), D(ImplicitOps | Stack),
-	N, D(DstMem | SrcReg | ModRM | BitOp | Lock),
+	DI(ImplicitOps, rsm), D(DstMem | SrcReg | ModRM | BitOp | Lock),
 	D(DstMem | SrcReg | Src2ImmByte | ModRM),
 	D(DstMem | SrcReg | Src2CL | ModRM),
 	D(ModRM), I(DstReg | SrcMem | ModRM, em_imul),
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 2853625..ec99d0a 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3868,6 +3868,9 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
 	update_cr0_intercept(svm);
 }
 
+#define PRE_EX(exit)  { .exit_code = (exit), \
+			.stage = X86_ICPT_PRE_EXCEPT, \
+			.valid = true }
 #define POST_EX(exit) { .exit_code = (exit), \
 			.stage = X86_ICPT_POST_EXCEPT, \
 			.valid = true }
@@ -3907,8 +3910,18 @@ static struct __x86_intercept {
 	[x86_intercept_rdtscp]		= POST_EX(SVM_EXIT_RDTSCP),
 	[x86_intercept_monitor]		= POST_MEM(SVM_EXIT_MONITOR),
 	[x86_intercept_mwait]		= POST_EX(SVM_EXIT_MWAIT),
+	[x86_intercept_invlpg]		= POST_EX(SVM_EXIT_INVLPG),
+	[x86_intercept_invd]		= POST_EX(SVM_EXIT_INVD),
+	[x86_intercept_wbinvd]		= POST_EX(SVM_EXIT_WBINVD),
+	[x86_intercept_wrmsr]		= POST_EX(SVM_EXIT_MSR),
+	[x86_intercept_rdtsc]		= POST_EX(SVM_EXIT_RDTSC),
+	[x86_intercept_rdmsr]		= POST_EX(SVM_EXIT_MSR),
+	[x86_intercept_rdpmc]		= POST_EX(SVM_EXIT_RDPMC),
+	[x86_intercept_cpuid]		= PRE_EX(SVM_EXIT_CPUID),
+	[x86_intercept_rsm]		= PRE_EX(SVM_EXIT_RSM),
 };
 
+#undef PRE_EX
 #undef POST_EX
 #undef POST_MEM
 
@@ -3969,6 +3982,12 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
 	case SVM_EXIT_WRITE_DR0:
 		icpt_info.exit_code += info->modrm_reg;
 		break;
+	case SVM_EXIT_MSR:
+		if (info->intercept == x86_intercept_wrmsr)
+			vmcb->control.exit_info_1 = 1;
+		else
+			vmcb->control.exit_info_1 = 0;
+		break;
 	default:
 		break;
 	}
-- 
1.7.1

Re: [PATCH 10/13] KVM: SVM: Add intercept checks for remaining twobyte instructions

[Avi Kivity]

On 03/28/2011 12:46 PM, Joerg Roedel wrote:
> This patch adds intercepts checks for the remaining twobyte
> instructions to the KVM instruction emulator.
>
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index e0eed4c..4a5881d 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -2570,8 +2570,8 @@ static struct opcode twobyte_table[256] = {
>   	N, N, N, N,
>   	N, N, N, N, N, N, N, N,
>   	/* 0x30 - 0x3F */
> -	D(ImplicitOps | Priv), II(ImplicitOps, em_rdtsc, rdtsc),
> -	D(ImplicitOps | Priv), N,
> +	DI(ImplicitOps | Priv, wrmsr), II(ImplicitOps, em_rdtsc, rdtsc),

cr4.tsd check

> +	DI(ImplicitOps | Priv, rdmsr), DI(ImplicitOps | Priv, rdpmc),

Not privileged if cr4.pce=1.



-- 
error compiling committee.c: too many arguments to function

[PATCH 11/13] KVM: SVM: Add intercept checks for one-byte instructions

[Joerg Roedel]

This patch add intercept checks for emulated one-byte
instructions to the KVM instruction emulation path.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/emulate.c |    4 ++--
 arch/x86/kvm/svm.c     |   14 ++++++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 4a5881d..af04be4 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2504,7 +2504,7 @@ static struct opcode opcode_table[256] = {
 	D(DstMem | SrcNone | ModRM | Mov), D(ModRM | SrcMem | NoAccess | DstReg),
 	D(ImplicitOps | SrcMem16 | ModRM), G(0, group1A),
 	/* 0x90 - 0x97 */
-	X8(D(SrcAcc | DstReg)),
+	DI(SrcAcc | DstReg, pause), X7(D(SrcAcc | DstReg)),
 	/* 0x98 - 0x9F */
 	D(DstAcc | SrcNone), I(ImplicitOps | SrcAcc, em_cwd),
 	I(SrcImmFAddr | No64, em_call_far), N,
@@ -2546,7 +2546,7 @@ static struct opcode opcode_table[256] = {
 	D(SrcImmFAddr | No64), D(SrcImmByte | ImplicitOps),
 	D2bv(SrcNone | DstAcc),	D2bv(SrcAcc | ImplicitOps),
 	/* 0xF0 - 0xF7 */
-	N, N, N, N,
+	N, DI(ImplicitOps, icebp), N, N,
 	DI(ImplicitOps | Priv, hlt), D(ImplicitOps),
 	G(ByteOp, group3), G(0, group3),
 	/* 0xF8 - 0xFF */
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index ec99d0a..67997b3 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3919,6 +3919,13 @@ static struct __x86_intercept {
 	[x86_intercept_rdpmc]		= POST_EX(SVM_EXIT_RDPMC),
 	[x86_intercept_cpuid]		= PRE_EX(SVM_EXIT_CPUID),
 	[x86_intercept_rsm]		= PRE_EX(SVM_EXIT_RSM),
+	[x86_intercept_pause]		= PRE_EX(SVM_EXIT_PAUSE),
+	[x86_intercept_pushf]		= PRE_EX(SVM_EXIT_PUSHF),
+	[x86_intercept_popf]		= PRE_EX(SVM_EXIT_POPF),
+	[x86_intercept_intn]		= PRE_EX(SVM_EXIT_SWINT),
+	[x86_intercept_iret]		= PRE_EX(SVM_EXIT_IRET),
+	[x86_intercept_icebp]		= PRE_EX(SVM_EXIT_ICEBP),
+	[x86_intercept_hlt]		= POST_EX(SVM_EXIT_HLT),
 };
 
 #undef PRE_EX
@@ -3988,6 +3995,13 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
 		else
 			vmcb->control.exit_info_1 = 0;
 		break;
+	case SVM_EXIT_PAUSE:
+		/*
+		 * We get this for NOP only, but pause
+		 * is rep not, check this here
+		 */
+		if (info->rep_prefix != REPE_PREFIX)
+			goto out;
 	default:
 		break;
 	}
-- 
1.7.1

[PATCH 12/13] KVM: SVM: Add checks for IO instructions

[Joerg Roedel]

This patch adds code to check for IOIO intercepts on
instructions decoded by the KVM instruction emulator.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_emulate.h |    4 ++++
 arch/x86/kvm/emulate.c             |   10 ++++++----
 arch/x86/kvm/svm.c                 |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index db0bbc2..53bfc99 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -335,6 +335,10 @@ enum x86_intercept {
 	x86_intercept_mwait,
 	x86_intercept_rdmsr,
 	x86_intercept_wrmsr,
+	x86_intercept_in,
+	x86_intercept_ins,
+	x86_intercept_out,
+	x86_intercept_outs,
 
 	nr_x86_intercepts
 };
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index af04be4..ecfea47 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2359,6 +2359,7 @@ static int em_mov(struct x86_emulate_ctxt *ctxt)
 	{ .flags = (_f), .u.execute = (_e), .intercept = x86_intercept_##_i }
 
 #define D2bv(_f)      D((_f) | ByteOp), D(_f)
+#define D2bvI(_f, _i) DI((_f) | ByteOp, _i), DI((_f), _i)
 #define I2bv(_f, _e)  I((_f) | ByteOp, _e), I(_f, _e)
 
 #define D6ALU(_f) D2bv((_f) | DstMem | SrcReg | ModRM),			\
@@ -2488,8 +2489,8 @@ static struct opcode opcode_table[256] = {
 	I(DstReg | SrcMem | ModRM | Src2Imm, em_imul_3op),
 	I(SrcImmByte | Mov | Stack, em_push),
 	I(DstReg | SrcMem | ModRM | Src2ImmByte, em_imul_3op),
-	D2bv(DstDI | Mov | String), /* insb, insw/insd */
-	D2bv(SrcSI | ImplicitOps | String), /* outsb, outsw/outsd */
+	D2bvI(DstDI | Mov | String, ins), /* insb, insw/insd */
+	D2bvI(SrcSI | ImplicitOps | String, outs), /* outsb, outsw/outsd */
 	/* 0x70 - 0x7F */
 	X16(D(SrcImmByte)),
 	/* 0x80 - 0x87 */
@@ -2540,11 +2541,11 @@ static struct opcode opcode_table[256] = {
 	N, N, N, N, N, N, N, N,
 	/* 0xE0 - 0xE7 */
 	X4(D(SrcImmByte)),
-	D2bv(SrcImmUByte | DstAcc), D2bv(SrcAcc | DstImmUByte),
+	D2bvI(SrcImmUByte | DstAcc, in), D2bvI(SrcAcc | DstImmUByte, out),
 	/* 0xE8 - 0xEF */
 	D(SrcImm | Stack), D(SrcImm | ImplicitOps),
 	D(SrcImmFAddr | No64), D(SrcImmByte | ImplicitOps),
-	D2bv(SrcNone | DstAcc),	D2bv(SrcAcc | ImplicitOps),
+	D2bvI(SrcNone | DstAcc, in), D2bvI(SrcAcc | ImplicitOps, out),
 	/* 0xF0 - 0xF7 */
 	N, DI(ImplicitOps, icebp), N, N,
 	DI(ImplicitOps | Priv, hlt), D(ImplicitOps),
@@ -2629,6 +2630,7 @@ static struct opcode twobyte_table[256] = {
 #undef EXT
 
 #undef D2bv
+#undef D2bvI
 #undef I2bv
 #undef D6ALU
 
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 67997b3..3701ded 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3926,6 +3926,10 @@ static struct __x86_intercept {
 	[x86_intercept_iret]		= PRE_EX(SVM_EXIT_IRET),
 	[x86_intercept_icebp]		= PRE_EX(SVM_EXIT_ICEBP),
 	[x86_intercept_hlt]		= POST_EX(SVM_EXIT_HLT),
+	[x86_intercept_in]		= POST_EX(SVM_EXIT_IOIO),
+	[x86_intercept_ins]		= POST_EX(SVM_EXIT_IOIO),
+	[x86_intercept_out]		= POST_EX(SVM_EXIT_IOIO),
+	[x86_intercept_outs]		= POST_EX(SVM_EXIT_IOIO),
 };
 
 #undef PRE_EX
@@ -4002,6 +4006,38 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
 		 */
 		if (info->rep_prefix != REPE_PREFIX)
 			goto out;
+	case SVM_EXIT_IOIO: {
+		u64 exit_info;
+		u32 bytes;
+
+		exit_info = (vcpu->arch.regs[VCPU_REGS_RDX] & 0xffff) << 16;
+
+		if (info->intercept == x86_intercept_in ||
+		    info->intercept == x86_intercept_ins) {
+			exit_info |= SVM_IOIO_TYPE_MASK;
+			bytes = info->src_bytes;
+		} else {
+			bytes = info->dst_bytes;
+		}
+
+		if (info->intercept == x86_intercept_outs ||
+		    info->intercept == x86_intercept_ins)
+			exit_info |= SVM_IOIO_STR_MASK;
+
+		if (info->rep_prefix)
+			exit_info |= SVM_IOIO_REP_MASK;
+
+		bytes = min(bytes, 4u);
+
+		exit_info |= bytes << SVM_IOIO_SIZE_SHIFT;
+
+		exit_info |= (u32)info->ad_bytes << (SVM_IOIO_ASIZE_SHIFT - 1);
+
+		vmcb->control.exit_info_1 = exit_info;
+		vmcb->control.exit_info_2 = info->next_rip;
+
+		break;
+	}
 	default:
 		break;
 	}
-- 
1.7.1

Re: [PATCH 12/13] KVM: SVM: Add checks for IO instructions

[Avi Kivity]

On 03/28/2011 12:46 PM, Joerg Roedel wrote:
> This patch adds code to check for IOIO intercepts on
> instructions decoded by the KVM instruction emulator.
>
>
> @@ -3926,6 +3926,10 @@ static struct __x86_intercept {
>   	[x86_intercept_iret]		= PRE_EX(SVM_EXIT_IRET),
>   	[x86_intercept_icebp]		= PRE_EX(SVM_EXIT_ICEBP),
>   	[x86_intercept_hlt]		= POST_EX(SVM_EXIT_HLT),
> +	[x86_intercept_in]		= POST_EX(SVM_EXIT_IOIO),
> +	[x86_intercept_ins]		= POST_EX(SVM_EXIT_IOIO),
> +	[x86_intercept_out]		= POST_EX(SVM_EXIT_IOIO),
> +	[x86_intercept_outs]		= POST_EX(SVM_EXIT_IOIO),
>   };

The spec indicates we need to check the TSS and IOPL based permissions 
before the intercept (vmx agrees).  With the code as is, it happens 
afterwards.

One way to do this is to have an ExtraChecks bit in the opcode::flags.  
Then opcode::u.xcheck->perms() is the pre-intercept check and 
opcode::u.xcheck->execute() is the post-intercept execution.  Should 
work for monitor/mwait/rdtsc(p)/rdpmc/other crap x86 throws at us.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 12/13] KVM: SVM: Add checks for IO instructions

[Roedel, Joerg]

On Mon, Mar 28, 2011 at 08:28:12AM -0400, Avi Kivity wrote:
> The spec indicates we need to check the TSS and IOPL based permissions 
> before the intercept (vmx agrees).  With the code as is, it happens 
> afterwards.
> 
> One way to do this is to have an ExtraChecks bit in the opcode::flags.  
> Then opcode::u.xcheck->perms() is the pre-intercept check and 
> opcode::u.xcheck->execute() is the post-intercept execution.  Should 
> work for monitor/mwait/rdtsc(p)/rdpmc/other crap x86 throws at us.

Okay, as you suggested, I put these checks into the instruction emulator
and let the hard work of implementing per-arch checks to the nested-vmx
people ;)
I doubt that this makes the opcode-tables more readable, but lets see :)

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 12/13] KVM: SVM: Add checks for IO instructions

[Avi Kivity]

On 03/31/2011 09:14 AM, Roedel, Joerg wrote:
> On Mon, Mar 28, 2011 at 08:28:12AM -0400, Avi Kivity wrote:
> >  The spec indicates we need to check the TSS and IOPL based permissions
> >  before the intercept (vmx agrees).  With the code as is, it happens
> >  afterwards.
> >
> >  One way to do this is to have an ExtraChecks bit in the opcode::flags.
> >  Then opcode::u.xcheck->perms() is the pre-intercept check and
> >  opcode::u.xcheck->execute() is the post-intercept execution.  Should
> >  work for monitor/mwait/rdtsc(p)/rdpmc/other crap x86 throws at us.
>
> Okay, as you suggested, I put these checks into the instruction emulator
> and let the hard work of implementing per-arch checks to the nested-vmx
> people ;)
> I doubt that this makes the opcode-tables more readable, but lets see :)

I think we're miscommunicating.  I'm talking about x86 checks, not virt 
vendor specific checks.

For example, the flow for IOIO would be:

   #UD check (lock prefix)
   PE/IOPL/CPL/VM check
   TSS bitmap check (can cause #PF)
   Intercept check
   Operand segment check
   Possible #PF
   Execution

We need to make sure the TSS bitmap check happens before the intercept, 
so we need to split ->execute() into two.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 12/13] KVM: SVM: Add checks for IO instructions

[Roedel, Joerg]

On Thu, Mar 31, 2011 at 05:18:28AM -0400, Avi Kivity wrote:
> On 03/31/2011 09:14 AM, Roedel, Joerg wrote:
> > On Mon, Mar 28, 2011 at 08:28:12AM -0400, Avi Kivity wrote:
> > >  The spec indicates we need to check the TSS and IOPL based permissions
> > >  before the intercept (vmx agrees).  With the code as is, it happens
> > >  afterwards.
> > >
> > >  One way to do this is to have an ExtraChecks bit in the opcode::flags.
> > >  Then opcode::u.xcheck->perms() is the pre-intercept check and
> > >  opcode::u.xcheck->execute() is the post-intercept execution.  Should
> > >  work for monitor/mwait/rdtsc(p)/rdpmc/other crap x86 throws at us.
> >
> > Okay, as you suggested, I put these checks into the instruction emulator
> > and let the hard work of implementing per-arch checks to the nested-vmx
> > people ;)
> > I doubt that this makes the opcode-tables more readable, but lets see :)
> 
> I think we're miscommunicating.  I'm talking about x86 checks, not virt 
> vendor specific checks.

The place of the intercept check may be vendor specific. I havn't looked
at the Intel spec, though. But there are probably differences.

> For example, the flow for IOIO would be:
> 
>    #UD check (lock prefix)
>    PE/IOPL/CPL/VM check
>    TSS bitmap check (can cause #PF)
>    Intercept check
>    Operand segment check
>    Possible #PF
>    Execution
> 
> We need to make sure the TSS bitmap check happens before the intercept, 
> so we need to split ->execute() into two.

Right. For the generic case, how about factor out the checks (for the
POST_EX intercept case) into a seperate excp_check-callback (similar to the
execute-callback) and execute it before the post-exception-intercept
check?

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 12/13] KVM: SVM: Add checks for IO instructions

[Avi Kivity]

On 03/31/2011 11:42 AM, Roedel, Joerg wrote:
> On Thu, Mar 31, 2011 at 05:18:28AM -0400, Avi Kivity wrote:
> >  On 03/31/2011 09:14 AM, Roedel, Joerg wrote:
> >  >  On Mon, Mar 28, 2011 at 08:28:12AM -0400, Avi Kivity wrote:
> >  >  >   The spec indicates we need to check the TSS and IOPL based permissions
> >  >  >   before the intercept (vmx agrees).  With the code as is, it happens
> >  >  >   afterwards.
> >  >  >
> >  >  >   One way to do this is to have an ExtraChecks bit in the opcode::flags.
> >  >  >   Then opcode::u.xcheck->perms() is the pre-intercept check and
> >  >  >   opcode::u.xcheck->execute() is the post-intercept execution.  Should
> >  >  >   work for monitor/mwait/rdtsc(p)/rdpmc/other crap x86 throws at us.
> >  >
> >  >  Okay, as you suggested, I put these checks into the instruction emulator
> >  >  and let the hard work of implementing per-arch checks to the nested-vmx
> >  >  people ;)
> >  >  I doubt that this makes the opcode-tables more readable, but lets see :)
> >
> >  I think we're miscommunicating.  I'm talking about x86 checks, not virt
> >  vendor specific checks.
>
> The place of the intercept check may be vendor specific. I havn't looked
> at the Intel spec, though. But there are probably differences.

That's why there are three hooks: pre-ex, post-ex, post-mem.  If an 
intercept fits in between, use the pre-ex hook and duplicate the checks 
in the intercept.

As far as I recall, everything should fit into those three, though.

> >  For example, the flow for IOIO would be:
> >
> >     #UD check (lock prefix)
> >     PE/IOPL/CPL/VM check
> >     TSS bitmap check (can cause #PF)
> >     Intercept check
> >     Operand segment check
> >     Possible #PF
> >     Execution
> >
> >  We need to make sure the TSS bitmap check happens before the intercept,
> >  so we need to split ->execute() into two.
>
> Right. For the generic case, how about factor out the checks (for the
> POST_EX intercept case) into a seperate excp_check-callback (similar to the
> execute-callback) and execute it before the post-exception-intercept
> check?

That is exactly my suggestion.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 12/13] KVM: SVM: Add checks for IO instructions

[Roedel, Joerg]

On Thu, Mar 31, 2011 at 06:03:37AM -0400, Avi Kivity wrote:
> On 03/31/2011 11:42 AM, Roedel, Joerg wrote:
> > On Thu, Mar 31, 2011 at 05:18:28AM -0400, Avi Kivity wrote:
> > >  On 03/31/2011 09:14 AM, Roedel, Joerg wrote:
> > >  >  On Mon, Mar 28, 2011 at 08:28:12AM -0400, Avi Kivity wrote:
> > >  >  >   The spec indicates we need to check the TSS and IOPL based permissions
> > >  >  >   before the intercept (vmx agrees).  With the code as is, it happens
> > >  >  >   afterwards.
> > >  >  >
> > >  >  >   One way to do this is to have an ExtraChecks bit in the opcode::flags.
> > >  >  >   Then opcode::u.xcheck->perms() is the pre-intercept check and
> > >  >  >   opcode::u.xcheck->execute() is the post-intercept execution.  Should
> > >  >  >   work for monitor/mwait/rdtsc(p)/rdpmc/other crap x86 throws at us.
> > >  >
> > >  >  Okay, as you suggested, I put these checks into the instruction emulator
> > >  >  and let the hard work of implementing per-arch checks to the nested-vmx
> > >  >  people ;)
> > >  >  I doubt that this makes the opcode-tables more readable, but lets see :)
> > >
> > >  I think we're miscommunicating.  I'm talking about x86 checks, not virt
> > >  vendor specific checks.
> >
> > The place of the intercept check may be vendor specific. I havn't looked
> > at the Intel spec, though. But there are probably differences.
> 
> That's why there are three hooks: pre-ex, post-ex, post-mem.  If an 
> intercept fits in between, use the pre-ex hook and duplicate the checks 
> in the intercept.
> 
> As far as I recall, everything should fit into those three, though.

Okay, thats the way to go then, thanks.

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

[PATCH 13/13] KVM: SVM: Remove nested sel_cr0_write handling code

[Joerg Roedel]

This patch removes all the old code which handled the nested
selective cr0 write intercepts. This code was only in place
as a work-around until the instruction emulator is capable
of doing the same. This is the case with this patch-set and
so the code can be removed.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |   78 +++++++++++++++++----------------------------------
 1 files changed, 26 insertions(+), 52 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 3701ded..a6de05a 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -93,14 +93,6 @@ struct nested_state {
 	/* A VMEXIT is required but not yet emulated */
 	bool exit_required;
 
-	/*
-	 * If we vmexit during an instruction emulation we need this to restore
-	 * the l1 guest rip after the emulation
-	 */
-	unsigned long vmexit_rip;
-	unsigned long vmexit_rsp;
-	unsigned long vmexit_rax;
-
 	/* cache for intercepts of the guest */
 	u32 intercept_cr;
 	u32 intercept_dr;
@@ -1362,31 +1354,6 @@ static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
-	if (is_guest_mode(vcpu)) {
-		/*
-		 * We are here because we run in nested mode, the host kvm
-		 * intercepts cr0 writes but the l1 hypervisor does not.
-		 * But the L1 hypervisor may intercept selective cr0 writes.
-		 * This needs to be checked here.
-		 */
-		unsigned long old, new;
-
-		/* Remove bits that would trigger a real cr0 write intercept */
-		old = vcpu->arch.cr0 & SVM_CR0_SELECTIVE_MASK;
-		new = cr0 & SVM_CR0_SELECTIVE_MASK;
-
-		if (old == new) {
-			/* cr0 write with ts and mp unchanged */
-			svm->vmcb->control.exit_code = SVM_EXIT_CR0_SEL_WRITE;
-			if (nested_svm_exit_handled(svm) == NESTED_EXIT_DONE) {
-				svm->nested.vmexit_rip = kvm_rip_read(vcpu);
-				svm->nested.vmexit_rsp = kvm_register_read(vcpu, VCPU_REGS_RSP);
-				svm->nested.vmexit_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
-				return;
-			}
-		}
-	}
-
 #ifdef CONFIG_X86_64
 	if (vcpu->arch.efer & EFER_LME) {
 		if (!is_paging(vcpu) && (cr0 & X86_CR0_PG)) {
@@ -2673,6 +2640,29 @@ static int emulate_on_interception(struct vcpu_svm *svm)
 	return emulate_instruction(&svm->vcpu, 0) == EMULATE_DONE;
 }
 
+bool check_selective_cr0_intercepted(struct vcpu_svm *svm, unsigned long val)
+{
+	unsigned long cr0 = svm->vcpu.arch.cr0;
+	bool ret = false;
+	u64 intercept;
+
+	intercept = svm->nested.intercept;
+
+	if (!is_guest_mode(&svm->vcpu) ||
+	    (!(intercept & (1ULL << INTERCEPT_SELECTIVE_CR0))))
+		return false;
+
+	cr0 &= ~SVM_CR0_SELECTIVE_MASK;
+	val &= ~SVM_CR0_SELECTIVE_MASK;
+
+	if (cr0 ^ val) {
+		svm->vmcb->control.exit_code = SVM_EXIT_CR0_SEL_WRITE;
+		ret = (nested_svm_exit_handled(svm) == NESTED_EXIT_DONE);
+	}
+
+	return ret;
+}
+
 #define CR_VALID (1ULL << 63)
 
 static int cr_interception(struct vcpu_svm *svm)
@@ -2696,7 +2686,8 @@ static int cr_interception(struct vcpu_svm *svm)
 		val = kvm_register_read(&svm->vcpu, reg);
 		switch (cr) {
 		case 0:
-			err = kvm_set_cr0(&svm->vcpu, val);
+			if (!check_selective_cr0_intercepted(svm, val))
+				err = kvm_set_cr0(&svm->vcpu, val);
 			break;
 		case 3:
 			err = kvm_set_cr3(&svm->vcpu, val);
@@ -2741,23 +2732,6 @@ static int cr_interception(struct vcpu_svm *svm)
 	return 1;
 }
 
-static int cr0_write_interception(struct vcpu_svm *svm)
-{
-	struct kvm_vcpu *vcpu = &svm->vcpu;
-	int r;
-
-	r = cr_interception(svm);
-
-	if (svm->nested.vmexit_rip) {
-		kvm_register_write(vcpu, VCPU_REGS_RIP, svm->nested.vmexit_rip);
-		kvm_register_write(vcpu, VCPU_REGS_RSP, svm->nested.vmexit_rsp);
-		kvm_register_write(vcpu, VCPU_REGS_RAX, svm->nested.vmexit_rax);
-		svm->nested.vmexit_rip = 0;
-	}
-
-	return r;
-}
-
 static int dr_interception(struct vcpu_svm *svm)
 {
 	int reg, dr;
@@ -3045,7 +3019,7 @@ static int (*svm_exit_handlers[])(struct vcpu_svm *svm) = {
 	[SVM_EXIT_READ_CR4]			= cr_interception,
 	[SVM_EXIT_READ_CR8]			= cr_interception,
 	[SVM_EXIT_CR0_SEL_WRITE]		= emulate_on_interception,
-	[SVM_EXIT_WRITE_CR0]			= cr0_write_interception,
+	[SVM_EXIT_WRITE_CR0]			= cr_interception,
 	[SVM_EXIT_WRITE_CR3]			= cr_interception,
 	[SVM_EXIT_WRITE_CR4]			= cr_interception,
 	[SVM_EXIT_WRITE_CR8]			= cr8_write_interception,
-- 
1.7.1

[PATCH 0/13] KVM: Make the instruction emulator aware of Nested Virtualization v2

[Joerg Roedel]

Hi,

this is version 2 of the patch-set to make the KVM instruction emulator
aware of intercepted instructions. Noting the differences to v1 does not
make a lot of sense this this is basically a re-implementation so that
almost everything changed :-)
The re-write was done on the basis of Avi's patches he sent after the
last discussion. With these changes the implementation in the SVM code
got a lot smaller and more generic (and easier to extend). The big
switch is now only necessary for handling special cases.

Comments and feedback is appreciated.

Regards,

	Joerg

Diffstat:

 arch/x86/include/asm/kvm_emulate.h |   67 +++++++++
 arch/x86/include/asm/kvm_host.h    |   21 +++
 arch/x86/kvm/emulate.c             |  128 ++++++++++++++----
 arch/x86/kvm/svm.c                 |  264 +++++++++++++++++++++++++++++-------
 arch/x86/kvm/x86.c                 |   30 ++++
 5 files changed, 432 insertions(+), 78 deletions(-)

Shortlog:

Avi Kivity (2):
      KVM: x86 emulator: add framework for instruction
      KVM: x86 emulator: add SVM intercepts

Joerg Roedel (11):
      KVM: X86: Don't write-back cpu-state on X86EMUL_INTERCEPTED
      KVM: X86: Add x86 callback for intercept check
      KVM: SVM: Add intercept check for emulated cr accesses
      KVM: SVM: Add intercept check for accessing dr registers
      KVM: SVM: Add intercept checks for descriptor table accesses
      KVM: SVM: Add intercept checks for SVM instructions
      KVM: SVM: Add intercept checks for remaining group7 instructions
      KVM: SVM: Add intercept checks for remaining twobyte instructions
      KVM: SVM: Add intercept checks for one-byte instructions
      KVM: SVM: Add checks for IO instructions
      KVM: SVM: Remove nested sel_cr0_write handling code

[PATCH 07/13] KVM: SVM: Add intercept checks for descriptor table accesses

[Joerg Roedel]

This patch add intercept checks into the KVM instruction
emulator to check for the 8 instructions that access the
descriptor table addresses.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/emulate.c |   13 +++++++++++--
 arch/x86/kvm/svm.c     |   13 +++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 0719954..505348f 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2370,8 +2370,17 @@ static struct opcode group5[] = {
 	D(SrcMem | ModRM | Stack), N,
 };
 
+static struct opcode group6[] = {
+	DI(ModRM,	 sldt),
+	DI(ModRM,	 str),
+	DI(ModRM | Priv, lldt),
+	DI(ModRM | Priv, ltr),
+	N, N, N, N,
+};
+
 static struct group_dual group7 = { {
-	N, N, DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
+	DI(ModRM | DstMem | Priv, sgdt), DI(ModRM | DstMem | Priv, sidt),
+	DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
 	DI(SrcNone | ModRM | DstMem | Mov, smsw), N,
 	DI(SrcMem16 | ModRM | Mov | Priv, lmsw),
 	DI(SrcMem | ModRM | ByteOp | Priv | NoAccess, invlpg),
@@ -2502,7 +2511,7 @@ static struct opcode opcode_table[256] = {
 
 static struct opcode twobyte_table[256] = {
 	/* 0x00 - 0x0F */
-	N, GD(0, &group7), N, N,
+	G(0, group6), GD(0, &group7), N, N,
 	N, D(ImplicitOps | VendorSpecific), DI(ImplicitOps | Priv, clts), N,
 	DI(ImplicitOps | Priv, invd), DI(ImplicitOps | Priv, wbinvd), N, N,
 	N, D(ImplicitOps | ModRM), N, N,
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 25d7460..faa959e 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -3874,6 +3874,10 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
 #define POST_EX(exit) { .exit_code = (exit), \
 			.stage = x86_icpt_post_except, \
 			.valid = true }
+#define POST_MEM(exit) { .exit_code = (exit), \
+			 .stage = x86_icpt_post_memaccess, \
+			 .valid = true }
+
 
 static struct __x86_intercept {
 	u32 exit_code;
@@ -3887,9 +3891,18 @@ static struct __x86_intercept {
 	[x86_intercept_smsw]		= POST_EX(SVM_EXIT_READ_CR0),
 	[x86_intercept_dr_read]		= POST_EX(SVM_EXIT_READ_DR0),
 	[x86_intercept_dr_write]	= POST_EX(SVM_EXIT_WRITE_DR0),
+	[x86_intercept_sldt]		= POST_MEM(SVM_EXIT_LDTR_READ),
+	[x86_intercept_str]		= POST_MEM(SVM_EXIT_TR_READ),
+	[x86_intercept_lldt]		= POST_MEM(SVM_EXIT_LDTR_WRITE),
+	[x86_intercept_ltr]		= POST_MEM(SVM_EXIT_TR_WRITE),
+	[x86_intercept_sgdt]		= POST_MEM(SVM_EXIT_GDTR_READ),
+	[x86_intercept_sidt]		= POST_MEM(SVM_EXIT_IDTR_READ),
+	[x86_intercept_lgdt]		= POST_MEM(SVM_EXIT_GDTR_WRITE),
+	[x86_intercept_lidt]		= POST_MEM(SVM_EXIT_IDTR_WRITE),
 };
 
 #undef POST_EX
+#undef POST_MEM
 
 static int svm_check_intercept(struct kvm_vcpu *vcpu,
 			       struct x86_instruction_info *info,
-- 
1.7.1

Re: [PATCH 07/13] KVM: SVM: Add intercept checks for descriptor table accesses

[Avi Kivity]

On 03/25/2011 11:29 AM, Joerg Roedel wrote:
> This patch add intercept checks into the KVM instruction
> emulator to check for the 8 instructions that access the
> descriptor table addresses.
>
>
> +static struct opcode group6[] = {
> +	DI(ModRM,	 sldt),
> +	DI(ModRM,	 str),
> +	DI(ModRM | Priv, lldt),
> +	DI(ModRM | Priv, ltr),
> +	N, N, N, N,
> +};
> +
>   static struct group_dual group7 = { {
> -	N, N, DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
> +	DI(ModRM | DstMem | Priv, sgdt), DI(ModRM | DstMem | Priv, sidt),
> +	DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
>   	DI(SrcNone | ModRM | DstMem | Mov, smsw), N,
>   	DI(SrcMem16 | ModRM | Mov | Priv, lmsw),
>   	DI(SrcMem | ModRM | ByteOp | Priv | NoAccess, invlpg),

You're adding decode info for instructions that aren't emulated.  Safe 
AFAICT because of 'default: goto cannot_emulate', I guess.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH 07/13] KVM: SVM: Add intercept checks for descriptor table accesses

[Joerg Roedel]

On Sat, Mar 26, 2011 at 11:43:47AM +0200, Avi Kivity wrote:
> On 03/25/2011 11:29 AM, Joerg Roedel wrote:
>> This patch add intercept checks into the KVM instruction
>> emulator to check for the 8 instructions that access the
>> descriptor table addresses.
>>
>>
>> +static struct opcode group6[] = {
>> +	DI(ModRM,	 sldt),
>> +	DI(ModRM,	 str),
>> +	DI(ModRM | Priv, lldt),
>> +	DI(ModRM | Priv, ltr),
>> +	N, N, N, N,
>> +};
>> +
>>   static struct group_dual group7 = { {
>> -	N, N, DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
>> +	DI(ModRM | DstMem | Priv, sgdt), DI(ModRM | DstMem | Priv, sidt),
>> +	DI(ModRM | SrcMem | Priv, lgdt), DI(ModRM | SrcMem | Priv, lidt),
>>   	DI(SrcNone | ModRM | DstMem | Mov, smsw), N,
>>   	DI(SrcMem16 | ModRM | Mov | Priv, lmsw),
>>   	DI(SrcMem | ModRM | ByteOp | Priv | NoAccess, invlpg),
>
> You're adding decode info for instructions that aren't emulated.  Safe  
> AFAICT because of 'default: goto cannot_emulate', I guess.

Right, all that can happen is that the nested guest shoot itself in the
foot by getting an #UD. Same for the SVM instructions.

	Joerg