* [PATCH 0/4] KVM: x86 emulator: Clean up decoder a bit
@ 2011-07-30 8:58 Takuya Yoshikawa
2011-07-30 9:00 ` [PATCH 1/4] KVM: x86 emulator: Use ctxt->_eip directly in do_insn_fetch_byte() Takuya Yoshikawa
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Takuya Yoshikawa @ 2011-07-30 8:58 UTC (permalink / raw)
To: avi, mtosatti; +Cc: kvm, yoshikawa.takuya
Passed emulator.flat test:
SUMMARY: 88 tests, 0 failures
Takuya
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/4] KVM: x86 emulator: Use ctxt->_eip directly in do_insn_fetch_byte()
2011-07-30 8:58 [PATCH 0/4] KVM: x86 emulator: Clean up decoder a bit Takuya Yoshikawa
@ 2011-07-30 9:00 ` Takuya Yoshikawa
2011-07-30 9:01 ` [PATCH 2/4] KVM: x86 emulator: Drop _size argument from insn_fetch() Takuya Yoshikawa
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Takuya Yoshikawa @ 2011-07-30 9:00 UTC (permalink / raw)
To: avi, mtosatti; +Cc: kvm, yoshikawa.takuya
From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Instead of passing ctxt->_eip from insn_fetch() call sites, get it from
ctxt in do_insn_fetch_byte(). This is done by replacing the argument
_eip of insn_fetch() with _ctxt, which should be better than letting the
macro use ctxt silently in its body.
Though this changes the place where ctxt->_eip is incremented from
insn_fetch() to do_insn_fetch_byte(), this does not have any real
effect.
Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
arch/x86/kvm/emulate.c | 79 ++++++++++++++++++++++++++----------------------
1 files changed, 43 insertions(+), 36 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 6f08bc9..fbb2e3a 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -651,18 +651,26 @@ static int segmented_read_std(struct x86_emulate_ctxt *ctxt,
return ctxt->ops->read_std(ctxt, linear, data, size, &ctxt->exception);
}
-static int do_insn_fetch_byte(struct x86_emulate_ctxt *ctxt,
- unsigned long eip, u8 *dest)
+/*
+ * Fetch the next byte of the instruction being emulated which is pointed to
+ * by ctxt->_eip, then increment ctxt->_eip.
+ *
+ * Also prefetch the remaining bytes of the instruction without crossing page
+ * boundary if they are not in fetch_cache yet.
+ */
+static int do_insn_fetch_byte(struct x86_emulate_ctxt *ctxt, u8 *dest)
{
struct fetch_cache *fc = &ctxt->fetch;
int rc;
int size, cur_size;
- if (eip == fc->end) {
+ if (ctxt->_eip == fc->end) {
unsigned long linear;
- struct segmented_address addr = { .seg=VCPU_SREG_CS, .ea=eip};
+ struct segmented_address addr = { .seg = VCPU_SREG_CS,
+ .ea = ctxt->_eip };
cur_size = fc->end - fc->start;
- size = min(15UL - cur_size, PAGE_SIZE - offset_in_page(eip));
+ size = min(15UL - cur_size,
+ PAGE_SIZE - offset_in_page(ctxt->_eip));
rc = __linearize(ctxt, addr, size, false, true, &linear);
if (rc != X86EMUL_CONTINUE)
return rc;
@@ -672,20 +680,21 @@ static int do_insn_fetch_byte(struct x86_emulate_ctxt *ctxt,
return rc;
fc->end += size;
}
- *dest = fc->data[eip - fc->start];
+ *dest = fc->data[ctxt->_eip - fc->start];
+ ctxt->_eip++;
return X86EMUL_CONTINUE;
}
static int do_insn_fetch(struct x86_emulate_ctxt *ctxt,
- unsigned long eip, void *dest, unsigned size)
+ void *dest, unsigned size)
{
int rc;
/* x86 instructions are limited to 15 bytes. */
- if (eip + size - ctxt->eip > 15)
+ if (ctxt->_eip + size - ctxt->eip > 15)
return X86EMUL_UNHANDLEABLE;
while (size--) {
- rc = do_insn_fetch_byte(ctxt, eip++, dest++);
+ rc = do_insn_fetch_byte(ctxt, dest++);
if (rc != X86EMUL_CONTINUE)
return rc;
}
@@ -693,20 +702,18 @@ static int do_insn_fetch(struct x86_emulate_ctxt *ctxt,
}
/* Fetch next part of the instruction being emulated. */
-#define insn_fetch(_type, _size, _eip) \
+#define insn_fetch(_type, _size, _ctxt) \
({ unsigned long _x; \
- rc = do_insn_fetch(ctxt, (_eip), &_x, (_size)); \
+ rc = do_insn_fetch(_ctxt, &_x, (_size)); \
if (rc != X86EMUL_CONTINUE) \
goto done; \
- (_eip) += (_size); \
(_type)_x; \
})
-#define insn_fetch_arr(_arr, _size, _eip) \
-({ rc = do_insn_fetch(ctxt, (_eip), _arr, (_size)); \
+#define insn_fetch_arr(_arr, _size, _ctxt) \
+({ rc = do_insn_fetch(_ctxt, _arr, (_size)); \
if (rc != X86EMUL_CONTINUE) \
goto done; \
- (_eip) += (_size); \
})
/*
@@ -894,7 +901,7 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
ctxt->modrm_rm = base_reg = (ctxt->rex_prefix & 1) << 3; /* REG.B */
}
- ctxt->modrm = insn_fetch(u8, 1, ctxt->_eip);
+ ctxt->modrm = insn_fetch(u8, 1, ctxt);
ctxt->modrm_mod |= (ctxt->modrm & 0xc0) >> 6;
ctxt->modrm_reg |= (ctxt->modrm & 0x38) >> 3;
ctxt->modrm_rm |= (ctxt->modrm & 0x07);
@@ -928,13 +935,13 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
switch (ctxt->modrm_mod) {
case 0:
if (ctxt->modrm_rm == 6)
- modrm_ea += insn_fetch(u16, 2, ctxt->_eip);
+ modrm_ea += insn_fetch(u16, 2, ctxt);
break;
case 1:
- modrm_ea += insn_fetch(s8, 1, ctxt->_eip);
+ modrm_ea += insn_fetch(s8, 1, ctxt);
break;
case 2:
- modrm_ea += insn_fetch(u16, 2, ctxt->_eip);
+ modrm_ea += insn_fetch(u16, 2, ctxt);
break;
}
switch (ctxt->modrm_rm) {
@@ -971,13 +978,13 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
} else {
/* 32/64-bit ModR/M decode. */
if ((ctxt->modrm_rm & 7) == 4) {
- sib = insn_fetch(u8, 1, ctxt->_eip);
+ sib = insn_fetch(u8, 1, ctxt);
index_reg |= (sib >> 3) & 7;
base_reg |= sib & 7;
scale = sib >> 6;
if ((base_reg & 7) == 5 && ctxt->modrm_mod == 0)
- modrm_ea += insn_fetch(s32, 4, ctxt->_eip);
+ modrm_ea += insn_fetch(s32, 4, ctxt);
else
modrm_ea += ctxt->regs[base_reg];
if (index_reg != 4)
@@ -990,13 +997,13 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
switch (ctxt->modrm_mod) {
case 0:
if (ctxt->modrm_rm == 5)
- modrm_ea += insn_fetch(s32, 4, ctxt->_eip);
+ modrm_ea += insn_fetch(s32, 4, ctxt);
break;
case 1:
- modrm_ea += insn_fetch(s8, 1, ctxt->_eip);
+ modrm_ea += insn_fetch(s8, 1, ctxt);
break;
case 2:
- modrm_ea += insn_fetch(s32, 4, ctxt->_eip);
+ modrm_ea += insn_fetch(s32, 4, ctxt);
break;
}
}
@@ -1013,13 +1020,13 @@ static int decode_abs(struct x86_emulate_ctxt *ctxt,
op->type = OP_MEM;
switch (ctxt->ad_bytes) {
case 2:
- op->addr.mem.ea = insn_fetch(u16, 2, ctxt->_eip);
+ op->addr.mem.ea = insn_fetch(u16, 2, ctxt);
break;
case 4:
- op->addr.mem.ea = insn_fetch(u32, 4, ctxt->_eip);
+ op->addr.mem.ea = insn_fetch(u32, 4, ctxt);
break;
case 8:
- op->addr.mem.ea = insn_fetch(u64, 8, ctxt->_eip);
+ op->addr.mem.ea = insn_fetch(u64, 8, ctxt);
break;
}
done:
@@ -3309,13 +3316,13 @@ static int decode_imm(struct x86_emulate_ctxt *ctxt, struct operand *op,
/* NB. Immediates are sign-extended as necessary. */
switch (op->bytes) {
case 1:
- op->val = insn_fetch(s8, 1, ctxt->_eip);
+ op->val = insn_fetch(s8, 1, ctxt);
break;
case 2:
- op->val = insn_fetch(s16, 2, ctxt->_eip);
+ op->val = insn_fetch(s16, 2, ctxt);
break;
case 4:
- op->val = insn_fetch(s32, 4, ctxt->_eip);
+ op->val = insn_fetch(s32, 4, ctxt);
break;
}
if (!sign_extension) {
@@ -3374,7 +3381,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
/* Legacy prefixes. */
for (;;) {
- switch (ctxt->b = insn_fetch(u8, 1, ctxt->_eip)) {
+ switch (ctxt->b = insn_fetch(u8, 1, ctxt)) {
case 0x66: /* operand-size override */
op_prefix = true;
/* switch between 2/4 bytes */
@@ -3430,7 +3437,7 @@ done_prefixes:
/* Two-byte opcode? */
if (ctxt->b == 0x0f) {
ctxt->twobyte = 1;
- ctxt->b = insn_fetch(u8, 1, ctxt->_eip);
+ ctxt->b = insn_fetch(u8, 1, ctxt);
opcode = twobyte_table[ctxt->b];
}
ctxt->d = opcode.flags;
@@ -3438,13 +3445,13 @@ done_prefixes:
while (ctxt->d & GroupMask) {
switch (ctxt->d & GroupMask) {
case Group:
- ctxt->modrm = insn_fetch(u8, 1, ctxt->_eip);
+ ctxt->modrm = insn_fetch(u8, 1, ctxt);
--ctxt->_eip;
goffset = (ctxt->modrm >> 3) & 7;
opcode = opcode.u.group[goffset];
break;
case GroupDual:
- ctxt->modrm = insn_fetch(u8, 1, ctxt->_eip);
+ ctxt->modrm = insn_fetch(u8, 1, ctxt);
--ctxt->_eip;
goffset = (ctxt->modrm >> 3) & 7;
if ((ctxt->modrm >> 6) == 3)
@@ -3577,7 +3584,7 @@ done_prefixes:
ctxt->src.type = OP_IMM;
ctxt->src.addr.mem.ea = ctxt->_eip;
ctxt->src.bytes = ctxt->op_bytes + 2;
- insn_fetch_arr(ctxt->src.valptr, ctxt->src.bytes, ctxt->_eip);
+ insn_fetch_arr(ctxt->src.valptr, ctxt->src.bytes, ctxt);
break;
case SrcMemFAddr:
memop.bytes = ctxt->op_bytes + 2;
@@ -3630,7 +3637,7 @@ done_prefixes:
ctxt->dst.type = OP_IMM;
ctxt->dst.addr.mem.ea = ctxt->_eip;
ctxt->dst.bytes = 1;
- ctxt->dst.val = insn_fetch(u8, 1, ctxt->_eip);
+ ctxt->dst.val = insn_fetch(u8, 1, ctxt);
break;
case DstMem:
case DstMem64:
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4] KVM: x86 emulator: Drop _size argument from insn_fetch()
2011-07-30 8:58 [PATCH 0/4] KVM: x86 emulator: Clean up decoder a bit Takuya Yoshikawa
2011-07-30 9:00 ` [PATCH 1/4] KVM: x86 emulator: Use ctxt->_eip directly in do_insn_fetch_byte() Takuya Yoshikawa
@ 2011-07-30 9:01 ` Takuya Yoshikawa
2011-07-30 9:02 ` [PATCH 3/4] KVM: x86 emulator: Let compiler know insn_fetch() rarely fails Takuya Yoshikawa
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Takuya Yoshikawa @ 2011-07-30 9:01 UTC (permalink / raw)
To: avi, mtosatti; +Cc: kvm, yoshikawa.takuya
From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
_type is enough to know the size.
Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
arch/x86/kvm/emulate.c | 44 ++++++++++++++++++++++----------------------
1 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index fbb2e3a..54f26d9 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -702,9 +702,9 @@ static int do_insn_fetch(struct x86_emulate_ctxt *ctxt,
}
/* Fetch next part of the instruction being emulated. */
-#define insn_fetch(_type, _size, _ctxt) \
+#define insn_fetch(_type, _ctxt) \
({ unsigned long _x; \
- rc = do_insn_fetch(_ctxt, &_x, (_size)); \
+ rc = do_insn_fetch(_ctxt, &_x, sizeof(_type)); \
if (rc != X86EMUL_CONTINUE) \
goto done; \
(_type)_x; \
@@ -901,7 +901,7 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
ctxt->modrm_rm = base_reg = (ctxt->rex_prefix & 1) << 3; /* REG.B */
}
- ctxt->modrm = insn_fetch(u8, 1, ctxt);
+ ctxt->modrm = insn_fetch(u8, ctxt);
ctxt->modrm_mod |= (ctxt->modrm & 0xc0) >> 6;
ctxt->modrm_reg |= (ctxt->modrm & 0x38) >> 3;
ctxt->modrm_rm |= (ctxt->modrm & 0x07);
@@ -935,13 +935,13 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
switch (ctxt->modrm_mod) {
case 0:
if (ctxt->modrm_rm == 6)
- modrm_ea += insn_fetch(u16, 2, ctxt);
+ modrm_ea += insn_fetch(u16, ctxt);
break;
case 1:
- modrm_ea += insn_fetch(s8, 1, ctxt);
+ modrm_ea += insn_fetch(s8, ctxt);
break;
case 2:
- modrm_ea += insn_fetch(u16, 2, ctxt);
+ modrm_ea += insn_fetch(u16, ctxt);
break;
}
switch (ctxt->modrm_rm) {
@@ -978,13 +978,13 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
} else {
/* 32/64-bit ModR/M decode. */
if ((ctxt->modrm_rm & 7) == 4) {
- sib = insn_fetch(u8, 1, ctxt);
+ sib = insn_fetch(u8, ctxt);
index_reg |= (sib >> 3) & 7;
base_reg |= sib & 7;
scale = sib >> 6;
if ((base_reg & 7) == 5 && ctxt->modrm_mod == 0)
- modrm_ea += insn_fetch(s32, 4, ctxt);
+ modrm_ea += insn_fetch(s32, ctxt);
else
modrm_ea += ctxt->regs[base_reg];
if (index_reg != 4)
@@ -997,13 +997,13 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
switch (ctxt->modrm_mod) {
case 0:
if (ctxt->modrm_rm == 5)
- modrm_ea += insn_fetch(s32, 4, ctxt);
+ modrm_ea += insn_fetch(s32, ctxt);
break;
case 1:
- modrm_ea += insn_fetch(s8, 1, ctxt);
+ modrm_ea += insn_fetch(s8, ctxt);
break;
case 2:
- modrm_ea += insn_fetch(s32, 4, ctxt);
+ modrm_ea += insn_fetch(s32, ctxt);
break;
}
}
@@ -1020,13 +1020,13 @@ static int decode_abs(struct x86_emulate_ctxt *ctxt,
op->type = OP_MEM;
switch (ctxt->ad_bytes) {
case 2:
- op->addr.mem.ea = insn_fetch(u16, 2, ctxt);
+ op->addr.mem.ea = insn_fetch(u16, ctxt);
break;
case 4:
- op->addr.mem.ea = insn_fetch(u32, 4, ctxt);
+ op->addr.mem.ea = insn_fetch(u32, ctxt);
break;
case 8:
- op->addr.mem.ea = insn_fetch(u64, 8, ctxt);
+ op->addr.mem.ea = insn_fetch(u64, ctxt);
break;
}
done:
@@ -3316,13 +3316,13 @@ static int decode_imm(struct x86_emulate_ctxt *ctxt, struct operand *op,
/* NB. Immediates are sign-extended as necessary. */
switch (op->bytes) {
case 1:
- op->val = insn_fetch(s8, 1, ctxt);
+ op->val = insn_fetch(s8, ctxt);
break;
case 2:
- op->val = insn_fetch(s16, 2, ctxt);
+ op->val = insn_fetch(s16, ctxt);
break;
case 4:
- op->val = insn_fetch(s32, 4, ctxt);
+ op->val = insn_fetch(s32, ctxt);
break;
}
if (!sign_extension) {
@@ -3381,7 +3381,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
/* Legacy prefixes. */
for (;;) {
- switch (ctxt->b = insn_fetch(u8, 1, ctxt)) {
+ switch (ctxt->b = insn_fetch(u8, ctxt)) {
case 0x66: /* operand-size override */
op_prefix = true;
/* switch between 2/4 bytes */
@@ -3437,7 +3437,7 @@ done_prefixes:
/* Two-byte opcode? */
if (ctxt->b == 0x0f) {
ctxt->twobyte = 1;
- ctxt->b = insn_fetch(u8, 1, ctxt);
+ ctxt->b = insn_fetch(u8, ctxt);
opcode = twobyte_table[ctxt->b];
}
ctxt->d = opcode.flags;
@@ -3445,13 +3445,13 @@ done_prefixes:
while (ctxt->d & GroupMask) {
switch (ctxt->d & GroupMask) {
case Group:
- ctxt->modrm = insn_fetch(u8, 1, ctxt);
+ ctxt->modrm = insn_fetch(u8, ctxt);
--ctxt->_eip;
goffset = (ctxt->modrm >> 3) & 7;
opcode = opcode.u.group[goffset];
break;
case GroupDual:
- ctxt->modrm = insn_fetch(u8, 1, ctxt);
+ ctxt->modrm = insn_fetch(u8, ctxt);
--ctxt->_eip;
goffset = (ctxt->modrm >> 3) & 7;
if ((ctxt->modrm >> 6) == 3)
@@ -3637,7 +3637,7 @@ done_prefixes:
ctxt->dst.type = OP_IMM;
ctxt->dst.addr.mem.ea = ctxt->_eip;
ctxt->dst.bytes = 1;
- ctxt->dst.val = insn_fetch(u8, 1, ctxt);
+ ctxt->dst.val = insn_fetch(u8, ctxt);
break;
case DstMem:
case DstMem64:
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] KVM: x86 emulator: Let compiler know insn_fetch() rarely fails
2011-07-30 8:58 [PATCH 0/4] KVM: x86 emulator: Clean up decoder a bit Takuya Yoshikawa
2011-07-30 9:00 ` [PATCH 1/4] KVM: x86 emulator: Use ctxt->_eip directly in do_insn_fetch_byte() Takuya Yoshikawa
2011-07-30 9:01 ` [PATCH 2/4] KVM: x86 emulator: Drop _size argument from insn_fetch() Takuya Yoshikawa
@ 2011-07-30 9:02 ` Takuya Yoshikawa
2011-07-30 9:03 ` [PATCH 4/4] KVM: x86 emulator: Make x86_decode_insn() return proper macros Takuya Yoshikawa
2011-07-31 8:46 ` [PATCH 0/4] KVM: x86 emulator: Clean up decoder a bit Avi Kivity
4 siblings, 0 replies; 8+ messages in thread
From: Takuya Yoshikawa @ 2011-07-30 9:02 UTC (permalink / raw)
To: avi, mtosatti; +Cc: kvm, yoshikawa.takuya
From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Fetching the instruction which was to be executed by the guest cannot
fail normally. So compiler should always predict that it will succeed.
Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
arch/x86/kvm/emulate.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 54f26d9..ae8d23c 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -672,11 +672,11 @@ static int do_insn_fetch_byte(struct x86_emulate_ctxt *ctxt, u8 *dest)
size = min(15UL - cur_size,
PAGE_SIZE - offset_in_page(ctxt->_eip));
rc = __linearize(ctxt, addr, size, false, true, &linear);
- if (rc != X86EMUL_CONTINUE)
+ if (unlikely(rc != X86EMUL_CONTINUE))
return rc;
rc = ctxt->ops->fetch(ctxt, linear, fc->data + cur_size,
size, &ctxt->exception);
- if (rc != X86EMUL_CONTINUE)
+ if (unlikely(rc != X86EMUL_CONTINUE))
return rc;
fc->end += size;
}
@@ -691,7 +691,7 @@ static int do_insn_fetch(struct x86_emulate_ctxt *ctxt,
int rc;
/* x86 instructions are limited to 15 bytes. */
- if (ctxt->_eip + size - ctxt->eip > 15)
+ if (unlikely(ctxt->_eip + size - ctxt->eip > 15))
return X86EMUL_UNHANDLEABLE;
while (size--) {
rc = do_insn_fetch_byte(ctxt, dest++);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] KVM: x86 emulator: Make x86_decode_insn() return proper macros
2011-07-30 8:58 [PATCH 0/4] KVM: x86 emulator: Clean up decoder a bit Takuya Yoshikawa
` (2 preceding siblings ...)
2011-07-30 9:02 ` [PATCH 3/4] KVM: x86 emulator: Let compiler know insn_fetch() rarely fails Takuya Yoshikawa
@ 2011-07-30 9:03 ` Takuya Yoshikawa
2011-07-31 8:48 ` Avi Kivity
2011-07-31 8:46 ` [PATCH 0/4] KVM: x86 emulator: Clean up decoder a bit Avi Kivity
4 siblings, 1 reply; 8+ messages in thread
From: Takuya Yoshikawa @ 2011-07-30 9:03 UTC (permalink / raw)
To: avi, mtosatti; +Cc: kvm, yoshikawa.takuya
From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Return EMULATION_OK/FAILED consistently. Also treat instruction fetch
errors, not restricted to X86EMUL_UNHANDLEABLE, as EMULATION_FAILED;
although this cannot happen in practice, the current logic will continue
the emulation even if the decoder fails to fetch the instruction.
Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
arch/x86/kvm/emulate.c | 12 ++++++------
arch/x86/kvm/x86.c | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index ae8d23c..0453c07 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3373,7 +3373,7 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len)
break;
#endif
default:
- return -1;
+ return EMULATION_FAILED;
}
ctxt->op_bytes = def_op_bytes;
@@ -3465,7 +3465,7 @@ done_prefixes:
break;
case Prefix:
if (ctxt->rep_prefix && op_prefix)
- return X86EMUL_UNHANDLEABLE;
+ return EMULATION_FAILED;
simd_prefix = op_prefix ? 0x66 : ctxt->rep_prefix;
switch (simd_prefix) {
case 0x00: opcode = opcode.u.gprefix->pfx_no; break;
@@ -3475,7 +3475,7 @@ done_prefixes:
}
break;
default:
- return X86EMUL_UNHANDLEABLE;
+ return EMULATION_FAILED;
}
ctxt->d &= ~GroupMask;
@@ -3488,10 +3488,10 @@ done_prefixes:
/* Unrecognised? */
if (ctxt->d == 0 || (ctxt->d & Undefined))
- return -1;
+ return EMULATION_FAILED;
if (!(ctxt->d & VendorSpecific) && ctxt->only_vendor_specific_insn)
- return -1;
+ return EMULATION_FAILED;
if (mode == X86EMUL_MODE_PROT64 && (ctxt->d & Stack))
ctxt->op_bytes = 8;
@@ -3683,7 +3683,7 @@ done:
if (memopp && memopp->type == OP_MEM && ctxt->rip_relative)
memopp->addr.mem.ea += ctxt->_eip;
- return (rc == X86EMUL_UNHANDLEABLE) ? EMULATION_FAILED : EMULATION_OK;
+ return (rc != X86EMUL_CONTINUE) ? EMULATION_FAILED : EMULATION_OK;
}
static bool string_insn_completed(struct x86_emulate_ctxt *ctxt)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6cb353c..baa427a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4833,7 +4833,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
trace_kvm_emulate_insn_start(vcpu);
++vcpu->stat.insn_emulation;
- if (r) {
+ if (r != EMULATION_OK) {
if (emulation_type & EMULTYPE_TRAP_UD)
return EMULATE_FAIL;
if (reexecute_instruction(vcpu, cr2))
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/4] KVM: x86 emulator: Clean up decoder a bit
2011-07-30 8:58 [PATCH 0/4] KVM: x86 emulator: Clean up decoder a bit Takuya Yoshikawa
` (3 preceding siblings ...)
2011-07-30 9:03 ` [PATCH 4/4] KVM: x86 emulator: Make x86_decode_insn() return proper macros Takuya Yoshikawa
@ 2011-07-31 8:46 ` Avi Kivity
4 siblings, 0 replies; 8+ messages in thread
From: Avi Kivity @ 2011-07-31 8:46 UTC (permalink / raw)
To: Takuya Yoshikawa; +Cc: mtosatti, kvm, yoshikawa.takuya
On 07/30/2011 11:58 AM, Takuya Yoshikawa wrote:
> Passed emulator.flat test:
> SUMMARY: 88 tests, 0 failures
>
Applied, thanks.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/4] KVM: x86 emulator: Make x86_decode_insn() return proper macros
2011-07-30 9:03 ` [PATCH 4/4] KVM: x86 emulator: Make x86_decode_insn() return proper macros Takuya Yoshikawa
@ 2011-07-31 8:48 ` Avi Kivity
2011-07-31 13:48 ` Takuya Yoshikawa
0 siblings, 1 reply; 8+ messages in thread
From: Avi Kivity @ 2011-07-31 8:48 UTC (permalink / raw)
To: Takuya Yoshikawa; +Cc: mtosatti, kvm, yoshikawa.takuya
On 07/30/2011 12:03 PM, Takuya Yoshikawa wrote:
> From: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
>
> Return EMULATION_OK/FAILED consistently. Also treat instruction fetch
> errors, not restricted to X86EMUL_UNHANDLEABLE, as EMULATION_FAILED;
> although this cannot happen in practice, the current logic will continue
> the emulation even if the decoder fails to fetch the instruction.
In fact it can happen in practice, but not through normal usage. For
example, one vcpu can execute an instruction which traps into the
emulator, while another vcpu changes the page tables to make the
instruction unfetchable.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/4] KVM: x86 emulator: Make x86_decode_insn() return proper macros
2011-07-31 8:48 ` Avi Kivity
@ 2011-07-31 13:48 ` Takuya Yoshikawa
0 siblings, 0 replies; 8+ messages in thread
From: Takuya Yoshikawa @ 2011-07-31 13:48 UTC (permalink / raw)
To: Avi Kivity; +Cc: mtosatti, kvm, yoshikawa.takuya
On Sun, 31 Jul 2011 11:48:40 +0300
Avi Kivity <avi@redhat.com> wrote:
> On 07/30/2011 12:03 PM, Takuya Yoshikawa wrote:
> > From: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
> >
> > Return EMULATION_OK/FAILED consistently. Also treat instruction fetch
> > errors, not restricted to X86EMUL_UNHANDLEABLE, as EMULATION_FAILED;
> > although this cannot happen in practice, the current logic will continue
> > the emulation even if the decoder fails to fetch the instruction.
>
> In fact it can happen in practice, but not through normal usage. For
> example, one vcpu can execute an instruction which traps into the
> emulator, while another vcpu changes the page tables to make the
> instruction unfetchable.
>
Really virtualization specific!
I need to think about multiple vcpus more.
Thanks,
Takuya
--
Takuya Yoshikawa <takuya.yoshikawa@gmail.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-07-31 13:49 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-30 8:58 [PATCH 0/4] KVM: x86 emulator: Clean up decoder a bit Takuya Yoshikawa
2011-07-30 9:00 ` [PATCH 1/4] KVM: x86 emulator: Use ctxt->_eip directly in do_insn_fetch_byte() Takuya Yoshikawa
2011-07-30 9:01 ` [PATCH 2/4] KVM: x86 emulator: Drop _size argument from insn_fetch() Takuya Yoshikawa
2011-07-30 9:02 ` [PATCH 3/4] KVM: x86 emulator: Let compiler know insn_fetch() rarely fails Takuya Yoshikawa
2011-07-30 9:03 ` [PATCH 4/4] KVM: x86 emulator: Make x86_decode_insn() return proper macros Takuya Yoshikawa
2011-07-31 8:48 ` Avi Kivity
2011-07-31 13:48 ` Takuya Yoshikawa
2011-07-31 8:46 ` [PATCH 0/4] KVM: x86 emulator: Clean up decoder a bit Avi Kivity
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox