diff mbox series

[1/5] x86/cpu: Drop cpu_devs[] and $VENDOR_init_cpu() hooks

Message ID 1554409592-28572-2-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86/cpu: Rework of X86_VENDOR_* constants | expand

Commit Message

Andrew Cooper April 4, 2019, 8:26 p.m. UTC 
These helpers each fill in a single cpu_devs[] pointer, and since c/s
00b4f4d0f "x86/cpuid: Drop get_cpu_vendor() completely", this array is read
exactly once on boot.

Delete the hooks and cpu_devs[], and have early_cpu_detect() pick the
appropriate cpu_dev structure directly.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: David Wang <davidwang@zhaoxin.com>
CC: Pu Wen <puwen@hygon.cn>
---
 xen/arch/x86/cpu/amd.c      |  8 +-------
 xen/arch/x86/cpu/centaur.c  |  8 +-------
 xen/arch/x86/cpu/common.c   | 16 +++++++---------
 xen/arch/x86/cpu/cpu.h      |  8 ++------
 xen/arch/x86/cpu/intel.c    | 11 +----------
 xen/arch/x86/cpu/shanghai.c |  8 +-------
 6 files changed, 13 insertions(+), 46 deletions(-)

Comments

Jan Beulich April 5, 2019, 8:57 a.m. UTC | #1 
>>> On 04.04.19 at 22:26, <andrew.cooper3@citrix.com> wrote:
> These helpers each fill in a single cpu_devs[] pointer, and since c/s
> 00b4f4d0f "x86/cpuid: Drop get_cpu_vendor() completely", this array is read
> exactly once on boot.
> 
> Delete the hooks and cpu_devs[], and have early_cpu_detect() pick the
> appropriate cpu_dev structure directly.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one remark:

> @@ -687,10 +689,6 @@ static cpumask_t cpu_initialized;
>  
>  void __init early_cpu_init(void)
>  {
> -	intel_cpu_init();
> -	amd_init_cpu();
> -	centaur_init_cpu();
> -	shanghai_init_cpu();
>  	early_cpu_detect();
>  }

I think this function should then go away as well, unless you have
plans to put new stuff into it. To avoid modifying the caller,
perhaps best to simply renamed early_cpu_detect()? My R-b
stands with this change, if you decide to fold it in.

Jan
Andrew Cooper April 5, 2019, 9:27 a.m. UTC | #2 
On 05/04/2019 09:57, Jan Beulich wrote:
>>>> On 04.04.19 at 22:26, <andrew.cooper3@citrix.com> wrote:
>> These helpers each fill in a single cpu_devs[] pointer, and since c/s
>> 00b4f4d0f "x86/cpuid: Drop get_cpu_vendor() completely", this array is read
>> exactly once on boot.
>>
>> Delete the hooks and cpu_devs[], and have early_cpu_detect() pick the
>> appropriate cpu_dev structure directly.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with one remark:
>
>> @@ -687,10 +689,6 @@ static cpumask_t cpu_initialized;
>>  
>>  void __init early_cpu_init(void)
>>  {
>> -	intel_cpu_init();
>> -	amd_init_cpu();
>> -	centaur_init_cpu();
>> -	shanghai_init_cpu();
>>  	early_cpu_detect();
>>  }
> I think this function should then go away as well, unless you have
> plans to put new stuff into it. To avoid modifying the caller,
> perhaps best to simply renamed early_cpu_detect()? My R-b
> stands with this change, if you decide to fold it in.

I was considering whether to do that, so I will.

~Andrew
diff mbox series

Patch

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 7a73d62..d58952b 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -792,14 +792,8 @@  static void init_amd(struct cpuinfo_x86 *c)
 	check_syscfg_dram_mod_en();
 }
 
-static const struct cpu_dev amd_cpu_dev = {
+const struct cpu_dev amd_cpu_dev = {
 	.c_vendor	= "AMD",
 	.c_early_init	= early_init_amd,
 	.c_init		= init_amd,
 };
-
-int __init amd_init_cpu(void)
-{
-	cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;
-	return 0;
-}
diff --git a/xen/arch/x86/cpu/centaur.c b/xen/arch/x86/cpu/centaur.c
index 71f6503..268f4d4 100644
--- a/xen/arch/x86/cpu/centaur.c
+++ b/xen/arch/x86/cpu/centaur.c
@@ -54,13 +54,7 @@  static void init_centaur(struct cpuinfo_x86 *c)
 		init_c3(c);
 }
 
