[Xen-changelog] [xen master] x86_emulate: fix flag setting for 8-bit signed multiplication

[patchbot@xxxxxxx]

commit 3abe8efb431d171e824f593615ad4a8efbdbbcf1
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Fri Sep 20 11:04:52 2013 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Sep 20 11:04:52 2013 +0200

    x86_emulate: fix flag setting for 8-bit signed multiplication
    
    We really need to check for a signed overflow of 8 bits, while the
    previous check compared the sign-extended 8-bit result with the
    zero-extended 16-bit one (which was wrong for all negative results).
    
    Once at it
    - also adjust the 16-bit comparison for symmetry
    - improve the 8-bit multiplication (no need to zero-extend to 32-bits
      the sign-extended to 16 bits original 8-bit value)
    - fold both signed multiplication variants
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Keir Fraser <keir@xxxxxxx>
---
 tools/tests/x86_emulator/x86_emulate.c |    2 +
 xen/arch/x86/x86_emulate/x86_emulate.c |   56 ++++++++++----------------------
 2 files changed, 19 insertions(+), 39 deletions(-)

diff --git a/tools/tests/x86_emulator/x86_emulate.c 
b/tools/tests/x86_emulator/x86_emulate.c
index f18f615..b157ade 100644
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -1,3 +1,4 @@
+#include <assert.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
@@ -8,6 +9,7 @@
 typedef bool bool_t;
 
 #define BUG() abort()
+#define ASSERT assert
 
 #define cpu_has_amd_erratum(nr) 0
 #define mark_regs_dirty(r) ((void)(r))
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
b/xen/arch/x86/x86_emulate/x86_emulate.c
index b4a24c8..d3023de 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2096,40 +2096,13 @@ x86_emulate(
         goto push;
 
     case 0x69: /* imul imm16/32 */
-    case 0x6b: /* imul imm8 */ {
-        unsigned long src1; /* ModR/M source operand */
+    case 0x6b: /* imul imm8 */
         if ( ea.type == OP_REG )
-            src1 = *ea.reg;
+            dst.val = *ea.reg;
         else if ( (rc = read_ulong(ea.mem.seg, ea.mem.off,
-                                   &src1, op_bytes, ctxt, ops)) )
+                                   &dst.val, op_bytes, ctxt, ops)) )
             goto done;
-        _regs.eflags &= ~(EFLG_OF|EFLG_CF);
-        switch ( dst.bytes )
-        {
-        case 2:
-            dst.val = ((uint32_t)(int16_t)src.val *
-                       (uint32_t)(int16_t)src1);
-            if ( (int16_t)dst.val != (uint32_t)dst.val )
-                _regs.eflags |= EFLG_OF|EFLG_CF;
-            break;
-#ifdef __x86_64__
-        case 4:
-            dst.val = ((uint64_t)(int32_t)src.val *
-                       (uint64_t)(int32_t)src1);
-            if ( (int32_t)dst.val != dst.val )
-                _regs.eflags |= EFLG_OF|EFLG_CF;
-            break;
-#endif
-        default: {
-            unsigned long m[2] = { src.val, src1 };
-            if ( imul_dbl(m) )
-                _regs.eflags |= EFLG_OF|EFLG_CF;
-            dst.val = m[0];
-            break;
-        }
-        }
-        break;
-    }
+        goto imul;
 
     case 0x6a: /* push imm8 */
         src.val = insn_fetch_type(int8_t);
@@ -3404,22 +3377,25 @@ x86_emulate(
             dst.type = OP_REG;
             dst.reg  = (unsigned long *)&_regs.eax;
             dst.val  = *dst.reg;
+            dst.bytes = src.bytes;
+        imul:
             _regs.eflags &= ~(EFLG_OF|EFLG_CF);
-            switch ( dst.bytes = src.bytes )
+            switch ( dst.bytes )
             {
             case 1:
-                dst.val = ((uint16_t)(int8_t)src.val *
-                           (uint16_t)(int8_t)dst.val);
-                if ( (int8_t)dst.val != (uint16_t)dst.val )
+                dst.val = (int8_t)src.val * (int8_t)dst.val;
+                if ( (int8_t)dst.val != (int16_t)dst.val )
                     _regs.eflags |= EFLG_OF|EFLG_CF;
+                ASSERT(b > 0x6b);
                 dst.bytes = 2;
                 break;
             case 2:
                 dst.val = ((uint32_t)(int16_t)src.val *
                            (uint32_t)(int16_t)dst.val);
-                if ( (int16_t)dst.val != (uint32_t)dst.val )
+                if ( (int16_t)dst.val != (int32_t)dst.val )
                     _regs.eflags |= EFLG_OF|EFLG_CF;
-                *(uint16_t *)&_regs.edx = dst.val >> 16;
+                if ( b > 0x6b )
+                    *(uint16_t *)&_regs.edx = dst.val >> 16;
                 break;
 #ifdef __x86_64__
             case 4:
@@ -3427,14 +3403,16 @@ x86_emulate(
                            (uint64_t)(int32_t)dst.val);
                 if ( (int32_t)dst.val != dst.val )
                     _regs.eflags |= EFLG_OF|EFLG_CF;
-                _regs.edx = (uint32_t)(dst.val >> 32);
+                if ( b > 0x6b )
+                    _regs.edx = (uint32_t)(dst.val >> 32);
                 break;
 #endif
             default: {
                 unsigned long m[2] = { src.val, dst.val };
                 if ( imul_dbl(m) )
                     _regs.eflags |= EFLG_OF|EFLG_CF;
-                _regs.edx = m[1];
+                if ( b > 0x6b )
+                    _regs.edx = m[1];
                 dst.val  = m[0];
                 break;
             }
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog