target-i386: fix confusion in xcr0 bit position vs. mask

The xsave and xrstor helpers are accessing the x86_ext_save_areas array
using a bit mask instead of a bit position.  Provide two sets of XSTATE_*
definitions and use XSTATE_*_BIT when a bit position is requested.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

4 files changed, 59 insertions, 42 deletions

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0af43a3ae1..912a376b82 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -473,19 +473,26 @@ static const X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
 #undef REGISTER
 
 const ExtSaveArea x86_ext_save_areas[] = {
-    [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
+    [XSTATE_YMM_BIT] =
+          { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
             .offset = 0x240, .size = 0x100 },
-    [3] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX,
+    [XSTATE_BNDREGS_BIT] =
+          { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX,
             .offset = 0x3c0, .size = 0x40  },
-    [4] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX,
+    [XSTATE_BNDCSR_BIT] =
+          { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX,
             .offset = 0x400, .size = 0x40  },
-    [5] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
+    [XSTATE_OPMASK_BIT] =
+          { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
             .offset = 0x440, .size = 0x40 },
-    [6] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
+    [XSTATE_ZMM_Hi256_BIT] =
+          { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
             .offset = 0x480, .size = 0x200 },
-    [7] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
+    [XSTATE_Hi16_ZMM_BIT] =
+          { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
             .offset = 0x680, .size = 0x400 },
-    [9] = { .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU,
+    [XSTATE_PKRU_BIT] =
+          { .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU,
             .offset = 0xA80, .size = 0x8 },
 };
 
@@ -2483,7 +2490,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                     *ecx = MAX(*ecx, esa->offset + esa->size);
                 }
             }
