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 <[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. 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.
