[Xen-devel] [PATCH v2 03/14] x86/cpuid: Only recalculate the shared feature bits once

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

With accurate vendor information available, the shared bits can be sorted out
during recalculation, rather than at query time in the legacy cpuid path.

This means that:
 * Duplication can be dropped from the automatically generated cpuid data.
 * The toolstack need not worry about setting them appropriately.
 * They can be dropped from the system maximum featuresets.

While editing gen-cpuid.py, reflow some comments which exceeded the expected
line length.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Reviewed-by: Doug Goldstein <cardoe@xxxxxxxxxx>
Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 tools/libxc/xc_cpuid_x86.c | 19 -------------------
 xen/arch/x86/cpuid.c       | 25 +++++--------------------
 xen/tools/gen-cpuid.py     | 29 ++++++++++-------------------
 3 files changed, 15 insertions(+), 58 deletions(-)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 96d6025..918590f 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -41,7 +41,6 @@ enum {
 #define DEF_MAX_BASE 0x0000000du
 #define DEF_MAX_INTELEXT  0x80000008u
 #define DEF_MAX_AMDEXT    0x8000001cu
-#define COMMON_1D CPUID_COMMON_1D_FEATURES
 
 int xc_get_cpu_levelling_caps(xc_interface *xch, uint32_t *caps)
 {
@@ -712,24 +711,6 @@ static void sanitise_featureset(struct cpuid_domain_info 
*info)
             disabled_features[i] &= ~dfs[i];
         }
     }
-
-    switch ( info->vendor )
-    {
-    case VENDOR_INTEL:
-        /* Intel clears the common bits in e1d. */
-        info->featureset[featureword_of(X86_FEATURE_SYSCALL)] &= ~COMMON_1D;
-        break;
-
-    case VENDOR_AMD:
-        /* AMD duplicates the common bits between 1d and e1d. */
-        info->featureset[featureword_of(X86_FEATURE_SYSCALL)] =
-            ((info->featureset[featureword_of(X86_FEATURE_FPU)] & COMMON_1D) |
-             (info->featureset[featureword_of(X86_FEATURE_SYSCALL)] & 
~COMMON_1D));
-        break;
-
-    default:
-        break;
-    }
 }
 
 int xc_cpuid_apply_policy(xc_interface *xch, domid_t domid,
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 3f398b5..87ec02f 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -67,18 +67,6 @@ static void sanitise_featureset(uint32_t *fs)
             disabled_features[j] &= ~dfs[j];
         }
     }
-
-    /*
-     * Sort out shared bits.  We are constructing a featureset which needs to
-     * be applicable to a cross-vendor case.  Intel strictly clears the common
-     * bits in e1d, while AMD strictly duplicates them.
-     *
-     * We duplicate them here to be compatible with AMD while on Intel, and
-     * rely on logic closer to the guest to make the featureset stricter if
-     * emulating Intel.
-     */
-    fs[FEATURESET_e1d] = ((fs[FEATURESET_1d]  &  CPUID_COMMON_1D_FEATURES) |
-                          (fs[FEATURESET_e1d] & ~CPUID_COMMON_1D_FEATURES));
 }
 
 static void recalculate_xstate(struct cpuid_policy *p)
@@ -169,6 +157,8 @@ static void recalculate_xstate(struct cpuid_policy *p)
  */
 static void recalculate_misc(struct cpuid_policy *p)
 {
+    p->extd.e1d &= ~CPUID_COMMON_1D_FEATURES;
+
     switch ( p->x86_vendor )
     {
     case X86_VENDOR_INTEL:
@@ -181,6 +171,8 @@ static void recalculate_misc(struct cpuid_policy *p)
         p->extd.vendor_ebx = p->basic.vendor_ebx;
         p->extd.vendor_ecx = p->basic.vendor_ecx;
         p->extd.vendor_edx = p->basic.vendor_edx;
+
+        p->extd.e1d |= p->basic._1d & CPUID_COMMON_1D_FEATURES;
         break;
     }
 }
