xen-project/xen
1
0 Fork 1
mirror of https://github.com/xen-project/xen.git synced 2026-10-28 10:11:27 +01:00
Code Issues Activity

Revert "x86/CPUID: shrink max_{,sub}leaf fields according to actual leaf contents"

Browse Source 
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 540d911c28, as well as the
partial fix attempt in 81da2b544c (which added
one new case where cpuid= settings might not apply correctly) and restores the
same behaviour as Xen 4.15.

Fixes: 540d911c28 ("x86/CPUID: shrink max_{,sub}leaf fields according to actual leaf contents")
Fixes: 81da2b544c ("x86/cpuid: prevent shrinking migrated policies max leaves")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release_Acked-by: Ian Jackson <iwj@xenproject.org>
This commit is contained in:
Andrew Cooper authored and Ian Jackson committed 2021-11-25 11:06:12 +00:00
1 parent 59505f48fa
commit c4b41ee2b4
5 files changed
-164
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

tools/libs/guest/xg_cpuid_x86.c
-7
View File
@@ -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 )
{
tools/tests/cpu-policy/test-cpu-policy.c
-101
View File
@@ -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();
xen/arch/x86/cpuid.c
-10
View File
@@ -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)
xen/include/xen/lib/x86/cpuid.h
-7
View File
@@ -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;
xen/lib/x86/cpuid.c
-39
View File
@@ -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;
Reference in new issue
xen-project/xen
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.

The note is not visible to the blocked user.