[Xen-devel] [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

This avoids needing a translation table between hardware ordering and Xen's
ordering.

This also fixes a bug whereby an encoding using REX.R wasn't ignored.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>

v2:
 * Mask out REX.R, which is ignored by hardware.
 * Expose the BUILD_BUG_ON()s to the test harness, and fix its build.
---
 tools/tests/x86_emulator/x86_emulate.c | 13 +++++++++++++
 xen/arch/x86/x86_emulate/x86_emulate.c | 35 +++++++++++++++-------------------
 xen/arch/x86/x86_emulate/x86_emulate.h |  4 ++--
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/tools/tests/x86_emulator/x86_emulate.c 
b/tools/tests/x86_emulator/x86_emulate.c
index af90b6e..c46b7fc 100644
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -19,6 +19,16 @@ typedef bool bool_t;
 #define ASSERT assert
 #define ASSERT_UNREACHABLE() assert(!__LINE__)
 
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
+/* Force a compilation error if condition is true */
+#define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); })
+#define BUILD_BUG_ON_ZERO(cond) \
+    sizeof(struct { _Static_assert(!(cond), "!(" #cond ")"); })
+#else
+#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
+#define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
+#endif
+
 #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
 #define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
 
@@ -37,4 +47,7 @@ typedef bool bool_t;
 #define get_stub(stb) ((void *)((stb).addr = (uintptr_t)(stb).buf))
 #define put_stub(stb)
 
+#define __init
+#define __maybe_unused __attribute__((__unused__))
+
 #include "x86_emulate/x86_emulate.c"
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
b/xen/arch/x86/x86_emulate/x86_emulate.c
index a1821d5..295907e 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1557,22 +1557,6 @@ decode_register(
     return p;
 }
 
-#define decode_segment_failed x86_seg_tr
-static enum x86_segment
-decode_segment(uint8_t modrm_reg)
-{
-    switch ( modrm_reg )
-    {
-    case 0: return x86_seg_es;
-    case 1: return x86_seg_cs;
-    case 2: return x86_seg_ss;
-    case 3: return x86_seg_ds;
-    case 4: return x86_seg_fs;
-    case 5: return x86_seg_gs;
-    }
-    return decode_segment_failed;
-}
-
 static bool is_aligned(enum x86_segment seg, unsigned long offs,
                        unsigned int size, struct x86_emulate_ctxt *ctxt,
                        const struct x86_emulate_ops *ops)
@@ -2982,8 +2966,8 @@ x86_emulate(
         break;
 
     case 0x8c: /* mov Sreg,r/m */
-        seg = decode_segment(modrm_reg);
-        generate_exception_if(seg == decode_segment_failed, EXC_UD, -1);
+        seg = modrm_reg & 7; /* REX.R is ignored. */
+        generate_exception_if(!is_x86_user_segment(seg), EXC_UD, -1);
     store_selector:
         fail_if(ops->read_segment == NULL);
         if ( (rc = ops->read_segment(seg, &sreg, ctxt)) != 0 )
@@ -2994,8 +2978,8 @@ x86_emulate(
         break;
 
     case 0x8e: /* mov r/m,Sreg */
-        seg = decode_segment(modrm_reg);
-        generate_exception_if(seg == decode_segment_failed, EXC_UD, -1);
+        seg = modrm_reg & 7; /* REX.R is ignored. */
+        generate_exception_if(!is_x86_user_segment(seg), EXC_UD, -1);
         generate_exception_if(seg == x86_seg_cs, EXC_UD, -1);
         if ( (rc = load_seg(seg, src.val, 0, NULL, ctxt, ops)) != 0 )
             goto done;
@@ -5438,6 +5422,17 @@ x86_emulate(
 #undef override_seg
 #undef ea
 
+static void __init __maybe_unused build_assertions(void)
+{
+    /* Check the values against SReg3 encoding in opcode/ModRM bytes. */
+    BUILD_BUG_ON(x86_seg_es != 0);
+    BUILD_BUG_ON(x86_seg_cs != 1);
+    BUILD_BUG_ON(x86_seg_ss != 2);
+    BUILD_BUG_ON(x86_seg_ds != 3);
+    BUILD_BUG_ON(x86_seg_fs != 4);
+    BUILD_BUG_ON(x86_seg_gs != 5);
+}
+
 #ifdef __XEN__
 
 #include <xen/err.h>
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h 
b/xen/arch/x86/x86_emulate/x86_emulate.h
index 2b39b81..639356a 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -29,11 +29,11 @@ struct x86_emulate_ctxt;
 
 /* Comprehensive enumeration of x86 segment registers. */
 enum x86_segment {
-    /* General purpose. */
+    /* General purpose.  Matches the SReg3 encoding in opcode/ModRM bytes. */
+    x86_seg_es,
     x86_seg_cs,
     x86_seg_ss,
     x86_seg_ds,
-    x86_seg_es,
     x86_seg_fs,
     x86_seg_gs,
     /* System. */
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel