All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH, resend] x86-64: don't track CPU model data that is used by 32-bit code only
@ 2013-10-16 11:45 Jan Beulich
  2013-10-18  6:24 ` Ingo Molnar
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2013-10-16 11:45 UTC (permalink / raw)
  To: mingo, tglx, hpa; +Cc: linux-kernel

struct cpu_dev's c_models is only ever set inside CONFIG_X86_32
conditionals (or code that's being built for 32-bit only), so there's
no use of reserving the (empty) space for the model names in a 64-bit
kernel.

Similarly, c_size_cache is only used in the #else of a CONFIG_X86_64
conditional, so reserving space for (and in one case even initializing)
that field is pointless for 64-bit kernels too.

While moving both fields to the end of the structure, I also noticed
that
- the c_models array size was one too small, potentially causing
  table_lookup_model() to return garbage on Intel CPUs (intel.c's
  instance was lacking the sentinel with family being zero), so the
  patch bumps that by one,
- c_models' vendor sub-field was unused (and anyway redundant with
  the base structure's c_x86_vendor field), so the patch deletes it.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
 arch/x86/kernel/cpu/amd.c     |    2 +-
 arch/x86/kernel/cpu/centaur.c |    6 ++++--
 arch/x86/kernel/cpu/common.c  |    4 +++-
 arch/x86/kernel/cpu/cpu.h     |   16 +++++++---------
 arch/x86/kernel/cpu/intel.c   |    8 ++++----
 arch/x86/kernel/cpu/umc.c     |    2 +-
 6 files changed, 20 insertions(+), 18 deletions(-)

--- 3.12-rc5/arch/x86/kernel/cpu/amd.c
+++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/amd.c
@@ -824,7 +824,7 @@ static const struct cpu_dev amd_cpu_dev 
 	.c_ident	= { "AuthenticAMD" },
 #ifdef CONFIG_X86_32
 	.c_models = {
-		{ .vendor = X86_VENDOR_AMD, .family = 4, .model_names =
+		{ .family = 4, .model_names =
 		  {
 			  [3] = "486 DX/2",
 			  [7] = "486 DX/2-WB",
--- 3.12-rc5/arch/x86/kernel/cpu/centaur.c
+++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/centaur.c
@@ -468,10 +468,10 @@ static void init_centaur(struct cpuinfo_
 #endif
 }
 
+#ifdef CONFIG_X86_32
 static unsigned int
 centaur_size_cache(struct cpuinfo_x86 *c, unsigned int size)
 {
-#ifdef CONFIG_X86_32
 	/* VIA C3 CPUs (670-68F) need further shifting. */
 	if ((c->x86 == 6) && ((c->x86_model == 7) || (c->x86_model == 8)))
 		size >>= 8;
@@ -484,16 +484,18 @@ centaur_size_cache(struct cpuinfo_x86 *c
 	if ((c->x86 == 6) && (c->x86_model == 9) &&
 				(c->x86_mask == 1) && (size == 65))
 		size -= 1;
-#endif
 	return size;
 }
+#endif
 
 static const struct cpu_dev centaur_cpu_dev = {
 	.c_vendor	= "Centaur",
 	.c_ident	= { "CentaurHauls" },
 	.c_early_init	= early_init_centaur,
 	.c_init		= init_centaur,
+#ifdef CONFIG_X86_32
 	.c_size_cache	= centaur_size_cache,
+#endif
 	.c_x86_vendor	= X86_VENDOR_CENTAUR,
 };
 
--- 3.12-rc5/arch/x86/kernel/cpu/common.c
+++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/common.c
@@ -346,6 +346,7 @@ static void filter_cpuid_features(struct
 /* Look up CPU names by table lookup. */
 static const char *table_lookup_model(struct cpuinfo_x86 *c)
 {
+#ifdef CONFIG_X86_32
 	const struct cpu_model_info *info;
 
 	if (c->x86_model >= 16)
@@ -356,11 +357,12 @@ static const char *table_lookup_model(st
 
 	info = this_cpu->c_models;
 
-	while (info && info->family) {
+	while (info->family) {
 		if (info->family == c->x86)
 			return info->model_names[c->x86_model];
 		info++;
 	}
+#endif
 	return NULL;		/* Not found */
 }
 
--- 3.12-rc5/arch/x86/kernel/cpu/cpu.h
+++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/cpu.h
@@ -1,12 +1,6 @@
 #ifndef ARCH_X86_CPU_H
 #define ARCH_X86_CPU_H
 
-struct cpu_model_info {
-	int		vendor;
-	int		family;
-	const char	*model_names[16];
-};
-
 /* attempt to consolidate cpu attributes */
 struct cpu_dev {
 	const char	*c_vendor;
@@ -14,15 +8,19 @@ struct cpu_dev {
 	/* some have two possibilities for cpuid string */
 	const char	*c_ident[2];
 
-	struct		cpu_model_info c_models[4];
-
 	void            (*c_early_init)(struct cpuinfo_x86 *);
 	void		(*c_bsp_init)(struct cpuinfo_x86 *);
 	void		(*c_init)(struct cpuinfo_x86 *);
 	void		(*c_identify)(struct cpuinfo_x86 *);
 	void		(*c_detect_tlb)(struct cpuinfo_x86 *);
-	unsigned int	(*c_size_cache)(struct cpuinfo_x86 *, unsigned int);
 	int		c_x86_vendor;
+#ifdef CONFIG_X86_32
+	unsigned int	(*c_size_cache)(struct cpuinfo_x86 *, unsigned int);
+	struct cpu_model_info {
+		int		family;
+		const char	*model_names[16];
+	}		c_models[5];
+#endif
 };
 
 struct _tlb_table {
--- 3.12-rc5/arch/x86/kernel/cpu/intel.c
+++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/intel.c
@@ -666,7 +666,7 @@ static const struct cpu_dev intel_cpu_de
 	.c_ident	= { "GenuineIntel" },
 #ifdef CONFIG_X86_32
 	.c_models = {
-		{ .vendor = X86_VENDOR_INTEL, .family = 4, .model_names =
+		{ .family = 4, .model_names =
 		  {
 			  [0] = "486 DX-25/33",
 			  [1] = "486 DX-50",
@@ -679,7 +679,7 @@ static const struct cpu_dev intel_cpu_de
 			  [9] = "486 DX/4-WB"
 		  }
 		},
-		{ .vendor = X86_VENDOR_INTEL, .family = 5, .model_names =
+		{ .family = 5, .model_names =
 		  {
 			  [0] = "Pentium 60/66 A-step",
 			  [1] = "Pentium 60/66",
@@ -690,7 +690,7 @@ static const struct cpu_dev intel_cpu_de
 			  [8] = "Mobile Pentium MMX"
 		  }
 		},
-		{ .vendor = X86_VENDOR_INTEL, .family = 6, .model_names =
+		{ .family = 6, .model_names =
 		  {
 			  [0] = "Pentium Pro A-step",
 			  [1] = "Pentium Pro",
@@ -704,7 +704,7 @@ static const struct cpu_dev intel_cpu_de
 			  [11] = "Pentium III (Tualatin)",
 		  }
 		},
-		{ .vendor = X86_VENDOR_INTEL, .family = 15, .model_names =
+		{ .family = 15, .model_names =
 		  {
 			  [0] = "Pentium 4 (Unknown)",
 			  [1] = "Pentium 4 (Willamette)",
--- 3.12-rc5/arch/x86/kernel/cpu/umc.c
+++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/umc.c
@@ -12,7 +12,7 @@ static const struct cpu_dev umc_cpu_dev 
 	.c_vendor	= "UMC",
 	.c_ident	= { "UMC UMC UMC" },
 	.c_models = {
-		{ .vendor = X86_VENDOR_UMC, .family = 4, .model_names =
+		{ .family = 4, .model_names =
 		  {
 			  [1] = "U5D",
 			  [2] = "U5S",



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, resend] x86-64: don't track CPU model data that is used by 32-bit code only
  2013-10-16 11:45 [PATCH, resend] x86-64: don't track CPU model data that is used by 32-bit code only Jan Beulich
@ 2013-10-18  6:24 ` Ingo Molnar
  2013-10-21  8:35   ` [PATCH v2] " Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Ingo Molnar @ 2013-10-18  6:24 UTC (permalink / raw)
  To: Jan Beulich; +Cc: tglx, hpa, linux-kernel


* Jan Beulich <JBeulich@suse.com> wrote:

> struct cpu_dev's c_models is only ever set inside CONFIG_X86_32
> conditionals (or code that's being built for 32-bit only), so there's
> no use of reserving the (empty) space for the model names in a 64-bit
> kernel.
> 
> Similarly, c_size_cache is only used in the #else of a CONFIG_X86_64
> conditional, so reserving space for (and in one case even initializing)
> that field is pointless for 64-bit kernels too.
> 
> While moving both fields to the end of the structure, I also noticed
> that
> - the c_models array size was one too small, potentially causing
>   table_lookup_model() to return garbage on Intel CPUs (intel.c's
>   instance was lacking the sentinel with family being zero), so the
>   patch bumps that by one,
> - c_models' vendor sub-field was unused (and anyway redundant with
>   the base structure's c_x86_vendor field), so the patch deletes it.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
>  arch/x86/kernel/cpu/amd.c     |    2 +-
>  arch/x86/kernel/cpu/centaur.c |    6 ++++--
>  arch/x86/kernel/cpu/common.c  |    4 +++-
>  arch/x86/kernel/cpu/cpu.h     |   16 +++++++---------
>  arch/x86/kernel/cpu/intel.c   |    8 ++++----
>  arch/x86/kernel/cpu/umc.c     |    2 +-
>  6 files changed, 20 insertions(+), 18 deletions(-)
> 
> --- 3.12-rc5/arch/x86/kernel/cpu/amd.c
> +++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/amd.c
> @@ -824,7 +824,7 @@ static const struct cpu_dev amd_cpu_dev 
>  	.c_ident	= { "AuthenticAMD" },
>  #ifdef CONFIG_X86_32
>  	.c_models = {
> -		{ .vendor = X86_VENDOR_AMD, .family = 4, .model_names =
> +		{ .family = 4, .model_names =
>  		  {
>  			  [3] = "486 DX/2",
>  			  [7] = "486 DX/2-WB",
> --- 3.12-rc5/arch/x86/kernel/cpu/centaur.c
> +++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/centaur.c
> @@ -468,10 +468,10 @@ static void init_centaur(struct cpuinfo_
>  #endif
>  }
>  
> +#ifdef CONFIG_X86_32
>  static unsigned int
>  centaur_size_cache(struct cpuinfo_x86 *c, unsigned int size)
>  {
> -#ifdef CONFIG_X86_32
>  	/* VIA C3 CPUs (670-68F) need further shifting. */
>  	if ((c->x86 == 6) && ((c->x86_model == 7) || (c->x86_model == 8)))
>  		size >>= 8;
> @@ -484,16 +484,18 @@ centaur_size_cache(struct cpuinfo_x86 *c
>  	if ((c->x86 == 6) && (c->x86_model == 9) &&
>  				(c->x86_mask == 1) && (size == 65))
>  		size -= 1;
> -#endif
>  	return size;
>  }
> +#endif
>  
>  static const struct cpu_dev centaur_cpu_dev = {
>  	.c_vendor	= "Centaur",
>  	.c_ident	= { "CentaurHauls" },
>  	.c_early_init	= early_init_centaur,
>  	.c_init		= init_centaur,
> +#ifdef CONFIG_X86_32
>  	.c_size_cache	= centaur_size_cache,
> +#endif
>  	.c_x86_vendor	= X86_VENDOR_CENTAUR,
>  };
>  
> --- 3.12-rc5/arch/x86/kernel/cpu/common.c
> +++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/common.c
> @@ -346,6 +346,7 @@ static void filter_cpuid_features(struct
>  /* Look up CPU names by table lookup. */
>  static const char *table_lookup_model(struct cpuinfo_x86 *c)
>  {
> +#ifdef CONFIG_X86_32
>  	const struct cpu_model_info *info;
>  
>  	if (c->x86_model >= 16)
> @@ -356,11 +357,12 @@ static const char *table_lookup_model(st
>  
>  	info = this_cpu->c_models;
>  
> -	while (info && info->family) {
> +	while (info->family) {
>  		if (info->family == c->x86)
>  			return info->model_names[c->x86_model];
>  		info++;
>  	}
> +#endif
>  	return NULL;		/* Not found */
>  }
>  
> --- 3.12-rc5/arch/x86/kernel/cpu/cpu.h
> +++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/cpu.h
> @@ -1,12 +1,6 @@
>  #ifndef ARCH_X86_CPU_H
>  #define ARCH_X86_CPU_H
>  
> -struct cpu_model_info {
> -	int		vendor;
> -	int		family;
> -	const char	*model_names[16];
> -};
> -
>  /* attempt to consolidate cpu attributes */
>  struct cpu_dev {
>  	const char	*c_vendor;
> @@ -14,15 +8,19 @@ struct cpu_dev {
>  	/* some have two possibilities for cpuid string */
>  	const char	*c_ident[2];
>  
> -	struct		cpu_model_info c_models[4];
> -
>  	void            (*c_early_init)(struct cpuinfo_x86 *);
>  	void		(*c_bsp_init)(struct cpuinfo_x86 *);
>  	void		(*c_init)(struct cpuinfo_x86 *);
>  	void		(*c_identify)(struct cpuinfo_x86 *);
>  	void		(*c_detect_tlb)(struct cpuinfo_x86 *);
> -	unsigned int	(*c_size_cache)(struct cpuinfo_x86 *, unsigned int);
>  	int		c_x86_vendor;
> +#ifdef CONFIG_X86_32
> +	unsigned int	(*c_size_cache)(struct cpuinfo_x86 *, unsigned int);
> +	struct cpu_model_info {
> +		int		family;
> +		const char	*model_names[16];
> +	}		c_models[5];
> +#endif
>  };
>  
>  struct _tlb_table {
> --- 3.12-rc5/arch/x86/kernel/cpu/intel.c
> +++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/intel.c
> @@ -666,7 +666,7 @@ static const struct cpu_dev intel_cpu_de
>  	.c_ident	= { "GenuineIntel" },
>  #ifdef CONFIG_X86_32
>  	.c_models = {
> -		{ .vendor = X86_VENDOR_INTEL, .family = 4, .model_names =
> +		{ .family = 4, .model_names =
>  		  {
>  			  [0] = "486 DX-25/33",
>  			  [1] = "486 DX-50",
> @@ -679,7 +679,7 @@ static const struct cpu_dev intel_cpu_de
>  			  [9] = "486 DX/4-WB"
>  		  }
>  		},
> -		{ .vendor = X86_VENDOR_INTEL, .family = 5, .model_names =
> +		{ .family = 5, .model_names =
>  		  {
>  			  [0] = "Pentium 60/66 A-step",
>  			  [1] = "Pentium 60/66",
> @@ -690,7 +690,7 @@ static const struct cpu_dev intel_cpu_de
>  			  [8] = "Mobile Pentium MMX"
>  		  }
>  		},
> -		{ .vendor = X86_VENDOR_INTEL, .family = 6, .model_names =
> +		{ .family = 6, .model_names =
>  		  {
>  			  [0] = "Pentium Pro A-step",
>  			  [1] = "Pentium Pro",
> @@ -704,7 +704,7 @@ static const struct cpu_dev intel_cpu_de
>  			  [11] = "Pentium III (Tualatin)",
>  		  }
>  		},
> -		{ .vendor = X86_VENDOR_INTEL, .family = 15, .model_names =
> +		{ .family = 15, .model_names =
>  		  {
>  			  [0] = "Pentium 4 (Unknown)",
>  			  [1] = "Pentium 4 (Willamette)",
> --- 3.12-rc5/arch/x86/kernel/cpu/umc.c
> +++ 3.12-rc5-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/umc.c
> @@ -12,7 +12,7 @@ static const struct cpu_dev umc_cpu_dev 
>  	.c_vendor	= "UMC",
>  	.c_ident	= { "UMC UMC UMC" },
>  	.c_models = {
> -		{ .vendor = X86_VENDOR_UMC, .family = 4, .model_names =
> +		{ .family = 4, .model_names =
>  		  {
>  			  [1] = "U5D",
>  			  [2] = "U5S",

So I'm not totally convinced about this as it increases the #ifdef count - 
that's why I didn't pick up the earlier submission.

But I guess we could do it if you do one more cleanup: please rename it 
all to ->legacy_model_names, ->legacy_cache_size, etc., to make sure it's 
apparent at first sight that this is an old, secondary identification 
mechanism for pre-sane-CPUID CPUs.

Also maybe describe the fields in a comment line as well.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] x86-64: don't track CPU model data that is used by 32-bit code only
  2013-10-18  6:24 ` Ingo Molnar
