x86/cpuid: Only recalculate the shared feature bits once

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@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 96d6025dfbe965ad692d76e2c1936320681908c7..918590f87a97e1fae51b08656738ee2a17301799 100644 (file)
--- 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 3f398b5c3496ab34fade834c481c5ee747f5fd0a..87ec02fbcae76f74cf8508f536d28b80347fc9dd 100644 (file)
--- 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 6212e4fc4b0a0f3600667945f52d0df44650867f..5cab6db2c259221589f50e4db3063f3cb7ab4622 100755 (executable)
--- 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())