All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Emulator INTn and SCAS fixes
@ 2010-08-17 16:44 Avi Kivity
  2010-08-17 16:44 ` [PATCH v2 1/3] KVM: x86 emulator: fix INTn emulation not pushing EFLAGS and CS Avi Kivity
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Avi Kivity @ 2010-08-17 16:44 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm

The following patchset makes INTn work and implements SCAS (used by vgabios).
With the patchset, vgabios is able to display its splash screen but gets
confused shortly afterwards.

Based on the non-atomic-injection branch.

Avi Kivity (4):
  KVM: Initialize operand and address sizes before emulating interrupts
    - dropped - doesn't apply to mainline yet
  KVM: x86 emulator: fix INTn emulation not pushing EFLAGS and CS
  KVM: x86 emulator: implement SCAS (opcodes AE, AF)
  KVM: x86 emulator: fix REPZ/REPNZ termination condition
    - drop new unused variable
    - if ZF test succeeds, avoid following test

 arch/x86/kvm/emulate.c |   56 +++++++++++++++++++++++++++--------------------
 1 files changed, 32 insertions(+), 24 deletions(-)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] KVM: x86 emulator: fix INTn emulation not pushing EFLAGS and CS
  2010-08-17 16:44 [PATCH v2 0/3] Emulator INTn and SCAS fixes Avi Kivity
@ 2010-08-17 16:44 ` Avi Kivity
  2010-08-17 16:44 ` [PATCH v2 2/3] KVM: x86 emulator: implement SCAS (opcodes AE, AF) Avi Kivity
  2010-08-17 16:44 ` [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition Avi Kivity
  2 siblings, 0 replies; 10+ messages in thread
From: Avi Kivity @ 2010-08-17 16:44 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm

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 ac13831..ed985a9 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1232,7 +1232,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;
@@ -1242,14 +1242,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] 10+ messages in thread

* [PATCH v2 2/3] KVM: x86 emulator: implement SCAS (opcodes AE, AF)
  2010-08-17 16:44 [PATCH v2 0/3] Emulator INTn and SCAS fixes Avi Kivity
  2010-08-17 16:44 ` [PATCH v2 1/3] KVM: x86 emulator: fix INTn emulation not pushing EFLAGS and CS Avi Kivity
@ 2010-08-17 16:44 ` Avi Kivity
  2010-08-17 16:44 ` [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition Avi Kivity
  2 siblings, 0 replies; 10+ messages in thread
From: Avi Kivity @ 2010-08-17 16:44 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm

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 ed985a9..729853a 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2312,7 +2312,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 */
@@ -3047,8 +3047,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] 10+ messages in thread

* [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition
  2010-08-17 16:44 [PATCH v2 0/3] Emulator INTn and SCAS fixes Avi Kivity
  2010-08-17 16:44 ` [PATCH v2 1/3] KVM: x86 emulator: fix INTn emulation not pushing EFLAGS and CS Avi Kivity
  2010-08-17 16:44 ` [PATCH v2 2/3] KVM: x86 emulator: implement SCAS (opcodes AE, AF) Avi Kivity
@ 2010-08-17 16:44 ` Avi Kivity
  2010-08-19  4:55   ` Wei Yongjun
  2010-08-19 10:55   ` Gleb Natapov
  2 siblings, 2 replies; 10+ messages in thread
From: Avi Kivity @ 2010-08-17 16:44 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm

EFLAGS.ZF needs to be checked after each iteration, not before.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 729853a..d15a746 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2782,28 +2782,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) && !(c->d & NoAccess)) {
@@ -3230,13 +3212,29 @@ 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))
+		    && (((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))
+		else if ((rc->end == 0 && !(c->regs[VCPU_REGS_RCX] & 0x3ff)) ||
+			 (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] 10+ messages in thread

* Re: [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition
  2010-08-17 16:44 ` [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition Avi Kivity
@ 2010-08-19  4:55   ` Wei Yongjun
  2010-08-19  6:59     ` Avi Kivity
  2010-08-19 10:55   ` Gleb Natapov
  1 sibling, 1 reply; 10+ messages in thread
From: Wei Yongjun @ 2010-08-19  4:55 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

Hi Avi Kivity:

> EFLAGS.ZF needs to be checked after each iteration, not before.
>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/emulate.c |   38 ++++++++++++++++++--------------------
>  1 files changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 729853a..d15a746 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -2782,28 +2782,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;
>   

It seems that you cannot remove the above line, the assign for eip is need.
remove it will break FreeDOS livecd. Not sure why need this.

>  	}
>  
>  	if ((c->src.type == OP_MEM) && !(c->d & NoAccess)) {
> @@ -3230,13 +3212,29 @@ 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))
> +		    && (((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))
> +		else if ((rc->end == 0 && !(c->regs[VCPU_REGS_RCX] & 0x3ff)) ||
> +			 (rc->end != 0 && rc->end == rc->pos)) {
>  			ctxt->restart = false;
> +			c->eip = ctxt->eip;
> +		}
>  	}
>  	/*
>  	 * reset read cache here in case string instruction is restared
>   

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition
  2010-08-19  4:55   ` Wei Yongjun
@ 2010-08-19  6:59     ` Avi Kivity
  2010-08-19  8:32       ` Wei Yongjun
  2010-08-19  8:39       ` Wei Yongjun
  0 siblings, 2 replies; 10+ messages in thread
From: Avi Kivity @ 2010-08-19  6:59 UTC (permalink / raw)
  To: Wei Yongjun; +Cc: Marcelo Tosatti, kvm

 On 08/19/2010 07:55 AM, Wei Yongjun wrote:
> Hi Avi Kivity:
>
>> EFLAGS.ZF needs to be checked after each iteration, not before.
>>
>> Signed-off-by: Avi Kivity <avi@redhat.com>
>> ---
>>  arch/x86/kvm/emulate.c |   38 ++++++++++++++++++--------------------
>>  1 files changed, 18 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> index 729853a..d15a746 100644
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -2782,28 +2782,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;
>>   
> It seems that you cannot remove the above line, the assign for eip is need.
> remove it will break FreeDOS livecd. Not sure why need this.

I'll try it out. Are you running FreeDOS with
emulate_invalid_guest_state=0 or 1?

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition
  2010-08-19  6:59     ` Avi Kivity
@ 2010-08-19  8:32       ` Wei Yongjun
  2010-08-19  8:39       ` Wei Yongjun
  1 sibling, 0 replies; 10+ messages in thread
From: Wei Yongjun @ 2010-08-19  8:32 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm



>  On 08/19/2010 07:55 AM, Wei Yongjun wrote:
>   
>> Hi Avi Kivity:
>>
>>     
>>> EFLAGS.ZF needs to be checked after each iteration, not before.
>>>
>>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>> ---
>>>  arch/x86/kvm/emulate.c |   38 ++++++++++++++++++--------------------
>>>  1 files changed, 18 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>>> index 729853a..d15a746 100644
>>> --- a/arch/x86/kvm/emulate.c
>>> +++ b/arch/x86/kvm/emulate.c
>>> @@ -2782,28 +2782,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;
>>>   
>>>       
>> It seems that you cannot remove the above line, the assign for eip is need.
>> remove it will break FreeDOS livecd. Not sure why need this.
>>     
> I'll try it out. Are you running FreeDOS with
> emulate_invalid_guest_state=0 or 1?
>   

I try it with emulate_invalid_guest_state=1.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition
  2010-08-19  6:59     ` Avi Kivity
  2010-08-19  8:32       ` Wei Yongjun
@ 2010-08-19  8:39       ` Wei Yongjun
  1 sibling, 0 replies; 10+ messages in thread
From: Wei Yongjun @ 2010-08-19  8:39 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm


>  On 08/19/2010 07:55 AM, Wei Yongjun wrote:
>   
>> Hi Avi Kivity:
>>
>>     
>>> EFLAGS.ZF needs to be checked after each iteration, not before.
>>>
>>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>> ---
>>>  arch/x86/kvm/emulate.c |   38 ++++++++++++++++++--------------------
>>>  1 files changed, 18 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>>> index 729853a..d15a746 100644
>>> --- a/arch/x86/kvm/emulate.c
>>> +++ b/arch/x86/kvm/emulate.c
>>> @@ -2782,28 +2782,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;
>>>   
>>>       
>> It seems that you cannot remove the above line, the assign for eip is need.
>> remove it will break FreeDOS livecd. Not sure why need this.
>>     
> I'll try it out. Are you running FreeDOS with
> emulate_invalid_guest_state=0 or 1?
>   

It broken the boot from cdrom, even emulate_invalid_guest_state=0

#qemu-system-x86_64 --boot d --cdrom fdbasews.iso

This will return can not boot from CDROM.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition
  2010-08-17 16:44 ` [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition Avi Kivity
  2010-08-19  4:55   ` Wei Yongjun
@ 2010-08-19 10:55   ` Gleb Natapov
  2010-08-19 11:31     ` Avi Kivity
  1 sibling, 1 reply; 10+ messages in thread
From: Gleb Natapov @ 2010-08-19 10:55 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

On Tue, Aug 17, 2010 at 07:44:20PM +0300, Avi Kivity wrote:
>  	if ((c->src.type == OP_MEM) && !(c->d & NoAccess)) {
> @@ -3230,13 +3212,29 @@ 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))
> +		    && (((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))
> +		else if ((rc->end == 0 && !(c->regs[VCPU_REGS_RCX] & 0x3ff)) ||
> +			 (rc->end != 0 && rc->end == rc->pos)) {
>  			ctxt->restart = false;
> +			c->eip = ctxt->eip;
If io exit to use space is needed we may not get here, so ctxt->eip will
be updated to point past instruction in the middle of instruction
emulation and that may cause incomplete instruction emulation.

> +		}
>  	}
>  	/*
>  	 * 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] 10+ messages in thread

* Re: [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition
  2010-08-19 10:55   ` Gleb Natapov
@ 2010-08-19 11:31     ` Avi Kivity
  0 siblings, 0 replies; 10+ messages in thread
From: Avi Kivity @ 2010-08-19 11:31 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: Marcelo Tosatti, kvm

  On 08/19/2010 01:55 PM, Gleb Natapov wrote:
> On Tue, Aug 17, 2010 at 07:44:20PM +0300, Avi Kivity wrote:
>>   	if ((c->src.type == OP_MEM)&&  !(c->d&  NoAccess)) {
>> @@ -3230,13 +3212,29 @@ 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))
>> +		&&  (((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))
>> +		else if ((rc->end == 0&&  !(c->regs[VCPU_REGS_RCX]&  0x3ff)) ||
>> +			 (rc->end != 0&&  rc->end == rc->pos)) {
>>   			ctxt->restart = false;
>> +			c->eip = ctxt->eip;
> If io exit to use space is needed we may not get here, so ctxt->eip will
> be updated to point past instruction in the middle of instruction
> emulation and that may cause incomplete instruction emulation.

Right.  Fixing this made -cdrom work again.

Will post an updated patch.

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-08-19 11:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-17 16:44 [PATCH v2 0/3] Emulator INTn and SCAS fixes Avi Kivity
2010-08-17 16:44 ` [PATCH v2 1/3] KVM: x86 emulator: fix INTn emulation not pushing EFLAGS and CS Avi Kivity
2010-08-17 16:44 ` [PATCH v2 2/3] KVM: x86 emulator: implement SCAS (opcodes AE, AF) Avi Kivity
2010-08-17 16:44 ` [PATCH v2 3/3] KVM: x86 emulator: fix REPZ/REPNZ termination condition Avi Kivity
2010-08-19  4:55   ` Wei Yongjun
2010-08-19  6:59     ` Avi Kivity
2010-08-19  8:32       ` Wei Yongjun
2010-08-19  8:39       ` Wei Yongjun
2010-08-19 10:55   ` Gleb Natapov
2010-08-19 11:31     ` Avi Kivity

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.