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

[xen stable-4.16] Revert "x86/CPUID: shrink max_{,sub}leaf fields according to actual leaf contents"



commit c4b41ee2b44ab89cfca75facbe6702376098fc56
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Wed Nov 24 21:11:52 2021 +0000
Commit:     Ian Jackson <iwj@xxxxxxxxxxxxxx>
CommitDate: Thu Nov 25 11:06:12 2021 +0000

    Revert "x86/CPUID: shrink max_{,sub}leaf fields according to actual leaf 
contents"
    
    OSSTest has identified a 3rd regression caused by this change.  Migration
    between Xen 4.15 and 4.16 on the nocera pair of machines (AMD Opteron 4133)
    fails with:
    
      xc: error: Failed to set CPUID policy: leaf 00000000, subleaf ffffffff, 
msr ffffffff (22 = Invalid argument): Internal error
      xc: error: Restore failed (22 = Invalid argument): Internal error
    
    which is a safety check to prevent resuming the guest when the CPUID data 
has
    been truncated.  The problem is caused by shrinking of the max policies, 
which
    is an ABI that needs handling compatibly between different versions of Xen.
    
    Furthermore, shrinking of the default policies also breaks things in some
    cases, because certain cpuid= settings in a VM config file which used to 
have
    an effect will now be silently discarded.
    
    This reverts commit 540d911c2813c3d8f4cdbb3f5672119e5e768a3d, as well as the
    partial fix attempt in 81da2b544cbb003a5447c9b14d275746ad22ab37 (which added
    one new case where cpuid= settings might not apply correctly) and restores 
the
    same behaviour as Xen 4.15.
    
    Fixes: 540d911c2813 ("x86/CPUID: shrink max_{,sub}leaf fields according to 
actual leaf contents")
    Fixes: 81da2b544cbb ("x86/cpuid: prevent shrinking migrated policies max 
leaves")
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
    Release_Acked-by: Ian Jackson <iwj@xxxxxxxxxxxxxx>
---
 tools/libs/guest/xg_cpuid_x86.c          |   7 ---
 tools/tests/cpu-policy/test-cpu-policy.c | 101 -------------------------------
 xen/arch/x86/cpuid.c                     |  10 ---
 xen/include/xen/lib/x86/cpuid.h          |   7 ---
 xen/lib/x86/cpuid.c                      |  39 ------------
 5 files changed, 164 deletions(-)

diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index 3ffd5f683b..198892ebdf 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -638,13 +638,6 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t 
domid, bool restore,
         }
     }
 