@@ -669,10 +661,6 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, 
struct cpuid_leaf *res)
         res->c = p->extd.e1c;
         res->d = p->extd.e1d;
 
-        /* If not emulating AMD, clear the duplicated features in e1d. */
-        if ( p->x86_vendor != X86_VENDOR_AMD )
-            res->d &= ~CPUID_COMMON_1D_FEATURES;
-
         /*
          * MTRR used to unconditionally leak into PV guests.  They cannot MTRR
          * infrastructure at all, and shouldn't be able to see the feature.
@@ -792,11 +780,8 @@ static void hvm_cpuid(uint32_t leaf, uint32_t subleaf, 
struct cpuid_leaf *res)
         res->c = p->extd.e1c;
         res->d = p->extd.e1d;
 
-        /* If not emulating AMD, clear the duplicated features in e1d. */
-        if ( p->x86_vendor != X86_VENDOR_AMD )
-            res->d &= ~CPUID_COMMON_1D_FEATURES;
         /* fast-forward MSR_APIC_BASE.EN if it hasn't already been clobbered. 
*/
-        else if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
+        if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
             res->d &= ~cpufeat_bit(X86_FEATURE_APIC);
 
         /*
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index 6212e4f..5cab6db 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -129,16 +129,7 @@ def crunch_numbers(state):
     common_1d = (FPU, VME, DE, PSE, TSC, MSR, PAE, MCE, CX8, APIC,
                  MTRR, PGE, MCA, CMOV, PAT, PSE36, MMX, FXSR)
 
-    # All known features.  Duplicate the common features in e1d
-    e1d_base = SYSCALL & ~31
-    state.known = featureset_to_uint32s(
-        state.names.keys() + [ e1d_base + (x % 32) for x in common_1d ],
-        nr_entries)
-
-    # Fold common back into names
-    for f in common_1d:
-        state.names[e1d_base + (f % 32)] = "E1D_" + state.names[f]
-
+    state.known = featureset_to_uint32s(state.names.keys(), nr_entries)
     state.common_1d = featureset_to_uint32s(common_1d, 1)[0]
     state.special = featureset_to_uint32s(state.raw_special, nr_entries)
     state.pv = featureset_to_uint32s(state.raw_pv, nr_entries)
@@ -248,13 +239,14 @@ def crunch_numbers(state):
         # standard 3DNow in the earlier K6 processors.
         _3DNOW: [_3DNOWEXT],
 
-        # This is just the dependency between AVX512 and AVX2 of XSTATE 
feature flags.
-        # If want to use AVX512, AVX2 must be supported and enabled.
+        # This is just the dependency between AVX512 and AVX2 of XSTATE
+        # feature flags.  If want to use AVX512, AVX2 must be supported and
+        # enabled.
         AVX2: [AVX512F],
 
-        # AVX512F is taken to mean hardware support for EVEX encoded 
instructions,
-        # 512bit registers, and the instructions themselves. All further 
AVX512 features
-        # are built on top of AVX512F
+        # AVX512F is taken to mean hardware support for EVEX encoded
+        # instructions, 512bit registers, and the instructions themselves. All
+        # further AVX512 features are built on top of AVX512F
         AVX512F: [AVX512DQ, AVX512IFMA, AVX512PF, AVX512ER, AVX512CD,
                   AVX512BW, AVX512VL, AVX512VBMI, AVX512_4VNNIW,
                   AVX512_4FMAPS, AVX512_VPOPCNTDQ],
@@ -305,10 +297,9 @@ def crunch_numbers(state):
             if name and name[0] in "0123456789":
                 name = "_" + name
 
-            # Don't generate names for the duplicate features, or ones
-            # fast-forwarded from other state
-            if (name.startswith("E1D_") or
-                name in ("APIC", "OSXSAVE", "OSPKE")):
+            # Don't generate names for features fast-forwarded from other
+            # state
+            if name in ("APIC", "OSXSAVE", "OSPKE"):
                 name = ""
 
             names.append(name.lower())
-- 
2.1.4


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