* [RFC PATCH 00/11] x86 vendor check optimisations
@ 2025-11-26 16:44 Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
` (11 more replies)
0 siblings, 12 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
Hi,
I'd like some input on some rather aggressive optimisations I'm after.
This is a WIP series (so, don't pay attention to bisectability just yet) to
perform aggressive DCE based on vendor checks. I've added some bloat-o-meter
outputs for the everything-enabled vs an amd-only builds for comparison. The
results are very promising.
Please focus on patch 3, as that's the key to the optimisations.
What I'm ultimately after is what I've come to call the "single-vendor
optimisation". In doing this it's fairly easy to optimise other cases too, such
as removing unused vendors and any branches they use exclusively. Please, tell
me what you think of the approach taken by the series.
Single-vendor optimisation
==========================
- If we compile Xen for a single-vendor AND
Xen validates it always boots on such vendor AND
We remove the default CPU path AND
Xen sanitises guest CPU policies so they always match the boot vendor, THEN
It's fair game to fold all vendor checks onto true/false depending on
whether you're testing against the single vendor you compiled-in or not.
Effectively ignoring the runtime component, and allowing DCE to perform
eliminations left, right and center. The results are fairly dramatic in
places like init_speculation_mitigations(). See bloat-o-meter below.
Compiled-out-vendor elimination
===============================
- If we have all possible x86 vendors described in Kconfig AND
We compile out a vendor from Xen THEN
It should be possible to remove such vendor from every vendor check in the
system. Either we didn't boot it or we're using the default CPU ops anyway.
This causes &-checks to turn into ==- checks (more size-efficient) for
multi-vendor checks turning into single-vendor checks, or even straight-up
"false" when all vendors in the check are compiled out.
This is key to enable transparent DCE-removals in multi-vendor settings.
This allows very transparent removal of all Hygon, Centaur and Shanghai code
in the system when it's not required, as it's most typically found in branches
around common x86 code gated by a runtime vendor check.
The secret sauce is a static always_inline function that operates on the
assumption that the second argument is always a constant and the first always
a variable (as it invairably is the case with these checks). It's written
carefully to fold everything into a constant for the appropriate cases. There's
a few more optimisations in the code, like a no-vendor optimisation where the
checks always return false when comparing against a non-unknown vendor when
no explicit vendor is compiled in.
Patch 1 adds the missing vendors and the default path to Kconfig, as they are
currently absent.
Patch 2 ensures consistency between host and guest CPU policies wrt CPU vendor
Patch 3 introduces the x86_vendor_is() function. This is the key.
Patch 4 migrates the early_cpu_init() vendor switch. It's a bit tricky because
it must be done compatibly with the single-vendor optimisation, but there's
nothing complex about it.
Patch 5 is simply code removal at the Makefile level for free thanks to DCE
Patches 6 through 11 is replacements of regular checks with x86_vendor_is(),
with 6 being the one with the most dramatic effect in the diffstat.
===============================================================================
Bloat-o-meter
===============================================================================
all-vendors+default-path
========================
add/remove: 0/1 grow/shrink: 12/10 up/down: 175/-266 (-91)
Function old new delta
start_vmx 1507 1582 +75
x86_cpu_policies_are_compatible 157 194 +37
xen_config_data 1479 1506 +27
early_cpu_init 948 958 +10
setup_apic_nmi_watchdog 977 986 +9
init_speculation_mitigations 9836 9841 +5
intel_mcheck_init 2398 2401 +3
set_cx_pminfo 1691 1693 +2
init_bsp_APIC 193 195 +2
guest_common_max_feature_adjustments 110 112 +2
disable_lapic_nmi_watchdog 119 121 +2
do_get_hw_residencies 1289 1290 +1
mce_firstbank 37 36 -1
mcheck_init 1227 1225 -2
hvm_vcpu_virtual_to_linear 631 628 -3
init_nonfatal_mce_checker 160 156 -4
domain_cpu_policy_changed 677 672 -5
recalculate_misc 898 890 -8
traps_init 543 527 -16
default_cpu 16 - -16
cpufreq_driver_init 468 441 -27
vmce_wrmsr 993 909 -84
vmce_rdmsr 1134 1034 -100
Total: Before=3726243, After=3726152, chg -0.00%
amd-only+no-default-path
========================
add/remove: 0/14 grow/shrink: 4/58 up/down: 93/-10948 (-10855)
Function old new delta
x86_cpu_policies_are_compatible 157 194 +37
amd_check_entrysign 807 829 +22
init_guest_cpu_policies 1364 1382 +18
xen_config_data 1471 1487 +16
opt_gds_mit 1 - -1
nmi_p6_event_width 4 - -4
nmi_p4_cccr_val 4 - -4
init_e820 1037 1033 -4
pci_cfg_ok 307 301 -6
get_hw_residencies 213 205 -8
recalculate_cpuid_policy 909 900 -9
init_amd 2508 2499 -9
dom0_setup_permissions 3809 3800 -9
arch_ioreq_server_get_type_addr 250 241 -9
cpu_has_amd_erratum 230 219 -11
parse_spec_ctrl 2321 2307 -14
amd_nonfatal_mcheck_init 192 177 -15
shanghai_cpu_dev 16 - -16
hygon_cpu_dev 16 - -16
default_init 16 - -16
default_cpu 16 - -16
centaur_cpu_dev 16 - -16
x86emul_0fae 2758 2741 -17
vmce_init_vcpu 153 136 -17
cpufreq_cpu_init 34 15 -19
nmi_watchdog_tick 534 514 -20
vmce_restore_vcpu 160 139 -21
init_nonfatal_mce_checker 142 120 -22
ucode_update_hcall_cont 888 865 -23
mce_firstbank 37 10 -27
init_shanghai 29 - -29
validate_gl4e 617 587 -30
l4e_propagate_from_guest 451 421 -30
guest_walk_tables_4_levels 3411 3381 -30
clear_msr_range 30 - -30
acpi_dead_idle 430 398 -32
print_mtrr_state 719 684 -35
amd_mcheck_init 451 416 -35
hvm_vcpu_virtual_to_linear 631 595 -36
do_IRQ 1783 1747 -36
init_bsp_APIC 193 149 -44
cpu_callback 4650 4600 -50
mc_memerr_dhandler 903 851 -52
mcheck_init 1187 1122 -65
microcode_nmi_callback 205 139 -66
disable_lapic_nmi_watchdog 119 49 -70
__start_xen 9448 9378 -70
alternative_instructions 154 82 -72
traps_init 543 468 -75
protmode_load_seg 1904 1829 -75
set_cx_pminfo 1691 1614 -77
init_intel_cacheinfo 1191 1111 -80
is_cpu_primary 93 - -93
do_mca 3181 3085 -96
guest_cpuid 2395 2292 -103
guest_common_max_feature_adjustments 110 - -110
read_msr 1431 1319 -112
x86emul_decode 12729 12597 -132
guest_common_default_feature_adjustments 232 62 -170
do_microcode_update 787 602 -185
cpufreq_driver_init 453 263 -190
vmce_wrmsr 967 768 -199
recalculate_misc 898 689 -209
vmce_rdmsr 1083 872 -211
early_cpu_init 948 732 -216
guest_wrmsr 2853 2622 -231
init_centaur 238 - -238
domain_cpu_policy_changed 677 408 -269
write_msr 1749 1465 -284
x86_emulate 222198 221891 -307
init_hygon 389 - -389
start_vmx 1507 1105 -402
guest_rdmsr 2308 1881 -427
setup_apic_nmi_watchdog 977 276 -701
do_get_hw_residencies 1289 9 -1280
init_speculation_mitigations 9714 6788 -2926
Total: Before=3679044, After=3668189, chg -0.30%
There's a few more patches needed to add conditional inclusion of amd.c and
intel.c at the Makefile level, but that can be done just as well. It just adds
5 patches worth of noise I don't want to discuss atm.
Just knowing x86_vendor_is() is "good to have" is good enough as it enables our
downstream to customise it with whatever optimisations we need.
I also suspect other areas of the hypervisor could benefit from this meld of
runtime+compiletime sort of checking, allowing transparent code removal.
I'm thinking DOM0LESS_BOOT vs DOM0_BOOT vs PVSHIM_BOOT, or AMD_SVM vs INTEL_VMX
in HVM-only builds, or family checks to have (i.e) a explicit "older-than-zen"
Kconfig option with a similar approach on a family range check.
This is maybe one of several such uses.
So... thoughts? I'm definitely fond of the single-vendor bloat-o-meter output.
Cheers,
Alejandro
Alejandro Vallejo (11):
x86: Add more granularity to the vendors in Kconfig
x86: Reject CPU policies with vendors other than the host's
x86: Add x86_vendor_is() by itself before using it
x86: Refactor early vendor lookup code to use x86_vendor_is()
x86: Conditionalise the inclusion of Hygon/Centaur/Shanghai cpu/ files
x86: Migrate switch-based vendor checks to x86_vendor_is()
x86: Migrate MSR handler vendor checks to x86_vendor_is()
x86: Migrate insn emulator to use x86_vendor_is()
x86: Migrate spec_ctrl vendor checks to x86_vendor_is()
x86: Migrate everything under cpu/ to use x86_vendor_is()
x86: Migrate every remaining raw vendor check to x86_vendor_is()
xen/arch/x86/Kconfig.cpu | 45 +++++++++++++++++++++
xen/arch/x86/acpi/cpu_idle.c | 19 ++++-----
xen/arch/x86/acpi/cpufreq/acpi.c | 2 +-
xen/arch/x86/acpi/cpufreq/cpufreq.c | 32 +++++----------
xen/arch/x86/alternative.c | 30 ++++++--------
xen/arch/x86/apic.c | 2 +-
xen/arch/x86/cpu-policy.c | 41 +++++++++----------
xen/arch/x86/cpu/Makefile | 6 +--
xen/arch/x86/cpu/amd.c | 6 +--
xen/arch/x86/cpu/common.c | 50 +++++++++++++++--------
xen/arch/x86/cpu/intel_cacheinfo.c | 5 +--
xen/arch/x86/cpu/mcheck/amd_nonfatal.c | 2 +-
xen/arch/x86/cpu/mcheck/mcaction.c | 3 +-
xen/arch/x86/cpu/mcheck/mce.c | 41 +++++--------------
xen/arch/x86/cpu/mcheck/mce.h | 20 +++++----
xen/arch/x86/cpu/mcheck/mce_amd.c | 6 +--
xen/arch/x86/cpu/mcheck/mce_intel.c | 6 +--
xen/arch/x86/cpu/mcheck/non-fatal.c | 20 +++------
xen/arch/x86/cpu/mcheck/vmce.c | 50 ++++++-----------------
xen/arch/x86/cpu/microcode/amd.c | 2 +-
xen/arch/x86/cpu/microcode/core.c | 2 +-
xen/arch/x86/cpu/mtrr/generic.c | 4 +-
xen/arch/x86/cpu/mwait-idle.c | 4 +-
xen/arch/x86/cpuid.c | 4 +-
xen/arch/x86/dom0_build.c | 3 +-
xen/arch/x86/domain.c | 37 ++++++++---------
xen/arch/x86/e820.c | 3 +-
xen/arch/x86/guest/xen/xen.c | 19 ++++-----
xen/arch/x86/hvm/hvm.c | 5 ++-
xen/arch/x86/hvm/ioreq.c | 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 6 +--
xen/arch/x86/include/asm/cpuid.h | 56 +++++++++++++++++++++++++-
xen/arch/x86/include/asm/guest_pt.h | 4 +-
xen/arch/x86/irq.c | 4 +-
xen/arch/x86/msr.c | 41 ++++++++++---------
xen/arch/x86/nmi.c | 18 +++------
xen/arch/x86/pv/emul-priv-op.c | 24 +++++------
xen/arch/x86/setup.c | 2 +-
xen/arch/x86/spec_ctrl.c | 34 ++++++++--------
xen/arch/x86/traps-setup.c | 18 ++++-----
xen/arch/x86/x86_emulate/private.h | 4 +-
xen/arch/x86/x86_emulate/x86_emulate.c | 2 +-
xen/lib/x86/policy.c | 3 +-
43 files changed, 360 insertions(+), 327 deletions(-)
base-commit: fb0e37df71a31318c61e0715ffed3e149ca8a4aa
--
2.43.0
^ permalink raw reply [flat|nested] 37+ messages in thread
* [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
@ 2025-11-26 16:44 ` Alejandro Vallejo
2025-11-27 9:43 ` Jan Beulich
` (2 more replies)
2025-11-26 16:44 ` [RFC PATCH 02/11] x86: Reject CPU policies with vendors other than the host's Alejandro Vallejo
` (10 subsequent siblings)
11 siblings, 3 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
This enables very aggressive DCE passes on single-vendor builds in later
patches, as it will allow most vendor checks to become statically chosen
branches. A lot of statics go away and a lot more inlining is allowed.
In order to allow x86_vendor_is() to fold into constants, expand Kconfig
to have the full set of vendors. Adds Hygon, Centaur, Shanghai and the
default path.
Have Hygon depend on AMD, and Centaur+Shanghai depend on Intel.
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/Kconfig.cpu | 45 ++++++++++++++++++++++++++++++++
xen/arch/x86/cpu/common.c | 17 +++++++-----
xen/arch/x86/include/asm/cpuid.h | 7 +++++
3 files changed, 62 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu
index 5fb18db1aa..aaf70fb37b 100644
--- a/xen/arch/x86/Kconfig.cpu
+++ b/xen/arch/x86/Kconfig.cpu
@@ -19,4 +19,49 @@ config INTEL
May be turned off in builds targetting other vendors. Otherwise,
must be enabled for Xen to work suitably on Intel platforms.
+config HYGON
+ bool "Support Hygon CPUs"
+ depends on AMD
+ default y
+ help
+ Detection, tunings and quirks for Hygon platforms.
+
+ May be turned off in builds targetting other vendors. Otherwise,
+ must be enabled for Xen to work suitably on Hygon platforms.
+
+
+config CENTAUR
+ bool "Support Centaur CPUs"
+ depends on INTEL
+ default y
+ help
+ Detection, tunings and quirks for Centaur platforms.
+
+ May be turned off in builds targetting other vendors. Otherwise,
+ must be enabled for Xen to work suitably on Centaur platforms.
+
+config SHANGHAI
+ bool "Support Shanghai CPUs"
+ depends on INTEL
+ default y
+ help
+ Detection, tunings and quirks for Shanghai platforms.
+
+ May be turned off in builds targetting other vendors. Otherwise,
+ must be enabled for Xen to work suitably on Shanghai platforms.
+
+config UNKNOWN_CPU
+ bool "Support unknown CPUs"
+ default y
+ help
+ This option prevents a panic on boot when the host CPU vendor isn't
+ supported by going into a legacy compatibility mode and not applying
+ any relevant tunings or quirks.
+
+ Not selecting this options while selecting multiple vendors doesn't have
+ any major effect on code size, but while selecting a single vendor
+ it produces a smaller build especially optimised for size.
+
+ If unsure, say Y.
+
endmenu
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 37820a3a08..393c30227f 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -118,7 +118,7 @@ static void cf_check default_init(struct cpuinfo_x86 * c)
__clear_bit(X86_FEATURE_SEP, c->x86_capability);
}
-static const struct cpu_dev __initconst_cf_clobber __used default_cpu = {
+static const struct cpu_dev __initconst_cf_clobber default_cpu = {
.c_init = default_init,
};
static struct cpu_dev __ro_after_init actual_cpu;
@@ -340,7 +340,8 @@ void __init early_cpu_init(bool verbose)
*(u32 *)&c->x86_vendor_id[8] = ecx;
*(u32 *)&c->x86_vendor_id[4] = edx;
- c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
+ c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx) &
+ X86_ENABLED_VENDORS;
switch (c->x86_vendor) {
case X86_VENDOR_INTEL: intel_unlock_cpuid_leaves(c);
actual_cpu = intel_cpu_dev; break;
@@ -349,12 +350,14 @@ void __init early_cpu_init(bool verbose)
case X86_VENDOR_SHANGHAI: actual_cpu = shanghai_cpu_dev; break;
case X86_VENDOR_HYGON: actual_cpu = hygon_cpu_dev; break;
default:
+ if (verbose || !IS_ENABLED(CONFIG_UNKNOWN_CPU))
+ printk(XENLOG_ERR
+ "Unrecognised or unsupported CPU vendor '%.12s'\n",
+ c->x86_vendor_id);
+ if (!IS_ENABLED(CONFIG_UNKNOWN_CPU))
+ panic("Cannot run in unknown/compiled-out CPU vendor.\n");
+
actual_cpu = default_cpu;
- if (!verbose)
- break;
- printk(XENLOG_ERR
- "Unrecognised or unsupported CPU vendor '%.12s'\n",
- c->x86_vendor_id);
}
cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
diff --git a/xen/arch/x86/include/asm/cpuid.h b/xen/arch/x86/include/asm/cpuid.h
index f1b9e37a42..bf1c635cdd 100644
--- a/xen/arch/x86/include/asm/cpuid.h
+++ b/xen/arch/x86/include/asm/cpuid.h
@@ -49,6 +49,13 @@ struct cpuid_leaf;
void guest_cpuid(const struct vcpu *v, uint32_t leaf,
uint32_t subleaf, struct cpuid_leaf *res);
+#define X86_ENABLED_VENDORS \
+ ((IS_ENABLED(CONFIG_INTEL) ? X86_VENDOR_INTEL : 0) | \
+ (IS_ENABLED(CONFIG_AMD) ? X86_VENDOR_AMD : 0) | \
+ (IS_ENABLED(CONFIG_CENTAUR) ? X86_VENDOR_CENTAUR : 0) | \
+ (IS_ENABLED(CONFIG_SHANGHAI) ? X86_VENDOR_SHANGHAI : 0) | \
+ (IS_ENABLED(CONFIG_HYGON) ? X86_VENDOR_HYGON : 0))
+
#endif /* !__X86_CPUID_H__ */
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [RFC PATCH 02/11] x86: Reject CPU policies with vendors other than the host's
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
@ 2025-11-26 16:44 ` Alejandro Vallejo
2025-11-27 20:15 ` Andrew Cooper
2025-11-26 16:44 ` [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it Alejandro Vallejo
` (9 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
While in principle it's possible to have a vendor virtualising another,
this is fairly tricky in practice. Not doing so enables certain
optimisations with regards to vendor checks in later patches.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
I originally had a Kconfig option to allow cross-vendor virt and
conditionally disable the check on policy compatibility. In practice,
I suspect there's 0% of people that would want that, so I decided to
simply remove it altogether. Happy to put it back if there's anyone
interested.
---
xen/lib/x86/policy.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/lib/x86/policy.c b/xen/lib/x86/policy.c
index f033d22785..4c0c5386ea 100644
--- a/xen/lib/x86/policy.c
+++ b/xen/lib/x86/policy.c
@@ -15,7 +15,8 @@ int x86_cpu_policies_are_compatible(const struct cpu_policy *host,
#define FAIL_MSR(m) \
do { e.msr = (m); goto out; } while ( 0 )
- if ( guest->basic.max_leaf > host->basic.max_leaf )
+ if ( (guest->basic.max_leaf > host->basic.max_leaf) ||
+ (guest->x86_vendor != host->x86_vendor) )
FAIL_CPUID(0, NA);
if ( guest->feat.max_subleaf > host->feat.max_subleaf )
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 02/11] x86: Reject CPU policies with vendors other than the host's Alejandro Vallejo
@ 2025-11-26 16:44 ` Alejandro Vallejo
2025-11-27 10:46 ` Jan Beulich
2025-11-26 16:44 ` [RFC PATCH 04/11] x86: Refactor early vendor lookup code to use x86_vendor_is() Alejandro Vallejo
` (8 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
This function is meant to replace all instances of the following
patterns in CPU policies and boot_cpu_data:
- x->x86_vendor == X86_VENDOR_FOO
- x->x86_vendor != X86_VENDOR_FOO
- x->x86_vendor & (X86_VENDOR_FOO | X86_VENDOR_BAR)
The secret sauce is that all branches inside the helper resolve at
compile time, so for the all-vendors-compiled-in case the function
resolves to equivalent code as that without the helper and you get
progressively more aggressive DCE as you disable vendors. The function
folds into a constant once you remove the fallback CPU vendor setting.
While at this, move an include out of place so they sort alphabetically.
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/include/asm/cpuid.h | 49 +++++++++++++++++++++++++++++++-
1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/include/asm/cpuid.h b/xen/arch/x86/include/asm/cpuid.h
index bf1c635cdd..a4280d1b0d 100644
--- a/xen/arch/x86/include/asm/cpuid.h
+++ b/xen/arch/x86/include/asm/cpuid.h
@@ -2,10 +2,12 @@
#define __X86_CPUID_H__
#include <asm/cpufeatureset.h>
+#include <asm/x86-vendors.h>
-#include <xen/types.h>
+#include <xen/compiler.h>
#include <xen/kernel.h>
#include <xen/percpu.h>
+#include <xen/types.h>
#include <public/sysctl.h>
@@ -56,6 +58,51 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
(IS_ENABLED(CONFIG_SHANGHAI) ? X86_VENDOR_SHANGHAI : 0) | \
(IS_ENABLED(CONFIG_HYGON) ? X86_VENDOR_HYGON : 0))
+/*
+ * When compiling Xen for a single vendor with no fallback vendor there's no
+ * need no check the candidate. `vendor` is always a compile-time constant,
+ * which means this all can fold into a constant boolean.
+ *
+ * A runtime check at the time of CPUID probing guarantees we never run on
+ * wrong hardware and another check when loading CPU policies guarantees we
+ * never run policies for a vendor in another vendor's silicon.
+ *
+ * By the same token, the same folding can happen when no vendor is compiled
+ * in and the fallback path is present.
+ */
+static always_inline bool x86_vendor_is(uint8_t candidate, uint8_t vendor)
+{
+ uint8_t filtered_vendor = vendor & X86_ENABLED_VENDORS;
+
+ if ( vendor == X86_VENDOR_UNKNOWN )
+ {
+ if ( IS_ENABLED(CONFIG_UNKNOWN_CPU) )
+ /* no-vendor optimisation */
+ return X86_ENABLED_VENDORS ? vendor == candidate : true;
+
+ /* unknown-vendor-elimination optimisation */
+ return false;
+ }
+
+ /* single-vendor optimisation */
+ if ( !IS_ENABLED(CONFIG_UNKNOWN_CPU) &&
+ (ISOLATE_LSB(X86_ENABLED_VENDORS) == X86_ENABLED_VENDORS) )
+ return filtered_vendor == X86_ENABLED_VENDORS;
+
+ /* compiled-out-vendor-elimination optimisation */
+ if ( !filtered_vendor )
+ return false;
+
+ /*
+ * When checking against a single vendor, perform an equality check, as
+ * it yields (marginally) better codegen
+ */
+ if ( ISOLATE_LSB(filtered_vendor) == filtered_vendor )
+ return filtered_vendor == candidate ;
+
+ return filtered_vendor & candidate;
+}
+
#endif /* !__X86_CPUID_H__ */
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [RFC PATCH 04/11] x86: Refactor early vendor lookup code to use x86_vendor_is()
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
` (2 preceding siblings ...)
2025-11-26 16:44 ` [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it Alejandro Vallejo
@ 2025-11-26 16:44 ` Alejandro Vallejo
2025-11-27 10:51 ` Jan Beulich
2025-11-26 16:44 ` [RFC PATCH 05/11] x86: Conditionalise the inclusion of Hygon/Centaur/Shanghai cpu/ files Alejandro Vallejo
` (7 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
Move the "unknown" vendor ahead of all others and have it NOT rely
on x86_vendor_is(), as that would yield incorrect values for the
single-vendor+no-fallback case when running in incorrect hardware
(because x86_vendor_is() becomes a folded constant of the single
compiled-in vendor).
This is one of the two places where x86_vendor_is() cannot be used,
along with the compatibility check on loaded guest CPU policies.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/cpu/common.c | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 393c30227f..c0c3606dd2 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -342,23 +342,38 @@ void __init early_cpu_init(bool verbose)
c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx) &
X86_ENABLED_VENDORS;
- switch (c->x86_vendor) {
- case X86_VENDOR_INTEL: intel_unlock_cpuid_leaves(c);
- actual_cpu = intel_cpu_dev; break;
- case X86_VENDOR_AMD: actual_cpu = amd_cpu_dev; break;
- case X86_VENDOR_CENTAUR: actual_cpu = centaur_cpu_dev; break;
- case X86_VENDOR_SHANGHAI: actual_cpu = shanghai_cpu_dev; break;
- case X86_VENDOR_HYGON: actual_cpu = hygon_cpu_dev; break;
- default:
+
+ /*
+ * We can't rely on x86_vendor_is() here due to the single-vendor
+ * optimisation. It makes x86_vendor_is(x, y) rely on the constant `y`
+ * matching the single vendor Xen was compiled for and ignore the
+ * runtime variable `x`. In order to preserve sanity we must assert here
+ * that we never boot such a build in a CPU from another vendor, or
+ * major chaos would ensue.
+ */
+ if (c->x86_vendor == X86_VENDOR_UNKNOWN)
+ {
if (verbose || !IS_ENABLED(CONFIG_UNKNOWN_CPU))
printk(XENLOG_ERR
"Unrecognised or unsupported CPU vendor '%.12s'\n",
c->x86_vendor_id);
+
if (!IS_ENABLED(CONFIG_UNKNOWN_CPU))
panic("Cannot run in unknown/compiled-out CPU vendor.\n");
actual_cpu = default_cpu;
}
+ else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_INTEL)) {
+ intel_unlock_cpuid_leaves(c);
+ actual_cpu = intel_cpu_dev;
+ } else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD))
+ actual_cpu = amd_cpu_dev;
+ else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_CENTAUR))
+ actual_cpu = centaur_cpu_dev;
+ else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_SHANGHAI))
+ actual_cpu = shanghai_cpu_dev;
+ else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_HYGON))
+ actual_cpu = hygon_cpu_dev;
cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
c->x86 = get_cpu_family(eax, &c->x86_model, &c->x86_mask);
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [RFC PATCH 05/11] x86: Conditionalise the inclusion of Hygon/Centaur/Shanghai cpu/ files
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
` (3 preceding siblings ...)
2025-11-26 16:44 ` [RFC PATCH 04/11] x86: Refactor early vendor lookup code to use x86_vendor_is() Alejandro Vallejo
@ 2025-11-26 16:44 ` Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 06/11] x86: Migrate switch-based vendor checks to x86_vendor_is() Alejandro Vallejo
` (6 subsequent siblings)
11 siblings, 0 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
x86_vendor_is() is doing the elimination now, so we can just drop them
when the Kconfig options aren't set.
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/cpu/Makefile | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
index 7cfe28b7ec..84b060aa41 100644
--- a/xen/arch/x86/cpu/Makefile
+++ b/xen/arch/x86/cpu/Makefile
@@ -3,13 +3,13 @@ obj-y += microcode/
obj-y += mtrr/
obj-y += amd.o
-obj-y += centaur.o
+obj-$(CONFIG_CENTAUR) += centaur.o
obj-y += common.o
-obj-y += hygon.o
obj-y += intel.o
+obj-$(CONFIG_HYGON) += hygon.o
obj-y += intel_cacheinfo.o
obj-$(CONFIG_INTEL) += mwait-idle.o
-obj-y += shanghai.o
+obj-$(CONFIG_SHANGHAI) += shanghai.o
obj-y += vpmu.o
obj-$(CONFIG_AMD) += vpmu_amd.o
obj-$(CONFIG_INTEL) += vpmu_intel.o
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [RFC PATCH 06/11] x86: Migrate switch-based vendor checks to x86_vendor_is()
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
` (4 preceding siblings ...)
2025-11-26 16:44 ` [RFC PATCH 05/11] x86: Conditionalise the inclusion of Hygon/Centaur/Shanghai cpu/ files Alejandro Vallejo
@ 2025-11-26 16:44 ` Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 07/11] x86: Migrate MSR handler " Alejandro Vallejo
` (5 subsequent siblings)
11 siblings, 0 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
This enables each branch to be optimised-out when the vendor isn't
enabled in Kconfig and combines N checks into 1 for the strict fallthrough
cases.
Plus, the diffstat looks fantastic and we save tons of vertical space.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/acpi/cpufreq/cpufreq.c | 28 ++++++------------
xen/arch/x86/alternative.c | 30 ++++++++-----------
xen/arch/x86/cpu-policy.c | 31 ++++++++-----------
xen/arch/x86/cpu/mcheck/mce.c | 27 +++--------------
xen/arch/x86/cpu/mcheck/mce.h | 20 ++++++-------
xen/arch/x86/cpu/mcheck/non-fatal.c | 20 ++++---------
xen/arch/x86/cpu/mcheck/vmce.c | 46 +++++++----------------------
xen/arch/x86/domain.c | 12 ++++----
xen/arch/x86/guest/xen/xen.c | 19 +++++-------
xen/arch/x86/nmi.c | 18 ++++-------
xen/arch/x86/traps-setup.c | 18 +++++------
11 files changed, 87 insertions(+), 182 deletions(-)
diff --git a/xen/arch/x86/acpi/cpufreq/cpufreq.c b/xen/arch/x86/acpi/cpufreq/cpufreq.c
index 94e8e11c15..89e2b3d167 100644
--- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
+++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
@@ -135,20 +135,17 @@ static int __init cf_check cpufreq_driver_init(void)
ret = -ENOENT;
- switch ( boot_cpu_data.x86_vendor )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
{
- case X86_VENDOR_INTEL:
for ( i = 0; i < cpufreq_xen_cnt; i++ )
{
switch ( cpufreq_xen_opts[i] )
{
case CPUFREQ_xen:
- ret = IS_ENABLED(CONFIG_INTEL) ?
- acpi_cpufreq_register() : -ENODEV;
+ ret = acpi_cpufreq_register();
break;
case CPUFREQ_hwp:
- ret = IS_ENABLED(CONFIG_INTEL) ?
- hwp_register_driver() : -ENODEV;
+ ret = hwp_register_driver();
break;
case CPUFREQ_none:
ret = 0;
@@ -163,11 +160,10 @@ static int __init cf_check cpufreq_driver_init(void)
if ( !ret || ret == -EBUSY )
break;
}
- break;
-
- case X86_VENDOR_AMD:
- case X86_VENDOR_HYGON:
-#ifdef CONFIG_AMD
+ }
+ else if ( x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ {
for ( i = 0; i < cpufreq_xen_cnt; i++ )
{
switch ( cpufreq_xen_opts[i] )
@@ -193,15 +189,9 @@ static int __init cf_check cpufreq_driver_init(void)
if ( !ret || ret == -EBUSY )
break;
}
-#else
- ret = -ENODEV;
-#endif /* CONFIG_AMD */
- break;
-
- default:
- printk(XENLOG_ERR "Cpufreq: unsupported x86 vendor\n");
- break;
}
+ else
+ printk(XENLOG_ERR "Cpufreq: unsupported x86 vendor\n");
/*
* After successful cpufreq driver registeration, XEN_PROCESSOR_PM_CPPC
diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index 9f844241bc..071871c242 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -89,32 +89,28 @@ static bool init_or_livepatch_read_mostly toolchain_nops_are_ideal;
static void __init arch_init_ideal_nops(void)
{
- switch ( boot_cpu_data.x86_vendor )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
{
- case X86_VENDOR_INTEL:
/*
* Due to a decoder implementation quirk, some specific Intel CPUs
* actually perform better with the "k8_nops" than with the SDM-
* recommended NOPs.
*/
- if ( boot_cpu_data.x86 != 6 )
- break;
-
- switch ( boot_cpu_data.x86_model )
+ if ( boot_cpu_data.x86 == 6 )
{
- case 0x0f ... 0x1b:
- case 0x1d ... 0x25:
- case 0x28 ... 0x2f:
- ideal_nops = k8_nops;
- break;
+ switch ( boot_cpu_data.x86_model )
+ {
+ case 0x0f ... 0x1b:
+ case 0x1d ... 0x25:
+ case 0x28 ... 0x2f:
+ ideal_nops = k8_nops;
+ break;
+ }
}
- break;
-
- case X86_VENDOR_AMD:
- if ( boot_cpu_data.x86 <= 0xf )
- ideal_nops = k8_nops;
- break;
}
+ else if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD) &&
+ (boot_cpu_data.x86 <= 0xf) )
+ ideal_nops = k8_nops;
#ifdef HAVE_AS_NOPS_DIRECTIVE
if ( memcmp(ideal_nops[ASM_NOP_MAX], toolchain_nops, ASM_NOP_MAX) == 0 )
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 0a7ef15f72..1acd7c5124 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -279,9 +279,8 @@ static void recalculate_misc(struct cpu_policy *p)
p->extd.raw[0x8].d = 0;
- switch ( p->x86_vendor )
+ if ( x86_vendor_is(p->x86_vendor, X86_VENDOR_INTEL) )
{
- case X86_VENDOR_INTEL:
p->basic.l2_nr_queries = 1; /* Fixed to 1 query. */
p->basic.raw[0x3] = EMPTY_LEAF; /* PSN - always hidden. */
p->basic.raw[0x9] = EMPTY_LEAF; /* DCA - always hidden. */
@@ -297,10 +296,9 @@ static void recalculate_misc(struct cpu_policy *p)
p->extd.raw[0x8].a &= 0x0000ffff;
p->extd.raw[0x8].c = 0;
- break;
-
- case X86_VENDOR_AMD:
- case X86_VENDOR_HYGON:
+ }
+ else if ( x86_vendor_is(p->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ {
zero_leaves(p->basic.raw, 0x2, 0x3);
memset(p->cache.raw, 0, sizeof(p->cache.raw));
zero_leaves(p->basic.raw, 0x9, 0xa);
@@ -331,7 +329,6 @@ static void recalculate_misc(struct cpu_policy *p)
p->extd.raw[0x20] = EMPTY_LEAF; /* Platform QoS */
p->extd.raw[0x21].b = 0;
p->extd.raw[0x21].d = 0;
- break;
}
}
@@ -416,9 +413,8 @@ static void __init guest_common_default_leaves(struct cpu_policy *p)
static void __init guest_common_max_feature_adjustments(uint32_t *fs)
{
- switch ( boot_cpu_data.vendor )
+ if ( x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_INTEL) )
{
- case X86_VENDOR_INTEL:
/*
* MSR_ARCH_CAPS is just feature data, and we can offer it to guests
* unconditionally, although limit it to Intel systems as it is highly
@@ -477,9 +473,9 @@ static void __init guest_common_max_feature_adjustments(uint32_t *fs)
*/
if ( test_bit(X86_FEATURE_RTM, fs) )
__set_bit(X86_FEATURE_RTM_ALWAYS_ABORT, fs);
- break;
-
- case X86_VENDOR_AMD:
+ }
+ else if ( x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD) )
+ {
/*
* This bit indicates that the VERW instruction may have gained
* scrubbing side effects. With pooling, it means "you might migrate
@@ -488,7 +484,6 @@ static void __init guest_common_max_feature_adjustments(uint32_t *fs)
* has been around since the 286.
*/
__set_bit(X86_FEATURE_VERW_CLEAR, fs);
- break;
}
/*
@@ -510,9 +505,8 @@ static void __init guest_common_max_feature_adjustments(uint32_t *fs)
static void __init guest_common_default_feature_adjustments(uint32_t *fs)
{
- switch ( boot_cpu_data.vendor )
+ if ( x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_INTEL) )
{
- case X86_VENDOR_INTEL:
/*
* IvyBridge client parts suffer from leakage of RDRAND data due to SRBDS
* (XSA-320 / CVE-2020-0543), and won't be receiving microcode to
@@ -570,9 +564,9 @@ static void __init guest_common_default_feature_adjustments(uint32_t *fs)
__clear_bit(X86_FEATURE_RTM, fs);
__set_bit(X86_FEATURE_RTM_ALWAYS_ABORT, fs);
}
- break;
-
- case X86_VENDOR_AMD:
+ }
+ else if ( x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD) )
+ {
/*
* This bit indicate that the VERW instruction may have gained
* scrubbing side effects. The max policy has it set for migration
@@ -581,7 +575,6 @@ static void __init guest_common_default_feature_adjustments(uint32_t *fs)
*/
if ( !cpu_has_verw_clear )
__clear_bit(X86_FEATURE_VERW_CLEAR, fs);
- break;
}
/*
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 9277781bff..9bef1da385 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -766,30 +766,11 @@ void mcheck_init(struct cpuinfo_x86 *c, bool bsp)
else if ( cpu_bank_alloc(cpu) )
panic("Insufficient memory for MCE bank allocations\n");
- switch ( c->x86_vendor )
- {
-#ifdef CONFIG_AMD
- case X86_VENDOR_AMD:
- case X86_VENDOR_HYGON:
+ if ( x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
inited = amd_mcheck_init(c, bsp);
- break;
-#endif
-
-#ifdef CONFIG_INTEL
- case X86_VENDOR_INTEL:
- switch ( c->x86 )
- {
- case 6:
- case 15:
- inited = intel_mcheck_init(c, bsp);
- break;
- }
- break;
-#endif
-
- default:
- break;
- }
+ else if ( x86_vendor_is(c->x86_vendor, X86_VENDOR_INTEL) &&
+ (c->x86 == 6 || c->x86 == 15) )
+ inited = intel_mcheck_init(c, bsp);
show_mca_info(inited, c);
if ( inited == mcheck_none || inited == mcheck_unset )
diff --git a/xen/arch/x86/cpu/mcheck/mce.h b/xen/arch/x86/cpu/mcheck/mce.h
index 920b075355..14261f925b 100644
--- a/xen/arch/x86/cpu/mcheck/mce.h
+++ b/xen/arch/x86/cpu/mcheck/mce.h
@@ -137,28 +137,26 @@ void x86_mcinfo_dump(struct mc_info *mi);
static inline int mce_vendor_bank_msr(const struct vcpu *v, uint32_t msr)
{
- switch (boot_cpu_data.x86_vendor) {
- case X86_VENDOR_INTEL:
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
+ {
if (msr >= MSR_IA32_MC0_CTL2 &&
msr < MSR_IA32_MCx_CTL2(v->arch.vmce.mcg_cap & MCG_CAP_COUNT) )
return 1;
- fallthrough;
-
- case X86_VENDOR_CENTAUR:
- case X86_VENDOR_SHANGHAI:
- if (msr == MSR_P5_MC_ADDR || msr == MSR_P5_MC_TYPE)
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_CENTAUR |
+ X86_VENDOR_SHANGHAI) &&
+ (msr == MSR_P5_MC_ADDR || msr == MSR_P5_MC_TYPE) )
return 1;
- break;
-
- case X86_VENDOR_AMD:
+ }
+ else if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD) )
+ {
switch (msr) {
case MSR_F10_MC4_MISC1:
case MSR_F10_MC4_MISC2:
case MSR_F10_MC4_MISC3:
return 1;
}
- break;
}
+
return 0;
}
diff --git a/xen/arch/x86/cpu/mcheck/non-fatal.c b/xen/arch/x86/cpu/mcheck/non-fatal.c
index a9ee9bb94f..db0ddc5b7b 100644
--- a/xen/arch/x86/cpu/mcheck/non-fatal.c
+++ b/xen/arch/x86/cpu/mcheck/non-fatal.c
@@ -23,25 +23,15 @@ static int __init cf_check init_nonfatal_mce_checker(void)
/*
* Check for non-fatal errors every MCE_RATE s
*/
- switch (c->x86_vendor) {
-#ifdef CONFIG_AMD
- case X86_VENDOR_AMD:
- case X86_VENDOR_HYGON:
+ if ( x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
/* Assume we are on K8 or newer AMD or Hygon CPU here */
amd_nonfatal_mcheck_init(c);
- break;
-#endif
-
-#ifdef CONFIG_INTEL
- case X86_VENDOR_INTEL:
+ else if ( x86_vendor_is(c->x86_vendor, X86_VENDOR_INTEL) )
intel_nonfatal_mcheck_init(c);
- break;
-#endif
-
- default:
- /* unhandled vendor isn't really an error */
+ else
+ /* unhandled vendor isn't really an error */
return 0;
- }
+
printk(KERN_INFO "mcheck_poll: Machine check polling timer started.\n");
return 0;
}
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index 1a7e92506a..dd1ccecfe5 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -136,27 +136,14 @@ static int bank_mce_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
break;
default:
- switch ( boot_cpu_data.x86_vendor )
- {
-#ifdef CONFIG_INTEL
- case X86_VENDOR_CENTAUR:
- case X86_VENDOR_SHANGHAI:
- case X86_VENDOR_INTEL:
+ ret = 0;
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_CENTAUR |
+ X86_VENDOR_SHANGHAI |
+ X86_VENDOR_INTEL) )
ret = vmce_intel_rdmsr(v, msr, val);
- break;
-#endif
-
-#ifdef CONFIG_AMD
- case X86_VENDOR_AMD:
- case X86_VENDOR_HYGON:
+ else if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD |
+ X86_VENDOR_HYGON) )
ret = vmce_amd_rdmsr(v, msr, val);
- break;
-#endif
-
- default:
- ret = 0;
- break;
- }
break;
}
@@ -273,25 +260,12 @@ static int bank_mce_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
break;
default:
- switch ( boot_cpu_data.x86_vendor )
- {
-#ifdef CONFIG_INTEL
- case X86_VENDOR_INTEL:
+ ret = 0;
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
ret = vmce_intel_wrmsr(v, msr, val);
- break;
-#endif
-
-#ifdef CONFIG_AMD
- case X86_VENDOR_AMD:
- case X86_VENDOR_HYGON:
+ else if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD |
+ X86_VENDOR_HYGON) )
ret = vmce_amd_wrmsr(v, msr, val);
- break;
-#endif
-
- default:
- ret = 0;
- break;
- }
break;
}
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 3a21e035f4..f3e4ae4a4d 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -319,9 +319,8 @@ void domain_cpu_policy_changed(struct domain *d)
if ( cpu_has_htt )
edx |= cpufeat_mask(X86_FEATURE_HTT);
- switch ( boot_cpu_data.x86_vendor )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
{
- case X86_VENDOR_INTEL:
/*
* Intel masking MSRs are documented as AND masks.
* Experimentally, they are applied after OSXSAVE and APIC
@@ -336,10 +335,10 @@ void domain_cpu_policy_changed(struct domain *d)
edx = cpufeat_mask(X86_FEATURE_APIC);
mask |= ((uint64_t)edx << 32) | ecx;
- break;
-
- case X86_VENDOR_AMD:
- case X86_VENDOR_HYGON:
+ }
+ else if ( x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ {
mask &= ((uint64_t)ecx << 32) | edx;
/*
@@ -362,7 +361,6 @@ void domain_cpu_policy_changed(struct domain *d)
ecx |= cpufeat_mask(X86_FEATURE_HYPERVISOR);
mask |= ((uint64_t)ecx << 32) | edx;
- break;
}
d->arch.pv.cpuidmasks->_1cd = mask;
diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c
index 77a3a8742a..7802b5f506 100644
--- a/xen/arch/x86/guest/xen/xen.c
+++ b/xen/arch/x86/guest/xen/xen.c
@@ -59,23 +59,18 @@ void asmlinkage __init early_hypercall_setup(void)
boot_cpu_data.x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
}
- switch ( boot_cpu_data.x86_vendor )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL |
+ X86_VENDOR_CENTAUR |
+ X86_VENDOR_SHANGHAI) )
{
- case X86_VENDOR_INTEL:
- case X86_VENDOR_CENTAUR:
- case X86_VENDOR_SHANGHAI:
early_hypercall_insn = 0;
setup_force_cpu_cap(X86_FEATURE_USE_VMCALL);
- break;
-
- case X86_VENDOR_AMD:
- case X86_VENDOR_HYGON:
+ }
+ else if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD |
+ X86_VENDOR_HYGON) )
early_hypercall_insn = 1;
- break;
-
- default:
+ else
BUG();
- }
}
static void __init find_xen_leaves(void)
diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index a0c9194ff0..a35e7109fe 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -216,11 +216,10 @@ void disable_lapic_nmi_watchdog(void)
{
if (nmi_active <= 0)
return;
- switch (boot_cpu_data.x86_vendor) {
- case X86_VENDOR_AMD:
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD) )
wrmsrns(MSR_K7_EVNTSEL0, 0);
- break;
- case X86_VENDOR_INTEL:
+ else if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
+ {
switch (boot_cpu_data.x86) {
case 6:
wrmsrns(MSR_P6_EVNTSEL(0), 0);
@@ -230,7 +229,6 @@ void disable_lapic_nmi_watchdog(void)
wrmsr(MSR_P4_CRU_ESCR0, 0);
break;
}
- break;
}
nmi_active = -1;
/* tell do_nmi() and others that we're not active any more */
@@ -387,13 +385,10 @@ void setup_apic_nmi_watchdog(void)
if ( nmi_watchdog == NMI_NONE )
return;
- switch ( boot_cpu_data.x86_vendor )
- {
- case X86_VENDOR_AMD:
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD) )
setup_k7_watchdog();
- break;
-
- case X86_VENDOR_INTEL:
+ else if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
+ {
switch (boot_cpu_data.x86) {
case 6:
setup_p6_watchdog((boot_cpu_data.x86_model < 14)
@@ -404,7 +399,6 @@ void setup_apic_nmi_watchdog(void)
setup_p4_watchdog();
break;
}
- break;
}
if ( nmi_perfctr_msr == 0 )
diff --git a/xen/arch/x86/traps-setup.c b/xen/arch/x86/traps-setup.c
index d77be8f839..83070b050a 100644
--- a/xen/arch/x86/traps-setup.c
+++ b/xen/arch/x86/traps-setup.c
@@ -243,19 +243,15 @@ static void __init init_ler(void)
* Intel Pentium 4 is the only known CPU to not use the architectural MSR
* indicies.
*/
- switch ( boot_cpu_data.x86_vendor )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_INTEL | X86_VENDOR_AMD | X86_VENDOR_HYGON) )
{
- case X86_VENDOR_INTEL:
- if ( boot_cpu_data.x86 == 0xf )
- {
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) &&
+ (boot_cpu_data.x86 == 0xf) )
msr = MSR_P4_LER_FROM_LIP;
- break;
- }
- fallthrough;
- case X86_VENDOR_AMD:
- case X86_VENDOR_HYGON:
- msr = MSR_IA32_LASTINTFROMIP;
- break;
+ else if ( x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ msr = MSR_IA32_LASTINTFROMIP;
}
if ( msr == 0 )
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [RFC PATCH 07/11] x86: Migrate MSR handler vendor checks to x86_vendor_is()
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
` (5 preceding siblings ...)
2025-11-26 16:44 ` [RFC PATCH 06/11] x86: Migrate switch-based vendor checks to x86_vendor_is() Alejandro Vallejo
@ 2025-11-26 16:44 ` Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 08/11] x86: Migrate insn emulator to use x86_vendor_is() Alejandro Vallejo
` (4 subsequent siblings)
11 siblings, 0 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
Many handlers are vendor-specific and are currently gated on runtime
checks. If we migrate those to x86_vendor_is() they will effectively
cause the ellision of handling code for CPU vendors not compiled in.
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/msr.c | 41 +++++++++++++++++++++++------------------
1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index ad75a2e108..f022332971 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -157,8 +157,9 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
* The MSR has existed on all Intel parts since before the 64bit days,
* and is implemented by other vendors.
*/
- if ( !(cp->x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR |
- X86_VENDOR_SHANGHAI)) )
+ if ( !x86_vendor_is(cp->x86_vendor, X86_VENDOR_INTEL |
+ X86_VENDOR_CENTAUR |
+ X86_VENDOR_SHANGHAI) )
goto gp_fault;
*val = IA32_FEATURE_CONTROL_LOCK;
@@ -169,8 +170,8 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
break;
case MSR_IA32_PLATFORM_ID:
- if ( !(cp->x86_vendor & X86_VENDOR_INTEL) ||
- !(boot_cpu_data.x86_vendor & X86_VENDOR_INTEL) )
+ if ( !x86_vendor_is(cp->x86_vendor, X86_VENDOR_INTEL) ||
+ !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
goto gp_fault;
rdmsrl(MSR_IA32_PLATFORM_ID, *val);
break;
@@ -189,9 +190,10 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
* from Xen's last microcode load, which can be forwarded straight to
* the guest.
*/
- if ( !(cp->x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_AMD)) ||
- !(boot_cpu_data.x86_vendor &
- (X86_VENDOR_INTEL | X86_VENDOR_AMD)) ||
+ if ( !x86_vendor_is(cp->x86_vendor,
+ X86_VENDOR_INTEL | X86_VENDOR_AMD) ||
+ !x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_INTEL | X86_VENDOR_AMD) ||
rdmsr_safe(MSR_AMD_PATCHLEVEL, val) )
goto gp_fault;
break;
@@ -236,7 +238,8 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
*/
case MSR_IA32_PERF_STATUS:
case MSR_IA32_PERF_CTL:
- if ( !(cp->x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR)) )
+ if ( !x86_vendor_is(cp->x86_vendor,
+ X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
goto gp_fault;
*val = 0;
@@ -245,7 +248,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
goto gp_fault;
case MSR_IA32_THERM_STATUS:
- if ( cp->x86_vendor != X86_VENDOR_INTEL )
+ if ( !x86_vendor_is(cp->x86_vendor, X86_VENDOR_INTEL) )
goto gp_fault;
*val = 0;
break;
@@ -302,7 +305,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
case MSR_K8_IORR_MASK1:
case MSR_K8_TSEG_BASE:
case MSR_K8_TSEG_MASK:
- if ( !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
+ if ( !x86_vendor_is(cp->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
goto gp_fault;
if ( !is_hardware_domain(d) )
return X86EMUL_UNHANDLEABLE;
@@ -314,14 +317,15 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
break;
case MSR_K8_HWCR:
- if ( !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
+ if ( !x86_vendor_is(cp->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
goto gp_fault;
*val = 0;
break;
case MSR_FAM10H_MMIO_CONF_BASE:
if ( !is_hardware_domain(d) ||
- !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) ||
+ !x86_vendor_is(cp->x86_vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON) ||
rdmsr_safe(msr, val) )
goto gp_fault;
@@ -338,7 +342,7 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
break;
case MSR_AMD64_DE_CFG:
- if ( !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
+ if ( !x86_vendor_is(cp->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
goto gp_fault;
*val = AMD64_DE_CFG_LFENCE_SERIALISE;
break;
@@ -461,7 +465,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
* for backwards compatiblity, the OS should write 0 to it before
* trying to access the current microcode version.
*/
- if ( cp->x86_vendor != X86_VENDOR_INTEL || val != 0 )
+ if ( !x86_vendor_is(cp->x86_vendor, X86_VENDOR_INTEL) || val != 0 )
goto gp_fault;
break;
@@ -471,7 +475,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
* to AMD CPUs as well (at least the architectural/CPUID part does).
*/
if ( is_pv_domain(d) ||
- cp->x86_vendor != X86_VENDOR_AMD )
+ !x86_vendor_is(cp->x86_vendor, X86_VENDOR_AMD) )
goto gp_fault;
break;
@@ -483,7 +487,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
* by any CPUID bit.
*/
if ( is_pv_domain(d) ||
- cp->x86_vendor != X86_VENDOR_INTEL )
+ !x86_vendor_is(cp->x86_vendor, X86_VENDOR_INTEL) )
goto gp_fault;
break;
@@ -553,7 +557,8 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
* a cpufreq controller dom0 which has full access.
*/
case MSR_IA32_PERF_CTL:
- if ( !(cp->x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR)) )
+ if ( !x86_vendor_is(cp->x86_vendor,
+ X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
goto gp_fault;
if ( likely(!is_cpufreq_controller(d)) || wrmsr_safe(msr, val) == 0 )
@@ -663,7 +668,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
* OpenBSD 6.7 will panic if writing to DE_CFG triggers a #GP:
* https://www.illumos.org/issues/12998 - drop writes.
*/
- if ( !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
+ if ( !x86_vendor_is(cp->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
goto gp_fault;
break;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [RFC PATCH 08/11] x86: Migrate insn emulator to use x86_vendor_is()
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
` (6 preceding siblings ...)
2025-11-26 16:44 ` [RFC PATCH 07/11] x86: Migrate MSR handler " Alejandro Vallejo
@ 2025-11-26 16:44 ` Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is() Alejandro Vallejo
` (3 subsequent siblings)
11 siblings, 0 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/x86_emulate/private.h | 4 +++-
xen/arch/x86/x86_emulate/x86_emulate.c | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/x86_emulate/private.h b/xen/arch/x86/x86_emulate/private.h
index 24c79c4e8f..57c5689a1e 100644
--- a/xen/arch/x86/x86_emulate/private.h
+++ b/xen/arch/x86/x86_emulate/private.h
@@ -15,6 +15,7 @@
# include <xen/kernel.h>
# include <asm/cpu-user-regs.h>
+# include <asm/cpuid.h>
# include <asm/endbr.h>
# include <asm/msr-index.h>
# include <asm/stubs.h>
@@ -32,6 +33,7 @@ void BUG(void);
#else /* !__XEN__ */
# include "x86-emulate.h"
+# define x86_vendor_is(x, y) ((x) & (y))
#endif
#ifdef __i386__
@@ -520,7 +522,7 @@ in_protmode(
static inline bool
_amd_like(const struct cpu_policy *cp)
{
- return cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON);
+ return x86_vendor_is(cp->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON);
}
static inline bool
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index d830aea430..36544adcec 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -3104,7 +3104,7 @@ x86_emulate(
* in fact risking to make guest OSes vulnerable to the equivalent of
* XSA-7 (CVE-2012-0217).
*/
- generate_exception_if(cp->x86_vendor == X86_VENDOR_INTEL &&
+ generate_exception_if(x86_vendor_is(cp->x86_vendor, X86_VENDOR_INTEL) &&
op_bytes == 8 && !is_canonical_address(_regs.rcx),
X86_EXC_GP, 0);
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is()
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
` (7 preceding siblings ...)
2025-11-26 16:44 ` [RFC PATCH 08/11] x86: Migrate insn emulator to use x86_vendor_is() Alejandro Vallejo
@ 2025-11-26 16:44 ` Alejandro Vallejo
2025-12-08 16:01 ` Jan Beulich
2025-12-08 16:04 ` Jan Beulich
2025-11-26 16:44 ` [RFC PATCH 10/11] x86: Migrate everything under cpu/ to use x86_vendor_is() Alejandro Vallejo
` (2 subsequent siblings)
11 siblings, 2 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
This is the file with the most dramatic effect in terms of DCE, so
single it out here.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/spec_ctrl.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index e71f62c601..a464c88908 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -388,7 +388,7 @@ int8_t __ro_after_init opt_xpti_domu = -1;
static __init void xpti_init_default(void)
{
- if ( (boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) ||
+ if ( (x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON)) ||
cpu_has_rdcl_no )
{
if ( opt_xpti_hwdom < 0 )
@@ -712,7 +712,7 @@ static bool __init check_smt_enabled(void)
* At the time of writing, it is almost completely undocumented, so isn't
* virtualised reliably.
*/
- if ( boot_cpu_data.vendor == X86_VENDOR_INTEL &&
+ if ( x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_INTEL) &&
boot_cpu_data.family != 0xf && !cpu_has_hypervisor &&
!rdmsr_safe(MSR_INTEL_CORE_THREAD_COUNT, &val) )
return (MASK_EXTR(val, MSR_CTC_CORE_MASK) !=
@@ -738,10 +738,10 @@ static bool __init retpoline_calculations(void)
unsigned int ucode_rev = this_cpu(cpu_sig).rev;
bool safe = false;
- if ( boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ if ( x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
return true;
- if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
+ if ( !x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_INTEL) ||
boot_cpu_data.family != 6 )
return false;
@@ -938,7 +938,7 @@ static bool __init retpoline_calculations(void)
*/
static bool __init rsb_is_full_width(void)
{
- if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
boot_cpu_data.family != 6 )
return true;
@@ -966,7 +966,7 @@ static bool __init should_use_eager_fpu(void)
* Assume all unrecognised processors are ok. This is only known to
* affect Intel Family 6 processors.
*/
- if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
boot_cpu_data.family != 6 )
return false;
@@ -1033,7 +1033,7 @@ static bool __init should_use_eager_fpu(void)
*/
static void __init srso_calculations(bool hw_smt_enabled)
{
- if ( !(boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
+ if ( !x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
return;
/*
@@ -1099,7 +1099,7 @@ static void __init srso_calculations(bool hw_smt_enabled)
*/
static bool __init has_div_vuln(void)
{
- if ( !(boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
+ if ( !x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
return false;
if ( boot_cpu_data.family != 0x17 && boot_cpu_data.family != 0x18 )
@@ -1137,7 +1137,7 @@ static void __init ibpb_calculations(void)
return;
}
- if ( boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ if ( x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
{
/*
* AMD/Hygon CPUs to date (June 2022) don't flush the RAS. Future
@@ -1222,7 +1222,7 @@ static __init void l1tf_calculations(void)
l1d_maxphysaddr = paddr_bits;
/* L1TF is only known to affect Intel Family 6 processors at this time. */
- if ( boot_cpu_data.vendor == X86_VENDOR_INTEL &&
+ if ( x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_INTEL) &&
boot_cpu_data.family == 6 )
{
switch ( boot_cpu_data.model )
@@ -1358,7 +1358,7 @@ static __init void l1tf_calculations(void)
static __init void mds_calculations(void)
{
/* MDS is only known to affect Intel Family 6 processors at this time. */
- if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
boot_cpu_data.family != 6 )
return;
@@ -1469,7 +1469,7 @@ static __init void mds_calculations(void)
static void __init rfds_calculations(void)
{
/* RFDS is only known to affect Intel Family 6 processors at this time. */
- if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
boot_cpu_data.family != 6 )
return;
@@ -1535,7 +1535,7 @@ static void __init tsa_calculations(void)
unsigned int curr_rev, min_rev;
/* TSA is only known to affect AMD processors at this time. */
- if ( boot_cpu_data.vendor != X86_VENDOR_AMD )
+ if ( !x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD) )
return;
/* If we're virtualised, don't attempt to synthesise anything. */
@@ -1659,7 +1659,7 @@ static void __init gds_calculations(void)
bool cpu_has_bug_gds, mitigated = false;
/* GDS is only known to affect Intel Family 6 processors at this time. */
- if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
boot_cpu_data.family != 6 )
return;
@@ -1754,7 +1754,7 @@ static void __init gds_calculations(void)
static bool __init cpu_has_bug_bhi(void)
{
/* BHI is only known to affect Intel Family 6 processors at this time. */
- if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
boot_cpu_data.family != 6 )
return false;
@@ -1878,7 +1878,7 @@ static void __init its_calculations(void)
return;
/* ITS is only known to affect Intel processors at this time. */
- if ( boot_cpu_data.vendor != X86_VENDOR_INTEL )
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
return;
/*
@@ -2181,7 +2181,7 @@ void __init init_speculation_mitigations(void)
* before going idle is less overhead than flushing on PV entry.
*/
if ( !opt_rsb_pv && hw_smt_enabled &&
- (boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) &&
+ x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) &&
(boot_cpu_data.family == 0x17 || boot_cpu_data.family == 0x18) )
setup_force_cpu_cap(X86_FEATURE_SC_RSB_IDLE);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [RFC PATCH 10/11] x86: Migrate everything under cpu/ to use x86_vendor_is()
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
` (8 preceding siblings ...)
2025-11-26 16:44 ` [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is() Alejandro Vallejo
@ 2025-11-26 16:44 ` Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 11/11] x86: Migrate every remaining raw vendor check to x86_vendor_is() Alejandro Vallejo
2025-11-27 22:11 ` [RFC PATCH 00/11] x86 vendor check optimisations Andrew Cooper
11 siblings, 0 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
There's nothing special about this folder, but it doing it ahead alleviates the
size of the following patch.
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/cpu/amd.c | 6 +++---
xen/arch/x86/cpu/common.c | 2 +-
xen/arch/x86/cpu/intel_cacheinfo.c | 5 ++---
xen/arch/x86/cpu/mcheck/amd_nonfatal.c | 2 +-
xen/arch/x86/cpu/mcheck/mcaction.c | 3 ++-
xen/arch/x86/cpu/mcheck/mce.c | 14 +++++++-------
xen/arch/x86/cpu/mcheck/mce_amd.c | 6 +++---
xen/arch/x86/cpu/mcheck/mce_intel.c | 6 +++---
xen/arch/x86/cpu/mcheck/vmce.c | 4 ++--
xen/arch/x86/cpu/microcode/amd.c | 2 +-
xen/arch/x86/cpu/microcode/core.c | 2 +-
xen/arch/x86/cpu/mtrr/generic.c | 4 ++--
xen/arch/x86/cpu/mwait-idle.c | 4 ++--
13 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 9b02e1ba67..1205253e13 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -428,7 +428,7 @@ int cpu_has_amd_erratum(const struct cpuinfo_x86 *cpu, int osvw_id, ...)
u32 range;
u32 ms;
- if (cpu->x86_vendor != X86_VENDOR_AMD)
+ if (!x86_vendor_is(cpu->x86_vendor, X86_VENDOR_AMD))
return 0;
if (osvw_id >= 0 && cpu_has(cpu, X86_FEATURE_OSVW)) {
@@ -519,8 +519,8 @@ static void check_syscfg_dram_mod_en(void)
{
uint64_t syscfg;
- if (!((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
- (boot_cpu_data.x86 >= 0x0f)))
+ if (!(x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD) &&
+ (boot_cpu_data.x86 >= 0x0f)))
return;
rdmsrl(MSR_K8_SYSCFG, syscfg);
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index c0c3606dd2..df81ef9136 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -463,7 +463,7 @@ void __init early_cpu_init(bool verbose)
paddr_bits -= (ebx >> 6) & 0x3f;
}
- if (!(c->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)))
+ if (!x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON))
park_offline_cpus = opt_mce;
initialize_cpu_data(0);
diff --git a/xen/arch/x86/cpu/intel_cacheinfo.c b/xen/arch/x86/cpu/intel_cacheinfo.c
index e88faa7545..7dc778cd55 100644
--- a/xen/arch/x86/cpu/intel_cacheinfo.c
+++ b/xen/arch/x86/cpu/intel_cacheinfo.c
@@ -168,9 +168,8 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
* Don't use cpuid2 if cpuid4 is supported. For P4, we use cpuid2 for
* trace cache
*/
- if ((num_cache_leaves == 0 || c->x86 == 15) && c->cpuid_level > 1 &&
- c->x86_vendor != X86_VENDOR_SHANGHAI)
- {
+ if (((num_cache_leaves == 0 || c->x86 == 15) && c->cpuid_level > 1) &&
+ !x86_vendor_is(c->x86_vendor, X86_VENDOR_SHANGHAI)) {
/* supports eax=2 call */
unsigned int i, j, n, regs[4];
unsigned char *dp = (unsigned char *)regs;
diff --git a/xen/arch/x86/cpu/mcheck/amd_nonfatal.c b/xen/arch/x86/cpu/mcheck/amd_nonfatal.c
index 7d48c9ab5f..c2c829a397 100644
--- a/xen/arch/x86/cpu/mcheck/amd_nonfatal.c
+++ b/xen/arch/x86/cpu/mcheck/amd_nonfatal.c
@@ -191,7 +191,7 @@ static void cf_check mce_amd_work_fn(void *data)
void __init amd_nonfatal_mcheck_init(struct cpuinfo_x86 *c)
{
- if (!(c->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)))
+ if (!x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON))
return;
/* Assume we are on K8 or newer AMD or Hygon CPU here */
diff --git a/xen/arch/x86/cpu/mcheck/mcaction.c b/xen/arch/x86/cpu/mcheck/mcaction.c
index bf7a0de965..a43e3240c3 100644
--- a/xen/arch/x86/cpu/mcheck/mcaction.c
+++ b/xen/arch/x86/cpu/mcheck/mcaction.c
@@ -101,7 +101,8 @@ mc_memerr_dhandler(struct mca_binfo *binfo,
* not always precise. In that case, fallback to broadcast.
*/
global->mc_domid != bank->mc_domid ||
- (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
+ (x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_INTEL) &&
(!(global->mc_gstatus & MCG_STATUS_LMCE) ||
!(d->vcpu[mc_vcpuid]->arch.vmce.mcg_ext_ctl &
MCG_EXT_CTL_LMCE_EN))) )
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 9bef1da385..40c8c10df9 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -333,8 +333,7 @@ mcheck_mca_logout(enum mca_source who, struct mca_banks *bankmask,
ASSERT(mig);
mca_init_global(mc_flags, mig);
/* A hook here to get global extended msrs */
- if ( IS_ENABLED(CONFIG_INTEL) &&
- boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
intel_get_extended_msrs(mig, mci);
}
}
@@ -564,8 +563,8 @@ bool mce_available(const struct cpuinfo_x86 *c)
*/
unsigned int mce_firstbank(struct cpuinfo_x86 *c)
{
- return c->x86 == 6 &&
- c->x86_vendor == X86_VENDOR_INTEL && c->x86_model < 0x1a;
+ return x86_vendor_is(c->x86_vendor, X86_VENDOR_INTEL) &&
+ c->x86 == 6 && c->x86_model < 0x1a;
}
static int show_mca_info(int inited, struct cpuinfo_x86 *c)
@@ -1107,7 +1106,7 @@ static bool __maybe_unused x86_mc_msrinject_verify(struct xen_mc_msrinject *mci)
if ( IS_MCA_BANKREG(reg, mci->mcinj_cpunr) )
{
- if ( c->x86_vendor == X86_VENDOR_AMD )
+ if ( x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD) )
{
/*
* On AMD we can set MCi_STATUS_WREN in the
@@ -1142,7 +1141,7 @@ static bool __maybe_unused x86_mc_msrinject_verify(struct xen_mc_msrinject *mci)
case MSR_F10_MC4_MISC1:
case MSR_F10_MC4_MISC2:
case MSR_F10_MC4_MISC3:
- if ( c->x86_vendor != X86_VENDOR_AMD )
+ if ( !x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD) )
reason = "only supported on AMD";
else if ( c->x86 < 0x10 )
reason = "only supported on AMD Fam10h+";
@@ -1150,7 +1149,8 @@ static bool __maybe_unused x86_mc_msrinject_verify(struct xen_mc_msrinject *mci)
/* MSRs that the HV will take care of */
case MSR_K8_HWCR:
- if ( c->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ if ( x86_vendor_is(c->x86_vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON) )
reason = "HV will operate HWCR";
else
reason = "only supported on AMD or Hygon";
diff --git a/xen/arch/x86/cpu/mcheck/mce_amd.c b/xen/arch/x86/cpu/mcheck/mce_amd.c
index 25c29eb3d2..e664bf0443 100644
--- a/xen/arch/x86/cpu/mcheck/mce_amd.c
+++ b/xen/arch/x86/cpu/mcheck/mce_amd.c
@@ -160,7 +160,7 @@ mcequirk_lookup_amd_quirkdata(const struct cpuinfo_x86 *c)
{
unsigned int i;
- BUG_ON(c->x86_vendor != X86_VENDOR_AMD);
+ BUG_ON(!x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD));
for ( i = 0; i < ARRAY_SIZE(mce_amd_quirks); i++ )
{
@@ -291,7 +291,7 @@ amd_mcheck_init(const struct cpuinfo_x86 *c, bool bsp)
uint32_t i;
enum mcequirk_amd_flags quirkflag = 0;
- if ( c->x86_vendor != X86_VENDOR_HYGON )
+ if ( !x86_vendor_is(c->x86_vendor, X86_VENDOR_HYGON) )
quirkflag = mcequirk_lookup_amd_quirkdata(c);
/* Assume that machine check support is available.
@@ -337,6 +337,6 @@ amd_mcheck_init(const struct cpuinfo_x86 *c, bool bsp)
ppin_msr = MSR_AMD_PPIN;
}
- return c->x86_vendor == X86_VENDOR_HYGON ?
+ return x86_vendor_is(c->x86_vendor, X86_VENDOR_HYGON) ?
mcheck_hygon : mcheck_amd_famXX;
}
diff --git a/xen/arch/x86/cpu/mcheck/mce_intel.c b/xen/arch/x86/cpu/mcheck/mce_intel.c
index 839a0e5ba9..9f3ae4277b 100644
--- a/xen/arch/x86/cpu/mcheck/mce_intel.c
+++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
@@ -711,8 +711,8 @@ static bool mce_is_broadcast(struct cpuinfo_x86 *c)
* DisplayFamily_DisplayModel encoding of 06H_EH and above,
* a MCA signal is broadcast to all logical processors in the system
*/
- if ( c->x86_vendor == X86_VENDOR_INTEL && c->x86 == 6 &&
- c->x86_model >= 0xe )
+ if ( x86_vendor_is(c->x86_vendor, X86_VENDOR_INTEL) &&
+ c->x86 == 6 && c->x86_model >= 0xe )
return true;
return false;
}
@@ -1036,7 +1036,7 @@ int vmce_intel_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
return 1;
}
- if ( !(cp->x86_vendor & X86_VENDOR_INTEL) )
+ if ( !x86_vendor_is(cp->x86_vendor, X86_VENDOR_INTEL) )
return 0;
if ( bank < GUEST_MC_BANK_NUM )
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index dd1ccecfe5..7f0c413412 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -45,7 +45,7 @@ void vmce_init_vcpu(struct vcpu *v)
int i;
/* global MCA MSRs init */
- if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
v->arch.vmce.mcg_cap = INTEL_GUEST_MCG_CAP;
else
v->arch.vmce.mcg_cap = AMD_GUEST_MCG_CAP;
@@ -63,7 +63,7 @@ int vmce_restore_vcpu(struct vcpu *v, const struct hvm_vmce_vcpu *ctxt)
{
unsigned long guest_mcg_cap;
- if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
guest_mcg_cap = INTEL_GUEST_MCG_CAP | MCG_LMCE_P;
else
guest_mcg_cap = AMD_GUEST_MCG_CAP;
diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 4a7573c885..da5573445e 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -616,7 +616,7 @@ void __init amd_check_entrysign(void)
unsigned int curr_rev;
uint8_t fixed_rev;
- if ( boot_cpu_data.vendor != X86_VENDOR_AMD ||
+ if ( !x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD) ||
boot_cpu_data.family < 0x17 ||
boot_cpu_data.family > 0x1a )
return;
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index fe47c3a6c1..e931bca95e 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -199,7 +199,7 @@ static struct microcode_patch *parse_blob(const char *buf, size_t len)
/* Returns true if ucode should be loaded on a given cpu */
static bool is_cpu_primary(unsigned int cpu)
{
- if ( boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ if ( x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON) )
/* Load ucode on every logical thread/core */
return true;
diff --git a/xen/arch/x86/cpu/mtrr/generic.c b/xen/arch/x86/cpu/mtrr/generic.c
index c587e9140e..88cf6a5e8e 100644
--- a/xen/arch/x86/cpu/mtrr/generic.c
+++ b/xen/arch/x86/cpu/mtrr/generic.c
@@ -218,9 +218,9 @@ static void __init print_mtrr_state(const char *level)
printk("%s %u disabled\n", level, i);
}
- if ((boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
+ if ((x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD) &&
boot_cpu_data.x86 >= 0xf) ||
- boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
+ x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_HYGON)) {
uint64_t syscfg, tom2;
rdmsrl(MSR_K8_SYSCFG, syscfg);
diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
index f47fdfb569..c284375b24 100644
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -1598,7 +1598,7 @@ static int __init mwait_idle_probe(void)
const struct x86_cpu_id *id;
const char *str;
- if (boot_cpu_data.vendor != X86_VENDOR_INTEL)
+ if (!x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_INTEL))
return -ENODEV;
id = x86_match_cpu(intel_idle_ids);
@@ -1816,7 +1816,7 @@ bool __init mwait_pc10_supported(void)
{
unsigned int ecx, edx, dummy;
- if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+ if (!x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
!cpu_has_monitor ||
boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
return false;
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [RFC PATCH 11/11] x86: Migrate every remaining raw vendor check to x86_vendor_is()
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
` (9 preceding siblings ...)
2025-11-26 16:44 ` [RFC PATCH 10/11] x86: Migrate everything under cpu/ to use x86_vendor_is() Alejandro Vallejo
@ 2025-11-26 16:44 ` Alejandro Vallejo
2025-11-27 22:11 ` [RFC PATCH 00/11] x86 vendor check optimisations Andrew Cooper
11 siblings, 0 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-26 16:44 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/acpi/cpu_idle.c | 19 ++++++++++---------
xen/arch/x86/acpi/cpufreq/acpi.c | 2 +-
xen/arch/x86/acpi/cpufreq/cpufreq.c | 4 ++--
xen/arch/x86/apic.c | 2 +-
xen/arch/x86/cpu-policy.c | 10 ++++++----
xen/arch/x86/cpuid.c | 4 ++--
xen/arch/x86/dom0_build.c | 3 ++-
xen/arch/x86/domain.c | 25 +++++++++++--------------
xen/arch/x86/e820.c | 3 ++-
xen/arch/x86/hvm/hvm.c | 5 +++--
xen/arch/x86/hvm/ioreq.c | 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 6 +++---
xen/arch/x86/include/asm/guest_pt.h | 4 ++--
xen/arch/x86/irq.c | 4 ++--
xen/arch/x86/pv/emul-priv-op.c | 24 ++++++++++++------------
xen/arch/x86/setup.c | 2 +-
16 files changed, 61 insertions(+), 58 deletions(-)
diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index d60a07bfd5..1d7f19aadb 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -178,7 +178,7 @@ static void cf_check do_get_hw_residencies(void *arg)
struct cpuinfo_x86 *c = ¤t_cpu_data;
struct hw_residencies *hw_res = arg;
- if ( c->x86_vendor != X86_VENDOR_INTEL || c->x86 != 6 )
+ if ( !x86_vendor_is(c->x86_vendor, X86_VENDOR_INTEL) || c->x86 != 6 )
return;
switch ( c->x86_model )
@@ -915,8 +915,8 @@ void cf_check acpi_dead_idle(void)
mwait(cx->address, 0);
}
}
- else if ( (current_cpu_data.x86_vendor &
- (X86_VENDOR_AMD | X86_VENDOR_HYGON)) &&
+ else if ( x86_vendor_is(current_cpu_data.x86_vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON) &&
cx->entry_method == ACPI_CSTATE_EM_SYSIO )
{
/* Intel prefers not to use SYSIO */
@@ -1042,8 +1042,9 @@ static void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flag
flags->bm_check = 0;
if ( num_online_cpus() == 1 )
flags->bm_check = 1;
- else if ( (c->x86_vendor == X86_VENDOR_INTEL) ||
- ((c->x86_vendor == X86_VENDOR_AMD) && (c->x86 == 0x15)) )
+ else if ( x86_vendor_is(c->x86_vendor, X86_VENDOR_INTEL) ||
+ (x86_vendor_is(c->x86_vendor,
+ X86_VENDOR_AMD) && (c->x86 == 0x15)) )
{
/*
* Today all MP CPUs that support C3 share cache.
@@ -1059,7 +1060,7 @@ static void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flag
* is not required while entering C3 type state on
* P4, Core and beyond CPUs
*/
- if ( c->x86_vendor == X86_VENDOR_INTEL &&
+ if ( x86_vendor_is(c->x86_vendor, X86_VENDOR_INTEL) &&
(c->x86 > 0x6 || (c->x86 == 6 && c->x86_model >= 14)) )
flags->bm_control = 0;
}
@@ -1421,7 +1422,7 @@ static void amd_cpuidle_init(struct acpi_processor_power *power)
case 0x1a:
case 0x19:
case 0x18:
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_HYGON )
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_HYGON) )
{
default:
vendor_override = -1;
@@ -1648,8 +1649,8 @@ static int cf_check cpu_callback(
break;
case CPU_ONLINE:
- if ( (boot_cpu_data.x86_vendor &
- (X86_VENDOR_AMD | X86_VENDOR_HYGON)) &&
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON) &&
processor_powers[cpu] )
amd_cpuidle_init(processor_powers[cpu]);
break;
diff --git a/xen/arch/x86/acpi/cpufreq/acpi.c b/xen/arch/x86/acpi/cpufreq/acpi.c
index b027459417..b8bfb9fb56 100644
--- a/xen/arch/x86/acpi/cpufreq/acpi.c
+++ b/xen/arch/x86/acpi/cpufreq/acpi.c
@@ -471,7 +471,7 @@ static int cf_check acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
/* Check for APERF/MPERF support in hardware
* also check for boost support */
- if (c->x86_vendor == X86_VENDOR_INTEL && c->cpuid_level >= 6)
+ if (x86_vendor_is(c->x86_vendor, X86_VENDOR_INTEL) && c->cpuid_level >= 6)
on_selected_cpus(cpumask_of(cpu), feature_detect, policy, 1);
/*
diff --git a/xen/arch/x86/acpi/cpufreq/cpufreq.c b/xen/arch/x86/acpi/cpufreq/cpufreq.c
index 89e2b3d167..27770269ba 100644
--- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
+++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
@@ -244,8 +244,8 @@ __initcall(cpufreq_driver_late_init);
int cpufreq_cpu_init(unsigned int cpu)
{
/* Currently we only handle Intel, AMD and Hygon processor */
- if ( boot_cpu_data.x86_vendor &
- (X86_VENDOR_INTEL | X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor,
+ (X86_VENDOR_INTEL | X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
return cpufreq_add_cpu(cpu);
return -EOPNOTSUPP;
diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index fb38be7ec3..6e78250a4a 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -406,7 +406,7 @@ void __init init_bsp_APIC(void)
value |= APIC_SPIV_APIC_ENABLED;
/* This bit is reserved on P4/Xeon and should be cleared */
- if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) && (boot_cpu_data.x86 == 15))
+ if (x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) && (boot_cpu_data.x86 == 15))
value &= ~APIC_SPIV_FOCUS_DISABLED;
else
value |= APIC_SPIV_FOCUS_DISABLED;
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 1acd7c5124..3f3c95eb82 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -759,7 +759,8 @@ static void __init calculate_hvm_max_policy(void)
* long mode (and init_amd() has cleared it out of host capabilities), but
* HVM guests are able if running in protected mode.
*/
- if ( (boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) &&
+ if ( x86_vendor_is(boot_cpu_data.vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON) &&
raw_cpu_policy.basic.sep )
__set_bit(X86_FEATURE_SEP, fs);
@@ -952,8 +953,9 @@ void recalculate_cpuid_policy(struct domain *d)
p->basic.max_leaf = min(p->basic.max_leaf, max->basic.max_leaf);
p->feat.max_subleaf = min(p->feat.max_subleaf, max->feat.max_subleaf);
p->extd.max_leaf = 0x80000000U | min(p->extd.max_leaf & 0xffff,
- ((p->x86_vendor & (X86_VENDOR_AMD |
- X86_VENDOR_HYGON))
+ (x86_vendor_is(p->x86_vendor,
+ X86_VENDOR_AMD |
+ X86_VENDOR_HYGON)
? CPUID_GUEST_NR_EXTD_AMD
: CPUID_GUEST_NR_EXTD_INTEL) - 1);
@@ -987,7 +989,7 @@ void recalculate_cpuid_policy(struct domain *d)
if ( is_pv_32bit_domain(d) )
{
__clear_bit(X86_FEATURE_LM, max_fs);
- if ( !(boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
+ if ( !(x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
__clear_bit(X86_FEATURE_SYSCALL, max_fs);
}
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 5decfad8cd..7eca6ad2f5 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -437,7 +437,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
case 0xa:
/* TODO: Rework vPMU control in terms of toolstack choices. */
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
!vpmu_available(v) )
*res = EMPTY_LEAF;
else
@@ -483,7 +483,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
case 0x80000001U:
/* SYSCALL is hidden outside of long mode on Intel. */
- if ( p->x86_vendor == X86_VENDOR_INTEL &&
+ if ( x86_vendor_is(p->x86_vendor, X86_VENDOR_INTEL) &&
is_hvm_domain(d) && !hvm_long_mode_active(v) )
res->d &= ~cpufeat_mask(X86_FEATURE_SYSCALL);
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 0b467fd4a4..11cfd6be20 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -572,7 +572,8 @@ int __init dom0_setup_permissions(struct domain *d)
rc |= iomem_deny_access(d, mfn, mfn);
}
/* HyperTransport range. */
- if ( boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON) )
{
mfn = paddr_to_pfn(1UL <<
(boot_cpu_data.x86 < 0x17 ? 40 : paddr_bits));
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index f3e4ae4a4d..3c878c92a5 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -370,7 +370,7 @@ void domain_cpu_policy_changed(struct domain *d)
{
uint64_t mask = cpuidmask_defaults._6c;
- if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD) )
mask &= (~0ULL << 32) | p->basic.raw[6].c;
d->arch.pv.cpuidmasks->_6c = mask;
@@ -385,8 +385,8 @@ void domain_cpu_policy_changed(struct domain *d)
* wholesale from the policy, but clamp the features in 7[0].ebx
* per usual.
*/
- if ( boot_cpu_data.x86_vendor &
- (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD |
+ X86_VENDOR_HYGON) )
mask = (((uint64_t)p->feat.max_subleaf << 32) |
((uint32_t)mask & p->feat._7b0));
@@ -398,7 +398,7 @@ void domain_cpu_policy_changed(struct domain *d)
uint64_t mask = cpuidmask_defaults.Da1;
uint32_t eax = p->xstate.Da1;
- if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
mask &= (~0ULL << 32) | eax;
d->arch.pv.cpuidmasks->Da1 = mask;
@@ -422,17 +422,15 @@ void domain_cpu_policy_changed(struct domain *d)
* If not emulating AMD or Hygon, clear the duplicated features
* in e1d.
*/
- if ( !(p->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
+ if ( !x86_vendor_is(p->x86_vendor, X86_VENDOR_AMD |
+ X86_VENDOR_HYGON) )
edx &= ~CPUID_COMMON_1D_FEATURES;
- switch ( boot_cpu_data.x86_vendor )
- {
- case X86_VENDOR_INTEL:
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
mask &= ((uint64_t)edx << 32) | ecx;
- break;
-
- case X86_VENDOR_AMD:
- case X86_VENDOR_HYGON:
+ else if ( x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ {
mask &= ((uint64_t)ecx << 32) | edx;
/*
@@ -443,7 +441,6 @@ void domain_cpu_policy_changed(struct domain *d)
edx = cpufeat_mask(X86_FEATURE_APIC);
mask |= ((uint64_t)ecx << 32) | edx;
- break;
}
d->arch.pv.cpuidmasks->e1cd = mask;
@@ -455,7 +452,7 @@ void domain_cpu_policy_changed(struct domain *d)
cpu_policy_updated(v);
/* If PMU version is zero then the guest doesn't have VPMU */
- if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) &&
p->basic.pmu_version == 0 )
vpmu_destroy(v);
}
diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
index ca577c0bde..3cbcd98254 100644
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -426,7 +426,8 @@ static uint64_t __init mtrr_top_of_ram(void)
/* By default we check only Intel systems. */
if ( e820_mtrr_clip == -1 )
- e820_mtrr_clip = boot_cpu_data.x86_vendor == X86_VENDOR_INTEL;
+ e820_mtrr_clip = x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_INTEL);
if ( !e820_mtrr_clip )
return 0;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 4084b610fa..8e4050b8ce 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2668,8 +2668,9 @@ bool hvm_vcpu_virtual_to_linear(
}
else if ( last_byte > reg->limit )
goto out; /* last byte is beyond limit */
- else if ( last_byte < offset &&
- v->domain->arch.cpuid->x86_vendor == X86_VENDOR_AMD )
+ else if ( x86_vendor_is(v->domain->arch.cpuid->x86_vendor,
+ X86_VENDOR_AMD) &&
+ last_byte < offset )
goto out; /* access wraps */
}
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index a5fa97e149..8bdf52b506 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -286,7 +286,7 @@ bool arch_ioreq_server_get_type_addr(const struct domain *d,
*addr = ((uint64_t)sbdf.sbdf << 32) | reg;
/* AMD extended configuration space access? */
if ( CF8_ADDR_HI(cf8) &&
- d->arch.cpuid->x86_vendor == X86_VENDOR_AMD &&
+ x86_vendor_is(d->arch.cpuid->x86_vendor, X86_VENDOR_AMD) &&
(x86_fam = get_cpu_family(
d->arch.cpuid->basic.raw_fms, NULL, NULL)) >= 0x10 &&
x86_fam < 0x17 )
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 6b407226c4..769840569c 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3057,7 +3057,7 @@ static bool __init has_if_pschange_mc(void)
* IF_PSCHANGE_MC is only known to affect Intel Family 6 processors at
* this time.
*/
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
boot_cpu_data.x86 != 6 )
return false;
@@ -3409,7 +3409,7 @@ static void __init lbr_tsx_fixup_check(void)
* fixed up as well.
*/
if ( cpu_has_hle || cpu_has_rtm ||
- boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+ !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
boot_cpu_data.x86 != 6 )
return;
@@ -3454,7 +3454,7 @@ static void __init ler_to_fixup_check(void)
* that are not equal to bit[47]. Attempting to context switch this value
* may cause a #GP. Software should sign extend the MSR.
*/
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
boot_cpu_data.x86 != 6 )
return;
diff --git a/xen/arch/x86/include/asm/guest_pt.h b/xen/arch/x86/include/asm/guest_pt.h
index 21473f9bbc..a44909d35d 100644
--- a/xen/arch/x86/include/asm/guest_pt.h
+++ b/xen/arch/x86/include/asm/guest_pt.h
@@ -314,8 +314,8 @@ static always_inline bool guest_l4e_rsvd_bits(const struct vcpu *v,
guest_l4e_t l4e)
{
return l4e.l4 & (guest_rsvd_bits(v) | GUEST_L4_PAGETABLE_RSVD |
- ((v->domain->arch.cpuid->x86_vendor == X86_VENDOR_AMD)
- ? _PAGE_GLOBAL : 0));
+ (x86_vendor_is(v->domain->arch.cpuid->x86_vendor,
+ X86_VENDOR_AMD) ? _PAGE_GLOBAL : 0));
}
#endif /* GUEST_PAGING_LEVELS >= 4 */
#endif /* GUEST_PAGING_LEVELS >= 3 */
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 92b8604dc8..2490331ec3 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2010,8 +2010,8 @@ void do_IRQ(struct cpu_user_regs *regs)
* interrupts have been delivered to CPUs
* different than the BSP.
*/
- (boot_cpu_data.x86_vendor & (X86_VENDOR_AMD |
- X86_VENDOR_HYGON))) &&
+ x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON)) &&
bogus_8259A_irq(vector - FIRST_LEGACY_VECTOR)) )
{
printk("CPU%u: No irq handler for vector %02x (IRQ %d%s)\n",
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 225d4cff03..e69450f949 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -244,7 +244,7 @@ static bool pci_cfg_ok(struct domain *currd, unsigned int start,
start |= CF8_ADDR_LO(currd->arch.pci_cf8);
/* AMD extended configuration space access? */
if ( CF8_ADDR_HI(currd->arch.pci_cf8) &&
- boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
+ x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD) &&
boot_cpu_data.x86 >= 0x10 && boot_cpu_data.x86 < 0x17 )
{
uint64_t msr_val;
@@ -868,7 +868,7 @@ static uint64_t guest_efer(const struct domain *d)
*/
if ( is_pv_32bit_domain(d) )
val &= ~(EFER_LME | EFER_LMA |
- (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL
+ (x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL)
? EFER_SCE : 0));
return val;
}
@@ -943,7 +943,7 @@ static int cf_check read_msr(
case MSR_K8_PSTATE5:
case MSR_K8_PSTATE6:
case MSR_K8_PSTATE7:
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_AMD )
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD) )
break;
if ( unlikely(is_cpufreq_controller(currd)) )
goto normal;
@@ -951,7 +951,7 @@ static int cf_check read_msr(
return X86EMUL_OKAY;
case MSR_FAM10H_MMIO_CONF_BASE:
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD) ||
boot_cpu_data.x86 < 0x10 || boot_cpu_data.x86 >= 0x17 )
break;
/* fall through */
@@ -976,7 +976,7 @@ static int cf_check read_msr(
case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL(3):
case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTR2:
case MSR_CORE_PERF_FIXED_CTR_CTRL ... MSR_CORE_PERF_GLOBAL_OVF_CTRL:
- if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
{
vpmu_msr = true;
/* fall through */
@@ -1103,7 +1103,7 @@ static int cf_check write_msr(
break;
case MSR_FAM10H_MMIO_CONF_BASE:
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_AMD) ||
boot_cpu_data.x86 < 0x10 || boot_cpu_data.x86 >= 0x17 )
break;
if ( !is_hwdom_pinned_vcpu(curr) )
@@ -1132,8 +1132,8 @@ static int cf_check write_msr(
case MSR_IA32_MPERF:
case MSR_IA32_APERF:
- if ( !(boot_cpu_data.x86_vendor &
- (X86_VENDOR_INTEL | X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_INTEL | X86_VENDOR_AMD | X86_VENDOR_HYGON) )
break;
if ( likely(!is_cpufreq_controller(currd)) ||
wrmsr_safe(reg, val) == 0 )
@@ -1142,7 +1142,7 @@ static int cf_check write_msr(
case MSR_IA32_THERM_CONTROL:
case MSR_IA32_ENERGY_PERF_BIAS:
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL )
+ if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
break;
if ( !is_hwdom_pinned_vcpu(curr) || wrmsr_safe(reg, val) == 0 )
return X86EMUL_OKAY;
@@ -1152,13 +1152,13 @@ static int cf_check write_msr(
case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL(3):
case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTR2:
case MSR_CORE_PERF_FIXED_CTR_CTRL ... MSR_CORE_PERF_GLOBAL_OVF_CTRL:
- if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
+ if ( x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) )
{
vpmu_msr = true;
case MSR_AMD_FAM15H_EVNTSEL0 ... MSR_AMD_FAM15H_PERFCTR5:
case MSR_K7_EVNTSEL0 ... MSR_K7_PERFCTR3:
- if ( vpmu_msr || (boot_cpu_data.x86_vendor &
- (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
+ if ( vpmu_msr || x86_vendor_is(boot_cpu_data.x86_vendor,
+ X86_VENDOR_AMD | X86_VENDOR_HYGON) )
{
if ( (vpmu_mode & XENPMU_MODE_ALL) &&
!is_hardware_domain(currd) )
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 872a8c63f9..0eca058850 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1384,7 +1384,7 @@ void asmlinkage __init noreturn __start_xen(void)
* supervisor shadow stacks are now safe to use.
*/
bool cpu_has_bug_shstk_fracture =
- boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
+ x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) &&
!boot_cpu_has(X86_FEATURE_CET_SSS);
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig
2025-11-26 16:44 ` [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
@ 2025-11-27 9:43 ` Jan Beulich
2025-11-27 12:36 ` Alejandro Vallejo
2025-11-27 9:59 ` Jan Beulich
2025-11-27 21:01 ` Andrew Cooper
2 siblings, 1 reply; 37+ messages in thread
From: Jan Beulich @ 2025-11-27 9:43 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 26.11.2025 17:44, Alejandro Vallejo wrote:
> This enables very aggressive DCE passes on single-vendor builds in later
> patches, as it will allow most vendor checks to become statically chosen
> branches. A lot of statics go away and a lot more inlining is allowed.
>
> In order to allow x86_vendor_is() to fold into constants, expand Kconfig
> to have the full set of vendors. Adds Hygon, Centaur, Shanghai and the
> default path.
>
> Have Hygon depend on AMD, and Centaur+Shanghai depend on Intel.
>
> Not a functional change.
>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
> xen/arch/x86/Kconfig.cpu | 45 ++++++++++++++++++++++++++++++++
> xen/arch/x86/cpu/common.c | 17 +++++++-----
> xen/arch/x86/include/asm/cpuid.h | 7 +++++
> 3 files changed, 62 insertions(+), 7 deletions(-)
Shouldn't patch 5 be folded into here? Or, if there were some dependencies
on patches 2-4 (albeit I can't spot anything, as the files are all self-
contained), at least the parts which can be done right away?
> --- a/xen/arch/x86/Kconfig.cpu
> +++ b/xen/arch/x86/Kconfig.cpu
> @@ -19,4 +19,49 @@ config INTEL
> May be turned off in builds targetting other vendors. Otherwise,
> must be enabled for Xen to work suitably on Intel platforms.
>
> +config HYGON
> + bool "Support Hygon CPUs"
> + depends on AMD
> + default y
> + help
> + Detection, tunings and quirks for Hygon platforms.
> +
> + May be turned off in builds targetting other vendors. Otherwise,
> + must be enabled for Xen to work suitably on Hygon platforms.
> +
> +
> +config CENTAUR
> + bool "Support Centaur CPUs"
> + depends on INTEL
> + default y
> + help
> + Detection, tunings and quirks for Centaur platforms.
> +
> + May be turned off in builds targetting other vendors. Otherwise,
> + must be enabled for Xen to work suitably on Centaur platforms.
> +
> +config SHANGHAI
> + bool "Support Shanghai CPUs"
> + depends on INTEL
> + default y
> + help
> + Detection, tunings and quirks for Shanghai platforms.
> +
> + May be turned off in builds targetting other vendors. Otherwise,
> + must be enabled for Xen to work suitably on Shanghai platforms.
> +
> +config UNKNOWN_CPU
> + bool "Support unknown CPUs"
"Unknown CPUs" can be of two kinds: Such of vendors we don't explicitly support,
and such of vendors we do explicitly support, but where we aren't aware of the
particular model. This needs to be unambiguous here, perhaps by it becoming
UNKNOWN_CPU_VENDOR (and the prompt changing accordingly).
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -118,7 +118,7 @@ static void cf_check default_init(struct cpuinfo_x86 * c)
> __clear_bit(X86_FEATURE_SEP, c->x86_capability);
> }
>
> -static const struct cpu_dev __initconst_cf_clobber __used default_cpu = {
> +static const struct cpu_dev __initconst_cf_clobber default_cpu = {
This change isn't explained in the description. __used here was introduced not
all this long ago together with __initconst_cf_clobber. Maybe this really was
a mistake, but if so it's correction should be explained.
> @@ -340,7 +340,8 @@ void __init early_cpu_init(bool verbose)
> *(u32 *)&c->x86_vendor_id[8] = ecx;
> *(u32 *)&c->x86_vendor_id[4] = edx;
>
> - c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
> + c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx) &
> + X86_ENABLED_VENDORS;
May I suggest the & to move ...
> switch (c->x86_vendor) {
... here? Yes, you panic() below, but I see no reason to store inaccurate
data when that's easy to avoid.
> case X86_VENDOR_INTEL: intel_unlock_cpuid_leaves(c);
> actual_cpu = intel_cpu_dev; break;
> @@ -349,12 +350,14 @@ void __init early_cpu_init(bool verbose)
> case X86_VENDOR_SHANGHAI: actual_cpu = shanghai_cpu_dev; break;
> case X86_VENDOR_HYGON: actual_cpu = hygon_cpu_dev; break;
> default:
> + if (verbose || !IS_ENABLED(CONFIG_UNKNOWN_CPU))
> + printk(XENLOG_ERR
> + "Unrecognised or unsupported CPU vendor '%.12s'\n",
> + c->x86_vendor_id);
> + if (!IS_ENABLED(CONFIG_UNKNOWN_CPU))
> + panic("Cannot run in unknown/compiled-out CPU vendor.\n");
The text reads somewhat odd to me, "run in" in particular. Also nit: No full stop
please at the end of log messages, except maybe in extraordinary situations.
Jan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig
2025-11-26 16:44 ` [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
2025-11-27 9:43 ` Jan Beulich
@ 2025-11-27 9:59 ` Jan Beulich
2025-11-27 12:37 ` Alejandro Vallejo
2025-11-27 21:01 ` Andrew Cooper
2 siblings, 1 reply; 37+ messages in thread
From: Jan Beulich @ 2025-11-27 9:59 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 26.11.2025 17:44, Alejandro Vallejo wrote:
> This enables very aggressive DCE passes on single-vendor builds in later
> patches, as it will allow most vendor checks to become statically chosen
> branches. A lot of statics go away and a lot more inlining is allowed.
>
> In order to allow x86_vendor_is() to fold into constants, expand Kconfig
> to have the full set of vendors. Adds Hygon, Centaur, Shanghai and the
> default path.
Oh, one more thing: There's x86_vendor_is() yet, so what it is going to be
needs at least roughly explaining here.
Jan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it
2025-11-26 16:44 ` [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it Alejandro Vallejo
@ 2025-11-27 10:46 ` Jan Beulich
2025-11-27 13:15 ` Alejandro Vallejo
0 siblings, 1 reply; 37+ messages in thread
From: Jan Beulich @ 2025-11-27 10:46 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 26.11.2025 17:44, Alejandro Vallejo wrote:
> This function is meant to replace all instances of the following
> patterns in CPU policies and boot_cpu_data:
>
> - x->x86_vendor == X86_VENDOR_FOO
> - x->x86_vendor != X86_VENDOR_FOO
> - x->x86_vendor & (X86_VENDOR_FOO | X86_VENDOR_BAR)
>
> The secret sauce is that all branches inside the helper resolve at
> compile time, so for the all-vendors-compiled-in case the function
> resolves to equivalent code as that without the helper and you get
> progressively more aggressive DCE as you disable vendors. The function
> folds into a constant once you remove the fallback CPU vendor setting.
Here and below in the comment, "fallback CPU vendor" wants clarifying. I
don't view it as obvious that what's presently named UNKNOWN_CPU is that
"fallback" (as imo that really isn't any kind of fallback, but merely a
placeholder).
> While at this, move an include out of place so they sort alphabetically.
I'd rather suggest to simply ...
> --- a/xen/arch/x86/include/asm/cpuid.h
> +++ b/xen/arch/x86/include/asm/cpuid.h
> @@ -2,10 +2,12 @@
> #define __X86_CPUID_H__
>
> #include <asm/cpufeatureset.h>
> +#include <asm/x86-vendors.h>
>
> -#include <xen/types.h>
> +#include <xen/compiler.h>
> #include <xen/kernel.h>
> #include <xen/percpu.h>
> +#include <xen/types.h>
... drop it. xen/kernel.h, for example, already gets it for you anyway.
> @@ -56,6 +58,51 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
> (IS_ENABLED(CONFIG_SHANGHAI) ? X86_VENDOR_SHANGHAI : 0) | \
> (IS_ENABLED(CONFIG_HYGON) ? X86_VENDOR_HYGON : 0))
>
> +/*
> + * When compiling Xen for a single vendor with no fallback vendor there's no
> + * need no check the candidate. `vendor` is always a compile-time constant,
> + * which means this all can fold into a constant boolean.
DYM "`vendor` is always supposed to be a compile-time constant, ..." ?
> + * A runtime check at the time of CPUID probing guarantees we never run on
> + * wrong hardware and another check when loading CPU policies guarantees we
> + * never run policies for a vendor in another vendor's silicon.
> + *
> + * By the same token, the same folding can happen when no vendor is compiled
> + * in and the fallback path is present.
> + */
> +static always_inline bool x86_vendor_is(uint8_t candidate, uint8_t vendor)
I fear the comment, no matter that it's pretty large already, doesn't make
clear how this function is to be used, i.e. how for this being an "is"
predicate the two arguments should be chosen. My typical expectation would be
for "is" predicates to apply to a single property, with other parameters (if
any) only being auxiliary ones. Maybe it would already help if the first
parameter wasn't named "candidate" but e.g. "actual" (from looking at just
the next patch). Or maybe (depending on the number of possible different
inputs for the first parameter) there want to be a few wrappers, so the
"single property" aspect would be achieved at use sites.
Then I see no reason for the parameters to be other than unsigned int. (Same
for the local variable then, obviously.)
I'm further uncertain this is a good place for the function. In the old days
it may have been, but cpuid.[ch] now are only about guest exposure of CPUID,
when this predicate is intended to be used for both host and guest. (As I
realize only now, this also applies to the addition patch 1 does.) One
might think processor.h might be a good home, but we're actually trying to
slim that one down. So one of cpufeature.h and cpufeatures.h, I guess. (Maybe
other x86 maintainers also have thoughts here.)
> +{
> + uint8_t filtered_vendor = vendor & X86_ENABLED_VENDORS;
> +
> + if ( vendor == X86_VENDOR_UNKNOWN )
> + {
> + if ( IS_ENABLED(CONFIG_UNKNOWN_CPU) )
> + /* no-vendor optimisation */
Nit: Comment style (also again below).
> + return X86_ENABLED_VENDORS ? vendor == candidate : true;
With the surrounding if() this effectively (and more explicitly) is
return X86_ENABLED_VENDORS ? candidate == X86_VENDOR_UNKNOWN : true;
First: Would one ever pass X86_VENDOR_UNKNOWN for "vendor"? The next patch,
for example, specifically doesn't. And then why not shorter as
return !X86_ENABLED_VENDORS || candidate == X86_VENDOR_UNKNOWN;
Which raises the next question: Should we even allow a hypervisor to be built
with X86_ENABLED_VENDORS == 0? Plus, question more on patch 1, what's the
(useful) difference between a build with all vendors set to N and
(a) UNKNOWN_CPU=n vs (b) UNKNOWN_CPU=y? With all vendor support explicitly
turned off, all CPUs are going to be "unknown".
> +
> + /* unknown-vendor-elimination optimisation */
> + return false;
> + }
> +
> + /* single-vendor optimisation */
> + if ( !IS_ENABLED(CONFIG_UNKNOWN_CPU) &&
> + (ISOLATE_LSB(X86_ENABLED_VENDORS) == X86_ENABLED_VENDORS) )
> + return filtered_vendor == X86_ENABLED_VENDORS;
> +
> + /* compiled-out-vendor-elimination optimisation */
> + if ( !filtered_vendor )
> + return false;
> +
> + /*
> + * When checking against a single vendor, perform an equality check, as
> + * it yields (marginally) better codegen
> + */
> + if ( ISOLATE_LSB(filtered_vendor) == filtered_vendor )
So one may pass a combination of multiple vendors for "vendor"? Is so, why
is the parameter name singular?
> + return filtered_vendor == candidate ;
Nit: Stray blank.
Jan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 04/11] x86: Refactor early vendor lookup code to use x86_vendor_is()
2025-11-26 16:44 ` [RFC PATCH 04/11] x86: Refactor early vendor lookup code to use x86_vendor_is() Alejandro Vallejo
@ 2025-11-27 10:51 ` Jan Beulich
2025-11-27 13:35 ` Alejandro Vallejo
0 siblings, 1 reply; 37+ messages in thread
From: Jan Beulich @ 2025-11-27 10:51 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 26.11.2025 17:44, Alejandro Vallejo wrote:
> Move the "unknown" vendor ahead of all others and have it NOT rely
> on x86_vendor_is(), as that would yield incorrect values for the
> single-vendor+no-fallback case when running in incorrect hardware
> (because x86_vendor_is() becomes a folded constant of the single
> compiled-in vendor).
>
> This is one of the two places where x86_vendor_is() cannot be used,
> along with the compatibility check on loaded guest CPU policies.
>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
> xen/arch/x86/cpu/common.c | 31 +++++++++++++++++++++++--------
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> index 393c30227f..c0c3606dd2 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -342,23 +342,38 @@ void __init early_cpu_init(bool verbose)
>
> c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx) &
> X86_ENABLED_VENDORS;
> - switch (c->x86_vendor) {
> - case X86_VENDOR_INTEL: intel_unlock_cpuid_leaves(c);
> - actual_cpu = intel_cpu_dev; break;
> - case X86_VENDOR_AMD: actual_cpu = amd_cpu_dev; break;
> - case X86_VENDOR_CENTAUR: actual_cpu = centaur_cpu_dev; break;
> - case X86_VENDOR_SHANGHAI: actual_cpu = shanghai_cpu_dev; break;
> - case X86_VENDOR_HYGON: actual_cpu = hygon_cpu_dev; break;
> - default:
> +
> + /*
> + * We can't rely on x86_vendor_is() here due to the single-vendor
> + * optimisation. It makes x86_vendor_is(x, y) rely on the constant `y`
> + * matching the single vendor Xen was compiled for and ignore the
> + * runtime variable `x`. In order to preserve sanity we must assert here
> + * that we never boot such a build in a CPU from another vendor, or
> + * major chaos would ensue.
> + */
> + if (c->x86_vendor == X86_VENDOR_UNKNOWN)
> + {
Nit: No mix of styles please. Here it wants to be Linux style.
> if (verbose || !IS_ENABLED(CONFIG_UNKNOWN_CPU))
> printk(XENLOG_ERR
> "Unrecognised or unsupported CPU vendor '%.12s'\n",
> c->x86_vendor_id);
> +
> if (!IS_ENABLED(CONFIG_UNKNOWN_CPU))
> panic("Cannot run in unknown/compiled-out CPU vendor.\n");
>
> actual_cpu = default_cpu;
> }
> + else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_INTEL)) {
> + intel_unlock_cpuid_leaves(c);
> + actual_cpu = intel_cpu_dev;
> + } else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD))
> + actual_cpu = amd_cpu_dev;
> + else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_CENTAUR))
> + actual_cpu = centaur_cpu_dev;
> + else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_SHANGHAI))
> + actual_cpu = shanghai_cpu_dev;
> + else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_HYGON))
> + actual_cpu = hygon_cpu_dev;
If it needs to be like this, then so be it, but I view it as a downside to
not be able to use switch() anymore. It's not quite clear to me though what
extra gains the transformation brings. The masking by X86_ENABLED_VENDORS
already does most of what you want, and X86_VENDOR_UNKNOWN continues to be
handled separately anyway.
Jan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig
2025-11-27 9:43 ` Jan Beulich
@ 2025-11-27 12:36 ` Alejandro Vallejo
2025-11-27 13:19 ` Jan Beulich
0 siblings, 1 reply; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-27 12:36 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
Hi,
Thanks for taking the time to look at this
On Thu Nov 27, 2025 at 10:43 AM CET, Jan Beulich wrote:
> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>> This enables very aggressive DCE passes on single-vendor builds in later
>> patches, as it will allow most vendor checks to become statically chosen
>> branches. A lot of statics go away and a lot more inlining is allowed.
>>
>> In order to allow x86_vendor_is() to fold into constants, expand Kconfig
>> to have the full set of vendors. Adds Hygon, Centaur, Shanghai and the
>> default path.
>>
>> Have Hygon depend on AMD, and Centaur+Shanghai depend on Intel.
>>
>> Not a functional change.
>>
>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>> ---
>> xen/arch/x86/Kconfig.cpu | 45 ++++++++++++++++++++++++++++++++
>> xen/arch/x86/cpu/common.c | 17 +++++++-----
>> xen/arch/x86/include/asm/cpuid.h | 7 +++++
>> 3 files changed, 62 insertions(+), 7 deletions(-)
>
> Shouldn't patch 5 be folded into here? Or, if there were some dependencies
> on patches 2-4 (albeit I can't spot anything, as the files are all self-
> contained), at least the parts which can be done right away?
I didn't expect that to work before transforming the switch, but of course
the prior & X86_ENABLED_VENDORS is already telling the compiler which switch
branches are unreachable.
Will do and send soon as non-RFC if there's no objection to the full vendor
palette (including the unknown vendor case) being on display in Kconfig.
>
>> --- a/xen/arch/x86/Kconfig.cpu
>> +++ b/xen/arch/x86/Kconfig.cpu
>> @@ -19,4 +19,49 @@ config INTEL
>> May be turned off in builds targetting other vendors. Otherwise,
>> must be enabled for Xen to work suitably on Intel platforms.
>>
>> +config HYGON
>> + bool "Support Hygon CPUs"
>> + depends on AMD
>> + default y
>> + help
>> + Detection, tunings and quirks for Hygon platforms.
>> +
>> + May be turned off in builds targetting other vendors. Otherwise,
>> + must be enabled for Xen to work suitably on Hygon platforms.
>> +
>> +
>> +config CENTAUR
>> + bool "Support Centaur CPUs"
>> + depends on INTEL
>> + default y
>> + help
>> + Detection, tunings and quirks for Centaur platforms.
>> +
>> + May be turned off in builds targetting other vendors. Otherwise,
>> + must be enabled for Xen to work suitably on Centaur platforms.
>> +
>> +config SHANGHAI
>> + bool "Support Shanghai CPUs"
>> + depends on INTEL
>> + default y
>> + help
>> + Detection, tunings and quirks for Shanghai platforms.
>> +
>> + May be turned off in builds targetting other vendors. Otherwise,
>> + must be enabled for Xen to work suitably on Shanghai platforms.
>> +
>> +config UNKNOWN_CPU
>> + bool "Support unknown CPUs"
>
> "Unknown CPUs" can be of two kinds: Such of vendors we don't explicitly support,
> and such of vendors we do explicitly support, but where we aren't aware of the
> particular model. This needs to be unambiguous here, perhaps by it becoming
> UNKNOWN_CPU_VENDOR (and the prompt changing accordingly).
Right, what I do in this RFC is have compiled-out vendors fall back onto the
unknown vendor path. Because it really is unknown to the binary.
I could call it GENERIC_CPU_VENDOR, or anything else, but the main question
is whether a toggle for this seems acceptable upstream. I don't see obvious
drawbacks.
>
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -118,7 +118,7 @@ static void cf_check default_init(struct cpuinfo_x86 * c)
>> __clear_bit(X86_FEATURE_SEP, c->x86_capability);
>> }
>>
>> -static const struct cpu_dev __initconst_cf_clobber __used default_cpu = {
>> +static const struct cpu_dev __initconst_cf_clobber default_cpu = {
>
> This change isn't explained in the description. __used here was introduced not
> all this long ago together with __initconst_cf_clobber. Maybe this really was
> a mistake, but if so it's correction should be explained.
It wasn't clear to me why __used was there (I assume it shouldn't have been),
but it definitely clashes with the intent of having it gone when UNKNOWN_CPU=n.
If __used was misplaced to begin with I'm happy to get rid of it in a separate
patch. I don't think it warrants a Fixes tag, though.
>
>> @@ -340,7 +340,8 @@ void __init early_cpu_init(bool verbose)
>> *(u32 *)&c->x86_vendor_id[8] = ecx;
>> *(u32 *)&c->x86_vendor_id[4] = edx;
>>
>> - c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
>> + c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx) &
>> + X86_ENABLED_VENDORS;
>
> May I suggest the & to move ...
>
>> switch (c->x86_vendor) {
>
> ... here? Yes, you panic() below, but I see no reason to store inaccurate
> data when that's easy to avoid.
That's intentional. Otherwise further checks of c->x86_vendor in other parts of
the code may not go through the same branch as the one here.
>
>> case X86_VENDOR_INTEL: intel_unlock_cpuid_leaves(c);
>> actual_cpu = intel_cpu_dev; break;
>> @@ -349,12 +350,14 @@ void __init early_cpu_init(bool verbose)
>> case X86_VENDOR_SHANGHAI: actual_cpu = shanghai_cpu_dev; break;
>> case X86_VENDOR_HYGON: actual_cpu = hygon_cpu_dev; break;
>> default:
>> + if (verbose || !IS_ENABLED(CONFIG_UNKNOWN_CPU))
>> + printk(XENLOG_ERR
>> + "Unrecognised or unsupported CPU vendor '%.12s'\n",
>> + c->x86_vendor_id);
>> + if (!IS_ENABLED(CONFIG_UNKNOWN_CPU))
>> + panic("Cannot run in unknown/compiled-out CPU vendor.\n");
>
> The text reads somewhat odd to me, "run in" in particular. Also nit: No full stop
> please at the end of log messages, except maybe in extraordinary situations.
ack. As for the text, might as well be just "Unsupported hardware\n". The prior
printk already gives all relevant information. The particulars on the text can
be argued after I send a non-RFC version.
>
> Jan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig
2025-11-27 9:59 ` Jan Beulich
@ 2025-11-27 12:37 ` Alejandro Vallejo
0 siblings, 0 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-27 12:37 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On Thu Nov 27, 2025 at 10:59 AM CET, Jan Beulich wrote:
> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>> This enables very aggressive DCE passes on single-vendor builds in later
>> patches, as it will allow most vendor checks to become statically chosen
>> branches. A lot of statics go away and a lot more inlining is allowed.
>>
>> In order to allow x86_vendor_is() to fold into constants, expand Kconfig
>> to have the full set of vendors. Adds Hygon, Centaur, Shanghai and the
>> default path.
>
> Oh, one more thing: There's x86_vendor_is() yet, so what it is going to be
> needs at least roughly explaining here.
>
> Jan
Fair enough.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it
2025-11-27 10:46 ` Jan Beulich
@ 2025-11-27 13:15 ` Alejandro Vallejo
2025-11-27 13:37 ` Jan Beulich
2025-11-27 20:36 ` Andrew Cooper
0 siblings, 2 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-27 13:15 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
Hi,
On Thu Nov 27, 2025 at 11:46 AM CET, Jan Beulich wrote:
> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>> This function is meant to replace all instances of the following
>> patterns in CPU policies and boot_cpu_data:
>>
>> - x->x86_vendor == X86_VENDOR_FOO
>> - x->x86_vendor != X86_VENDOR_FOO
>> - x->x86_vendor & (X86_VENDOR_FOO | X86_VENDOR_BAR)
>>
>> The secret sauce is that all branches inside the helper resolve at
>> compile time, so for the all-vendors-compiled-in case the function
>> resolves to equivalent code as that without the helper and you get
>> progressively more aggressive DCE as you disable vendors. The function
>> folds into a constant once you remove the fallback CPU vendor setting.
>
> Here and below in the comment, "fallback CPU vendor" wants clarifying. I
> don't view it as obvious that what's presently named UNKNOWN_CPU is that
> "fallback" (as imo that really isn't any kind of fallback, but merely a
> placeholder).
I'll rename all fallback references in commits/comments to "generic vendor".
Though do note there's a fallback behaviour. It's introduced in patch 1 due
to the ANDing of the x86_vendor with the mask of compiled-in vendors.
We can trivially get rid of this behaviour, but I assumed booting in untuned
mode is preferable to panicking. And if you _do_ care about panicking when you
don't know about a CPU you're better off setting UNKNOWN_CPU=n and getting that
exact behaviour.
>
>> While at this, move an include out of place so they sort alphabetically.
>
> I'd rather suggest to simply ...
>
>> --- a/xen/arch/x86/include/asm/cpuid.h
>> +++ b/xen/arch/x86/include/asm/cpuid.h
>> @@ -2,10 +2,12 @@
>> #define __X86_CPUID_H__
>>
>> #include <asm/cpufeatureset.h>
>> +#include <asm/x86-vendors.h>
>>
>> -#include <xen/types.h>
>> +#include <xen/compiler.h>
>> #include <xen/kernel.h>
>> #include <xen/percpu.h>
>> +#include <xen/types.h>
>
> ... drop it. xen/kernel.h, for example, already gets it for you anyway.
Good call.
>
>> @@ -56,6 +58,51 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>> (IS_ENABLED(CONFIG_SHANGHAI) ? X86_VENDOR_SHANGHAI : 0) | \
>> (IS_ENABLED(CONFIG_HYGON) ? X86_VENDOR_HYGON : 0))
>>
>> +/*
>> + * When compiling Xen for a single vendor with no fallback vendor there's no
>> + * need no check the candidate. `vendor` is always a compile-time constant,
>> + * which means this all can fold into a constant boolean.
>
> DYM "`vendor` is always supposed to be a compile-time constant, ..." ?
Yes. We could guard against it not being so by having an initial branch where:
if ( !__builtin_constant_p(vendor) )
return candidate & filtered_vendor;
>
>> + * A runtime check at the time of CPUID probing guarantees we never run on
>> + * wrong hardware and another check when loading CPU policies guarantees we
>> + * never run policies for a vendor in another vendor's silicon.
>> + *
>> + * By the same token, the same folding can happen when no vendor is compiled
>> + * in and the fallback path is present.
>> + */
>> +static always_inline bool x86_vendor_is(uint8_t candidate, uint8_t vendor)
>
> I fear the comment, no matter that it's pretty large already, doesn't make
> clear how this function is to be used, i.e. how for this being an "is"
> predicate the two arguments should be chosen. My typical expectation would be
> for "is" predicates to apply to a single property, with other parameters (if
> any) only being auxiliary ones. Maybe it would already help if the first
> parameter wasn't named "candidate" but e.g. "actual" (from looking at just
> the next patch). Or maybe (depending on the number of possible different
> inputs for the first parameter) there want to be a few wrappers, so the
> "single property" aspect would be achieved at use sites.
>
> Then I see no reason for the parameters to be other than unsigned int. (Same
> for the local variable then, obviously.)
I could also call it x86_vendor_in(), to mean it's a set membership check,
leaving its prototype as:
bool x86_vendor_in(unsigned int actual, unsigned int bitmap);
bitmap is a special kind because literal 0 has a special meaning (unknown). So
I'd expect x86_vendor_in(X86_VENDOR_UNKNOWN, X86_VENDOR_UNKNOWN) to return true
when UNKNOWN_CPU=y. One way to alleviate complexity would be to promote the
unknown case to a first-class bit. It's not like it's zero for any good reason.
As for the what goes in the first parameter, it's invariably the x86_vendor
field of cpuinfo_x86 (for boot_cpu_data), or of any cpu_policy.
This is meant to replace checks on vendors, so a natural (and universally used)
pattern across the codebase is to have a runtime variable checked against a
constant. Here's a longer list of patterns and expected transformations.
from: cp->x86_vendor == X86_VENDOR_AMD
to: x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD)
from: cp->x86_vendor != X86_VENDOR_AMD
to: !x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD)
from: cp->x86_vendor & X86_VENDOR_AMD
to: x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD)
from: cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)
to: x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON)
from: !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON))
to: !x86_vendor_is(cp->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON)
from: cp->x86_vendor == X86_VENDOR_UNKNOWN
to: x86_vendor_is(cp->x86_vendor, X86_VENDOR_UNKNOWN)
And switch statements converted to if-elseif chains.
With the second argument alaways being a constant there's j
> I'm further uncertain this is a good place for the function. In the old days
> it may have been, but cpuid.[ch] now are only about guest exposure of CPUID,
This function would be used for both cpuinfo_x86 and cpu policies. Either here
or cpufeature.h works. I used to have it in x86-vendors.h, but that's exposed
to the toolstack and nothing would come out of doing that.
> when this predicate is intended to be used for both host and guest. (As I
> realize only now, this also applies to the addition patch 1 does.) One
> might think processor.h might be a good home, but we're actually trying to
> slim that one down. So one of cpufeature.h and cpufeatures.h, I guess. (Maybe
> other x86 maintainers also have thoughts here.)
>
>> +{
>> + uint8_t filtered_vendor = vendor & X86_ENABLED_VENDORS;
>> +
>> + if ( vendor == X86_VENDOR_UNKNOWN )
>> + {
>> + if ( IS_ENABLED(CONFIG_UNKNOWN_CPU) )
>> + /* no-vendor optimisation */
>
> Nit: Comment style (also again below).
>
>> + return X86_ENABLED_VENDORS ? vendor == candidate : true;
>
> With the surrounding if() this effectively (and more explicitly) is
>
> return X86_ENABLED_VENDORS ? candidate == X86_VENDOR_UNKNOWN : true;
Sure.
>
> First: Would one ever pass X86_VENDOR_UNKNOWN for "vendor"? The next patch,
> for example, specifically doesn't.
I don't think so. There's definitely not any existing case atm. Still, I think
it's better to preserve the invariant that the follwing transformation:
from: cp->x86_vendor == X86_VENDOR_X
to: x86_vendor_is(cp->x86_vendor, X86_VENDOR_X)
holds for every vendor variant in the "everything compiled-in" case.
> And then why not shorter as
> return !X86_ENABLED_VENDORS || candidate == X86_VENDOR_UNKNOWN;
Because I didn't think of it. I like your version better.
>
> Which raises the next question: Should we even allow a hypervisor to be built
> with X86_ENABLED_VENDORS == 0?
That's the most extreme case of "should we boot on a CPU known CPU vendor that
has been compiled out?", the current code in the RFC uses the unknown vendor
as fallback. We could also panic. We could be trying to exercise the
"no assumptions about the vendor" paths.
It's a policy decision for you (x86 mantainers) to take. I personally think the
default path is silly in this day and age and we could get rid of it entirely.
Without it X86_ENABLED_VENDORS=0 would be indeed illegal. On that topic...
> Plus, question more on patch 1, what's the (useful) difference between a build with all vendors set to N and
> (a) UNKNOWN_CPU=n vs (b) UNKNOWN_CPU=y? With all vendor support explicitly
> turned off, all CPUs are going to be "unknown".
... (a) causes a panic because all vendor go on the unknown path that leads to
such panic, (b) has them run as if they belonged to a new unknown vendor FOOBAR.
You could do (b) to exercise the paths on unknown vendors. Or we could get rid
of it altogether and panic on compiled-out vendors. Kconfig is a middle-ground
where the default CPU path serves a not-so-theoretical purpose.
>
>> +
>> + /* unknown-vendor-elimination optimisation */
>> + return false;
>> + }
>> +
>> + /* single-vendor optimisation */
>> + if ( !IS_ENABLED(CONFIG_UNKNOWN_CPU) &&
>> + (ISOLATE_LSB(X86_ENABLED_VENDORS) == X86_ENABLED_VENDORS) )
>> + return filtered_vendor == X86_ENABLED_VENDORS;
>> +
>> + /* compiled-out-vendor-elimination optimisation */
>> + if ( !filtered_vendor )
>> + return false;
>> +
>> + /*
>> + * When checking against a single vendor, perform an equality check, as
>> + * it yields (marginally) better codegen
>> + */
>> + if ( ISOLATE_LSB(filtered_vendor) == filtered_vendor )
>
> So one may pass a combination of multiple vendors for "vendor"? Is so, why
> is the parameter name singular?
Yes, it's a bitmap. Parameter could be in fact "bitmap", except the 0 case is
a special.
>
>> + return filtered_vendor == candidate ;
>
> Nit: Stray blank.
Oops.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig
2025-11-27 12:36 ` Alejandro Vallejo
@ 2025-11-27 13:19 ` Jan Beulich
2025-11-27 13:44 ` Alejandro Vallejo
0 siblings, 1 reply; 37+ messages in thread
From: Jan Beulich @ 2025-11-27 13:19 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 27.11.2025 13:36, Alejandro Vallejo wrote:
> On Thu Nov 27, 2025 at 10:43 AM CET, Jan Beulich wrote:
>> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>>> --- a/xen/arch/x86/Kconfig.cpu
>>> +++ b/xen/arch/x86/Kconfig.cpu
>>> @@ -19,4 +19,49 @@ config INTEL
>>> May be turned off in builds targetting other vendors. Otherwise,
>>> must be enabled for Xen to work suitably on Intel platforms.
>>>
>>> +config HYGON
>>> + bool "Support Hygon CPUs"
>>> + depends on AMD
>>> + default y
>>> + help
>>> + Detection, tunings and quirks for Hygon platforms.
>>> +
>>> + May be turned off in builds targetting other vendors. Otherwise,
>>> + must be enabled for Xen to work suitably on Hygon platforms.
>>> +
>>> +
>>> +config CENTAUR
>>> + bool "Support Centaur CPUs"
>>> + depends on INTEL
>>> + default y
>>> + help
>>> + Detection, tunings and quirks for Centaur platforms.
>>> +
>>> + May be turned off in builds targetting other vendors. Otherwise,
>>> + must be enabled for Xen to work suitably on Centaur platforms.
>>> +
>>> +config SHANGHAI
>>> + bool "Support Shanghai CPUs"
>>> + depends on INTEL
>>> + default y
>>> + help
>>> + Detection, tunings and quirks for Shanghai platforms.
>>> +
>>> + May be turned off in builds targetting other vendors. Otherwise,
>>> + must be enabled for Xen to work suitably on Shanghai platforms.
>>> +
>>> +config UNKNOWN_CPU
>>> + bool "Support unknown CPUs"
>>
>> "Unknown CPUs" can be of two kinds: Such of vendors we don't explicitly support,
>> and such of vendors we do explicitly support, but where we aren't aware of the
>> particular model. This needs to be unambiguous here, perhaps by it becoming
>> UNKNOWN_CPU_VENDOR (and the prompt changing accordingly).
>
> Right, what I do in this RFC is have compiled-out vendors fall back onto the
> unknown vendor path. Because it really is unknown to the binary.
>
> I could call it GENERIC_CPU_VENDOR, or anything else, but the main question
> is whether a toggle for this seems acceptable upstream. I don't see obvious
> drawbacks.
I'd recommend against "generic" or anything alike, as it'll rather suggest any
vendor's CPU will work reasonably. I'm fine with "unknown", just that the nature
of the unknown-ness needs making unambiguous.
>>> --- a/xen/arch/x86/cpu/common.c
>>> +++ b/xen/arch/x86/cpu/common.c
>>> @@ -118,7 +118,7 @@ static void cf_check default_init(struct cpuinfo_x86 * c)
>>> __clear_bit(X86_FEATURE_SEP, c->x86_capability);
>>> }
>>>
>>> -static const struct cpu_dev __initconst_cf_clobber __used default_cpu = {
>>> +static const struct cpu_dev __initconst_cf_clobber default_cpu = {
>>
>> This change isn't explained in the description. __used here was introduced not
>> all this long ago together with __initconst_cf_clobber. Maybe this really was
>> a mistake, but if so it's correction should be explained.
>
> It wasn't clear to me why __used was there (I assume it shouldn't have been),
> but it definitely clashes with the intent of having it gone when UNKNOWN_CPU=n.
>
> If __used was misplaced to begin with I'm happy to get rid of it in a separate
> patch. I don't think it warrants a Fixes tag, though.
I can only vaguely reconstruct that it may have been put there so the
.init.rodata.cf_clobber entry wouldn't go away. But as long as the compiler
also eliminates the function pointed at, that would be of no concern.
>>> @@ -340,7 +340,8 @@ void __init early_cpu_init(bool verbose)
>>> *(u32 *)&c->x86_vendor_id[8] = ecx;
>>> *(u32 *)&c->x86_vendor_id[4] = edx;
>>>
>>> - c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
>>> + c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx) &
>>> + X86_ENABLED_VENDORS;
>>
>> May I suggest the & to move ...
>>
>>> switch (c->x86_vendor) {
>>
>> ... here? Yes, you panic() below, but I see no reason to store inaccurate
>> data when that's easy to avoid.
>
> That's intentional. Otherwise further checks of c->x86_vendor in other parts of
> the code may not go through the same branch as the one here.
Hmm. I would kind of expect x86_vendor_is() to be capable of dealing with
that.
Jan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 04/11] x86: Refactor early vendor lookup code to use x86_vendor_is()
2025-11-27 10:51 ` Jan Beulich
@ 2025-11-27 13:35 ` Alejandro Vallejo
0 siblings, 0 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-27 13:35 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On Thu Nov 27, 2025 at 11:51 AM CET, Jan Beulich wrote:
> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>> Move the "unknown" vendor ahead of all others and have it NOT rely
>> on x86_vendor_is(), as that would yield incorrect values for the
>> single-vendor+no-fallback case when running in incorrect hardware
>> (because x86_vendor_is() becomes a folded constant of the single
>> compiled-in vendor).
>>
>> This is one of the two places where x86_vendor_is() cannot be used,
>> along with the compatibility check on loaded guest CPU policies.
>>
>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>> ---
>> xen/arch/x86/cpu/common.c | 31 +++++++++++++++++++++++--------
>> 1 file changed, 23 insertions(+), 8 deletions(-)
>>
>> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
>> index 393c30227f..c0c3606dd2 100644
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -342,23 +342,38 @@ void __init early_cpu_init(bool verbose)
>>
>> c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx) &
>> X86_ENABLED_VENDORS;
>> - switch (c->x86_vendor) {
>> - case X86_VENDOR_INTEL: intel_unlock_cpuid_leaves(c);
>> - actual_cpu = intel_cpu_dev; break;
>> - case X86_VENDOR_AMD: actual_cpu = amd_cpu_dev; break;
>> - case X86_VENDOR_CENTAUR: actual_cpu = centaur_cpu_dev; break;
>> - case X86_VENDOR_SHANGHAI: actual_cpu = shanghai_cpu_dev; break;
>> - case X86_VENDOR_HYGON: actual_cpu = hygon_cpu_dev; break;
>> - default:
>> +
>> + /*
>> + * We can't rely on x86_vendor_is() here due to the single-vendor
>> + * optimisation. It makes x86_vendor_is(x, y) rely on the constant `y`
>> + * matching the single vendor Xen was compiled for and ignore the
>> + * runtime variable `x`. In order to preserve sanity we must assert here
>> + * that we never boot such a build in a CPU from another vendor, or
>> + * major chaos would ensue.
>> + */
>> + if (c->x86_vendor == X86_VENDOR_UNKNOWN)
>> + {
>
> Nit: No mix of styles please. Here it wants to be Linux style.
>
>> if (verbose || !IS_ENABLED(CONFIG_UNKNOWN_CPU))
>> printk(XENLOG_ERR
>> "Unrecognised or unsupported CPU vendor '%.12s'\n",
>> c->x86_vendor_id);
>> +
>> if (!IS_ENABLED(CONFIG_UNKNOWN_CPU))
>> panic("Cannot run in unknown/compiled-out CPU vendor.\n");
>>
>> actual_cpu = default_cpu;
>> }
>> + else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_INTEL)) {
>> + intel_unlock_cpuid_leaves(c);
>> + actual_cpu = intel_cpu_dev;
>> + } else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD))
>> + actual_cpu = amd_cpu_dev;
>> + else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_CENTAUR))
>> + actual_cpu = centaur_cpu_dev;
>> + else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_SHANGHAI))
>> + actual_cpu = shanghai_cpu_dev;
>> + else if (x86_vendor_is(c->x86_vendor, X86_VENDOR_HYGON))
>> + actual_cpu = hygon_cpu_dev;
>
> If it needs to be like this, then so be it, but I view it as a downside to
> not be able to use switch() anymore. It's not quite clear to me though what
> extra gains the transformation brings. The masking by X86_ENABLED_VENDORS
> already does most of what you want, and X86_VENDOR_UNKNOWN continues to be
> handled separately anyway.
In this particular switch that's the case, but it's not so elsewhere.
Any switch would resolve to an unavoidable runtime check with compiled-out
branches left out (so long as the variable is ANDed with the enabled vendors
mask, which it currently isn't). However, this still forces the compiler to emit
a runtime check in case the vendor is actually zero. The conversion to if-elseif
ensures we can statically decide a branch at compile time and remove all others
(including the default one).
On a more stylistic note, though obviously this is all subjective, the patch
that migrates switch statements to if-elseif chains shaves 100 LoC and manages
to squash multiple checks onto single ones by making use of the bitmap nature of
the vendor field. The gains there are marginal, so I don't care that much, about
that but it's a measurable benefit in LoC and a (small) benefit in codegen.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it
2025-11-27 13:15 ` Alejandro Vallejo
@ 2025-11-27 13:37 ` Jan Beulich
2025-11-27 14:06 ` Alejandro Vallejo
2025-11-27 20:36 ` Andrew Cooper
1 sibling, 1 reply; 37+ messages in thread
From: Jan Beulich @ 2025-11-27 13:37 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 27.11.2025 14:15, Alejandro Vallejo wrote:
> On Thu Nov 27, 2025 at 11:46 AM CET, Jan Beulich wrote:
>> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>>> + * A runtime check at the time of CPUID probing guarantees we never run on
>>> + * wrong hardware and another check when loading CPU policies guarantees we
>>> + * never run policies for a vendor in another vendor's silicon.
>>> + *
>>> + * By the same token, the same folding can happen when no vendor is compiled
>>> + * in and the fallback path is present.
>>> + */
>>> +static always_inline bool x86_vendor_is(uint8_t candidate, uint8_t vendor)
>>
>> I fear the comment, no matter that it's pretty large already, doesn't make
>> clear how this function is to be used, i.e. how for this being an "is"
>> predicate the two arguments should be chosen. My typical expectation would be
>> for "is" predicates to apply to a single property, with other parameters (if
>> any) only being auxiliary ones. Maybe it would already help if the first
>> parameter wasn't named "candidate" but e.g. "actual" (from looking at just
>> the next patch). Or maybe (depending on the number of possible different
>> inputs for the first parameter) there want to be a few wrappers, so the
>> "single property" aspect would be achieved at use sites.
>>
>> Then I see no reason for the parameters to be other than unsigned int. (Same
>> for the local variable then, obviously.)
>
> I could also call it x86_vendor_in(), to mean it's a set membership check,
> leaving its prototype as:
>
> bool x86_vendor_in(unsigned int actual, unsigned int bitmap);
>
> bitmap is a special kind because literal 0 has a special meaning (unknown). So
>
> I'd expect x86_vendor_in(X86_VENDOR_UNKNOWN, X86_VENDOR_UNKNOWN) to return true
> when UNKNOWN_CPU=y. One way to alleviate complexity would be to promote the
> unknown case to a first-class bit. It's not like it's zero for any good reason.
>
> As for the what goes in the first parameter, it's invariably the x86_vendor
> field of cpuinfo_x86 (for boot_cpu_data), or of any cpu_policy.
>
> This is meant to replace checks on vendors, so a natural (and universally used)
> pattern across the codebase is to have a runtime variable checked against a
> constant. Here's a longer list of patterns and expected transformations.
>
> from: cp->x86_vendor == X86_VENDOR_AMD
> to: x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD)
>
> from: cp->x86_vendor != X86_VENDOR_AMD
> to: !x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD)
>
> from: cp->x86_vendor & X86_VENDOR_AMD
> to: x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD)
>
> from: cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)
> to: x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON)
There's a mix of c and cp up from here, but I hope the distinction isn't
relevant in this context. What is relevant though is that you shouldn't
be using struct cpuinfo_x86's x86_vendor field anymore. See the struct
definition.
> from: !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON))
> to: !x86_vendor_is(cp->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON)
>
> from: cp->x86_vendor == X86_VENDOR_UNKNOWN
> to: x86_vendor_is(cp->x86_vendor, X86_VENDOR_UNKNOWN)
For it to be clear what the "is" applies to, all of the above would imo
better be x86_vendor_is(c, X86_VENDOR_...) or
x86_vendor_is(cp, X86_VENDOR_...) at the call sites. The c / cp are what
I called "auxiliary data" elsewhere, and the property checked clearly is
the 2nd argument. To achieve this you could introduce a wrapper macro,
which would do the de-ref of the ->vendor field. (As a prereq, struct
cpu_policy would then also need to gain a "vendor" alias of the present
"x86_vendor" field.)
> And switch statements converted to if-elseif chains.
I've voiced concern towards this elsewhere.
>> First: Would one ever pass X86_VENDOR_UNKNOWN for "vendor"? The next patch,
>> for example, specifically doesn't.
>
> I don't think so. There's definitely not any existing case atm. Still, I think
> it's better to preserve the invariant that the follwing transformation:
>
> from: cp->x86_vendor == X86_VENDOR_X
> to: x86_vendor_is(cp->x86_vendor, X86_VENDOR_X)
>
> holds for every vendor variant in the "everything compiled-in" case.
Otoh the code could be simplified if you simply rejected the passing of
X86_VENDOR_UNKNOWN.
>>> + /* single-vendor optimisation */
>>> + if ( !IS_ENABLED(CONFIG_UNKNOWN_CPU) &&
>>> + (ISOLATE_LSB(X86_ENABLED_VENDORS) == X86_ENABLED_VENDORS) )
>>> + return filtered_vendor == X86_ENABLED_VENDORS;
>>> +
>>> + /* compiled-out-vendor-elimination optimisation */
>>> + if ( !filtered_vendor )
>>> + return false;
>>> +
>>> + /*
>>> + * When checking against a single vendor, perform an equality check, as
>>> + * it yields (marginally) better codegen
>>> + */
>>> + if ( ISOLATE_LSB(filtered_vendor) == filtered_vendor )
>>
>> So one may pass a combination of multiple vendors for "vendor"? Is so, why
>> is the parameter name singular?
>
> Yes, it's a bitmap. Parameter could be in fact "bitmap", except the 0 case is
> a special.
We have empty bitmaps elsewhere, as a more or less special case, so this doesn't
look overly concerning.
Jan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig
2025-11-27 13:19 ` Jan Beulich
@ 2025-11-27 13:44 ` Alejandro Vallejo
2025-11-27 14:00 ` Jan Beulich
0 siblings, 1 reply; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-27 13:44 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On Thu Nov 27, 2025 at 2:19 PM CET, Jan Beulich wrote:
> On 27.11.2025 13:36, Alejandro Vallejo wrote:
>> On Thu Nov 27, 2025 at 10:43 AM CET, Jan Beulich wrote:
>>> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>>>> --- a/xen/arch/x86/Kconfig.cpu
>>>> +++ b/xen/arch/x86/Kconfig.cpu
>>>> @@ -19,4 +19,49 @@ config INTEL
>>>> May be turned off in builds targetting other vendors. Otherwise,
>>>> must be enabled for Xen to work suitably on Intel platforms.
>>>>
>>>> +config HYGON
>>>> + bool "Support Hygon CPUs"
>>>> + depends on AMD
>>>> + default y
>>>> + help
>>>> + Detection, tunings and quirks for Hygon platforms.
>>>> +
>>>> + May be turned off in builds targetting other vendors. Otherwise,
>>>> + must be enabled for Xen to work suitably on Hygon platforms.
>>>> +
>>>> +
>>>> +config CENTAUR
>>>> + bool "Support Centaur CPUs"
>>>> + depends on INTEL
>>>> + default y
>>>> + help
>>>> + Detection, tunings and quirks for Centaur platforms.
>>>> +
>>>> + May be turned off in builds targetting other vendors. Otherwise,
>>>> + must be enabled for Xen to work suitably on Centaur platforms.
>>>> +
>>>> +config SHANGHAI
>>>> + bool "Support Shanghai CPUs"
>>>> + depends on INTEL
>>>> + default y
>>>> + help
>>>> + Detection, tunings and quirks for Shanghai platforms.
>>>> +
>>>> + May be turned off in builds targetting other vendors. Otherwise,
>>>> + must be enabled for Xen to work suitably on Shanghai platforms.
>>>> +
>>>> +config UNKNOWN_CPU
>>>> + bool "Support unknown CPUs"
>>>
>>> "Unknown CPUs" can be of two kinds: Such of vendors we don't explicitly support,
>>> and such of vendors we do explicitly support, but where we aren't aware of the
>>> particular model. This needs to be unambiguous here, perhaps by it becoming
>>> UNKNOWN_CPU_VENDOR (and the prompt changing accordingly).
>>
>> Right, what I do in this RFC is have compiled-out vendors fall back onto the
>> unknown vendor path. Because it really is unknown to the binary.
>>
>> I could call it GENERIC_CPU_VENDOR, or anything else, but the main question
>> is whether a toggle for this seems acceptable upstream. I don't see obvious
>> drawbacks.
>
> I'd recommend against "generic" or anything alike, as it'll rather suggest any
> vendor's CPU will work reasonably. I'm fine with "unknown", just that the nature
> of the unknown-ness needs making unambiguous.
Got it, if UNKNOWN_CPU_VENDOR sounds better I'm fine with that.
What are your thoughts on the panic-on-compiled-out-vendor vs use-unknown?
>
>>>> --- a/xen/arch/x86/cpu/common.c
>>>> +++ b/xen/arch/x86/cpu/common.c
>>>> @@ -118,7 +118,7 @@ static void cf_check default_init(struct cpuinfo_x86 * c)
>>>> __clear_bit(X86_FEATURE_SEP, c->x86_capability);
>>>> }
>>>>
>>>> -static const struct cpu_dev __initconst_cf_clobber __used default_cpu = {
>>>> +static const struct cpu_dev __initconst_cf_clobber default_cpu = {
>>>
>>> This change isn't explained in the description. __used here was introduced not
>>> all this long ago together with __initconst_cf_clobber. Maybe this really was
>>> a mistake, but if so it's correction should be explained.
>>
>> It wasn't clear to me why __used was there (I assume it shouldn't have been),
>> but it definitely clashes with the intent of having it gone when UNKNOWN_CPU=n.
>>
>> If __used was misplaced to begin with I'm happy to get rid of it in a separate
>> patch. I don't think it warrants a Fixes tag, though.
>
> I can only vaguely reconstruct that it may have been put there so the
> .init.rodata.cf_clobber entry wouldn't go away. But as long as the compiler
> also eliminates the function pointed at, that would be of no concern.
ack.
>
>>>> @@ -340,7 +340,8 @@ void __init early_cpu_init(bool verbose)
>>>> *(u32 *)&c->x86_vendor_id[8] = ecx;
>>>> *(u32 *)&c->x86_vendor_id[4] = edx;
>>>>
>>>> - c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
>>>> + c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx) &
>>>> + X86_ENABLED_VENDORS;
>>>
>>> May I suggest the & to move ...
>>>
>>>> switch (c->x86_vendor) {
>>>
>>> ... here? Yes, you panic() below, but I see no reason to store inaccurate
>>> data when that's easy to avoid.
>>
>> That's intentional. Otherwise further checks of c->x86_vendor in other parts of
>> the code may not go through the same branch as the one here.
>
> Hmm. I would kind of expect x86_vendor_is() to be capable of dealing with
> that.
Sure, but that's not done until the end of the series. and otherwise I'd be
introducing the fallback behaviour in some places and not others. I could
remove this AND at the end. Or remove it altogether if we go for a panic on
compiled-out vendor strategy.
Cheers,
Aljandro
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig
2025-11-27 13:44 ` Alejandro Vallejo
@ 2025-11-27 14:00 ` Jan Beulich
0 siblings, 0 replies; 37+ messages in thread
From: Jan Beulich @ 2025-11-27 14:00 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 27.11.2025 14:44, Alejandro Vallejo wrote:
> On Thu Nov 27, 2025 at 2:19 PM CET, Jan Beulich wrote:
>> On 27.11.2025 13:36, Alejandro Vallejo wrote:
>>> On Thu Nov 27, 2025 at 10:43 AM CET, Jan Beulich wrote:
>>>> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>>>>> --- a/xen/arch/x86/Kconfig.cpu
>>>>> +++ b/xen/arch/x86/Kconfig.cpu
>>>>> @@ -19,4 +19,49 @@ config INTEL
>>>>> May be turned off in builds targetting other vendors. Otherwise,
>>>>> must be enabled for Xen to work suitably on Intel platforms.
>>>>>
>>>>> +config HYGON
>>>>> + bool "Support Hygon CPUs"
>>>>> + depends on AMD
>>>>> + default y
>>>>> + help
>>>>> + Detection, tunings and quirks for Hygon platforms.
>>>>> +
>>>>> + May be turned off in builds targetting other vendors. Otherwise,
>>>>> + must be enabled for Xen to work suitably on Hygon platforms.
>>>>> +
>>>>> +
>>>>> +config CENTAUR
>>>>> + bool "Support Centaur CPUs"
>>>>> + depends on INTEL
>>>>> + default y
>>>>> + help
>>>>> + Detection, tunings and quirks for Centaur platforms.
>>>>> +
>>>>> + May be turned off in builds targetting other vendors. Otherwise,
>>>>> + must be enabled for Xen to work suitably on Centaur platforms.
>>>>> +
>>>>> +config SHANGHAI
>>>>> + bool "Support Shanghai CPUs"
>>>>> + depends on INTEL
>>>>> + default y
>>>>> + help
>>>>> + Detection, tunings and quirks for Shanghai platforms.
>>>>> +
>>>>> + May be turned off in builds targetting other vendors. Otherwise,
>>>>> + must be enabled for Xen to work suitably on Shanghai platforms.
>>>>> +
>>>>> +config UNKNOWN_CPU
>>>>> + bool "Support unknown CPUs"
>>>>
>>>> "Unknown CPUs" can be of two kinds: Such of vendors we don't explicitly support,
>>>> and such of vendors we do explicitly support, but where we aren't aware of the
>>>> particular model. This needs to be unambiguous here, perhaps by it becoming
>>>> UNKNOWN_CPU_VENDOR (and the prompt changing accordingly).
>>>
>>> Right, what I do in this RFC is have compiled-out vendors fall back onto the
>>> unknown vendor path. Because it really is unknown to the binary.
>>>
>>> I could call it GENERIC_CPU_VENDOR, or anything else, but the main question
>>> is whether a toggle for this seems acceptable upstream. I don't see obvious
>>> drawbacks.
>>
>> I'd recommend against "generic" or anything alike, as it'll rather suggest any
>> vendor's CPU will work reasonably. I'm fine with "unknown", just that the nature
>> of the unknown-ness needs making unambiguous.
>
> Got it, if UNKNOWN_CPU_VENDOR sounds better I'm fine with that.
>
> What are your thoughts on the panic-on-compiled-out-vendor vs use-unknown?
Both have benefits and downsides, I think. The choice may need to be another
Kconfig or command line option. I'm also curious what other x86 maintainers
may think.
Jan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it
2025-11-27 13:37 ` Jan Beulich
@ 2025-11-27 14:06 ` Alejandro Vallejo
2025-11-27 14:14 ` Jan Beulich
0 siblings, 1 reply; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-27 14:06 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On Thu Nov 27, 2025 at 2:37 PM CET, Jan Beulich wrote:
> On 27.11.2025 14:15, Alejandro Vallejo wrote:
>> On Thu Nov 27, 2025 at 11:46 AM CET, Jan Beulich wrote:
>>> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>>>> + * A runtime check at the time of CPUID probing guarantees we never run on
>>>> + * wrong hardware and another check when loading CPU policies guarantees we
>>>> + * never run policies for a vendor in another vendor's silicon.
>>>> + *
>>>> + * By the same token, the same folding can happen when no vendor is compiled
>>>> + * in and the fallback path is present.
>>>> + */
>>>> +static always_inline bool x86_vendor_is(uint8_t candidate, uint8_t vendor)
>>>
>>> I fear the comment, no matter that it's pretty large already, doesn't make
>>> clear how this function is to be used, i.e. how for this being an "is"
>>> predicate the two arguments should be chosen. My typical expectation would be
>>> for "is" predicates to apply to a single property, with other parameters (if
>>> any) only being auxiliary ones. Maybe it would already help if the first
>>> parameter wasn't named "candidate" but e.g. "actual" (from looking at just
>>> the next patch). Or maybe (depending on the number of possible different
>>> inputs for the first parameter) there want to be a few wrappers, so the
>>> "single property" aspect would be achieved at use sites.
>>>
>>> Then I see no reason for the parameters to be other than unsigned int. (Same
>>> for the local variable then, obviously.)
>>
>> I could also call it x86_vendor_in(), to mean it's a set membership check,
>> leaving its prototype as:
>>
>> bool x86_vendor_in(unsigned int actual, unsigned int bitmap);
>>
>> bitmap is a special kind because literal 0 has a special meaning (unknown). So
>>
>> I'd expect x86_vendor_in(X86_VENDOR_UNKNOWN, X86_VENDOR_UNKNOWN) to return true
>> when UNKNOWN_CPU=y. One way to alleviate complexity would be to promote the
>> unknown case to a first-class bit. It's not like it's zero for any good reason.
>>
>> As for the what goes in the first parameter, it's invariably the x86_vendor
>> field of cpuinfo_x86 (for boot_cpu_data), or of any cpu_policy.
>>
>> This is meant to replace checks on vendors, so a natural (and universally used)
>> pattern across the codebase is to have a runtime variable checked against a
>> constant. Here's a longer list of patterns and expected transformations.
>>
>> from: cp->x86_vendor == X86_VENDOR_AMD
>> to: x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD)
>>
>> from: cp->x86_vendor != X86_VENDOR_AMD
>> to: !x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD)
>>
>> from: cp->x86_vendor & X86_VENDOR_AMD
>> to: x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD)
>>
>> from: cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)
>> to: x86_vendor_is(c->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON)
>
> There's a mix of c and cp up from here, but I hope the distinction isn't
> relevant in this context. What is relevant though is that you shouldn't
> be using struct cpuinfo_x86's x86_vendor field anymore. See the struct
> definition.
It indeed isn't relevant. Fair enough, I was misled by its use in cpu/common.c
>
>> from: !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON))
>> to: !x86_vendor_is(cp->x86_vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON)
>>
>> from: cp->x86_vendor == X86_VENDOR_UNKNOWN
>> to: x86_vendor_is(cp->x86_vendor, X86_VENDOR_UNKNOWN)
>
> For it to be clear what the "is" applies to, all of the above would imo
> better be x86_vendor_is(c, X86_VENDOR_...) or
> x86_vendor_is(cp, X86_VENDOR_...) at the call sites. The c / cp are what
> I called "auxiliary data" elsewhere, and the property checked clearly is
> the 2nd argument. To achieve this you could introduce a wrapper macro,
> which would do the de-ref of the ->vendor field. (As a prereq, struct
> cpu_policy would then also need to gain a "vendor" alias of the present
> "x86_vendor" field.)
I'd be fine with that. It'd also trim the diffstat substantially by allowing
a number of cases to become single-line checks rather than splitting
>
>> And switch statements converted to if-elseif chains.
>
> I've voiced concern towards this elsewhere.
>
>>> First: Would one ever pass X86_VENDOR_UNKNOWN for "vendor"? The next patch,
>>> for example, specifically doesn't.
>>
>> I don't think so. There's definitely not any existing case atm. Still, I think
>> it's better to preserve the invariant that the follwing transformation:
>>
>> from: cp->x86_vendor == X86_VENDOR_X
>> to: x86_vendor_is(cp->x86_vendor, X86_VENDOR_X)
>>
>> holds for every vendor variant in the "everything compiled-in" case.
>
> Otoh the code could be simplified if you simply rejected the passing of
> X86_VENDOR_UNKNOWN.
How would this rejection go? Something like this at the top?
if ( vendor == X86_VENDOR_UNKNOWN )
BUG();
I'd rather have something that causes a compile-time error, but I'm not sure
how to cause a compile time failure when a constant matches FOO.
Surely there must be a way...
>
>>>> + /* single-vendor optimisation */
>>>> + if ( !IS_ENABLED(CONFIG_UNKNOWN_CPU) &&
>>>> + (ISOLATE_LSB(X86_ENABLED_VENDORS) == X86_ENABLED_VENDORS) )
>>>> + return filtered_vendor == X86_ENABLED_VENDORS;
>>>> +
>>>> + /* compiled-out-vendor-elimination optimisation */
>>>> + if ( !filtered_vendor )
>>>> + return false;
>>>> +
>>>> + /*
>>>> + * When checking against a single vendor, perform an equality check, as
>>>> + * it yields (marginally) better codegen
>>>> + */
>>>> + if ( ISOLATE_LSB(filtered_vendor) == filtered_vendor )
>>>
>>> So one may pass a combination of multiple vendors for "vendor"? Is so, why
>>> is the parameter name singular?
>>
>> Yes, it's a bitmap. Parameter could be in fact "bitmap", except the 0 case is
>> a special.
>
> We have empty bitmaps elsewhere, as a more or less special case, so this doesn't
> look overly concerning.
>
> Jan
Ack,
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it
2025-11-27 14:06 ` Alejandro Vallejo
@ 2025-11-27 14:14 ` Jan Beulich
0 siblings, 0 replies; 37+ messages in thread
From: Jan Beulich @ 2025-11-27 14:14 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 27.11.2025 15:06, Alejandro Vallejo wrote:
> On Thu Nov 27, 2025 at 2:37 PM CET, Jan Beulich wrote:
>> On 27.11.2025 14:15, Alejandro Vallejo wrote:
>>> On Thu Nov 27, 2025 at 11:46 AM CET, Jan Beulich wrote:
>>>> First: Would one ever pass X86_VENDOR_UNKNOWN for "vendor"? The next patch,
>>>> for example, specifically doesn't.
>>>
>>> I don't think so. There's definitely not any existing case atm. Still, I think
>>> it's better to preserve the invariant that the follwing transformation:
>>>
>>> from: cp->x86_vendor == X86_VENDOR_X
>>> to: x86_vendor_is(cp->x86_vendor, X86_VENDOR_X)
>>>
>>> holds for every vendor variant in the "everything compiled-in" case.
>>
>> Otoh the code could be simplified if you simply rejected the passing of
>> X86_VENDOR_UNKNOWN.
>
> How would this rejection go? Something like this at the top?
>
> if ( vendor == X86_VENDOR_UNKNOWN )
> BUG();
>
> I'd rather have something that causes a compile-time error, but I'm not sure
> how to cause a compile time failure when a constant matches FOO.
>
> Surely there must be a way...
Take a look at BUILD_ERROR(), which I think may fit here.
Jan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 02/11] x86: Reject CPU policies with vendors other than the host's
2025-11-26 16:44 ` [RFC PATCH 02/11] x86: Reject CPU policies with vendors other than the host's Alejandro Vallejo
@ 2025-11-27 20:15 ` Andrew Cooper
0 siblings, 0 replies; 37+ messages in thread
From: Andrew Cooper @ 2025-11-27 20:15 UTC (permalink / raw)
To: Alejandro Vallejo, xen-devel
Cc: andrew.cooper3, Jan Beulich, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini
On 26/11/2025 4:44 pm, Alejandro Vallejo wrote:
> While in principle it's possible to have a vendor virtualising another,
> this is fairly tricky in practice. Not doing so enables certain
> optimisations with regards to vendor checks in later patches.
>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
> I originally had a Kconfig option to allow cross-vendor virt and
> conditionally disable the check on policy compatibility. In practice,
> I suspect there's 0% of people that would want that, so I decided to
> simply remove it altogether. Happy to put it back if there's anyone
> interested.
We've debated dropping cross-vendor support several times.
Prior to speculation, it was actually the case that PV guests worked
fairly well (Xen abstracts away the details surprisingly well), and HVM
functioned to a first approximation (emulating the SYS* instructions is
horrible).
After speculation, there's no hope in hell of getting a viable VM in a
cross-vendor environment, so we should drop it.
In addition to this hunk, you'll want to drop is_cross_vendor(),
should_emulate logic in hvm_ud_intercept(), and the cross-vendor aspect
of determining #UD interception in {svm,vmx}_cpuid_policy_changed().
Also, I think you can clean up MSR_IA32_PLATFORM_ID / MSR_AMD_PATCHLEVEL
in guest_rdmsr() which have some pretty well hidden is-cross-vendor checks.
Also you'll want a note in CHANGELOG.md.
I'd also suggest splitting it out of this series. It's quite different
to the rest of the series.
~Andrew
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it
2025-11-27 13:15 ` Alejandro Vallejo
2025-11-27 13:37 ` Jan Beulich
@ 2025-11-27 20:36 ` Andrew Cooper
2025-11-28 9:08 ` Alejandro Vallejo
1 sibling, 1 reply; 37+ messages in thread
From: Andrew Cooper @ 2025-11-27 20:36 UTC (permalink / raw)
To: Alejandro Vallejo, Jan Beulich
Cc: andrew.cooper3, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 27/11/2025 1:15 pm, Alejandro Vallejo wrote:
>> Which raises the next question: Should we even allow a hypervisor to be built
>> with X86_ENABLED_VENDORS == 0?
> That's the most extreme case of "should we boot on a CPU known CPU vendor that
> has been compiled out?", the current code in the RFC uses the unknown vendor
> as fallback. We could also panic. We could be trying to exercise the
> "no assumptions about the vendor" paths.
>
> It's a policy decision for you (x86 mantainers) to take. I personally think the
> default path is silly in this day and age and we could get rid of it entirely.
> Without it X86_ENABLED_VENDORS=0 would be indeed illegal. On that topic...
We allow compiling out both PV and HVM, so Randconfig can search for
broken corners of the abstraction. The same principle applies here.
For running such a hypervisor, I can't see anything that would
fundamentally interfere with PV guests; PV guests are all architectural x86.
I don't like there being an explicit Kconfig option for UNKNOWN.
UNKNOWN should simply be "didn't match anything we compiled in".
~Andrew
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig
2025-11-26 16:44 ` [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
2025-11-27 9:43 ` Jan Beulich
2025-11-27 9:59 ` Jan Beulich
@ 2025-11-27 21:01 ` Andrew Cooper
2 siblings, 0 replies; 37+ messages in thread
From: Andrew Cooper @ 2025-11-27 21:01 UTC (permalink / raw)
To: Alejandro Vallejo, xen-devel
Cc: andrew.cooper3, Jan Beulich, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini
On 26/11/2025 4:44 pm, Alejandro Vallejo wrote:
> diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu
> index 5fb18db1aa..aaf70fb37b 100644
> --- a/xen/arch/x86/Kconfig.cpu
> +++ b/xen/arch/x86/Kconfig.cpu
> @@ -19,4 +19,49 @@ config INTEL
> May be turned off in builds targetting other vendors. Otherwise,
> must be enabled for Xen to work suitably on Intel platforms.
>
> +config HYGON
> + bool "Support Hygon CPUs"
> + depends on AMD
> + default y
> + help
> + Detection, tunings and quirks for Hygon platforms.
> +
> + May be turned off in builds targetting other vendors. Otherwise,
> + must be enabled for Xen to work suitably on Hygon platforms.
> +
> +
> +config CENTAUR
> + bool "Support Centaur CPUs"
> + depends on INTEL
> + default y
> + help
> + Detection, tunings and quirks for Centaur platforms.
> +
> + May be turned off in builds targetting other vendors. Otherwise,
> + must be enabled for Xen to work suitably on Centaur platforms.
> +
> +config SHANGHAI
> + bool "Support Shanghai CPUs"
> + depends on INTEL
> + default y
> + help
> + Detection, tunings and quirks for Shanghai platforms.
> +
> + May be turned off in builds targetting other vendors. Otherwise,
> + must be enabled for Xen to work suitably on Shanghai platforms.
Minor nit, but all these 3 should be select AMD/INTEL, not depends.
It's an implementation detail that they reach sideways into AMD/INTEL to
operate.
An end user in front of menuconfig wants to see all 5 to configure.
It's unreasonable to require them to know that they need to enable AMD
in order to be able to configure Hygon.
> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> index 37820a3a08..393c30227f 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -118,7 +118,7 @@ static void cf_check default_init(struct cpuinfo_x86 * c)
> __clear_bit(X86_FEATURE_SEP, c->x86_capability);
> }
>
> -static const struct cpu_dev __initconst_cf_clobber __used default_cpu = {
> +static const struct cpu_dev __initconst_cf_clobber default_cpu = {
To follow up on a different question, the __used came from 660f8a75013
but it does look wrong on a second look. It might have been a leftover
from an older revision of the patch.
~Andrew
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 00/11] x86 vendor check optimisations
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
` (10 preceding siblings ...)
2025-11-26 16:44 ` [RFC PATCH 11/11] x86: Migrate every remaining raw vendor check to x86_vendor_is() Alejandro Vallejo
@ 2025-11-27 22:11 ` Andrew Cooper
2025-11-28 9:18 ` Alejandro Vallejo
11 siblings, 1 reply; 37+ messages in thread
From: Andrew Cooper @ 2025-11-27 22:11 UTC (permalink / raw)
To: Alejandro Vallejo, xen-devel
Cc: andrew.cooper3, Jan Beulich, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini
On 26/11/2025 4:44 pm, Alejandro Vallejo wrote:
> Just knowing x86_vendor_is() is "good to have" is good enough as it enables our
> downstream to customise it with whatever optimisations we need.
>
> I also suspect other areas of the hypervisor could benefit from this meld of
> runtime+compiletime sort of checking, allowing transparent code removal.
>
> I'm thinking DOM0LESS_BOOT vs DOM0_BOOT vs PVSHIM_BOOT, or AMD_SVM vs INTEL_VMX
> in HVM-only builds, or family checks to have (i.e) a explicit "older-than-zen"
> Kconfig option with a similar approach on a family range check.
>
> This is maybe one of several such uses.
>
> So... thoughts? I'm definitely fond of the single-vendor bloat-o-meter output.
Having looked through the whole series, I'm not a massive fan of
converting the switch() statements, but it's the only way to do the
DCE. So be it.
I think x86_vendor_is(var, MASK) wants to become boot_vendor(MASK).
Most cases want the boot vendor, and those that appear to want something
else don't actually. When you disable the cross-vendor case (patch 2
pulled out ahead), then cp->vendor == boot_vendor and then you don't
need a variable to pass in.
This also reduces the verbosity of the new lines which is an improvement.
That said, this series also collides substantially with the Intel Fam
18/19 work, which is more urgent and needs backporting to 4.21. More
specifically, there are a bunch of changes which interfere with VFM
conversion, and for which I can't see an obvious DCE reason to have, so
I'm wondering if they were just part of "convert everything".
~Andrew
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it
2025-11-27 20:36 ` Andrew Cooper
@ 2025-11-28 9:08 ` Alejandro Vallejo
0 siblings, 0 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-28 9:08 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich
Cc: Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini, xen-devel
On Thu Nov 27, 2025 at 9:36 PM CET, Andrew Cooper wrote:
> On 27/11/2025 1:15 pm, Alejandro Vallejo wrote:
>>> Which raises the next question: Should we even allow a hypervisor to be built
>>> with X86_ENABLED_VENDORS == 0?
>> That's the most extreme case of "should we boot on a CPU known CPU vendor that
>> has been compiled out?", the current code in the RFC uses the unknown vendor
>> as fallback. We could also panic. We could be trying to exercise the
>> "no assumptions about the vendor" paths.
>>
>> It's a policy decision for you (x86 mantainers) to take. I personally think the
>> default path is silly in this day and age and we could get rid of it entirely.
>> Without it X86_ENABLED_VENDORS=0 would be indeed illegal. On that topic...
>
> We allow compiling out both PV and HVM, so Randconfig can search for
> broken corners of the abstraction. The same principle applies here.
That answers how to prevent that configuration from appearing in randconfig,
not whether that configuration should be valid or not. That's what I meant is a
policy decision.
As the RFC stands:
CONFIG POLICY
X86_ENABLED_VENDORS=0 | all boots use the default CPU
UNKNOWN_CPU=y |
X86_ENABLED_VENDORS=0 | panic on boot
UNKNOWN_CPU=n |
X86_ENABLED_VENDORS=<all> | current all-vendors-compiled-in
UNKNOWN_CPU=y |
X86_ENABLED_VENDORS=<all> | Marginal code removal in early_cpu_init() of
UNKNOWN_CPU=n | the default CPU branch.
X86_ENABLED_VENDORS=<more-than-1> | x86_vendor_is() folds into false when
UNKNOWN_CPU=<any> | checking against all-compiled-out vendors
| Also, some &-checks turn into ==-checks.
X86_ENABLED_VENDORS=<single> | Same as the one above.
UNKNOWN_CPU=y |
X86_ENABLED_VENDORS=<single> | x86_vendor_is() folds into constant true/false
UNKNOWN_CPU=n |
Does this sound like a credible approach? What could be different?
- Should we remove the "default" case altogether"? That removes the Kconfig
and simplifies matters somewhat.
- If the default case is to stay, should we allow running with no-explicit
vendors set?
>
> For running such a hypervisor, I can't see anything that would
> fundamentally interfere with PV guests; PV guests are all architectural x86.
You could also run HVM, seeing how SVM and VMX don't intrinsically require AMD
and INTEL respectively, but it wouldn't be pleasant, secure or safe. It would
be wonky at best. In general it's a path I wouldn't like to fallback to on the
basis that it's completely untested. I don't believe OSSTest ever exercised it
(I wouldn't even know how), nor GitlabCI, nor XenRT.
>
> I don't like there being an explicit Kconfig option for UNKNOWN.
> UNKNOWN should simply be "didn't match anything we compiled in".
What I require for the optimisation to fully work out is precisely the removal
of that the "didn't match anything we compiled in" as a plausible successful
boot scenario in order for all x86_vendor_is(x, X86_VENDOR_AMD) to fold into
"true". I could reverse the polarity and have a REQUIRE_KNOWN_VENDOR, but that's
a non additive option that clashes with allyesconfig/allnoconfig.
>
> ~Andrew
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 00/11] x86 vendor check optimisations
2025-11-27 22:11 ` [RFC PATCH 00/11] x86 vendor check optimisations Andrew Cooper
@ 2025-11-28 9:18 ` Alejandro Vallejo
0 siblings, 0 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-11-28 9:18 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: Jan Beulich, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini
On Thu Nov 27, 2025 at 11:11 PM CET, Andrew Cooper wrote:
> On 26/11/2025 4:44 pm, Alejandro Vallejo wrote:
>> Just knowing x86_vendor_is() is "good to have" is good enough as it enables our
>> downstream to customise it with whatever optimisations we need.
>>
>> I also suspect other areas of the hypervisor could benefit from this meld of
>> runtime+compiletime sort of checking, allowing transparent code removal.
>>
>> I'm thinking DOM0LESS_BOOT vs DOM0_BOOT vs PVSHIM_BOOT, or AMD_SVM vs INTEL_VMX
>> in HVM-only builds, or family checks to have (i.e) a explicit "older-than-zen"
>> Kconfig option with a similar approach on a family range check.
>>
>> This is maybe one of several such uses.
>>
>> So... thoughts? I'm definitely fond of the single-vendor bloat-o-meter output.
>
> Having looked through the whole series, I'm not a massive fan of
> converting the switch() statements, but it's the only way to do the
> DCE. So be it.
>
> I think x86_vendor_is(var, MASK) wants to become boot_vendor(MASK).
>
> Most cases want the boot vendor, and those that appear to want something
> else don't actually. When you disable the cross-vendor case (patch 2
> pulled out ahead), then cp->vendor == boot_vendor and then you don't
> need a variable to pass in.
>
> This also reduces the verbosity of the new lines which is an improvement.
>
>
> That said, this series also collides substantially with the Intel Fam
> 18/19 work, which is more urgent and needs backporting to 4.21. More
> specifically, there are a bunch of changes which interfere with VFM
> conversion, and for which I can't see an obvious DCE reason to have, so
> I'm wondering if they were just part of "convert everything".
>
> ~Andrew
I converted everything under the sun because it's easier to find all of them
than the strict subset the will give me gains. It's also easier to have new
commits using the function when there are no seemingly random instances where
it's not used.
What's exactly the area you need to touch for the Fam 18/19 work? I can leave
that conversion for last in the series so you have time to push and backport.
It's quite likely not in an area of the tree I care deeply about.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is()
2025-11-26 16:44 ` [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is() Alejandro Vallejo
@ 2025-12-08 16:01 ` Jan Beulich
2025-12-08 16:04 ` Jan Beulich
1 sibling, 0 replies; 37+ messages in thread
From: Jan Beulich @ 2025-12-08 16:01 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 26.11.2025 17:44, Alejandro Vallejo wrote:
> This is the file with the most dramatic effect in terms of DCE, so
> single it out here.
>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
If we accept the basic concept, changes like this are of course okay. Just one
remark:
> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -388,7 +388,7 @@ int8_t __ro_after_init opt_xpti_domu = -1;
>
> static __init void xpti_init_default(void)
> {
> - if ( (boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) ||
> + if ( (x86_vendor_is(boot_cpu_data.vendor, X86_VENDOR_AMD | X86_VENDOR_HYGON)) ||
Here and elsewhere please keep line length within 80 chars.
Jan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is()
2025-11-26 16:44 ` [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is() Alejandro Vallejo
2025-12-08 16:01 ` Jan Beulich
@ 2025-12-08 16:04 ` Jan Beulich
2025-12-11 10:31 ` Alejandro Vallejo
1 sibling, 1 reply; 37+ messages in thread
From: Jan Beulich @ 2025-12-08 16:04 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 26.11.2025 17:44, Alejandro Vallejo wrote:
> @@ -938,7 +938,7 @@ static bool __init retpoline_calculations(void)
> */
> static bool __init rsb_is_full_width(void)
> {
> - if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
> + if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
One other aspect: If already you touch lines still using the old (being
phased out) field names, please rename at the same time. This may then
also help with line length in some cases.
Jan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is()
2025-12-08 16:04 ` Jan Beulich
@ 2025-12-11 10:31 ` Alejandro Vallejo
2025-12-11 15:38 ` Andrew Cooper
0 siblings, 1 reply; 37+ messages in thread
From: Alejandro Vallejo @ 2025-12-11 10:31 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On Mon Dec 8, 2025 at 5:04 PM CET, Jan Beulich wrote:
> On 26.11.2025 17:44, Alejandro Vallejo wrote:
>> @@ -938,7 +938,7 @@ static bool __init retpoline_calculations(void)
>> */
>> static bool __init rsb_is_full_width(void)
>> {
>> - if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
>> + if ( !x86_vendor_is(boot_cpu_data.x86_vendor, X86_VENDOR_INTEL) ||
>
> One other aspect: If already you touch lines still using the old (being
> phased out) field names, please rename at the same time. This may then
> also help with line length in some cases.
>
> Jan
Yes, of course. I didn't even notice the difference at first. On the note on
length, I'm revising the idea, keeping the same principle but making it less
verbose.
Seeing how both you and Andrew seem onboard with dropping cross-vendor support
and having these turn into constants, I'm leaning into transforming everything
to a single "cpu_vendor", both host checks and policy checks would become
something of the form:
if ( cpu_vendor != X86_VENDOR_INTEL )
...
if ( cpu_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON))
...
where cpu_vendor would be defined as a runtime check on boot_cpu_data.vendor
masked with X86_ENABLED_VENDORS when multiple vendors are compiled in, and
the single vendor compiled-in. Perhaps something like... (untested)
#define cpu_vendor \
((!IS_ENABLED(CONFIG_UNKNOWN_CPU_VENDOR) && X86_ENABLED_VENDORS &&
(X86_ENABLED_VENDORS == ISOLATE_LSB(X86_ENABLED_VENDORS)))
? X86_ENABLED_VENDORS
: (boot_cpu_data.vendor & X86_ENABLED_VENDORS))
I _think_ it would have the same DCE implications, but it would be much nicer
to read at the callsites and cause far fewer line wraps.
This brings down complexity, allows for the switches to stay (DCE understands
unreachable labels) and improves the general code quality.
I'm busy atm chasing emulator woes, but a non-rfc series of this will reach
the mailing list at some point. I'll make sure to s/x86_vendor/vendor while at
it.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is()
2025-12-11 10:31 ` Alejandro Vallejo
@ 2025-12-11 15:38 ` Andrew Cooper
2025-12-11 16:58 ` Alejandro Vallejo
0 siblings, 1 reply; 37+ messages in thread
From: Andrew Cooper @ 2025-12-11 15:38 UTC (permalink / raw)
To: Alejandro Vallejo, Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk,
Xenia Ragiadakou, Stefano Stabellini, xen-devel
On 11/12/2025 10:31 am, Alejandro Vallejo wrote:
> Seeing how both you and Andrew seem onboard with dropping cross-vendor support
I found another cross-vendor dropping which you'll want to look into.
struct svm_vcpu contains three guest_sysenter_* MSRs.
In AMD CPUs, these MSRs only have 32 bits of storage, with the upper
halfs write-discard. They are switched via VMLOAD/VMSAVE.
However, in the cross-vendor case, the upper halves are needed for 64bit
kernels setting up SYSENTER support. Therefore, they're unconditionally
intercepted so we can avoid losing the upper half.
By dropping cross-vendor support, we can get rid of these fields, allow
the guest unconditional access, and simply the MSR intercept logic a little.
~Andrew
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is()
2025-12-11 15:38 ` Andrew Cooper
@ 2025-12-11 16:58 ` Alejandro Vallejo
0 siblings, 0 replies; 37+ messages in thread
From: Alejandro Vallejo @ 2025-12-11 16:58 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich
Cc: Roger Pau Monné, Jason Andryuk, Xenia Ragiadakou,
Stefano Stabellini, xen-devel
On Thu Dec 11, 2025 at 4:38 PM CET, Andrew Cooper wrote:
> On 11/12/2025 10:31 am, Alejandro Vallejo wrote:
>> Seeing how both you and Andrew seem onboard with dropping cross-vendor support
>
> I found another cross-vendor dropping which you'll want to look into.
>
> struct svm_vcpu contains three guest_sysenter_* MSRs.
>
> In AMD CPUs, these MSRs only have 32 bits of storage, with the upper
> halfs write-discard. They are switched via VMLOAD/VMSAVE.
>
> However, in the cross-vendor case, the upper halves are needed for 64bit
> kernels setting up SYSENTER support. Therefore, they're unconditionally
> intercepted so we can avoid losing the upper half.
>
> By dropping cross-vendor support, we can get rid of these fields, allow
> the guest unconditional access, and simply the MSR intercept logic a little.
>
> ~Andrew
Sounds straightforward, I'll add it to the pile.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2025-12-11 16:59 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-26 16:44 [RFC PATCH 00/11] x86 vendor check optimisations Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 01/11] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
2025-11-27 9:43 ` Jan Beulich
2025-11-27 12:36 ` Alejandro Vallejo
2025-11-27 13:19 ` Jan Beulich
2025-11-27 13:44 ` Alejandro Vallejo
2025-11-27 14:00 ` Jan Beulich
2025-11-27 9:59 ` Jan Beulich
2025-11-27 12:37 ` Alejandro Vallejo
2025-11-27 21:01 ` Andrew Cooper
2025-11-26 16:44 ` [RFC PATCH 02/11] x86: Reject CPU policies with vendors other than the host's Alejandro Vallejo
2025-11-27 20:15 ` Andrew Cooper
2025-11-26 16:44 ` [RFC PATCH 03/11] x86: Add x86_vendor_is() by itself before using it Alejandro Vallejo
2025-11-27 10:46 ` Jan Beulich
2025-11-27 13:15 ` Alejandro Vallejo
2025-11-27 13:37 ` Jan Beulich
2025-11-27 14:06 ` Alejandro Vallejo
2025-11-27 14:14 ` Jan Beulich
2025-11-27 20:36 ` Andrew Cooper
2025-11-28 9:08 ` Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 04/11] x86: Refactor early vendor lookup code to use x86_vendor_is() Alejandro Vallejo
2025-11-27 10:51 ` Jan Beulich
2025-11-27 13:35 ` Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 05/11] x86: Conditionalise the inclusion of Hygon/Centaur/Shanghai cpu/ files Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 06/11] x86: Migrate switch-based vendor checks to x86_vendor_is() Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 07/11] x86: Migrate MSR handler " Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 08/11] x86: Migrate insn emulator to use x86_vendor_is() Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 09/11] x86: Migrate spec_ctrl vendor checks to x86_vendor_is() Alejandro Vallejo
2025-12-08 16:01 ` Jan Beulich
2025-12-08 16:04 ` Jan Beulich
2025-12-11 10:31 ` Alejandro Vallejo
2025-12-11 15:38 ` Andrew Cooper
2025-12-11 16:58 ` Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 10/11] x86: Migrate everything under cpu/ to use x86_vendor_is() Alejandro Vallejo
2025-11-26 16:44 ` [RFC PATCH 11/11] x86: Migrate every remaining raw vendor check to x86_vendor_is() Alejandro Vallejo
2025-11-27 22:11 ` [RFC PATCH 00/11] x86 vendor check optimisations Andrew Cooper
2025-11-28 9:18 ` Alejandro Vallejo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.