|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen staging] Revert "x86/CPUID: shrink max_{,sub}leaf fields according to actual leaf contents"
commit 0e6c87b93e1d35fedf9cebd65395c2f79b4af11a
Author: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Wed Nov 24 19:06:02 2021 +0000
Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Thu Nov 25 17:14:18 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
work
will now be refused. Also external toolstacks that attempt to set the CPUID
policy from a featureset might now see some filled leaves not reachable due
to
the shrinking done to the default domain policy before applying the
featureset.
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>
---
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#staging
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |