* [PATCH 1/4] KVM: Initialize operand and address sizes before emulating interrupts
2010-08-17 8:26 [PATCH 0/4] Emulator INTn and SCAS fixes Avi Kivity
@ 2010-08-17 8:26 ` Avi Kivity
2010-08-17 8:26 ` [PATCH 2/4] KVM: x86 emulator: fix INTn emulation not pushing EFLAGS and CS Avi Kivity
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Avi Kivity @ 2010-08-17 8:26 UTC (permalink / raw)
To: kvm, Marcelo Tosatti, Mohammed Gamal
The emulator needs the operand and address sizes to be valid.
Signed-off-by: Avi Kivity <avi@redhat.com>
---
arch/x86/kvm/x86.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6a77fa1..f6a31a1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3965,6 +3965,8 @@ int kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq)
init_emulate_ctxt(vcpu);
+ vcpu->arch.emulate_ctxt.decode.op_bytes = 2;
+ vcpu->arch.emulate_ctxt.decode.ad_bytes = 2;
ret = emulate_int_real(&vcpu->arch.emulate_ctxt, &emulate_ops, irq);
if (ret != X86EMUL_CONTINUE)
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/4] KVM: x86 emulator: fix INTn emulation not pushing EFLAGS and CS
2010-08-17 8:26 [PATCH 0/4] Emulator INTn and SCAS fixes Avi Kivity
2010-08-17 8:26 ` [PATCH 1/4] KVM: Initialize operand and address sizes before emulating interrupts Avi Kivity
@ 2010-08-17 8:26 ` Avi Kivity
2010-08-17 8:26 ` [PATCH 3/4] KVM: x86 emulator: implement SCAS (opcodes AE, AF) Avi Kivity
2010-08-17 8:26 ` [PATCH 4/4] KVM: x86 emulator: fix REPZ/REPNZ termination condition Avi Kivity
3 siblings, 0 replies; 9+ messages in thread
From: Avi Kivity @ 2010-08-17 8:26 UTC (permalink / raw)
To: kvm, Marcelo Tosatti, Mohammed Gamal
emulate_push() only schedules a push; it doesn't actually push anything.
Call writeback() to flush out the write.
Signed-off-by: Avi Kivity <avi@redhat.com>
---
arch/x86/kvm/emulate.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index f1ec023..0e8f25e 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1228,7 +1228,7 @@ int emulate_int_real(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops, int irq)
{
struct decode_cache *c = &ctxt->decode;
- int rc = X86EMUL_CONTINUE;
+ int rc;
struct desc_ptr dt;
gva_t cs_addr;
gva_t eip_addr;
@@ -1238,14 +1238,25 @@ int emulate_int_real(struct x86_emulate_ctxt *ctxt,
/* TODO: Add limit checks */
c->src.val = ctxt->eflags;
emulate_push(ctxt, ops);
+ rc = writeback(ctxt, ops);
+ if (rc != X86EMUL_CONTINUE)
+ return rc;
ctxt->eflags &= ~(EFLG_IF | EFLG_TF | EFLG_AC);
c->src.val = ops->get_segment_selector(VCPU_SREG_CS, ctxt->vcpu);
emulate_push(ctxt, ops);
+ rc = writeback(ctxt, ops);
+ if (rc != X86EMUL_CONTINUE)
+ return rc;
c->src.val = c->eip;
emulate_push(ctxt, ops);
+ rc = writeback(ctxt, ops);
+ if (rc != X86EMUL_CONTINUE)
+ return rc;
+
+ c->dst.type = OP_NONE;
ops->get_idt(&dt, ctxt->vcpu);
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/4] KVM: x86 emulator: implement SCAS (opcodes AE, AF)
2010-08-17 8:26 [PATCH 0/4] Emulator INTn and SCAS fixes Avi Kivity
2010-08-17 8:26 ` [PATCH 1/4] KVM: Initialize operand and address sizes before emulating interrupts Avi Kivity
2010-08-17 8:26 ` [PATCH 2/4] KVM: x86 emulator: fix INTn emulation not pushing EFLAGS and CS Avi Kivity
@ 2010-08-17 8:26 ` Avi Kivity
2010-08-17 8:26 ` [PATCH 4/4] KVM: x86 emulator: fix REPZ/REPNZ termination condition Avi Kivity
3 siblings, 0 replies; 9+ messages in thread
From: Avi Kivity @ 2010-08-17 8:26 UTC (permalink / raw)
To: kvm, Marcelo Tosatti, Mohammed Gamal
Signed-off-by: Avi Kivity <avi@redhat.com>
---
arch/x86/kvm/emulate.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 0e8f25e..0c0ada9 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2308,7 +2308,7 @@ static struct opcode opcode_table[256] = {
D(DstAcc | SrcImmByte | ByteOp), D(DstAcc | SrcImm),
D(ByteOp | SrcAcc | DstDI | Mov | String), D(SrcAcc | DstDI | Mov | String),
D(ByteOp | SrcSI | DstAcc | Mov | String), D(SrcSI | DstAcc | Mov | String),
- D(ByteOp | DstDI | String), D(DstDI | String),
+ D(ByteOp | SrcAcc | DstDI | String), D(SrcAcc | DstDI | String),
/* 0xB0 - 0xB7 */
X8(D(ByteOp | DstReg | SrcImm | Mov)),
/* 0xB8 - 0xBF */
@@ -3068,8 +3068,7 @@ special_insn:
case 0xac ... 0xad: /* lods */
goto mov;
case 0xae ... 0xaf: /* scas */
- DPRINTF("Urk! I don't handle SCAS.\n");
- goto cannot_emulate;
+ goto cmp;
case 0xb0 ... 0xbf: /* mov r, imm */
goto mov;
case 0xc0 ... 0xc1:
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/4] KVM: x86 emulator: fix REPZ/REPNZ termination condition
2010-08-17 8:26 [PATCH 0/4] Emulator INTn and SCAS fixes Avi Kivity
` (2 preceding siblings ...)
2010-08-17 8:26 ` [PATCH 3/4] KVM: x86 emulator: implement SCAS (opcodes AE, AF) Avi Kivity
@ 2010-08-17 8:26 ` Avi Kivity
2010-08-17 9:13 ` Gleb Natapov
3 siblings, 1 reply; 9+ messages in thread
From: Avi Kivity @ 2010-08-17 8:26 UTC (permalink / raw)
To: kvm, Marcelo Tosatti, Mohammed Gamal
EFLAGS.ZF needs to be checked after each iteration, not before.
Signed-off-by: Avi Kivity <avi@redhat.com>
---
arch/x86/kvm/emulate.c | 42 +++++++++++++++++++++++-------------------
1 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 0c0ada9..a2edfb1 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2747,6 +2747,7 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
int rc = X86EMUL_CONTINUE;
int saved_dst_type = c->dst.type;
int irq; /* Used for int 3, int, and into */
+ ulong old_eip;
ctxt->decode.mem_read.pos = 0;
@@ -2771,28 +2772,10 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
ctxt->restart = true;
/* All REP prefixes have the same first termination condition */
if (address_mask(c, c->regs[VCPU_REGS_RCX]) == 0) {
- string_done:
ctxt->restart = false;
ctxt->eip = c->eip;
goto done;
}
- /* The second termination condition only applies for REPE
- * and REPNE. Test if the repeat string operation prefix is
- * REPE/REPZ or REPNE/REPNZ and if it's the case it tests the
- * corresponding termination condition according to:
- * - if REPE/REPZ and ZF = 0 then done
- * - if REPNE/REPNZ and ZF = 1 then done
- */
- if ((c->b == 0xa6) || (c->b == 0xa7) ||
- (c->b == 0xae) || (c->b == 0xaf)) {
- if ((c->rep_prefix == REPE_PREFIX) &&
- ((ctxt->eflags & EFLG_ZF) == 0))
- goto string_done;
- if ((c->rep_prefix == REPNE_PREFIX) &&
- ((ctxt->eflags & EFLG_ZF) == EFLG_ZF))
- goto string_done;
- }
- c->eip = ctxt->eip;
}
if (c->src.type == OP_MEM) {
@@ -3229,6 +3212,7 @@ special_insn:
}
writeback:
+ old_eip = c->eip;
rc = writeback(ctxt, ops);
if (rc != X86EMUL_CONTINUE)
goto done;
@@ -3250,13 +3234,33 @@ writeback:
if (c->rep_prefix && (c->d & String)) {
struct read_cache *rc = &ctxt->decode.io_read;
register_address_increment(c, &c->regs[VCPU_REGS_RCX], -1);
+ /* The second termination condition only applies for REPE
+ * and REPNE. Test if the repeat string operation prefix is
+ * REPE/REPZ or REPNE/REPNZ and if it's the case it tests the
+ * corresponding termination condition according to:
+ * - if REPE/REPZ and ZF = 0 then done
+ * - if REPNE/REPNZ and ZF = 1 then done
+ */
+ if ((c->b == 0xa6) || (c->b == 0xa7) ||
+ (c->b == 0xae) || (c->b == 0xaf)) {
+ trace_printk("c->eip %lx ctxt->eip %lx\n",
+ c->eip, ctxt->eip);
+ if (((c->rep_prefix == REPE_PREFIX) &&
+ ((ctxt->eflags & EFLG_ZF) == 0))
+ || ((c->rep_prefix == REPNE_PREFIX) &&
+ ((ctxt->eflags & EFLG_ZF) == EFLG_ZF))) {
+ ctxt->restart = false;
+ }
+ }
/*
* Re-enter guest when pio read ahead buffer is empty or,
* if it is not used, after each 1024 iteration.
*/
if ((rc->end == 0 && !(c->regs[VCPU_REGS_RCX] & 0x3ff)) ||
- (rc->end != 0 && rc->end == rc->pos))
+ (rc->end != 0 && rc->end == rc->pos)) {
ctxt->restart = false;
+ c->eip = ctxt->eip;
+ }
}
/*
* reset read cache here in case string instruction is restared
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] KVM: x86 emulator: fix REPZ/REPNZ termination condition
2010-08-17 8:26 ` [PATCH 4/4] KVM: x86 emulator: fix REPZ/REPNZ termination condition Avi Kivity
@ 2010-08-17 9:13 ` Gleb Natapov
2010-08-17 9:20 ` Avi Kivity
0 siblings, 1 reply; 9+ messages in thread
From: Gleb Natapov @ 2010-08-17 9:13 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Marcelo Tosatti, Mohammed Gamal
On Tue, Aug 17, 2010 at 11:26:43AM +0300, Avi Kivity wrote:
> EFLAGS.ZF needs to be checked after each iteration, not before.
>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
> arch/x86/kvm/emulate.c | 42 +++++++++++++++++++++++-------------------
> 1 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 0c0ada9..a2edfb1 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -2747,6 +2747,7 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
> int rc = X86EMUL_CONTINUE;
> int saved_dst_type = c->dst.type;
> int irq; /* Used for int 3, int, and into */
> + ulong old_eip;
Is never used.
>
> ctxt->decode.mem_read.pos = 0;
>
> @@ -2771,28 +2772,10 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
> ctxt->restart = true;
> /* All REP prefixes have the same first termination condition */
> if (address_mask(c, c->regs[VCPU_REGS_RCX]) == 0) {
> - string_done:
> ctxt->restart = false;
> ctxt->eip = c->eip;
> goto done;
> }
> - /* The second termination condition only applies for REPE
> - * and REPNE. Test if the repeat string operation prefix is
> - * REPE/REPZ or REPNE/REPNZ and if it's the case it tests the
> - * corresponding termination condition according to:
> - * - if REPE/REPZ and ZF = 0 then done
> - * - if REPNE/REPNZ and ZF = 1 then done
> - */
> - if ((c->b == 0xa6) || (c->b == 0xa7) ||
> - (c->b == 0xae) || (c->b == 0xaf)) {
> - if ((c->rep_prefix == REPE_PREFIX) &&
> - ((ctxt->eflags & EFLG_ZF) == 0))
> - goto string_done;
> - if ((c->rep_prefix == REPNE_PREFIX) &&
> - ((ctxt->eflags & EFLG_ZF) == EFLG_ZF))
> - goto string_done;
> - }
> - c->eip = ctxt->eip;
> }
>
> if (c->src.type == OP_MEM) {
> @@ -3229,6 +3212,7 @@ special_insn:
> }
>
> writeback:
> + old_eip = c->eip;
> rc = writeback(ctxt, ops);
> if (rc != X86EMUL_CONTINUE)
> goto done;
> @@ -3250,13 +3234,33 @@ writeback:
> if (c->rep_prefix && (c->d & String)) {
> struct read_cache *rc = &ctxt->decode.io_read;
> register_address_increment(c, &c->regs[VCPU_REGS_RCX], -1);
> + /* The second termination condition only applies for REPE
> + * and REPNE. Test if the repeat string operation prefix is
> + * REPE/REPZ or REPNE/REPNZ and if it's the case it tests the
> + * corresponding termination condition according to:
> + * - if REPE/REPZ and ZF = 0 then done
> + * - if REPNE/REPNZ and ZF = 1 then done
> + */
> + if ((c->b == 0xa6) || (c->b == 0xa7) ||
> + (c->b == 0xae) || (c->b == 0xaf)) {
> + trace_printk("c->eip %lx ctxt->eip %lx\n",
> + c->eip, ctxt->eip);
> + if (((c->rep_prefix == REPE_PREFIX) &&
> + ((ctxt->eflags & EFLG_ZF) == 0))
> + || ((c->rep_prefix == REPNE_PREFIX) &&
> + ((ctxt->eflags & EFLG_ZF) == EFLG_ZF))) {
> + ctxt->restart = false;
Why not jump to string_done label here?
> + }
> + }
> /*
> * Re-enter guest when pio read ahead buffer is empty or,
> * if it is not used, after each 1024 iteration.
> */
> if ((rc->end == 0 && !(c->regs[VCPU_REGS_RCX] & 0x3ff)) ||
> - (rc->end != 0 && rc->end == rc->pos))
> + (rc->end != 0 && rc->end == rc->pos)) {
> ctxt->restart = false;
> + c->eip = ctxt->eip;
We can get here when instruction is completed by above "if", so same
instruction will reexecute once again.
> + }
> }
> /*
> * reset read cache here in case string instruction is restared
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gleb.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] KVM: x86 emulator: fix REPZ/REPNZ termination condition
2010-08-17 9:13 ` Gleb Natapov
@ 2010-08-17 9:20 ` Avi Kivity
2010-08-17 9:27 ` Gleb Natapov
0 siblings, 1 reply; 9+ messages in thread
From: Avi Kivity @ 2010-08-17 9:20 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, Marcelo Tosatti, Mohammed Gamal
On 08/17/2010 12:13 PM, Gleb Natapov wrote:
> On Tue, Aug 17, 2010 at 11:26:43AM +0300, Avi Kivity wrote:
>> EFLAGS.ZF needs to be checked after each iteration, not before.
>>
>> Signed-off-by: Avi Kivity<avi@redhat.com>
>> ---
>> arch/x86/kvm/emulate.c | 42 +++++++++++++++++++++++-------------------
>> 1 files changed, 23 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index 0c0ada9..a2edfb1 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -2747,6 +2747,7 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
>> int rc = X86EMUL_CONTINUE;
>> int saved_dst_type = c->dst.type;
>> int irq; /* Used for int 3, int, and into */
>> + ulong old_eip;
> Is never used.
>
Whoops.
>> @@ -3250,13 +3234,33 @@ writeback:
>> if (c->rep_prefix&& (c->d& String)) {
>> struct read_cache *rc =&ctxt->decode.io_read;
>> register_address_increment(c,&c->regs[VCPU_REGS_RCX], -1);
>> + /* The second termination condition only applies for REPE
>> + * and REPNE. Test if the repeat string operation prefix is
>> + * REPE/REPZ or REPNE/REPNZ and if it's the case it tests the
>> + * corresponding termination condition according to:
>> + * - if REPE/REPZ and ZF = 0 then done
>> + * - if REPNE/REPNZ and ZF = 1 then done
>> + */
>> + if ((c->b == 0xa6) || (c->b == 0xa7) ||
>> + (c->b == 0xae) || (c->b == 0xaf)) {
>> + trace_printk("c->eip %lx ctxt->eip %lx\n",
>> + c->eip, ctxt->eip);
>> + if (((c->rep_prefix == REPE_PREFIX)&&
>> + ((ctxt->eflags& EFLG_ZF) == 0))
>> + || ((c->rep_prefix == REPNE_PREFIX)&&
>> + ((ctxt->eflags& EFLG_ZF) == EFLG_ZF))) {
>> + ctxt->restart = false;
> Why not jump to string_done label here?
It does a 'goto done;' which skips a couple of things.
>> + }
>> + }
>> /*
>> * Re-enter guest when pio read ahead buffer is empty or,
>> * if it is not used, after each 1024 iteration.
>> */
>> if ((rc->end == 0&& !(c->regs[VCPU_REGS_RCX]& 0x3ff)) ||
>> - (rc->end != 0&& rc->end == rc->pos))
>> + (rc->end != 0&& rc->end == rc->pos)) {
>> ctxt->restart = false;
>> + c->eip = ctxt->eip;
> We can get here when instruction is completed by above "if", so same
> instruction will reexecute once again.
Not good. Will redo (and write tests).
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] KVM: x86 emulator: fix REPZ/REPNZ termination condition
2010-08-17 9:20 ` Avi Kivity
@ 2010-08-17 9:27 ` Gleb Natapov
2010-08-17 9:29 ` Avi Kivity
0 siblings, 1 reply; 9+ messages in thread
From: Gleb Natapov @ 2010-08-17 9:27 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Marcelo Tosatti, Mohammed Gamal
On Tue, Aug 17, 2010 at 12:20:34PM +0300, Avi Kivity wrote:
> On 08/17/2010 12:13 PM, Gleb Natapov wrote:
> >On Tue, Aug 17, 2010 at 11:26:43AM +0300, Avi Kivity wrote:
> >>EFLAGS.ZF needs to be checked after each iteration, not before.
> >>
> >>Signed-off-by: Avi Kivity<avi@redhat.com>
> >>---
> >> arch/x86/kvm/emulate.c | 42 +++++++++++++++++++++++-------------------
> >> 1 files changed, 23 insertions(+), 19 deletions(-)
> >>
> >>diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> >>index 0c0ada9..a2edfb1 100644
> >>--- a/arch/x86/kvm/emulate.c
> >>+++ b/arch/x86/kvm/emulate.c
> >>@@ -2747,6 +2747,7 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
> >> int rc = X86EMUL_CONTINUE;
> >> int saved_dst_type = c->dst.type;
> >> int irq; /* Used for int 3, int, and into */
> >>+ ulong old_eip;
> >Is never used.
> >
>
> Whoops.
>
> >>@@ -3250,13 +3234,33 @@ writeback:
> >> if (c->rep_prefix&& (c->d& String)) {
> >> struct read_cache *rc =&ctxt->decode.io_read;
> >> register_address_increment(c,&c->regs[VCPU_REGS_RCX], -1);
> >>+ /* The second termination condition only applies for REPE
> >>+ * and REPNE. Test if the repeat string operation prefix is
> >>+ * REPE/REPZ or REPNE/REPNZ and if it's the case it tests the
> >>+ * corresponding termination condition according to:
> >>+ * - if REPE/REPZ and ZF = 0 then done
> >>+ * - if REPNE/REPNZ and ZF = 1 then done
> >>+ */
> >>+ if ((c->b == 0xa6) || (c->b == 0xa7) ||
> >>+ (c->b == 0xae) || (c->b == 0xaf)) {
> >>+ trace_printk("c->eip %lx ctxt->eip %lx\n",
> >>+ c->eip, ctxt->eip);
> >>+ if (((c->rep_prefix == REPE_PREFIX)&&
> >>+ ((ctxt->eflags& EFLG_ZF) == 0))
> >>+ || ((c->rep_prefix == REPNE_PREFIX)&&
> >>+ ((ctxt->eflags& EFLG_ZF) == EFLG_ZF))) {
> >>+ ctxt->restart = false;
> >Why not jump to string_done label here?
>
> It does a 'goto done;' which skips a couple of things.
>
The only thing it skips is:
ctxt->decode.mem_read.end = 0;
as far as I can see. And this is ok if instruction is completed.
> >>+ }
> >>+ }
> >> /*
> >> * Re-enter guest when pio read ahead buffer is empty or,
> >> * if it is not used, after each 1024 iteration.
> >> */
> >> if ((rc->end == 0&& !(c->regs[VCPU_REGS_RCX]& 0x3ff)) ||
> >>- (rc->end != 0&& rc->end == rc->pos))
> >>+ (rc->end != 0&& rc->end == rc->pos)) {
> >> ctxt->restart = false;
> >>+ c->eip = ctxt->eip;
> >We can get here when instruction is completed by above "if", so same
> >instruction will reexecute once again.
>
> Not good. Will redo (and write tests).
>
>
> --
> error compiling committee.c: too many arguments to function
--
Gleb.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] KVM: x86 emulator: fix REPZ/REPNZ termination condition
2010-08-17 9:27 ` Gleb Natapov
@ 2010-08-17 9:29 ` Avi Kivity
0 siblings, 0 replies; 9+ messages in thread
From: Avi Kivity @ 2010-08-17 9:29 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, Marcelo Tosatti, Mohammed Gamal
On 08/17/2010 12:27 PM, Gleb Natapov wrote:
> On Tue, Aug 17, 2010 at 12:20:34PM +0300, Avi Kivity wrote:
>> On 08/17/2010 12:13 PM, Gleb Natapov wrote:
>>> On Tue, Aug 17, 2010 at 11:26:43AM +0300, Avi Kivity wrote:
>>>> EFLAGS.ZF needs to be checked after each iteration, not before.
>>>>
>>>> Signed-off-by: Avi Kivity<avi@redhat.com>
>>>> ---
>>>> arch/x86/kvm/emulate.c | 42 +++++++++++++++++++++++-------------------
>>>> 1 files changed, 23 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>>>> index 0c0ada9..a2edfb1 100644
>>>> --- a/arch/x86/kvm/emulate.c
>>>> +++ b/arch/x86/kvm/emulate.c
>>>> @@ -2747,6 +2747,7 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
>>>> int rc = X86EMUL_CONTINUE;
>>>> int saved_dst_type = c->dst.type;
>>>> int irq; /* Used for int 3, int, and into */
>>>> + ulong old_eip;
>>> Is never used.
>>>
>> Whoops.
>>
>>>> @@ -3250,13 +3234,33 @@ writeback:
>>>> if (c->rep_prefix&& (c->d& String)) {
>>>> struct read_cache *rc =&ctxt->decode.io_read;
>>>> register_address_increment(c,&c->regs[VCPU_REGS_RCX], -1);
>>>> + /* The second termination condition only applies for REPE
>>>> + * and REPNE. Test if the repeat string operation prefix is
>>>> + * REPE/REPZ or REPNE/REPNZ and if it's the case it tests the
>>>> + * corresponding termination condition according to:
>>>> + * - if REPE/REPZ and ZF = 0 then done
>>>> + * - if REPNE/REPNZ and ZF = 1 then done
>>>> + */
>>>> + if ((c->b == 0xa6) || (c->b == 0xa7) ||
>>>> + (c->b == 0xae) || (c->b == 0xaf)) {
>>>> + trace_printk("c->eip %lx ctxt->eip %lx\n",
>>>> + c->eip, ctxt->eip);
>>>> + if (((c->rep_prefix == REPE_PREFIX)&&
>>>> + ((ctxt->eflags& EFLG_ZF) == 0))
>>>> + || ((c->rep_prefix == REPNE_PREFIX)&&
>>>> + ((ctxt->eflags& EFLG_ZF) == EFLG_ZF))) {
>>>> + ctxt->restart = false;
>>> Why not jump to string_done label here?
>> It does a 'goto done;' which skips a couple of things.
>>
> The only thing it skips is:
> ctxt->decode.mem_read.end = 0;
> as far as I can see. And this is ok if instruction is completed.
I'd like to avoid this interdependency (and there are enough gotos in
the emulator already).
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 9+ messages in thread