Re: [PATCH v3] x86: Remove x86 prefixed names from cpuinfo for intel.c

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.11.2025 20:19, Kevin Lampis wrote:
> struct cpuinfo_x86
>   .x86        => .family
>   .x86_vendor => .vendor
>   .x86_model  => .model
>   .x86_mask   => .stepping
> 
> No functional change.
> 
> Signed-off-by: Kevin Lampis <kevin.lampis@xxxxxxxxxx>
> ---
> Changes in v2:
> - Convert the two switch statements in probe_masking_msrs(),
>   intel_log_freq() and check_memory_type_self_snoop_errata()
> - Requested style changes
> 
> Changes in v3:
> - Re-order the CPU model names in the switch statements
> - Better style changes

As to the latter, ...

> @@ -458,7 +460,8 @@ static void Intel_errata_workarounds(struct cpuinfo_x86 
> *c)
>  {
>       uint64_t val;
>  
> -     if ((c->x86 == 15) && (c->x86_model == 1) && (c->x86_mask == 1)) {
> +     if ( c->vfm == INTEL_P4_WILLAMETTE && c->stepping == 1 )
> +     {

... this is still introducing a mix of styles (partly Linux, partly Xen). Imo
we want to avoid this.

> @@ -468,8 +471,10 @@ static void Intel_errata_workarounds(struct cpuinfo_x86 
> *c)
>               }
>       }
>  
> -     if (c->x86 == 6 && cpu_has_clflush &&
> -         (c->x86_model == 29 || c->x86_model == 46 || c->x86_model == 47))
> +     if ( cpu_has_clflush &&
> +         ( c->vfm == INTEL_CORE2_DUNNINGTON ||
> +           c->vfm == INTEL_NEHALEM_EX ||
> +           c->vfm == INTEL_WESTMERE_EX ))
>               setup_force_cpu_cap(X86_BUG_CLFLUSH_MONITOR);

Same here, plus in neither style there's provision for blanks immediately
inside inner parentheses. Indentation of that opening parenthesis would
also be off by one.

> @@ -657,14 +670,16 @@ static void cf_check init_intel(struct cpuinfo_x86 *c)
>       /* Work around errata */
>       Intel_errata_workarounds(c);
>  
> -     if ((c->x86 == 0xf && c->x86_model >= 0x03) ||
> -             (c->x86 == 0x6 && c->x86_model >= 0x0e))
> +     if ( ( c->family == 15 && c->model >= 0x03 ) ||
> +          ( c->family == 6 && c->model >= 0x0e ) )

Again undue mix of styles and undue blanks inside inner parentheses.

> --- a/xen/arch/x86/include/asm/intel-family.h
> +++ b/xen/arch/x86/include/asm/intel-family.h
> @@ -179,6 +179,10 @@
>  #define INTEL_XEON_PHI_KNL           IFM(6, 0x57) /* Knights Landing */
>  #define INTEL_XEON_PHI_KNM           IFM(6, 0x85) /* Knights Mill */
>  
> +/* Family 15 - NetBurst */
> +#define INTEL_P4_WILLAMETTE          IFM(15, 0x01) /* Also Xeon Foster */
> +#define INTEL_P4_PRESCOTT            IFM(15, 0x03)

Andrew, can you please get in your re-syncing patch, so this part can be
omitted?

With the style adjustments (which I can certainly do while committing if no
other need for a v4 arises):
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan