Re: [PATCH 2/3] KVM: x86 emulator: move string instruction completion check into separate function

[Gleb Natapov <gleb@redhat.com>]

On Tue, Aug 24, 2010 at 04:11:20PM +0300, Avi Kivity wrote:
>  On 08/24/2010 02:30 PM, Gleb Natapov wrote:
> >Signed-off-by: Gleb Natapov<gleb@redhat.com>
> >---
> >  arch/x86/kvm/emulate.c |   42 +++++++++++++++++++++++++++++-------------
> >  1 files changed, 29 insertions(+), 13 deletions(-)
> >
> >diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> >index f9f8353..d34d706 100644
> >--- a/arch/x86/kvm/emulate.c
> >+++ b/arch/x86/kvm/emulate.c
> >@@ -2921,6 +2921,32 @@ done:
> >  	return (rc == X86EMUL_UNHANDLEABLE) ? -1 : 0;
> >  }
> >
> >+static bool string_inst_completed(struct x86_emulate_ctxt *ctxt)
> 
> s/inst/insn/.
> 
> >+{
> >+	struct decode_cache *c =&ctxt->decode;
> >+
> >+	/* All REP prefixes have the same first termination condition */
> >+	if (address_mask(c, c->regs[VCPU_REGS_RCX]) == 0)
> >+		return true;
> 
> This is checked during the beginning of the instruction, not after
> completion.  Why is it here?  it will just be duplicated.
> 
SDM describes REP instruction algorithm this way:

WHILE CountReg ≠ 0
  DO
       Service pending interrupts (if any);
       Execute associated string instruction;
       CountReg ← (CountReg – 1);
       IF CountReg = 0
            THEN exit WHILE loop; FI;
       IF (Repeat prefix is REPZ or REPE) and (ZF = 0)
       or (Repeat prefix is REPNZ or REPNE) and (ZF = 1)
            THEN exit WHILE loop; FI;
  OD;

So CountReg is checked at the beginning and after each iteration.
Practically it will save us one return to a guest and exit back
to emulator at the end of rep instruction (not a big deal).

> >+
> >+	/* 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))))
> >+		return true;
> >+
> >+	return false;
> >+}
> >+
> 
> -- 
> error compiling committee.c: too many arguments to function

--
			Gleb.