Re: [PATCH 03/12] x86: Add cpu_vendor() as a wrapper for the host's CPU vendor

["Roger Pau Monné" <roger.pau@citrix.com>]

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.