From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH] KVM: emulator: implement AAD instruction Date: Fri, 14 Dec 2012 14:33:15 +0100 Message-ID: <50CB2A9B.1030208@redhat.com> References: <20121210094230.GI19514@redhat.com> <50CB0306.6020909@redhat.com> <20121214112919.GN29003@redhat.com> <20121214124200.GO29003@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, mtosatti@redhat.com To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:40990 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752282Ab2LNNdX (ORCPT ); Fri, 14 Dec 2012 08:33:23 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBEDXNlW006801 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 14 Dec 2012 08:33:23 -0500 In-Reply-To: <20121214124200.GO29003@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Il 14/12/2012 13:42, Gleb Natapov ha scritto: > 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 >>>> 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? Ok, I'll also add a testcase. Paolo