Skip to contentSkip to conversation

KVM: emulator: implement AAD instruction

PATCH In review Gleb Natapov 2 participants 5 messages Dec 10, 2012 – Dec 14, 2012

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Gleb Natapov

author
#0 lore raw

PATCH 1/1 KVM: emulator: implement AAD instruction

Windows2000 uses it during boot. This fixes
https://bugzilla.kernel.org/show_bug.cgi?id=50921

Signed-off-by Gleb Natapov <[email protected]>
arch/x86/kvm/emulate.c +22 -1
index 39171cb..92c7292 100644
@@ -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.

maintainer
#1 lore raw

Il 10/12/2012 10:42, Gleb Natapov ha scritto:

Quoted 17 lines

Windows2000 uses it during boot. This fixes
https://bugzilla.kernel.org/show_bug.cgi?id=50921

Signed-off-by Gleb Natapov <[email protected]>
arch/x86/kvm/emulate.c +5 -0
index 39171cb..92c7292 100644
@@ -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;
+

arch/x86/kvm/emulate.c static int em_das(struct x86_emulate_ctxt *ctxt) · line 2869

Quoted 10 lines
arch/x86/kvm/emulate.c +10 -0
+	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

Quoted 25 lines
arch/x86/kvm/emulate.c +7 -2
+	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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#2 lore raw
Quoted 22 lines · Paolo Bonzini

Il 10/12/2012 10:42, Gleb Natapov ha scritto:

Quoted 21 lines

Windows2000 uses it during boot. This fixes
https://bugzilla.kernel.org/show_bug.cgi?id=50921

Signed-off-by Gleb Natapov <[email protected]>
arch/x86/kvm/emulate.c +9 -0
index 39171cb..92c7292 100644
@@ -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;
+
Quoted 9 lines

arch/x86/kvm/emulate.c static int em_das(struct x86_emulate_ctxt *ctxt) · line 2869

Quoted 6 lines
arch/x86/kvm/emulate.c +6 -0
+	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.

Quoted 7 lines

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.

Quoted 20 lines

Paolo

Quoted 17 lines
arch/x86/kvm/emulate.c +7 -1
+	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,
Quoted 8 lines
Quoted 8 lines
arch/x86/kvm/emulate.c +0 -1
 	/* 0xE0 - 0xE7 */
--

Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Gleb.

author
#3 lore raw
Quoted 34 lines · Gleb Natapov
Quoted 22 lines · Paolo Bonzini

Il 10/12/2012 10:42, Gleb Natapov ha scritto:

Quoted 21 lines

Windows2000 uses it during boot. This fixes
https://bugzilla.kernel.org/show_bug.cgi?id=50921

Signed-off-by Gleb Natapov <[email protected]>
arch/x86/kvm/emulate.c +9 -0
index 39171cb..92c7292 100644
@@ -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;
+
Quoted 9 lines

arch/x86/kvm/emulate.c static int em_das(struct x86_emulate_ctxt *ctxt) · line 2869

Quoted 6 lines
arch/x86/kvm/emulate.c +6 -0
+	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.

Quoted 9 lines
Quoted 7 lines

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.

maintainer
#4 lore raw

Il 14/12/2012 13:42, Gleb Natapov ha scritto:

Quoted 37 lines · Gleb Natapov
Quoted 36 lines · Paolo Bonzini
Quoted 22 lines

Il 10/12/2012 10:42, Gleb Natapov ha scritto:

Quoted 21 lines

Windows2000 uses it during boot. This fixes
https://bugzilla.kernel.org/show_bug.cgi?id=50921

Signed-off-by Gleb Natapov <[email protected]>
arch/x86/kvm/emulate.c +9 -0
index 39171cb..92c7292 100644
@@ -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;
+
Quoted 9 lines

arch/x86/kvm/emulate.c static int em_das(struct x86_emulate_ctxt *ctxt) · line 2869

Quoted 6 lines
arch/x86/kvm/emulate.c +6 -0
+	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.

Quoted 2 lines

Perhaps you can use the trick of em_das:

Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
        /* 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