[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 3/6] x86/cpuid: Move all xstate leaf handling into guest_cpuid()



The xstate union now contains sanitised values, so it can be handled fully in
the non-legacy path.

c/s 1c0bc709d "x86/cpuid: Perform max_leaf calculations in guest_cpuid()"
accidentally introduced a boundary error for the subleaf check, although it
was masked by the correct logic in the legacy path.

Two dynamic adjustments need making, but a TODO and BUILD_BUG_ON() are left to
cover a latent bug which will present itself when Xen starts supporting XSS
states for guests.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
---
 xen/arch/x86/cpuid.c            | 179 +++++++---------------------------------
 xen/arch/x86/domctl.c           |   4 +-
 xen/include/asm-x86/processor.h |  10 +++
 3 files changed, 45 insertions(+), 148 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 303568d..9f16502 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -484,8 +484,6 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, 
struct cpuid_leaf *res)
 
     switch ( leaf )
     {
-        uint32_t tmp;
-
     case 0x00000001:
         res->c = p->basic._1c;
         res->d = p->basic._1d;
@@ -637,57 +635,6 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, 
struct cpuid_leaf *res)
             res->a = (res->a & ~0xff) | 3;
         break;
 
-    case XSTATE_CPUID:
-        if ( !p->basic.xsave || subleaf >= 63 )
-            goto unsupported;
-        switch ( subleaf )
-        {
-        case 0:
-        {
-            uint64_t xfeature_mask = XSTATE_FP_SSE;
-            uint32_t xstate_size = XSTATE_AREA_MIN_SIZE;
-
-            if ( p->basic.avx )
-            {
-                xfeature_mask |= XSTATE_YMM;
-                xstate_size = (xstate_offsets[_XSTATE_YMM] +
-                               xstate_sizes[_XSTATE_YMM]);
-            }
-
-            if ( p->feat.avx512f )
-            {
-                xfeature_mask |= XSTATE_OPMASK | XSTATE_ZMM | XSTATE_HI_ZMM;
-                xstate_size = max(xstate_size,
-                                  xstate_offsets[_XSTATE_OPMASK] +
-                                  xstate_sizes[_XSTATE_OPMASK]);
-                xstate_size = max(xstate_size,
-                                  xstate_offsets[_XSTATE_ZMM] +
-                                  xstate_sizes[_XSTATE_ZMM]);
-                xstate_size = max(xstate_size,
-                                  xstate_offsets[_XSTATE_HI_ZMM] +
-                                  xstate_sizes[_XSTATE_HI_ZMM]);
-            }
-
-            res->a = xfeature_mask;
-            res->d = xfeature_mask >> 32;
-            res->c = xstate_size;
-
-            /*
-             * Always read CPUID.0xD[ECX=0].EBX from hardware, rather than
-             * domain policy.  It varies with enabled xstate, and the correct
-             * xcr0 is in context.
-             */
-            cpuid_count(leaf, subleaf, &tmp, &res->b, &tmp, &tmp);
-            break;
-        }
-
-        case 1:
-            res->a = p->xstate.Da1;
-            res->b = res->c = res->d = 0;
-            break;
-        }
-        break;
-
     case 0x80000001:
         res->c = p->extd.e1c;
         res->d = p->extd.e1d;
@@ -730,6 +677,7 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, 
struct cpuid_leaf *res)
         break;
 
     case 0x7:
+    case XSTATE_CPUID:
         ASSERT_UNREACHABLE();
         /* Now handled in guest_cpuid(). */
     }
@@ -797,98 +745,6 @@ static void hvm_cpuid(uint32_t leaf, uint32_t subleaf, 
struct cpuid_leaf *res)
         res->d = v->vcpu_id * 2;
         break;
 
-    case XSTATE_CPUID:
-        if ( !p->basic.xsave || subleaf >= 63 )
-        {
-            *res = EMPTY_LEAF;
-            break;
-        }
-        switch ( subleaf )
-        {
-        case 0:
-        {
-            uint64_t xfeature_mask = XSTATE_FP_SSE;
-            uint32_t xstate_size = XSTATE_AREA_MIN_SIZE;
-
-            if ( p->basic.avx )
-            {
-                xfeature_mask |= XSTATE_YMM;
-                xstate_size = max(xstate_size,
-                                  xstate_offsets[_XSTATE_YMM] +
-                                  xstate_sizes[_XSTATE_YMM]);
-            }
-
-            if ( p->feat.mpx )
-            {
-                xfeature_mask |= XSTATE_BNDREGS | XSTATE_BNDCSR;
-                xstate_size = max(xstate_size,
-                                  xstate_offsets[_XSTATE_BNDCSR] +
-                                  xstate_sizes[_XSTATE_BNDCSR]);
-            }
-
-            if ( p->feat.avx512f )
-            {
-                xfeature_mask |= XSTATE_OPMASK | XSTATE_ZMM | XSTATE_HI_ZMM;
-                xstate_size = max(xstate_size,
-                                  xstate_offsets[_XSTATE_OPMASK] +
-                                  xstate_sizes[_XSTATE_OPMASK]);
-                xstate_size = max(xstate_size,
-                                  xstate_offsets[_XSTATE_ZMM] +
-                                  xstate_sizes[_XSTATE_ZMM]);
-                xstate_size = max(xstate_size,
-                                  xstate_offsets[_XSTATE_HI_ZMM] +
-                                  xstate_sizes[_XSTATE_HI_ZMM]);
-            }
-
-            if ( p->feat.pku )
-            {
-                xfeature_mask |= XSTATE_PKRU;
-                xstate_size = max(xstate_size,
-                                  xstate_offsets[_XSTATE_PKRU] +
-                                  xstate_sizes[_XSTATE_PKRU]);
-            }
-
-            if ( p->extd.lwp )
-            {
-                xfeature_mask |= XSTATE_LWP;
-                xstate_size = max(xstate_size,
-                                  xstate_offsets[_XSTATE_LWP] +
-                                  xstate_sizes[_XSTATE_LWP]);
-            }
-
-            res->a = xfeature_mask;
-            res->d = xfeature_mask >> 32;
-            res->c = xstate_size;
-
-            /*
-             * Always read CPUID[0xD,0].EBX from hardware, rather than domain
-             * policy.  It varies with enabled xstate, and the correct xcr0 is
-             * in context.
-             */
-            cpuid_count(leaf, subleaf, &tmp, &res->b, &tmp, &tmp);
-            break;
-        }
-
-        case 1:
-            res->a = p->xstate.Da1;
-
-            if ( p->xstate.xsaves )
-            {
-                /*
-                 * Always read CPUID[0xD,1].EBX from hardware, rather than
-                 * domain policy.  It varies with enabled xstate, and the
-                 * correct xcr0/xss are in context.
-                 */
-                cpuid_count(leaf, subleaf, &tmp, &res->b, &tmp, &tmp);
-            }
-            else
-                res->b = 0;
-
-            res->c = res->d = 0;
-            break;
-        }
-        break;
-
     case 0x0000000a: /* Architectural Performance Monitor Features (Intel) */
         if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL || !vpmu_enabled(v) )
         {
@@ -970,6 +826,7 @@ static void hvm_cpuid(uint32_t leaf, uint32_t subleaf, 
struct cpuid_leaf *res)
         break;
 
     case 0x7:
+    case XSTATE_CPUID:
         ASSERT_UNREACHABLE();
         /* Now handled in guest_cpuid(). */
     }
@@ -1007,10 +864,13 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
             break;
 
         case XSTATE_CPUID:
-            if ( subleaf > ARRAY_SIZE(p->xstate.raw) )
+            if ( !p->basic.xsave || subleaf >= ARRAY_SIZE(p->xstate.raw) )
                 return;
 
-            /* Fallthrough. */
+            BUG_ON(subleaf >= ARRAY_SIZE(p->xstate.raw));
+            *res = p->xstate.raw[subleaf];
+            break;
+
         default:
             goto legacy;
         }
@@ -1067,6 +927,31 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
             break;
         }
         break;
+
+    case XSTATE_CPUID:
+        switch ( subleaf )
+        {
+        case 1:
+            if ( p->xstate.xsaves )
+            {
+                /*
+                 * TODO: Figure out what to do for XSS state.  VT-x manages
+                 * host vs guest MSR_XSS automatically, so as soon as we start
+                 * supporting any XSS states, the wrong XSS will be in
+                 * context.
+                 */
+                BUILD_BUG_ON(XSTATE_XSAVES_ONLY != 0);
+
+                /*
+                 * Read CPUID[0xD,0/1].EBX from hardware.  They vary with
+                 * enabled XSTATE, and appropraite XCR0|XSS are in context.
+                 */
+        case 0:
+                res->b = cpuid_count_ebx(leaf, subleaf);
+            }
+            break;
+        }
+        break;
     }
 
     /* Done. */
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 772c5d2..8e8437d 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -105,8 +105,10 @@ static int update_domain_cpuid_info(struct domain *d,
         if ( ctl->input[0] == 7 &&
              ctl->input[1] >= ARRAY_SIZE(p->feat.raw) )
             return 0;
+
+        BUILD_BUG_ON(ARRAY_SIZE(p->xstate.raw) < 2);
         if ( ctl->input[0] == XSTATE_CPUID &&
-             ctl->input[1] >= ARRAY_SIZE(p->xstate.raw) )
+             ctl->input[1] != 1 ) /* Everything else automatically calculated. 
*/
             return 0;
         break;
 
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index b130f47..54d0a17 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -319,6 +319,16 @@ static always_inline unsigned int cpuid_edx(unsigned int 
op)
     return edx;
 }
 
+static always_inline unsigned int cpuid_count_ebx(
+    unsigned int leaf, unsigned int subleaf)
+{
+    unsigned int ebx, tmp;
+
+    cpuid_count(leaf, subleaf, &tmp, &ebx, &tmp, &tmp);
+
+    return ebx;
+}
+
 static inline unsigned long read_cr0(void)
 {
     unsigned long cr0;
-- 
2.1.4


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.