Re: [PATCH 10/12] x86/mcheck: Migrate vendor checks to use cpu_vendor()

[Alejandro Vallejo <alejandro.garciavallejo@amd.com>]

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