1. Patchew
  2. Xen
  3. View series

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

Kevin Lampis posted 1 patch 2 weeks, 4 days ago
Failed in applying to current master (apply log)
There is a newer version of this series
xen/arch/x86/cpu/intel.c                | 112 ++++++++++++++----------
xen/arch/x86/include/asm/intel-family.h |   4 +
2 files changed, 68 insertions(+), 48 deletions(-)
[PATCH v2] x86: Remove x86 prefixed names from cpuinfo for intel.c
Posted by Kevin Lampis 2 weeks, 4 days ago 
struct cpuinfo_x86
  .x86        => .family
  .x86_vendor => .vendor
  .x86_model  => .model
  .x86_mask   => .stepping

No functional change.

Signed-off-by: Kevin Lampis <kevin.lampis@citrix.com>
---
Changes in v2:
- Convert the two switch statements in probe_masking_msrs()
  and check_memory_type_self_snoop_errata()
- Requested style changes

 xen/arch/x86/cpu/intel.c                | 112 ++++++++++++++----------
 xen/arch/x86/include/asm/intel-family.h |   4 +
 2 files changed, 68 insertions(+), 48 deletions(-)

diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index 6f71365b7e..1edc95b046 100644
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -86,18 +86,19 @@ static void __init check_memory_type_self_snoop_errata(void)
 	if (!boot_cpu_has(X86_FEATURE_SS))
 		return;
 
-	switch (boot_cpu_data.x86_model) {
-	case 0x0f: /* Merom */
-	case 0x16: /* Merom L */
-	case 0x17: /* Penryn */
-	case 0x1d: /* Dunnington */
-	case 0x1e: /* Nehalem */
-	case 0x1f: /* Auburndale / Havendale */
-	case 0x1a: /* Nehalem EP */
-	case 0x2e: /* Nehalem EX */
-	case 0x25: /* Westmere */
-	case 0x2c: /* Westmere EP */
-	case 0x2a: /* SandyBridge */
+	switch ( boot_cpu_data.vfm )
+	{
+	case INTEL_CORE2_MEROM:
+	case INTEL_CORE2_MEROM_L:
+	case INTEL_CORE2_PENRYN:
+	case INTEL_CORE2_DUNNINGTON:
+	case INTEL_NEHALEM:
+	case INTEL_NEHALEM_G:
+	case INTEL_NEHALEM_EP:
+	case INTEL_NEHALEM_EX:
+	case INTEL_WESTMERE:
+	case INTEL_WESTMERE_EP:
+	case INTEL_SANDYBRIDGE:
 		return;
 	}
 
@@ -137,28 +138,29 @@ static void __init probe_masking_msrs(void)
 	unsigned int exp_msr_basic, exp_msr_ext, exp_msr_xsave;
 
 	/* Only family 6 supports this feature. */
-	if (c->x86 != 6)
+	if (c->family != 6)
 		return;
 
-	switch (c->x86_model) {
-	case 0x17: /* Yorkfield, Wolfdale, Penryn, Harpertown(DP) */
-	case 0x1d: /* Dunnington(MP) */
+	switch ( c->vfm )
+	{
+	case INTEL_CORE2_PENRYN:
+	case INTEL_CORE2_DUNNINGTON:
 		msr_basic = MSR_INTEL_MASK_V1_CPUID1;
 		break;
 
-	case 0x1a: /* Bloomfield, Nehalem-EP(Gainestown) */
-	case 0x1e: /* Clarksfield, Lynnfield, Jasper Forest */
-	case 0x1f: /* Something Nehalem-based - perhaps Auburndale/Havendale? */
-	case 0x25: /* Arrandale, Clarksdale */
-	case 0x2c: /* Gulftown, Westmere-EP */
-	case 0x2e: /* Nehalem-EX(Beckton) */
-	case 0x2f: /* Westmere-EX */
+	case INTEL_NEHALEM_EP:
+	case INTEL_NEHALEM:
+	case INTEL_NEHALEM_G:
+	case INTEL_WESTMERE:
+	case INTEL_WESTMERE_EP:
+	case INTEL_NEHALEM_EX:
+	case INTEL_WESTMERE_EX:
 		msr_basic = MSR_INTEL_MASK_V2_CPUID1;
 		msr_ext   = MSR_INTEL_MASK_V2_CPUID80000001;
 		break;
 
-	case 0x2a: /* SandyBridge */
-	case 0x2d: /* SandyBridge-E, SandyBridge-EN, SandyBridge-EP */
+	case INTEL_SANDYBRIDGE:
+	case INTEL_SANDYBRIDGE_X:
 		msr_basic = MSR_INTEL_MASK_V3_CPUID1;
 		msr_ext   = MSR_INTEL_MASK_V3_CPUID80000001;
 		msr_xsave = MSR_INTEL_MASK_V3_CPUIDD_01;
@@ -191,7 +193,7 @@ static void __init probe_masking_msrs(void)
 	       expected_levelling_cap, levelling_caps,
 	       (expected_levelling_cap ^ levelling_caps) & levelling_caps);
 	printk(XENLOG_WARNING "Fam %#x, model %#x expected (%#x/%#x/%#x), "
-	       "got (%#x/%#x/%#x)\n", c->x86, c->x86_model,
+	       "got (%#x/%#x/%#x)\n", c->family, c->model,
 	       exp_msr_basic, exp_msr_ext, exp_msr_xsave,
 	       msr_basic, msr_ext, msr_xsave);
 	printk(XENLOG_WARNING
@@ -265,7 +267,7 @@ static void __init noinline intel_init_levelling(void)
 	 * so skip it altogether. In the case where Xen is virtualized these
 	 * MSRs may be emulated though, so we allow it in that case.
 	 */
-	if ((boot_cpu_data.x86 != 0xf || cpu_has_hypervisor) &&
+	if ((boot_cpu_data.family != 0xf || cpu_has_hypervisor) &&
 	    probe_cpuid_faulting()) {
 		expected_levelling_cap |= LCAP_faulting;
 		levelling_caps |= LCAP_faulting;
@@ -348,7 +350,7 @@ void intel_unlock_cpuid_leaves(struct cpuinfo_x86 *c)
 static void cf_check early_init_intel(struct cpuinfo_x86 *c)
 {
 	/* Netburst reports 64 bytes clflush size, but does IO in 128 bytes */
-	if (c->x86 == 15 && c->x86_cache_alignment == 64)
+	if (c->family == 15 && c->x86_cache_alignment == 64)
 		c->x86_cache_alignment = 128;
 
 	if (c == &boot_cpu_data &&
@@ -358,8 +360,8 @@ static void cf_check early_init_intel(struct cpuinfo_x86 *c)
 	intel_unlock_cpuid_leaves(c);
 
 	/* CPUID workaround for Intel 0F33/0F34 CPU */
-	if (boot_cpu_data.x86 == 0xF && boot_cpu_data.x86_model == 3 &&
-	    (boot_cpu_data.x86_mask == 3 || boot_cpu_data.x86_mask == 4))
+	if (boot_cpu_data.vfm == INTEL_P4_PRESCOTT &&
+	    (boot_cpu_data.stepping == 3 || boot_cpu_data.stepping == 4))
 		paddr_bits = 36;
 
 	if (c == &boot_cpu_data) {
@@ -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 )
+	{
 		val = rdmsr(MSR_IA32_MISC_ENABLE);
 		if ((val & (1 << 9)) == 0) {
 			printk (KERN_INFO "CPU: C0 stepping P4 Xeon detected.\n");
@@ -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);
 
 	probe_c3_errata(c);
@@ -540,7 +545,7 @@ static void intel_log_freq(const struct cpuinfo_x86 *c)
         }
     }
 
-    switch ( c->x86 )
+    switch ( c->family )
     {
         static const unsigned short core_factors[] =
             { 26667, 13333, 20000, 16667, 33333, 10000, 40000 };
@@ -553,10 +558,13 @@ static void intel_log_freq(const struct cpuinfo_x86 *c)
         if ( !max_ratio )
             return;
 
-        switch ( c->x86_model )
+        switch ( c->vfm )
         {
-        case 0x0e: /* Core */
-        case 0x0f: case 0x16: case 0x17: case 0x1d: /* Core2 */
+        case INTEL_CORE_YONAH:
+        case INTEL_CORE2_MEROM:
+        case INTEL_CORE2_MEROM_L:
+        case INTEL_CORE2_PENRYN:
+        case INTEL_CORE2_DUNNINGTON:
             /*
              * PLATFORM_INFO, while not documented for these, appears to exist
              * in at least some cases, but what it holds doesn't match the
@@ -572,8 +580,13 @@ static void intel_log_freq(const struct cpuinfo_x86 *c)
             factor = core_factors[msrval];
             break;
 
-        case 0x1a: case 0x1e: case 0x1f: case 0x2e: /* Nehalem */
-        case 0x25: case 0x2c: case 0x2f: /* Westmere */
+        case INTEL_NEHALEM_EP:
+        case INTEL_NEHALEM:
+        case INTEL_NEHALEM_G:
+        case INTEL_NEHALEM_EX:
+        case INTEL_WESTMERE:
+        case INTEL_WESTMERE_EP:
+        case INTEL_WESTMERE_EX:
             factor = 13333;
             break;
 
@@ -583,7 +596,7 @@ static void intel_log_freq(const struct cpuinfo_x86 *c)
         }
         break;
 
-    case 0xf:
+    case 15:
         if ( rdmsr_safe(MSR_IA32_EBC_FREQUENCY_ID, &msrval) )
             return;
         max_ratio = msrval >> 24;
@@ -657,14 +670,17 @@ 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))
-		__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
-	if (cpu_has(c, X86_FEATURE_ITSC)) {
-		__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
-		__set_bit(X86_FEATURE_NONSTOP_TSC, c->x86_capability);
-		__set_bit(X86_FEATURE_TSC_RELIABLE, c->x86_capability);
-	}
+    if ( ( c->family == 15 && c->model >= 0x03 ) ||
+         ( c->family == 6 && c->model >= 0x0e ) )
+        __set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
+
+    if ( cpu_has(c, X86_FEATURE_ITSC) )
+    {
+        __set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
+        __set_bit(X86_FEATURE_NONSTOP_TSC, c->x86_capability);
+        __set_bit(X86_FEATURE_TSC_RELIABLE, c->x86_capability);
+    }
+
 	if ( opt_arat &&
 	     ( c->cpuid_level >= 0x00000006 ) &&
 	     ( cpuid_eax(0x00000006) & (1u<<2) ) )
diff --git a/xen/arch/x86/include/asm/intel-family.h b/xen/arch/x86/include/asm/intel-family.h
index d8c0bcc406..c71e744731 100644
--- 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)
+
 /* Family 5 */
 #define INTEL_FAM5_QUARK_X1000		0x09 /* Quark X1000 SoC */
 #define INTEL_QUARK_X1000		IFM(5, 0x09) /* Quark X1000 SoC */
-- 
2.51.1
Re: [PATCH v2] x86: Remove x86 prefixed names from cpuinfo for intel.c
Posted by Jan Beulich 2 weeks, 3 days ago 
On 25.11.2025 21:27, 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@citrix.com>
> ---
> Changes in v2:
> - Convert the two switch statements in probe_masking_msrs()
>   and check_memory_type_self_snoop_errata()
> - Requested style changes

As to the latter - not quite, see below.

> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -86,18 +86,19 @@ static void __init check_memory_type_self_snoop_errata(void)
>  	if (!boot_cpu_has(X86_FEATURE_SS))
>  		return;
>  
> -	switch (boot_cpu_data.x86_model) {
> -	case 0x0f: /* Merom */
> -	case 0x16: /* Merom L */
> -	case 0x17: /* Penryn */
> -	case 0x1d: /* Dunnington */
> -	case 0x1e: /* Nehalem */
> -	case 0x1f: /* Auburndale / Havendale */
> -	case 0x1a: /* Nehalem EP */
> -	case 0x2e: /* Nehalem EX */
> -	case 0x25: /* Westmere */
> -	case 0x2c: /* Westmere EP */
> -	case 0x2a: /* SandyBridge */
> +	switch ( boot_cpu_data.vfm )
> +	{
> +	case INTEL_CORE2_MEROM:
> +	case INTEL_CORE2_MEROM_L:
> +	case INTEL_CORE2_PENRYN:
> +	case INTEL_CORE2_DUNNINGTON:
> +	case INTEL_NEHALEM:
> +	case INTEL_NEHALEM_G:
> +	case INTEL_NEHALEM_EP:
> +	case INTEL_NEHALEM_EX:
> +	case INTEL_WESTMERE:
> +	case INTEL_WESTMERE_EP:
> +	case INTEL_SANDYBRIDGE:

The ordering here would imo ...

> @@ -137,28 +138,29 @@ static void __init probe_masking_msrs(void)
>  	unsigned int exp_msr_basic, exp_msr_ext, exp_msr_xsave;
>  
>  	/* Only family 6 supports this feature. */
> -	if (c->x86 != 6)
> +	if (c->family != 6)
>  		return;
>  
> -	switch (c->x86_model) {
> -	case 0x17: /* Yorkfield, Wolfdale, Penryn, Harpertown(DP) */
> -	case 0x1d: /* Dunnington(MP) */
> +	switch ( c->vfm )
> +	{
> +	case INTEL_CORE2_PENRYN:
> +	case INTEL_CORE2_DUNNINGTON:
>  		msr_basic = MSR_INTEL_MASK_V1_CPUID1;
>  		break;
>  
> -	case 0x1a: /* Bloomfield, Nehalem-EP(Gainestown) */
> -	case 0x1e: /* Clarksfield, Lynnfield, Jasper Forest */
> -	case 0x1f: /* Something Nehalem-based - perhaps Auburndale/Havendale? */
> -	case 0x25: /* Arrandale, Clarksdale */
> -	case 0x2c: /* Gulftown, Westmere-EP */
> -	case 0x2e: /* Nehalem-EX(Beckton) */
> -	case 0x2f: /* Westmere-EX */
> +	case INTEL_NEHALEM_EP:
> +	case INTEL_NEHALEM:
> +	case INTEL_NEHALEM_G:
> +	case INTEL_WESTMERE:
> +	case INTEL_WESTMERE_EP:
> +	case INTEL_NEHALEM_EX:
> +	case INTEL_WESTMERE_EX:

... best also be followed here, even if that means some re-ordering
compared to what original code had.

> @@ -572,8 +580,13 @@ static void intel_log_freq(const struct cpuinfo_x86 *c)
>              factor = core_factors[msrval];
>              break;
>  
> -        case 0x1a: case 0x1e: case 0x1f: case 0x2e: /* Nehalem */
> -        case 0x25: case 0x2c: case 0x2f: /* Westmere */
> +        case INTEL_NEHALEM_EP:
> +        case INTEL_NEHALEM:
> +        case INTEL_NEHALEM_G:
> +        case INTEL_NEHALEM_EX:
> +        case INTEL_WESTMERE:
> +        case INTEL_WESTMERE_EP:
> +        case INTEL_WESTMERE_EX:
>              factor = 13333;
>              break;

Same here. (This iirc also wasn't there in v1, but isn't mentioned in
the changelog.)

> @@ -657,14 +670,17 @@ 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))
> -		__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> -	if (cpu_has(c, X86_FEATURE_ITSC)) {
> -		__set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> -		__set_bit(X86_FEATURE_NONSTOP_TSC, c->x86_capability);
> -		__set_bit(X86_FEATURE_TSC_RELIABLE, c->x86_capability);
> -	}
> +    if ( ( c->family == 15 && c->model >= 0x03 ) ||
> +         ( c->family == 6 && c->model >= 0x0e ) )
> +        __set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> +
> +    if ( cpu_has(c, X86_FEATURE_ITSC) )
> +    {
> +        __set_bit(X86_FEATURE_CONSTANT_TSC, c->x86_capability);
> +        __set_bit(X86_FEATURE_NONSTOP_TSC, c->x86_capability);
> +        __set_bit(X86_FEATURE_TSC_RELIABLE, c->x86_capability);
> +    }
> +
>  	if ( opt_arat &&
>  	     ( c->cpuid_level >= 0x00000006 ) &&
>  	     ( cpuid_eax(0x00000006) & (1u<<2) ) )

From your v1 reply I concluded that you understood that this isn't the way
to go. Within a function, indentation shouldn't vary like this. I suggest
anyway that you really wait with submitting a new version until discussion
has settled.

Jan

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.