-    /*
-     * Do not try to shrink the policy if restoring, as that could cause
-     * guest visible changes in the maximum leaf fields.
-     */
-    if ( !restore )
-        x86_cpuid_policy_shrink_max_leaves(p);
-
     rc = x86_cpuid_copy_to_buffer(p, leaves, &nr_leaves);
     if ( rc )
     {
diff --git a/tools/tests/cpu-policy/test-cpu-policy.c 
b/tools/tests/cpu-policy/test-cpu-policy.c
index 75973298df..ed450a0997 100644
--- a/tools/tests/cpu-policy/test-cpu-policy.c
+++ b/tools/tests/cpu-policy/test-cpu-policy.c
@@ -8,13 +8,10 @@
 #include <err.h>
 
 #include <xen-tools/libs.h>
-#include <xen/asm/x86-defns.h>
 #include <xen/asm/x86-vendors.h>
 #include <xen/lib/x86/cpu-policy.h>
 #include <xen/domctl.h>
 
-#define XSTATE_FP_SSE  (X86_XCR0_FP | X86_XCR0_SSE)
-
 static unsigned int nr_failures;
 #define fail(fmt, ...)                          \
 ({                                              \
@@ -573,103 +570,6 @@ static void test_cpuid_out_of_range_clearing(void)
     }
 }
 
-static void test_cpuid_maximum_leaf_shrinking(void)
-{
-    static const struct test {
-        const char *name;
-        struct cpuid_policy p;
-    } tests[] = {
-        {
-            .name = "basic",
-            .p = {
-                /* Very basic information only. */
-                .basic.max_leaf = 1,
-                .basic.raw_fms = 0xc2,
-            },
-        },
-        {
-            .name = "cache",
-            .p = {
-                /* Cache subleaves present. */
-                .basic.max_leaf = 4,
-                .cache.subleaf[0].type = 1,
-            },
-        },
-        {
-            .name = "feat#0",
-            .p = {
-                /* Subleaf 0 only with some valid bit. */
-                .basic.max_leaf = 7,
-                .feat.max_subleaf = 0,
-                .feat.fsgsbase = 1,
-            },
-        },
-        {
-            .name = "feat#1",
-            .p = {
-                /* Subleaf 1 only with some valid bit. */
-                .basic.max_leaf = 7,
-                .feat.max_subleaf = 1,
-                .feat.avx_vnni = 1,
-            },
-        },
-        {
-            .name = "topo",
-            .p = {
-                /* Topology subleaves present. */
-                .basic.max_leaf = 0xb,
-                .topo.subleaf[0].type = 1,
-            },
-        },
-        {
-            .name = "xstate",
-            .p = {
-                /* First subleaf always valid (and then non-zero). */
-                .basic.max_leaf = 0xd,
-                .xstate.xcr0_low = XSTATE_FP_SSE,
-            },
-        },
-        {
-            .name = "extd",
-            .p = {
-                /* Commonly available information only. */
-                .extd.max_leaf = 0x80000008,
-                .extd.maxphysaddr = 0x28,
-                .extd.maxlinaddr = 0x30,
-            },
-        },
-    };
-
-    printf("Testing CPUID maximum leaf shrinking:\n");
-
-    for ( size_t i = 0; i < ARRAY_SIZE(tests); ++i )
-    {
-        const struct test *t = &tests[i];
-        struct cpuid_policy *p = memdup(&t->p);
-
-        p->basic.max_leaf = ARRAY_SIZE(p->basic.raw) - 1;
-        p->feat.max_subleaf = ARRAY_SIZE(p->feat.raw) - 1;
-        p->extd.max_leaf = 0x80000000 | (ARRAY_SIZE(p->extd.raw) - 1);
-
-        x86_cpuid_policy_shrink_max_leaves(p);
-
-        /* Check the the resulting max (sub)leaf values against expecations. */
-        if ( p->basic.max_leaf != t->p.basic.max_leaf )
-             fail("  Test %s basic fail - expected %#x, got %#x\n",
-                  t->name, t->p.basic.max_leaf, p->basic.max_leaf);
-
-        if ( p->extd.max_leaf != t->p.extd.max_leaf )
-             fail("  Test %s extd fail - expected %#x, got %#x\n",
-                  t->name, t->p.extd.max_leaf, p->extd.max_leaf);
-
-        if ( p->feat.max_subleaf != t->p.feat.max_subleaf )
-             fail("  Test %s feat fail - expected %#x, got %#x\n",
-                  t->name, t->p.feat.max_subleaf, p->feat.max_subleaf);
-
-        free(p);
-    }
-}
-
 static void test_is_compatible_success(void)
 {
     static struct test {
@@ -785,7 +685,6 @@ int main(int argc, char **argv)
     test_cpuid_serialise_success();
     test_cpuid_deserialise_failure();
     test_cpuid_out_of_range_clearing();
-    test_cpuid_maximum_leaf_shrinking();
 
     test_msr_serialise_success();
     test_msr_deserialise_failure();
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 8ac55f0806..151944f657 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -353,8 +353,6 @@ static void __init calculate_host_policy(void)
         p->extd.raw[0xa].d |= ((1u << SVM_FEATURE_VMCBCLEAN) |
                                (1u << SVM_FEATURE_TSCRATEMSR));
     }
-
-    x86_cpuid_policy_shrink_max_leaves(p);
 }
 
 static void __init guest_common_default_feature_adjustments(uint32_t *fs)
@@ -434,8 +432,6 @@ static void __init calculate_pv_max_policy(void)
     recalculate_xstate(p);
 
     p->extd.raw[0xa] = EMPTY_LEAF; /* No SVM for PV guests. */
-
-    x86_cpuid_policy_shrink_max_leaves(p);
 }
 
 static void __init calculate_pv_def_policy(void)
