tools/libs/guest/xg_cpuid_x86.c | 14 +++++++++++++- xen/arch/x86/cpu-policy.c | 16 ++++++++++++++-- xen/arch/x86/lib/cpu-policy/cpuid.c | 2 +- xen/include/public/arch-x86/cpufeatureset.h | 2 +- xen/tools/gen-cpuid.py | 20 +++++++++++++++++--- 5 files changed, 46 insertions(+), 8 deletions(-)
AMD CPUs need LKGS depend on NSCB, but this dependency cannot be unconditional
as it will break FRED on Intel CPUs which don't need to enumerate the absence
of a bug.
The deep dependecy logic is formed of two parts; a deep_features bitmap
indicating which features have dependencies, and deep_deps; the mapping of
feature to dependent features. Given that NSCB is an unconnected root of a
dependency, we can have the row in deep_deps and conditionally exclude it the
deep_features level.
Rename INIT_DEEP_FEATURES to INIT_ALL_DEEP_FEATURES and add AMD and INTEL
forms too. In both xc_cpuid_apply_policy() and sanitise_featureset(), choose
the appropriate {amd,intel}_deep_features based on vendor.
Introduce the NSCB <-> LKGS dependency and exclude the NSCB row from
intel_deep_features.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Teddy Astie <teddy.astie@vates.tech>
Only compile tested so far.
---
tools/libs/guest/xg_cpuid_x86.c | 14 +++++++++++++-
xen/arch/x86/cpu-policy.c | 16 ++++++++++++++--
xen/arch/x86/lib/cpu-policy/cpuid.c | 2 +-
xen/include/public/arch-x86/cpufeatureset.h | 2 +-
xen/tools/gen-cpuid.py | 20 +++++++++++++++++---
5 files changed, 46 insertions(+), 8 deletions(-)
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index 0db6d77cd801..0b00672c4762 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -650,9 +650,12 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
if ( featureset )
{
+ static const uint32_t amd_deep_features[] = INIT_AMD_DEEP_FEATURES;
+ static const uint32_t intel_deep_features[] = INIT_INTEL_DEEP_FEATURES;
+
+ const uint32_t *deep_features;
uint32_t disabled_features[FEATURESET_NR_ENTRIES],
feat[FEATURESET_NR_ENTRIES] = {};
- static const uint32_t deep_features[] = INIT_DEEP_FEATURES;
unsigned int i, b;
/*
@@ -670,6 +673,15 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
memcpy(feat, featureset, sizeof(*featureset) * user_len);
+ /*
+ * At the time of writing, amd_deep_features contains one extra
+ * dependency over intel for a "hardware no longer has this bug" bit.
+ */
+ if ( p->policy.x86_vendor & (X86_VENDOR_AMD|X86_VENDOR_HYGON) )
+ deep_features = amd_deep_features;
+ else
+ deep_features = intel_deep_features;
+
/* Disable deep dependencies of disabled features. */
for ( i = 0; i < ARRAY_SIZE(disabled_features); ++i )
disabled_features[i] = ~feat[i] & deep_features[i];
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 5273fe0ae435..2228c52ffc79 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -39,7 +39,8 @@ static const uint32_t __initconst hvm_shadow_def_featuremask[] =
INIT_HVM_SHADOW_DEF_FEATURES;
static const uint32_t __initconst hvm_hap_def_featuremask[] =
INIT_HVM_HAP_DEF_FEATURES;
-static const uint32_t deep_features[] = INIT_DEEP_FEATURES;
+static const uint32_t amd_deep_features[] = INIT_AMD_DEEP_FEATURES;
+static const uint32_t intel_deep_features[] = INIT_INTEL_DEEP_FEATURES;
static const struct feature_name {
const char *name;
@@ -158,11 +159,21 @@ static void zero_leaves(struct cpuid_leaf *l,
static void sanitise_featureset(uint32_t *fs)
{
+ const uint32_t *deep_features;
/* bitmap_for_each() uses unsigned longs. Extend with zeroes. */
uint32_t disabled_features[
ROUNDUP(FSCAPINTS, sizeof(unsigned long)/sizeof(uint32_t))] = {};
unsigned int i;
+ /*
+ * At the time of writing, amd_deep_features contains one extra dependency
+ * over intel for a "hardware no longer has this bug" bit.
+ */
+ if ( boot_cpu_data.vendor & (X86_VENDOR_AMD|X86_VENDOR_HYGON) )
+ deep_features = amd_deep_features;
+ else
+ deep_features = intel_deep_features;
+
for ( i = 0; i < FSCAPINTS; ++i )
{
/* Clamp to known mask. */
@@ -1110,7 +1121,8 @@ static void __init __maybe_unused build_assertions(void)
BUILD_BUG_ON(ARRAY_SIZE(pv_max_featuremask) != FSCAPINTS);
BUILD_BUG_ON(ARRAY_SIZE(hvm_shadow_max_featuremask) != FSCAPINTS);
BUILD_BUG_ON(ARRAY_SIZE(hvm_hap_max_featuremask) != FSCAPINTS);
- BUILD_BUG_ON(ARRAY_SIZE(deep_features) != FSCAPINTS);
+ BUILD_BUG_ON(ARRAY_SIZE(amd_deep_features) != FSCAPINTS);
+ BUILD_BUG_ON(ARRAY_SIZE(intel_deep_features) != FSCAPINTS);
/* Find some more clever allocation scheme if this trips. */
BUILD_BUG_ON(sizeof(struct cpu_policy) > PAGE_SIZE);
diff --git a/xen/arch/x86/lib/cpu-policy/cpuid.c b/xen/arch/x86/lib/cpu-policy/cpuid.c
index 3162e795bc21..73ea68690b4a 100644
--- a/xen/arch/x86/lib/cpu-policy/cpuid.c
+++ b/xen/arch/x86/lib/cpu-policy/cpuid.c
@@ -293,7 +293,7 @@ void x86_cpu_policy_clear_out_of_range_leaves(struct cpu_policy *p)
const uint32_t *x86_cpu_policy_lookup_deep_deps(uint32_t feature)
{
- static const uint32_t deep_features[] = INIT_DEEP_FEATURES;
+ static const uint32_t deep_features[] = INIT_ALL_DEEP_FEATURES;
static const struct {
uint32_t feature;
uint32_t fs[FEATURESET_NR_ENTRIES];
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index c4d3276f2f89..acee5a0544a0 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -326,7 +326,7 @@ XEN_CPUFEATURE(NO_NEST_BP, 11*32+ 0) /*A No Nested Data Breakpoints */
XEN_CPUFEATURE(FS_GS_NS, 11*32+ 1) /*S| FS/GS base MSRs non-serialising */
XEN_CPUFEATURE(LFENCE_DISPATCH, 11*32+ 2) /*A LFENCE always serializing */
XEN_CPUFEATURE(VERW_CLEAR, 11*32+ 5) /*!A| VERW clears microarchitectural buffers */
-XEN_CPUFEATURE(NSCB, 11*32+ 6) /*A Null Selector Clears Base (and limit too) */
+XEN_CPUFEATURE(NSCB, 11*32+ 6) /*!A Null Selector Clears Base (and limit too) */
XEN_CPUFEATURE(AUTO_IBRS, 11*32+ 8) /*S Automatic IBRS */
XEN_CPUFEATURE(AMD_FSRS, 11*32+10) /*A Fast Short REP STOSB */
XEN_CPUFEATURE(AMD_FSRC, 11*32+11) /*A Fast Short REP CMPSB */
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index 13d85a43482a..a0fff6c45676 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -344,6 +344,12 @@ def crunch_numbers(state):
# The ARCH_CAPS CPUID bit enumerates the availability of the whole register.
ARCH_CAPS: feat_range(RDCL_NO, RDCL_NO + 63),
+ # AMD-only special case. NullSelectorClearsBase is really a "hardware
+ # doesn't have this bug any more" bit. All FRED-capable hardware has
+ # NSCB properties, so disallow configurations which would cause LGKS
+ # to behave unexpectedly.
+ NSCB: [LKGS],
+
# The behaviour described by RRSBA depend on eIBRS being active.
EIBRS: [RRSBA],
@@ -383,9 +389,13 @@ def crunch_numbers(state):
state.deep_deps[feat] = seen
- state.deep_features = deps.keys()
state.nr_deep_deps = len(state.deep_deps.keys())
+ # deep_features is split per vendor to exlcude certain rows from
+ # processing.
+ state.all_deep_features = set(deps.keys())
+ state.intel_deep_features = state.all_deep_features - set((NSCB, ))
+
# Calculate the bitfield name declarations. Leave 4 placeholders on the end
for word in range(state.nr_entries + 4):
@@ -447,7 +457,10 @@ def write_results(state):
#define NR_DEEP_DEPS %sU
-#define INIT_DEEP_FEATURES { \\\n%s\n}
+#define INIT_ALL_DEEP_FEATURES { \\\n%s\n}
+
+#define INIT_AMD_DEEP_FEATURES INIT_ALL_DEEP_FEATURES
+#define INIT_INTEL_DEEP_FEATURES { \\\n%s\n}
#define INIT_DEEP_DEPS { \\
""" % (state.nr_entries,
@@ -462,7 +475,8 @@ def write_results(state):
format_uint32s(state, state.hvm_hap_def, 4),
format_uint32s(state, state.hvm_hap_max, 4),
state.nr_deep_deps,
- format_uint32s(state, state.deep_features, 4),
+ format_uint32s(state, state.all_deep_features, 4),
+ format_uint32s(state, state.intel_deep_features, 4),
))
for dep in sorted(state.deep_deps.keys()):
--
2.39.5
On 08.04.2026 18:35, Andrew Cooper wrote:
> AMD CPUs need LKGS depend on NSCB, but this dependency cannot be unconditional
> as it will break FRED on Intel CPUs which don't need to enumerate the absence
> of a bug.
>
> The deep dependecy logic is formed of two parts; a deep_features bitmap
> indicating which features have dependencies, and deep_deps; the mapping of
> feature to dependent features. Given that NSCB is an unconnected root of a
> dependency, we can have the row in deep_deps and conditionally exclude it the
> deep_features level.
>
> Rename INIT_DEEP_FEATURES to INIT_ALL_DEEP_FEATURES and add AMD and INTEL
> forms too. In both xc_cpuid_apply_policy() and sanitise_featureset(), choose
> the appropriate {amd,intel}_deep_features based on vendor.
>
> Introduce the NSCB <-> LKGS dependency and exclude the NSCB row from
> intel_deep_features.
This reads as if there was a bi-directional dependency.
> --- a/tools/libs/guest/xg_cpuid_x86.c
> +++ b/tools/libs/guest/xg_cpuid_x86.c
> @@ -650,9 +650,12 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
>
> if ( featureset )
> {
> + static const uint32_t amd_deep_features[] = INIT_AMD_DEEP_FEATURES;
> + static const uint32_t intel_deep_features[] = INIT_INTEL_DEEP_FEATURES;
Nit: One blank too many each (after '=').
> @@ -158,11 +159,21 @@ static void zero_leaves(struct cpuid_leaf *l,
>
> static void sanitise_featureset(uint32_t *fs)
> {
> + const uint32_t *deep_features;
> /* bitmap_for_each() uses unsigned longs. Extend with zeroes. */
> uint32_t disabled_features[
> ROUNDUP(FSCAPINTS, sizeof(unsigned long)/sizeof(uint32_t))] = {};
> unsigned int i;
>
> + /*
> + * At the time of writing, amd_deep_features contains one extra dependency
> + * over intel for a "hardware no longer has this bug" bit.
> + */
> + if ( boot_cpu_data.vendor & (X86_VENDOR_AMD|X86_VENDOR_HYGON) )
> + deep_features = amd_deep_features;
> + else
> + deep_features = intel_deep_features;
Aren't there going to be other somewhat similar features which may want
marking like this (no matter whether right away they have a dependency)?
Deferring the special-casing until a dependency appears is only risking
to forget to add them.
> --- a/xen/tools/gen-cpuid.py
> +++ b/xen/tools/gen-cpuid.py
> @@ -344,6 +344,12 @@ def crunch_numbers(state):
> # The ARCH_CAPS CPUID bit enumerates the availability of the whole register.
> ARCH_CAPS: feat_range(RDCL_NO, RDCL_NO + 63),
>
> + # AMD-only special case. NullSelectorClearsBase is really a "hardware
> + # doesn't have this bug any more" bit. All FRED-capable hardware has
> + # NSCB properties, so disallow configurations which would cause LGKS
> + # to behave unexpectedly.
> + NSCB: [LKGS],
Along the lines of my reply to your review comments on my LKGS patch, why
is LKGS different from other selector loads? Don't we rather need to
disallow (perhaps not physically, but verbally) the suppressing of NSCB?
And then also for a few other "features", as per above? FDP_EXCP_ONLY and
NO_FPU_SEL clearly would be of that kind, and (for possible dependencies)
AMD doesn't know of them.
Also, nit: "LKGS" (in the comment).
Jan
© 2016 - 2026 Red Hat, Inc.