* [PATCH 00/12] const-ify vendor checks
@ 2026-02-06 16:15 Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's Alejandro Vallejo
` (13 more replies)
0 siblings, 14 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
Hi,
This series is a big revamp of vendor-checking to enable it to perform DCE.
It improves all configurations useful in practice at minimal cost in the full
build, and at a massive advantage for the single-vendor case. Many ifdefs can
go away as a side effect of the series.
This series depends on cross-vendor removal:
https://lore.kernel.org/xen-devel/20260205170923.38425-1-alejandro.garciavallejo@amd.com/T/#m4c3d318f37e4f24d0f8c62b104221aa5d428cebc
Patch 1 in this series matches that of cross-vendor removal. It's logically
required, but that's the single requirement.
High level description
======================
When compared to the RFC this makes a different approach The series introduces
cpu_vendor() which maps to a constant in the single vendor case and to
(boot_cpu_data.vendor & X86_ENABLED_VENDORS), where X86_ENABLED_VENDORS is a
mask of the compile-time chosen vendors. This enables the compiler to detect
dead-code at the uses and remove all unreachable branches, including in
switches.
When compared to the x86_vendor_is() macro introduced in the RFC, this is
simpler. It achieves MOST of what the older macro did without touching the
switches, with a few caveats:
1. Compiled-out vendors cause a panic, they don't fallback onto the unknown
vendor case. In retrospect, this is a much saner thing to do.
2. equalities and inequalities have been replaced by equivalent (cpu_vendor() & ...)
forms. This isn't stylistic preference. This form allows the compiler
to merge the compared-against constant with X86_ENABLED_VENDORS, yielding
much better codegen throughout the tree.
The effect of (2) triples the delta in the full build below.
Some differences might be attributable to the change from policy vendor checks
to boot_cpu_data. In the case of the emulator it caused a 400 bytes increase
due to the way it checks using LOTS of macro invocations, so I left that one
piece using the policy vendor except for the single vendor case.
And finally, some bloat-o-meters to better grasp the effects
1. AMD + wo/ unknown (compared against prior AMD-only build)
add/remove: 0/12 grow/shrink: 4/66 up/down: 108/-11595 (-11487)
2. AMD + Intel + wo/ unknown (compared against prior full build)
(excludes Hygon, Shanghai, Centaur and any other unknown CPU vendors)
add/remove: 1/6 grow/shrink: 27/31 up/down: 356/-1552 (-1196)
3. All vendors + w/ unknown (compared against prior full build)
add/remove: 0/0 grow/shrink: 33/19 up/down: 398/-273 (125)
1. AMD + wo/ unknown (compared against prior AMD-only build)
============================================================
add/remove: 0/12 grow/shrink: 4/66 up/down: 108/-11595 (-11487)
Function old new delta
x86_cpu_policies_are_compatible 157 194 +37
xen_config_data 1476 1509 +33
amd_check_entrysign 807 827 +20
init_guest_cpu_policies 1364 1382 +18
opt_gds_mit 1 - -1
nmi_p6_event_width 4 - -4
nmi_p4_cccr_val 4 - -4
init_e820 1037 1033 -4
x86_mcinfo_dump 477 471 -6
pci_cfg_ok 307 301 -6
get_hw_residencies 213 205 -8
recalculate_cpuid_policy 909 900 -9
dom0_setup_permissions 3809 3800 -9
arch_ioreq_server_get_type_addr 250 241 -9
cpu_has_amd_erratum 230 219 -11
init_amd 2449 2437 -12
parse_spec_ctrl 2321 2307 -14
amd_nonfatal_mcheck_init 192 177 -15
shanghai_cpu_dev 16 - -16
hygon_cpu_dev 16 - -16
centaur_cpu_dev 16 - -16
x86emul_0fae 2758 2741 -17
vmce_init_vcpu 153 136 -17
pge_init 60 42 -18
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
vpmu_arch_initialise 195 168 -27
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
symbols_offsets 30800 30752 -48
cpu_callback 4650 4600 -50
mc_memerr_dhandler 903 851 -52
vpmu_init 309 248 -61
mcheck_init 1187 1122 -65
microcode_nmi_callback 205 139 -66
init_IRQ 477 407 -70
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
symbols_sorted_offsets 60760 60664 -96
do_mca 3181 3085 -96
guest_cpuid 2395 2292 -103
guest_common_max_feature_adjustments 110 - -110
read_msr 1471 1346 -125
x86emul_decode 12729 12597 -132
guest_common_default_feature_adjustments 232 62 -170
do_microcode_update 793 608 -185
cpufreq_driver_init 453 263 -190
vmce_wrmsr 967 768 -199
symbols_names 108114 107914 -200
recalculate_misc 898 689 -209
vmce_rdmsr 1083 872 -211
early_cpu_init 948 721 -227
guest_wrmsr 2853 2622 -231
init_centaur 238 - -238
domain_cpu_policy_changed 677 408 -269
write_msr 1752 1455 -297
x86_emulate 222203 221896 -307
init_hygon 402 - -402
guest_rdmsr 2308 1881 -427
start_vmx 1607 1105 -502
setup_apic_nmi_watchdog 977 276 -701
do_get_hw_residencies 1289 9 -1280
init_speculation_mitigations 9714 6788 -2926
Total: Before=3878514, After=3867027, chg -0.30%
2. AMD + Intel + wo/ unknown (compared against prior full build)
================================================================
add/remove: 1/6 grow/shrink: 27/31 up/down: 356/-1552 (-1196)
Function old new delta
start_vmx 1607 1686 +79
vcpu_info_reset - 38 +38
xen_config_data 1483 1520 +37
x86_cpu_policies_are_compatible 157 194 +37
amd_check_entrysign 807 838 +31
init_speculation_mitigations 9836 9862 +26
set_cx_pminfo 1691 1709 +18
read_msr 1471 1486 +15
guest_cpuid 2395 2405 +10
vmce_restore_vcpu 160 167 +7
recalculate_cpuid_policy 909 916 +7
vmce_init_vcpu 153 159 +6
do_mc_get_cpu_info 584 590 +6
init_e820 1037 1042 +5
x86_mcinfo_dump 477 480 +3
setup_apic_nmi_watchdog 977 980 +3
guest_common_max_feature_adjustments 110 113 +3
guest_common_default_feature_adjustments 257 260 +3
disable_lapic_nmi_watchdog 119 122 +3
cpu_has_amd_erratum 230 233 +3
amd_nonfatal_mcheck_init 192 195 +3
alternative_instructions 154 157 +3
pge_init 60 62 +2
mce_firstbank 37 39 +2
init_bsp_APIC 193 195 +2
do_get_hw_residencies 1289 1291 +2
mcheck_init 1227 1228 +1
intel_mcheck_init 2398 2399 +1
pci_cfg_ok 307 306 -1
init_nonfatal_mce_checker 160 159 -1
mc_memerr_dhandler 903 901 -2
recalculate_misc 898 890 -8
acpi_cpufreq_cpu_init 823 815 -8
cpufreq_driver_init 468 459 -9
vmce_intel_rdmsr 161 150 -11
validate_gl4e 617 605 -12
guest_walk_tables_4_levels 3411 3399 -12
traps_init 543 528 -15
l4e_propagate_from_guest 451 436 -15
shanghai_cpu_dev 16 - -16
hygon_cpu_dev 16 - -16
centaur_cpu_dev 16 - -16
write_msr 1752 1735 -17
cpu_callback 5100 5080 -20
amd_mcheck_init 451 431 -20
hvm_vcpu_virtual_to_linear 631 610 -21
domain_cpu_policy_changed 677 656 -21
vpmu_arch_initialise 195 173 -22
acpi_dead_idle 430 407 -23
symbols_offsets 31800 31776 -24
print_mtrr_state 719 693 -26
guest_rdmsr 2308 2282 -26
do_mca 3181 3153 -28
vpmu_init 320 291 -29
vcpu_create 864 835 -29
unmap_guest_area 198 169 -29
init_shanghai 29 - -29
symbols_sorted_offsets 62760 62720 -40
vmce_wrmsr 993 936 -57
vmce_rdmsr 1134 1073 -61
symbols_names 112215 112141 -74
init_intel_cacheinfo 1191 1111 -80
early_cpu_init 948 854 -94
init_centaur 238 - -238
init_hygon 402 - -402
Total: Before=3932243, After=3931047, chg -0.03%
3. All vendors + w/ unknown (compared against prior full build)
===============================================================
add/remove: 0/0 grow/shrink: 33/19 up/down: 398/-273 (125)
Function old new delta
start_vmx 1607 1686 +79
early_cpu_init 948 986 +38
x86_cpu_policies_are_compatible 157 194 +37
xen_config_data 1483 1515 +32
amd_check_entrysign 807 838 +31
init_speculation_mitigations 9836 9862 +26
set_cx_pminfo 1691 1709 +18
read_msr 1471 1486 +15
vmce_wrmsr 993 1005 +12
init_nonfatal_mce_checker 160 170 +10
guest_cpuid 2395 2405 +10
mcheck_init 1227 1235 +8
vmce_restore_vcpu 160 167 +7
recalculate_cpuid_policy 909 916 +7
vmce_init_vcpu 153 159 +6
init_intel_cacheinfo 1191 1197 +6
do_mc_get_cpu_info 584 590 +6
cpufreq_driver_init 468 474 +6
init_e820 1037 1042 +5
amd_mcheck_init 451 456 +5
x86_mcinfo_dump 477 480 +3
setup_apic_nmi_watchdog 977 980 +3
guest_common_max_feature_adjustments 110 113 +3
guest_common_default_feature_adjustments 257 260 +3
disable_lapic_nmi_watchdog 119 122 +3
cpu_has_amd_erratum 230 233 +3
cpu_callback 5100 5103 +3
amd_nonfatal_mcheck_init 192 195 +3
alternative_instructions 154 157 +3
mce_firstbank 37 39 +2
init_bsp_APIC 193 195 +2
do_get_hw_residencies 1289 1291 +2
intel_mcheck_init 2398 2399 +1
recalculate_misc 898 897 -1
pci_cfg_ok 307 306 -1
mc_memerr_dhandler 903 901 -2
vmce_rdmsr 1134 1127 -7
traps_init 543 535 -8
acpi_cpufreq_cpu_init 823 815 -8
vmce_intel_rdmsr 161 150 -11
validate_gl4e 617 605 -12
guest_walk_tables_4_levels 3411 3399 -12
l4e_propagate_from_guest 451 437 -14
write_msr 1752 1735 -17
vpmu_init 320 302 -18
print_mtrr_state 719 698 -21
hvm_vcpu_virtual_to_linear 631 610 -21
domain_cpu_policy_changed 677 656 -21
vpmu_arch_initialise 195 173 -22
acpi_dead_idle 430 407 -23
guest_rdmsr 2308 2282 -26
do_mca 3181 3153 -28
Total: Before=3932245, After=3932370, chg +0.00%
Alejandro Vallejo (12):
x86: Reject CPU policies with vendors other than the host's
x86: Add more granularity to the vendors in Kconfig
x86: Add cpu_vendor() as a wrapper for the host's CPU vendor
x86: Migrate MSR handler vendor checks to cpu_vendor()
x86: Migrate spec_ctrl vendor checks to cpu_vendor()
x86: Migrate switch vendor checks to cpu_vendor()
x86: Have x86_emulate/ implement the single-vendor optimisation
x86/acpi: Migrate vendor checks to cpu_vendor()
x86/pv: Migrate vendor checks to cpu_vendor()
x86/mcheck: Migrate vendor checks to use cpu_vendor()
x86/cpu: Migrate vendor checks to use cpu_vendor()
x86: Migrate every remaining raw vendor check to cpu_vendor()
xen/arch/x86/Kconfig.cpu | 43 ++++++++++++++++++++++++++
xen/arch/x86/acpi/cpu_idle.c | 16 +++++-----
xen/arch/x86/acpi/cpufreq/acpi.c | 2 +-
xen/arch/x86/acpi/cpufreq/cpufreq.c | 15 +++------
xen/arch/x86/alternative.c | 2 +-
xen/arch/x86/apic.c | 2 +-
xen/arch/x86/cpu-policy.c | 14 ++++-----
xen/arch/x86/cpu/Makefile | 6 ++--
xen/arch/x86/cpu/amd.c | 6 ++--
xen/arch/x86/cpu/common.c | 8 +++--
xen/arch/x86/cpu/intel_cacheinfo.c | 5 ++-
xen/arch/x86/cpu/mcheck/amd_nonfatal.c | 2 +-
xen/arch/x86/cpu/mcheck/mcaction.c | 2 +-
xen/arch/x86/cpu/mcheck/mce.c | 23 ++++++--------
xen/arch/x86/cpu/mcheck/mce.h | 2 +-
xen/arch/x86/cpu/mcheck/mce_amd.c | 7 ++---
xen/arch/x86/cpu/mcheck/mce_intel.c | 7 ++---
xen/arch/x86/cpu/mcheck/non-fatal.c | 6 +---
xen/arch/x86/cpu/mcheck/vmce.c | 16 +++-------
xen/arch/x86/cpu/microcode/amd.c | 2 +-
xen/arch/x86/cpu/microcode/core.c | 2 +-
xen/arch/x86/cpu/mtrr/generic.c | 5 ++-
xen/arch/x86/cpu/mwait-idle.c | 5 ++-
xen/arch/x86/cpu/vpmu.c | 9 ++----
xen/arch/x86/cpuid.c | 5 ++-
xen/arch/x86/dom0_build.c | 2 +-
xen/arch/x86/domain.c | 16 +++++-----
xen/arch/x86/e820.c | 2 +-
xen/arch/x86/guest/xen/xen.c | 6 +++-
xen/arch/x86/hvm/hvm.c | 3 +-
xen/arch/x86/hvm/ioreq.c | 3 +-
xen/arch/x86/hvm/vmx/vmx.c | 8 ++---
xen/arch/x86/i8259.c | 5 ++-
xen/arch/x86/include/asm/cpufeature.h | 27 ++++++++++++++++
xen/arch/x86/include/asm/guest_pt.h | 3 +-
xen/arch/x86/irq.c | 3 +-
xen/arch/x86/msr.c | 35 +++++++++------------
xen/arch/x86/nmi.c | 4 +--
xen/arch/x86/pv/domain.c | 2 +-
xen/arch/x86/pv/emul-priv-op.c | 27 +++++++---------
xen/arch/x86/setup.c | 7 ++---
xen/arch/x86/spec_ctrl.c | 42 ++++++++++---------------
xen/arch/x86/traps-setup.c | 2 +-
xen/arch/x86/x86_emulate/private.h | 10 +++++-
xen/arch/x86/x86_emulate/x86_emulate.c | 2 +-
xen/lib/x86/policy.c | 3 +-
46 files changed, 224 insertions(+), 200 deletions(-)
base-commit: 381b4ff16f7ff83a2dc44f16b8dd0208f3255ec7
--
2.43.0
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-10 8:19 ` Roger Pau Monné
2026-02-11 15:41 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 02/12] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
` (12 subsequent siblings)
13 siblings, 2 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
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>
---
Patch 1 from the cross-vendor series. Do not merge here. It's simply for
consistency.
---
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..079c42a29b 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->x86_vendor != host->x86_vendor) ||
+ (guest->basic.max_leaf > host->basic.max_leaf) )
FAIL_CPUID(0, NA);
if ( guest->feat.max_subleaf > host->feat.max_subleaf )
--
2.43.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 02/12] x86: Add more granularity to the vendors in Kconfig
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-10 8:26 ` Roger Pau Monné
2026-02-11 16:06 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor Alejandro Vallejo
` (11 subsequent siblings)
13 siblings, 2 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
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 will be allowed.
A prerequisite for such optimisation is to expand Kconfig to have the
full set of vendors. Adds Hygon, Centaur, Shanghai and the unknown-vendor
path. Have Hygon select AMD, and Centaur|Shanghai select Intel.
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
rfc -> v1:
* s/depends on/select/
* s/UNKNOWN_CPU/UNKNOWN_CPU_VENDOR/
* Removed "if unsure, say Y"
---
xen/arch/x86/Kconfig.cpu | 43 +++++++++++++++++++++++++++++++++++++++
xen/arch/x86/cpu/Makefile | 6 +++---
2 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu
index 5fb18db1aa..f895cfe97a 100644
--- a/xen/arch/x86/Kconfig.cpu
+++ b/xen/arch/x86/Kconfig.cpu
@@ -19,4 +19,47 @@ 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"
+ select 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"
+ select 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"
+ select 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_VENDOR
+ bool "Support unknown CPU vendors"
+ 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 selecting a single vendor with this
+ option cleared produces a much smaller build.
+
endmenu
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] 55+ messages in thread
* [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 02/12] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-10 8:46 ` Roger Pau Monné
` (2 more replies)
2026-02-06 16:15 ` [PATCH 04/12] x86: Migrate MSR handler vendor checks to cpu_vendor() Alejandro Vallejo
` (10 subsequent siblings)
13 siblings, 3 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
Introduces various optimisations that rely on constant folding, Value
Range Propagation (VRP), and Dead Code Elimination (DCE) to aggressively
eliminate code surrounding the uses of the function.
* For single-vendor+no-unknown-vendor builds returns a compile-time
constant.
* For all other cases it ANDs the result with the mask of compiled
vendors, with the effect of performing DCE in switch cases, removing
dead conditionals, etc.
It's difficult to reason about codegen in general in a project this big,
but in this case the ANDed constant combines with the values typically
checked against, folding into a comparison against zero. Thus, it's better
for codegen to AND its result with the desired compared-against vendor,
rather than using (in)equality operators. That way the comparison is
always against zero.
"cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)"
turns into (cpu_vendor() & X86_VENDOR_AMD) in AMD-only builds (AND +
cmp with zero). Whereas this...
"cpu_vendor() == X86_VENDOR_AMD"
forces cpu_vendor() to be ANDed and then compared to a non-zero value.
Later patches take the opportunity and make this refactor as cpu_vendor()
is introduced throughout the tree.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/cpu/common.c | 6 +++++-
xen/arch/x86/guest/xen/xen.c | 4 ++++
xen/arch/x86/include/asm/cpufeature.h | 27 +++++++++++++++++++++++++++
3 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index ebe2baf8b9..6f4e723172 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -328,7 +328,11 @@ void __init early_cpu_init(bool verbose)
*(u32 *)&c->x86_vendor_id[4] = edx;
c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
- switch (c->x86_vendor) {
+ if ( c->x86_vendor != cpu_vendor() )
+ panic("CPU vendor not compiled-in: %s",
+ x86_cpuid_vendor_to_str(c->x86_vendor));
+
+ switch (cpu_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;
diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c
index 77a3a8742a..ec558bcbdb 100644
--- a/xen/arch/x86/guest/xen/xen.c
+++ b/xen/arch/x86/guest/xen/xen.c
@@ -57,6 +57,10 @@ void asmlinkage __init early_hypercall_setup(void)
cpuid(0, &eax, &ebx, &ecx, &edx);
boot_cpu_data.x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
+
+ if ( cpu_vendor() != boot_cpu_data.x86_vendor )
+ panic("CPU vendor not compiled-in: %s",
+ x86_cpuid_vendor_to_str(boot_cpu_data.x86_vendor));
}
switch ( boot_cpu_data.x86_vendor )
diff --git a/xen/arch/x86/include/asm/cpufeature.h b/xen/arch/x86/include/asm/cpufeature.h
index dcd223d84f..10309e40b6 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -14,6 +14,7 @@
#include <asm/cpu-policy.h>
#include <asm/cpuid.h>
#include <xen/lib/x86/cpu-policy.h>
+#include <asm/x86-vendors.h>
#else
#include <asm/cpufeatureset.h>
#endif
@@ -116,6 +117,32 @@ static inline bool boot_cpu_has(unsigned int feat)
#define CPUID_PM_LEAF 6
+#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))
+
+static always_inline uint8_t cpu_vendor(void)
+{
+ uint8_t vendor = boot_cpu_data.vendor;
+
+ /*
+ * const-ify the CPU vendor if we compiled for a single vendor and there's
+ * no boot path for an unknown vendor.
+ */
+ if ( !IS_ENABLED(CONFIG_UNKNOWN_CPU_VENDOR) &&
+ (ISOLATE_LSB(X86_ENABLED_VENDORS) == X86_ENABLED_VENDORS) )
+ return X86_ENABLED_VENDORS;
+
+ /*
+ * This allows the compiler to know more in its VRP pass about the valid
+ * range of `vendor`. It enhances DCE by eliminating impossible vendors.
+ */
+ return vendor & X86_ENABLED_VENDORS;
+}
+
/* CPUID level 0x00000001.edx */
#define cpu_has_fpu 1
#define cpu_has_de 1
--
2.43.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 04/12] x86: Migrate MSR handler vendor checks to cpu_vendor()
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
` (2 preceding siblings ...)
2026-02-06 16:15 ` [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-11 16:15 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 05/12] x86: Migrate spec_ctrl " Alejandro Vallejo
` (9 subsequent siblings)
13 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
Many handlers are vendor-specific and are currently gated on runtime
checks. If we migrate those to cpu_vendor() 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 | 35 +++++++++++++++--------------------
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index ad75a2e108..34033c1982 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -157,8 +157,8 @@ 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 ( !(cpu_vendor() & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR |
+ X86_VENDOR_SHANGHAI)) )
goto gp_fault;
*val = IA32_FEATURE_CONTROL_LOCK;
@@ -169,8 +169,7 @@ 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 ( !(cpu_vendor() & X86_VENDOR_INTEL) )
goto gp_fault;
rdmsrl(MSR_IA32_PLATFORM_ID, *val);
break;
@@ -189,9 +188,7 @@ 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 ( !(cpu_vendor() & (X86_VENDOR_INTEL | X86_VENDOR_AMD)) ||
rdmsr_safe(MSR_AMD_PATCHLEVEL, val) )
goto gp_fault;
break;
@@ -236,7 +233,7 @@ 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 ( !(cpu_vendor() & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR)) )
goto gp_fault;
*val = 0;
@@ -245,7 +242,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 ( !(cpu_vendor() & X86_VENDOR_INTEL) )
goto gp_fault;
*val = 0;
break;
@@ -302,7 +299,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 ( !(cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
goto gp_fault;
if ( !is_hardware_domain(d) )
return X86EMUL_UNHANDLEABLE;
@@ -314,14 +311,14 @@ 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 ( !(cpu_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)) ||
+ !(cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) ||
rdmsr_safe(msr, val) )
goto gp_fault;
@@ -338,7 +335,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 ( !(cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
goto gp_fault;
*val = AMD64_DE_CFG_LFENCE_SERIALISE;
break;
@@ -461,7 +458,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 ( !(cpu_vendor() & X86_VENDOR_INTEL) || val != 0 )
goto gp_fault;
break;
@@ -470,8 +467,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
* See note on MSR_IA32_UCODE_WRITE below, which may or may not apply
* to AMD CPUs as well (at least the architectural/CPUID part does).
*/
- if ( is_pv_domain(d) ||
- cp->x86_vendor != X86_VENDOR_AMD )
+ if ( is_pv_domain(d) || !(cpu_vendor() & X86_VENDOR_AMD) )
goto gp_fault;
break;
@@ -482,8 +478,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
* for such attempts. Also the MSR is architectural and not qualified
* by any CPUID bit.
*/
- if ( is_pv_domain(d) ||
- cp->x86_vendor != X86_VENDOR_INTEL )
+ if ( is_pv_domain(d) || !(cpu_vendor() & X86_VENDOR_INTEL) )
goto gp_fault;
break;
@@ -553,7 +548,7 @@ 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 ( !(cpu_vendor() & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR)) )
goto gp_fault;
if ( likely(!is_cpufreq_controller(d)) || wrmsr_safe(msr, val) == 0 )
@@ -663,7 +658,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 ( !(cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
goto gp_fault;
break;
--
2.43.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 05/12] x86: Migrate spec_ctrl vendor checks to cpu_vendor()
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
` (3 preceding siblings ...)
2026-02-06 16:15 ` [PATCH 04/12] x86: Migrate MSR handler vendor checks to cpu_vendor() Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-12 10:49 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 06/12] x86: Migrate switch " Alejandro Vallejo
` (8 subsequent siblings)
13 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
This is the file with the most dramatic effect in terms of DCE, so
single it out here.
On an AMD-only build:
add/remove: 0/1 grow/shrink: 0/5 up/down: 0/-2968 (-2968)
Function old new delta
opt_gds_mit 1 - -1
symbols_offsets 30776 30772 -4
symbols_sorted_offsets 60712 60704 -8
parse_spec_ctrl 2321 2307 -14
symbols_names 108031 108016 -15
init_speculation_mitigations 9714 6788 -2926
Total: Before=3876849, After=3873881, chg -0.08%
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/spec_ctrl.c | 42 ++++++++++++++++------------------------
1 file changed, 17 insertions(+), 25 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index dd0413e1fc..246d8895f1 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 ( (cpu_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 ( (cpu_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,11 +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 ( cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
return true;
- if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
- boot_cpu_data.family != 6 )
+ if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.family != 6 )
return false;
/*
@@ -938,8 +937,7 @@ static bool __init retpoline_calculations(void)
*/
static bool __init rsb_is_full_width(void)
{
- if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
- boot_cpu_data.family != 6 )
+ if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.family != 6 )
return true;
switch ( boot_cpu_data.model )
@@ -966,8 +964,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 ||
- boot_cpu_data.family != 6 )
+ if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.family != 6 )
return false;
switch ( boot_cpu_data.model )
@@ -1033,7 +1030,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 ( !(cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
return;
/*
@@ -1099,7 +1096,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 ( !(cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
return false;
if ( boot_cpu_data.family != 0x17 && boot_cpu_data.family != 0x18 )
@@ -1137,7 +1134,7 @@ static void __init ibpb_calculations(void)
return;
}
- if ( boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
+ if ( cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
{
/*
* AMD/Hygon CPUs to date (June 2022) don't flush the RAS. Future
@@ -1222,8 +1219,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 &&
- boot_cpu_data.family == 6 )
+ if ( (cpu_vendor() & X86_VENDOR_INTEL) && boot_cpu_data.family == 6 )
{
switch ( boot_cpu_data.model )
{
@@ -1358,8 +1354,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 ||
- boot_cpu_data.family != 6 )
+ if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.family != 6 )
return;
/* Any processor advertising MDS_NO should be not vulnerable to MDS. */
@@ -1469,8 +1464,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 ||
- boot_cpu_data.family != 6 )
+ if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.family != 6 )
return;
/*
@@ -1535,7 +1529,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 ( !(cpu_vendor() & X86_VENDOR_AMD) )
return;
/* If we're virtualised, don't attempt to synthesise anything. */
@@ -1659,8 +1653,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 ||
- boot_cpu_data.family != 6 )
+ if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.family != 6 )
return;
cpu_has_bug_gds = cpu_has_gds();
@@ -1754,8 +1747,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 ||
- boot_cpu_data.family != 6 )
+ if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.family != 6 )
return false;
if ( boot_cpu_has(X86_FEATURE_BHI_NO) )
@@ -1878,7 +1870,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 ( !(cpu_vendor() & X86_VENDOR_INTEL) )
return;
/*
@@ -2181,7 +2173,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)) &&
+ (cpu_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] 55+ messages in thread
* [PATCH 06/12] x86: Migrate switch vendor checks to cpu_vendor()
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
` (4 preceding siblings ...)
2026-02-06 16:15 ` [PATCH 05/12] x86: Migrate spec_ctrl " Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-12 11:06 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 07/12] x86: Have x86_emulate/ implement the single-vendor optimisation Alejandro Vallejo
` (7 subsequent siblings)
13 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
In the single-vendor case (with no unknown vendor) all switches turn
to a branchless straight line through the single compiled-in case.
In the multivendor case where some vendors are disabled the VRP pass
ensures those branches are eliminated.
It's hard to tell the VRP pass zero is also unreachable when
CONFIG_UNKNOWN_CPU_VENDORS is disabled, so default cases tend to remain.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
The removals of ifdefs are proof of VRP doing the work of DCE.
---
xen/arch/x86/acpi/cpufreq/cpufreq.c | 12 +++---------
xen/arch/x86/alternative.c | 2 +-
xen/arch/x86/cpu-policy.c | 6 +++---
xen/arch/x86/cpu/mcheck/mce.c | 6 +-----
xen/arch/x86/cpu/mcheck/mce.h | 2 +-
xen/arch/x86/cpu/mcheck/non-fatal.c | 6 +-----
xen/arch/x86/cpu/mcheck/vmce.c | 12 ++----------
xen/arch/x86/cpu/vpmu.c | 6 +-----
xen/arch/x86/domain.c | 4 ++--
xen/arch/x86/guest/xen/xen.c | 2 +-
xen/arch/x86/nmi.c | 4 ++--
xen/arch/x86/traps-setup.c | 2 +-
12 files changed, 19 insertions(+), 45 deletions(-)
diff --git a/xen/arch/x86/acpi/cpufreq/cpufreq.c b/xen/arch/x86/acpi/cpufreq/cpufreq.c
index 5740c0d438..4ee03ecc65 100644
--- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
+++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
@@ -133,7 +133,7 @@ static int __init cf_check cpufreq_driver_init(void)
ret = -ENOENT;
- switch ( boot_cpu_data.x86_vendor )
+ switch( cpu_vendor() )
{
case X86_VENDOR_INTEL:
for ( i = 0; i < cpufreq_xen_cnt; i++ )
@@ -141,12 +141,10 @@ static int __init cf_check cpufreq_driver_init(void)
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;
@@ -165,7 +163,6 @@ static int __init cf_check cpufreq_driver_init(void)
case X86_VENDOR_AMD:
case X86_VENDOR_HYGON:
-#ifdef CONFIG_AMD
for ( i = 0; i < cpufreq_xen_cnt; i++ )
{
switch ( cpufreq_xen_opts[i] )
@@ -191,9 +188,6 @@ static int __init cf_check cpufreq_driver_init(void)
if ( !ret || ret == -EBUSY )
break;
}
-#else
- ret = -ENODEV;
-#endif /* CONFIG_AMD */
break;
default:
diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index 9f844241bc..c723417807 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -89,7 +89,7 @@ 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 )
+ switch ( cpu_vendor() )
{
case X86_VENDOR_INTEL:
/*
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 5273fe0ae4..62aff61d8c 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -279,7 +279,7 @@ static void recalculate_misc(struct cpu_policy *p)
p->extd.raw[0x8].d = 0;
- switch ( p->x86_vendor )
+ switch ( cpu_vendor() )
{
case X86_VENDOR_INTEL:
p->basic.l2_nr_queries = 1; /* Fixed to 1 query. */
@@ -420,7 +420,7 @@ 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 )
+ switch ( cpu_vendor() )
{
case X86_VENDOR_INTEL:
/*
@@ -514,7 +514,7 @@ 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 )
+ switch ( cpu_vendor() )
{
case X86_VENDOR_INTEL:
/*
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 9277781bff..4b295aeeb5 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -766,16 +766,13 @@ 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 )
+ switch ( cpu_vendor() )
{
-#ifdef CONFIG_AMD
case X86_VENDOR_AMD:
case X86_VENDOR_HYGON:
inited = amd_mcheck_init(c, bsp);
break;
-#endif
-#ifdef CONFIG_INTEL
case X86_VENDOR_INTEL:
switch ( c->x86 )
{
@@ -785,7 +782,6 @@ void mcheck_init(struct cpuinfo_x86 *c, bool bsp)
break;
}
break;
-#endif
default:
break;
diff --git a/xen/arch/x86/cpu/mcheck/mce.h b/xen/arch/x86/cpu/mcheck/mce.h
index 920b075355..4828c74d47 100644
--- a/xen/arch/x86/cpu/mcheck/mce.h
+++ b/xen/arch/x86/cpu/mcheck/mce.h
@@ -137,7 +137,7 @@ 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) {
+ switch (cpu_vendor()) {
case X86_VENDOR_INTEL:
if (msr >= MSR_IA32_MC0_CTL2 &&
msr < MSR_IA32_MCx_CTL2(v->arch.vmce.mcg_cap & MCG_CAP_COUNT) )
diff --git a/xen/arch/x86/cpu/mcheck/non-fatal.c b/xen/arch/x86/cpu/mcheck/non-fatal.c
index a9ee9bb94f..3a0399c121 100644
--- a/xen/arch/x86/cpu/mcheck/non-fatal.c
+++ b/xen/arch/x86/cpu/mcheck/non-fatal.c
@@ -23,20 +23,16 @@ 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
+ switch (cpu_vendor()) {
case X86_VENDOR_AMD:
case 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:
intel_nonfatal_mcheck_init(c);
break;
-#endif
default:
/* unhandled vendor isn't really an error */
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index 1a7e92506a..0c1bfb691b 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -136,22 +136,18 @@ static int bank_mce_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
break;
default:
- switch ( boot_cpu_data.x86_vendor )
+ switch ( cpu_vendor() )
{
-#ifdef CONFIG_INTEL
case X86_VENDOR_CENTAUR:
case X86_VENDOR_SHANGHAI:
case X86_VENDOR_INTEL:
ret = vmce_intel_rdmsr(v, msr, val);
break;
-#endif
-#ifdef CONFIG_AMD
case X86_VENDOR_AMD:
case X86_VENDOR_HYGON:
ret = vmce_amd_rdmsr(v, msr, val);
break;
-#endif
default:
ret = 0;
@@ -273,20 +269,16 @@ static int bank_mce_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
break;
default:
- switch ( boot_cpu_data.x86_vendor )
+ switch ( cpu_vendor() )
{
-#ifdef CONFIG_INTEL
case X86_VENDOR_INTEL:
ret = vmce_intel_wrmsr(v, msr, val);
break;
-#endif
-#ifdef CONFIG_AMD
case X86_VENDOR_AMD:
case X86_VENDOR_HYGON:
ret = vmce_amd_wrmsr(v, msr, val);
break;
-#endif
default:
ret = 0;
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index c28192ea26..b1ad4ebcf6 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -815,7 +815,7 @@ static struct notifier_block cpu_nfb = {
static int __init cf_check vpmu_init(void)
{
- int vendor = current_cpu_data.x86_vendor;
+ int vendor = cpu_vendor();
const struct arch_vpmu_ops *ops = NULL;
if ( !opt_vpmu_enabled )
@@ -832,7 +832,6 @@ static int __init cf_check vpmu_init(void)
switch ( vendor )
{
-#ifdef CONFIG_AMD
case X86_VENDOR_AMD:
ops = amd_vpmu_init();
break;
@@ -840,13 +839,10 @@ static int __init cf_check vpmu_init(void)
case X86_VENDOR_HYGON:
ops = hygon_vpmu_init();
break;
-#endif
-#ifdef CONFIG_INTEL
case X86_VENDOR_INTEL:
ops = core2_vpmu_init();
break;
-#endif
default:
printk(XENLOG_WARNING "VPMU: Unknown CPU vendor: %d. "
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 8b2f33f1a0..10a558e515 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -319,7 +319,7 @@ void domain_cpu_policy_changed(struct domain *d)
if ( cpu_has_htt )
edx |= cpufeat_mask(X86_FEATURE_HTT);
- switch ( boot_cpu_data.x86_vendor )
+ switch( cpu_vendor() )
{
case X86_VENDOR_INTEL:
/*
@@ -427,7 +427,7 @@ void domain_cpu_policy_changed(struct domain *d)
if ( !(p->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
edx &= ~CPUID_COMMON_1D_FEATURES;
- switch ( boot_cpu_data.x86_vendor )
+ switch( cpu_vendor() )
{
case X86_VENDOR_INTEL:
mask &= ((uint64_t)edx << 32) | ecx;
diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c
index ec558bcbdb..590a4bad82 100644
--- a/xen/arch/x86/guest/xen/xen.c
+++ b/xen/arch/x86/guest/xen/xen.c
@@ -63,7 +63,7 @@ void asmlinkage __init early_hypercall_setup(void)
x86_cpuid_vendor_to_str(boot_cpu_data.x86_vendor));
}
- switch ( boot_cpu_data.x86_vendor )
+ switch ( cpu_vendor() )
{
case X86_VENDOR_INTEL:
case X86_VENDOR_CENTAUR:
diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index a0c9194ff0..a3ba59ce0b 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -216,7 +216,7 @@ void disable_lapic_nmi_watchdog(void)
{
if (nmi_active <= 0)
return;
- switch (boot_cpu_data.x86_vendor) {
+ switch(cpu_vendor()) {
case X86_VENDOR_AMD:
wrmsrns(MSR_K7_EVNTSEL0, 0);
break;
@@ -387,7 +387,7 @@ void setup_apic_nmi_watchdog(void)
if ( nmi_watchdog == NMI_NONE )
return;
- switch ( boot_cpu_data.x86_vendor )
+ switch( cpu_vendor() )
{
case X86_VENDOR_AMD:
setup_k7_watchdog();
diff --git a/xen/arch/x86/traps-setup.c b/xen/arch/x86/traps-setup.c
index d77be8f839..7d7554a7ba 100644
--- a/xen/arch/x86/traps-setup.c
+++ b/xen/arch/x86/traps-setup.c
@@ -243,7 +243,7 @@ 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 )
+ switch( cpu_vendor() )
{
case X86_VENDOR_INTEL:
if ( boot_cpu_data.x86 == 0xf )
--
2.43.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 07/12] x86: Have x86_emulate/ implement the single-vendor optimisation
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
` (5 preceding siblings ...)
2026-02-06 16:15 ` [PATCH 06/12] x86: Migrate switch " Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-12 11:26 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 07/12] x86: Migrate x86_emulate/ to use cpu_vendor() Alejandro Vallejo
` (6 subsequent siblings)
13 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
Open code the vendor check through the policy as a one-off. The emulator
embeds amd_like() in macros and is called in MANY places. Using a
local variable (cp->x86_vendor) makes it a lot smaller (300-400 bytes
smaller). So treat this as the exception it is and let it use the policy
rather than boot_cpu_data.
Also keep the current behaviour of using the policy vendor when
compiled for userspace, where cross-vendor configurations are expected.
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/x86_emulate/private.h | 10 +++++++++-
xen/arch/x86/x86_emulate/x86_emulate.c | 2 +-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/x86_emulate/private.h b/xen/arch/x86/x86_emulate/private.h
index 24c79c4e8f..95f2015c44 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/cpufeature.h>
# include <asm/endbr.h>
# include <asm/msr-index.h>
# include <asm/stubs.h>
@@ -30,8 +31,15 @@ void BUG(void);
# define X86EMUL_NO_SIMD
# endif
+/* intentionally avoid cpu_vendor(), as it produces much worse codegen */
+# define x86emul_cpu(cp) ((X86_ENABLED_VENDORS == \
+ ISOLATE_LSB(X86_ENABLED_VENDORS)) \
+ ? X86_ENABLED_VENDORS \
+ : ((cp)->x86_vendor & X86_ENABLED_VENDORS))
+
#else /* !__XEN__ */
# include "x86-emulate.h"
+# define x86emul_cpu(cp) ((cp)->x86_vendor)
#endif
#ifdef __i386__
@@ -520,7 +528,7 @@ in_protmode(
static inline bool
_amd_like(const struct cpu_policy *cp)
{
- return cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON);
+ return x86emul_cpu(cp) & (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 7751a67130..308ec3579f 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((x86emul_cpu(cp) & 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] 55+ messages in thread
* [PATCH 07/12] x86: Migrate x86_emulate/ to use cpu_vendor()
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
` (6 preceding siblings ...)
2026-02-06 16:15 ` [PATCH 07/12] x86: Have x86_emulate/ implement the single-vendor optimisation Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-12 11:31 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 08/12] x86/acpi: Migrate vendor checks to cpu_vendor() Alejandro Vallejo
` (5 subsequent siblings)
13 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
... but keep the current behaviour of using the policy vendor when
compiled for userspace, where cross-vendor configurations are expected.
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/x86_emulate/private.h | 6 +++++-
xen/arch/x86/x86_emulate/x86_emulate.c | 2 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/x86_emulate/private.h b/xen/arch/x86/x86_emulate/private.h
index 24c79c4e8f..1b6fc48ab6 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/cpufeature.h>
# include <asm/endbr.h>
# include <asm/msr-index.h>
# include <asm/stubs.h>
@@ -30,8 +31,11 @@ void BUG(void);
# define X86EMUL_NO_SIMD
# endif
+# define x86emul_cpu(cp) ({ (void)(cp); cpu_vendor(); })
+
#else /* !__XEN__ */
# include "x86-emulate.h"
+# define x86emul_cpu(cp) ((cp)->x86_vendor)
#endif
#ifdef __i386__
@@ -520,7 +524,7 @@ in_protmode(
static inline bool
_amd_like(const struct cpu_policy *cp)
{
- return cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON);
+ return x86emul_cpu(cp) & (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 7751a67130..fcd8cbccc9 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((x86emul_cpu(cp) == 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] 55+ messages in thread
* [PATCH 08/12] x86/acpi: Migrate vendor checks to cpu_vendor()
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
` (7 preceding siblings ...)
2026-02-06 16:15 ` [PATCH 07/12] x86: Migrate x86_emulate/ to use cpu_vendor() Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-12 11:52 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 09/12] x86/pv: " Alejandro Vallejo
` (4 subsequent siblings)
13 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/acpi/cpu_idle.c | 16 +++++++---------
xen/arch/x86/acpi/cpufreq/acpi.c | 2 +-
xen/arch/x86/acpi/cpufreq/cpufreq.c | 3 +--
3 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index 0b3d0631dd..0a49b2730b 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 ( !(cpu_vendor() & X86_VENDOR_INTEL) || c->x86 != 6 )
return;
switch ( c->x86_model )
@@ -915,8 +915,7 @@ 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 ( cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON) &&
cx->entry_method == ACPI_CSTATE_EM_SYSIO )
{
/* Intel prefers not to use SYSIO */
@@ -1042,8 +1041,8 @@ 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 ( (cpu_vendor() & X86_VENDOR_INTEL) ||
+ ((cpu_vendor() & X86_VENDOR_AMD) && c->x86 == 0x15) )
{
/*
* Today all MP CPUs that support C3 share cache.
@@ -1059,7 +1058,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 ( (cpu_vendor() & X86_VENDOR_INTEL) &&
(c->x86 > 0x6 || (c->x86 == 6 && c->x86_model >= 14)) )
flags->bm_control = 0;
}
@@ -1421,7 +1420,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 ( !(cpu_vendor() & X86_VENDOR_HYGON) )
{
default:
vendor_override = -1;
@@ -1648,8 +1647,7 @@ static int cf_check cpu_callback(
break;
case CPU_ONLINE:
- if ( (boot_cpu_data.x86_vendor &
- (X86_VENDOR_AMD | X86_VENDOR_HYGON)) &&
+ if ( cpu_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 d0ca660db1..a5e5c223b3 100644
--- a/xen/arch/x86/acpi/cpufreq/acpi.c
+++ b/xen/arch/x86/acpi/cpufreq/acpi.c
@@ -454,7 +454,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 ((cpu_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 4ee03ecc65..d731a21644 100644
--- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
+++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
@@ -246,8 +246,7 @@ __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 ( cpu_vendor() & (X86_VENDOR_INTEL | X86_VENDOR_AMD | X86_VENDOR_HYGON) )
return cpufreq_add_cpu(cpu);
return -EOPNOTSUPP;
--
2.43.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 09/12] x86/pv: Migrate vendor checks to cpu_vendor()
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
` (8 preceding siblings ...)
2026-02-06 16:15 ` [PATCH 08/12] x86/acpi: Migrate vendor checks to cpu_vendor() Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 10/12] x86/mcheck: Migrate vendor checks to use cpu_vendor() Alejandro Vallejo
` (3 subsequent siblings)
13 siblings, 0 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/pv/domain.c | 2 +-
xen/arch/x86/pv/emul-priv-op.c | 27 ++++++++++++---------------
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index d16583a745..7084274dff 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -174,7 +174,7 @@ static int __init cf_check pge_init(void)
{
if ( opt_global_pages == -1 )
opt_global_pages = !cpu_has_hypervisor ||
- !(boot_cpu_data.x86_vendor &
+ !(cpu_vendor() &
(X86_VENDOR_AMD | X86_VENDOR_HYGON));
return 0;
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 1d9e57ad27..02b6e8f01f 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -243,8 +243,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 &&
+ if ( CF8_ADDR_HI(currd->arch.pci_cf8) && (cpu_vendor() & X86_VENDOR_AMD) &&
boot_cpu_data.x86 >= 0x10 && boot_cpu_data.x86 < 0x17 )
{
uint64_t msr_val;
@@ -869,8 +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
- ? EFER_SCE : 0));
+ ((cpu_vendor() & X86_VENDOR_INTEL) ? EFER_SCE : 0));
return val;
}
@@ -958,7 +956,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 ( !(cpu_vendor() & X86_VENDOR_AMD) )
break;
if ( unlikely(is_cpufreq_controller(currd)) )
goto normal;
@@ -966,7 +964,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 ( !(cpu_vendor() & X86_VENDOR_AMD) ||
boot_cpu_data.x86 < 0x10 || boot_cpu_data.x86 >= 0x17 )
break;
/* fall through */
@@ -991,13 +989,13 @@ 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 ( cpu_vendor() & X86_VENDOR_INTEL )
{
vpmu_msr = true;
/* fall through */
case MSR_AMD_FAM15H_EVNTSEL0 ... MSR_AMD_FAM15H_PERFCTR5:
case MSR_K7_EVNTSEL0 ... MSR_K7_PERFCTR3:
- if ( vpmu_msr || (boot_cpu_data.x86_vendor &
+ if ( vpmu_msr || (cpu_vendor() &
(X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
{
if ( vpmu_do_rdmsr(reg, val) )
@@ -1100,8 +1098,7 @@ static int cf_check write_msr(
case MSR_K8_PSTATE6:
case MSR_K8_PSTATE7:
case MSR_K8_HWCR:
- if ( !(boot_cpu_data.x86_vendor &
- (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
+ if ( !(cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
break;
if ( likely(!is_cpufreq_controller(currd)) ||
wrmsr_safe(reg, val) == 0 )
@@ -1119,7 +1116,7 @@ static int cf_check write_msr(
break;
case MSR_FAM10H_MMIO_CONF_BASE:
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
+ if ( !(cpu_vendor() & X86_VENDOR_AMD) ||
boot_cpu_data.x86 < 0x10 || boot_cpu_data.x86 >= 0x17 )
break;
if ( !is_hwdom_pinned_vcpu(curr) )
@@ -1148,7 +1145,7 @@ static int cf_check write_msr(
case MSR_IA32_MPERF:
case MSR_IA32_APERF:
- if ( !(boot_cpu_data.x86_vendor &
+ if ( !(cpu_vendor() &
(X86_VENDOR_INTEL | X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
break;
if ( likely(!is_cpufreq_controller(currd)) ||
@@ -1158,7 +1155,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 ( !(cpu_vendor() & X86_VENDOR_INTEL) )
break;
if ( !is_hwdom_pinned_vcpu(curr) || wrmsr_safe(reg, val) == 0 )
return X86EMUL_OKAY;
@@ -1168,12 +1165,12 @@ 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 ( cpu_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 &
+ if ( vpmu_msr || (cpu_vendor() &
(X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
{
if ( (vpmu_mode & XENPMU_MODE_ALL) &&
--
2.43.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 10/12] x86/mcheck: Migrate vendor checks to use cpu_vendor()
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
` (9 preceding siblings ...)
2026-02-06 16:15 ` [PATCH 09/12] x86/pv: " Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-12 12:02 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 11/12] x86/cpu: " Alejandro Vallejo
` (2 subsequent siblings)
13 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/cpu/mcheck/amd_nonfatal.c | 2 +-
xen/arch/x86/cpu/mcheck/mcaction.c | 2 +-
xen/arch/x86/cpu/mcheck/mce.c | 17 ++++++++---------
xen/arch/x86/cpu/mcheck/mce_amd.c | 7 +++----
xen/arch/x86/cpu/mcheck/mce_intel.c | 7 +++----
xen/arch/x86/cpu/mcheck/vmce.c | 4 ++--
6 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/xen/arch/x86/cpu/mcheck/amd_nonfatal.c b/xen/arch/x86/cpu/mcheck/amd_nonfatal.c
index 7d48c9ab5f..ebe13911ba 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 (!(cpu_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..b7fc5056d8 100644
--- a/xen/arch/x86/cpu/mcheck/mcaction.c
+++ b/xen/arch/x86/cpu/mcheck/mcaction.c
@@ -101,7 +101,7 @@ 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 &&
+ ((cpu_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 4b295aeeb5..91c5033edb 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 ( cpu_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 (cpu_vendor() & X86_VENDOR_INTEL) &&
+ c->x86 == 6 && c->x86_model < 0x1a;
}
static int show_mca_info(int inited, struct cpuinfo_x86 *c)
@@ -862,7 +861,7 @@ static void x86_mcinfo_apei_save(
memset(&m, 0, sizeof(struct mce));
m.cpu = mc_global->mc_coreid;
- m.cpuvendor = boot_cpu_data.x86_vendor;
+ m.cpuvendor = cpu_vendor();
m.cpuid = cpuid_eax(1);
m.socketid = mc_global->mc_socketid;
m.apicid = mc_global->mc_apicid;
@@ -964,7 +963,7 @@ static void cf_check __maybe_unused do_mc_get_cpu_info(void *v)
&xcp->mc_ncores_active, &xcp->mc_nthreads);
xcp->mc_cpuid_level = c->cpuid_level;
xcp->mc_family = c->x86;
- xcp->mc_vendor = c->x86_vendor;
+ xcp->mc_vendor = cpu_vendor();
xcp->mc_model = c->x86_model;
xcp->mc_step = c->x86_mask;
xcp->mc_cache_size = c->x86_cache_size;
@@ -1122,7 +1121,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 ( (cpu_vendor() & X86_VENDOR_AMD) )
{
/*
* On AMD we can set MCi_STATUS_WREN in the
@@ -1157,7 +1156,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 ( !(cpu_vendor() & X86_VENDOR_AMD) )
reason = "only supported on AMD";
else if ( c->x86 < 0x10 )
reason = "only supported on AMD Fam10h+";
@@ -1165,7 +1164,7 @@ 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 ( cpu_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..b4d09fd6c1 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(!(cpu_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 ( !(cpu_vendor() & X86_VENDOR_HYGON) )
quirkflag = mcequirk_lookup_amd_quirkdata(c);
/* Assume that machine check support is available.
@@ -337,6 +337,5 @@ amd_mcheck_init(const struct cpuinfo_x86 *c, bool bsp)
ppin_msr = MSR_AMD_PPIN;
}
- return c->x86_vendor == X86_VENDOR_HYGON ?
- mcheck_hygon : mcheck_amd_famXX;
+ return (cpu_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..90dbb60236 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 ( (cpu_vendor() & X86_VENDOR_INTEL) &&
+ c->x86 == 6 && c->x86_model >= 0xe )
return true;
return false;
}
@@ -1018,7 +1018,6 @@ int vmce_intel_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
int vmce_intel_rdmsr(const struct vcpu *v, uint32_t msr, uint64_t *val)
{
- const struct cpu_policy *cp = v->domain->arch.cpu_policy;
unsigned int bank = msr - MSR_IA32_MC0_CTL2;
switch ( msr )
@@ -1036,7 +1035,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 ( !(cpu_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 0c1bfb691b..393bf8c787 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 ( cpu_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 ( cpu_vendor() & X86_VENDOR_INTEL )
guest_mcg_cap = INTEL_GUEST_MCG_CAP | MCG_LMCE_P;
else
guest_mcg_cap = AMD_GUEST_MCG_CAP;
--
2.43.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 11/12] x86/cpu: Migrate vendor checks to use cpu_vendor()
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
` (10 preceding siblings ...)
2026-02-06 16:15 ` [PATCH 10/12] x86/mcheck: Migrate vendor checks to use cpu_vendor() Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-12 13:17 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 12/12] x86: Migrate every remaining raw vendor check to cpu_vendor() Alejandro Vallejo
2026-02-09 9:21 ` [PATCH 00/12] const-ify vendor checks Jan Beulich
13 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
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/microcode/amd.c | 2 +-
xen/arch/x86/cpu/microcode/core.c | 2 +-
xen/arch/x86/cpu/mtrr/generic.c | 5 ++---
xen/arch/x86/cpu/mwait-idle.c | 5 ++---
xen/arch/x86/cpu/vpmu.c | 3 +--
8 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index fc496dc43e..422d81472e 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -327,7 +327,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 (!(cpu_vendor() & X86_VENDOR_AMD))
return 0;
if (osvw_id >= 0 && cpu_has(cpu, X86_FEATURE_OSVW)) {
@@ -418,8 +418,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 (!((cpu_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 6f4e723172..9a6fb90fec 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -436,7 +436,7 @@ void __init early_cpu_init(bool verbose)
paddr_bits -= (ebx >> 6) & 0x3f;
}
- if (!(c->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)))
+ if (!(cpu_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..f95ed90809 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) &&
+ (!(cpu_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/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 90f10ac8be..b4c8264e27 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)
uint8_t fixed_rev;
if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) ||
- boot_cpu_data.vendor != X86_VENDOR_AMD ||
+ !(cpu_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 ea0b35c499..ebe4d21026 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -216,7 +216,7 @@ static struct patch_with_flags nmi_patch =
/* 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 ( cpu_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..eab72dc103 100644
--- a/xen/arch/x86/cpu/mtrr/generic.c
+++ b/xen/arch/x86/cpu/mtrr/generic.c
@@ -218,9 +218,8 @@ 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 &&
- boot_cpu_data.x86 >= 0xf) ||
- boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
+ if (((cpu_vendor() & X86_VENDOR_AMD) && boot_cpu_data.x86 >= 0xf) ||
+ (cpu_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 5962ec1db9..8536bc97f6 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 (!(cpu_vendor() & X86_VENDOR_INTEL))
return -ENODEV;
id = x86_match_cpu(intel_idle_ids);
@@ -1816,8 +1816,7 @@ bool __init mwait_pc10_supported(void)
{
unsigned int ecx, edx, dummy;
- if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
- !cpu_has_monitor ||
+ if (!(cpu_vendor() & X86_VENDOR_INTEL) || !cpu_has_monitor ||
boot_cpu_data.cpuid_level < CPUID_MWAIT_LEAF)
return false;
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index b1ad4ebcf6..b79d57d0ec 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -398,7 +398,6 @@ int vpmu_load(struct vcpu *v, bool from_guest)
static int vpmu_arch_initialise(struct vcpu *v)
{
struct vpmu_struct *vpmu = vcpu_vpmu(v);
- uint8_t vendor = current_cpu_data.x86_vendor;
int ret;
BUILD_BUG_ON(sizeof(struct xen_pmu_intel_ctxt) > XENPMU_CTXT_PAD_SZ);
@@ -418,7 +417,7 @@ static int vpmu_arch_initialise(struct vcpu *v)
if ( vpmu_mode != XENPMU_MODE_OFF )
{
printk(XENLOG_G_WARNING "VPMU: Unknown CPU vendor %d. "
- "Disabling VPMU\n", vendor);
+ "Disabling VPMU\n", cpu_vendor());
opt_vpmu_enabled = 0;
vpmu_mode = XENPMU_MODE_OFF;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [PATCH 12/12] x86: Migrate every remaining raw vendor check to cpu_vendor()
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
` (11 preceding siblings ...)
2026-02-06 16:15 ` [PATCH 11/12] x86/cpu: " Alejandro Vallejo
@ 2026-02-06 16:15 ` Alejandro Vallejo
2026-02-12 13:29 ` Jan Beulich
2026-02-09 9:21 ` [PATCH 00/12] const-ify vendor checks Jan Beulich
13 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-06 16:15 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Jason Andryuk
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/apic.c | 2 +-
xen/arch/x86/cpu-policy.c | 8 ++++----
xen/arch/x86/cpuid.c | 5 ++---
xen/arch/x86/dom0_build.c | 2 +-
xen/arch/x86/domain.c | 12 +++++-------
xen/arch/x86/e820.c | 2 +-
xen/arch/x86/hvm/hvm.c | 3 +--
xen/arch/x86/hvm/ioreq.c | 3 +--
xen/arch/x86/hvm/vmx/vmx.c | 8 +++-----
xen/arch/x86/i8259.c | 5 ++---
xen/arch/x86/include/asm/guest_pt.h | 3 +--
xen/arch/x86/irq.c | 3 +--
xen/arch/x86/setup.c | 7 +++----
13 files changed, 26 insertions(+), 37 deletions(-)
diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index fb38be7ec3..fd3ac07aeb 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 ((cpu_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 62aff61d8c..055862b0b2 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -770,7 +770,7 @@ 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 ( (cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) &&
raw_cpu_policy.basic.sep )
__set_bit(X86_FEATURE_SEP, fs);
@@ -963,8 +963,8 @@ 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))
+ ((cpu_vendor() & (X86_VENDOR_AMD |
+ X86_VENDOR_HYGON))
? CPUID_GUEST_NR_EXTD_AMD
: CPUID_GUEST_NR_EXTD_INTEL) - 1);
@@ -998,7 +998,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 ( !(cpu_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..d64030bc09 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -437,8 +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 ||
- !vpmu_available(v) )
+ if ( !(cpu_vendor() & X86_VENDOR_INTEL) || !vpmu_available(v) )
*res = EMPTY_LEAF;
else
{
@@ -483,7 +482,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 ( (cpu_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..4b7f1bf034 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -572,7 +572,7 @@ 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 ( cpu_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 10a558e515..fd9c7f0be5 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -372,7 +372,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 ( cpu_vendor() & X86_VENDOR_AMD )
mask &= (~0ULL << 32) | p->basic.raw[6].c;
d->arch.pv.cpuidmasks->_6c = mask;
@@ -387,8 +387,7 @@ 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 ( cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
mask = (((uint64_t)p->feat.max_subleaf << 32) |
((uint32_t)mask & p->feat._7b0));
@@ -400,7 +399,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 ( cpu_vendor() & X86_VENDOR_INTEL )
mask &= (~0ULL << 32) | eax;
d->arch.pv.cpuidmasks->Da1 = mask;
@@ -424,7 +423,7 @@ 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 ( cpu_vendor() & ~(X86_VENDOR_AMD | X86_VENDOR_HYGON) )
edx &= ~CPUID_COMMON_1D_FEATURES;
switch( cpu_vendor() )
@@ -457,8 +456,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 &&
- p->basic.pmu_version == 0 )
+ if ( (cpu_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..0bf90f2283 100644
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -426,7 +426,7 @@ 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 = cpu_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 4d37a93c57..52a6cc69e5 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2681,8 +2681,7 @@ 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 ( (cpu_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..7a55b14f55 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -285,8 +285,7 @@ bool arch_ioreq_server_get_type_addr(const struct domain *d,
*type = XEN_DMOP_IO_RANGE_PCI;
*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 &&
+ if ( CF8_ADDR_HI(cf8) && (cpu_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 82c55f49ae..5637ee15e7 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3073,8 +3073,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 ||
- boot_cpu_data.x86 != 6 )
+ if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.x86 != 6 )
return false;
switch ( boot_cpu_data.x86_model )
@@ -3431,7 +3430,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 ||
+ !(cpu_vendor() & X86_VENDOR_INTEL) ||
boot_cpu_data.x86 != 6 )
return;
@@ -3476,8 +3475,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 ||
- boot_cpu_data.x86 != 6 )
+ if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.x86 != 6 )
return;
switch ( boot_cpu_data.x86_model )
diff --git a/xen/arch/x86/i8259.c b/xen/arch/x86/i8259.c
index 5c7e21a751..c19d2fe7b4 100644
--- a/xen/arch/x86/i8259.c
+++ b/xen/arch/x86/i8259.c
@@ -419,9 +419,8 @@ void __init init_IRQ(void)
* the interrupt.
*/
cpumask_copy(desc->arch.cpu_mask,
- (boot_cpu_data.x86_vendor &
- (X86_VENDOR_AMD | X86_VENDOR_HYGON) ? &cpumask_all
- : cpumask_of(cpu)));
+ ((cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON))
+ ? &cpumask_all : cpumask_of(cpu)));
desc->arch.vector = LEGACY_VECTOR(irq);
}
diff --git a/xen/arch/x86/include/asm/guest_pt.h b/xen/arch/x86/include/asm/guest_pt.h
index 21473f9bbc..cc3e9b504c 100644
--- a/xen/arch/x86/include/asm/guest_pt.h
+++ b/xen/arch/x86/include/asm/guest_pt.h
@@ -314,8 +314,7 @@ 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));
+ ((cpu_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 cc2934bfca..8e3706fb98 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2011,8 +2011,7 @@ 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))) &&
+ cpu_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/setup.c b/xen/arch/x86/setup.c
index 27c63d1d97..26b17aab8f 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1406,8 +1406,7 @@ void asmlinkage __init noreturn __start_xen(void)
* CPUs with this addressed enumerate CET-SSS to indicate that
* supervisor shadow stacks are now safe to use.
*/
- bool cpu_has_bug_shstk_fracture =
- boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
+ bool cpu_has_bug_shstk_fracture = (cpu_vendor() & X86_VENDOR_INTEL) &&
!boot_cpu_has(X86_FEATURE_CET_SSS);
/*
@@ -2038,10 +2037,10 @@ void asmlinkage __init noreturn __start_xen(void)
/* Do not enable SMEP/SMAP in PV shim on AMD and Hygon by default */
if ( opt_smep == -1 )
- opt_smep = !pv_shim || !(boot_cpu_data.x86_vendor &
+ opt_smep = !pv_shim || !(cpu_vendor() &
(X86_VENDOR_AMD | X86_VENDOR_HYGON));
if ( opt_smap == -1 )
- opt_smap = !pv_shim || !(boot_cpu_data.x86_vendor &
+ opt_smap = !pv_shim || !(cpu_vendor() &
(X86_VENDOR_AMD | X86_VENDOR_HYGON));
if ( !opt_smep )
--
2.43.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: [PATCH 00/12] const-ify vendor checks
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
` (12 preceding siblings ...)
2026-02-06 16:15 ` [PATCH 12/12] x86: Migrate every remaining raw vendor check to cpu_vendor() Alejandro Vallejo
@ 2026-02-09 9:21 ` Jan Beulich
2026-02-09 10:05 ` Alejandro Vallejo
13 siblings, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-09 9:21 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> Hi,
>
> This series is a big revamp of vendor-checking to enable it to perform DCE.
> It improves all configurations useful in practice at minimal cost in the full
> build, and at a massive advantage for the single-vendor case. Many ifdefs can
> go away as a side effect of the series.
>
> This series depends on cross-vendor removal:
> https://lore.kernel.org/xen-devel/20260205170923.38425-1-alejandro.garciavallejo@amd.com/T/#m4c3d318f37e4f24d0f8c62b104221aa5d428cebc
>
> Patch 1 in this series matches that of cross-vendor removal. It's logically
> required, but that's the single requirement.
>
> High level description
> ======================
>
> When compared to the RFC this makes a different approach The series introduces
> cpu_vendor() which maps to a constant in the single vendor case and to
> (boot_cpu_data.vendor & X86_ENABLED_VENDORS), where X86_ENABLED_VENDORS is a
> mask of the compile-time chosen vendors. This enables the compiler to detect
> dead-code at the uses and remove all unreachable branches, including in
> switches.
>
> When compared to the x86_vendor_is() macro introduced in the RFC, this is
> simpler. It achieves MOST of what the older macro did without touching the
> switches, with a few caveats:
>
> 1. Compiled-out vendors cause a panic, they don't fallback onto the unknown
> vendor case. In retrospect, this is a much saner thing to do.
I'm unconvinced here. Especially our Centaur and Shanghai support is at best
rudimentary. Treating those worse when configured-out than when configured-in
feels questionable.
> 2. equalities and inequalities have been replaced by equivalent (cpu_vendor() & ...)
> forms. This isn't stylistic preference. This form allows the compiler
> to merge the compared-against constant with X86_ENABLED_VENDORS, yielding
> much better codegen throughout the tree.
>
> The effect of (2) triples the delta in the full build below.
>
> Some differences might be attributable to the change from policy vendor checks
> to boot_cpu_data. In the case of the emulator it caused a 400 bytes increase
> due to the way it checks using LOTS of macro invocations, so I left that one
> piece using the policy vendor except for the single vendor case.
For the emulator I'd like to point out this question that I raised in the
AVX10 series:
"Since it'll be reducing code size, we may want to further convert
host_and_vcpu_must_have() to just vcpu_must_have() where appropriate
(should be [almost?] everywhere)."
Sadly there was no feedback an that (or really on almost all of that work) at
all so far.
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 00/12] const-ify vendor checks
2026-02-09 9:21 ` [PATCH 00/12] const-ify vendor checks Jan Beulich
@ 2026-02-09 10:05 ` Alejandro Vallejo
2026-02-09 10:15 ` Jan Beulich
0 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-09 10:05 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Mon Feb 9, 2026 at 10:21 AM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> Hi,
>>
>> This series is a big revamp of vendor-checking to enable it to perform DCE.
>> It improves all configurations useful in practice at minimal cost in the full
>> build, and at a massive advantage for the single-vendor case. Many ifdefs can
>> go away as a side effect of the series.
>>
>> This series depends on cross-vendor removal:
>> https://lore.kernel.org/xen-devel/20260205170923.38425-1-alejandro.garciavallejo@amd.com/T/#m4c3d318f37e4f24d0f8c62b104221aa5d428cebc
>>
>> Patch 1 in this series matches that of cross-vendor removal. It's logically
>> required, but that's the single requirement.
>>
>> High level description
>> ======================
>>
>> When compared to the RFC this makes a different approach The series introduces
>> cpu_vendor() which maps to a constant in the single vendor case and to
>> (boot_cpu_data.vendor & X86_ENABLED_VENDORS), where X86_ENABLED_VENDORS is a
>> mask of the compile-time chosen vendors. This enables the compiler to detect
>> dead-code at the uses and remove all unreachable branches, including in
>> switches.
>>
>> When compared to the x86_vendor_is() macro introduced in the RFC, this is
>> simpler. It achieves MOST of what the older macro did without touching the
>> switches, with a few caveats:
>>
>> 1. Compiled-out vendors cause a panic, they don't fallback onto the unknown
>> vendor case. In retrospect, this is a much saner thing to do.
>
> I'm unconvinced here. Especially our Centaur and Shanghai support is at best
> rudimentary. Treating those worse when configured-out than when configured-in
> feels questionable.
Isn't that the point of configuring out?
Besides the philosophical matter of whether or not a compiled-out vendor
should be allowed to run there's the more practical matter of what to do
with the x86_vendor field of boot_cpu_data. Because at that point our take
that cross-vendor support is forbidden is a lot weaker. If I can run an
AMD-hypervisor on an Intel host, what then? What policies would be allowed? If I
wipe x86_vendor then policies with some unknown vendor would be fine. Should the
leaves match too? If I do not wipe the field, should I do black magic to ensure
the behaviour is different depending on whether the vendor is compiled in or
not? What if I want to migrate a VM currently running in this hypothetical
hypervisor? The rules becomes seriously complex.
It's just a lot cleaner to take the stance that compiled out vendors can't run.
Then everything else is crystal clear and we avoid a universe's worth of corner
cases. I expect upstream Xen to support all cases (I'm skeptical about the
utility of the unknown vendor path, but oh well), but many downstreams might
benefit from killing off support for vendors they really will never touch.
>
>> 2. equalities and inequalities have been replaced by equivalent (cpu_vendor() & ...)
>> forms. This isn't stylistic preference. This form allows the compiler
>> to merge the compared-against constant with X86_ENABLED_VENDORS, yielding
>> much better codegen throughout the tree.
>>
>> The effect of (2) triples the delta in the full build below.
>>
>> Some differences might be attributable to the change from policy vendor checks
>> to boot_cpu_data. In the case of the emulator it caused a 400 bytes increase
>> due to the way it checks using LOTS of macro invocations, so I left that one
>> piece using the policy vendor except for the single vendor case.
>
> For the emulator I'd like to point out this question that I raised in the
> AVX10 series:
There's no optimisation shortage for the emulator. For that patch I just
ensure I didn't make a tricky situation worse. It is much better in the single-vendor case.
>
> "Since it'll be reducing code size, we may want to further convert
> host_and_vcpu_must_have() to just vcpu_must_have() where appropriate
> (should be [almost?] everywhere)."
>
> Sadly there was no feedback an that (or really on almost all of that work) at
> all so far.
It sounds fairly orthogonal to this, unless I'm missing the point.
In principle that would be fine. the vCPU features whose emulation requires
special instructions are most definitely a subset of those of the host anyway.
I agree many cases could be simplified as you describe.
I do see a worrying danger of XSA should the max policy ever exceed the
capabilities of the host on a feature required for emulating some instruction
for that very feature. Then the guest could abuse the emulator to trigger #UD
inside the hypervisor's emulation path.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 00/12] const-ify vendor checks
2026-02-09 10:05 ` Alejandro Vallejo
@ 2026-02-09 10:15 ` Jan Beulich
2026-02-09 11:56 ` Alejandro Vallejo
0 siblings, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-09 10:15 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 09.02.2026 11:05, Alejandro Vallejo wrote:
> On Mon Feb 9, 2026 at 10:21 AM CET, Jan Beulich wrote:
>> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>>> High level description
>>> ======================
>>>
>>> When compared to the RFC this makes a different approach The series introduces
>>> cpu_vendor() which maps to a constant in the single vendor case and to
>>> (boot_cpu_data.vendor & X86_ENABLED_VENDORS), where X86_ENABLED_VENDORS is a
>>> mask of the compile-time chosen vendors. This enables the compiler to detect
>>> dead-code at the uses and remove all unreachable branches, including in
>>> switches.
>>>
>>> When compared to the x86_vendor_is() macro introduced in the RFC, this is
>>> simpler. It achieves MOST of what the older macro did without touching the
>>> switches, with a few caveats:
>>>
>>> 1. Compiled-out vendors cause a panic, they don't fallback onto the unknown
>>> vendor case. In retrospect, this is a much saner thing to do.
>>
>> I'm unconvinced here. Especially our Centaur and Shanghai support is at best
>> rudimentary. Treating those worse when configured-out than when configured-in
>> feels questionable.
>
> Isn't that the point of configuring out?
That's what I'm unsure about.
> Besides the philosophical matter of whether or not a compiled-out vendor
> should be allowed to run there's the more practical matter of what to do
> with the x86_vendor field of boot_cpu_data. Because at that point our take
> that cross-vendor support is forbidden is a lot weaker. If I can run an
> AMD-hypervisor on an Intel host, what then? What policies would be allowed? If I
> wipe x86_vendor then policies with some unknown vendor would be fine. Should the
> leaves match too? If I do not wipe the field, should I do black magic to ensure
> the behaviour is different depending on whether the vendor is compiled in or
> not? What if I want to migrate a VM currently running in this hypothetical
> hypervisor? The rules becomes seriously complex.
>
> It's just a lot cleaner to take the stance that compiled out vendors can't run.
> Then everything else is crystal clear and we avoid a universe's worth of corner
> cases. I expect upstream Xen to support all cases (I'm skeptical about the
> utility of the unknown vendor path, but oh well), but many downstreams might
> benefit from killing off support for vendors they really will never touch.
To them, will panic()ing (or not) make a difference?
>>> 2. equalities and inequalities have been replaced by equivalent (cpu_vendor() & ...)
>>> forms. This isn't stylistic preference. This form allows the compiler
>>> to merge the compared-against constant with X86_ENABLED_VENDORS, yielding
>>> much better codegen throughout the tree.
>>>
>>> The effect of (2) triples the delta in the full build below.
>>>
>>> Some differences might be attributable to the change from policy vendor checks
>>> to boot_cpu_data. In the case of the emulator it caused a 400 bytes increase
>>> due to the way it checks using LOTS of macro invocations, so I left that one
>>> piece using the policy vendor except for the single vendor case.
>>
>> For the emulator I'd like to point out this question that I raised in the
>> AVX10 series:
>
> There's no optimisation shortage for the emulator. For that patch I just
> ensure I didn't make a tricky situation worse. It is much better in the single-vendor case.
>
>> "Since it'll be reducing code size, we may want to further convert
>> host_and_vcpu_must_have() to just vcpu_must_have() where appropriate
>> (should be [almost?] everywhere)."
>>
>> Sadly there was no feedback an that (or really on almost all of that work) at
>> all so far.
>
> It sounds fairly orthogonal to this, unless I'm missing the point.
It's largely orthogonal, except that if we had gone that route already, your
codegen diff might look somewhat different.
> In principle that would be fine. the vCPU features whose emulation requires
> special instructions are most definitely a subset of those of the host anyway.
>
> I agree many cases could be simplified as you describe.
>
> I do see a worrying danger of XSA should the max policy ever exceed the
> capabilities of the host on a feature required for emulating some instruction
> for that very feature. Then the guest could abuse the emulator to trigger #UD
> inside the hypervisor's emulation path.
Well, that max-policy related question is why I've raised the point, rather
than making (more) patches right away.
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 00/12] const-ify vendor checks
2026-02-09 10:15 ` Jan Beulich
@ 2026-02-09 11:56 ` Alejandro Vallejo
2026-02-09 12:52 ` Jan Beulich
0 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-09 11:56 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Mon Feb 9, 2026 at 11:15 AM CET, Jan Beulich wrote:
> On 09.02.2026 11:05, Alejandro Vallejo wrote:
>> On Mon Feb 9, 2026 at 10:21 AM CET, Jan Beulich wrote:
>>> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>>>> High level description
>>>> ======================
>>>>
>>>> When compared to the RFC this makes a different approach The series introduces
>>>> cpu_vendor() which maps to a constant in the single vendor case and to
>>>> (boot_cpu_data.vendor & X86_ENABLED_VENDORS), where X86_ENABLED_VENDORS is a
>>>> mask of the compile-time chosen vendors. This enables the compiler to detect
>>>> dead-code at the uses and remove all unreachable branches, including in
>>>> switches.
>>>>
>>>> When compared to the x86_vendor_is() macro introduced in the RFC, this is
>>>> simpler. It achieves MOST of what the older macro did without touching the
>>>> switches, with a few caveats:
>>>>
>>>> 1. Compiled-out vendors cause a panic, they don't fallback onto the unknown
>>>> vendor case. In retrospect, this is a much saner thing to do.
>>>
>>> I'm unconvinced here. Especially our Centaur and Shanghai support is at best
>>> rudimentary. Treating those worse when configured-out than when configured-in
>>> feels questionable.
>>
>> Isn't that the point of configuring out?
>
> That's what I'm unsure about.
I'm really missing what you're trying to make, sorry. How, if at all, is it
helpful for a hypervisor with a compiled out vendor to be bootable on a machine
of that vendor?
>
>> Besides the philosophical matter of whether or not a compiled-out vendor
>> should be allowed to run there's the more practical matter of what to do
>> with the x86_vendor field of boot_cpu_data. Because at that point our take
>> that cross-vendor support is forbidden is a lot weaker. If I can run an
>> AMD-hypervisor on an Intel host, what then? What policies would be allowed? If I
>> wipe x86_vendor then policies with some unknown vendor would be fine. Should the
>> leaves match too? If I do not wipe the field, should I do black magic to ensure
>> the behaviour is different depending on whether the vendor is compiled in or
>> not? What if I want to migrate a VM currently running in this hypothetical
>> hypervisor? The rules becomes seriously complex.
>>
>> It's just a lot cleaner to take the stance that compiled out vendors can't run.
>> Then everything else is crystal clear and we avoid a universe's worth of corner
>> cases. I expect upstream Xen to support all cases (I'm skeptical about the
>> utility of the unknown vendor path, but oh well), but many downstreams might
>> benefit from killing off support for vendors they really will never touch.
>
> To them, will panic()ing (or not) make a difference?
One would hope not because the're compiling them out for a reason.
But for upstream, not panicking brings a sea of corner cases. The ones I
mentioned above is not the whole list.
Turning the question around. Who benefits from not panicking?
>
>>>> 2. equalities and inequalities have been replaced by equivalent (cpu_vendor() & ...)
>>>> forms. This isn't stylistic preference. This form allows the compiler
>>>> to merge the compared-against constant with X86_ENABLED_VENDORS, yielding
>>>> much better codegen throughout the tree.
>>>>
>>>> The effect of (2) triples the delta in the full build below.
>>>>
>>>> Some differences might be attributable to the change from policy vendor checks
>>>> to boot_cpu_data. In the case of the emulator it caused a 400 bytes increase
>>>> due to the way it checks using LOTS of macro invocations, so I left that one
>>>> piece using the policy vendor except for the single vendor case.
>>>
>>> For the emulator I'd like to point out this question that I raised in the
>>> AVX10 series:
>>
>> There's no optimisation shortage for the emulator. For that patch I just
>> ensure I didn't make a tricky situation worse. It is much better in the single-vendor case.
>>
>>> "Since it'll be reducing code size, we may want to further convert
>>> host_and_vcpu_must_have() to just vcpu_must_have() where appropriate
>>> (should be [almost?] everywhere)."
>>>
>>> Sadly there was no feedback an that (or really on almost all of that work) at
>>> all so far.
>>
>> It sounds fairly orthogonal to this, unless I'm missing the point.
>
> It's largely orthogonal, except that if we had gone that route already, your
> codegen diff might look somewhat different.
>
>> In principle that would be fine. the vCPU features whose emulation requires
>> special instructions are most definitely a subset of those of the host anyway.
>>
>> I agree many cases could be simplified as you describe.
>>
>> I do see a worrying danger of XSA should the max policy ever exceed the
>> capabilities of the host on a feature required for emulating some instruction
>> for that very feature. Then the guest could abuse the emulator to trigger #UD
>> inside the hypervisor's emulation path.
>
> Well, that max-policy related question is why I've raised the point, rather
> than making (more) patches right away.
All jmp_rel() macros have amd_like() inside, and other checks are open-coded
in many places. The problem is that offsets into the policy pointer (which
is stored in a register) end up being global accesses to an offset from a
non-register-cached 64bit address. And that adds up. having an amd_like boolean
in the ctxt would've helped, but I went for the least intrusive solution.
I'm not sure how what you brought up would've helped for this particular codegen
matter.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 00/12] const-ify vendor checks
2026-02-09 11:56 ` Alejandro Vallejo
@ 2026-02-09 12:52 ` Jan Beulich
2026-02-09 14:37 ` Alejandro Vallejo
0 siblings, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-09 12:52 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 09.02.2026 12:56, Alejandro Vallejo wrote:
> On Mon Feb 9, 2026 at 11:15 AM CET, Jan Beulich wrote:
>> On 09.02.2026 11:05, Alejandro Vallejo wrote:
>>> On Mon Feb 9, 2026 at 10:21 AM CET, Jan Beulich wrote:
>>>> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>>>>> High level description
>>>>> ======================
>>>>>
>>>>> When compared to the RFC this makes a different approach The series introduces
>>>>> cpu_vendor() which maps to a constant in the single vendor case and to
>>>>> (boot_cpu_data.vendor & X86_ENABLED_VENDORS), where X86_ENABLED_VENDORS is a
>>>>> mask of the compile-time chosen vendors. This enables the compiler to detect
>>>>> dead-code at the uses and remove all unreachable branches, including in
>>>>> switches.
>>>>>
>>>>> When compared to the x86_vendor_is() macro introduced in the RFC, this is
>>>>> simpler. It achieves MOST of what the older macro did without touching the
>>>>> switches, with a few caveats:
>>>>>
>>>>> 1. Compiled-out vendors cause a panic, they don't fallback onto the unknown
>>>>> vendor case. In retrospect, this is a much saner thing to do.
>>>>
>>>> I'm unconvinced here. Especially our Centaur and Shanghai support is at best
>>>> rudimentary. Treating those worse when configured-out than when configured-in
>>>> feels questionable.
>>>
>>> Isn't that the point of configuring out?
>>
>> That's what I'm unsure about.
>
> I'm really missing what you're trying to make, sorry. How, if at all, is it
> helpful for a hypervisor with a compiled out vendor to be bootable on a machine
> of that vendor?
No more and no less than for a system with CPUs from a vendor we don't have
support for at all. Let's assume someone wants to start adding support for
a new vendor. They may first try Xen as-is. This wouldn't panic. Depending
on how exactly they would start adding stuff, Xen might suddenly panic,
despite functionally nothing having changed.
>>> Besides the philosophical matter of whether or not a compiled-out vendor
>>> should be allowed to run there's the more practical matter of what to do
>>> with the x86_vendor field of boot_cpu_data. Because at that point our take
>>> that cross-vendor support is forbidden is a lot weaker. If I can run an
>>> AMD-hypervisor on an Intel host, what then? What policies would be allowed? If I
>>> wipe x86_vendor then policies with some unknown vendor would be fine. Should the
>>> leaves match too? If I do not wipe the field, should I do black magic to ensure
>>> the behaviour is different depending on whether the vendor is compiled in or
>>> not? What if I want to migrate a VM currently running in this hypothetical
>>> hypervisor? The rules becomes seriously complex.
>>>
>>> It's just a lot cleaner to take the stance that compiled out vendors can't run.
>>> Then everything else is crystal clear and we avoid a universe's worth of corner
>>> cases. I expect upstream Xen to support all cases (I'm skeptical about the
>>> utility of the unknown vendor path, but oh well), but many downstreams might
>>> benefit from killing off support for vendors they really will never touch.
>>
>> To them, will panic()ing (or not) make a difference?
>
> One would hope not because the're compiling them out for a reason.
> But for upstream, not panicking brings a sea of corner cases. The ones I
> mentioned above is not the whole list.
>
> Turning the question around. Who benefits from not panicking?
Certain things may work. But more generally - see above. Turning this
question around also isn't quite appropriate imo: You want to introduce
the panic(), so it's on you to justify doing so (which includes making
clear why omitting that small piece of code would be a bad idea).
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 00/12] const-ify vendor checks
2026-02-09 12:52 ` Jan Beulich
@ 2026-02-09 14:37 ` Alejandro Vallejo
0 siblings, 0 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-09 14:37 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Mon Feb 9, 2026 at 1:52 PM CET, Jan Beulich wrote:
> On 09.02.2026 12:56, Alejandro Vallejo wrote:
>> On Mon Feb 9, 2026 at 11:15 AM CET, Jan Beulich wrote:
>>> On 09.02.2026 11:05, Alejandro Vallejo wrote:
>>>> On Mon Feb 9, 2026 at 10:21 AM CET, Jan Beulich wrote:
>>>>> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>>>>>> High level description
>>>>>> ======================
>>>>>>
>>>>>> When compared to the RFC this makes a different approach The series introduces
>>>>>> cpu_vendor() which maps to a constant in the single vendor case and to
>>>>>> (boot_cpu_data.vendor & X86_ENABLED_VENDORS), where X86_ENABLED_VENDORS is a
>>>>>> mask of the compile-time chosen vendors. This enables the compiler to detect
>>>>>> dead-code at the uses and remove all unreachable branches, including in
>>>>>> switches.
>>>>>>
>>>>>> When compared to the x86_vendor_is() macro introduced in the RFC, this is
>>>>>> simpler. It achieves MOST of what the older macro did without touching the
>>>>>> switches, with a few caveats:
>>>>>>
>>>>>> 1. Compiled-out vendors cause a panic, they don't fallback onto the unknown
>>>>>> vendor case. In retrospect, this is a much saner thing to do.
>>>>>
>>>>> I'm unconvinced here. Especially our Centaur and Shanghai support is at best
>>>>> rudimentary. Treating those worse when configured-out than when configured-in
>>>>> feels questionable.
>>>>
>>>> Isn't that the point of configuring out?
>>>
>>> That's what I'm unsure about.
>>
>> I'm really missing what you're trying to make, sorry. How, if at all, is it
>> helpful for a hypervisor with a compiled out vendor to be bootable on a machine
>> of that vendor?
>
> No more and no less than for a system with CPUs from a vendor we don't have
> support for at all. Let's assume someone wants to start adding support for
> a new vendor. They may first try Xen as-is. This wouldn't panic.
> on how exactly they would start adding stuff, Xen might suddenly panic,
> despite functionally nothing having changed.
There's an "unknown CPU vendor" option for that. With that option enabled
unknown vendor keep working as intended. If it booted in the first place it's
because the unknown vendor option was enabled. The panic would happen iff they
add their own vendor to the code, hook it up on the lookup function and miss
the X86_ENABLED_VENDORS addition. Which, incidentally, would notify them right
away through a panic rather than having them waste time trying to trigger code
DCE is intentionally removing.
>
>>>> Besides the philosophical matter of whether or not a compiled-out vendor
>>>> should be allowed to run there's the more practical matter of what to do
>>>> with the x86_vendor field of boot_cpu_data. Because at that point our take
>>>> that cross-vendor support is forbidden is a lot weaker. If I can run an
>>>> AMD-hypervisor on an Intel host, what then? What policies would be allowed? If I
>>>> wipe x86_vendor then policies with some unknown vendor would be fine. Should the
>>>> leaves match too? If I do not wipe the field, should I do black magic to ensure
>>>> the behaviour is different depending on whether the vendor is compiled in or
>>>> not? What if I want to migrate a VM currently running in this hypothetical
>>>> hypervisor? The rules becomes seriously complex.
>>>>
>>>> It's just a lot cleaner to take the stance that compiled out vendors can't run.
>>>> Then everything else is crystal clear and we avoid a universe's worth of corner
>>>> cases. I expect upstream Xen to support all cases (I'm skeptical about the
>>>> utility of the unknown vendor path, but oh well), but many downstreams might
>>>> benefit from killing off support for vendors they really will never touch.
>>>
>>> To them, will panic()ing (or not) make a difference?
>>
>> One would hope not because the're compiling them out for a reason.
>> But for upstream, not panicking brings a sea of corner cases. The ones I
>> mentioned above is not the whole list.
>>
>> Turning the question around. Who benefits from not panicking?
>
> Certain things may work. But more generally - see above. Turning this
> question around also isn't quite appropriate imo: You want to introduce
> the panic(), so it's on you to justify doing so (which includes making
> clear why omitting that small piece of code would be a bad idea).
The justification is twofold. To aleviate complexity in Xen, and fullful a
security invariant in the presence of misuse. Letting a guest run is easy (not
quite a oneliner, but not much more). However it raises a number of questions
with complicated answers:
Save/restore | Live migration
=============================
The biggest hurdle, as I mentioned, is live-migrations/save-restore.
If I xl restore a Hygon VM on a host running in "unknown" mode (being an
AMD-only hypervisor with the unknown vendor cfg enabled on an Centaur host): What
should the vendor policy check say when I restore?
a. They are incompatible: Because Hygon is compiled out.
b. They are incompatible: Because Hygon doesn't match the vendor bytes of the host.
b. They are compatible: Because they are both "unknown".
c. What if the host _was_ Hygon, but Hygon was disabled?
It's just a mess. And the fact that the unknown vendor path has even less
testing than that of Centaur we can probably count on it being extremely buggy.
And on the topic of the consequences of bugs, there's the other argument...
Security argument
=================
If an AMD-only hypervisor runs on Intel hardware it just wouldn't be safe. We
can make statements that this configuration being unsupported and known unsafe,
and everything along those lines but mistakes happen. A mistake in production
where a vendor-trimmed hypervisor lands on the very trimmed vendor means a
security hole. And saying "that's a 'you' problem" doesn't cut it, because I can
make the same statement to users and developers of new vendors.
I can only see very theoretical use cases for the lack of a panic and a very
clear danger of misuse. early_cpu_init() runs after COM ports are configured so
it's not like we'd reboot without stating why.
================================================
If it's a hard requirement, I can change it so the panic behaves as it did
on the RFC, but IMO it creates real problems for the benefit of imaginary use
cases.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's
2026-02-06 16:15 ` [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's Alejandro Vallejo
@ 2026-02-10 8:19 ` Roger Pau Monné
2026-02-10 10:11 ` Alejandro Vallejo
2026-02-11 15:41 ` Jan Beulich
1 sibling, 1 reply; 55+ messages in thread
From: Roger Pau Monné @ 2026-02-10 8:19 UTC (permalink / raw)
To: Alejandro Vallejo; +Cc: xen-devel, Jan Beulich, Andrew Cooper, Jason Andryuk
On Fri, Feb 06, 2026 at 05:15:23PM +0100, 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>
> ---
> Patch 1 from the cross-vendor series. Do not merge here. It's simply for
> consistency.
> ---
> 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..079c42a29b 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->x86_vendor != host->x86_vendor) ||
> + (guest->basic.max_leaf > host->basic.max_leaf) )
You possibly want to expand test-cpu-policy.c to add a small test to
ensure this works as expected? Not that it shouldn't, but it's
trivial to expand test_is_compatible_{success,failure}() to add a
small test for the vendor checking.
Thanks, Roger.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 02/12] x86: Add more granularity to the vendors in Kconfig
2026-02-06 16:15 ` [PATCH 02/12] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
@ 2026-02-10 8:26 ` Roger Pau Monné
2026-02-10 10:04 ` Alejandro Vallejo
2026-02-11 16:06 ` Jan Beulich
1 sibling, 1 reply; 55+ messages in thread
From: Roger Pau Monné @ 2026-02-10 8:26 UTC (permalink / raw)
To: Alejandro Vallejo; +Cc: xen-devel, Jan Beulich, Andrew Cooper, Jason Andryuk
On Fri, Feb 06, 2026 at 05:15:24PM +0100, 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 will be allowed.
>
> A prerequisite for such optimisation is to expand Kconfig to have the
> full set of vendors. Adds Hygon, Centaur, Shanghai and the unknown-vendor
> path. Have Hygon select AMD, and Centaur|Shanghai select Intel.
>
> Not a functional change.
>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
> rfc -> v1:
> * s/depends on/select/
> * s/UNKNOWN_CPU/UNKNOWN_CPU_VENDOR/
> * Removed "if unsure, say Y"
> ---
> xen/arch/x86/Kconfig.cpu | 43 +++++++++++++++++++++++++++++++++++++++
> xen/arch/x86/cpu/Makefile | 6 +++---
> 2 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu
> index 5fb18db1aa..f895cfe97a 100644
> --- a/xen/arch/x86/Kconfig.cpu
> +++ b/xen/arch/x86/Kconfig.cpu
> @@ -19,4 +19,47 @@ 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"
> + select 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"
> + select 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"
> + select 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_VENDOR
> + bool "Support unknown CPU vendors"
> + 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 selecting a single vendor with this
> + option cleared produces a much smaller build.
> +
> endmenu
> 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
Maybe I'm missing some context here, but I think this would break the
build?
cpu/centaur.c exports centaur_cpu_dev which is referenced from
cpu/common.c, and I don't see you making that reference in
cpu/common.c conditional to CONFIG_CENTAUR being defined?
I think the same applies to Hygon and Shanghai.
Thanks, Roger.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor
2026-02-06 16:15 ` [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor Alejandro Vallejo
@ 2026-02-10 8:46 ` Roger Pau Monné
2026-02-10 10:35 ` Alejandro Vallejo
2026-02-11 16:04 ` Jan Beulich
2026-02-12 10:52 ` Jan Beulich
2 siblings, 1 reply; 55+ messages in thread
From: Roger Pau Monné @ 2026-02-10 8:46 UTC (permalink / raw)
To: Alejandro Vallejo; +Cc: xen-devel, Jan Beulich, Andrew Cooper, Jason Andryuk
On Fri, Feb 06, 2026 at 05:15:25PM +0100, Alejandro Vallejo wrote:
> Introduces various optimisations that rely on constant folding, Value
> Range Propagation (VRP), and Dead Code Elimination (DCE) to aggressively
> eliminate code surrounding the uses of the function.
>
> * For single-vendor+no-unknown-vendor builds returns a compile-time
> constant.
This is kind of misleading IMO. It will possibly allow such
optimization for Intel or AMD, but not for Hygon/Centaur/Shanghai, as
for those CPUs you will always end up selecting either Intel or AMD as
a requisite (so X86_ENABLED_VENDORS will never have only a single bit
set).
Not saying it's bad, but I think the comment above should be adjusted
a bit to notice that such compile time optimizations for single vendor
builds will only be applicable to Intel or AMD builds.
> * For all other cases it ANDs the result with the mask of compiled
> vendors, with the effect of performing DCE in switch cases, removing
> dead conditionals, etc.
>
> It's difficult to reason about codegen in general in a project this big,
> but in this case the ANDed constant combines with the values typically
> checked against, folding into a comparison against zero. Thus, it's better
> for codegen to AND its result with the desired compared-against vendor,
> rather than using (in)equality operators. That way the comparison is
> always against zero.
>
> "cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)"
>
> turns into (cpu_vendor() & X86_VENDOR_AMD) in AMD-only builds (AND +
> cmp with zero). Whereas this...
>
> "cpu_vendor() == X86_VENDOR_AMD"
>
> forces cpu_vendor() to be ANDed and then compared to a non-zero value.
>
> Later patches take the opportunity and make this refactor as cpu_vendor()
> is introduced throughout the tree.
>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
> xen/arch/x86/cpu/common.c | 6 +++++-
> xen/arch/x86/guest/xen/xen.c | 4 ++++
> xen/arch/x86/include/asm/cpufeature.h | 27 +++++++++++++++++++++++++++
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> index ebe2baf8b9..6f4e723172 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -328,7 +328,11 @@ void __init early_cpu_init(bool verbose)
> *(u32 *)&c->x86_vendor_id[4] = edx;
>
> c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
> - switch (c->x86_vendor) {
> + if ( c->x86_vendor != cpu_vendor() )
> + panic("CPU vendor not compiled-in: %s",
> + x86_cpuid_vendor_to_str(c->x86_vendor));
I think you want to print both the current compiled in support plus
the host vendor as part of the panic message.
> +
> + switch (cpu_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;
> diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c
> index 77a3a8742a..ec558bcbdb 100644
> --- a/xen/arch/x86/guest/xen/xen.c
> +++ b/xen/arch/x86/guest/xen/xen.c
> @@ -57,6 +57,10 @@ void asmlinkage __init early_hypercall_setup(void)
> cpuid(0, &eax, &ebx, &ecx, &edx);
>
> boot_cpu_data.x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
> +
> + if ( cpu_vendor() != boot_cpu_data.x86_vendor )
> + panic("CPU vendor not compiled-in: %s",
> + x86_cpuid_vendor_to_str(boot_cpu_data.x86_vendor));
Is this going to be useful? I fear the panic here might happen even
before the console is setup, so a user won't get any output from Xen
at all.
Would it be fine to allow such mismatch in the hypercall setup, just
for the sake of getting the console page setup so that
early_cpu_init() can print a proper error message?
Allowing the vendor mismatch here won't require any extra code, it's
just the selection of the instruction to use to call into Xen when
running in guest/shim mode.
Thanks, Roger.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 02/12] x86: Add more granularity to the vendors in Kconfig
2026-02-10 8:26 ` Roger Pau Monné
@ 2026-02-10 10:04 ` Alejandro Vallejo
0 siblings, 0 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-10 10:04 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: xen-devel, Jan Beulich, Andrew Cooper, Jason Andryuk
On Tue Feb 10, 2026 at 9:26 AM CET, Roger Pau Monné wrote:
> On Fri, Feb 06, 2026 at 05:15:24PM +0100, 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 will be allowed.
>>
>> A prerequisite for such optimisation is to expand Kconfig to have the
>> full set of vendors. Adds Hygon, Centaur, Shanghai and the unknown-vendor
>> path. Have Hygon select AMD, and Centaur|Shanghai select Intel.
>>
>> Not a functional change.
>>
>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>> ---
>> rfc -> v1:
>> * s/depends on/select/
>> * s/UNKNOWN_CPU/UNKNOWN_CPU_VENDOR/
>> * Removed "if unsure, say Y"
>> ---
>> xen/arch/x86/Kconfig.cpu | 43 +++++++++++++++++++++++++++++++++++++++
>> xen/arch/x86/cpu/Makefile | 6 +++---
>> 2 files changed, 46 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu
>> index 5fb18db1aa..f895cfe97a 100644
>> --- a/xen/arch/x86/Kconfig.cpu
>> +++ b/xen/arch/x86/Kconfig.cpu
>> @@ -19,4 +19,47 @@ 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"
>> + select 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"
>> + select 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"
>> + select 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_VENDOR
>> + bool "Support unknown CPU vendors"
>> + 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 selecting a single vendor with this
>> + option cleared produces a much smaller build.
>> +
>> endmenu
>> 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
>
> Maybe I'm missing some context here, but I think this would break the
> build?
>
> cpu/centaur.c exports centaur_cpu_dev which is referenced from
> cpu/common.c, and I don't see you making that reference in
> cpu/common.c conditional to CONFIG_CENTAUR being defined?
>
> I think the same applies to Hygon and Shanghai.
>
> Thanks, Roger.
Hmm. Yes, these hunks go on the following patch for bisectability. They work due
to the AND operation at the switch condition.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's
2026-02-10 8:19 ` Roger Pau Monné
@ 2026-02-10 10:11 ` Alejandro Vallejo
0 siblings, 0 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-10 10:11 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: xen-devel, Jan Beulich, Andrew Cooper, Jason Andryuk
On Tue Feb 10, 2026 at 9:19 AM CET, Roger Pau Monné wrote:
> On Fri, Feb 06, 2026 at 05:15:23PM +0100, 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>
>> ---
>> Patch 1 from the cross-vendor series. Do not merge here. It's simply for
>> consistency.
>> ---
>> 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..079c42a29b 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->x86_vendor != host->x86_vendor) ||
>> + (guest->basic.max_leaf > host->basic.max_leaf) )
>
> You possibly want to expand test-cpu-policy.c to add a small test to
> ensure this works as expected? Not that it shouldn't, but it's
> trivial to expand test_is_compatible_{success,failure}() to add a
> small test for the vendor checking.
>
> Thanks, Roger.
I didn't consider it. I guess I could. What I'm thinking also is that this
vendor check should probably check the encoded vendor (ebcx/ecx/edx) rather
than the decoded one.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor
2026-02-10 8:46 ` Roger Pau Monné
@ 2026-02-10 10:35 ` Alejandro Vallejo
2026-02-10 12:06 ` Roger Pau Monné
0 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-10 10:35 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: xen-devel, Jan Beulich, Andrew Cooper, Jason Andryuk
On Tue Feb 10, 2026 at 9:46 AM CET, Roger Pau Monné wrote:
> On Fri, Feb 06, 2026 at 05:15:25PM +0100, Alejandro Vallejo wrote:
>> Introduces various optimisations that rely on constant folding, Value
>> Range Propagation (VRP), and Dead Code Elimination (DCE) to aggressively
>> eliminate code surrounding the uses of the function.
>>
>> * For single-vendor+no-unknown-vendor builds returns a compile-time
>> constant.
>
> This is kind of misleading IMO. It will possibly allow such
> optimization for Intel or AMD, but not for Hygon/Centaur/Shanghai, as
> for those CPUs you will always end up selecting either Intel or AMD as
> a requisite (so X86_ENABLED_VENDORS will never have only a single bit
> set).
>
> Not saying it's bad, but I think the comment above should be adjusted
> a bit to notice that such compile time optimizations for single vendor
> builds will only be applicable to Intel or AMD builds.
You can't build a Hygon-only hypervisor with these changes. I can rewrite the
commit message to clearly state which vendors are subject to the optimisation,
though I'd fully expect users to notice they just can't deselect AMD when
building for Hygon.
>
>> * For all other cases it ANDs the result with the mask of compiled
>> vendors, with the effect of performing DCE in switch cases, removing
>> dead conditionals, etc.
>>
>> It's difficult to reason about codegen in general in a project this big,
>> but in this case the ANDed constant combines with the values typically
>> checked against, folding into a comparison against zero. Thus, it's better
>> for codegen to AND its result with the desired compared-against vendor,
>> rather than using (in)equality operators. That way the comparison is
>> always against zero.
>>
>> "cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)"
>>
>> turns into (cpu_vendor() & X86_VENDOR_AMD) in AMD-only builds (AND +
>> cmp with zero). Whereas this...
>>
>> "cpu_vendor() == X86_VENDOR_AMD"
>>
>> forces cpu_vendor() to be ANDed and then compared to a non-zero value.
>>
>> Later patches take the opportunity and make this refactor as cpu_vendor()
>> is introduced throughout the tree.
>>
>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>> ---
>> xen/arch/x86/cpu/common.c | 6 +++++-
>> xen/arch/x86/guest/xen/xen.c | 4 ++++
>> xen/arch/x86/include/asm/cpufeature.h | 27 +++++++++++++++++++++++++++
>> 3 files changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
>> index ebe2baf8b9..6f4e723172 100644
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -328,7 +328,11 @@ void __init early_cpu_init(bool verbose)
>> *(u32 *)&c->x86_vendor_id[4] = edx;
>>
>> c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
>> - switch (c->x86_vendor) {
>> + if ( c->x86_vendor != cpu_vendor() )
>> + panic("CPU vendor not compiled-in: %s",
>> + x86_cpuid_vendor_to_str(c->x86_vendor));
>
> I think you want to print both the current compiled in support plus
> the host vendor as part of the panic message.
The mask of supported vendors, you mean? That could be helpful.
>
>> +
>> + switch (cpu_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;
>> diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c
>> index 77a3a8742a..ec558bcbdb 100644
>> --- a/xen/arch/x86/guest/xen/xen.c
>> +++ b/xen/arch/x86/guest/xen/xen.c
>> @@ -57,6 +57,10 @@ void asmlinkage __init early_hypercall_setup(void)
>> cpuid(0, &eax, &ebx, &ecx, &edx);
>>
>> boot_cpu_data.x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
>> +
>> + if ( cpu_vendor() != boot_cpu_data.x86_vendor )
>> + panic("CPU vendor not compiled-in: %s",
>> + x86_cpuid_vendor_to_str(boot_cpu_data.x86_vendor));
>
> Is this going to be useful? I fear the panic here might happen even
> before the console is setup, so a user won't get any output from Xen
> at all.
It is true that early_cpu_init() is invoked immediately after serial is
set up, so any other vendor check ends up being fairly useless.
OTOH, thinking about it may stand to reason to have:
1. A very early panic in assembly, like that of missing nx.
2. Have this early hypercall setup nonsense present ONLY when INTEL && AMD
are both enabled. It really makes no sense to probe when you have
explilcitly compiled for a single vendor.
>
> Would it be fine to allow such mismatch in the hypercall setup, just
> for the sake of getting the console page setup so that
> early_cpu_init() can print a proper error message?
>
> Allowing the vendor mismatch here won't require any extra code, it's
> just the selection of the instruction to use to call into Xen when
> running in guest/shim mode.
It'd be fine, yes. Maybe with a comment noting we can use vm{m,}call whether
or not cpu_vendor() == 0 because the instruction itself is supported by HW.
OTOH, I could also fully drop the dynamic detection logic on AMDLIKE-only or
INTELLIKE-only builds like I mentioned above.
I sort of like that second option, as it allows removing hypercall.S and
hook it to the real hypercall machinery, that at that point can have the
alternatives removed.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor
2026-02-10 10:35 ` Alejandro Vallejo
@ 2026-02-10 12:06 ` Roger Pau Monné
0 siblings, 0 replies; 55+ messages in thread
From: Roger Pau Monné @ 2026-02-10 12:06 UTC (permalink / raw)
To: Alejandro Vallejo; +Cc: xen-devel, Jan Beulich, Andrew Cooper, Jason Andryuk
On Tue, Feb 10, 2026 at 11:35:20AM +0100, Alejandro Vallejo wrote:
> On Tue Feb 10, 2026 at 9:46 AM CET, Roger Pau Monné wrote:
> > On Fri, Feb 06, 2026 at 05:15:25PM +0100, Alejandro Vallejo wrote:
> >> Introduces various optimisations that rely on constant folding, Value
> >> Range Propagation (VRP), and Dead Code Elimination (DCE) to aggressively
> >> eliminate code surrounding the uses of the function.
> >>
> >> * For single-vendor+no-unknown-vendor builds returns a compile-time
> >> constant.
> >
> > This is kind of misleading IMO. It will possibly allow such
> > optimization for Intel or AMD, but not for Hygon/Centaur/Shanghai, as
> > for those CPUs you will always end up selecting either Intel or AMD as
> > a requisite (so X86_ENABLED_VENDORS will never have only a single bit
> > set).
> >
> > Not saying it's bad, but I think the comment above should be adjusted
> > a bit to notice that such compile time optimizations for single vendor
> > builds will only be applicable to Intel or AMD builds.
>
> You can't build a Hygon-only hypervisor with these changes. I can rewrite the
> commit message to clearly state which vendors are subject to the optimisation,
> though I'd fully expect users to notice they just can't deselect AMD when
> building for Hygon.
Yes, right, I guess someone would notice that right now it's not
possible to select just Hygon for example, and that it always pulls
AMD.
In theory it should be possible to decouple the set of supported CPUs
in X86_ENABLED_VENDORS from the set of support code that you build the
hypervisor with. IOW: it should be possible to build an hypervisor
with CONFIG_AMD & CONFIG_HYGON that only has CONFIG_HYGON in
X86_ENABLED_VENDORS. For the purpose of enabling secondary vendors
(like Hygon) to also take advantage of such build time
optimizations.
> >
> >> * For all other cases it ANDs the result with the mask of compiled
> >> vendors, with the effect of performing DCE in switch cases, removing
> >> dead conditionals, etc.
> >>
> >> It's difficult to reason about codegen in general in a project this big,
> >> but in this case the ANDed constant combines with the values typically
> >> checked against, folding into a comparison against zero. Thus, it's better
> >> for codegen to AND its result with the desired compared-against vendor,
> >> rather than using (in)equality operators. That way the comparison is
> >> always against zero.
> >>
> >> "cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)"
> >>
> >> turns into (cpu_vendor() & X86_VENDOR_AMD) in AMD-only builds (AND +
> >> cmp with zero). Whereas this...
> >>
> >> "cpu_vendor() == X86_VENDOR_AMD"
> >>
> >> forces cpu_vendor() to be ANDed and then compared to a non-zero value.
> >>
> >> Later patches take the opportunity and make this refactor as cpu_vendor()
> >> is introduced throughout the tree.
> >>
> >> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> >> ---
> >> xen/arch/x86/cpu/common.c | 6 +++++-
> >> xen/arch/x86/guest/xen/xen.c | 4 ++++
> >> xen/arch/x86/include/asm/cpufeature.h | 27 +++++++++++++++++++++++++++
> >> 3 files changed, 36 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> >> index ebe2baf8b9..6f4e723172 100644
> >> --- a/xen/arch/x86/cpu/common.c
> >> +++ b/xen/arch/x86/cpu/common.c
> >> @@ -328,7 +328,11 @@ void __init early_cpu_init(bool verbose)
> >> *(u32 *)&c->x86_vendor_id[4] = edx;
> >>
> >> c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
> >> - switch (c->x86_vendor) {
> >> + if ( c->x86_vendor != cpu_vendor() )
> >> + panic("CPU vendor not compiled-in: %s",
> >> + x86_cpuid_vendor_to_str(c->x86_vendor));
> >
> > I think you want to print both the current compiled in support plus
> > the host vendor as part of the panic message.
>
> The mask of supported vendors, you mean? That could be helpful.
Yes, print both c->x86_vendor and cpu_vendor() as part of the error
message.
> >
> >> +
> >> + switch (cpu_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;
> >> diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c
> >> index 77a3a8742a..ec558bcbdb 100644
> >> --- a/xen/arch/x86/guest/xen/xen.c
> >> +++ b/xen/arch/x86/guest/xen/xen.c
> >> @@ -57,6 +57,10 @@ void asmlinkage __init early_hypercall_setup(void)
> >> cpuid(0, &eax, &ebx, &ecx, &edx);
> >>
> >> boot_cpu_data.x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
> >> +
> >> + if ( cpu_vendor() != boot_cpu_data.x86_vendor )
> >> + panic("CPU vendor not compiled-in: %s",
> >> + x86_cpuid_vendor_to_str(boot_cpu_data.x86_vendor));
> >
> > Is this going to be useful? I fear the panic here might happen even
> > before the console is setup, so a user won't get any output from Xen
> > at all.
>
> It is true that early_cpu_init() is invoked immediately after serial is
> set up, so any other vendor check ends up being fairly useless.
>
> OTOH, thinking about it may stand to reason to have:
>
> 1. A very early panic in assembly, like that of missing nx.
> 2. Have this early hypercall setup nonsense present ONLY when INTEL && AMD
> are both enabled. It really makes no sense to probe when you have
> explilcitly compiled for a single vendor.
If you don't probe at all, then yes, you would need an assembly-like
message. However given the hypercall page initialization is likely to
be used mostly in pv-shim mode, not even the assembly message would
get out I'm afraid, as the code to print such message only outputs to
the VGA and the serial, but not to the Xen PV console provided in shim
mode. pv-shim runs inside of a PVH container, that doesn't have
emulated serial or VGA.
You could implement early PV console support to print such message,
but it seems simpler to me to defer the panic until early_cpu_init()
for the sake of simplicity.
>
> >
> > Would it be fine to allow such mismatch in the hypercall setup, just
> > for the sake of getting the console page setup so that
> > early_cpu_init() can print a proper error message?
> >
> > Allowing the vendor mismatch here won't require any extra code, it's
> > just the selection of the instruction to use to call into Xen when
> > running in guest/shim mode.
>
> It'd be fine, yes. Maybe with a comment noting we can use vm{m,}call whether
> or not cpu_vendor() == 0 because the instruction itself is supported by HW.
>
> OTOH, I could also fully drop the dynamic detection logic on AMDLIKE-only or
> INTELLIKE-only builds like I mentioned above.
That would still leave us in a difficult situation w.r.t printing
anything to warn the user in case of mismatch.
> I sort of like that second option, as it allows removing hypercall.S and
> hook it to the real hypercall machinery, that at that point can have the
> alternatives removed.
I think you possible need a way to cope with vendor mismatch in the
early hypercall setup, just enough so that you can get into
early_cpu_init() to panic and print an error message.
Thanks, Roger.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's
2026-02-06 16:15 ` [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's Alejandro Vallejo
2026-02-10 8:19 ` Roger Pau Monné
@ 2026-02-11 15:41 ` Jan Beulich
2026-02-11 17:41 ` Alejandro Vallejo
1 sibling, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-11 15:41 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> --- 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->x86_vendor != host->x86_vendor) ||
With the subsequent "unknown" discussion also in mind, I wonder: Should we
fail the request here when either side is "unknown"?
Jan
> + (guest->basic.max_leaf > host->basic.max_leaf) )
> FAIL_CPUID(0, NA);
>
> if ( guest->feat.max_subleaf > host->feat.max_subleaf )
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor
2026-02-06 16:15 ` [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor Alejandro Vallejo
2026-02-10 8:46 ` Roger Pau Monné
@ 2026-02-11 16:04 ` Jan Beulich
2026-02-11 17:35 ` Alejandro Vallejo
2026-02-12 10:52 ` Jan Beulich
2 siblings, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-11 16:04 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -328,7 +328,11 @@ void __init early_cpu_init(bool verbose)
> *(u32 *)&c->x86_vendor_id[4] = edx;
>
> c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
> - switch (c->x86_vendor) {
> + if ( c->x86_vendor != cpu_vendor() )
When we introduce new functions to Linux-style files, nowadays we take
the liberty and make them Xen-style right away. In entirely Linux-
style functions the style should remain consistently Linux'es, though.
> + panic("CPU vendor not compiled-in: %s",
> + x86_cpuid_vendor_to_str(c->x86_vendor));
This will be somewhat awkward when c->x86_vendor is UNKNOWN.
There'll also be a degenerate case, but I'll comment on that on the
earlier patch causing that.
> --- a/xen/arch/x86/include/asm/cpufeature.h
> +++ b/xen/arch/x86/include/asm/cpufeature.h
> @@ -14,6 +14,7 @@
> #include <asm/cpu-policy.h>
> #include <asm/cpuid.h>
> #include <xen/lib/x86/cpu-policy.h>
> +#include <asm/x86-vendors.h>
Nit: Can the asm/-s please all stay together?
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 02/12] x86: Add more granularity to the vendors in Kconfig
2026-02-06 16:15 ` [PATCH 02/12] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
2026-02-10 8:26 ` Roger Pau Monné
@ 2026-02-11 16:06 ` Jan Beulich
2026-02-11 17:51 ` Alejandro Vallejo
1 sibling, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-11 16:06 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/Kconfig.cpu
> +++ b/xen/arch/x86/Kconfig.cpu
> @@ -19,4 +19,47 @@ 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"
> + select 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"
> + select 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"
> + select 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_VENDOR
> + bool "Support unknown CPU vendors"
> + 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 selecting a single vendor with this
> + option cleared produces a much smaller build.
Shouldn't this option's prompt be suppressed when all other vendors were
selected off? Else the degenerate case can result that I mentioned in
reply to patch 03.
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 04/12] x86: Migrate MSR handler vendor checks to cpu_vendor()
2026-02-06 16:15 ` [PATCH 04/12] x86: Migrate MSR handler vendor checks to cpu_vendor() Alejandro Vallejo
@ 2026-02-11 16:15 ` Jan Beulich
0 siblings, 0 replies; 55+ messages in thread
From: Jan Beulich @ 2026-02-11 16:15 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> Many handlers are vendor-specific and are currently gated on runtime
> checks. If we migrate those to cpu_vendor() they will effectively
> cause the ellision of handling code for CPU vendors not compiled in.
This builds upon the still very new "cp->x86_vendor can't be different
from boot_cpu_data.vendor". This imo wants mentioning, not only for ...
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -157,8 +157,8 @@ 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 ( !(cpu_vendor() & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR |
> + X86_VENDOR_SHANGHAI)) )
... this kind of transformation, but even more so ...
> @@ -169,8 +169,7 @@ 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 ( !(cpu_vendor() & X86_VENDOR_INTEL) )
... when two checks are folded into a single one.
Yet as mentioned earlier - the UNKNOWN case still will want settling on
from an abstract perspective. (This isn't because there would be an issue
here, but to have a clear understanding where we're heading.)
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor
2026-02-11 16:04 ` Jan Beulich
@ 2026-02-11 17:35 ` Alejandro Vallejo
2026-02-11 17:57 ` Alejandro Vallejo
0 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-11 17:35 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Wed Feb 11, 2026 at 5:04 PM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -328,7 +328,11 @@ void __init early_cpu_init(bool verbose)
>> *(u32 *)&c->x86_vendor_id[4] = edx;
>>
>> c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
>> - switch (c->x86_vendor) {
>> + if ( c->x86_vendor != cpu_vendor() )
>
> When we introduce new functions to Linux-style files, nowadays we take
> the liberty and make them Xen-style right away. In entirely Linux-
> style functions the style should remain consistently Linux'es, though.
>
Seeing how I added the tabs I did intend to do that.
Muscle memory and all. Will correct on the next one.
>> + panic("CPU vendor not compiled-in: %s",
>> + x86_cpuid_vendor_to_str(c->x86_vendor));
>
> This will be somewhat awkward when c->x86_vendor is UNKNOWN.
Which only happens on virtualised or imaginary CPUs. In either case you know
what you're running on and the information is not particularly useful. The
important part is the "this hypervisor cannot run on your vendor. Enable it
there".
>
> There'll also be a degenerate case, but I'll comment on that on the
> earlier patch causing that.
>
>> --- a/xen/arch/x86/include/asm/cpufeature.h
>> +++ b/xen/arch/x86/include/asm/cpufeature.h
>> @@ -14,6 +14,7 @@
>> #include <asm/cpu-policy.h>
>> #include <asm/cpuid.h>
>> #include <xen/lib/x86/cpu-policy.h>
>> +#include <asm/x86-vendors.h>
>
> Nit: Can the asm/-s please all stay together?
Sure, I didn't notice the xen/ because of the lack of a blank line.
>
> Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's
2026-02-11 15:41 ` Jan Beulich
@ 2026-02-11 17:41 ` Alejandro Vallejo
2026-02-12 7:16 ` Jan Beulich
0 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-11 17:41 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Wed Feb 11, 2026 at 4:41 PM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> --- 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->x86_vendor != host->x86_vendor) ||
>
> With the subsequent "unknown" discussion also in mind, I wonder: Should we
> fail the request here when either side is "unknown"?
That'd preclude the creation of VMs on new vendors. At that point, might as
well drop support for unknown vendors altogether. I wouldn't mind that.
I was thinking of comparing the x86_vendor_id bytes instead, as I answered
to Roger. Then the invariant that only $VENDOR VMs run on $VENDOR hosts is
preserved even if we don't know about them.
Cheers,
Alejandro.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 02/12] x86: Add more granularity to the vendors in Kconfig
2026-02-11 16:06 ` Jan Beulich
@ 2026-02-11 17:51 ` Alejandro Vallejo
2026-02-12 7:24 ` Jan Beulich
0 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-11 17:51 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Wed Feb 11, 2026 at 5:06 PM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> --- a/xen/arch/x86/Kconfig.cpu
>> +++ b/xen/arch/x86/Kconfig.cpu
>> @@ -19,4 +19,47 @@ 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"
>> + select 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"
>> + select 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"
>> + select 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_VENDOR
>> + bool "Support unknown CPU vendors"
>> + 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 selecting a single vendor with this
>> + option cleared produces a much smaller build.
>
> Shouldn't this option's prompt be suppressed when all other vendors were
> selected off? Else the degenerate case can result that I mentioned in
> reply to patch 03.
>
> Jan
You mean having the option disappear when any explicit vendor is enabled?
I guess we could. It wouldn't be very obvious to the user this can be toggled,
but now again, if they are unticking vendors they would end up noticing.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor
2026-02-11 17:35 ` Alejandro Vallejo
@ 2026-02-11 17:57 ` Alejandro Vallejo
0 siblings, 0 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-11 17:57 UTC (permalink / raw)
To: Alejandro Vallejo, Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel,
Xen-devel
On Wed Feb 11, 2026 at 6:35 PM CET, Alejandro Vallejo wrote:
> On Wed Feb 11, 2026 at 5:04 PM CET, Jan Beulich wrote:
>> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>>> --- a/xen/arch/x86/cpu/common.c
>>> +++ b/xen/arch/x86/cpu/common.c
>>> @@ -328,7 +328,11 @@ void __init early_cpu_init(bool verbose)
>>> *(u32 *)&c->x86_vendor_id[4] = edx;
>>>
>>> c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
>>> - switch (c->x86_vendor) {
>>> + if ( c->x86_vendor != cpu_vendor() )
>>
>> When we introduce new functions to Linux-style files, nowadays we take
>> the liberty and make them Xen-style right away. In entirely Linux-
>> style functions the style should remain consistently Linux'es, though.
>>
>
> Seeing how I added the tabs I did intend to do that.
>
> Muscle memory and all. Will correct on the next one.
>
>>> + panic("CPU vendor not compiled-in: %s",
>>> + x86_cpuid_vendor_to_str(c->x86_vendor));
>>
>> This will be somewhat awkward when c->x86_vendor is UNKNOWN.
>
> Which only happens on virtualised or imaginary CPUs. In either case you know
> what you're running on and the information is not particularly useful. The
> important part is the "this hypervisor cannot run on your vendor. Enable it
> there".
Oh, wait. You meant when we run on an unknown CPU and the unknown path was
disabled? Yes, that's missing in the condition.
if ((c->x86_vendor != cpu_vendor()) ||
(!IS_ENABLED(CONFIG_UNKNOWN_CPU_VENDORS) && !c->x86_vendor))
A hunk got missed in refactors in the default case so that default_cpu got
dropped when the unknown vendor was missing. That was present originally in the
RFC.
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's
2026-02-11 17:41 ` Alejandro Vallejo
@ 2026-02-12 7:16 ` Jan Beulich
0 siblings, 0 replies; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 7:16 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 11.02.2026 18:41, Alejandro Vallejo wrote:
> On Wed Feb 11, 2026 at 4:41 PM CET, Jan Beulich wrote:
>> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>>> --- 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->x86_vendor != host->x86_vendor) ||
>>
>> With the subsequent "unknown" discussion also in mind, I wonder: Should we
>> fail the request here when either side is "unknown"?
>
> That'd preclude the creation of VMs on new vendors. At that point, might as
> well drop support for unknown vendors altogether. I wouldn't mind that.
>
> I was thinking of comparing the x86_vendor_id bytes instead, as I answered
> to Roger. Then the invariant that only $VENDOR VMs run on $VENDOR hosts is
> preserved even if we don't know about them.
This would be okay with me.
I wonder though if we aren't getting too restrictive here at the same
time (even with the above check): Hygon (Fam18) can likely run AMD Fam17
guests quite fine, for example. I wonder what Andrew's considerations
here are.
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 02/12] x86: Add more granularity to the vendors in Kconfig
2026-02-11 17:51 ` Alejandro Vallejo
@ 2026-02-12 7:24 ` Jan Beulich
0 siblings, 0 replies; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 7:24 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 11.02.2026 18:51, Alejandro Vallejo wrote:
> On Wed Feb 11, 2026 at 5:06 PM CET, Jan Beulich wrote:
>> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>>> --- a/xen/arch/x86/Kconfig.cpu
>>> +++ b/xen/arch/x86/Kconfig.cpu
>>> @@ -19,4 +19,47 @@ 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"
>>> + select 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"
>>> + select 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"
>>> + select 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_VENDOR
>>> + bool "Support unknown CPU vendors"
>>> + 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 selecting a single vendor with this
>>> + option cleared produces a much smaller build.
>>
>> Shouldn't this option's prompt be suppressed when all other vendors were
>> selected off? Else the degenerate case can result that I mentioned in
>> reply to patch 03.
>
> You mean having the option disappear when any explicit vendor is enabled?
No, I mean the prompt (not the option) to disappear when _no_ explicit
vendor is enabled. Because of the select-s above, that could effectively
be as simple as
config UNKNOWN_CPU_VENDOR
bool "Support unknown CPU vendors" if INTEL || AMD
default y
Otherwise, as indicated, the conditional around the panic() in patch 03
degenerates: The panic() would be skipped despite UNKNOWN_CPU_VENDOR=n,
as with X86_ENABLED_VENDORS being 0, cpu_vendor() can't tell
UNKNOWN_CPU_VENDOR=n from UNKNOWN_CPU_VENDOR=y.
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 05/12] x86: Migrate spec_ctrl vendor checks to cpu_vendor()
2026-02-06 16:15 ` [PATCH 05/12] x86: Migrate spec_ctrl " Alejandro Vallejo
@ 2026-02-12 10:49 ` Jan Beulich
2026-02-12 14:55 ` Alejandro Vallejo
0 siblings, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 10:49 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> @@ -738,11 +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 ( cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
> return true;
>
> - if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
> - boot_cpu_data.family != 6 )
> + if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.family != 6 )
> return false;
At the example of this (applies throughout this patch): With the panic() in
patch 03 the transformation looks correct. Without that panic(), or without
being explicitly aware of it, this gives the impression of explicitly doing
an unsafe thing: Even though by way of boot_cpu_data.vendor we know what
vendor's CPU we're on, we're acting as if we didn't know. I'm really
uncertain whether such changes are worth it with the mere goal of reducing
code size. Even beyond the concern raised, this feels like it might be
increasing the risk of introducing subtle bugs.
I wonder what Andrew and Roger think in this regard.
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor
2026-02-06 16:15 ` [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor Alejandro Vallejo
2026-02-10 8:46 ` Roger Pau Monné
2026-02-11 16:04 ` Jan Beulich
@ 2026-02-12 10:52 ` Jan Beulich
2026-02-12 14:36 ` Alejandro Vallejo
2 siblings, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 10:52 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> Introduces various optimisations that rely on constant folding, Value
> Range Propagation (VRP), and Dead Code Elimination (DCE) to aggressively
> eliminate code surrounding the uses of the function.
>
> * For single-vendor+no-unknown-vendor builds returns a compile-time
> constant.
> * For all other cases it ANDs the result with the mask of compiled
> vendors, with the effect of performing DCE in switch cases, removing
> dead conditionals, etc.
>
> It's difficult to reason about codegen in general in a project this big,
> but in this case the ANDed constant combines with the values typically
> checked against, folding into a comparison against zero. Thus, it's better
> for codegen to AND its result with the desired compared-against vendor,
> rather than using (in)equality operators. That way the comparison is
> always against zero.
>
> "cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)"
>
> turns into (cpu_vendor() & X86_VENDOR_AMD) in AMD-only builds (AND +
> cmp with zero). Whereas this...
>
> "cpu_vendor() == X86_VENDOR_AMD"
>
> forces cpu_vendor() to be ANDed and then compared to a non-zero value.
Coming back to this: How does the value compared against being zero or
non-zero matter here? As long as cpu_vendor() yields a compile-time
constant, the compiler should be able to leverage that for DCE? And
even if it's not a compile time constant, bits masked off in principle
allow the compiler to leverage that, too. It may of course be that
even up-to-date compilers fall short of doing so.
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/12] x86: Migrate switch vendor checks to cpu_vendor()
2026-02-06 16:15 ` [PATCH 06/12] x86: Migrate switch " Alejandro Vallejo
@ 2026-02-12 11:06 ` Jan Beulich
2026-02-12 15:06 ` Alejandro Vallejo
0 siblings, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 11:06 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
> +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
> @@ -133,7 +133,7 @@ static int __init cf_check cpufreq_driver_init(void)
>
> ret = -ENOENT;
>
> - switch ( boot_cpu_data.x86_vendor )
> + switch( cpu_vendor() )
Nit: Please avoid screwing up style.
> @@ -141,12 +141,10 @@ static int __init cf_check cpufreq_driver_init(void)
> 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;
This of course is a neat (side) effect.
> @@ -165,7 +163,6 @@ static int __init cf_check cpufreq_driver_init(void)
>
> case X86_VENDOR_AMD:
> case X86_VENDOR_HYGON:
> -#ifdef CONFIG_AMD
> for ( i = 0; i < cpufreq_xen_cnt; i++ )
> {
> switch ( cpufreq_xen_opts[i] )
> @@ -191,9 +188,6 @@ static int __init cf_check cpufreq_driver_init(void)
> if ( !ret || ret == -EBUSY )
> break;
> }
> -#else
> - ret = -ENODEV;
> -#endif /* CONFIG_AMD */
> break;
>
> default:
There's a change to the function's return value, though: When reaching default:,
-ENOENT will result, when previously -ENODEV would have been returned for
compiled-out cases. It may well be that there is a dependency somewhere on the
particular return value - did you thoroughly check that?
Of course this may well apply elsewhere as well; I did not go through and check
every of the switch()es you alter.
> --- a/xen/arch/x86/cpu/mcheck/mce.h
> +++ b/xen/arch/x86/cpu/mcheck/mce.h
> @@ -137,7 +137,7 @@ 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) {
> + switch (cpu_vendor()) {
Would be nice if style was updated while touching such.
> --- a/xen/arch/x86/cpu/vpmu.c
> +++ b/xen/arch/x86/cpu/vpmu.c
> @@ -815,7 +815,7 @@ static struct notifier_block cpu_nfb = {
>
> static int __init cf_check vpmu_init(void)
> {
> - int vendor = current_cpu_data.x86_vendor;
> + int vendor = cpu_vendor();
It is only seeing the plain int here that I notice that cpu_vendor() returns
uint8_t. I don't think that's necessary, and hence as per ./CODING_STYLE it
should rather be unsigned int. Which is then waht would want using here as
well.
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -319,7 +319,7 @@ void domain_cpu_policy_changed(struct domain *d)
> if ( cpu_has_htt )
> edx |= cpufeat_mask(X86_FEATURE_HTT);
>
> - switch ( boot_cpu_data.x86_vendor )
> + switch( cpu_vendor() )
> {
> case X86_VENDOR_INTEL:
> /*
> @@ -427,7 +427,7 @@ void domain_cpu_policy_changed(struct domain *d)
> if ( !(p->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
> edx &= ~CPUID_COMMON_1D_FEATURES;
>
> - switch ( boot_cpu_data.x86_vendor )
> + switch( cpu_vendor() )
As they recur, I wonder where these bogus style adjustments are coming from.
It's not like ...
> --- a/xen/arch/x86/guest/xen/xen.c
> +++ b/xen/arch/x86/guest/xen/xen.c
> @@ -63,7 +63,7 @@ void asmlinkage __init early_hypercall_setup(void)
> x86_cpuid_vendor_to_str(boot_cpu_data.x86_vendor));
> }
>
> - switch ( boot_cpu_data.x86_vendor )
> + switch ( cpu_vendor() )
... you would have used a bad sed pattern globally, as here style remains
intact. Further down it breaks again.
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 07/12] x86: Have x86_emulate/ implement the single-vendor optimisation
2026-02-06 16:15 ` [PATCH 07/12] x86: Have x86_emulate/ implement the single-vendor optimisation Alejandro Vallejo
@ 2026-02-12 11:26 ` Jan Beulich
2026-02-12 15:29 ` Alejandro Vallejo
0 siblings, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 11:26 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> Open code the vendor check through the policy as a one-off. The emulator
> embeds amd_like() in macros and is called in MANY places. Using a
> local variable (cp->x86_vendor) makes it a lot smaller (300-400 bytes
> smaller). So treat this as the exception it is and let it use the policy
> rather than boot_cpu_data.
As elsewhere you mainly discuss benefits for the single-vendor case, is the
above about the opposite situation? Else why would codegen suffer this much
here?
Using cp also is preferable for test and fuzzing harnesses, which don't
even know boot_cpu_data.
> @@ -30,8 +31,15 @@ void BUG(void);
> # define X86EMUL_NO_SIMD
> # endif
>
> +/* intentionally avoid cpu_vendor(), as it produces much worse codegen */
Nit (style): Capital letter wanted at the start.
> +# define x86emul_cpu(cp) ((X86_ENABLED_VENDORS == \
> + ISOLATE_LSB(X86_ENABLED_VENDORS)) \
> + ? X86_ENABLED_VENDORS \
> + : ((cp)->x86_vendor & X86_ENABLED_VENDORS))
Nit: Indentation. The ? and : want to align with the controlling expression.
Further, is this a good name, without "vendor" in it?
And then I'm of two minds here as to the use of the macro parameter: On one
hand we can be pretty certain what is passed in won't have side effects.
Otoh in a hypothetical odd case (seeing that this lives in a header file,
not local to an isolated piece of code) where there would be one, the
argument being evaluated unreliably could cause an unpleasant surprise.
The more ...
> #else /* !__XEN__ */
> # include "x86-emulate.h"
> +# define x86emul_cpu(cp) ((cp)->x86_vendor)
... that the same wouldn't be observable in the fuzzing or test harnesses.
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 07/12] x86: Migrate x86_emulate/ to use cpu_vendor()
2026-02-06 16:15 ` [PATCH 07/12] x86: Migrate x86_emulate/ to use cpu_vendor() Alejandro Vallejo
@ 2026-02-12 11:31 ` Jan Beulich
2026-02-12 15:30 ` Alejandro Vallejo
0 siblings, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 11:31 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> ... but keep the current behaviour of using the policy vendor when
> compiled for userspace, where cross-vendor configurations are expected.
>
> Not a functional change.
>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
Hmm, ...
> --- 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/cpufeature.h>
> # include <asm/endbr.h>
> # include <asm/msr-index.h>
> # include <asm/stubs.h>
> @@ -30,8 +31,11 @@ void BUG(void);
> # define X86EMUL_NO_SIMD
> # endif
>
> +# define x86emul_cpu(cp) ({ (void)(cp); cpu_vendor(); })
> +
> #else /* !__XEN__ */
> # include "x86-emulate.h"
> +# define x86emul_cpu(cp) ((cp)->x86_vendor)
> #endif
... this looked familiar, yet different at the same time. It was only after
some eyebrow raising that I noticed that patch 07/12 appears twice in the
series, with different titles.
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 08/12] x86/acpi: Migrate vendor checks to cpu_vendor()
2026-02-06 16:15 ` [PATCH 08/12] x86/acpi: Migrate vendor checks to cpu_vendor() Alejandro Vallejo
@ 2026-02-12 11:52 ` Jan Beulich
2026-02-12 15:34 ` Alejandro Vallejo
0 siblings, 1 reply; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 11:52 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> --- 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 ( !(cpu_vendor() & X86_VENDOR_INTEL) || c->x86 != 6 )
> return;
>
> switch ( c->x86_model )
> @@ -915,8 +915,7 @@ 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 ( cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON) &&
While we certainly make that assumption, shouldn't you add explicit checks
that APs' vendors match the BSP's, in order to be able to also replace
current_cpu_data uses? Or do we have such a check, and I'm merely overlooking
it?
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 10/12] x86/mcheck: Migrate vendor checks to use cpu_vendor()
2026-02-06 16:15 ` [PATCH 10/12] x86/mcheck: Migrate vendor checks to use cpu_vendor() Alejandro Vallejo
@ 2026-02-12 12:02 ` Jan Beulich
2026-02-12 12:21 ` Jan Beulich
2026-02-12 15:46 ` Alejandro Vallejo
0 siblings, 2 replies; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 12:02 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> @@ -862,7 +861,7 @@ static void x86_mcinfo_apei_save(
> memset(&m, 0, sizeof(struct mce));
>
> m.cpu = mc_global->mc_coreid;
> - m.cpuvendor = boot_cpu_data.x86_vendor;
> + m.cpuvendor = cpu_vendor();
> m.cpuid = cpuid_eax(1);
> m.socketid = mc_global->mc_socketid;
> m.apicid = mc_global->mc_apicid;
> @@ -964,7 +963,7 @@ static void cf_check __maybe_unused do_mc_get_cpu_info(void *v)
> &xcp->mc_ncores_active, &xcp->mc_nthreads);
> xcp->mc_cpuid_level = c->cpuid_level;
> xcp->mc_family = c->x86;
> - xcp->mc_vendor = c->x86_vendor;
> + xcp->mc_vendor = cpu_vendor();
> xcp->mc_model = c->x86_model;
> xcp->mc_step = c->x86_mask;
> xcp->mc_cache_size = c->x86_cache_size;
What we do internally is one thing. But what we report to the outside shouldn't
be inaccurate, imo.
> @@ -1122,7 +1121,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 ( (cpu_vendor() & X86_VENDOR_AMD) )
Nit: No need for the extra parentheses here.
> --- 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(!(cpu_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 ( !(cpu_vendor() & X86_VENDOR_HYGON) )
> quirkflag = mcequirk_lookup_amd_quirkdata(c);
I wonder if this wasn't better switched to the positive for (checking for AMD).
This would the render the BUG_ON() above useless.
> --- 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 ( (cpu_vendor() & X86_VENDOR_INTEL) &&
> + c->x86 == 6 && c->x86_model >= 0xe )
Nit: Why the re-wrapping of lines?
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 10/12] x86/mcheck: Migrate vendor checks to use cpu_vendor()
2026-02-12 12:02 ` Jan Beulich
@ 2026-02-12 12:21 ` Jan Beulich
2026-02-12 15:46 ` Alejandro Vallejo
1 sibling, 0 replies; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 12:21 UTC (permalink / raw)
To: Andrew Cooper, Roger Pau Monné
Cc: Jason Andryuk, xen-devel, Alejandro Vallejo
On 12.02.2026 13:02, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> @@ -862,7 +861,7 @@ static void x86_mcinfo_apei_save(
>> memset(&m, 0, sizeof(struct mce));
>>
>> m.cpu = mc_global->mc_coreid;
>> - m.cpuvendor = boot_cpu_data.x86_vendor;
>> + m.cpuvendor = cpu_vendor();
>> m.cpuid = cpuid_eax(1);
>> m.socketid = mc_global->mc_socketid;
>> m.apicid = mc_global->mc_apicid;
>> @@ -964,7 +963,7 @@ static void cf_check __maybe_unused do_mc_get_cpu_info(void *v)
>> &xcp->mc_ncores_active, &xcp->mc_nthreads);
>> xcp->mc_cpuid_level = c->cpuid_level;
>> xcp->mc_family = c->x86;
>> - xcp->mc_vendor = c->x86_vendor;
>> + xcp->mc_vendor = cpu_vendor();
>> xcp->mc_model = c->x86_model;
>> xcp->mc_step = c->x86_mask;
>> xcp->mc_cache_size = c->x86_cache_size;
>
> What we do internally is one thing. But what we report to the outside shouldn't
> be inaccurate, imo.
There may be an underlying issue here: For the former case, struct mce is an
entirely private definition, having in particular:
uint8_t cpuvendor; /* cpu vendor as encoded in system.h */
which pretty clearly was inherited from Linux, and hence was broken when we
disconnected our X86_VENDOR_* values from Linux'es (in 4.9).
For the latter case, xen_mc_logical_cpu_t is part of the Xen ABI, and hence
it got screwed not only as per above, but another time when we converted
X86_VENDOR_* values from enumeration-like to bitfield-like (in 4.13).
Question of course is - if there were any consumers, they may have followed
our breaking of things. So it's hard to tell whether fixing might do more
bad than good. (I'll make a fix nevertheless, just in case).
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 11/12] x86/cpu: Migrate vendor checks to use cpu_vendor()
2026-02-06 16:15 ` [PATCH 11/12] x86/cpu: " Alejandro Vallejo
@ 2026-02-12 13:17 ` Jan Beulich
0 siblings, 0 replies; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 13:17 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> --- 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) &&
> + (!(cpu_vendor() & X86_VENDOR_SHANGHAI))) {
Nit: Again, no need for the extra parentheses (here: around the !()). Nor do
I see a need to alter the first line at all.
> --- a/xen/arch/x86/cpu/mtrr/generic.c
> +++ b/xen/arch/x86/cpu/mtrr/generic.c
> @@ -218,9 +218,8 @@ 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 &&
> - boot_cpu_data.x86 >= 0xf) ||
> - boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
> + if (((cpu_vendor() & X86_VENDOR_AMD) && boot_cpu_data.x86 >= 0xf) ||
> + (cpu_vendor() & X86_VENDOR_HYGON)) {
Perhaps easier as
if ((cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) &&
boot_cpu_data.x86 >= 0xf) {
, seeing that there's no Hygon hardware lower than Fam18?
> --- a/xen/arch/x86/cpu/vpmu.c
> +++ b/xen/arch/x86/cpu/vpmu.c
> @@ -398,7 +398,6 @@ int vpmu_load(struct vcpu *v, bool from_guest)
> static int vpmu_arch_initialise(struct vcpu *v)
> {
> struct vpmu_struct *vpmu = vcpu_vpmu(v);
> - uint8_t vendor = current_cpu_data.x86_vendor;
> int ret;
>
> BUILD_BUG_ON(sizeof(struct xen_pmu_intel_ctxt) > XENPMU_CTXT_PAD_SZ);
> @@ -418,7 +417,7 @@ static int vpmu_arch_initialise(struct vcpu *v)
> if ( vpmu_mode != XENPMU_MODE_OFF )
> {
> printk(XENLOG_G_WARNING "VPMU: Unknown CPU vendor %d. "
> - "Disabling VPMU\n", vendor);
> + "Disabling VPMU\n", cpu_vendor());
Why would you want to make the log message less useful?
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 12/12] x86: Migrate every remaining raw vendor check to cpu_vendor()
2026-02-06 16:15 ` [PATCH 12/12] x86: Migrate every remaining raw vendor check to cpu_vendor() Alejandro Vallejo
@ 2026-02-12 13:29 ` Jan Beulich
0 siblings, 0 replies; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 13:29 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 06.02.2026 17:15, Alejandro Vallejo wrote:
> @@ -424,7 +423,7 @@ 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 ( cpu_vendor() & ~(X86_VENDOR_AMD | X86_VENDOR_HYGON) )
> edx &= ~CPUID_COMMON_1D_FEATURES;
The usual transformation here would have been to simply replace p->x86_vendor
by cpu_vendor(). Such an unusual pattern, if indeed necessary, imo wants
explaining in the description.
As this is entirely unexpected, I now wonder whether I overlooked any other
unmentioned unusual transformations.
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -426,7 +426,7 @@ 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 = cpu_vendor() == X86_VENDOR_INTEL;
Unexpectedly retaining == here?
> --- a/xen/arch/x86/i8259.c
> +++ b/xen/arch/x86/i8259.c
> @@ -419,9 +419,8 @@ void __init init_IRQ(void)
> * the interrupt.
> */
> cpumask_copy(desc->arch.cpu_mask,
> - (boot_cpu_data.x86_vendor &
> - (X86_VENDOR_AMD | X86_VENDOR_HYGON) ? &cpumask_all
> - : cpumask_of(cpu)));
> + ((cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON))
> + ? &cpumask_all : cpumask_of(cpu)));
Nit: Indentation again, and apparently also excess parentheses.
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -2011,8 +2011,7 @@ 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))) &&
> + cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) &&
Whereas here parentheses look to be missing (to isolate from the || not visible
in context).
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1406,8 +1406,7 @@ void asmlinkage __init noreturn __start_xen(void)
> * CPUs with this addressed enumerate CET-SSS to indicate that
> * supervisor shadow stacks are now safe to use.
> */
> - bool cpu_has_bug_shstk_fracture =
> - boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
> + bool cpu_has_bug_shstk_fracture = (cpu_vendor() & X86_VENDOR_INTEL) &&
> !boot_cpu_has(X86_FEATURE_CET_SSS);
I think retaining the prior wrapping would be better here. When done like you
do, really it should become
bool cpu_has_bug_shstk_fracture = (cpu_vendor() & X86_VENDOR_INTEL) &&
!boot_cpu_has(X86_FEATURE_CET_SSS);
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor
2026-02-12 10:52 ` Jan Beulich
@ 2026-02-12 14:36 ` Alejandro Vallejo
0 siblings, 0 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-12 14:36 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Thu Feb 12, 2026 at 11:52 AM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> Introduces various optimisations that rely on constant folding, Value
>> Range Propagation (VRP), and Dead Code Elimination (DCE) to aggressively
>> eliminate code surrounding the uses of the function.
>>
>> * For single-vendor+no-unknown-vendor builds returns a compile-time
>> constant.
>> * For all other cases it ANDs the result with the mask of compiled
>> vendors, with the effect of performing DCE in switch cases, removing
>> dead conditionals, etc.
>>
>> It's difficult to reason about codegen in general in a project this big,
>> but in this case the ANDed constant combines with the values typically
>> checked against, folding into a comparison against zero. Thus, it's better
>> for codegen to AND its result with the desired compared-against vendor,
>> rather than using (in)equality operators. That way the comparison is
>> always against zero.
>>
>> "cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON)"
>>
>> turns into (cpu_vendor() & X86_VENDOR_AMD) in AMD-only builds (AND +
>> cmp with zero). Whereas this...
>>
>> "cpu_vendor() == X86_VENDOR_AMD"
>>
>> forces cpu_vendor() to be ANDed and then compared to a non-zero value.
>
> Coming back to this: How does the value compared against being zero or
> non-zero matter here? As long as cpu_vendor() yields a compile-time
> constant, the compiler should be able to leverage that for DCE? And
Yes, for true single-vendor cases it doesn't matter. It matters on multivendor
cases where some vendors are off and cpu_vendor() is not a constant.
> even if it's not a compile time constant, bits masked off in principle
> allow the compiler to leverage that, too. It may of course be that
> even up-to-date compilers fall short of doing so.
There might be some of that, but there's also a non-avoidable codegen hurdle
unless you can tell the compiler your variable is a power of 2 or 0 in
multivendor cases.
cpu_vendor() == X86_VENDOR_AMD, which expands to
(boot_cpu_data.vendor & X86_ENABLED_VENDORS) == X86_VENDOR_AMD, which expands to
(boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_INTEL) == X86_VENDOR_AMD
which produces a lot worse codegen, because now the compiler must AND the
variable with the AMD|INTEL mask, and then compare to the AMD mask, whereas
having & instead of == means the compiler can simply do a comparison with zero
and call it a day (due to the masks being folded together).
I tried creating unreachable paths in cpu_vendor() to assist the VRP pass
in noticing the variable invariant, but it just doesn't. It doesn't seem to
be sufficiently aggressive in range tracking. Bummer, because VRP is a very
unobstrusive technique for DCE that could be great if we could reliably teach it
invariants we know are held by certain variables as part of their accessors..
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 05/12] x86: Migrate spec_ctrl vendor checks to cpu_vendor()
2026-02-12 10:49 ` Jan Beulich
@ 2026-02-12 14:55 ` Alejandro Vallejo
0 siblings, 0 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-12 14:55 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Thu Feb 12, 2026 at 11:49 AM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> @@ -738,11 +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 ( cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
>> return true;
>>
>> - if ( boot_cpu_data.vendor != X86_VENDOR_INTEL ||
>> - boot_cpu_data.family != 6 )
>> + if ( !(cpu_vendor() & X86_VENDOR_INTEL) || boot_cpu_data.family != 6 )
>> return false;
>
> At the example of this (applies throughout this patch): With the panic() in
> patch 03 the transformation looks correct. Without that panic(), or without
> being explicitly aware of it, this gives the impression of explicitly doing
> an unsafe thing:
These patches wouldn't be functional changes without the X86_ENABLED_VENDORS
mask at cpu_vendor() or the single-vendor optimisation. I could split it that
way. Introduce the cpu_vendor() macro early. Transform the entire tree, and then
apply the optimisations.
> being explicitly aware of it, this gives the impression of explicitly doing
> an unsafe thing:
Without the panic, it'd indeed be (intentionally) doing an unsafe thing.
Removing the panic can't be safe, it's not just a matter of features missing.
It'd imply printing a banner saying the current configuration is unsupported.
> Even though by way of boot_cpu_data.vendor we know what
> vendor's CPU we're on, we're acting as if we didn't know.
Not with the panic in place, which is partially why I put it there.
> I'm really
> uncertain whether such changes are worth it with the mere goal of reducing
> code size.
It is unreachable code, and in a safety context every line of unreachable code
must be either removed or justified. And justifications cost time, effort and
are difficult to maintain.
> Even beyond the concern raised, this feels like it might be
> increasing the risk of introducing subtle bugs.
I would've agreed with you with x86_vendor_is(), which is why for v1 I took a
step back to make cpu_vendor(). That macro was complex. Too complex and it took
many trial and errors to fine tune it. cpu_vendor() is comparatively trivial and
relies on the compiler doing the heavy lifting. In the single-vendor case it's
a constant and it's hopefully uncontroversially fine to remove unreachable code
then.
In the multivendor case, the complexity amounts to a mask of available vendors.
I don't think there's an inherent danger in removing unreachable code, so long
as we can prove at boot time the reachability preconditions can't be met.
>
> I wonder what Andrew and Roger think in this regard.
+1
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 06/12] x86: Migrate switch vendor checks to cpu_vendor()
2026-02-12 11:06 ` Jan Beulich
@ 2026-02-12 15:06 ` Alejandro Vallejo
0 siblings, 0 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-12 15:06 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Thu Feb 12, 2026 at 12:06 PM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
>> +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
>> @@ -133,7 +133,7 @@ static int __init cf_check cpufreq_driver_init(void)
>>
>> ret = -ENOENT;
>>
>> - switch ( boot_cpu_data.x86_vendor )
>> + switch( cpu_vendor() )
>
> Nit: Please avoid screwing up style.
bah, yes.
>
>> @@ -141,12 +141,10 @@ static int __init cf_check cpufreq_driver_init(void)
>> 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;
>
> This of course is a neat (side) effect.
>
>> @@ -165,7 +163,6 @@ static int __init cf_check cpufreq_driver_init(void)
>>
>> case X86_VENDOR_AMD:
>> case X86_VENDOR_HYGON:
>> -#ifdef CONFIG_AMD
>> for ( i = 0; i < cpufreq_xen_cnt; i++ )
>> {
>> switch ( cpufreq_xen_opts[i] )
>> @@ -191,9 +188,6 @@ static int __init cf_check cpufreq_driver_init(void)
>> if ( !ret || ret == -EBUSY )
>> break;
>> }
>> -#else
>> - ret = -ENODEV;
>> -#endif /* CONFIG_AMD */
>> break;
>>
>> default:
>
> There's a change to the function's return value, though: When reaching default:,
> -ENOENT will result, when previously -ENODEV would have been returned for
> compiled-out cases. It may well be that there is a dependency somewhere on the
> particular return value - did you thoroughly check that?
It's a presmp_initcall. The specific ret value is inconsequential.
>
> Of course this may well apply elsewhere as well; I did not go through and check
> every of the switch()es you alter.
It may. I did check, but that doesn't mean I didn't miss any.
>
>> --- a/xen/arch/x86/cpu/mcheck/mce.h
>> +++ b/xen/arch/x86/cpu/mcheck/mce.h
>> @@ -137,7 +137,7 @@ 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) {
>> + switch (cpu_vendor()) {
>
> Would be nice if style was updated while touching such.
sure.
>
>> --- a/xen/arch/x86/cpu/vpmu.c
>> +++ b/xen/arch/x86/cpu/vpmu.c
>> @@ -815,7 +815,7 @@ static struct notifier_block cpu_nfb = {
>>
>> static int __init cf_check vpmu_init(void)
>> {
>> - int vendor = current_cpu_data.x86_vendor;
>> + int vendor = cpu_vendor();
>
> It is only seeing the plain int here that I notice that cpu_vendor() returns
> uint8_t. I don't think that's necessary, and hence as per ./CODING_STYLE it
> should rather be unsigned int. Which is then waht would want using here as
> well.
Hmm, I need to check whether it affects codegen. Probably not seeing how its
all inline. Will do unless it has terrible side effects.
>
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -319,7 +319,7 @@ void domain_cpu_policy_changed(struct domain *d)
>> if ( cpu_has_htt )
>> edx |= cpufeat_mask(X86_FEATURE_HTT);
>>
>> - switch ( boot_cpu_data.x86_vendor )
>> + switch( cpu_vendor() )
>> {
>> case X86_VENDOR_INTEL:
>> /*
>> @@ -427,7 +427,7 @@ void domain_cpu_policy_changed(struct domain *d)
>> if ( !(p->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
>> edx &= ~CPUID_COMMON_1D_FEATURES;
>>
>> - switch ( boot_cpu_data.x86_vendor )
>> + switch( cpu_vendor() )
> As they recur, I wonder where these bogus style adjustments are coming from.
> It's not like ...
>
>> --- a/xen/arch/x86/guest/xen/xen.c
>> +++ b/xen/arch/x86/guest/xen/xen.c
>> @@ -63,7 +63,7 @@ void asmlinkage __init early_hypercall_setup(void)
>> x86_cpuid_vendor_to_str(boot_cpu_data.x86_vendor));
>> }
>>
>> - switch ( boot_cpu_data.x86_vendor )
>> + switch ( cpu_vendor() )
>
> ... you would have used a bad sed pattern globally, as here style remains
> intact. Further down it breaks again.
This patch used to be several. One of the primordial commits seems to suffer
from this. Will fix globally. Thanks.
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 07/12] x86: Have x86_emulate/ implement the single-vendor optimisation
2026-02-12 11:26 ` Jan Beulich
@ 2026-02-12 15:29 ` Alejandro Vallejo
0 siblings, 0 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-12 15:29 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Thu Feb 12, 2026 at 12:26 PM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> Open code the vendor check through the policy as a one-off. The emulator
>> embeds amd_like() in macros and is called in MANY places. Using a
>> local variable (cp->x86_vendor) makes it a lot smaller (300-400 bytes
>> smaller). So treat this as the exception it is and let it use the policy
>> rather than boot_cpu_data.
>
> As elsewhere you mainly discuss benefits for the single-vendor case, is the
> above about the opposite situation? Else why would codegen suffer this much
> here?
In single-vendor it doesn't matter. A constant is a constant. It matters in
multivendor. And it matters for 2 reasons.
1. The x86_emulate() function is HUGE.
2. The x86_emulate() function has LOTS of amd_like() invocations.
When amd_like() uses the policy it has an advantage over using the global.
Namely, the policy is already cached in a register, so codegen simply has to
pull the vendor from an offset into a register. When we go for a global variable
we need to reach out and pull the variable from its 64bit address (because we
compile with model=large). It normally evens out with the codegen reductions
cpu_vendor() encourages you to have, but in here it just doesn't. There's way
too many accesses to global state and .text suffers.
The fix for this is caching the vendor somewhere else. A "bool amd_like" in a
local variable would shrink code substantially (by having rsp-relative access to
the solution of the question amd_like() asks, and by avoiding the masking). This
optimisation is worth doing with or without this patch in place.
Alas, I didn't test that, because this series was sufficiently complicated
as-is.
>
> Using cp also is preferable for test and fuzzing harnesses, which don't
> even know boot_cpu_data.
They don't now, which is why I made the x86emul_cpu() macro. New subsystems
added to a userlevel testing ground could simply have a boot_cpu_data with the
desired policy as part of their harness' global state.
>
>> @@ -30,8 +31,15 @@ void BUG(void);
>> # define X86EMUL_NO_SIMD
>> # endif
>>
>> +/* intentionally avoid cpu_vendor(), as it produces much worse codegen */
>
> Nit (style): Capital letter wanted at the start.
>
>> +# define x86emul_cpu(cp) ((X86_ENABLED_VENDORS == \
>> + ISOLATE_LSB(X86_ENABLED_VENDORS)) \
>> + ? X86_ENABLED_VENDORS \
>> + : ((cp)->x86_vendor & X86_ENABLED_VENDORS))
>
> Nit: Indentation. The ? and : want to align with the controlling expression.
sure.
>
> Further, is this a good name, without "vendor" in it?
x86emul_cpu_vendor() is fine with me too.
>
> And then I'm of two minds here as to the use of the macro parameter: On one
> hand we can be pretty certain what is passed in won't have side effects.
> Otoh in a hypothetical odd case (seeing that this lives in a header file,
> not local to an isolated piece of code) where there would be one, the
> argument being evaluated unreliably could cause an unpleasant surprise.
> The more ...
>
>> #else /* !__XEN__ */
>> # include "x86-emulate.h"
>> +# define x86emul_cpu(cp) ((cp)->x86_vendor)
>
> ... that the same wouldn't be observable in the fuzzing or test harnesses.
I can turn this into a static inline to avoid such worries.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 07/12] x86: Migrate x86_emulate/ to use cpu_vendor()
2026-02-12 11:31 ` Jan Beulich
@ 2026-02-12 15:30 ` Alejandro Vallejo
0 siblings, 0 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-12 15:30 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Thu Feb 12, 2026 at 12:31 PM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> ... but keep the current behaviour of using the policy vendor when
>> compiled for userspace, where cross-vendor configurations are expected.
>>
>> Not a functional change.
>>
>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>
> Hmm, ...
>
>> --- 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/cpufeature.h>
>> # include <asm/endbr.h>
>> # include <asm/msr-index.h>
>> # include <asm/stubs.h>
>> @@ -30,8 +31,11 @@ void BUG(void);
>> # define X86EMUL_NO_SIMD
>> # endif
>>
>> +# define x86emul_cpu(cp) ({ (void)(cp); cpu_vendor(); })
>> +
>> #else /* !__XEN__ */
>> # include "x86-emulate.h"
>> +# define x86emul_cpu(cp) ((cp)->x86_vendor)
>> #endif
>
> ... this looked familiar, yet different at the same time. It was only after
> some eyebrow raising that I noticed that patch 07/12 appears twice in the
> series, with different titles.
>
> Jan
Arg. Leftover patch in my patches folder. Please, ignore this patch, and
apologies.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 08/12] x86/acpi: Migrate vendor checks to cpu_vendor()
2026-02-12 11:52 ` Jan Beulich
@ 2026-02-12 15:34 ` Alejandro Vallejo
2026-02-12 15:52 ` Jan Beulich
0 siblings, 1 reply; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-12 15:34 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Thu Feb 12, 2026 at 12:52 PM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> --- 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 ( !(cpu_vendor() & X86_VENDOR_INTEL) || c->x86 != 6 )
>> return;
>>
>> switch ( c->x86_model )
>> @@ -915,8 +915,7 @@ 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 ( cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON) &&
>
> While we certainly make that assumption, shouldn't you add explicit checks
> that APs' vendors match the BSP's, in order to be able to also replace
> current_cpu_data uses? Or do we have such a check, and I'm merely overlooking
> it?
>
> Jan
In generic_identify()
c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
if (boot_cpu_data.x86_vendor != c->x86_vendor)
printk(XENLOG_ERR "CPU%u vendor %u mismatch against BSP %u\n",
smp_processor_id(), c->x86_vendor,
boot_cpu_data.x86_vendor);
But I'm not sure why this is not a panic() (I thought it was). Such check
triggering can only mean a Hypervisor bug and a security hole.
Do you see a problem with s/printk/panic/ here?
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 10/12] x86/mcheck: Migrate vendor checks to use cpu_vendor()
2026-02-12 12:02 ` Jan Beulich
2026-02-12 12:21 ` Jan Beulich
@ 2026-02-12 15:46 ` Alejandro Vallejo
1 sibling, 0 replies; 55+ messages in thread
From: Alejandro Vallejo @ 2026-02-12 15:46 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On Thu Feb 12, 2026 at 1:02 PM CET, Jan Beulich wrote:
> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>> @@ -862,7 +861,7 @@ static void x86_mcinfo_apei_save(
>> memset(&m, 0, sizeof(struct mce));
>>
>> m.cpu = mc_global->mc_coreid;
>> - m.cpuvendor = boot_cpu_data.x86_vendor;
>> + m.cpuvendor = cpu_vendor();
>> m.cpuid = cpuid_eax(1);
>> m.socketid = mc_global->mc_socketid;
>> m.apicid = mc_global->mc_apicid;
>> @@ -964,7 +963,7 @@ static void cf_check __maybe_unused do_mc_get_cpu_info(void *v)
>> &xcp->mc_ncores_active, &xcp->mc_nthreads);
>> xcp->mc_cpuid_level = c->cpuid_level;
>> xcp->mc_family = c->x86;
>> - xcp->mc_vendor = c->x86_vendor;
>> + xcp->mc_vendor = cpu_vendor();
>> xcp->mc_model = c->x86_model;
>> xcp->mc_step = c->x86_mask;
>> xcp->mc_cache_size = c->x86_cache_size;
>
> What we do internally is one thing. But what we report to the outside shouldn't
> be inaccurate, imo.
I agree. There's also zero benefit.
>
>> @@ -1122,7 +1121,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 ( (cpu_vendor() & X86_VENDOR_AMD) )
>
> Nit: No need for the extra parentheses here.
ack.
>
>> --- 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(!(cpu_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 ( !(cpu_vendor() & X86_VENDOR_HYGON) )
>> quirkflag = mcequirk_lookup_amd_quirkdata(c);
>
> I wonder if this wasn't better switched to the positive for (checking for AMD).
> This would the render the BUG_ON() above useless.
>
>> --- 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 ( (cpu_vendor() & X86_VENDOR_INTEL) &&
>> + c->x86 == 6 && c->x86_model >= 0xe )
>
> Nit: Why the re-wrapping of lines?
No good reason. I find it marginally easier to read. I can restore it's previous
state.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH 08/12] x86/acpi: Migrate vendor checks to cpu_vendor()
2026-02-12 15:34 ` Alejandro Vallejo
@ 2026-02-12 15:52 ` Jan Beulich
0 siblings, 0 replies; 55+ messages in thread
From: Jan Beulich @ 2026-02-12 15:52 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Jason Andryuk, xen-devel
On 12.02.2026 16:34, Alejandro Vallejo wrote:
> On Thu Feb 12, 2026 at 12:52 PM CET, Jan Beulich wrote:
>> On 06.02.2026 17:15, Alejandro Vallejo wrote:
>>> --- 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 ( !(cpu_vendor() & X86_VENDOR_INTEL) || c->x86 != 6 )
>>> return;
>>>
>>> switch ( c->x86_model )
>>> @@ -915,8 +915,7 @@ 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 ( cpu_vendor() & (X86_VENDOR_AMD | X86_VENDOR_HYGON) &&
>>
>> While we certainly make that assumption, shouldn't you add explicit checks
>> that APs' vendors match the BSP's, in order to be able to also replace
>> current_cpu_data uses? Or do we have such a check, and I'm merely overlooking
>> it?
>
> In generic_identify()
>
> c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
> if (boot_cpu_data.x86_vendor != c->x86_vendor)
> printk(XENLOG_ERR "CPU%u vendor %u mismatch against BSP %u\n",
> smp_processor_id(), c->x86_vendor,
> boot_cpu_data.x86_vendor);
>
> But I'm not sure why this is not a panic() (I thought it was). Such check
> triggering can only mean a Hypervisor bug and a security hole.
>
> Do you see a problem with s/printk/panic/ here?
Well, yes and no. If we settle on the other panic() to remain, maybe this one
would be okay-ish, too. Otoh anything in AP bringup would better not panic,
but simply fail onlining that particular CPU.
Jan
^ permalink raw reply [flat|nested] 55+ messages in thread
end of thread, other threads:[~2026-02-12 15:53 UTC | newest]
Thread overview: 55+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-06 16:15 [PATCH 00/12] const-ify vendor checks Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 01/12] x86: Reject CPU policies with vendors other than the host's Alejandro Vallejo
2026-02-10 8:19 ` Roger Pau Monné
2026-02-10 10:11 ` Alejandro Vallejo
2026-02-11 15:41 ` Jan Beulich
2026-02-11 17:41 ` Alejandro Vallejo
2026-02-12 7:16 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 02/12] x86: Add more granularity to the vendors in Kconfig Alejandro Vallejo
2026-02-10 8:26 ` Roger Pau Monné
2026-02-10 10:04 ` Alejandro Vallejo
2026-02-11 16:06 ` Jan Beulich
2026-02-11 17:51 ` Alejandro Vallejo
2026-02-12 7:24 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor Alejandro Vallejo
2026-02-10 8:46 ` Roger Pau Monné
2026-02-10 10:35 ` Alejandro Vallejo
2026-02-10 12:06 ` Roger Pau Monné
2026-02-11 16:04 ` Jan Beulich
2026-02-11 17:35 ` Alejandro Vallejo
2026-02-11 17:57 ` Alejandro Vallejo
2026-02-12 10:52 ` Jan Beulich
2026-02-12 14:36 ` Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 04/12] x86: Migrate MSR handler vendor checks to cpu_vendor() Alejandro Vallejo
2026-02-11 16:15 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 05/12] x86: Migrate spec_ctrl " Alejandro Vallejo
2026-02-12 10:49 ` Jan Beulich
2026-02-12 14:55 ` Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 06/12] x86: Migrate switch " Alejandro Vallejo
2026-02-12 11:06 ` Jan Beulich
2026-02-12 15:06 ` Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 07/12] x86: Have x86_emulate/ implement the single-vendor optimisation Alejandro Vallejo
2026-02-12 11:26 ` Jan Beulich
2026-02-12 15:29 ` Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 07/12] x86: Migrate x86_emulate/ to use cpu_vendor() Alejandro Vallejo
2026-02-12 11:31 ` Jan Beulich
2026-02-12 15:30 ` Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 08/12] x86/acpi: Migrate vendor checks to cpu_vendor() Alejandro Vallejo
2026-02-12 11:52 ` Jan Beulich
2026-02-12 15:34 ` Alejandro Vallejo
2026-02-12 15:52 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 09/12] x86/pv: " Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 10/12] x86/mcheck: Migrate vendor checks to use cpu_vendor() Alejandro Vallejo
2026-02-12 12:02 ` Jan Beulich
2026-02-12 12:21 ` Jan Beulich
2026-02-12 15:46 ` Alejandro Vallejo
2026-02-06 16:15 ` [PATCH 11/12] x86/cpu: " Alejandro Vallejo
2026-02-12 13:17 ` Jan Beulich
2026-02-06 16:15 ` [PATCH 12/12] x86: Migrate every remaining raw vendor check to cpu_vendor() Alejandro Vallejo
2026-02-12 13:29 ` Jan Beulich
2026-02-09 9:21 ` [PATCH 00/12] const-ify vendor checks Jan Beulich
2026-02-09 10:05 ` Alejandro Vallejo
2026-02-09 10:15 ` Jan Beulich
2026-02-09 11:56 ` Alejandro Vallejo
2026-02-09 12:52 ` Jan Beulich
2026-02-09 14:37 ` 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.