@@ -456,8 +452,6 @@ static void __init calculate_pv_def_policy(void)
     sanitise_featureset(pv_featureset);
     cpuid_featureset_to_policy(pv_featureset, p);
     recalculate_xstate(p);
-
-    x86_cpuid_policy_shrink_max_leaves(p);
 }
 
 static void __init calculate_hvm_max_policy(void)
@@ -523,8 +517,6 @@ static void __init calculate_hvm_max_policy(void)
     sanitise_featureset(hvm_featureset);
     cpuid_featureset_to_policy(hvm_featureset, p);
     recalculate_xstate(p);
-
-    x86_cpuid_policy_shrink_max_leaves(p);
 }
 
 static void __init calculate_hvm_def_policy(void)
@@ -549,8 +541,6 @@ static void __init calculate_hvm_def_policy(void)
     sanitise_featureset(hvm_featureset);
     cpuid_featureset_to_policy(hvm_featureset, p);
     recalculate_xstate(p);
-
-    x86_cpuid_policy_shrink_max_leaves(p);
 }
 
 void __init init_guest_cpuid(void)
diff --git a/xen/include/xen/lib/x86/cpuid.h b/xen/include/xen/lib/x86/cpuid.h
index 2300faf03e..a4d254ea96 100644
--- a/xen/include/xen/lib/x86/cpuid.h
+++ b/xen/include/xen/lib/x86/cpuid.h
@@ -386,13 +386,6 @@ void x86_cpuid_policy_fill_native(struct cpuid_policy *p);
  */
 void x86_cpuid_policy_clear_out_of_range_leaves(struct cpuid_policy *p);
 
-/**
- * Shrink max leaf/subleaf values such that the last respective valid entry
- * isn't all blank.  While permitted by the spec, such extraneous leaves may
- * provide undue "hints" to guests.
- */
-void x86_cpuid_policy_shrink_max_leaves(struct cpuid_policy *p);
-
 #ifdef __XEN__
 #include <public/arch-x86/xen.h>
 typedef XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_leaf_buffer_t;
diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c
index 1409c254c8..8eb88314f5 100644
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -236,45 +236,6 @@ void x86_cpuid_policy_clear_out_of_range_leaves(struct 
cpuid_policy *p)
                 ARRAY_SIZE(p->extd.raw) - 1);
 }
 
-void x86_cpuid_policy_shrink_max_leaves(struct cpuid_policy *p)
-{
-    unsigned int i;
-
-    p->basic.raw[0x4] = p->cache.raw[0];
-
-    for ( i = p->feat.max_subleaf; i; --i )
-        if ( p->feat.raw[i].a | p->feat.raw[i].b |
-             p->feat.raw[i].c | p->feat.raw[i].d )
-            break;
-    p->feat.max_subleaf = i;
-    p->basic.raw[0x7] = p->feat.raw[i];
-
-    p->basic.raw[0xb] = p->topo.raw[0];
-
-    /*
-     * Due to the way xstate gets handled in the hypervisor (see
-     * recalculate_xstate()) there is (for now at least) no need to fiddle
-     * with the xstate subleaves (IOW we assume they're already in consistent
-     * shape, for coming from either hardware or recalculate_xstate()).
-     */
-    p->basic.raw[0xd] = p->xstate.raw[0];
-
-    for ( i = p->basic.max_leaf; i; --i )
-        if ( p->basic.raw[i].a | p->basic.raw[i].b |
-             p->basic.raw[i].c | p->basic.raw[i].d )
-            break;
-    p->basic.max_leaf = i;
-
-    for ( i = p->extd.max_leaf & 0xffff; i; --i )
-        if ( p->extd.raw[i].a | p->extd.raw[i].b |
-             p->extd.raw[i].c | p->extd.raw[i].d )
-            break;
-    if ( i | p->extd.raw[0].b | p->extd.raw[0].c | p->extd.raw[0].d )
-        p->extd.max_leaf = 0x80000000 | i;
-    else
-        p->extd.max_leaf = 0;
-}
-
 const uint32_t *x86_cpuid_lookup_deep_deps(uint32_t feature)
 {
     static const uint32_t deep_features[] = INIT_DEEP_FEATURES;
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.16



 


Rackspace

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