* [PATCH] KVM: emulator: implement AAD instruction
@ 2012-12-10 9:42 Gleb Natapov
2012-12-14 10:44 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: Gleb Natapov @ 2012-12-10 9:42 UTC (permalink / raw)
To: kvm; +Cc: mtosatti
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;
+ if (al & 0x80)
+ ctxt->eflags |= X86_EFLAGS_SF;
+
+ return X86EMUL_CONTINUE;
+}
+
static int em_call(struct x86_emulate_ctxt *ctxt)
{
long rel = ctxt->src.val;
@@ -3801,7 +3822,7 @@ static const struct opcode opcode_table[256] = {
D(ImplicitOps | No64), II(ImplicitOps, em_iret, iret),
/* 0xD0 - 0xD7 */
D2bv(DstMem | SrcOne | ModRM), D2bv(DstMem | ModRM),
- N, N, N, N,
+ N, I(DstAcc | SrcImmByte | No64, em_aad), N, N,
/* 0xD8 - 0xDF */
N, N, N, N, N, N, N, N,
/* 0xE0 - 0xE7 */
--
Gleb.
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: emulator: implement AAD instruction
2012-12-10 9:42 [PATCH] KVM: emulator: implement AAD instruction Gleb Natapov
@ 2012-12-14 10:44 ` Paolo Bonzini
2012-12-14 11:29 ` Gleb Natapov
0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2012-12-14 10:44 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, mtosatti
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).
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");
Paolo
> + if (al & 0x80)
> + ctxt->eflags |= X86_EFLAGS_SF;
> +
> + return X86EMUL_CONTINUE;
> +}
> +
> static int em_call(struct x86_emulate_ctxt *ctxt)
> {
> long rel = ctxt->src.val;
> @@ -3801,7 +3822,7 @@ static const struct opcode opcode_table[256] = {
> D(ImplicitOps | No64), II(ImplicitOps, em_iret, iret),
> /* 0xD0 - 0xD7 */
> D2bv(DstMem | SrcOne | ModRM), D2bv(DstMem | ModRM),
> - N, N, N, N,
> + N, I(DstAcc | SrcImmByte | No64, em_aad), N, N,
> /* 0xD8 - 0xDF */
> N, N, N, N, N, N, N, N,
> /* 0xE0 - 0xE7 */
> --
> Gleb.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: emulator: implement AAD instruction
2012-12-14 10:44 ` Paolo Bonzini
@ 2012-12-14 11:29 ` Gleb Natapov
2012-12-14 12:42 ` Gleb Natapov
0 siblings, 1 reply; 5+ messages in thread
From: Gleb Natapov @ 2012-12-14 11:29 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, mtosatti
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.
>
> Paolo
>
> > + if (al & 0x80)
> > + ctxt->eflags |= X86_EFLAGS_SF;
> > +
> > + return X86EMUL_CONTINUE;
> > +}
> > +
> > static int em_call(struct x86_emulate_ctxt *ctxt)
> > {
> > long rel = ctxt->src.val;
> > @@ -3801,7 +3822,7 @@ static const struct opcode opcode_table[256] = {
> > D(ImplicitOps | No64), II(ImplicitOps, em_iret, iret),
> > /* 0xD0 - 0xD7 */
> > D2bv(DstMem | SrcOne | ModRM), D2bv(DstMem | ModRM),
> > - N, N, N, N,
> > + N, I(DstAcc | SrcImmByte | No64, em_aad), N, N,
> > /* 0xD8 - 0xDF */
> > N, N, N, N, N, N, N, N,
> > /* 0xE0 - 0xE7 */
> > --
> > Gleb.
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
--
Gleb.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: emulator: implement AAD instruction
2012-12-14 11:29 ` Gleb Natapov
@ 2012-12-14 12:42 ` Gleb Natapov
2012-12-14 13:33 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: Gleb Natapov @ 2012-12-14 12:42 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, mtosatti
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: emulator: implement AAD instruction
2012-12-14 12:42 ` Gleb Natapov
@ 2012-12-14 13:33 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2012-12-14 13:33 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, mtosatti
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-12-14 13:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-10 9:42 [PATCH] KVM: emulator: implement AAD instruction Gleb Natapov
2012-12-14 10:44 ` Paolo Bonzini
2012-12-14 11:29 ` Gleb Natapov
2012-12-14 12:42 ` Gleb Natapov
2012-12-14 13:33 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).