x86/hvm: Improvements to external users of decode_register()

 * Rename to decode_gpr() to be more specific as to its purpose
 * Drop the highbyte encoding handling, as no users currently care, and it
   unlikely that future users would care.
 * Change to a static inline, returning an unsigned long pointer.

Doing so highlights that the "invalid gpr" paths in hvm_mov_{to,from}_cr()
were actually unreachable.  All callers already passed in-range GPRs, and
out-of-range GPRs would have hit the BUG() previously.

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

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 8d67851d69192b7d0b59f3215fe29e7d405090c7..18d721d10d43f5f5a9b1b8031d0c2f2e58fe946b 100644 (file)
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2061,16 +2061,9 @@ static void hvm_set_uc_mode(struct vcpu *v, bool_t is_in_uc_mode)
 int hvm_mov_to_cr(unsigned int cr, unsigned int gpr)
 {
     struct vcpu *curr = current;
-    unsigned long val, *reg;
+    unsigned long val = *decode_gpr(guest_cpu_user_regs(), gpr);
     int rc;
 
-    if ( (reg = decode_register(gpr, guest_cpu_user_regs(), 0)) == NULL )
-    {
-        gdprintk(XENLOG_ERR, "invalid gpr: %u\n", gpr);
-        goto exit_and_crash;
-    }
-
-    val = *reg;
     HVMTRACE_LONG_2D(CR_WRITE, cr, TRC_PAR_LONG(val));
     HVM_DBG_LOG(DBG_LEVEL_1, "CR%u, value = %lx", cr, val);
 
@@ -2111,13 +2104,7 @@ int hvm_mov_to_cr(unsigned int cr, unsigned int gpr)
 int hvm_mov_from_cr(unsigned int cr, unsigned int gpr)
 {
     struct vcpu *curr = current;
-    unsigned long val = 0, *reg;
-
-    if ( (reg = decode_register(gpr, guest_cpu_user_regs(), 0)) == NULL )
-    {
-        gdprintk(XENLOG_ERR, "invalid gpr: %u\n", gpr);
-        goto exit_and_crash;
-    }
+    unsigned long val = 0, *reg = decode_gpr(guest_cpu_user_regs(), gpr);
 
     switch ( cr )
     {

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 885eab372b198f8423d89af57972b2b3e10e866d..dfe97b9705ed2b47f45ed204ca6de80c0231a5ee 100644 (file)
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -347,18 +347,14 @@ enum vmx_insn_errno set_vvmcs_real_safe(const struct vcpu *v, u32 encoding,
 static unsigned long reg_read(struct cpu_user_regs *regs,
                               unsigned int index)
 {
-    unsigned long *pval = decode_register(index, regs, 0);
-
-    return *pval;
+    return *decode_gpr(regs, index);
 }
 
 static void reg_write(struct cpu_user_regs *regs,
                       unsigned int index,
                       unsigned long value)
 {
-    unsigned long *pval = decode_register(index, regs, 0);
-
-    *pval = value;
+    *decode_gpr(regs, index) = value;
 }
 
 static inline u32 __n2_pin_exec_control(struct vcpu *v)
@@ -2483,14 +2479,8 @@ int nvmx_n2_vmexit_handler(struct cpu_user_regs *regs,
             case VMX_CONTROL_REG_ACCESS_TYPE_MOV_TO_CR:
             {
                 unsigned long gp = VMX_CONTROL_REG_ACCESS_GPR(exit_qualification);
-                unsigned long *reg;
+                val = *decode_gpr(guest_cpu_user_regs(), gp);
 
-                if ( (reg = decode_register(gp, guest_cpu_user_regs(), 0)) == NULL )
-                {
-                    gdprintk(XENLOG_ERR, "invalid gpr: %lx\n", gp);
-                    break;
-                }
-                val = *reg;
                 if ( cr == 0 )
                 {
                     u64 cr0_gh_mask = get_vvmcs(v, CR0_GUEST_HOST_MASK);

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 1ffbf9c3143ff0345d1671d8feb838e2269ca32d..3aa8965269bb59b17bc5837d08cbd1ceadbdff1e 100644 (file)
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1936,7 +1936,7 @@ load_seg(
 }
 
 /* Map GPRs by ModRM encoding to their offset within struct cpu_user_regs. */
-static const uint8_t cpu_user_regs_gpr_offsets[] = {
+const uint8_t cpu_user_regs_gpr_offsets[] = {
     offsetof(struct cpu_user_regs, r(ax)),
     offsetof(struct cpu_user_regs, r(cx)),
     offsetof(struct cpu_user_regs, r(dx)),
@@ -1973,18 +1973,7 @@ decode_register(
     };
 
     if ( !highbyte_regs )
-    {
-        /* Check that the array is a power of two. */
-        BUILD_BUG_ON(ARRAY_SIZE(cpu_user_regs_gpr_offsets) &
-                     (ARRAY_SIZE(cpu_user_regs_gpr_offsets) - 1));
-
-        ASSERT(modrm_reg < ARRAY_SIZE(cpu_user_regs_gpr_offsets));
-
-        /* For safety in release builds.  Debug builds will hit the ASSERT() */
-        modrm_reg &= ARRAY_SIZE(cpu_user_regs_gpr_offsets) - 1;
-
-        return (void *)regs + cpu_user_regs_gpr_offsets[modrm_reg];
-    }
+        return decode_gpr(regs, modrm_reg);
 
     /* Check that the array is a power of two. */
     BUILD_BUG_ON(ARRAY_SIZE(byte_reg_offsets) &

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index 0c8c80ad5a79807bf6042cbd40eb60f269788ea8..ab5ef4198cccea38fc54e1e5ecff127e6def79ba 100644 (file)
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -25,6 +25,14 @@
 
 #define MAX_INST_LEN 15
 
+#if defined(__i386__)
+# define X86_NR_GPRS 8
+#elif defined(__x86_64__)
+# define X86_NR_GPRS 16
+#else
+# error Unknown compilation width
+#endif
+
 struct x86_emulate_ctxt;
 
 /*
@@ -601,14 +609,27 @@ int x86_emulate_wrapper(
 #define x86_emulate x86_emulate_wrapper
 #endif
 
+/* Map GPRs by ModRM encoding to their offset within struct cpu_user_regs. */
+extern const uint8_t cpu_user_regs_gpr_offsets[X86_NR_GPRS];
+
 /*
  * Given the 'reg' portion of a ModRM byte, and a register block, return a
  * pointer into the block that addresses the relevant register.
- * @highbyte_regs specifies whether to decode AH,CH,DH,BH.
  */
-void *
-decode_register(
-    uint8_t modrm_reg, struct cpu_user_regs *regs, int highbyte_regs);
+static inline unsigned long *decode_gpr(struct cpu_user_regs *regs,
+                                        unsigned int modrm)
+{
+    /* Check that the array is a power of two. */
+    BUILD_BUG_ON(ARRAY_SIZE(cpu_user_regs_gpr_offsets) &
+                 (ARRAY_SIZE(cpu_user_regs_gpr_offsets) - 1));
+
+    ASSERT(modrm < ARRAY_SIZE(cpu_user_regs_gpr_offsets));
+
+    /* For safety in release builds.  Debug builds will hit the ASSERT() */
+    modrm &= ARRAY_SIZE(cpu_user_regs_gpr_offsets) - 1;
+
+    return (void *)regs + cpu_user_regs_gpr_offsets[modrm];
+}
 
 /* Unhandleable read, write or instruction fetch */
 int