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@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
}
}
- /*
- * 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 )
{
#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, ...) \
({ \
}
}
-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 {
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();
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)
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)
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)
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)
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)
*/
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;
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;
projects / xen.git / commitdiff