[RFC,v2,08/35] x86/topology: Switch over to GENERIC_CPU_DEVICES

Commit Message

James Morse Sept. 13, 2023, 4:37 p.m. UTC

Now that GENERIC_CPU_DEVICES calls arch_register_cpu(), which can be
overridden by the arch code, switch over to this to allow common code
to choose when the register_cpu() call is made.

x86's struct cpus come from struct x86_cpu, which has no other members
or users. Remove this and use the version defined by common code.

This is an intermediate step to the logic being moved to drivers/acpi,
where GENERIC_CPU_DEVICES will do the work when booting with acpi=off.

Signed-off-by: James Morse <[e-mail]>
----
Changes since RFC:
 * Fixed the second copy of arch_register_cpu() used for non-hotplug
---
 arch/x86/Kconfig           |  1 +
 arch/x86/include/asm/cpu.h |  4 ----
 arch/x86/kernel/topology.c | 25 ++++++-------------------
 3 files changed, 7 insertions(+), 23 deletions(-)

Comments

Russell King (Oracle) Sept. 14, 2023, 10:01 a.m. UTC | #1

On Wed, Sep 13, 2023 at 04:37:56PM +0000, James Morse wrote:
> Now that GENERIC_CPU_DEVICES calls arch_register_cpu(), which can be
> overridden by the arch code, switch over to this to allow common code
> to choose when the register_cpu() call is made.
> 
> x86's struct cpus come from struct x86_cpu, which has no other members
> or users. Remove this and use the version defined by common code.
> 
> This is an intermediate step to the logic being moved to drivers/acpi,
> where GENERIC_CPU_DEVICES will do the work when booting with acpi=off.

I think it should also be noted that this moves the registration of
CPUs from subsys to driver core initialisation (before any other
initcalls are run.)

Jonathan Cameron Sept. 14, 2023, 11:40 a.m. UTC | #2

On Wed, 13 Sep 2023 16:37:56 +0000
James Morse <[e-mail]> wrote:

> Now that GENERIC_CPU_DEVICES calls arch_register_cpu(), which can be
> overridden by the arch code, switch over to this to allow common code
> to choose when the register_cpu() call is made.
> 
> x86's struct cpus come from struct x86_cpu, which has no other members
> or users. Remove this and use the version defined by common code.
> 
> This is an intermediate step to the logic being moved to drivers/acpi,
> where GENERIC_CPU_DEVICES will do the work when booting with acpi=off.
> 
> Signed-off-by: James Morse <[e-mail]>
> ----
> Changes since RFC:
>  * Fixed the second copy of arch_register_cpu() used for non-hotplug

Hi James,

See below for comment on this.  Upshot - I think you can delete that
function instead and rely on the weak version.

If you can't because of a later change, useful to call that out
in this patch description for those like me who read an review
in a linear fashion!

...

>  EXPORT_SYMBOL(arch_unregister_cpu);
>  #else /* CONFIG_HOTPLUG_CPU */
>  
> -int __init arch_register_cpu(int num)
> +int arch_register_cpu(int num)
>  {
> -	return register_cpu(&per_cpu(cpu_devices, num).cpu, num);
> +	return register_cpu(&per_cpu(cpu_devices, num), num);
>  }

Looks like the weak version introduced in patch 3.  Can this
implementation go away and fallback to that?

>  #endif /* CONFIG_HOTPLUG_CPU */
> -
> -static int __init topology_init(void)
> -{
> -	int i;
> -
> -	for_each_present_cpu(i)
> -		arch_register_cpu(i);
> -
> -	return 0;
> -}
> -subsys_initcall(topology_init);

Patch

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a0100a1ab4a0..133ea5f561b5 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -148,6 +148,7 @@  config X86
 	select GENERIC_CLOCKEVENTS_MIN_ADJUST
 	select GENERIC_CMOS_UPDATE
 	select GENERIC_CPU_AUTOPROBE
+	select GENERIC_CPU_DEVICES
 	select GENERIC_CPU_VULNERABILITIES
 	select GENERIC_EARLY_IOREMAP
 	select GENERIC_ENTRY
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 96dc4665e87d..f349c94510e8 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -23,10 +23,6 @@  static inline void prefill_possible_map(void) {}
 
 #endif /* CONFIG_SMP */
 
-struct x86_cpu {
-	struct cpu cpu;
-};
-
 #ifdef CONFIG_HOTPLUG_CPU
 extern void arch_unregister_cpu(int);
 extern void soft_restart_cpu(void);
diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index 0bab03130033..ca08a1d138f0 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -35,38 +35,25 @@ 
 #include <asm/io_apic.h>
 #include <asm/cpu.h>
 
-static DEFINE_PER_CPU(struct x86_cpu, cpu_devices);
-
 #ifdef CONFIG_HOTPLUG_CPU
 int arch_register_cpu(int cpu)
 {
-	struct x86_cpu *xc = per_cpu_ptr(&cpu_devices, cpu);
+	struct cpu *c = per_cpu_ptr(&cpu_devices, cpu);
 
-	xc->cpu.hotpluggable = cpu > 0;
-	return register_cpu(&xc->cpu, cpu);
+	c->hotpluggable = cpu > 0;
+	return register_cpu(c, cpu);
 }
 EXPORT_SYMBOL(arch_register_cpu);
 
 void arch_unregister_cpu(int num)
 {
-	unregister_cpu(&per_cpu(cpu_devices, num).cpu);
+	unregister_cpu(&per_cpu(cpu_devices, num));
 }
 EXPORT_SYMBOL(arch_unregister_cpu);
 #else /* CONFIG_HOTPLUG_CPU */
 
-int __init arch_register_cpu(int num)
+int arch_register_cpu(int num)
 {
-	return register_cpu(&per_cpu(cpu_devices, num).cpu, num);
+	return register_cpu(&per_cpu(cpu_devices, num), num);
 }
 #endif /* CONFIG_HOTPLUG_CPU */
-
-static int __init topology_init(void)
-{
-	int i;
-
-	for_each_present_cpu(i)
-		arch_register_cpu(i);
-
-	return 0;
-}
-subsys_initcall(topology_init);