From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCH 2/3] KVM: x86 emulator: move string instruction
 completion check into separate function
Date: Tue, 24 Aug 2010 16:20:33 +0300
Message-ID: <20100824132033.GB10499@redhat.com>
References: <1282649455-9463-1-git-send-email-gleb@redhat.com>
 <1282649455-9463-2-git-send-email-gleb@redhat.com>
 <4C73C4F8.9030809@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: mtosatti@redhat.com, kvm@vger.kernel.org
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:38512 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1755083Ab0HXNUe convert rfc822-to-8bit (ORCPT
	<rfc822;kvm@vger.kernel.org>); Tue, 24 Aug 2010 09:20:34 -0400
Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])
	by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o7ODKYUF006219
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <kvm@vger.kernel.org>; Tue, 24 Aug 2010 09:20:34 -0400
Content-Disposition: inline
In-Reply-To: <4C73C4F8.9030809@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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 =3D=3D X86EMUL_UNHANDLEABLE) ? -1 : 0;
> >  }
> >
> >+static bool string_inst_completed(struct x86_emulate_ctxt *ctxt)
>=20
> s/inst/insn/.
>=20
> >+{
> >+	struct decode_cache *c =3D&ctxt->decode;
> >+
> >+	/* All REP prefixes have the same first termination condition */
> >+	if (address_mask(c, c->regs[VCPU_REGS_RCX]) =3D=3D 0)
> >+		return true;
>=20
> This is checked during the beginning of the instruction, not after
> completion.  Why is it here?  it will just be duplicated.
>=20
SDM describes REP instruction algorithm this way:

WHILE CountReg =E2=89=A0 0
  DO
       Service pending interrupts (if any);
       Execute associated string instruction;
       CountReg =E2=86=90 (CountReg =E2=80=93 1);
       IF CountReg =3D 0
            THEN exit WHILE loop; FI;
       IF (Repeat prefix is REPZ or REPE) and (ZF =3D 0)
       or (Repeat prefix is REPNZ or REPNE) and (ZF =3D 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 =3D 0 then done
> >+	 * 	- if REPNE/REPNZ and ZF =3D 1 then done
> >+	 */
> >+	if (((c->b =3D=3D 0xa6) || (c->b =3D=3D 0xa7) ||
> >+	     (c->b =3D=3D 0xae) || (c->b =3D=3D 0xaf))
> >+	&&  (((c->rep_prefix =3D=3D REPE_PREFIX)&&
> >+		 ((ctxt->eflags&  EFLG_ZF) =3D=3D 0))
> >+		|| ((c->rep_prefix =3D=3D REPNE_PREFIX)&&
> >+		    ((ctxt->eflags&  EFLG_ZF) =3D=3D EFLG_ZF))))
> >+		return true;
> >+
> >+	return false;
> >+}
> >+
>=20
> --=20
> error compiling committee.c: too many arguments to function

--
			Gleb.