x86emul: defer rIP-relative address calculation

By putting it after all instruction fetching has been done, we can both
simplify the existing handling of immediate operands and take care of
any future instructions allowing rIP-relative operands and getting
additional bytes fetched in x86_decode_*() (the current cases of extra
bytes getting fetched there are only for operands without ModR/M bytes,
or with them only allowing their register forms).

Similarly the new placement of truncate_ea() will take care of any
future cases of non-standard memory operands (the one existing case -
opcodes A0...A3 - are fine with and without this, as they fetch an
ad_bytes sized unsigned address anyway).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index dac057908bf446fc5d6750ba427e00f1f21d9012..a60b7aed98757121c3b088cfa6a75c512cf6799e 100644 (file)
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1929,6 +1929,7 @@ x86_decode(
     uint8_t b, d, sib, sib_index, sib_base;
     unsigned int def_op_bytes, def_ad_bytes, opcode;
     enum x86_segment override_seg = x86_seg_none;
+    bool pc_rel = false;
     int rc = X86EMUL_OKAY;
 
     memset(state, 0, sizeof(*state));
@@ -2294,7 +2295,6 @@ x86_decode(
                 ea.mem.off += insn_fetch_type(int16_t);
                 break;
             }
-            ea.mem.off = truncate_ea(ea.mem.off);
         }
         else
         {
@@ -2343,15 +2343,7 @@ x86_decode(
                 if ( (modrm_rm & 7) != 5 )
                     break;
                 ea.mem.off = insn_fetch_type(int32_t);
-                if ( !mode_64bit() )
-                    break;
-                /* Relative to RIP of next instruction. Argh! */
-                ea.mem.off += state->eip;
-                if ( (d & SrcMask) == SrcImm )
-                    ea.mem.off += (d & ByteOp) ? 1 :
-                        ((op_bytes == 8) ? 4 : op_bytes);
-                else if ( (d & SrcMask) == SrcImmByte )
-                    ea.mem.off += 1;
+                pc_rel = mode_64bit();
                 break;
             case 1:
                 ea.mem.off += insn_fetch_type(int8_t);
@@ -2360,7 +2352,6 @@ x86_decode(
                 ea.mem.off += insn_fetch_type(int32_t);
                 break;
             }
-            ea.mem.off = truncate_ea(ea.mem.off);
         }
     }
 
@@ -2425,6 +2416,14 @@ x86_decode(
         return X86EMUL_UNHANDLEABLE;
     }
 
+    if ( ea.type == OP_MEM )
+    {
+        if ( pc_rel )
+            ea.mem.off += state->eip;
+
+        ea.mem.off = truncate_ea(ea.mem.off);
+    }
+
     /*
      * Undo the operand-size override effect of prefix 66 when it was
      * determined to have another meaning.