From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCH] KVM: emulator: implement AAD instruction
Date: Fri, 14 Dec 2012 14:42:00 +0200
Message-ID: <20121214124200.GO29003@redhat.com>
References: <20121210094230.GI19514@redhat.com>
 <50CB0306.6020909@redhat.com>
 <20121214112919.GN29003@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, mtosatti@redhat.com
To: Paolo Bonzini <pbonzini@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:18990 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753952Ab2LNMmC (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 14 Dec 2012 07:42:02 -0500
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.14.4/8.14.4) with ESMTP id qBECg1RN030428
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <kvm@vger.kernel.org>; Fri, 14 Dec 2012 07:42:01 -0500
Content-Disposition: inline
In-Reply-To: <20121214112919.GN29003@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Fri, Dec 14, 2012 at 01:29:19PM +0200, Gleb Natapov wrote:
> On Fri, Dec 14, 2012 at 11:44:22AM +0100, Paolo Bonzini wrote:
> > Il 10/12/2012 10:42, Gleb Natapov ha scritto:
> > > Windows2000 uses it during boot. This fixes
> > > https://bugzilla.kernel.org/show_bug.cgi?id=50921
> > > 
> > > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> > > index 39171cb..92c7292 100644
> > > --- a/arch/x86/kvm/emulate.c
> > > +++ b/arch/x86/kvm/emulate.c
> > > @@ -2852,6 +2852,27 @@ static int em_das(struct x86_emulate_ctxt *ctxt)
> > >  	return X86EMUL_CONTINUE;
> > >  }
> > >  
> > > +static int em_aad(struct x86_emulate_ctxt *ctxt)
> > > +{
> > > +	u8 al = ctxt->dst.val & 0xff;
> > > +	u8 ah = (ctxt->dst.val >> 8) & 0xff;
> > > +
> > > +	al = (al + (ah * ctxt->src.val)) & 0xff;
> > > +
> > > +	ctxt->dst.val = (ctxt->dst.val & 0xffff0000) | al;
> > > +
> > > +	ctxt->eflags &= ~(X86_EFLAGS_PF | X86_EFLAGS_SF | X86_EFLAGS_ZF);
> > > +
> > > +	if (!al)
> > > +		ctxt->eflags |= X86_EFLAGS_ZF;
> > > +	if (!(al & 1))
> > > +		ctxt->eflags |= X86_EFLAGS_PF;
> > 
> > This is wrong, it should check the parity of al (even=1, odd=0).
> > 
> Oops, yes it should check number of bits set, not value itself.
> 
> > Perhaps you can use the trick of em_das:
> > 
> >         /* Set PF, ZF, SF */
> >         ctxt->src.type = OP_IMM;
> >         ctxt->src.val = 0;
> >         ctxt->src.bytes = 1;
> >         emulate_2op_SrcV(ctxt, "or");
> Will do.
> 
The patch is applied already. Paolo do you mind to send the fix?

--
			Gleb.