@ 2013-10-21  8:35   ` Jan Beulich
  2013-10-26 13:51     ` [tip:x86/cpu] x86/cpu: Track legacy CPU model data only on 32-bit kernels tip-bot for Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2013-10-21  8:35 UTC (permalink / raw)
  To: Ingo Molnar, tglx, hpa; +Cc: linux-kernel

struct cpu_dev's c_models is only ever set inside CONFIG_X86_32
conditionals (or code that's being built for 32-bit only), so there's
no use of reserving the (empty) space for the model names in a 64-bit
kernel.

Similarly, c_size_cache is only used in the #else of a CONFIG_X86_64
conditional, so reserving space for (and in one case even initializing)
that field is pointless for 64-bit kernels too.

While moving both fields to the end of the structure, I also noticed
that
- the c_models array size was one too small, potentially causing
  table_lookup_model() to return garbage on Intel CPUs (intel.c's
  instance was lacking the sentinel with family being zero), so the
  patch bumps that by one,
- c_models' vendor sub-field was unused (and anyway redundant with
  the base structure's c_x86_vendor field), so the patch deletes it.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Rename the legacy fields so that their legacy nature stands out.
    Comment their declarations. (Both requested by Ingo.)
---
 arch/x86/kernel/cpu/amd.c     |    6 +++---
 arch/x86/kernel/cpu/centaur.c |    8 +++++---
 arch/x86/kernel/cpu/common.c  |   12 +++++++-----
 arch/x86/kernel/cpu/cpu.h     |   20 +++++++++++---------
 arch/x86/kernel/cpu/intel.c   |   12 ++++++------
 arch/x86/kernel/cpu/umc.c     |    4 ++--
 6 files changed, 34 insertions(+), 28 deletions(-)

--- 3.12-rc6/arch/x86/kernel/cpu/amd.c
+++ 3.12-rc6-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/amd.c
@@ -823,8 +823,8 @@ static const struct cpu_dev amd_cpu_dev 
 	.c_vendor	= "AMD",
 	.c_ident	= { "AuthenticAMD" },
 #ifdef CONFIG_X86_32
-	.c_models = {
-		{ .vendor = X86_VENDOR_AMD, .family = 4, .model_names =
+	.legacy_models = {
+		{ .family = 4, .model_names =
 		  {
 			  [3] = "486 DX/2",
 			  [7] = "486 DX/2-WB",
@@ -835,7 +835,7 @@ static const struct cpu_dev amd_cpu_dev 
 		  }
 		},
 	},
-	.c_size_cache	= amd_size_cache,
+	.legacy_cache_size = amd_size_cache,
 #endif
 	.c_early_init   = early_init_amd,
 	.c_detect_tlb	= cpu_detect_tlb_amd,
--- 3.12-rc6/arch/x86/kernel/cpu/centaur.c
+++ 3.12-rc6-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/centaur.c
@@ -468,10 +468,10 @@ static void init_centaur(struct cpuinfo_
 #endif
 }
 
+#ifdef CONFIG_X86_32
 static unsigned int
 centaur_size_cache(struct cpuinfo_x86 *c, unsigned int size)
 {
-#ifdef CONFIG_X86_32
 	/* VIA C3 CPUs (670-68F) need further shifting. */
 	if ((c->x86 == 6) && ((c->x86_model == 7) || (c->x86_model == 8)))
 		size >>= 8;
@@ -484,16 +484,18 @@ centaur_size_cache(struct cpuinfo_x86 *c
 	if ((c->x86 == 6) && (c->x86_model == 9) &&
 				(c->x86_mask == 1) && (size == 65))
 		size -= 1;
-#endif
 	return size;
 }
+#endif
 
 static const struct cpu_dev centaur_cpu_dev = {
 	.c_vendor	= "Centaur",
 	.c_ident	= { "CentaurHauls" },
 	.c_early_init	= early_init_centaur,
 	.c_init		= init_centaur,
-	.c_size_cache	= centaur_size_cache,
+#ifdef CONFIG_X86_32
+	.legacy_cache_size = centaur_size_cache,
+#endif
 	.c_x86_vendor	= X86_VENDOR_CENTAUR,
 };
 
--- 3.12-rc6/arch/x86/kernel/cpu/common.c
+++ 3.12-rc6-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/common.c
@@ -346,7 +346,8 @@ static void filter_cpuid_features(struct
 /* Look up CPU names by table lookup. */
 static const char *table_lookup_model(struct cpuinfo_x86 *c)
 {
-	const struct cpu_model_info *info;
+#ifdef CONFIG_X86_32
+	const struct legacy_cpu_model_info *info;
 
 	if (c->x86_model >= 16)
 		return NULL;	/* Range check */
@@ -354,13 +355,14 @@ static const char *table_lookup_model(st
 	if (!this_cpu)
 		return NULL;
 
-	info = this_cpu->c_models;
+	info = this_cpu->legacy_models;
 
-	while (info && info->family) {
+	while (info->family) {
 		if (info->family == c->x86)
 			return info->model_names[c->x86_model];
 		info++;
 	}
+#endif
 	return NULL;		/* Not found */
 }
 
@@ -450,8 +452,8 @@ void cpu_detect_cache_sizes(struct cpuin
 	c->x86_tlbsize += ((ebx >> 16) & 0xfff) + (ebx & 0xfff);
 #else
 	/* do processor-specific cache resizing */
-	if (this_cpu->c_size_cache)
-		l2size = this_cpu->c_size_cache(c, l2size);
+	if (this_cpu->legacy_cache_size)
+		l2size = this_cpu->legacy_cache_size(c, l2size);
 
 	/* Allow user to override all this if necessary. */
 	if (cachesize_override != -1)
--- 3.12-rc6/arch/x86/kernel/cpu/cpu.h
+++ 3.12-rc6-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/cpu.h
@@ -1,12 +1,6 @@
 #ifndef ARCH_X86_CPU_H
 #define ARCH_X86_CPU_H
 
-struct cpu_model_info {
-	int		vendor;
-	int		family;
-	const char	*model_names[16];
-};
-
 /* attempt to consolidate cpu attributes */
 struct cpu_dev {
 	const char	*c_vendor;
@@ -14,15 +8,23 @@ struct cpu_dev {
 	/* some have two possibilities for cpuid string */
 	const char	*c_ident[2];
 
-	struct		cpu_model_info c_models[4];
-
 	void            (*c_early_init)(struct cpuinfo_x86 *);
 	void		(*c_bsp_init)(struct cpuinfo_x86 *);
 	void		(*c_init)(struct cpuinfo_x86 *);
 	void		(*c_identify)(struct cpuinfo_x86 *);
 	void		(*c_detect_tlb)(struct cpuinfo_x86 *);
-	unsigned int	(*c_size_cache)(struct cpuinfo_x86 *, unsigned int);
 	int		c_x86_vendor;
+#ifdef CONFIG_X86_32
+	/* Optional vendor specific routine to obtain the cache size. */
+	unsigned int	(*legacy_cache_size)(struct cpuinfo_x86 *,
+					     unsigned int);
+
+	/* Family/stepping-based lookup table for model names. */
+	struct legacy_cpu_model_info {
+		int		family;
+		const char	*model_names[16];
+	}		legacy_models[5];
+#endif
 };
 
 struct _tlb_table {
--- 3.12-rc6/arch/x86/kernel/cpu/intel.c
+++ 3.12-rc6-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/intel.c
@@ -665,8 +665,8 @@ static const struct cpu_dev intel_cpu_de
 	.c_vendor	= "Intel",
 	.c_ident	= { "GenuineIntel" },
 #ifdef CONFIG_X86_32
-	.c_models = {
-		{ .vendor = X86_VENDOR_INTEL, .family = 4, .model_names =
+	.legacy_models = {
+		{ .family = 4, .model_names =
 		  {
 			  [0] = "486 DX-25/33",
 			  [1] = "486 DX-50",
@@ -679,7 +679,7 @@ static const struct cpu_dev intel_cpu_de
 			  [9] = "486 DX/4-WB"
 		  }
 		},
-		{ .vendor = X86_VENDOR_INTEL, .family = 5, .model_names =
+		{ .family = 5, .model_names =
 		  {
 			  [0] = "Pentium 60/66 A-step",
 			  [1] = "Pentium 60/66",
@@ -690,7 +690,7 @@ static const struct cpu_dev intel_cpu_de
 			  [8] = "Mobile Pentium MMX"
 		  }
 		},
-		{ .vendor = X86_VENDOR_INTEL, .family = 6, .model_names =
+		{ .family = 6, .model_names =
 		  {
 			  [0] = "Pentium Pro A-step",
 			  [1] = "Pentium Pro",
@@ -704,7 +704,7 @@ static const struct cpu_dev intel_cpu_de
 			  [11] = "Pentium III (Tualatin)",
 		  }
 		},
-		{ .vendor = X86_VENDOR_INTEL, .family = 15, .model_names =
+		{ .family = 15, .model_names =
 		  {
 			  [0] = "Pentium 4 (Unknown)",
 			  [1] = "Pentium 4 (Willamette)",
@@ -714,7 +714,7 @@ static const struct cpu_dev intel_cpu_de
 		  }
 		},
 	},
-	.c_size_cache	= intel_size_cache,
+	.legacy_cache_size = intel_size_cache,
 #endif
 	.c_detect_tlb	= intel_detect_tlb,
 	.c_early_init   = early_init_intel,
--- 3.12-rc6/arch/x86/kernel/cpu/umc.c
+++ 3.12-rc6-x86-cpu-32bit-only-pieces/arch/x86/kernel/cpu/umc.c
@@ -11,8 +11,8 @@
 static const struct cpu_dev umc_cpu_dev = {
 	.c_vendor	= "UMC",
 	.c_ident	= { "UMC UMC UMC" },
-	.c_models = {
-		{ .vendor = X86_VENDOR_UMC, .family = 4, .model_names =
+	.legacy_models	= {
+		{ .family = 4, .model_names =
 		  {
 			  [1] = "U5D",
 			  [2] = "U5S",



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tip:x86/cpu] x86/cpu: Track legacy CPU model data only on 32-bit kernels
  2013-10-21  8:35   ` [PATCH v2] " Jan Beulich
@ 2013-10-26 13:51     ` tip-bot for Jan Beulich
  0 siblings, 0 replies; 4+ messages in thread
From: tip-bot for Jan Beulich @ 2013-10-26 13:51 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, jbeulich, JBeulich, tglx

Commit-ID:  09dc68d958c67c76cf672ec78b7391af453010f8
Gitweb:     http://git.kernel.org/tip/09dc68d958c67c76cf672ec78b7391af453010f8
Author:     Jan Beulich <JBeulich@suse.com>
AuthorDate: Mon, 21 Oct 2013 09:35:20 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 26 Oct 2013 13:34:39 +0200

x86/cpu: Track legacy CPU model data only on 32-bit kernels

struct cpu_dev's c_models is only ever set inside CONFIG_X86_32
conditionals (or code that's being built for 32-bit only), so
there's no use of reserving the (empty) space for the model
names in a 64-bit kernel.

Similarly, c_size_cache is only used in the #else of a
CONFIG_X86_64 conditional, so reserving space for (and in one
case even initializing) that field is pointless for 64-bit
kernels too.

While moving both fields to the end of the structure, I also
noticed that:

 - the c_models array size was one too small, potentially causing
   table_lookup_model() to return garbage on Intel CPUs (intel.c's
   instance was lacking the sentinel with family being zero), so the
   patch bumps that by one,

 - c_models' vendor sub-field was unused (and anyway redundant
   with the base structure's c_x86_vendor field), so the patch deletes it.

Also rename the legacy fields so that their legacy nature stands out
and comment their declarations.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Link: http://lkml.kernel.org/r/5265036802000078000FC4DB@nat28.tlf.novell.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/amd.c     |  6 +++---
 arch/x86/kernel/cpu/centaur.c |  8 +++++---
 arch/x86/kernel/cpu/common.c  | 12 +++++++-----
 arch/x86/kernel/cpu/cpu.h     | 20 +++++++++++---------
 arch/x86/kernel/cpu/intel.c   | 12 ++++++------
 arch/x86/kernel/cpu/umc.c     |  4 ++--
 6 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 903a264..3daece7 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -823,8 +823,8 @@ static const struct cpu_dev amd_cpu_dev = {
 	.c_vendor	= "AMD",
 	.c_ident	= { "AuthenticAMD" },
 #ifdef CONFIG_X86_32
-	.c_models = {
-		{ .vendor = X86_VENDOR_AMD, .family = 4, .model_names =
+	.legacy_models = {
+		{ .family = 4, .model_names =
 		  {
 			  [3] = "486 DX/2",
 			  [7] = "486 DX/2-WB",
@@ -835,7 +835,7 @@ static const struct cpu_dev amd_cpu_dev = {
 		  }
 		},
 	},
-	.c_size_cache	= amd_size_cache,
+	.legacy_cache_size = amd_size_cache,
 #endif
 	.c_early_init   = early_init_amd,
 	.c_detect_tlb	= cpu_detect_tlb_amd,
diff --git a/arch/x86/kernel/cpu/centaur.c b/arch/x86/kernel/cpu/centaur.c
index fbf6c3b..8d5652d 100644
--- a/arch/x86/kernel/cpu/centaur.c
+++ b/arch/x86/kernel/cpu/centaur.c
@@ -468,10 +468,10 @@ static void init_centaur(struct cpuinfo_x86 *c)
 #endif
 }
 
+#ifdef CONFIG_X86_32
 static unsigned int
 centaur_size_cache(struct cpuinfo_x86 *c, unsigned int size)
 {
-#ifdef CONFIG_X86_32
 	/* VIA C3 CPUs (670-68F) need further shifting. */
 	if ((c->x86 == 6) && ((c->x86_model == 7) || (c->x86_model == 8)))
 		size >>= 8;
@@ -484,16 +484,18 @@ centaur_size_cache(struct cpuinfo_x86 *c, unsigned int size)
 	if ((c->x86 == 6) && (c->x86_model == 9) &&
 				(c->x86_mask == 1) && (size == 65))
 		size -= 1;
-#endif
 	return size;
 }
+#endif
 
 static const struct cpu_dev centaur_cpu_dev = {
 	.c_vendor	= "Centaur",
 	.c_ident	= { "CentaurHauls" },
 	.c_early_init	= early_init_centaur,
 	.c_init		= init_centaur,
-	.c_size_cache	= centaur_size_cache,
+#ifdef CONFIG_X86_32
+	.legacy_cache_size = centaur_size_cache,
+#endif
 	.c_x86_vendor	= X86_VENDOR_CENTAUR,
 };
 
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 2793d1f..9ada0b3 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -346,7 +346,8 @@ static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
 /* Look up CPU names by table lookup. */
 static const char *table_lookup_model(struct cpuinfo_x86 *c)
 {
-	const struct cpu_model_info *info;
+#ifdef CONFIG_X86_32
+	const struct legacy_cpu_model_info *info;
 
 	if (c->x86_model >= 16)
 		return NULL;	/* Range check */
@@ -354,13 +355,14 @@ static const char *table_lookup_model(struct cpuinfo_x86 *c)
 	if (!this_cpu)
 		return NULL;
 
-	info = this_cpu->c_models;
+	info = this_cpu->legacy_models;
 
-	while (info && info->family) {
+	while (info->family) {
 		if (info->family == c->x86)
 			return info->model_names[c->x86_model];
 		info++;
 	}
+#endif
 	return NULL;		/* Not found */
 }
 
@@ -450,8 +452,8 @@ void cpu_detect_cache_sizes(struct cpuinfo_x86 *c)
 	c->x86_tlbsize += ((ebx >> 16) & 0xfff) + (ebx & 0xfff);
 #else
 	/* do processor-specific cache resizing */
-	if (this_cpu->c_size_cache)
-		l2size = this_cpu->c_size_cache(c, l2size);
+	if (this_cpu->legacy_cache_size)
+		l2size = this_cpu->legacy_cache_size(c, l2size);
 
 	/* Allow user to override all this if necessary. */
 	if (cachesize_override != -1)
diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h
index 4041c24..c37dc37 100644
--- a/arch/x86/kernel/cpu/cpu.h
+++ b/arch/x86/kernel/cpu/cpu.h
@@ -1,12 +1,6 @@
 #ifndef ARCH_X86_CPU_H
 #define ARCH_X86_CPU_H
 
-struct cpu_model_info {
-	int		vendor;
-	int		family;
-	const char	*model_names[16];
-};
-
 /* attempt to consolidate cpu attributes */
 struct cpu_dev {
 	const char	*c_vendor;
@@ -14,15 +8,23 @@ struct cpu_dev {
 	/* some have two possibilities for cpuid string */
 	const char	*c_ident[2];
 
-	struct		cpu_model_info c_models[4];
-
 	void            (*c_early_init)(struct cpuinfo_x86 *);
 	void		(*c_bsp_init)(struct cpuinfo_x86 *);
 	void		(*c_init)(struct cpuinfo_x86 *);
 	void		(*c_identify)(struct cpuinfo_x86 *);
 	void		(*c_detect_tlb)(struct cpuinfo_x86 *);
-	unsigned int	(*c_size_cache)(struct cpuinfo_x86 *, unsigned int);
 	int		c_x86_vendor;
+#ifdef CONFIG_X86_32
+	/* Optional vendor specific routine to obtain the cache size. */
+	unsigned int	(*legacy_cache_size)(struct cpuinfo_x86 *,
+					     unsigned int);
+
+	/* Family/stepping-based lookup table for model names. */
+	struct legacy_cpu_model_info {
+		int		family;
+		const char	*model_names[16];
+	}		legacy_models[5];
+#endif
 };
 
 struct _tlb_table {
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index ec72995..dc1ec0d 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -665,8 +665,8 @@ static const struct cpu_dev intel_cpu_dev = {
 	.c_vendor	= "Intel",
 	.c_ident	= { "GenuineIntel" },
 #ifdef CONFIG_X86_32
-	.c_models = {
-		{ .vendor = X86_VENDOR_INTEL, .family = 4, .model_names =
+	.legacy_models = {
+		{ .family = 4, .model_names =
 		  {
 			  [0] = "486 DX-25/33",
 			  [1] = "486 DX-50",
@@ -679,7 +679,7 @@ static const struct cpu_dev intel_cpu_dev = {
 			  [9] = "486 DX/4-WB"
 		  }
 		},
-		{ .vendor = X86_VENDOR_INTEL, .family = 5, .model_names =
+		{ .family = 5, .model_names =
 		  {
 			  [0] = "Pentium 60/66 A-step",
 			  [1] = "Pentium 60/66",
@@ -690,7 +690,7 @@ static const struct cpu_dev intel_cpu_dev = {
 			  [8] = "Mobile Pentium MMX"
 		  }
 		},
-		{ .vendor = X86_VENDOR_INTEL, .family = 6, .model_names =
+		{ .family = 6, .model_names =
 		  {
 			  [0] = "Pentium Pro A-step",
 			  [1] = "Pentium Pro",
@@ -704,7 +704,7 @@ static const struct cpu_dev intel_cpu_dev = {
 			  [11] = "Pentium III (Tualatin)",
 		  }
 		},
-		{ .vendor = X86_VENDOR_INTEL, .family = 15, .model_names =
+		{ .family = 15, .model_names =
 		  {
 			  [0] = "Pentium 4 (Unknown)",
 			  [1] = "Pentium 4 (Willamette)",
@@ -714,7 +714,7 @@ static const struct cpu_dev intel_cpu_dev = {
 		  }
 		},
 	},
-	.c_size_cache	= intel_size_cache,
+	.legacy_cache_size = intel_size_cache,
 #endif
 	.c_detect_tlb	= intel_detect_tlb,
 	.c_early_init   = early_init_intel,
diff --git a/arch/x86/kernel/cpu/umc.c b/arch/x86/kernel/cpu/umc.c
index 202759a..75c5ad5 100644
--- a/arch/x86/kernel/cpu/umc.c
+++ b/arch/x86/kernel/cpu/umc.c
@@ -11,8 +11,8 @@
 static const struct cpu_dev umc_cpu_dev = {
 	.c_vendor	= "UMC",
 	.c_ident	= { "UMC UMC UMC" },
-	.c_models = {
-		{ .vendor = X86_VENDOR_UMC, .family = 4, .model_names =
+	.legacy_models	= {
+		{ .family = 4, .model_names =
 		  {
 			  [1] = "U5D",
 			  [2] = "U5S",

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-10-26 13:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-16 11:45 [PATCH, resend] x86-64: don't track CPU model data that is used by 32-bit code only Jan Beulich
2013-10-18  6:24 ` Ingo Molnar
2013-10-21  8:35   ` [PATCH v2] " Jan Beulich
2013-10-26 13:51     ` [tip:x86/cpu] x86/cpu: Track legacy CPU model data only on 32-bit kernels tip-bot for Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.