-static const struct cpu_dev centaur_cpu_dev = {
+const struct cpu_dev centaur_cpu_dev = {
 	.c_vendor	= "Centaur",
 	.c_init		= init_centaur,
 };
-
-int __init centaur_init_cpu(void)
-{
-	cpu_devs[X86_VENDOR_CENTAUR] = &centaur_cpu_dev;
-	return 0;
-}
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index b2249b5..4cf9ec2 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -42,8 +42,6 @@  unsigned int __read_mostly levelling_caps;
 DEFINE_PER_CPU(struct cpuidmasks, cpuidmasks);
 struct cpuidmasks __read_mostly cpuidmask_defaults;
 
-const struct cpu_dev *__read_mostly cpu_devs[X86_VENDOR_NUM] = {};
-
 unsigned int paddr_bits __read_mostly = 36;
 unsigned int hap_paddr_bits __read_mostly = 36;
 unsigned int vaddr_bits __read_mostly = VADDR_BITS;
@@ -284,12 +282,16 @@  static void __init early_cpu_detect(void)
 	*(u32 *)&c->x86_vendor_id[4] = edx;
 
 	c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
-	if (c->x86_vendor < ARRAY_SIZE(cpu_devs) && cpu_devs[c->x86_vendor])
-		this_cpu = cpu_devs[c->x86_vendor];
-	else
+	switch (c->x86_vendor) {
+	case X86_VENDOR_INTEL:	  this_cpu = &intel_cpu_dev;    break;
+	case X86_VENDOR_AMD:	  this_cpu = &amd_cpu_dev;      break;
+	case X86_VENDOR_CENTAUR:  this_cpu = &centaur_cpu_dev;  break;
+	case X86_VENDOR_SHANGHAI: this_cpu = &shanghai_cpu_dev; break;
+	default:
 		printk(XENLOG_ERR
 		       "Unrecognised or unsupported CPU vendor '%.12s'\n",
 		       c->x86_vendor_id);
+	}
 
 	cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
 	c->x86 = get_cpu_family(eax, &c->x86_model, &c->x86_mask);
@@ -687,10 +689,6 @@  static cpumask_t cpu_initialized;
 
 void __init early_cpu_init(void)
 {
-	intel_cpu_init();
-	amd_init_cpu();
-	centaur_init_cpu();
-	shanghai_init_cpu();
 	early_cpu_detect();
 }
 
diff --git a/xen/arch/x86/cpu/cpu.h b/xen/arch/x86/cpu/cpu.h
index edc88b1..62e4b03 100644
--- a/xen/arch/x86/cpu/cpu.h
+++ b/xen/arch/x86/cpu/cpu.h
@@ -6,7 +6,8 @@  struct cpu_dev {
 	void		(*c_init)(struct cpuinfo_x86 * c);
 };
 
-extern const struct cpu_dev *cpu_devs[X86_VENDOR_NUM];
+extern const struct cpu_dev intel_cpu_dev, amd_cpu_dev, centaur_cpu_dev,
+    shanghai_cpu_dev;
 
 extern bool_t opt_arat;
 extern unsigned int opt_cpuid_mask_ecx, opt_cpuid_mask_edx;
@@ -15,8 +16,3 @@  extern unsigned int opt_cpuid_mask_ext_ecx, opt_cpuid_mask_ext_edx;
 
 extern int get_model_name(struct cpuinfo_x86 *c);
 extern void display_cacheinfo(struct cpuinfo_x86 *c);
-
-int intel_cpu_init(void);
-int amd_init_cpu(void);
-int centaur_init_cpu(void);
-int shanghai_init_cpu(void);
diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index f9c2ec4..fcb3708 100644
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -348,17 +348,8 @@  static void init_intel(struct cpuinfo_x86 *c)
 		__set_bit(X86_FEATURE_ARAT, c->x86_capability);
 }
 
-static const struct cpu_dev intel_cpu_dev = {
+const struct cpu_dev intel_cpu_dev = {
 	.c_vendor	= "Intel",
 	.c_early_init	= early_init_intel,
 	.c_init		= init_intel,
 };
-
-int __init intel_cpu_init(void)
-{
-	cpu_devs[X86_VENDOR_INTEL] = &intel_cpu_dev;
-	return 0;
-}
-
-// arch_initcall(intel_cpu_init);
-
diff --git a/xen/arch/x86/cpu/shanghai.c b/xen/arch/x86/cpu/shanghai.c
index 24af5c8..189e13e 100644
--- a/xen/arch/x86/cpu/shanghai.c
+++ b/xen/arch/x86/cpu/shanghai.c
@@ -15,13 +15,7 @@  static void init_shanghai(struct cpuinfo_x86 *c)
     init_intel_cacheinfo(c);
 }
 
-static const struct cpu_dev shanghai_cpu_dev = {
+const struct cpu_dev shanghai_cpu_dev = {
     .c_vendor   = "  Shang",
     .c_init     = init_shanghai,
 };
-
-int __init shanghai_init_cpu(void)
-{
-    cpu_devs[X86_VENDOR_SHANGHAI] = &shanghai_cpu_dev;
-    return 0;
-}