-            *eax |= ena_mask & (XSTATE_FP | XSTATE_SSE);
+            *eax |= ena_mask & (XSTATE_FP_MASK | XSTATE_SSE_MASK);
             *ebx = *ecx;
         } else if (count == 1) {
             *eax = env->features[FEAT_XSAVE];
@@ -2717,15 +2724,15 @@ static void x86_cpu_reset(CPUState *s)
     cpu_watchpoint_remove_all(s, BP_CPU);
 
     cr4 = 0;
-    xcr0 = XSTATE_FP;
+    xcr0 = XSTATE_FP_MASK;
 
 #ifdef CONFIG_USER_ONLY
     /* Enable all the features for user-mode.  */
     if (env->features[FEAT_1_EDX] & CPUID_SSE) {
-        xcr0 |= XSTATE_SSE;
+        xcr0 |= XSTATE_SSE_MASK;
     }
     if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_MPX) {
-        xcr0 |= XSTATE_BNDREGS | XSTATE_BNDCSR;
+        xcr0 |= XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK;
     }
     if (env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) {
         cr4 |= CR4_OSFXSR_MASK | CR4_OSXSAVE_MASK;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 94cb4db27d..03c00d55a3 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -405,16 +405,25 @@
 #define MSR_IA32_BNDCFGS                0x00000d90
 #define MSR_IA32_XSS                    0x00000da0
 
-#define XSTATE_FP                       (1ULL << 0)
-#define XSTATE_SSE                      (1ULL << 1)
-#define XSTATE_YMM                      (1ULL << 2)
-#define XSTATE_BNDREGS                  (1ULL << 3)
-#define XSTATE_BNDCSR                   (1ULL << 4)
-#define XSTATE_OPMASK                   (1ULL << 5)
-#define XSTATE_ZMM_Hi256                (1ULL << 6)
-#define XSTATE_Hi16_ZMM                 (1ULL << 7)
-#define XSTATE_PKRU                     (1ULL << 9)
-
+#define XSTATE_FP_BIT                   0
+#define XSTATE_SSE_BIT                  1
+#define XSTATE_YMM_BIT                  2
+#define XSTATE_BNDREGS_BIT              3
+#define XSTATE_BNDCSR_BIT               4
+#define XSTATE_OPMASK_BIT               5
+#define XSTATE_ZMM_Hi256_BIT            6
+#define XSTATE_Hi16_ZMM_BIT             7
+#define XSTATE_PKRU_BIT                 9
+
+#define XSTATE_FP_MASK                  (1ULL << XSTATE_FP_BIT)
+#define XSTATE_SSE_MASK                 (1ULL << XSTATE_SSE_BIT)
+#define XSTATE_YMM_MASK                 (1ULL << XSTATE_YMM_BIT)
+#define XSTATE_BNDREGS_MASK             (1ULL << XSTATE_BNDREGS_BIT)
+#define XSTATE_BNDCSR_MASK              (1ULL << XSTATE_BNDCSR_BIT)
+#define XSTATE_OPMASK_MASK              (1ULL << XSTATE_OPMASK_BIT)
+#define XSTATE_ZMM_Hi256_MASK           (1ULL << XSTATE_ZMM_Hi256_BIT)
+#define XSTATE_Hi16_ZMM_MASK            (1ULL << XSTATE_Hi16_ZMM_BIT)
+#define XSTATE_PKRU_MASK                (1ULL << XSTATE_PKRU_BIT)
 
 /* CPUID feature words */
 typedef enum FeatureWord {
diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c
index 9dfbc4c7a6..d1a7f4cbec 100644
--- a/target-i386/fpu_helper.c
+++ b/target-i386/fpu_helper.c
@@ -1215,7 +1215,7 @@ static uint64_t get_xinuse(CPUX86State *env)
        indicate in use.  That said, the state of BNDREGS is important
        enough to track in HFLAGS, so we might as well use that here.  */
     if ((env->hflags & HF_MPX_IU_MASK) == 0) {
-       inuse &= ~XSTATE_BNDREGS;
+       inuse &= ~XSTATE_BNDREGS_MASK;
     }
     return inuse;
 }
@@ -1239,22 +1239,22 @@ static void do_xsave(CPUX86State *env, target_ulong ptr, uint64_t rfbm,
     rfbm &= env->xcr0;
     opt &= rfbm;
 
-    if (opt & XSTATE_FP) {
+    if (opt & XSTATE_FP_MASK) {
         do_xsave_fpu(env, ptr, ra);
     }
-    if (rfbm & XSTATE_SSE) {
+    if (rfbm & XSTATE_SSE_MASK) {
         /* Note that saving MXCSR is not suppressed by XSAVEOPT.  */
         do_xsave_mxcsr(env, ptr, ra);
     }
-    if (opt & XSTATE_SSE) {
+    if (opt & XSTATE_SSE_MASK) {
         do_xsave_sse(env, ptr, ra);
     }
-    if (opt & XSTATE_BNDREGS) {
-        target_ulong off = x86_ext_save_areas[XSTATE_BNDREGS].offset;
+    if (opt & XSTATE_BNDREGS_MASK) {
+        target_ulong off = x86_ext_save_areas[XSTATE_BNDREGS_BIT].offset;
         do_xsave_bndregs(env, ptr + off, ra);
     }
-    if (opt & XSTATE_BNDCSR) {
-        target_ulong off = x86_ext_save_areas[XSTATE_BNDCSR].offset;
+    if (opt & XSTATE_BNDCSR_MASK) {
+        target_ulong off = x86_ext_save_areas[XSTATE_BNDCSR_BIT].offset;
         do_xsave_bndcsr(env, ptr + off, ra);
     }
 
@@ -1399,19 +1399,19 @@ void helper_xrstor(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
         raise_exception_ra(env, EXCP0D_GPF, ra);
     }
 
-    if (rfbm & XSTATE_FP) {
-        if (xstate_bv & XSTATE_FP) {
+    if (rfbm & XSTATE_FP_MASK) {
+        if (xstate_bv & XSTATE_FP_MASK) {
             do_xrstor_fpu(env, ptr, ra);
         } else {
             helper_fninit(env);
             memset(env->fpregs, 0, sizeof(env->fpregs));
         }
     }
-    if (rfbm & XSTATE_SSE) {
+    if (rfbm & XSTATE_SSE_MASK) {
         /* Note that the standard form of XRSTOR loads MXCSR from memory
            whether or not the XSTATE_BV bit is set.  */
         do_xrstor_mxcsr(env, ptr, ra);
-        if (xstate_bv & XSTATE_SSE) {
+        if (xstate_bv & XSTATE_SSE_MASK) {
             do_xrstor_sse(env, ptr, ra);
         } else {
             /* ??? When AVX is implemented, we may have to be more
@@ -1419,9 +1419,9 @@ void helper_xrstor(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
             memset(env->xmm_regs, 0, sizeof(env->xmm_regs));
         }
     }
-    if (rfbm & XSTATE_BNDREGS) {
-        if (xstate_bv & XSTATE_BNDREGS) {
-            target_ulong off = x86_ext_save_areas[XSTATE_BNDREGS].offset;
+    if (rfbm & XSTATE_BNDREGS_MASK) {
+        if (xstate_bv & XSTATE_BNDREGS_MASK) {
+            target_ulong off = x86_ext_save_areas[XSTATE_BNDREGS_BIT].offset;
             do_xrstor_bndregs(env, ptr + off, ra);
             env->hflags |= HF_MPX_IU_MASK;
         } else {
@@ -1429,9 +1429,9 @@ void helper_xrstor(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
             env->hflags &= ~HF_MPX_IU_MASK;
         }
     }
-    if (rfbm & XSTATE_BNDCSR) {
-        if (xstate_bv & XSTATE_BNDCSR) {
-            target_ulong off = x86_ext_save_areas[XSTATE_BNDCSR].offset;
+    if (rfbm & XSTATE_BNDCSR_MASK) {
+        if (xstate_bv & XSTATE_BNDCSR_MASK) {
+            target_ulong off = x86_ext_save_areas[XSTATE_BNDCSR_BIT].offset;
             do_xrstor_bndcsr(env, ptr + off, ra);
         } else {
             memset(&env->bndcs_regs, 0, sizeof(env->bndcs_regs));
@@ -1470,7 +1470,7 @@ void helper_xsetbv(CPUX86State *env, uint32_t ecx, uint64_t mask)
     }
 
     /* Only XCR0 is defined at present; the FPU may not be disabled.  */
-    if (ecx != 0 || (mask & XSTATE_FP) == 0) {
+    if (ecx != 0 || (mask & XSTATE_FP_MASK) == 0) {
         goto do_gpf;
     }
 
@@ -1482,7 +1482,8 @@ void helper_xsetbv(CPUX86State *env, uint32_t ecx, uint64_t mask)
     }
 
     /* Disallow enabling only half of MPX.  */
-    if ((mask ^ (mask * (XSTATE_BNDCSR / XSTATE_BNDREGS))) & XSTATE_BNDCSR) {
+    if ((mask ^ (mask * (XSTATE_BNDCSR_MASK / XSTATE_BNDREGS_MASK)))
+        & XSTATE_BNDCSR_MASK) {
         goto do_gpf;
     }
 
diff --git a/target-i386/mpx_helper.c b/target-i386/mpx_helper.c
index 1bf717af05..8c6c2e7227 100644
--- a/target-i386/mpx_helper.c
+++ b/target-i386/mpx_helper.c
@@ -35,7 +35,7 @@ void cpu_sync_bndcs_hflags(CPUX86State *env)
     }
 
     if ((env->cr[4] & CR4_OSXSAVE_MASK)
-        && (env->xcr0 & XSTATE_BNDCSR)
+        && (env->xcr0 & XSTATE_BNDCSR_MASK)
         && (bndcsr & BNDCFG_ENABLE)) {
         hflags |= HF_MPX_EN_MASK;
     } else {