PATCH 0/5] Consolidate the insb/outsb emulation into x86_emulate.c

PATCH 0/5] Consolidate the insb/outsb emulation into x86_emulate.c

[Laurent Vivier]

[-- Attachment #1.1: Type: text/plain, Size: 1441 bytes --]

[PATCH 0/5] Consolidate the insb/outsb emulation into x86_emulate.c

From the TODO, this serie of patches consolidates the insb/outsb emulation into
x86_emulate.c.

The prefix decoding part is extracted from x86_emulate_memop() and a function
called x86_decode_prefix() is created.
Then, in vmx.c, handle_io() calls x86_decode_prefix() instead of get_io_count();
in svm.c, io_interception() calls x86_decode_prefix() instead of io_address()
and io_get_override().

*** WARNING: the SVM part has not been tested because I don't have any AMD
system. ***

[PATCH 1/5] x86_ctxt_segment_array: to be able to extract easily prefix decoding
from x86_emulate_memop(), change ctxt.*_base to an array ctxt.base[X86EMUL_BASE_*]

[PATCH 2/5] x86_prefix: to be able to extract easily prefix decoding from
x86_emulate_memop(), group all prefix decoding results in a structure, called
x86_prefix.

[PATCH 3/5] x86_decode_prefix: extract prefix decoding part from
x86_emulate_memop() to x86_decode_prefix().

[PATCH 4/5] vmx-decode_prefix: vmx.c uses x86_decode_prefix() instead of
get_io_count().

[PATCH 5/5] svm-decode_prefix: svm.c uses x86_decode_prefix() instead of
io_address() and io_get_override().

*** WARNING: the SVM part has not been tested because I don't have any AMD
system. ***

-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
          "Software is hard" - Donald Knuth


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

[PATCH 1/5] change ctxt.*_base to an array ctxt.base[X86EMUL_BASE_*]

[Laurent Vivier]

[-- Attachment #1.1.1: Type: text/plain, Size: 334 bytes --]

To be able to extract easily prefix decoding from x86_emulate_memop(), change
ctxt.*_base to an array ctxt.base[X86EMUL_BASE_*]

Signed-off-by: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
          "Software is hard" - Donald Knuth

[-- Attachment #1.1.2: x86_ctxt_segment_array --]
[-- Type: text/plain, Size: 7271 bytes --]

Index: kvm/drivers/kvm/kvm_main.c
===================================================================
--- kvm.orig/drivers/kvm/kvm_main.c	2007-07-31 15:09:01.000000000 +0200
+++ kvm/drivers/kvm/kvm_main.c	2007-07-31 15:12:47.000000000 +0200
@@ -1236,19 +1236,25 @@
 		? X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16;
 
 	if (emulate_ctxt.mode == X86EMUL_MODE_PROT64) {
-		emulate_ctxt.cs_base = 0;
-		emulate_ctxt.ds_base = 0;
-		emulate_ctxt.es_base = 0;
-		emulate_ctxt.ss_base = 0;
+		emulate_ctxt.base[X86EMUL_BASE_CS] = 0;
+		emulate_ctxt.base[X86EMUL_BASE_DS] = 0;
+		emulate_ctxt.base[X86EMUL_BASE_ES] = 0;
+		emulate_ctxt.base[X86EMUL_BASE_SS] = 0;
 	} else {
-		emulate_ctxt.cs_base = get_segment_base(vcpu, VCPU_SREG_CS);
-		emulate_ctxt.ds_base = get_segment_base(vcpu, VCPU_SREG_DS);
-		emulate_ctxt.es_base = get_segment_base(vcpu, VCPU_SREG_ES);
-		emulate_ctxt.ss_base = get_segment_base(vcpu, VCPU_SREG_SS);
+		emulate_ctxt.base[X86EMUL_BASE_CS] =
+					get_segment_base(vcpu, VCPU_SREG_CS);
+		emulate_ctxt.base[X86EMUL_BASE_DS] =
+					get_segment_base(vcpu, VCPU_SREG_DS);
+		emulate_ctxt.base[X86EMUL_BASE_ES] =
+					get_segment_base(vcpu, VCPU_SREG_ES);
+		emulate_ctxt.base[X86EMUL_BASE_SS] =
+					get_segment_base(vcpu, VCPU_SREG_SS);
 	}
 
-	emulate_ctxt.gs_base = get_segment_base(vcpu, VCPU_SREG_GS);
-	emulate_ctxt.fs_base = get_segment_base(vcpu, VCPU_SREG_FS);
+	emulate_ctxt.base[X86EMUL_BASE_GS] =
+					get_segment_base(vcpu, VCPU_SREG_GS);
+	emulate_ctxt.base[X86EMUL_BASE_FS] =
+					get_segment_base(vcpu, VCPU_SREG_FS);
 
 	vcpu->mmio_is_write = 0;
 	r = x86_emulate_memop(&emulate_ctxt, &emulate_ops);
Index: kvm/drivers/kvm/x86_emulate.c
===================================================================
--- kvm.orig/drivers/kvm/x86_emulate.c	2007-07-31 15:09:01.000000000 +0200
+++ kvm/drivers/kvm/x86_emulate.c	2007-07-31 17:01:52.000000000 +0200
@@ -419,7 +419,8 @@
 /* Fetch next part of the instruction being emulated. */
 #define insn_fetch(_type, _size, _eip)                                  \
 ({	unsigned long _x;						\
-	rc = ops->read_std((unsigned long)(_eip) + ctxt->cs_base, &_x,	\
+	rc = ops->read_std((unsigned long)(_eip) + 			\
+			   ctxt->base[X86EMUL_BASE_CS], &_x,		\
                                                   (_size), ctxt->vcpu); \
 	if ( rc != 0 )							\
 		goto done;						\
@@ -484,7 +485,7 @@
 	unsigned d;
 	u8 b, sib, twobyte = 0, rex_prefix = 0;
 	u8 modrm, modrm_mod = 0, modrm_reg = 0, modrm_rm = 0;
-	unsigned long *override_base = NULL;
+	int override_base = -1;
 	unsigned int op_bytes, ad_bytes, lock_prefix = 0, rep_prefix = 0, i;
 	int rc = 0;
 	struct operand src, dst;
@@ -533,22 +534,22 @@
 				ad_bytes ^= 6;	/* switch between 2/4 bytes */
 			break;
 		case 0x2e:	/* CS override */
-			override_base = &ctxt->cs_base;
+			override_base = X86EMUL_BASE_CS;
 			break;
 		case 0x3e:	/* DS override */
-			override_base = &ctxt->ds_base;
+			override_base = X86EMUL_BASE_DS;
 			break;
 		case 0x26:	/* ES override */
-			override_base = &ctxt->es_base;
+			override_base = X86EMUL_BASE_ES;
 			break;
 		case 0x64:	/* FS override */
-			override_base = &ctxt->fs_base;
+			override_base = X86EMUL_BASE_FS;
 			break;
 		case 0x65:	/* GS override */
-			override_base = &ctxt->gs_base;
+			override_base = X86EMUL_BASE_GS;
 			break;
 		case 0x36:	/* SS override */
-			override_base = &ctxt->ss_base;
+			override_base = X86EMUL_BASE_SS;
 			break;
 		case 0xf0:	/* LOCK */
 			lock_prefix = 1;
@@ -654,8 +655,8 @@
 			}
 			if (modrm_rm == 2 || modrm_rm == 3 ||
 			    (modrm_rm == 6 && modrm_mod != 0))
-				if (!override_base)
-					override_base = &ctxt->ss_base;
+				if (override_base == -1)
+					override_base = X86EMUL_BASE_SS;
 			modrm_ea = (u16)modrm_ea;
 		} else {
 			/* 32/64-bit ModR/M decode. */
@@ -708,15 +709,15 @@
 				break;
 			}
 		}
-		if (!override_base)
-			override_base = &ctxt->ds_base;
+		if (override_base == -1)
+			override_base = X86EMUL_BASE_DS;
 		if (mode == X86EMUL_MODE_PROT64 &&
-		    override_base != &ctxt->fs_base &&
-		    override_base != &ctxt->gs_base)
-			override_base = NULL;
+		    override_base != X86EMUL_BASE_FS &&
+		    override_base != X86EMUL_BASE_GS)
+			override_base = -1;
 
-		if (override_base)
-			modrm_ea += *override_base;
+		if (override_base != -1)
+			modrm_ea += ctxt->base[override_base];
 
 		if (rip_relative) {
 			modrm_ea += _eip;
@@ -963,8 +964,9 @@
 		/* 64-bit mode: POP always pops a 64-bit operand. */
 		if (mode == X86EMUL_MODE_PROT64)
 			dst.bytes = 8;
-		if ((rc = ops->read_std(register_address(ctxt->ss_base,
-							 _regs[VCPU_REGS_RSP]),
+		if ((rc = ops->read_std(
+				register_address(ctxt->base[X86EMUL_BASE_SS],
+					_regs[VCPU_REGS_RSP]),
 					&dst.val, dst.bytes, ctxt->vcpu)) != 0)
 			goto done;
 		register_address_increment(_regs[VCPU_REGS_RSP], dst.bytes);
@@ -1056,8 +1058,8 @@
 			register_address_increment(_regs[VCPU_REGS_RSP],
 						   -dst.bytes);
 			if ((rc = ops->write_std(
-				     register_address(ctxt->ss_base,
-						      _regs[VCPU_REGS_RSP]),
+				register_address(ctxt->base[X86EMUL_BASE_SS],
+						 _regs[VCPU_REGS_RSP]),
 				     &dst.val, dst.bytes, ctxt->vcpu)) != 0)
 				goto done;
 			no_wb = 1;
@@ -1128,10 +1130,12 @@
 	case 0xa4 ... 0xa5:	/* movs */
 		dst.type = OP_MEM;
 		dst.bytes = (d & ByteOp) ? 1 : op_bytes;
-		dst.ptr = (unsigned long *)register_address(ctxt->es_base,
+		dst.ptr = (unsigned long *)register_address(
+						ctxt->base[X86EMUL_BASE_ES],
 							_regs[VCPU_REGS_RDI]);
 		if ((rc = ops->read_emulated(register_address(
-		      override_base ? *override_base : ctxt->ds_base,
+		      (override_base != -1) ?
+		      	ctxt->base[override_base] : ctxt->base[X86EMUL_BASE_DS],
 		      _regs[VCPU_REGS_RSI]), &dst.val, dst.bytes, ctxt->vcpu)) != 0)
 			goto done;
 		register_address_increment(_regs[VCPU_REGS_RSI],
@@ -1173,7 +1177,8 @@
 		dst.ptr = (unsigned long *)&_regs[b & 0x7];
 
 pop_instruction:
-		if ((rc = ops->read_std(register_address(ctxt->ss_base,
+		if ((rc = ops->read_std(
+			register_address(ctxt->base[X86EMUL_BASE_SS],
 			_regs[VCPU_REGS_RSP]), dst.ptr, op_bytes, ctxt->vcpu))
 			!= 0)
 			goto done;
Index: kvm/drivers/kvm/x86_emulate.h
===================================================================
--- kvm.orig/drivers/kvm/x86_emulate.h	2007-07-31 15:09:01.000000000 +0200
+++ kvm/drivers/kvm/x86_emulate.h	2007-07-31 17:00:22.000000000 +0200
@@ -112,6 +112,16 @@
 
 };
 
+enum {
+	X86EMUL_BASE_CS,
+	X86EMUL_BASE_DS,
+	X86EMUL_BASE_ES,
+	X86EMUL_BASE_SS,
+	X86EMUL_BASE_GS,
+	X86EMUL_BASE_FS,
+	X86EMUL_BASE_SIZE
+};
+
 struct x86_emulate_ctxt {
 	/* Register state before/after emulation. */
 	struct kvm_vcpu *vcpu;
@@ -122,13 +132,7 @@
 
 	/* Emulated execution mode, represented by an X86EMUL_MODE value. */
 	int mode;
-
-	unsigned long cs_base;
-	unsigned long ds_base;
-	unsigned long es_base;
-	unsigned long ss_base;
-	unsigned long gs_base;
-	unsigned long fs_base;
+	unsigned long base[X86EMUL_BASE_SIZE];
 };
 
 /* Execution mode, passed to the emulator. */

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

[PATCH 2/5] group all prefix decoding results in a structure called x86_prefix

[Laurent Vivier]

[-- Attachment #1.1.1: Type: text/plain, Size: 345 bytes --]

To be able to extract easily prefix decoding from x86_emulate_memop(), group all
prefix decoding results in a structure called x86_prefix.

Signed-off-by: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
          "Software is hard" - Donald Knuth

[-- Attachment #1.1.2: x86_prefix --]
[-- Type: text/plain, Size: 18464 bytes --]

Index: kvm/drivers/kvm/x86_emulate.h
===================================================================
--- kvm.orig/drivers/kvm/x86_emulate.h	2007-07-31 17:00:22.000000000 +0200
+++ kvm/drivers/kvm/x86_emulate.h	2007-07-31 17:03:26.000000000 +0200
@@ -141,6 +141,21 @@
 #define X86EMUL_MODE_PROT32   4	/* 32-bit protected mode. */
 #define X86EMUL_MODE_PROT64   8	/* 64-bit (long) mode.    */
 
+struct x86_prefix {
+	unsigned int op_bytes;
+	unsigned int ad_bytes;
+	int override_base;
+
+	u8 lock;	/* LOCK prefix */
+	u8 rep;		/* REP prefix */
+	u8 rex;		/* REX prefix */
+
+	int index_reg;
+	int base_reg;
+	u8 modrm_reg;
+	u8 modrm_rm;
+};
+
 /* Host execution mode. */
 #if defined(__i386__)
 #define X86EMUL_MODE_HOST X86EMUL_MODE_PROT32
Index: kvm/drivers/kvm/x86_emulate.c
===================================================================
--- kvm.orig/drivers/kvm/x86_emulate.c	2007-07-31 17:01:52.000000000 +0200
+++ kvm/drivers/kvm/x86_emulate.c	2007-07-31 17:07:30.000000000 +0200
@@ -430,18 +430,18 @@
 
 /* Access/update address held in a register, based on addressing mode. */
 #define register_address(base, reg)                                     \
-	((base) + ((ad_bytes == sizeof(unsigned long)) ? (reg) :	\
-		   ((reg) & ((1UL << (ad_bytes << 3)) - 1))))
+	((base) + ((prefix.ad_bytes == sizeof(unsigned long)) ? (reg) :	\
+		   ((reg) & ((1UL << (prefix.ad_bytes << 3)) - 1))))
 
 #define register_address_increment(reg, inc)                            \
 	do {								\
 		/* signed type ensures sign extension to long */        \
 		int _inc = (inc);					\
-		if ( ad_bytes == sizeof(unsigned long) )		\
+		if ( prefix.ad_bytes == sizeof(unsigned long) )		\
 			(reg) += _inc;					\
 		else							\
-			(reg) = ((reg) & ~((1UL << (ad_bytes << 3)) - 1)) | \
-			   (((reg) + _inc) & ((1UL << (ad_bytes << 3)) - 1)); \
+			(reg) = ((reg) & ~((1UL << (prefix.ad_bytes << 3)) - 1)) | \
+			   (((reg) + _inc) & ((1UL << (prefix.ad_bytes << 3)) - 1)); \
 	} while (0)
 
 /*
@@ -483,18 +483,18 @@
 x86_emulate_memop(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
 {
 	unsigned d;
-	u8 b, sib, twobyte = 0, rex_prefix = 0;
-	u8 modrm, modrm_mod = 0, modrm_reg = 0, modrm_rm = 0;
-	int override_base = -1;
-	unsigned int op_bytes, ad_bytes, lock_prefix = 0, rep_prefix = 0, i;
+	u8 b, sib, twobyte = 0;
+	u8 modrm, modrm_mod = 0;
+	unsigned int i;
 	int rc = 0;
 	struct operand src, dst;
 	unsigned long cr2 = ctxt->cr2;
 	int mode = ctxt->mode;
 	unsigned long modrm_ea;
-	int use_modrm_ea, index_reg = 0, base_reg = 0, scale, rip_relative = 0;
+	int use_modrm_ea, scale, rip_relative = 0;
 	int no_wb = 0;
 	u64 msr_data;
+	struct x86_prefix prefix;
 
 	/* Shadow copy of register state. Committed on successful emulation. */
 	unsigned long _regs[NR_VCPU_REGS];
@@ -502,19 +502,21 @@
 	unsigned long modrm_val = 0;
 
 	memcpy(_regs, ctxt->vcpu->regs, sizeof _regs);
+	memset(&prefix, 0, sizeof(prefix));
+	prefix.override_base = -1;
 
 	switch (mode) {
 	case X86EMUL_MODE_REAL:
 	case X86EMUL_MODE_PROT16:
-		op_bytes = ad_bytes = 2;
+		prefix.op_bytes = prefix.ad_bytes = 2;
 		break;
 	case X86EMUL_MODE_PROT32:
-		op_bytes = ad_bytes = 4;
+		prefix.op_bytes = prefix.ad_bytes = 4;
 		break;
 #ifdef CONFIG_X86_64
 	case X86EMUL_MODE_PROT64:
-		op_bytes = 4;
-		ad_bytes = 8;
+		prefix.op_bytes = 4;
+		prefix.ad_bytes = 8;
 		break;
 #endif
 	default:
@@ -525,37 +527,37 @@
 	for (i = 0; i < 8; i++) {
 		switch (b = insn_fetch(u8, 1, _eip)) {
 		case 0x66:	/* operand-size override */
-			op_bytes ^= 6;	/* switch between 2/4 bytes */
+			prefix.op_bytes ^= 6;	/* switch between 2/4 bytes */
 			break;
 		case 0x67:	/* address-size override */
 			if (mode == X86EMUL_MODE_PROT64)
-				ad_bytes ^= 12;	/* switch between 4/8 bytes */
+				prefix.ad_bytes ^= 12;	/* switch between 4/8 bytes */
 			else
-				ad_bytes ^= 6;	/* switch between 2/4 bytes */
+				prefix.ad_bytes ^= 6;	/* switch between 2/4 bytes */
 			break;
 		case 0x2e:	/* CS override */
-			override_base = X86EMUL_BASE_CS;
+			prefix.override_base = X86EMUL_BASE_CS;
 			break;
 		case 0x3e:	/* DS override */
-			override_base = X86EMUL_BASE_DS;
+			prefix.override_base = X86EMUL_BASE_DS;
 			break;
 		case 0x26:	/* ES override */
-			override_base = X86EMUL_BASE_ES;
+			prefix.override_base = X86EMUL_BASE_ES;
 			break;
 		case 0x64:	/* FS override */
-			override_base = X86EMUL_BASE_FS;
+			prefix.override_base = X86EMUL_BASE_FS;
 			break;
 		case 0x65:	/* GS override */
-			override_base = X86EMUL_BASE_GS;
+			prefix.override_base = X86EMUL_BASE_GS;
 			break;
 		case 0x36:	/* SS override */
-			override_base = X86EMUL_BASE_SS;
+			prefix.override_base = X86EMUL_BASE_SS;
 			break;
 		case 0xf0:	/* LOCK */
-			lock_prefix = 1;
+			prefix.lock = 1;
 			break;
 		case 0xf3:	/* REP/REPE/REPZ */
-			rep_prefix = 1;
+			prefix.rep = 1;
 			break;
 		case 0xf2:	/* REPNE/REPNZ */
 			break;
@@ -568,12 +570,12 @@
 
 	/* REX prefix. */
 	if ((mode == X86EMUL_MODE_PROT64) && ((b & 0xf0) == 0x40)) {
-		rex_prefix = b;
+		prefix.rex = b;
 		if (b & 8)
-			op_bytes = 8;	/* REX.W */
-		modrm_reg = (b & 4) << 1;	/* REX.R */
-		index_reg = (b & 2) << 2; /* REX.X */
-		modrm_rm = base_reg = (b & 1) << 3; /* REG.B */
+			prefix.op_bytes = 8;	/* REX.W */
+		prefix.modrm_reg = (b & 4) << 1;	/* REX.R */
+		prefix.index_reg = (b & 2) << 2; /* REX.X */
+		prefix.modrm_rm = prefix.base_reg = (b & 1) << 3; /* REG.B */
 		b = insn_fetch(u8, 1, _eip);
 	}
 
@@ -596,18 +598,18 @@
 	if (d & ModRM) {
 		modrm = insn_fetch(u8, 1, _eip);
 		modrm_mod |= (modrm & 0xc0) >> 6;
-		modrm_reg |= (modrm & 0x38) >> 3;
-		modrm_rm |= (modrm & 0x07);
+		prefix.modrm_reg |= (modrm & 0x38) >> 3;
+		prefix.modrm_rm |= (modrm & 0x07);
 		modrm_ea = 0;
 		use_modrm_ea = 1;
 
 		if (modrm_mod == 3) {
 			modrm_val = *(unsigned long *)
-				decode_register(modrm_rm, _regs, d & ByteOp);
+				decode_register(prefix.modrm_rm, _regs, d & ByteOp);
 			goto modrm_done;
 		}
 
-		if (ad_bytes == 2) {
+		if (prefix.ad_bytes == 2) {
 			unsigned bx = _regs[VCPU_REGS_RBX];
 			unsigned bp = _regs[VCPU_REGS_RBP];
 			unsigned si = _regs[VCPU_REGS_RSI];
@@ -616,7 +618,7 @@
 			/* 16-bit ModR/M decode. */
 			switch (modrm_mod) {
 			case 0:
-				if (modrm_rm == 6)
+				if (prefix.modrm_rm == 6)
 					modrm_ea += insn_fetch(u16, 2, _eip);
 				break;
 			case 1:
@@ -626,7 +628,7 @@
 				modrm_ea += insn_fetch(u16, 2, _eip);
 				break;
 			}
-			switch (modrm_rm) {
+			switch (prefix.modrm_rm) {
 			case 0:
 				modrm_ea += bx + si;
 				break;
@@ -653,52 +655,52 @@
 				modrm_ea += bx;
 				break;
 			}
-			if (modrm_rm == 2 || modrm_rm == 3 ||
-			    (modrm_rm == 6 && modrm_mod != 0))
-				if (override_base == -1)
-					override_base = X86EMUL_BASE_SS;
+			if (prefix.modrm_rm == 2 || prefix.modrm_rm == 3 ||
+			    (prefix.modrm_rm == 6 && modrm_mod != 0))
+				if (prefix.override_base == -1)
+					prefix.override_base = X86EMUL_BASE_SS;
 			modrm_ea = (u16)modrm_ea;
 		} else {
 			/* 32/64-bit ModR/M decode. */
-			switch (modrm_rm) {
+			switch (prefix.modrm_rm) {
 			case 4:
 			case 12:
 				sib = insn_fetch(u8, 1, _eip);
-				index_reg |= (sib >> 3) & 7;
-				base_reg |= sib & 7;
+				prefix.index_reg |= (sib >> 3) & 7;
+				prefix.base_reg |= sib & 7;
 				scale = sib >> 6;
 
-				switch (base_reg) {
+				switch (prefix.base_reg) {
 				case 5:
 					if (modrm_mod != 0)
-						modrm_ea += _regs[base_reg];
+						modrm_ea += _regs[prefix.base_reg];
 					else
 						modrm_ea += insn_fetch(s32, 4, _eip);
 					break;
 				default:
-					modrm_ea += _regs[base_reg];
+					modrm_ea += _regs[prefix.base_reg];
 				}
-				switch (index_reg) {
+				switch (prefix.index_reg) {
 				case 4:
 					break;
 				default:
-					modrm_ea += _regs[index_reg] << scale;
+					modrm_ea += _regs[prefix.index_reg] << scale;
 
 				}
 				break;
 			case 5:
 				if (modrm_mod != 0)
-					modrm_ea += _regs[modrm_rm];
+					modrm_ea += _regs[prefix.modrm_rm];
 				else if (mode == X86EMUL_MODE_PROT64)
 					rip_relative = 1;
 				break;
 			default:
-				modrm_ea += _regs[modrm_rm];
+				modrm_ea += _regs[prefix.modrm_rm];
 				break;
 			}
 			switch (modrm_mod) {
 			case 0:
-				if (modrm_rm == 5)
+				if (prefix.modrm_rm == 5)
 					modrm_ea += insn_fetch(s32, 4, _eip);
 				break;
 			case 1:
@@ -709,15 +711,15 @@
 				break;
 			}
 		}
-		if (override_base == -1)
-			override_base = X86EMUL_BASE_DS;
+		if (prefix.override_base == -1)
+			prefix.override_base = X86EMUL_BASE_DS;
 		if (mode == X86EMUL_MODE_PROT64 &&
-		    override_base != X86EMUL_BASE_FS &&
-		    override_base != X86EMUL_BASE_GS)
-			override_base = -1;
+		    prefix.override_base != X86EMUL_BASE_FS &&
+		    prefix.override_base != X86EMUL_BASE_GS)
+			prefix.override_base = -1;
 
-		if (override_base != -1)
-			modrm_ea += ctxt->base[override_base];
+		if (prefix.override_base != -1)
+			modrm_ea += ctxt->base[prefix.override_base];
 
 		if (rip_relative) {
 			modrm_ea += _eip;
@@ -729,13 +731,13 @@
 				if (d & ByteOp)
 					modrm_ea += 1;
 				else
-					if (op_bytes == 8)
+					if (prefix.op_bytes == 8)
 						modrm_ea += 4;
 					else
-						modrm_ea += op_bytes;
+						modrm_ea += prefix.op_bytes;
 			}
 		}
-		if (ad_bytes != 8)
+		if (prefix.ad_bytes != 8)
 			modrm_ea = (u32)modrm_ea;
 		cr2 = modrm_ea;
 	modrm_done:
@@ -752,13 +754,13 @@
 	case SrcReg:
 		src.type = OP_REG;
 		if (d & ByteOp) {
-			src.ptr = decode_register(modrm_reg, _regs,
-						  (rex_prefix == 0));
+			src.ptr = decode_register(prefix.modrm_reg, _regs,
+						  (prefix.rex == 0));
 			src.val = src.orig_val = *(u8 *) src.ptr;
 			src.bytes = 1;
 		} else {
-			src.ptr = decode_register(modrm_reg, _regs, 0);
-			switch ((src.bytes = op_bytes)) {
+			src.ptr = decode_register(prefix.modrm_reg, _regs, 0);
+			switch ((src.bytes = prefix.op_bytes)) {
 			case 2:
 				src.val = src.orig_val = *(u16 *) src.ptr;
 				break;
@@ -778,7 +780,7 @@
 		src.bytes = 4;
 		goto srcmem_common;
 	case SrcMem:
-		src.bytes = (d & ByteOp) ? 1 : op_bytes;
+		src.bytes = (d & ByteOp) ? 1 : prefix.op_bytes;
 	      srcmem_common:
 		src.type = OP_MEM;
 		src.ptr = (unsigned long *)cr2;
@@ -790,7 +792,7 @@
 	case SrcImm:
 		src.type = OP_IMM;
 		src.ptr = (unsigned long *)_eip;
-		src.bytes = (d & ByteOp) ? 1 : op_bytes;
+		src.bytes = (d & ByteOp) ? 1 : prefix.op_bytes;
 		if (src.bytes == 8)
 			src.bytes = 4;
 		/* NB. Immediates are sign-extended as necessary. */
@@ -823,13 +825,13 @@
 		dst.type = OP_REG;
 		if ((d & ByteOp)
 		    && !(twobyte && (b == 0xb6 || b == 0xb7))) {
-			dst.ptr = decode_register(modrm_reg, _regs,
-						  (rex_prefix == 0));
+			dst.ptr = decode_register(prefix.modrm_reg, _regs,
+						  (prefix.rex == 0));
 			dst.val = *(u8 *) dst.ptr;
 			dst.bytes = 1;
 		} else {
-			dst.ptr = decode_register(modrm_reg, _regs, 0);
-			switch ((dst.bytes = op_bytes)) {
+			dst.ptr = decode_register(prefix.modrm_reg, _regs, 0);
+			switch ((dst.bytes = prefix.op_bytes)) {
 			case 2:
 				dst.val = *(u16 *)dst.ptr;
 				break;
@@ -845,7 +847,7 @@
 	case DstMem:
 		dst.type = OP_MEM;
 		dst.ptr = (unsigned long *)cr2;
-		dst.bytes = (d & ByteOp) ? 1 : op_bytes;
+		dst.bytes = (d & ByteOp) ? 1 : prefix.op_bytes;
 		if (d & BitOp) {
 			unsigned long mask = ~(dst.bytes * 8 - 1);
 
@@ -901,7 +903,7 @@
 		dst.val = (s32) src.val;
 		break;
 	case 0x80 ... 0x83:	/* Grp1 */
-		switch (modrm_reg) {
+		switch (prefix.modrm_reg) {
 		case 0:
 			goto add;
 		case 1:
@@ -945,16 +947,16 @@
 		 * prefix.
 		 */
 		dst.val = src.val;
-		lock_prefix = 1;
+		prefix.lock = 1;
 		break;
 	case 0xa0 ... 0xa1:	/* mov */
 		dst.ptr = (unsigned long *)&_regs[VCPU_REGS_RAX];
 		dst.val = src.val;
-		_eip += ad_bytes;	/* skip src displacement */
+		_eip += prefix.ad_bytes;	/* skip src displacement */
 		break;
 	case 0xa2 ... 0xa3:	/* mov */
 		dst.val = (unsigned long)_regs[VCPU_REGS_RAX];
-		_eip += ad_bytes;	/* skip dst displacement */
+		_eip += prefix.ad_bytes;	/* skip dst displacement */
 		break;
 	case 0x88 ... 0x8b:	/* mov */
 	case 0xc6 ... 0xc7:	/* mov (sole member of Grp11) */
@@ -973,7 +975,7 @@
 		break;
 	case 0xc0 ... 0xc1:
 	      grp2:		/* Grp2 */
-		switch (modrm_reg) {
+		switch (prefix.modrm_reg) {
 		case 0:	/* rol */
 			emulate_2op_SrcB("rol", src, dst, _eflags);
 			break;
@@ -1005,7 +1007,7 @@
 		src.val = _regs[VCPU_REGS_RCX];
 		goto grp2;
 	case 0xf6 ... 0xf7:	/* Grp3 */
-		switch (modrm_reg) {
+		switch (prefix.modrm_reg) {
 		case 0 ... 1:	/* test */
 			/*
 			 * Special case in Grp3: test has an immediate
@@ -1013,7 +1015,7 @@
 			 */
 			src.type = OP_IMM;
 			src.ptr = (unsigned long *)_eip;
-			src.bytes = (d & ByteOp) ? 1 : op_bytes;
+			src.bytes = (d & ByteOp) ? 1 : prefix.op_bytes;
 			if (src.bytes == 8)
 				src.bytes = 4;
 			switch (src.bytes) {
@@ -1039,7 +1041,7 @@
 		}
 		break;
 	case 0xfe ... 0xff:	/* Grp4/Grp5 */
-		switch (modrm_reg) {
+		switch (prefix.modrm_reg) {
 		case 0:	/* inc */
 			emulate_1op("inc", dst, _eflags);
 			break;
@@ -1091,7 +1093,7 @@
 			}
 			break;
 		case OP_MEM:
-			if (lock_prefix)
+			if (prefix.lock)
 				rc = ops->cmpxchg_emulated((unsigned long)dst.
 							   ptr, &dst.orig_val,
 							   &dst.val, dst.bytes,
@@ -1118,7 +1120,7 @@
 special_insn:
 	if (twobyte)
 		goto twobyte_special_insn;
-	if (rep_prefix) {
+	if (prefix.rep) {
 		if (_regs[VCPU_REGS_RCX] == 0) {
 			ctxt->vcpu->rip = _eip;
 			goto done;
@@ -1129,13 +1131,13 @@
 	switch (b) {
 	case 0xa4 ... 0xa5:	/* movs */
 		dst.type = OP_MEM;
-		dst.bytes = (d & ByteOp) ? 1 : op_bytes;
+		dst.bytes = (d & ByteOp) ? 1 : prefix.op_bytes;
 		dst.ptr = (unsigned long *)register_address(
 						ctxt->base[X86EMUL_BASE_ES],
 							_regs[VCPU_REGS_RDI]);
 		if ((rc = ops->read_emulated(register_address(
-		      (override_base != -1) ?
-		      	ctxt->base[override_base] : ctxt->base[X86EMUL_BASE_DS],
+		      (prefix.override_base != -1) ?
+		      	ctxt->base[prefix.override_base] : ctxt->base[X86EMUL_BASE_DS],
 		      _regs[VCPU_REGS_RSI]), &dst.val, dst.bytes, ctxt->vcpu)) != 0)
 			goto done;
 		register_address_increment(_regs[VCPU_REGS_RSI],
@@ -1148,7 +1150,7 @@
 		goto cannot_emulate;
 	case 0xaa ... 0xab:	/* stos */
 		dst.type = OP_MEM;
-		dst.bytes = (d & ByteOp) ? 1 : op_bytes;
+		dst.bytes = (d & ByteOp) ? 1 : prefix.op_bytes;
 		dst.ptr = (unsigned long *)cr2;
 		dst.val = _regs[VCPU_REGS_RAX];
 		register_address_increment(_regs[VCPU_REGS_RDI],
@@ -1156,7 +1158,7 @@
 		break;
 	case 0xac ... 0xad:	/* lods */
 		dst.type = OP_REG;
-		dst.bytes = (d & ByteOp) ? 1 : op_bytes;
+		dst.bytes = (d & ByteOp) ? 1 : prefix.op_bytes;
 		dst.ptr = (unsigned long *)&_regs[VCPU_REGS_RAX];
 		if ((rc = ops->read_emulated(cr2, &dst.val, dst.bytes,
 					     ctxt->vcpu)) != 0)
@@ -1179,11 +1181,11 @@
 pop_instruction:
 		if ((rc = ops->read_std(
 			register_address(ctxt->base[X86EMUL_BASE_SS],
-			_regs[VCPU_REGS_RSP]), dst.ptr, op_bytes, ctxt->vcpu))
+			_regs[VCPU_REGS_RSP]), dst.ptr, prefix.op_bytes, ctxt->vcpu))
 			!= 0)
 			goto done;
 
-		register_address_increment(_regs[VCPU_REGS_RSP], op_bytes);
+		register_address_increment(_regs[VCPU_REGS_RSP], prefix.op_bytes);
 		no_wb = 1; /* Disable writeback. */
 		break;
 	}
@@ -1194,20 +1196,20 @@
 	case 0x01: /* lgdt, lidt, lmsw */
 		/* Disable writeback. */
 		no_wb = 1;
-		switch (modrm_reg) {
+		switch (prefix.modrm_reg) {
 			u16 size;
 			unsigned long address;
 
 		case 2: /* lgdt */
 			rc = read_descriptor(ctxt, ops, src.ptr,
-					     &size, &address, op_bytes);
+					     &size, &address, prefix.op_bytes);
 			if (rc)
 				goto done;
 			realmode_lgdt(ctxt->vcpu, size, address);
 			break;
 		case 3: /* lidt */
 			rc = read_descriptor(ctxt, ops, src.ptr,
-					     &size, &address, op_bytes);
+					     &size, &address, prefix.op_bytes);
 			if (rc)
 				goto done;
 			realmode_lidt(ctxt->vcpu, size, address);
@@ -1215,7 +1217,7 @@
 		case 4: /* smsw */
 			if (modrm_mod != 3)
 				goto cannot_emulate;
-			*(u16 *)&_regs[modrm_rm]
+			*(u16 *)&_regs[prefix.modrm_rm]
 				= realmode_get_cr(ctxt->vcpu, 0);
 			break;
 		case 6: /* lmsw */
@@ -1233,12 +1235,12 @@
 	case 0x21: /* mov from dr to reg */
 		if (modrm_mod != 3)
 			goto cannot_emulate;
-		rc = emulator_get_dr(ctxt, modrm_reg, &_regs[modrm_rm]);
+		rc = emulator_get_dr(ctxt, prefix.modrm_reg, &_regs[prefix.modrm_rm]);
 		break;
 	case 0x23: /* mov from reg to dr */
 		if (modrm_mod != 3)
 			goto cannot_emulate;
-		rc = emulator_set_dr(ctxt, modrm_reg, _regs[modrm_rm]);
+		rc = emulator_set_dr(ctxt, prefix.modrm_reg, _regs[prefix.modrm_rm]);
 		break;
 	case 0x40 ... 0x4f:	/* cmov */
 		dst.val = dst.orig_val = src.val;
@@ -1310,7 +1312,7 @@
 		emulate_2op_SrcV_nobyte("bts", src, dst, _eflags);
 		break;
 	case 0xb6 ... 0xb7:	/* movzx */
-		dst.bytes = op_bytes;
+		dst.bytes = prefix.op_bytes;
 		dst.val = (d & ByteOp) ? (u8) src.val : (u16) src.val;
 		break;
 	case 0xbb:
@@ -1319,7 +1321,7 @@
 		emulate_2op_SrcV_nobyte("btc", src, dst, _eflags);
 		break;
 	case 0xba:		/* Grp8 */
-		switch (modrm_reg & 3) {
+		switch (prefix.modrm_reg & 3) {
 		case 0:
 			goto bt;
 		case 1:
@@ -1331,7 +1333,7 @@
 		}
 		break;
 	case 0xbe ... 0xbf:	/* movsx */
-		dst.bytes = op_bytes;
+		dst.bytes = prefix.op_bytes;
 		dst.val = (d & ByteOp) ? (s8) src.val : (s16) src.val;
 		break;
 	}
@@ -1352,12 +1354,12 @@
 	case 0x20: /* mov cr, reg */
 		if (modrm_mod != 3)
 			goto cannot_emulate;
-		_regs[modrm_rm] = realmode_get_cr(ctxt->vcpu, modrm_reg);
+		_regs[prefix.modrm_rm] = realmode_get_cr(ctxt->vcpu, prefix.modrm_reg);
 		break;
 	case 0x22: /* mov reg, cr */
 		if (modrm_mod != 3)
 			goto cannot_emulate;
-		realmode_set_cr(ctxt->vcpu, modrm_reg, modrm_val, &_eflags);
+		realmode_set_cr(ctxt->vcpu, prefix.modrm_reg, modrm_val, &_eflags);
 		break;
 	case 0x30:
 		/* wrmsr */

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

[PATCH 3/5] extract prefix decoding part from x86_emulate_memop() to x86_decode_prefix()

[Laurent Vivier]

[-- Attachment #1.1.1: Type: text/plain, Size: 283 bytes --]

Extract prefix decoding part from x86_emulate_memop() to x86_decode_prefix().

Signed-off-by: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
          "Software is hard" - Donald Knuth

[-- Attachment #1.1.2: x86_decode_prefix --]
[-- Type: text/plain, Size: 5811 bytes --]

Index: kvm/drivers/kvm/x86_emulate.c
===================================================================
--- kvm.orig/drivers/kvm/x86_emulate.c	2007-07-31 17:44:35.000000000 +0200
+++ kvm/drivers/kvm/x86_emulate.c	2007-07-31 18:23:39.000000000 +0200
@@ -480,43 +480,24 @@
 }
 
 int
-x86_emulate_memop(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
+x86_decode_prefix(int mode, u8 *inst, struct x86_prefix *prefix)
 {
-	unsigned d;
-	u8 b, sib, twobyte = 0;
-	u8 modrm, modrm_mod = 0;
-	unsigned int i;
-	int rc = 0;
-	struct operand src, dst;
-	unsigned long cr2 = ctxt->cr2;
-	int mode = ctxt->mode;
-	unsigned long modrm_ea;
-	int use_modrm_ea, scale, rip_relative = 0;
-	int no_wb = 0;
-	u64 msr_data;
-	struct x86_prefix prefix;
-
-	/* Shadow copy of register state. Committed on successful emulation. */
-	unsigned long _regs[NR_VCPU_REGS];
-	unsigned long _eip = ctxt->vcpu->rip, _eflags = ctxt->eflags;
-	unsigned long modrm_val = 0;
-
-	memcpy(_regs, ctxt->vcpu->regs, sizeof _regs);
-	memset(&prefix, 0, sizeof(prefix));
-	prefix.override_base = -1;
+	unsigned int op_bytes, ad_bytes;
+	int i;
+	u8 b;
 
 	switch (mode) {
 	case X86EMUL_MODE_REAL:
 	case X86EMUL_MODE_PROT16:
-		prefix.op_bytes = prefix.ad_bytes = 2;
+		op_bytes = ad_bytes = 2;
 		break;
 	case X86EMUL_MODE_PROT32:
-		prefix.op_bytes = prefix.ad_bytes = 4;
+		op_bytes = ad_bytes = 4;
 		break;
 #ifdef CONFIG_X86_64
 	case X86EMUL_MODE_PROT64:
-		prefix.op_bytes = 4;
-		prefix.ad_bytes = 8;
+		op_bytes = 4;
+		ad_bytes = 8;
 		break;
 #endif
 	default:
@@ -525,39 +506,39 @@
 
 	/* Legacy prefixes. */
 	for (i = 0; i < 8; i++) {
-		switch (b = insn_fetch(u8, 1, _eip)) {
+		switch (b = inst[i]) {
 		case 0x66:	/* operand-size override */
-			prefix.op_bytes ^= 6;	/* switch between 2/4 bytes */
+			op_bytes ^= 6;	/* switch between 2/4 bytes */
 			break;
 		case 0x67:	/* address-size override */
 			if (mode == X86EMUL_MODE_PROT64)
-				prefix.ad_bytes ^= 12;	/* switch between 4/8 bytes */
+				ad_bytes ^= 12;	/* switch between 4/8 bytes */
 			else
-				prefix.ad_bytes ^= 6;	/* switch between 2/4 bytes */
+				ad_bytes ^= 6;	/* switch between 2/4 bytes */
 			break;
 		case 0x2e:	/* CS override */
-			prefix.override_base = X86EMUL_BASE_CS;
+			prefix->override_base = X86EMUL_BASE_CS;
 			break;
 		case 0x3e:	/* DS override */
-			prefix.override_base = X86EMUL_BASE_DS;
+			prefix->override_base = X86EMUL_BASE_DS;
 			break;
 		case 0x26:	/* ES override */
-			prefix.override_base = X86EMUL_BASE_ES;
+			prefix->override_base = X86EMUL_BASE_ES;
 			break;
 		case 0x64:	/* FS override */
-			prefix.override_base = X86EMUL_BASE_FS;
+			prefix->override_base = X86EMUL_BASE_FS;
 			break;
 		case 0x65:	/* GS override */
-			prefix.override_base = X86EMUL_BASE_GS;
+			prefix->override_base = X86EMUL_BASE_GS;
 			break;
 		case 0x36:	/* SS override */
-			prefix.override_base = X86EMUL_BASE_SS;
+			prefix->override_base = X86EMUL_BASE_SS;
 			break;
 		case 0xf0:	/* LOCK */
-			prefix.lock = 1;
+			prefix->lock = 1;
 			break;
 		case 0xf3:	/* REP/REPE/REPZ */
-			prefix.rep = 1;
+			prefix->rep = 1;
 			break;
 		case 0xf2:	/* REPNE/REPNZ */
 			break;
@@ -570,15 +551,62 @@
 
 	/* REX prefix. */
 	if ((mode == X86EMUL_MODE_PROT64) && ((b & 0xf0) == 0x40)) {
-		prefix.rex = b;
+		prefix->rex = b;
 		if (b & 8)
-			prefix.op_bytes = 8;	/* REX.W */
-		prefix.modrm_reg = (b & 4) << 1;	/* REX.R */
-		prefix.index_reg = (b & 2) << 2; /* REX.X */
-		prefix.modrm_rm = prefix.base_reg = (b & 1) << 3; /* REG.B */
-		b = insn_fetch(u8, 1, _eip);
+			op_bytes = 8;	/* REX.W */
+		prefix->modrm_reg = (b & 4) << 1;	/* REX.R */
+		prefix->index_reg = (b & 2) << 2; /* REX.X */
+		prefix->modrm_rm = prefix->base_reg = (b & 1) << 3; /* REG.B */
+		i++;
 	}
 
+	prefix->op_bytes = op_bytes;
+	prefix->ad_bytes = ad_bytes;
+
+	return i;
+}
+EXPORT_SYMBOL_GPL(x86_decode_prefix);
+
+int
+x86_emulate_memop(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
+{
+	unsigned d;
+	u8 b, sib, twobyte = 0;
+	u8 modrm, modrm_mod = 0;
+	int rc = 0;
+	struct operand src, dst;
+	unsigned long cr2 = ctxt->cr2;
+	int mode = ctxt->mode;
+	unsigned long modrm_ea;
+	int use_modrm_ea, scale, rip_relative = 0;
+	int no_wb = 0;
+	u64 msr_data, inst;
+	struct x86_prefix prefix;
+	int count;
+
+	/* Shadow copy of register state. Committed on successful emulation. */
+	unsigned long _regs[NR_VCPU_REGS];
+	unsigned long _eip = ctxt->vcpu->rip, _eflags = ctxt->eflags;
+	unsigned long modrm_val = 0;
+
+	memcpy(_regs, ctxt->vcpu->regs, sizeof _regs);
+
+	/* decode prefixes */
+
+	rc = ops->read_std(_eip + ctxt->base[X86EMUL_BASE_CS],
+			   &inst, sizeof(inst), ctxt->vcpu);
+	if ( rc != 0 )
+		goto done;
+
+	memset(&prefix, 0, sizeof(prefix));
+	prefix.override_base = -1;
+
+	count = x86_decode_prefix(mode, (u8*)&inst, &prefix);
+	if (count == -1)
+		return -1;
+	_eip += count;
+	b = insn_fetch(u8, 1, _eip);
+
 	/* Opcode byte(s). */
 	d = opcode_table[b];
 	if (d == 0) {
Index: kvm/drivers/kvm/x86_emulate.h
===================================================================
--- kvm.orig/drivers/kvm/x86_emulate.h	2007-07-31 18:00:51.000000000 +0200
+++ kvm/drivers/kvm/x86_emulate.h	2007-07-31 18:01:53.000000000 +0200
@@ -164,6 +164,13 @@
 #endif
 
 /*
+ * x86_decode_prefix: Decode instruction prefixes
+ * Returns -1 on failure, 0 on success.
+ */
+int
+x86_decode_prefix(int mode, u8 *inst, struct x86_prefix *prefix);
+
+/*
  * x86_emulate_memop: Emulate an instruction that faulted attempting to
  *                    read/write a 'special' memory area.
  * Returns -1 on failure, 0 on success.

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

[PATCH 4/5] vmx.c uses x86_decode_prefix() instead of get_io_count().

[Laurent Vivier]

[-- Attachment #1.1.1: Type: text/plain, Size: 263 bytes --]

vmx.c uses x86_decode_prefix() instead of get_io_count().

Signed-off-by: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
          "Software is hard" - Donald Knuth

[-- Attachment #1.1.2: vmx-decode_prefix --]
[-- Type: text/plain, Size: 2515 bytes --]

Index: kvm/drivers/kvm/vmx.c
===================================================================
--- kvm.orig/drivers/kvm/vmx.c	2007-08-01 10:37:40.000000000 +0200
+++ kvm/drivers/kvm/vmx.c	2007-08-01 10:40:04.000000000 +0200
@@ -1761,57 +1761,6 @@
 	return 0;
 }
 
-static int get_io_count(struct kvm_vcpu *vcpu, unsigned long *count)
-{
-	u64 inst;
-	gva_t rip;
-	int countr_size;
-	int i;
-
-	if ((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_VM)) {
-		countr_size = 2;
-	} else {
-		u32 cs_ar = vmcs_read32(GUEST_CS_AR_BYTES);
-
-		countr_size = (cs_ar & AR_L_MASK) ? 8:
-			      (cs_ar & AR_DB_MASK) ? 4: 2;
-	}
-
-	rip =  vmcs_readl(GUEST_RIP);
-	if (countr_size != 8)
-		rip += vmcs_readl(GUEST_CS_BASE);
-
-	if (emulator_read_std(rip, &inst, sizeof(inst), vcpu) !=
-							X86EMUL_CONTINUE)
-		return 0;
-
-	for (i = 0; i < sizeof(inst); i++) {
-		switch (((u8*)&inst)[i]) {
-		case 0xf0:
-		case 0xf2:
-		case 0xf3:
-		case 0x2e:
-		case 0x36:
-		case 0x3e:
-		case 0x26:
-		case 0x64:
-		case 0x65:
-		case 0x66:
-			break;
-		case 0x67:
-			countr_size = (countr_size == 2) ? 4: (countr_size >> 1);
-		default:
-			goto done;
-		}
-	}
-	return 0;
-done:
-	countr_size *= 8;
-	*count = vcpu->regs[VCPU_REGS_RCX] & (~0ULL >> (64 - countr_size));
-	//printk("cx: %lx\n", vcpu->regs[VCPU_REGS_RCX]);
-	return 1;
-}
-
 static int handle_io(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	u64 exit_qualification;
@@ -1831,8 +1780,32 @@
 	port = exit_qualification >> 16;
 	address = 0;
 	if (string) {
-		if (rep && !get_io_count(vcpu, &count))
+		int mode;
+		u64 inst;
+		gva_t rip;
+		struct x86_prefix prefix;
+		u32 cs_ar = vmcs_read32(GUEST_CS_AR_BYTES);
+		unsigned long addr_mask;
+
+		mode = (vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_VM) ?
+				X86EMUL_MODE_REAL : (cs_ar & AR_L_MASK)
+				? X86EMUL_MODE_PROT64 : (cs_ar & AR_DB_MASK)
+				? X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16;
+
+		rip = vmcs_readl(GUEST_RIP);
+		if (mode != X86EMUL_MODE_PROT64)
+			rip += vmcs_readl(GUEST_CS_BASE);
+		if (emulator_read_std(rip, &inst, sizeof(inst), vcpu)
+					!= X86EMUL_CONTINUE)
+			return 1;
+
+		if (x86_decode_prefix(mode, (u8*)&inst, &prefix) == -1)
 			return 1;
+
+		addr_mask = (~0ULL >> (64 - (prefix.ad_bytes <<3)));
+		if (rep)
+			count = vcpu->regs[VCPU_REGS_RCX] & addr_mask;
+
 		address = vmcs_readl(GUEST_LINEAR_ADDRESS);
 	}
 	return kvm_setup_pio(vcpu, kvm_run, in, size, count, string, down,

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

[PATCH 5/5] svm.c uses x86_decode_prefix() instead of io_address() and io_get_override().

[Laurent Vivier]

[-- Attachment #1.1.1: Type: text/plain, Size: 374 bytes --]

svm.c uses x86_decode_prefix() instead of io_address() and io_get_override().

*** WARNING: the SVM part has not been tested because I don't have any AMD
system. ***

Signed-off-by: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
          "Software is hard" - Donald Knuth

[-- Attachment #1.1.2: svm-decode_prefix --]
[-- Type: text/plain, Size: 6694 bytes --]

Index: kvm/drivers/kvm/svm.c
===================================================================
--- kvm.orig/drivers/kvm/svm.c	2007-08-01 10:10:24.000000000 +0200
+++ kvm/drivers/kvm/svm.c	2007-08-01 10:37:30.000000000 +0200
@@ -98,20 +98,6 @@
 	return svm_features & feat;
 }
 
-static unsigned get_addr_size(struct vcpu_svm *svm)
-{
-	struct vmcb_save_area *sa = &svm->vmcb->save;
-	u16 cs_attrib;
-
-	if (!(sa->cr0 & X86_CR0_PE) || (sa->rflags & X86_EFLAGS_VM))
-		return 2;
-
-	cs_attrib = sa->cs.attrib;
-
-	return (cs_attrib & SVM_SELECTOR_L_MASK) ? 8 :
-				(cs_attrib & SVM_SELECTOR_DB_MASK) ? 4 : 2;
-}
-
 static inline u8 pop_irq(struct kvm_vcpu *vcpu)
 {
 	int word_index = __ffs(vcpu->irq_summary);
@@ -996,112 +982,6 @@
 	return 0;
 }
 
-static int io_get_override(struct vcpu_svm *svm,
-			  struct vmcb_seg **seg,
-			  int *addr_override)
-{
-	u8 inst[MAX_INST_SIZE];
-	unsigned ins_length;
-	gva_t rip;
-	int i;
-
-	rip =  svm->vmcb->save.rip;
-	ins_length = svm->next_rip - rip;
-	rip += svm->vmcb->save.cs.base;
-
-	if (ins_length > MAX_INST_SIZE)
-		printk(KERN_DEBUG
-		       "%s: inst length err, cs base 0x%llx rip 0x%llx "
-		       "next rip 0x%llx ins_length %u\n",
-		       __FUNCTION__,
-		       svm->vmcb->save.cs.base,
-		       svm->vmcb->save.rip,
-		       svm->vmcb->control.exit_info_2,
-		       ins_length);
-
-	if (emulator_read_std(rip, inst, ins_length, &svm->vcpu)
-	    != X86EMUL_CONTINUE)
-		/* #PF */
-		return 0;
-
-	*addr_override = 0;
-	*seg = NULL;
-	for (i = 0; i < ins_length; i++)
-		switch (inst[i]) {
-		case 0xf0:
-		case 0xf2:
-		case 0xf3:
-		case 0x66:
-			continue;
-		case 0x67:
-			*addr_override = 1;
-			continue;
-		case 0x2e:
-			*seg = &svm->vmcb->save.cs;
-			continue;
-		case 0x36:
-			*seg = &svm->vmcb->save.ss;
-			continue;
-		case 0x3e:
-			*seg = &svm->vmcb->save.ds;
-			continue;
-		case 0x26:
-			*seg = &svm->vmcb->save.es;
-			continue;
-		case 0x64:
-			*seg = &svm->vmcb->save.fs;
-			continue;
-		case 0x65:
-			*seg = &svm->vmcb->save.gs;
-			continue;
-		default:
-			return 1;
-		}
-	printk(KERN_DEBUG "%s: unexpected\n", __FUNCTION__);
-	return 0;
-}
-
-static unsigned long io_address(struct vcpu_svm *svm, int ins, gva_t *address)
-{
-	unsigned long addr_mask;
-	unsigned long *reg;
-	struct vmcb_seg *seg;
-	int addr_override;
-	struct vmcb_save_area *save_area = &svm->vmcb->save;
-	u16 cs_attrib = save_area->cs.attrib;
-	unsigned addr_size = get_addr_size(svm);
-
-	if (!io_get_override(svm, &seg, &addr_override))
-		return 0;
-
-	if (addr_override)
-		addr_size = (addr_size == 2) ? 4: (addr_size >> 1);
-
-	if (ins) {
-		reg = &svm->vcpu.regs[VCPU_REGS_RDI];
-		seg = &svm->vmcb->save.es;
-	} else {
-		reg = &svm->vcpu.regs[VCPU_REGS_RSI];
-		seg = (seg) ? seg : &svm->vmcb->save.ds;
-	}
-
-	addr_mask = ~0ULL >> (64 - (addr_size * 8));
-
-	if ((cs_attrib & SVM_SELECTOR_L_MASK) &&
-	    !(svm->vmcb->save.rflags & X86_EFLAGS_VM)) {
-		*address = (*reg & addr_mask);
-		return addr_mask;
-	}
-
-	if (!(seg->attrib & SVM_SELECTOR_P_SHIFT)) {
-		svm_inject_gp(&svm->vcpu, 0);
-		return 0;
-	}
-
-	*address = (*reg & addr_mask) + seg->base;
-	return addr_mask;
-}
-
 static int io_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
 {
 	u32 io_info = svm->vmcb->control.exit_info_1; //address size bug?
@@ -1109,6 +989,7 @@
 	unsigned port;
 	unsigned long count;
 	gva_t address = 0;
+	struct vmcb_save_area *sa = &svm->vmcb->save;
 
 	++svm->vcpu.stat.io_exits;
 
@@ -1120,21 +1001,96 @@
 	string = (io_info & SVM_IOIO_STR_MASK) != 0;
 	rep = (io_info & SVM_IOIO_REP_MASK) != 0;
 	count = 1;
-	down = (svm->vmcb->save.rflags & X86_EFLAGS_DF) != 0;
+	down = (sa->rflags & X86_EFLAGS_DF) != 0;
 
 	if (string) {
-		unsigned addr_mask;
+		int mode;
+		u8 inst[MAX_INST_SIZE];
+		unsigned ins_length;
+		gva_t rip;
+		struct x86_prefix prefix;
+		u16 cs_ar = sa->cs.attrib;
+		unsigned long addr_mask;
+
+		mode = (!(sa->cr0 & X86_CR0_PE) ||
+			(sa->rflags & X86_EFLAGS_VM)) ?
+			X86EMUL_MODE_REAL :  (cs_ar & SVM_SELECTOR_L_MASK)
+			? X86EMUL_MODE_PROT64 : (cs_ar & SVM_SELECTOR_DB_MASK)
+			? X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16;
+
+		rip =  svm->vmcb->save.rip;
+		ins_length = svm->vmcb->save.rip - rip;
+		if (mode != X86EMUL_MODE_PROT64)
+			rip += svm->vmcb->save.cs.base;
+
+		if (ins_length > MAX_INST_SIZE)
+			printk(KERN_DEBUG
+			       "%s: inst length err, cs base 0x%llx rip 0x%llx "
+			       "next rip 0x%llx ins_length %u\n",
+			       __FUNCTION__,
+			       svm->vmcb->save.cs.base,
+			       svm->vmcb->save.rip,
+			       svm->vmcb->control.exit_info_2,
+			       ins_length);
 
-		addr_mask = io_address(svm, in, &address);
-		if (!addr_mask) {
-			printk(KERN_DEBUG "%s: get io address failed\n",
-			       __FUNCTION__);
+		if (emulator_read_std(rip, inst, ins_length, &svm->vcpu)
+					!= X86EMUL_CONTINUE)
+			return 1;
+
+		prefix.override_base = -1;
+		if (x86_decode_prefix(mode, (u8*)&inst, &prefix) == -1)
 			return 1;
-		}
 
+		addr_mask = (~0ULL >> (64 - (prefix.ad_bytes <<3)));
 		if (rep)
 			count = svm->vcpu.regs[VCPU_REGS_RCX] & addr_mask;
+
+		if (in)
+			address = svm->vcpu.regs[VCPU_REGS_RDI];
+		else
+			address = svm->vcpu.regs[VCPU_REGS_RSI];
+
+		address &= addr_mask;
+
+		if (mode != X86EMUL_MODE_PROT64) {
+			struct vmcb_seg *seg;
+
+			if (in)
+				seg = &svm->vmcb->save.es;
+			else switch(prefix.override_base) {
+				case -1:
+				case X86EMUL_BASE_DS:
+					seg = &svm->vmcb->save.ds;
+					break;
+				case X86EMUL_BASE_CS:
+					seg = &svm->vmcb->save.cs;
+					break;
+				case X86EMUL_BASE_ES:
+					seg = &svm->vmcb->save.es;
+					break;
+				case X86EMUL_BASE_GS:
+					seg = &svm->vmcb->save.gs;
+					break;
+				case X86EMUL_BASE_SS:
+					seg = &svm->vmcb->save.ss;
+					break;
+				default:
+					printk(KERN_DEBUG "%s: unexpected\n",
+						__FUNCTION__);
+					return 1;
+			}
+
+			if (seg->attrib & SVM_SELECTOR_P_SHIFT) {
+				svm_inject_gp(&svm->vcpu, 0);
+				printk(KERN_DEBUG "%s: get io address failed\n",
+					__FUNCTION__);
+				return 1;
+			}
+
+			address += seg->base;
+		}
 	}
+
 	return kvm_setup_pio(&svm->vcpu, kvm_run, in, size, count, string,
 			     down, address, rep, port);
 }
Index: kvm/drivers/kvm/.svm.c.swp
===================================================================
Binary files kvm.orig/drivers/kvm/.svm.c.swp	2007-08-01 10:12:34.000000000 +0200 and /dev/null	1970-01-01 00:00:00.000000000 +0000 differ

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH 4/5] vmx.c uses x86_decode_prefix() instead of get_io_count().

[Avi Kivity]

Laurent Vivier wrote:
> vmx.c uses x86_decode_prefix() instead of get_io_count().
>
>   
>  static int handle_io(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>  {
>  	u64 exit_qualification;
> @@ -1831,8 +1780,32 @@
>  	port = exit_qualification >> 16;
>  	address = 0;
>  	if (string) {
> -		if (rep && !get_io_count(vcpu, &count))
> +		int mode;
> +		u64 inst;
> +		gva_t rip;
> +		struct x86_prefix prefix;
> +		u32 cs_ar = vmcs_read32(GUEST_CS_AR_BYTES);
> +		unsigned long addr_mask;
> +
> +		mode = (vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_VM) ?
> +				X86EMUL_MODE_REAL : (cs_ar & AR_L_MASK)
> +				? X86EMUL_MODE_PROT64 : (cs_ar & AR_DB_MASK)
> +				? X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16;
> +
> +		rip = vmcs_readl(GUEST_RIP);
> +		if (mode != X86EMUL_MODE_PROT64)
> +			rip += vmcs_readl(GUEST_CS_BASE);
> +		if (emulator_read_std(rip, &inst, sizeof(inst), vcpu)
> +					!= X86EMUL_CONTINUE)
> +			return 1;
> +
> +		if (x86_decode_prefix(mode, (u8*)&inst, &prefix) == -1)
>  			return 1;
> +
> +		addr_mask = (~0ULL >> (64 - (prefix.ad_bytes <<3)));
> +		if (rep)
> +			count = vcpu->regs[VCPU_REGS_RCX] & addr_mask;
> +
>  		address = vmcs_readl(GUEST_LINEAR_ADDRESS);
>  	}
>  	return kvm_setup_pio(vcpu, kvm_run, in, size, count, string, down,

How about just calliing emulate_instruction() from here (just for the 
string case)?  That will eliminate all the setup code.

x86_emulate_memop() will need to be extended to decode ins/outs, but 
that's fairly easy.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 4/5] vmx.c uses x86_decode_prefix() instead of get_io_count().

[Laurent Vivier]

[-- Attachment #1.1: Type: text/plain, Size: 2844 bytes --]

Avi Kivity wrote:
> Laurent Vivier wrote:
>> vmx.c uses x86_decode_prefix() instead of get_io_count().
>>
>>    static int handle_io(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>  {
>>      u64 exit_qualification;
>> @@ -1831,8 +1780,32 @@
>>      port = exit_qualification >> 16;
>>      address = 0;
>>      if (string) {
>> -        if (rep && !get_io_count(vcpu, &count))
>> +        int mode;
>> +        u64 inst;
>> +        gva_t rip;
>> +        struct x86_prefix prefix;
>> +        u32 cs_ar = vmcs_read32(GUEST_CS_AR_BYTES);
>> +        unsigned long addr_mask;
>> +
>> +        mode = (vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_VM) ?
>> +                X86EMUL_MODE_REAL : (cs_ar & AR_L_MASK)
>> +                ? X86EMUL_MODE_PROT64 : (cs_ar & AR_DB_MASK)
>> +                ? X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16;
>> +
>> +        rip = vmcs_readl(GUEST_RIP);
>> +        if (mode != X86EMUL_MODE_PROT64)
>> +            rip += vmcs_readl(GUEST_CS_BASE);
>> +        if (emulator_read_std(rip, &inst, sizeof(inst), vcpu)
>> +                    != X86EMUL_CONTINUE)
>> +            return 1;
>> +
>> +        if (x86_decode_prefix(mode, (u8*)&inst, &prefix) == -1)
>>              return 1;
>> +
>> +        addr_mask = (~0ULL >> (64 - (prefix.ad_bytes <<3)));
>> +        if (rep)
>> +            count = vcpu->regs[VCPU_REGS_RCX] & addr_mask;
>> +
>>          address = vmcs_readl(GUEST_LINEAR_ADDRESS);
>>      }
>>      return kvm_setup_pio(vcpu, kvm_run, in, size, count, string, down,
> 
> How about just calliing emulate_instruction() from here (just for the
> string case)?  That will eliminate all the setup code.

But this setup is in emulate_instruction() so it will be executed anyway.

> x86_emulate_memop() will need to be extended to decode ins/outs, but
> that's fairly easy.

X86_decode_prefix() is a subset of instruction decoding part of
x86_emulate_memop(), kvm_setup_pio() can be seen as a subset of instruction
emulating part of x86_emulate_memop(). So I think in term of performance it is
better to do like that, but I agree by doing:

if (string)
	return emulate_instruction(vcpu, kvm_run, 0, 0);
else
	return kvm_setup_pio(vcpu, kvm_run, in, size, port);

it is more more ... more simple.

If you prefer simplicity, I can do like that ?
(but I know you prefer simplicity...)

BTW, I think PATCH 1,2 and 3 should be applied anyway because they allow to
introduce the separation between instruction decoding and instruction emulation
requested by the TODO "Split the emulator into two functions: one to decode into
the emulation context, and the other to actually execute the instruction."

Laurent
-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
          "Software is hard" - Donald Knuth


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: [PATCH 4/5] vmx.c uses x86_decode_prefix() instead of get_io_count().

[Avi Kivity]

Laurent Vivier wrote:
>>
>> How about just calliing emulate_instruction() from here (just for the
>> string case)?  That will eliminate all the setup code.
>>     
>
> But this setup is in emulate_instruction() so it will be executed anyway.
>
>   

I'm  not worried about run-time overhead, but about the amount of code.

>> x86_emulate_memop() will need to be extended to decode ins/outs, but
>> that's fairly easy.
>>     
>
> X86_decode_prefix() is a subset of instruction decoding part of
> x86_emulate_memop(), kvm_setup_pio() can be seen as a subset of instruction
> emulating part of x86_emulate_memop(). So I think in term of performance it is
> better to do like that, but I agree by doing:
>
> if (string)
> 	return emulate_instruction(vcpu, kvm_run, 0, 0);
> else
> 	return kvm_setup_pio(vcpu, kvm_run, in, size, port);
>
> it is more more ... more simple.
>
> If you prefer simplicity, I can do like that ?
> (but I know you prefer simplicity...)
>
>   

Yes.  Note that x86_emulate_memop() will eventually call kvm_setup_pio() 
to complete the emulation.

> BTW, I think PATCH 1,2 and 3 should be applied anyway because they allow to
> introduce the separation between instruction decoding and instruction emulation
> requested by the TODO "Split the emulator into two functions: one to decode into
> the emulation context, and the other to actually execute the instruction."
>   

I agree.  I'll look into it with more detail.


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH 4/5] vmx.c uses x86_decode_prefix() instead of get_io_count().

[Laurent Vivier]

[-- Attachment #1.1.1: Type: text/plain, Size: 1399 bytes --]

Avi Kivity wrote:
> Laurent Vivier wrote:
>>>
>>> How about just calliing emulate_instruction() from here (just for the
>>> string case)?  That will eliminate all the setup code.
>>>     
>>
>> But this setup is in emulate_instruction() so it will be executed anyway.
>>
>>   
> 
> I'm  not worried about run-time overhead, but about the amount of code.
> 
>>> x86_emulate_memop() will need to be extended to decode ins/outs, but
>>> that's fairly easy.
>>>     
>>
>> X86_decode_prefix() is a subset of instruction decoding part of
>> x86_emulate_memop(), kvm_setup_pio() can be seen as a subset of
>> instruction
>> emulating part of x86_emulate_memop(). So I think in term of
>> performance it is
>> better to do like that, but I agree by doing:
>>
>> if (string)
>>     return emulate_instruction(vcpu, kvm_run, 0, 0);
>> else
>>     return kvm_setup_pio(vcpu, kvm_run, in, size, port);
>>
>> it is more more ... more simple.
>>
>> If you prefer simplicity, I can do like that ?
>> (but I know you prefer simplicity...)
>>
>>   
> 
> Yes.  Note that x86_emulate_memop() will eventually call kvm_setup_pio()
> to complete the emulation.

Just to have some comments, have a look to my little dirty patch (see attachment)

Laurent
-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
          "Software is hard" - Donald Knuth

[-- Attachment #1.1.2: emulate_ins_outs --]
[-- Type: text/plain, Size: 10428 bytes --]

Index: kvm/drivers/kvm/kvm_main.c
===================================================================
--- kvm.orig/drivers/kvm/kvm_main.c	2007-08-02 18:32:19.000000000 +0200
+++ kvm/drivers/kvm/kvm_main.c	2007-08-02 18:32:22.000000000 +0200
@@ -1222,6 +1222,8 @@
 
 	vcpu->mmio_is_write = 0;
 	r = x86_emulate_memop(&emulate_ctxt, &emulate_ops);
+	if (vcpu->pio.count)
+		return EMULATE_DO_MMIO;
 
 	if ((r || vcpu->mmio_is_write) && run) {
 		run->exit_reason = KVM_EXIT_MMIO;
Index: kvm/drivers/kvm/svm.c
===================================================================
--- kvm.orig/drivers/kvm/svm.c	2007-08-02 18:32:19.000000000 +0200
+++ kvm/drivers/kvm/svm.c	2007-08-02 18:35:03.000000000 +0200
@@ -98,20 +98,6 @@
 	return svm_features & feat;
 }
 
-static unsigned get_addr_size(struct vcpu_svm *svm)
-{
-	struct vmcb_save_area *sa = &svm->vmcb->save;
-	u16 cs_attrib;
-
-	if (!(sa->cr0 & X86_CR0_PE) || (sa->rflags & X86_EFLAGS_VM))
-		return 2;
-
-	cs_attrib = sa->cs.attrib;
-
-	return (cs_attrib & SVM_SELECTOR_L_MASK) ? 8 :
-				(cs_attrib & SVM_SELECTOR_DB_MASK) ? 4 : 2;
-}
-
 static inline u8 pop_irq(struct kvm_vcpu *vcpu)
 {
 	int word_index = __ffs(vcpu->irq_summary);
@@ -995,147 +981,32 @@
 	return 0;
 }
 
-static int io_get_override(struct vcpu_svm *svm,
-			  struct vmcb_seg **seg,
-			  int *addr_override)
-{
-	u8 inst[MAX_INST_SIZE];
-	unsigned ins_length;
-	gva_t rip;
-	int i;
-
-	rip =  svm->vmcb->save.rip;
-	ins_length = svm->next_rip - rip;
-	rip += svm->vmcb->save.cs.base;
-
-	if (ins_length > MAX_INST_SIZE)
-		printk(KERN_DEBUG
-		       "%s: inst length err, cs base 0x%llx rip 0x%llx "
-		       "next rip 0x%llx ins_length %u\n",
-		       __FUNCTION__,
-		       svm->vmcb->save.cs.base,
-		       svm->vmcb->save.rip,
-		       svm->vmcb->control.exit_info_2,
-		       ins_length);
-
-	if (emulator_read_std(rip, inst, ins_length, &svm->vcpu)
-	    != X86EMUL_CONTINUE)
-		/* #PF */
-		return 0;
-
-	*addr_override = 0;
-	*seg = NULL;
-	for (i = 0; i < ins_length; i++)
-		switch (inst[i]) {
-		case 0xf0:
-		case 0xf2:
-		case 0xf3:
-		case 0x66:
-			continue;
-		case 0x67:
-			*addr_override = 1;
-			continue;
-		case 0x2e:
-			*seg = &svm->vmcb->save.cs;
-			continue;
-		case 0x36:
-			*seg = &svm->vmcb->save.ss;
-			continue;
-		case 0x3e:
-			*seg = &svm->vmcb->save.ds;
-			continue;
-		case 0x26:
-			*seg = &svm->vmcb->save.es;
-			continue;
-		case 0x64:
-			*seg = &svm->vmcb->save.fs;
-			continue;
-		case 0x65:
-			*seg = &svm->vmcb->save.gs;
-			continue;
-		default:
-			return 1;
-		}
-	printk(KERN_DEBUG "%s: unexpected\n", __FUNCTION__);
-	return 0;
-}
-
-static unsigned long io_address(struct vcpu_svm *svm, int ins, gva_t *address)
-{
-	unsigned long addr_mask;
-	unsigned long *reg;
-	struct vmcb_seg *seg;
-	int addr_override;
-	struct vmcb_save_area *save_area = &svm->vmcb->save;
-	u16 cs_attrib = save_area->cs.attrib;
-	unsigned addr_size = get_addr_size(svm);
-
-	if (!io_get_override(svm, &seg, &addr_override))
-		return 0;
-
-	if (addr_override)
-		addr_size = (addr_size == 2) ? 4: (addr_size >> 1);
-
-	if (ins) {
-		reg = &svm->vcpu.regs[VCPU_REGS_RDI];
-		seg = &svm->vmcb->save.es;
-	} else {
-		reg = &svm->vcpu.regs[VCPU_REGS_RSI];
-		seg = (seg) ? seg : &svm->vmcb->save.ds;
-	}
-
-	addr_mask = ~0ULL >> (64 - (addr_size * 8));
-
-	if ((cs_attrib & SVM_SELECTOR_L_MASK) &&
-	    !(svm->vmcb->save.rflags & X86_EFLAGS_VM)) {
-		*address = (*reg & addr_mask);
-		return addr_mask;
-	}
-
-	if (!(seg->attrib & SVM_SELECTOR_P_SHIFT)) {
-		svm_inject_gp(&svm->vcpu, 0);
-		return 0;
-	}
-
-	*address = (*reg & addr_mask) + seg->base;
-	return addr_mask;
-}
-
 static int io_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
 {
 	u32 io_info = svm->vmcb->control.exit_info_1; //address size bug?
 	int size, down, in, string, rep;
 	unsigned port;
-	unsigned long count;
-	gva_t address = 0;
 
 	++svm->vcpu.stat.io_exits;
 
 	svm->next_rip = svm->vmcb->control.exit_info_2;
 
+	string = (io_info & SVM_IOIO_STR_MASK) != 0;
+
+	if (string) {
+		if (emulate_instruction(&svm->vcpu, kvm_run, 0, 0) == EMULATE_DO_MMIO)
+			return 0;
+		return 1;
+	}
+
 	in = (io_info & SVM_IOIO_TYPE_MASK) != 0;
 	port = io_info >> 16;
 	size = (io_info & SVM_IOIO_SIZE_MASK) >> SVM_IOIO_SIZE_SHIFT;
-	string = (io_info & SVM_IOIO_STR_MASK) != 0;
 	rep = (io_info & SVM_IOIO_REP_MASK) != 0;
-	count = 1;
 	down = (svm->vmcb->save.rflags & X86_EFLAGS_DF) != 0;
 
-	if (string) {
-		unsigned addr_mask;
-
-		addr_mask = io_address(svm, in, &address);
-		if (!addr_mask) {
-			printk(KERN_DEBUG "%s: get io address failed\n",
-			       __FUNCTION__);
-			return 1;
-		}
-
-		if (rep)
-			count = svm->vcpu.regs[VCPU_REGS_RCX] & addr_mask;
-	}
-	return kvm_setup_pio(&svm->vcpu, kvm_run, in, size, count, string,
-			     down, address, rep, port);
+	return kvm_setup_pio(&svm->vcpu, kvm_run, in, size, 1, 0,
+			     down, 0, rep, port);
 }
 
 static int nop_on_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
Index: kvm/drivers/kvm/vmx.c
===================================================================
--- kvm.orig/drivers/kvm/vmx.c	2007-08-02 18:32:19.000000000 +0200
+++ kvm/drivers/kvm/vmx.c	2007-08-02 18:34:39.000000000 +0200
@@ -1761,82 +1761,30 @@
 	return 0;
 }
 
-static int get_io_count(struct kvm_vcpu *vcpu, unsigned long *count)
-{
-	u64 inst;
-	gva_t rip;
-	int countr_size;
-	int i;
-
-	if ((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_VM)) {
-		countr_size = 2;
-	} else {
-		u32 cs_ar = vmcs_read32(GUEST_CS_AR_BYTES);
-
-		countr_size = (cs_ar & AR_L_MASK) ? 8:
-			      (cs_ar & AR_DB_MASK) ? 4: 2;
-	}
-
-	rip =  vmcs_readl(GUEST_RIP);
-	if (countr_size != 8)
-		rip += vmcs_readl(GUEST_CS_BASE);
-
-	if (emulator_read_std(rip, &inst, sizeof(inst), vcpu) !=
-							X86EMUL_CONTINUE)
-		return 0;
-
-	for (i = 0; i < sizeof(inst); i++) {
-		switch (((u8*)&inst)[i]) {
-		case 0xf0:
-		case 0xf2:
-		case 0xf3:
-		case 0x2e:
-		case 0x36:
-		case 0x3e:
-		case 0x26:
-		case 0x64:
-		case 0x65:
-		case 0x66:
-			break;
-		case 0x67:
-			countr_size = (countr_size == 2) ? 4: (countr_size >> 1);
-		default:
-			goto done;
-		}
-	}
-	return 0;
-done:
-	countr_size *= 8;
-	*count = vcpu->regs[VCPU_REGS_RCX] & (~0ULL >> (64 - countr_size));
-	//printk("cx: %lx\n", vcpu->regs[VCPU_REGS_RCX]);
-	return 1;
-}
-
 static int handle_io(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	u64 exit_qualification;
 	int size, down, in, string, rep;
 	unsigned port;
-	unsigned long count;
-	gva_t address;
 
 	++vcpu->stat.io_exits;
 	exit_qualification = vmcs_read64(EXIT_QUALIFICATION);
-	in = (exit_qualification & 8) != 0;
-	size = (exit_qualification & 7) + 1;
 	string = (exit_qualification & 16) != 0;
+
+	if (string) {
+		if (emulate_instruction(vcpu, kvm_run, 0, 0) == EMULATE_DO_MMIO)
+			return 0;
+		return 1;
+	}
+
+	size = (exit_qualification & 7) + 1;
+	in = (exit_qualification & 8) != 0;
 	down = (vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_DF) != 0;
-	count = 1;
 	rep = (exit_qualification & 32) != 0;
 	port = exit_qualification >> 16;
-	address = 0;
-	if (string) {
-		if (rep && !get_io_count(vcpu, &count))
-			return 1;
-		address = vmcs_readl(GUEST_LINEAR_ADDRESS);
-	}
-	return kvm_setup_pio(vcpu, kvm_run, in, size, count, string, down,
-			     address, rep, port);
+
+	return kvm_setup_pio(vcpu, kvm_run, in, size, 1, 0, down,
+			     0, rep, port);
 }
 
 static void
Index: kvm/drivers/kvm/x86_emulate.c
===================================================================
--- kvm.orig/drivers/kvm/x86_emulate.c	2007-08-02 18:32:19.000000000 +0200
+++ kvm/drivers/kvm/x86_emulate.c	2007-08-02 18:34:06.000000000 +0200
@@ -103,9 +103,12 @@
 	/* 0x58 - 0x5F */
 	ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
 	ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
-	/* 0x60 - 0x6F */
+	/* 0x60 - 0x6B */
 	0, 0, 0, DstReg | SrcMem32 | ModRM | Mov /* movsxd (x86/64) */ ,
-	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	0, 0, 0, 0, 0, 0, 0, 0,
+	/* 0x6C - 0x6F */
+	ByteOp  | ImplicitOps, ImplicitOps,	/* insb, insw/insd */
+	ByteOp  | ImplicitOps, ImplicitOps,	/* outsb, outsw/outsd */
 	/* 0x70 - 0x7F */
 	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 	/* 0x80 - 0x87 */
@@ -428,10 +431,11 @@
 })
 
 /* Access/update address held in a register, based on addressing mode. */
+#define address_mask(reg)						\
+	((ad_bytes == sizeof(unsigned long)) ? 				\
+		(reg) :	((reg) & ((1UL << (ad_bytes << 3)) - 1)))
 #define register_address(base, reg)                                     \
-	((base) + ((ad_bytes == sizeof(unsigned long)) ? (reg) :	\
-		   ((reg) & ((1UL << (ad_bytes << 3)) - 1))))
-
+	((base) + address_mask(reg))
 #define register_address_increment(reg, inc)                            \
 	do {								\
 		/* signed type ensures sign extension to long */        \
@@ -1139,6 +1143,39 @@
 		register_address_increment(_regs[VCPU_REGS_RDI],
 			     (_eflags & EFLG_DF) ? -dst.bytes : dst.bytes);
 		break;
+	case 0x6c:		/* insb */
+	case 0x6d:		/* insw/insd */
+		 if (kvm_setup_pio(ctxt->vcpu, NULL,
+				1, 					/* in */
+				(d & ByteOp) ? 1 : op_bytes, 		/* size */
+				rep_prefix ?
+				address_mask(_regs[VCPU_REGS_RCX]) + 1: 1,	/* count */
+				1, 					/* strings */
+				(_eflags & EFLG_DF),			/* down */
+				register_address(ctxt->es_base,
+						 _regs[VCPU_REGS_RDI]),	/* address */
+				rep_prefix,
+				_regs[VCPU_REGS_RDX]			/* port */
+				) == 0)
+			return -1;
+		return 0;
+	case 0x6e:		/* outsb */
+	case 0x6f:		/* outsw/outsd */
+		if (kvm_setup_pio(ctxt->vcpu, NULL,
+				0, 					/* in */
+				(d & ByteOp) ? 1 : op_bytes, 		/* size */
+				rep_prefix ?
+				address_mask(_regs[VCPU_REGS_RCX]) + 1: 1,	/* count */
+				1, 					/* strings */
+				(_eflags & EFLG_DF),			/* down */
+				register_address(override_base ?
+						 *override_base : ctxt->ds_base,
+						 _regs[VCPU_REGS_RSI]),	/* address */
+				rep_prefix,
+				_regs[VCPU_REGS_RDX]			/* port */
+				) == 0)
+			return -1;
+		return 0;
 	case 0xa6 ... 0xa7:	/* cmps */
 		DPRINTF("Urk! I don't handle CMPS.\n");
 		goto cannot_emulate;

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel