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 <[email protected]> >> --- >> 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
