Re: [PATCH] KVM: emulator: implement AAD instruction

[Paolo Bonzini <pbonzini@redhat.com>]

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 <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?

Ok, I'll also add a testcase.

Paolo