Re: [PATCH 12/12] x86: Migrate every remaining raw vendor check to cpu_vendor()

[Jan Beulich <jbeulich@xxxxxxxx>]

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