Initial cleanups for vCPU hotplug

From: James Morse <[email protected]>

register_cpu_capacity_sysctl() adds a property to sysfs that describes the CPUs capacity. This is done from a subsys_initcall() that assumes all possible CPUs are registered.

With CPU hotplug, possible CPUs aren't registered until they become present, (or for arm64 enabled). This leads to messages during boot:
| register_cpu_capacity_sysctl: too early to get CPU1 device! and once these CPUs are added to the system, the file is missing.

Move this to a cpuhp callback, so that the file is created once CPUs are brought online. This covers CPUs that are added late by mechanisms like hotplug.
One observable difference is the file is now missing for offline CPUs.

Signed-off-by James Morse <[email protected]>

---
If the offline CPUs thing is a problem for the tools that consume this value, we'd need to move cpu_capacity to be part of cpu.c's common_cpu_attr_groups.

---
  drivers/base/arch_topology.c | 38 ++++++++++++++++++++++++------------
  1 file changed, 26 insertions(+), 12 deletions(-)

From: James Morse <[email protected]>

register_cpu_capacity_sysctl() adds a property to sysfs that describes the CPUs capacity. This is done from a subsys_initcall() that assumes all possible CPUs are registered.

With CPU hotplug, possible CPUs aren't registered until they become present, (or for arm64 enabled). This leads to messages during boot:
| register_cpu_capacity_sysctl: too early to get CPU1 device! and once these CPUs are added to the system, the file is missing.

Move this to a cpuhp callback, so that the file is created once CPUs are brought online. This covers CPUs that are added late by mechanisms like hotplug.
One observable difference is the file is now missing for offline CPUs.

Signed-off-by James Morse <[email protected]>

---
If the offline CPUs thing is a problem for the tools that consume this value, we'd need to move cpu_capacity to be part of cpu.c's common_cpu_attr_groups.

Quoted 16 lines · On Tue, 07 Nov 2023 10:29:23 +0000 Russell King <[email protected]>…

From: James Morse <[email protected]>

register_cpu_capacity_sysctl() adds a property to sysfs that describes the CPUs capacity. This is done from a subsys_initcall() that assumes all possible CPUs are registered.

With CPU hotplug, possible CPUs aren't registered until they become present, (or for arm64 enabled). This leads to messages during boot:
| register_cpu_capacity_sysctl: too early to get CPU1 device! and once these CPUs are added to the system, the file is missing.

Move this to a cpuhp callback, so that the file is created once CPUs are brought online. This covers CPUs that are added late by mechanisms like hotplug.
One observable difference is the file is now missing for offline CPUs.

Quoted 5 lines

Signed-off-by James Morse <[email protected]>

---
If the offline CPUs thing is a problem for the tools that consume this value, we'd need to move cpu_capacity to be part of cpu.c's common_cpu_attr_groups.

I'm not keen on squirting sysfs files in from code so
might be nice to do that anyway and use is_visible() / sysfs_update_group() but that would be a job for another day if at all.

Looking into doing this, the easy bit is adding the attribute group with an appropriate .is_visible dependent on cpu_present(), but we need to be able to call sysfs_update_groups() when the state of the .is_visible() changes.

Given the comment in sysfs_update_groups() about "if an error occurs", rather than making this part of common_cpu_attr_groups, would it be better that it's part of its own set of groups, thus limiting the damage from a possible error? I suspect, however, that any error at that point means that the system is rather fatally wounded.

This is what I have so far to implement your idea, less the necessary sysfs_update_groups() call when we need to change the visibility of the attributes.

From: James Morse <[email protected]>

intel_epb_init() is called as a subsys_initcall() to register cpuhp callbacks. The callbacks make use of get_cpu_device() which will return NULL unless register_cpu() has been called. register_cpu() is called from topology_init(), which is also a subsys_initcall().

This is fragile. Moving the register_cpu() to a different subsys_initcall() leads to a NULL dereference during boot.

Make intel_epb_init() a late_initcall(), user-space can't provide a policy before this point anyway.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
subsys_initcall_sync() would be an option, but moving the register_cpu() calls into ACPI also means adding a safety net for CPUs that are online but not described properly by firmware. This lives in subsys_initcall_sync().

---
  arch/x86/kernel/cpu/intel_epb.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

From: James Morse <[email protected]>

intel_epb_init() is called as a subsys_initcall() to register cpuhp callbacks. The callbacks make use of get_cpu_device() which will return NULL unless register_cpu() has been called. register_cpu() is called from topology_init(), which is also a subsys_initcall().

This is fragile. Moving the register_cpu() to a different subsys_initcall() leads to a NULL dereference during boot.

Make intel_epb_init() a late_initcall(), user-space can't provide a policy before this point anyway.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
subsys_initcall_sync() would be an option, but moving the register_cpu() calls into ACPI also means adding a safety net for CPUs that are online but not described properly by firmware. This lives in subsys_initcall_sync().

---
 arch/x86/kernel/cpu/intel_epb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/kernel/cpu/intel_epb.c +1 -1

index e4c3ba91321c..f18d35fe27a9 100644
@@ -237,4 +237,4 @@ static __init int intel_epb_init(void)
 	cpuhp_remove_state(CPUHP_AP_X86_INTEL_EPB_ONLINE);
 	return ret;
 }
-subsys_initcall(intel_epb_init);
+late_initcall(intel_epb_init);

Quoted 10 lines · On Tue, 07 Nov 2023 10:29:28 +0000 Russell King <[email protected]>…

From: James Morse <[email protected]>

intel_epb_init() is called as a subsys_initcall() to register cpuhp callbacks. The callbacks make use of get_cpu_device() which will return NULL unless register_cpu() has been called. register_cpu() is called from topology_init(), which is also a subsys_initcall().

This is fragile. Moving the register_cpu() to a different subsys_initcall() leads to a NULL dereference during boot.

Quoted 5 lines

Make intel_epb_init() a late_initcall(), user-space can't provide a policy before this point anyway.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

Seems reasonable. FWIW

Reviewed-by Jonathan Cameron <[email protected]>

arch_register_cpu() and arch_unregister_cpu() are not used by anything that can be a module - they are used by drivers/base/cpu.c and drivers/acpi/acpi_processor.c, neither of which can be a module.

Remove the exports.

Signed-off-by Russell King (Oracle) <[email protected]>

---
  arch/x86/kernel/topology.c | 2 --
  1 file changed, 2 deletions(-)

arch_register_cpu() and arch_unregister_cpu() are not used by anything that can be a module - they are used by drivers/base/cpu.c and drivers/acpi/acpi_processor.c, neither of which can be a module.

Remove the exports.

Signed-off-by Russell King (Oracle) <[email protected]>

arch_register_cpu() and arch_unregister_cpu() are not used by anything that can be a module - they are used by drivers/base/cpu.c and drivers/acpi/acpi_processor.c, neither of which can be a module.

Remove the exports.

Signed-off-by Russell King (Oracle) <[email protected]>

---
  arch/loongarch/kernel/topology.c | 2 --
  1 file changed, 2 deletions(-)

arch_register_cpu() and arch_unregister_cpu() are not used by anything that can be a module - they are used by drivers/base/cpu.c and drivers/acpi/acpi_processor.c, neither of which can be a module.

Remove the exports.

Signed-off-by Russell King (Oracle) <[email protected]>

From: James Morse <[email protected]>

Neither arm64 nor riscv support physical hotadd of CPUs that were not present at boot. For arm64 much of the platform description is in static tables which do not have update methods. arm64 does support HOTPLUG_CPU, which is backed by a firmware interface to turn CPUs on and off.

acpi_processor_hotadd_init() and acpi_processor_remove() are for adding and removing CPUs that were not present at boot. arm64 systems that do this are not supported as there is currently insufficient information in the platform description. (e.g. did the GICR get removed too?)

arm64 currently relies on the MADT enabled flag check in map_gicc_mpidr() to prevent CPUs that were not described as present at boot from being added to the system. Similarly, riscv relies on the same check in map_rintc_hartid(). Both architectures also rely on the weak 'always fails' definitions of acpi_map_cpu() and arch_register_cpu().

Subsequent changes will redefine ACPI_HOTPLUG_CPU as making possible CPUs present. Neither arm64 nor riscv support this.

Disable ACPI_HOTPLUG_CPU for arm64 and riscv by removing 'default y' and selecting it on the other three ACPI architectures. This allows the weak definitions of some symbols to be removed.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC:
* Expanded conditions to avoid ACPI_HOTPLUG_CPU being enabled when HOTPLUG_CPU isn't.
Changes since RFC v3:
* Dropped ia64 changes

---
  arch/loongarch/Kconfig        |  1 +
  arch/x86/Kconfig              |  1 +
  drivers/acpi/Kconfig          |  1 -
  drivers/acpi/acpi_processor.c | 18 ------------------
  4 files changed, 2 insertions(+), 19 deletions(-)

arch/loongarch/Kconfig +1 -0

index d889a0b97bc1..64620e90c12c 100644
@@ -5,6 +5,7 @@ config LOONGARCH
  	select ACPI
  	select ACPI_GENERIC_GSI if ACPI
  	select ACPI_MCFG if ACPI
+	select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU
  	select ACPI_PPTT if ACPI
  	select ACPI_SYSTEM_POWER_STATES_SUPPORT	if ACPI
  	select ARCH_BINFMT_ELF_STATE

arch/x86/Kconfig +1 -0

index 3762f41bb092..dbdcfc708369 100644
@@ -59,6 +59,7 @@ config X86
  	#
  	select ACPI_LEGACY_TABLES_LOOKUP	if ACPI
  	select ACPI_SYSTEM_POWER_STATES_SUPPORT	if ACPI
+	select ACPI_HOTPLUG_CPU			if ACPI_PROCESSOR && HOTPLUG_CPU
  	select ARCH_32BIT_OFF_T			if X86_32
  	select ARCH_CLOCKSOURCE_INIT
  	select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE

drivers/acpi/Kconfig +0 -1

index f819e760ff19..a3acfc750fce 100644
@@ -310,7 +310,6 @@ config ACPI_HOTPLUG_CPU
  	bool
  	depends on ACPI_PROCESSOR && HOTPLUG_CPU
  	select ACPI_CONTAINER
-	default y
 
  config ACPI_PROCESSOR_AGGREGATOR
  	tristate "Processor Aggregator"

drivers/acpi/acpi_processor.c +0 -18

index 0f5218e361df..4fe2ef54088c 100644
@@ -184,24 +184,6 @@ static void __init acpi_pcc_cpufreq_init(void) {}
 
  /* Initialization */
  #ifdef CONFIG_ACPI_HOTPLUG_CPU
-int __weak acpi_map_cpu(acpi_handle handle,
-		phys_cpuid_t physid, u32 acpi_id, int *pcpu)
-{
-	return -ENODEV;
-}
-
-int __weak acpi_unmap_cpu(int cpu)
-{
-	return -ENODEV;
-}
-
-int __weak arch_register_cpu(int cpu)
-{
-	return -ENODEV;
-}
-
-void __weak arch_unregister_cpu(int cpu) {}
-
  static int acpi_processor_hotadd_init(struct acpi_processor *pr)
  {
  	unsigned long long sta;

From: James Morse <[email protected]>

Neither arm64 nor riscv support physical hotadd of CPUs that were not present at boot. For arm64 much of the platform description is in static tables which do not have update methods. arm64 does support HOTPLUG_CPU, which is backed by a firmware interface to turn CPUs on and off.

acpi_processor_hotadd_init() and acpi_processor_remove() are for adding and removing CPUs that were not present at boot. arm64 systems that do this are not supported as there is currently insufficient information in the platform description. (e.g. did the GICR get removed too?)

arm64 currently relies on the MADT enabled flag check in map_gicc_mpidr() to prevent CPUs that were not described as present at boot from being added to the system. Similarly, riscv relies on the same check in map_rintc_hartid(). Both architectures also rely on the weak 'always fails' definitions of acpi_map_cpu() and arch_register_cpu().

Subsequent changes will redefine ACPI_HOTPLUG_CPU as making possible CPUs present. Neither arm64 nor riscv support this.

Disable ACPI_HOTPLUG_CPU for arm64 and riscv by removing 'default y' and selecting it on the other three ACPI architectures. This allows the weak definitions of some symbols to be removed.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC:
* Expanded conditions to avoid ACPI_HOTPLUG_CPU being enabled when HOTPLUG_CPU isn't.
Changes since RFC v3:
* Dropped ia64 changes

---
  arch/loongarch/Kconfig        |  1 +
  arch/x86/Kconfig              |  1 +
  drivers/acpi/Kconfig          |  1 -
  drivers/acpi/acpi_processor.c | 18 ------------------
  4 files changed, 2 insertions(+), 19 deletions(-)

arch/loongarch/Kconfig +1 -0

index d889a0b97bc1..64620e90c12c 100644
@@ -5,6 +5,7 @@ config LOONGARCH
  	select ACPI
  	select ACPI_GENERIC_GSI if ACPI
  	select ACPI_MCFG if ACPI
+	select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU
  	select ACPI_PPTT if ACPI
  	select ACPI_SYSTEM_POWER_STATES_SUPPORT	if ACPI
  	select ARCH_BINFMT_ELF_STATE

arch/x86/Kconfig +1 -0

index 3762f41bb092..dbdcfc708369 100644
@@ -59,6 +59,7 @@ config X86
  	#
  	select ACPI_LEGACY_TABLES_LOOKUP	if ACPI
  	select ACPI_SYSTEM_POWER_STATES_SUPPORT	if ACPI
+	select ACPI_HOTPLUG_CPU			if ACPI_PROCESSOR && HOTPLUG_CPU
  	select ARCH_32BIT_OFF_T			if X86_32
  	select ARCH_CLOCKSOURCE_INIT
  	select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE

drivers/acpi/Kconfig +0 -1

index f819e760ff19..a3acfc750fce 100644
@@ -310,7 +310,6 @@ config ACPI_HOTPLUG_CPU
  	bool
  	depends on ACPI_PROCESSOR && HOTPLUG_CPU
  	select ACPI_CONTAINER
-	default y
 
  config ACPI_PROCESSOR_AGGREGATOR
  	tristate "Processor Aggregator"

drivers/acpi/acpi_processor.c +0 -8

index 0f5218e361df..4fe2ef54088c 100644
@@ -184,24 +184,6 @@ static void __init acpi_pcc_cpufreq_init(void) {}
 
  /* Initialization */
  #ifdef CONFIG_ACPI_HOTPLUG_CPU
-int __weak acpi_map_cpu(acpi_handle handle,
-		phys_cpuid_t physid, u32 acpi_id, int *pcpu)
-{
-	return -ENODEV;
-}
-
-int __weak acpi_unmap_cpu(int cpu)
-{

Quoted 33 lines · Russell King (Oracle)

From: James Morse <[email protected]>

Neither arm64 nor riscv support physical hotadd of CPUs that were not present at boot. For arm64 much of the platform description is in static tables which do not have update methods. arm64 does support HOTPLUG_CPU, which is backed by a firmware interface to turn CPUs on and off.

acpi_processor_hotadd_init() and acpi_processor_remove() are for adding and removing CPUs that were not present at boot. arm64 systems that do this are not supported as there is currently insufficient information in the platform description. (e.g. did the GICR get removed too?)

arm64 currently relies on the MADT enabled flag check in map_gicc_mpidr() to prevent CPUs that were not described as present at boot from being added to the system. Similarly, riscv relies on the same check in map_rintc_hartid(). Both architectures also rely on the weak 'always fails' definitions of acpi_map_cpu() and arch_register_cpu().

Subsequent changes will redefine ACPI_HOTPLUG_CPU as making possible CPUs present. Neither arm64 nor riscv support this.

Disable ACPI_HOTPLUG_CPU for arm64 and riscv by removing 'default y' and selecting it on the other three ACPI architectures. This allows the weak definitions of some symbols to be removed.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC:
* Expanded conditions to avoid ACPI_HOTPLUG_CPU being enabled when HOTPLUG_CPU isn't.
Changes since RFC v3:
* Dropped ia64 changes

Quoted 7 lines

---
  arch/loongarch/Kconfig        |  1 +
  arch/x86/Kconfig              |  1 +
  drivers/acpi/Kconfig          |  1 -
  drivers/acpi/acpi_processor.c | 18 ------------------
  4 files changed, 2 insertions(+), 19 deletions(-)

With the following nits addressed:

Reviewed-by Gavin Shan <[email protected]>

Quoted 55 lines

arch/loongarch/Kconfig +1 -0

index d889a0b97bc1..64620e90c12c 100644
@@ -5,6 +5,7 @@ config LOONGARCH
  	select ACPI
  	select ACPI_GENERIC_GSI if ACPI
  	select ACPI_MCFG if ACPI
+	select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU
  	select ACPI_PPTT if ACPI
  	select ACPI_SYSTEM_POWER_STATES_SUPPORT	if ACPI
  	select ARCH_BINFMT_ELF_STATE

arch/x86/Kconfig +1 -0

index 3762f41bb092..dbdcfc708369 100644
@@ -59,6 +59,7 @@ config X86
  	#
  	select ACPI_LEGACY_TABLES_LOOKUP	if ACPI
  	select ACPI_SYSTEM_POWER_STATES_SUPPORT	if ACPI
+	select ACPI_HOTPLUG_CPU			if ACPI_PROCESSOR && HOTPLUG_CPU
  	select ARCH_32BIT_OFF_T			if X86_32
  	select ARCH_CLOCKSOURCE_INIT
  	select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE

drivers/acpi/Kconfig +0 -1

index f819e760ff19..a3acfc750fce 100644
@@ -310,7 +310,6 @@ config ACPI_HOTPLUG_CPU
  	bool
  	depends on ACPI_PROCESSOR && HOTPLUG_CPU
  	select ACPI_CONTAINER
-	default y
  config ACPI_PROCESSOR_AGGREGATOR
  	tristate "Processor Aggregator"

drivers/acpi/acpi_processor.c +0 -13

index 0f5218e361df..4fe2ef54088c 100644
@@ -184,24 +184,6 @@ static void __init acpi_pcc_cpufreq_init(void) {}
  /* Initialization */
  #ifdef CONFIG_ACPI_HOTPLUG_CPU
-int __weak acpi_map_cpu(acpi_handle handle,
-		phys_cpuid_t physid, u32 acpi_id, int *pcpu)
-{
-	return -ENODEV;
-}
-
-int __weak acpi_unmap_cpu(int cpu)
-{
-	return -ENODEV;
-}
-
-int __weak arch_register_cpu(int cpu)
-{

drivers/acpi/acpi_processor.c static void __init acpi_pcc_cpufreq_init(void) {} · line 204

Quoted 5 lines

drivers/acpi/acpi_processor.c +0 -5

-	return -ENODEV;
-}
-
-void __weak arch_unregister_cpu(int cpu) {}
-

Since we're here, EXPORT_SYMBOL() can be dropped for these functions on x86 and loongarch because they're not called from a module?

Quoted 101 lines · Gavin Shan

Quoted 33 lines · Russell King (Oracle)

From: James Morse <[email protected]>

Neither arm64 nor riscv support physical hotadd of CPUs that were not present at boot. For arm64 much of the platform description is in static tables which do not have update methods. arm64 does support HOTPLUG_CPU, which is backed by a firmware interface to turn CPUs on and off.

acpi_processor_hotadd_init() and acpi_processor_remove() are for adding and removing CPUs that were not present at boot. arm64 systems that do this are not supported as there is currently insufficient information in the platform description. (e.g. did the GICR get removed too?)

arm64 currently relies on the MADT enabled flag check in map_gicc_mpidr() to prevent CPUs that were not described as present at boot from being added to the system. Similarly, riscv relies on the same check in map_rintc_hartid(). Both architectures also rely on the weak 'always fails' definitions of acpi_map_cpu() and arch_register_cpu().

Subsequent changes will redefine ACPI_HOTPLUG_CPU as making possible CPUs present. Neither arm64 nor riscv support this.

Disable ACPI_HOTPLUG_CPU for arm64 and riscv by removing 'default y' and selecting it on the other three ACPI architectures. This allows the weak definitions of some symbols to be removed.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC:
* Expanded conditions to avoid ACPI_HOTPLUG_CPU being enabled when HOTPLUG_CPU isn't.
Changes since RFC v3:
* Dropped ia64 changes

Quoted 7 lines

---
   arch/loongarch/Kconfig        |  1 +
   arch/x86/Kconfig              |  1 +
   drivers/acpi/Kconfig          |  1 -
   drivers/acpi/acpi_processor.c | 18 ------------------
   4 files changed, 2 insertions(+), 19 deletions(-)

With the following nits addressed:

Reviewed-by Gavin Shan <[email protected]>

Quoted 55 lines

arch/loongarch/Kconfig +1 -0

index d889a0b97bc1..64620e90c12c 100644
@@ -5,6 +5,7 @@ config LOONGARCH
   	select ACPI
   	select ACPI_GENERIC_GSI if ACPI
   	select ACPI_MCFG if ACPI
+	select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU
   	select ACPI_PPTT if ACPI
   	select ACPI_SYSTEM_POWER_STATES_SUPPORT	if ACPI
   	select ARCH_BINFMT_ELF_STATE

arch/x86/Kconfig +1 -0

index 3762f41bb092..dbdcfc708369 100644
@@ -59,6 +59,7 @@ config X86
   	#
   	select ACPI_LEGACY_TABLES_LOOKUP	if ACPI
   	select ACPI_SYSTEM_POWER_STATES_SUPPORT	if ACPI
+	select ACPI_HOTPLUG_CPU			if ACPI_PROCESSOR && HOTPLUG_CPU
   	select ARCH_32BIT_OFF_T			if X86_32
   	select ARCH_CLOCKSOURCE_INIT
   	select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE

drivers/acpi/Kconfig +0 -1

index f819e760ff19..a3acfc750fce 100644
@@ -310,7 +310,6 @@ config ACPI_HOTPLUG_CPU
   	bool
   	depends on ACPI_PROCESSOR && HOTPLUG_CPU
   	select ACPI_CONTAINER
-	default y
   config ACPI_PROCESSOR_AGGREGATOR
   	tristate "Processor Aggregator"

drivers/acpi/acpi_processor.c +0 -13

index 0f5218e361df..4fe2ef54088c 100644
@@ -184,24 +184,6 @@ static void __init acpi_pcc_cpufreq_init(void) {}
   /* Initialization */
   #ifdef CONFIG_ACPI_HOTPLUG_CPU
-int __weak acpi_map_cpu(acpi_handle handle,
-		phys_cpuid_t physid, u32 acpi_id, int *pcpu)
-{
-	return -ENODEV;
-}
-
-int __weak acpi_unmap_cpu(int cpu)
-{
-	return -ENODEV;
-}
-
-int __weak arch_register_cpu(int cpu)
-{

Quoted 8 lines

drivers/acpi/acpi_processor.c static void __init acpi_pcc_cpufreq_init(void) {} · line 204

Quoted 5 lines

drivers/acpi/acpi_processor.c +0 -5

-	return -ENODEV;
-}
-
-void __weak arch_unregister_cpu(int cpu) {}
-

Since we're here, EXPORT_SYMBOL() can be dropped for these functions on x86 and loongarch because they're not called from a module?

I'm confused, and don't understand your comment. You've r-b the previous two patches that remove the EXPORT_SYMBOL()s for arch_register_cpu() and arch_unregister_cpu() from x86 and loongarch. So it seems your comment is already addressed, and thus makes no sense.

Please clarify.

From: James Morse <[email protected]>

Neither arm64 nor riscv support physical hotadd of CPUs that were not present at boot. For arm64 much of the platform description is in static tables which do not have update methods. arm64 does support HOTPLUG_CPU, which is backed by a firmware interface to turn CPUs on and off.

acpi_processor_hotadd_init() and acpi_processor_remove() are for adding and removing CPUs that were not present at boot. arm64 systems that do this are not supported as there is currently insufficient information in the platform description. (e.g. did the GICR get removed too?)

arm64 currently relies on the MADT enabled flag check in map_gicc_mpidr() to prevent CPUs that were not described as present at boot from being added to the system. Similarly, riscv relies on the same check in map_rintc_hartid(). Both architectures also rely on the weak 'always fails' definitions of acpi_map_cpu() and arch_register_cpu().

Subsequent changes will redefine ACPI_HOTPLUG_CPU as making possible CPUs present. Neither arm64 nor riscv support this.

Disable ACPI_HOTPLUG_CPU for arm64 and riscv by removing 'default y' and selecting it on the other three ACPI architectures. This allows the weak definitions of some symbols to be removed.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

From: James Morse <[email protected]>

Three of the five ACPI architectures create sysfs entries using register_cpu() for present CPUs, whereas arm64, riscv and all GENERIC_CPU_DEVICES do this for possible CPUs.

Registering a CPU is what causes them to show up in sysfs.

It makes very little sense to register all possible CPUs. Registering a CPU is what triggers the udev notifications allowing user-space to react to newly added CPUs.

To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change it to use for_each_present_cpu(). Making the ACPI architectures use GENERIC_CPU_DEVICES is a pre-requisite step to centralise their cpu_register() logic, before moving it into the ACPI processor driver. When ACPI is disabled this work would be done by cpu_dev_register_generic().

Of the ACPI architectures that register possible CPUs, arm64 and riscv do not support making possible CPUs present as they use the weak 'always fails' version of arch_register_cpu().

Only two of the eight architectures that use GENERIC_CPU_DEVICES have a distinction between present and possible CPUs.

The following architectures use GENERIC_CPU_DEVICES but are not SMP, so possible == present:
* m68k
* microblaze
* nios2

The following architectures use GENERIC_CPU_DEVICES and consider possible == present:
* csky: setup_smp()
* processor_probe() sets possible for all CPUs and present for all CPUs except the boot cpu, which will have been done by init/main.c::start_kernel().

um appears to be a subarchitecture of x86.

The remaining architecture using GENERIC_CPU_DEVICES are:
* openrisc and hexagon:
where smp_init_cpus() makes all CPUs < NR_CPUS possible, whereas smp_prepare_cpus() only makes CPUs < setup_max_cpus present.

After this change, openrisc and hexagon systems that use the max_cpus command line argument would not see the other CPUs present in sysfs. This should not be a problem as these CPUs can't bre brought online as

_cpu_up() checks cpu_present().

After this change, only CPUs which are present appear in sysfs.

Signed-off-by James Morse <[email protected]>

Reviewed-by Jonathan Cameron <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
  drivers/base/cpu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

drivers/base/cpu.c +1 -1

index 9ea22e165acd..34b48f660b6b 100644
@@ -533,7 +533,7 @@ static void __init cpu_dev_register_generic(void)
  #ifdef CONFIG_GENERIC_CPU_DEVICES
  	int i;
 
-	for_each_possible_cpu(i) {
+	for_each_present_cpu(i) {
  		if (register_cpu(&per_cpu(cpu_devices, i), i))
  			panic("Failed to register CPU device");
  	}

Hi Russell,

Quoted 12 lines · Russell King (Oracle)

From: James Morse <[email protected]>

Three of the five ACPI architectures create sysfs entries using register_cpu() for present CPUs, whereas arm64, riscv and all GENERIC_CPU_DEVICES do this for possible CPUs.

Registering a CPU is what causes them to show up in sysfs.

It makes very little sense to register all possible CPUs. Registering a CPU is what triggers the udev notifications allowing user-space to react to newly added CPUs.

Quoted 6 lines

To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change it to use for_each_present_cpu(). Making the ACPI architectures use GENERIC_CPU_DEVICES is a pre-requisite step to centralise their cpu_register() logic, before moving it into the ACPI processor driver. When ACPI is disabled this work would be done by cpu_dev_register_generic().

What do you actually mean about when ACPI is disabled this work would be

done by cpu_dev_register_generic()? Is the work means register the cpu?

I'm not quite understand that, and how about when ACPI is enabled, which function do this work?

Quoted 3 lines

After this change, openrisc and hexagon systems that use the max_cpus command line argument would not see the other CPUs present in sysfs. This should not be a problem as these CPUs can't bre brought online as

^ nit: can't be

Quoted 22 lines · Shaoqin Huang

Hi Russell,

Quoted 18 lines · Russell King (Oracle)

From: James Morse <[email protected]>

Three of the five ACPI architectures create sysfs entries using register_cpu() for present CPUs, whereas arm64, riscv and all GENERIC_CPU_DEVICES do this for possible CPUs.

Registering a CPU is what causes them to show up in sysfs.

It makes very little sense to register all possible CPUs. Registering a CPU is what triggers the udev notifications allowing user-space to react to newly added CPUs.

To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change it to use for_each_present_cpu(). Making the ACPI architectures use GENERIC_CPU_DEVICES is a pre-requisite step to centralise their cpu_register() logic, before moving it into the ACPI processor driver. When ACPI is disabled this work would be done by cpu_dev_register_generic().

Quoted 1 line

What do you actually mean about when ACPI is disabled this work would be

Firstly, please note that "you" is not appropriate here. This is James' commit message, not mine.

Quoted 1 line

done by cpu_dev_register_generic()? Is the work means register the cpu?

When ACPI is disabled _and_ CONFIG_GENERIC_CPU_DEVICES is enabled, then cpu_dev_register_generic() will call arch_register_cpu() for each present CPU after this commit, rather than for each _possible_ CPU (which is the actual code change here.)

Quoted 2 lines

I'm not quite understand that, and how about when ACPI is enabled, which function do this work?

This is what happens later in the series.

"drivers: base: Allow parts of GENERIC_CPU_DEVICES to be overridden" adds a test for CONFIG_GENERIC_CPU_DEVICES, so this will only be used with architectures using GENERIC_CPU_DEVICES. Then in:

"ACPI: processor: Register all CPUs from acpi_processor_get_info()" which is not part of this series, this adds a call to arch_register_cpu() in the ACPI code, and disables this path via a test for !acpi_disabled.

Essentially, this path gets used to register the present CPUs when firmware (ACPI) isn't going to be registering the present CPUs.

I've changed this to:

"It makes very little sense to register all possible CPUs. Registering a CPU is what triggers the udev notifications allowing user-space to react to newly added CPUs.

"To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change it to use for_each_present_cpu().

"Making the ACPI architectures use GENERIC_CPU_DEVICES is a pre-requisite step to centralise their register_cpu() logic, before moving it into the ACPI processor driver. When we add support for register CPUs from ACPI in a later patch, we will avoid registering CPUs in this path."

which I hope makes it clearer.

Quoted 4 lines

Quoted 3 lines

After this change, openrisc and hexagon systems that use the max_cpus command line argument would not see the other CPUs present in sysfs. This should not be a problem as these CPUs can't bre brought online as

^ nit: can't be

Thanks, I'll fix that.

From: James Morse <[email protected]>

Three of the five ACPI architectures create sysfs entries using register_cpu() for present CPUs, whereas arm64, riscv and all GENERIC_CPU_DEVICES do this for possible CPUs.

Registering a CPU is what causes them to show up in sysfs.

It makes very little sense to register all possible CPUs. Registering a CPU is what triggers the udev notifications allowing user-space to react to newly added CPUs.

To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change it to use for_each_present_cpu(). Making the ACPI architectures use GENERIC_CPU_DEVICES is a pre-requisite step to centralise their cpu_register() logic, before moving it into the ACPI processor driver. When ACPI is disabled this work would be done by cpu_dev_register_generic().

Of the ACPI architectures that register possible CPUs, arm64 and riscv do not support making possible CPUs present as they use the weak 'always fails' version of arch_register_cpu().

Only two of the eight architectures that use GENERIC_CPU_DEVICES have a distinction between present and possible CPUs.

The following architectures use GENERIC_CPU_DEVICES but are not SMP, so possible == present:
* m68k
* microblaze
* nios2

The following architectures use GENERIC_CPU_DEVICES and consider possible == present:
* csky: setup_smp()
* processor_probe() sets possible for all CPUs and present for all CPUs except the boot cpu, which will have been done by init/main.c::start_kernel().

um appears to be a subarchitecture of x86.

The remaining architecture using GENERIC_CPU_DEVICES are:
* openrisc and hexagon:
where smp_init_cpus() makes all CPUs < NR_CPUS possible, whereas smp_prepare_cpus() only makes CPUs < setup_max_cpus present.

After this change, openrisc and hexagon systems that use the max_cpus command line argument would not see the other CPUs present in sysfs. This should not be a problem as these CPUs can't bre brought online as _cpu_up() checks cpu_present().

After this change, only CPUs which are present appear in sysfs.

Signed-off-by James Morse <[email protected]>

Reviewed-by Jonathan Cameron <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
  drivers/base/cpu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

From: James Morse <[email protected]>

Architectures often have extra per-cpu work that needs doing before a CPU is registered, often to determine if a CPU is hotpluggable.

To allow the ACPI architectures to use GENERIC_CPU_DEVICES, move the cpu_register() call into arch_register_cpu(), which is made __weak so architectures with extra work can override it.
This aligns with the way x86, ia64 and loongarch register hotplug CPUs when they become present.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC:
* Dropped __init from x86/ia64 arch_register_cpu()
Changes since RFC v2:
* Dropped unnecessary Loongarch asm/cpu.h changes

---
  drivers/base/cpu.c  | 14 ++++++++++----
  include/linux/cpu.h |  4 ++++
  2 files changed, 14 insertions(+), 4 deletions(-)

drivers/base/cpu.c +10 -4

index 34b48f660b6b..579064fda97b 100644
@@ -525,19 +525,25 @@ bool cpu_is_hotpluggable(unsigned int cpu)
  EXPORT_SYMBOL_GPL(cpu_is_hotpluggable);
 
  #ifdef CONFIG_GENERIC_CPU_DEVICES
-static DEFINE_PER_CPU(struct cpu, cpu_devices);
+DEFINE_PER_CPU(struct cpu, cpu_devices);
+
+int __weak arch_register_cpu(int cpu)
+{
+	return register_cpu(&per_cpu(cpu_devices, cpu), cpu);
+}
  #endif
 
  static void __init cpu_dev_register_generic(void)
  {
-#ifdef CONFIG_GENERIC_CPU_DEVICES
  	int i;
 
+	if (!IS_ENABLED(CONFIG_GENERIC_CPU_DEVICES))
+		return;
+
  	for_each_present_cpu(i) {
-		if (register_cpu(&per_cpu(cpu_devices, i), i))
+		if (arch_register_cpu(i))
  			panic("Failed to register CPU device");
  	}
-#endif
  }
 
  #ifdef CONFIG_GENERIC_CPU_VULNERABILITIES

include/linux/cpu.h +4 -0

index fc8094419084..1e982d63eae8 100644
@@ -88,6 +88,10 @@ extern ssize_t arch_cpu_probe(const char *, size_t);
  extern ssize_t arch_cpu_release(const char *, size_t);
  #endif
 
+#ifdef CONFIG_GENERIC_CPU_DEVICES
+DECLARE_PER_CPU(struct cpu, cpu_devices);
+#endif
+
  /*
   * These states are not related to the core CPU hotplug mechanism. They are
   * used by various (sub)architectures to track internal state

From: James Morse <[email protected]>

Architectures often have extra per-cpu work that needs doing before a CPU is registered, often to determine if a CPU is hotpluggable.

To allow the ACPI architectures to use GENERIC_CPU_DEVICES, move the cpu_register() call into arch_register_cpu(), which is made __weak so architectures with extra work can override it.
This aligns with the way x86, ia64 and loongarch register hotplug CPUs when they become present.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC:
* Dropped __init from x86/ia64 arch_register_cpu()
Changes since RFC v2:
* Dropped unnecessary Loongarch asm/cpu.h changes

---
  drivers/base/cpu.c  | 14 ++++++++++----
  include/linux/cpu.h |  4 ++++
  2 files changed, 14 insertions(+), 4 deletions(-)

From: James Morse <[email protected]>

Architectures often have extra per-cpu work that needs doing before a CPU is registered, often to determine if a CPU is hotpluggable.

To allow the ACPI architectures to use GENERIC_CPU_DEVICES, move the cpu_register() call into arch_register_cpu(), which is made __weak so architectures with extra work can override it.
This aligns with the way x86, ia64 and loongarch register hotplug CPUs when they become present.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

From: James Morse <[email protected]>

Add arch_unregister_cpu() to allow the ACPI machinery to call unregister_cpu(). This is enough for arm64, riscv and loongarch, but needs to be overridden by x86 and ia64 who need to do more work.

CC Jean-Philippe Brucker <[email protected]>

Signed-off-by James Morse <[email protected]>

---
Changes since v1:
* Added CONFIG_HOTPLUG_CPU ifdeffery around unregister_cpu
Changes since RFC v2:
* Move earlier in the series

---
  drivers/base/cpu.c | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)

drivers/base/cpu.c +8 -1

index 579064fda97b..58bb86091b34 100644
@@ -531,7 +531,14 @@ int __weak arch_register_cpu(int cpu)
  {
  	return register_cpu(&per_cpu(cpu_devices, cpu), cpu);
  }
-#endif
+
+#ifdef CONFIG_HOTPLUG_CPU
+void __weak arch_unregister_cpu(int num)
+{
+	unregister_cpu(&per_cpu(cpu_devices, num));
+}
+#endif /* CONFIG_HOTPLUG_CPU */
+#endif /* CONFIG_GENERIC_CPU_DEVICES */
 
  static void __init cpu_dev_register_generic(void)
  {

From: James Morse <[email protected]>

Add arch_unregister_cpu() to allow the ACPI machinery to call unregister_cpu(). This is enough for arm64, riscv and loongarch, but needs to be overridden by x86 and ia64 who need to do more work.

CC Jean-Philippe Brucker <[email protected]>

Signed-off-by James Morse <[email protected]>

---
Changes since v1:
* Added CONFIG_HOTPLUG_CPU ifdeffery around unregister_cpu
Changes since RFC v2:
* Move earlier in the series

---
  drivers/base/cpu.c | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)

From: James Morse <[email protected]>

Add arch_unregister_cpu() to allow the ACPI machinery to call unregister_cpu(). This is enough for arm64, riscv and loongarch, but needs to be overridden by x86 and ia64 who need to do more work.

CC Jean-Philippe Brucker <[email protected]>

Signed-off-by James Morse <[email protected]>

---
Changes since v1:
* Added CONFIG_HOTPLUG_CPU ifdeffery around unregister_cpu
Changes since RFC v2:
* Move earlier in the series

---
 drivers/base/cpu.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

drivers/base/cpu.c +0 -0

index 579064fda97b..58bb86091b34 100644
@@ -531,7 +531,14 @@ int __weak arch_register_cpu(int cpu)
 {

drivers/base/cpu.c int __weak arch_register_cpu(int cpu) · line 540

Quoted 8 lines

drivers/base/cpu.c +7 -0

+
+#ifdef CONFIG_HOTPLUG_CPU
+void __weak arch_unregister_cpu(int num)
+{
+	unregister_cpu(&per_cpu(cpu_devices, num));
+}
+#endif /* CONFIG_HOTPLUG_CPU */  

I have previously asked the question whether we should provide a stub weak function for the !HOTPLUG_CPU case for this, which would alleviate the concerns around if (IS_ENABLED()) in some of the later hotplug vCPU patches... which failed to get _any_ responses.

So, I'm now going to deem the comment I received about if (IS_ENABLED()) potentially causing issues to be unimportant, and thus there's no need for a stub weak function. If we start getting compile errors, then we can address the issue at that point. So far, however, the kernel build bot has not identified that this as an issue... and it's been chewing on this entire patch set for well over a month now.

The differences between architecture specific implementations of arch_register_cpu() are down to whether the CPU is hotpluggable or not. Rather than overriding the weak version of arch_register_cpu(), provide a function that can be used to provide this detail instead.

Signed-off-by Russell King (Oracle) <[email protected]>

---
  drivers/base/cpu.c  | 11 ++++++++++-
  include/linux/cpu.h |  1 +
  2 files changed, 11 insertions(+), 1 deletion(-)

drivers/base/cpu.c +10 -1

index 58bb86091b34..221ffbeb1c9b 100644
@@ -527,9 +527,18 @@ EXPORT_SYMBOL_GPL(cpu_is_hotpluggable);
  #ifdef CONFIG_GENERIC_CPU_DEVICES
  DEFINE_PER_CPU(struct cpu, cpu_devices);
 
+bool __weak arch_cpu_is_hotpluggable(int cpu)
+{
+	return false;
+}
+
  int __weak arch_register_cpu(int cpu)
  {
-	return register_cpu(&per_cpu(cpu_devices, cpu), cpu);
+	struct cpu *c = &per_cpu(cpu_devices, cpu);
+
+	c->hotpluggable = arch_cpu_is_hotpluggable(cpu);
+
+	return register_cpu(c, cpu);
  }
 
  #ifdef CONFIG_HOTPLUG_CPU

include/linux/cpu.h +1 -0

index 1e982d63eae8..dcb89c987164 100644
@@ -80,6 +80,7 @@ extern __printf(4, 5)
  struct device *cpu_device_create(struct device *parent, void *drvdata,
  				 const struct attribute_group **groups,
  				 const char *fmt, ...);
+extern bool arch_cpu_is_hotpluggable(int cpu);
  extern int arch_register_cpu(int cpu);
  extern void arch_unregister_cpu(int cpu);
  #ifdef CONFIG_HOTPLUG_CPU

The differences between architecture specific implementations of arch_register_cpu() are down to whether the CPU is hotpluggable or not. Rather than overriding the weak version of arch_register_cpu(), provide a function that can be used to provide this detail instead.

Signed-off-by Russell King (Oracle) <[email protected]>

---
  drivers/base/cpu.c  | 11 ++++++++++-
  include/linux/cpu.h |  1 +
  2 files changed, 11 insertions(+), 1 deletion(-)

The differences between architecture specific implementations of arch_register_cpu() are down to whether the CPU is hotpluggable or not. Rather than overriding the weak version of arch_register_cpu(), provide a function that can be used to provide this detail instead.

Signed-off-by Russell King (Oracle) <[email protected]>

From: James Morse <[email protected]>

NUMA systems require the node descriptions to be ready before CPUs are registered. This is so that the node symlinks can be created in sysfs.

Currently no NUMA platform uses GENERIC_CPU_DEVICES, meaning that CPUs are registered by arch code, instead of cpu_dev_init().

Move cpu_dev_init() after node_dev_init() so that NUMA architectures can use GENERIC_CPU_DEVICES.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Note: Jonathan's comment still needs addressing - see
https://lore.kernel.org/r/[email protected]

---
  drivers/base/init.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 11 lines · Russell King (Oracle)

From: James Morse <[email protected]>

NUMA systems require the node descriptions to be ready before CPUs are registered. This is so that the node symlinks can be created in sysfs.

Currently no NUMA platform uses GENERIC_CPU_DEVICES, meaning that CPUs are registered by arch code, instead of cpu_dev_init().

Move cpu_dev_init() after node_dev_init() so that NUMA architectures can use GENERIC_CPU_DEVICES.

Quoted 9 lines

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Note: Jonathan's comment still needs addressing - see
https://lore.kernel.org/r/[email protected]

---
  drivers/base/init.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

With Jonathan's comments addressed:

Quoted 24 lines · Gavin Shan

Quoted 20 lines · Russell King (Oracle)

From: James Morse <[email protected]>

NUMA systems require the node descriptions to be ready before CPUs are registered. This is so that the node symlinks can be created in sysfs.

Currently no NUMA platform uses GENERIC_CPU_DEVICES, meaning that CPUs are registered by arch code, instead of cpu_dev_init().

Move cpu_dev_init() after node_dev_init() so that NUMA architectures can use GENERIC_CPU_DEVICES.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Note: Jonathan's comment still needs addressing - see
https://lore.kernel.org/r/[email protected]

---
  drivers/base/init.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 1 line

With Jonathan's comments addressed:

That needs James' input, which is why I made the note on the patch.

Quoted 27 lines · Russell King (Oracle)

Quoted 25 lines · Gavin Shan

Quoted 11 lines · Russell King (Oracle)

From: James Morse <[email protected]>

NUMA systems require the node descriptions to be ready before CPUs are registered. This is so that the node symlinks can be created in sysfs.

Currently no NUMA platform uses GENERIC_CPU_DEVICES, meaning that CPUs are registered by arch code, instead of cpu_dev_init().

Move cpu_dev_init() after node_dev_init() so that NUMA architectures can use GENERIC_CPU_DEVICES.

Quoted 9 lines

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Note: Jonathan's comment still needs addressing - see
https://lore.kernel.org/r/[email protected]

---
  drivers/base/init.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

With Jonathan's comments addressed:

Quoted 1 line

That needs James' input, which is why I made the note on the patch.

I'm going to be posting the series without RFC soon, and it will be with Jonathan's comment unaddressed - because as I've said several times it needs James' input and we have sadly not yet received that.

Short of waiting until James can respond, I don't think there are any other alternatives.

I do hope we can get this queued up for v6.8 though.

Quoted 34 lines · On Tue, 21 Nov 2023 13:27:08 +0000 "Russell King (Oracle)"…

Quoted 25 lines · Russell King (Oracle)

Quoted 24 lines · Gavin Shan

Quoted 20 lines · Russell King (Oracle)

From: James Morse <[email protected]>

NUMA systems require the node descriptions to be ready before CPUs are registered. This is so that the node symlinks can be created in sysfs.

Currently no NUMA platform uses GENERIC_CPU_DEVICES, meaning that CPUs are registered by arch code, instead of cpu_dev_init().

Move cpu_dev_init() after node_dev_init() so that NUMA architectures can use GENERIC_CPU_DEVICES.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Note: Jonathan's comment still needs addressing - see
https://lore.kernel.org/r/[email protected]

---
  drivers/base/init.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 3 lines

Quoted 1 line

With Jonathan's comments addressed:

That needs James' input, which is why I made the note on the patch.

I'm going to be posting the series without RFC soon, and it will be with Jonathan's comment unaddressed - because as I've said several times it needs James' input and we have sadly not yet received that.

Quoted 2 lines

Short of waiting until James can respond, I don't think there are any other alternatives.

In the interests of expediency I'm fine with that. (To be honest I'd forgotten I even made that comment ;)

Jonathan

Quoted 2 lines

I do hope we can get this queued up for v6.8 though.

Quoted 1 line

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 11 lines · Russell King (Oracle)

From: James Morse <[email protected]>

NUMA systems require the node descriptions to be ready before CPUs are registered. This is so that the node symlinks can be created in sysfs.

Currently no NUMA platform uses GENERIC_CPU_DEVICES, meaning that CPUs are registered by arch code, instead of cpu_dev_init().

Move cpu_dev_init() after node_dev_init() so that NUMA architectures can use GENERIC_CPU_DEVICES.

Quoted 9 lines

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Note: Jonathan's comment still needs addressing - see
https://lore.kernel.org/r/[email protected]

---
  drivers/base/init.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

With Jonathan's comments addressed:

Reviewed-by Gavin Shan <[email protected]>

Since "drivers: base: Move cpu_dev_init() after node_dev_init()", we can remove some redundant code.

node_dev_init() will walk through the nodes calling register_one_node() on each. This will trickle down to __register_one_node() which walks all present CPUs, calling register_cpu_under_node() on each.

register_cpu_under_node() will call get_cpu_device(cpu) for each, which will return NULL until the CPU is registered using register_cpu(). This now happens _after_ node_dev_init().

Therefore, calling register_cpu_under_node() from __register_one_node() becomes a no-op, and can be removed.

Signed-off-by Russell King (Oracle) <[email protected]>

---
  drivers/base/node.c | 7 -------
  1 file changed, 7 deletions(-)

drivers/base/node.c +0 -7

index 493d533f8375..4d5ac7cf8757 100644
@@ -867,7 +867,6 @@ void register_memory_blocks_under_node(int nid, unsigned long start_pfn,
  int __register_one_node(int nid)
  {
  	int error;
-	int cpu;
 
  	node_devices[nid] = kzalloc(sizeof(struct node), GFP_KERNEL);
  	if (!node_devices[nid])

@@ -875,12 +874,6 @@ int __register_one_node(int nid)
 
  	error = register_node(node_devices[nid], nid);
 
-	/* link cpu under this node */
-	for_each_present_cpu(cpu) {
-		if (cpu_to_node(cpu) == nid)
-			register_cpu_under_node(cpu, nid);
-	}
-
  	INIT_LIST_HEAD(&node_devices[nid]->access_list);
  	node_init_caches(nid);
 

Quoted 11 lines · Russell King (Oracle)

Since "drivers: base: Move cpu_dev_init() after node_dev_init()", we can remove some redundant code.

node_dev_init() will walk through the nodes calling register_one_node() on each. This will trickle down to __register_one_node() which walks all present CPUs, calling register_cpu_under_node() on each.

register_cpu_under_node() will call get_cpu_device(cpu) for each, which will return NULL until the CPU is registered using register_cpu(). This now happens _after_ node_dev_init().

Quoted 8 lines

Therefore, calling register_cpu_under_node() from __register_one_node() becomes a no-op, and can be removed.

Signed-off-by Russell King (Oracle) <[email protected]>

---
  drivers/base/node.c | 7 -------
  1 file changed, 7 deletions(-)

__register_one_node() can be called in memory hot add path either. In that path, a new NUMA node can be presented and becomes online. Does this become a problem after the logic of associating CPU with newly added NUMA node?

From: James Morse <[email protected]>

loongarch, mips, parisc, riscv and sh all print a warning if register_cpu() returns an error. Architectures that use GENERIC_CPU_DEVICES call panic() instead.

Errors in this path indicate something is wrong with the firmware description of the platform, but the kernel is able to keep running.

Downgrade this to a warning to make it easier to debug this issue.

This will allow architectures that switching over to GENERIC_CPU_DEVICES to drop their warning, but keep the existing behaviour.

Signed-off-by James Morse <[email protected]>

Reviewed-by Russell King (Oracle) <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
  drivers/base/cpu.c | 7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)

drivers/base/cpu.c +4 -3

index 221ffbeb1c9b..82b6a76125f5 100644
@@ -551,14 +551,15 @@ void __weak arch_unregister_cpu(int num)
 
  static void __init cpu_dev_register_generic(void)
  {
-	int i;
+	int i, ret;
 
  	if (!IS_ENABLED(CONFIG_GENERIC_CPU_DEVICES))
  		return;
 
  	for_each_present_cpu(i) {
-		if (arch_register_cpu(i))
-			panic("Failed to register CPU device");
+		ret = arch_register_cpu(i);
+		if (ret)
+			pr_warn("register_cpu %d failed (%d)\n", i, ret);
  	}
  }
 

From: James Morse <[email protected]>

loongarch, mips, parisc, riscv and sh all print a warning if register_cpu() returns an error. Architectures that use GENERIC_CPU_DEVICES call panic() instead.

Errors in this path indicate something is wrong with the firmware description of the platform, but the kernel is able to keep running.

Downgrade this to a warning to make it easier to debug this issue.

This will allow architectures that switching over to GENERIC_CPU_DEVICES to drop their warning, but keep the existing behaviour.

Signed-off-by James Morse <[email protected]>

Reviewed-by Russell King (Oracle) <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
  drivers/base/cpu.c | 7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)

From: James Morse <[email protected]>

loongarch, mips, parisc, riscv and sh all print a warning if register_cpu() returns an error. Architectures that use GENERIC_CPU_DEVICES call panic() instead.

Errors in this path indicate something is wrong with the firmware description of the platform, but the kernel is able to keep running.

Downgrade this to a warning to make it easier to debug this issue.

This will allow architectures that switching over to GENERIC_CPU_DEVICES to drop their warning, but keep the existing behaviour.

Signed-off-by James Morse <[email protected]>

Reviewed-by Russell King (Oracle) <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

From: James Morse <[email protected]>

To allow ACPI's _STA value to hide CPUs that are present, but not available to online right now due to VMM or firmware policy, the register_cpu() call needs to be made by the ACPI machinery when ACPI is in use. This allows it to hide CPUs that are unavailable from sysfs.

Switching to GENERIC_CPU_DEVICES is an intermediate step to allow all five ACPI architectures to be modified at once.

Switch over to GENERIC_CPU_DEVICES, and provide an arch_register_cpu() that populates the hotpluggable flag. arch_register_cpu() is also the interface the ACPI machinery expects.

The struct cpu in struct cpuinfo_arm64 is never used directly, remove it to use the one GENERIC_CPU_DEVICES provides.

This changes the CPUs visible in sysfs from possible to present, but on arm64 smp_prepare_cpus() ensures these are the same.

This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.

Signed-off-by James Morse <[email protected]>

Reviewed-by Russell King (Oracle) <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC v2:
* Add note about initialisation order change.

---
  arch/arm64/Kconfig           |  1 +
  arch/arm64/include/asm/cpu.h |  1 -
  arch/arm64/kernel/setup.c    | 13 ++++---------
  3 files changed, 5 insertions(+), 10 deletions(-)

arch/arm64/Kconfig +1 -0

index 7b071a00425d..84bce830e365 100644
@@ -132,6 +132,7 @@ config ARM64
  	select GENERIC_ARCH_TOPOLOGY
  	select GENERIC_CLOCKEVENTS_BROADCAST
  	select GENERIC_CPU_AUTOPROBE
+	select GENERIC_CPU_DEVICES
  	select GENERIC_CPU_VULNERABILITIES
  	select GENERIC_EARLY_IOREMAP
  	select GENERIC_IDLE_POLL_SETUP

arch/arm64/include/asm/cpu.h +0 -1

index f3034099fd95..b1e43f56ee46 100644
@@ -38,7 +38,6 @@ struct cpuinfo_32bit {
  };
 
  struct cpuinfo_arm64 {
-	struct cpu	cpu;
  	struct kobject	kobj;
  	u64		reg_ctr;
  	u64		reg_cntfrq;

arch/arm64/kernel/setup.c +4 -9

index 417a8a86b2db..165bd2c0dd5a 100644
@@ -402,19 +402,14 @@ static inline bool cpu_can_disable(unsigned int cpu)
  	return false;
  }
 
-static int __init topology_init(void)
+int arch_register_cpu(int num)
  {
-	int i;
+	struct cpu *cpu = &per_cpu(cpu_devices, num);
 
-	for_each_possible_cpu(i) {
-		struct cpu *cpu = &per_cpu(cpu_data.cpu, i);
-		cpu->hotpluggable = cpu_can_disable(i);
-		register_cpu(cpu, i);
-	}
+	cpu->hotpluggable = cpu_can_disable(num);
 
-	return 0;
+	return register_cpu(cpu, num);
  }
-subsys_initcall(topology_init);
 
  static void dump_kernel_offset(void)
  {

From: James Morse <[email protected]>

To allow ACPI's _STA value to hide CPUs that are present, but not available to online right now due to VMM or firmware policy, the register_cpu() call needs to be made by the ACPI machinery when ACPI is in use. This allows it to hide CPUs that are unavailable from sysfs.

Switching to GENERIC_CPU_DEVICES is an intermediate step to allow all five ACPI architectures to be modified at once.

Switch over to GENERIC_CPU_DEVICES, and provide an arch_register_cpu() that populates the hotpluggable flag. arch_register_cpu() is also the interface the ACPI machinery expects.

The struct cpu in struct cpuinfo_arm64 is never used directly, remove it to use the one GENERIC_CPU_DEVICES provides.

This changes the CPUs visible in sysfs from possible to present, but on arm64 smp_prepare_cpus() ensures these are the same.

This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.

Signed-off-by James Morse <[email protected]>

Reviewed-by Russell King (Oracle) <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC v2:
* Add note about initialisation order change.

---
  arch/arm64/Kconfig           |  1 +
  arch/arm64/include/asm/cpu.h |  1 -
  arch/arm64/kernel/setup.c    | 13 ++++---------
  3 files changed, 5 insertions(+), 10 deletions(-)

From: James Morse <[email protected]>

To allow ACPI's _STA value to hide CPUs that are present, but not available to online right now due to VMM or firmware policy, the register_cpu() call needs to be made by the ACPI machinery when ACPI is in use. This allows it to hide CPUs that are unavailable from sysfs.

Switching to GENERIC_CPU_DEVICES is an intermediate step to allow all five ACPI architectures to be modified at once.

Switch over to GENERIC_CPU_DEVICES, and provide an arch_register_cpu() that populates the hotpluggable flag. arch_register_cpu() is also the interface the ACPI machinery expects.

The struct cpu in struct cpuinfo_arm64 is never used directly, remove it to use the one GENERIC_CPU_DEVICES provides.

This changes the CPUs visible in sysfs from possible to present, but on arm64 smp_prepare_cpus() ensures these are the same.

This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.

Signed-off-by James Morse <[email protected]>

Reviewed-by Russell King (Oracle) <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC v2:
* Add note about initialisation order change.

---
 arch/arm64/Kconfig           |  1 +
 arch/arm64/include/asm/cpu.h |  1 -
 arch/arm64/kernel/setup.c    | 13 ++++---------
 3 files changed, 5 insertions(+), 10 deletions(-)

arch/arm64/Kconfig +1 -0

index 7b071a00425d..84bce830e365 100644
@@ -132,6 +132,7 @@ config ARM64
 	select GENERIC_ARCH_TOPOLOGY
 	select GENERIC_CLOCKEVENTS_BROADCAST
 	select GENERIC_CPU_AUTOPROBE
+	select GENERIC_CPU_DEVICES
 	select GENERIC_CPU_VULNERABILITIES
 	select GENERIC_EARLY_IOREMAP
 	select GENERIC_IDLE_POLL_SETUP

arch/arm64/include/asm/cpu.h +0 -1

index f3034099fd95..b1e43f56ee46 100644
@@ -38,7 +38,6 @@ struct cpuinfo_32bit {
 };
 
 struct cpuinfo_arm64 {
-	struct cpu	cpu;
 	struct kobject	kobj;
 	u64		reg_ctr;
 	u64		reg_cntfrq;

arch/arm64/kernel/setup.c +4 -9

index 417a8a86b2db..165bd2c0dd5a 100644
@@ -402,19 +402,14 @@ static inline bool cpu_can_disable(unsigned int cpu)
 	return false;
 }
 
-static int __init topology_init(void)
+int arch_register_cpu(int num)
 {
-	int i;
+	struct cpu *cpu = &per_cpu(cpu_devices, num);
 
-	for_each_possible_cpu(i) {
-		struct cpu *cpu = &per_cpu(cpu_data.cpu, i);
-		cpu->hotpluggable = cpu_can_disable(i);
-		register_cpu(cpu, i);
-	}
+	cpu->hotpluggable = cpu_can_disable(num);
 
-	return 0;
+	return register_cpu(cpu, num);
 }
-subsys_initcall(topology_init);
 
 static void dump_kernel_offset(void)
 {

Quoted 23 lines · On Tue, 07 Nov 2023 10:30:25 +0000 Russell King <[email protected]>…

From: James Morse <[email protected]>

To allow ACPI's _STA value to hide CPUs that are present, but not available to online right now due to VMM or firmware policy, the register_cpu() call needs to be made by the ACPI machinery when ACPI is in use. This allows it to hide CPUs that are unavailable from sysfs.

Switching to GENERIC_CPU_DEVICES is an intermediate step to allow all five ACPI architectures to be modified at once.

Switch over to GENERIC_CPU_DEVICES, and provide an arch_register_cpu() that populates the hotpluggable flag. arch_register_cpu() is also the interface the ACPI machinery expects.

The struct cpu in struct cpuinfo_arm64 is never used directly, remove it to use the one GENERIC_CPU_DEVICES provides.

This changes the CPUs visible in sysfs from possible to present, but on arm64 smp_prepare_cpus() ensures these are the same.

This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.

Quoted 3 lines

Signed-off-by James Morse <[email protected]>

Reviewed-by Russell King (Oracle) <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

Given this series adds an arch_cpu_is_hotpluggable() callback we probably want something in this patch description to say why this isn't using that, but instead overriding arch_register_cpu()

Convert arm64 to use the arch_cpu_is_hotpluggable() helper rather than arch_register_cpu().

Signed-off-by Russell King (Oracle) <[email protected]>

---
  arch/arm64/kernel/setup.c | 8 ++------
  1 file changed, 2 insertions(+), 6 deletions(-)

arch/arm64/kernel/setup.c +2 -6

index 165bd2c0dd5a..42c690bb2d60 100644
@@ -402,13 +402,9 @@ static inline bool cpu_can_disable(unsigned int cpu)
  	return false;
  }
 
-int arch_register_cpu(int num)
+bool arch_cpu_is_hotpluggable(int num)
  {
-	struct cpu *cpu = &per_cpu(cpu_devices, num);
-
-	cpu->hotpluggable = cpu_can_disable(num);
-
-	return register_cpu(cpu, num);
+	return cpu_can_disable(num);
  }
 
  static void dump_kernel_offset(void)

Convert arm64 to use the arch_cpu_is_hotpluggable() helper rather than arch_register_cpu().

Signed-off-by Russell King (Oracle) <[email protected]>

---
  arch/arm64/kernel/setup.c | 8 ++------
  1 file changed, 2 insertions(+), 6 deletions(-)

Convert arm64 to use the arch_cpu_is_hotpluggable() helper rather than arch_register_cpu().

Signed-off-by Russell King (Oracle) <[email protected]>

---
 arch/arm64/kernel/setup.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

arch/arm64/kernel/setup.c +2 -6

index 165bd2c0dd5a..42c690bb2d60 100644
@@ -402,13 +402,9 @@ static inline bool cpu_can_disable(unsigned int cpu)
 	return false;
 }
 
-int arch_register_cpu(int num)
+bool arch_cpu_is_hotpluggable(int num)
 {
-	struct cpu *cpu = &per_cpu(cpu_devices, num);
-
-	cpu->hotpluggable = cpu_can_disable(num);
-
-	return register_cpu(cpu, num);
+	return cpu_can_disable(num);
 }
 
 static void dump_kernel_offset(void)

Quoted 4 lines · On Tue, 07 Nov 2023 10:30:30 +0000 "Russell King (Oracle)"…

Convert arm64 to use the arch_cpu_is_hotpluggable() helper rather than arch_register_cpu().

Signed-off-by Russell King (Oracle) <[email protected]>

Ah. Or previous patch needs a forwards reference to the tweaking of it it here.

Maybe just smash the 2 together with a Co-developed: ?

From: James Morse <[email protected]>

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.

This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

----
Changes since RFC:
* Fixed the second copy of arch_register_cpu() used for non-hotplug
Changes since RFC v2:
* Remove duplicate of the weak generic arch_register_cpu(), spotted by Jonathan Cameron. Add note about initialisation order change.
Changes since RFC v3:
* Adapt to removal of EXPORT_SYMBOL()s

---
  arch/x86/Kconfig           |  1 +
  arch/x86/include/asm/cpu.h |  4 ----
  arch/x86/kernel/topology.c | 27 ++++-----------------------
  3 files changed, 5 insertions(+), 27 deletions(-)

From: James Morse <[email protected]>

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.

This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

Since the x86 version of arch_unregister_cpu() is the same as the weak version, drop the x86 specific version.

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC v3:
* Adapt to removal of EXPORT_SYMBOL()s

---
  arch/x86/kernel/topology.c | 5 -----
  1 file changed, 5 deletions(-)

arch/x86/kernel/topology.c +0 -5

index c2ed3145a93b..211863cb5b81 100644
@@ -43,9 +43,4 @@ int arch_register_cpu(int cpu)
  	c->hotpluggable = cpu > 0;
  	return register_cpu(c, cpu);
  }
-
-void arch_unregister_cpu(int num)
-{
-	unregister_cpu(&per_cpu(cpu_devices, num));
-}
  #endif /* CONFIG_HOTPLUG_CPU */

Since the x86 version of arch_unregister_cpu() is the same as the weak version, drop the x86 specific version.

Signed-off-by Russell King (Oracle) <[email protected]>

Convert x86 to use the arch_cpu_is_hotpluggable() helper rather than arch_register_cpu().

Signed-off-by Russell King (Oracle) <[email protected]>

---
  arch/x86/kernel/topology.c | 7 ++-----
  1 file changed, 2 insertions(+), 5 deletions(-)

Convert x86 to use the arch_cpu_is_hotpluggable() helper rather than arch_register_cpu().

Signed-off-by Russell King (Oracle) <[email protected]>

---
 arch/x86/kernel/topology.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

arch/x86/kernel/topology.c +2 -5

index 211863cb5b81..d42c28b8bfd8 100644
@@ -36,11 +36,8 @@
 #include <asm/cpu.h>
 
 #ifdef CONFIG_HOTPLUG_CPU
-int arch_register_cpu(int cpu)
+bool arch_cpu_is_hotpluggable(int cpu)
 {
-	struct cpu *c = per_cpu_ptr(&cpu_devices, cpu);
-
-	c->hotpluggable = cpu > 0;
-	return register_cpu(c, cpu);
+	return cpu > 0;
 }
 #endif /* CONFIG_HOTPLUG_CPU */

From: James Morse <[email protected]>

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.

This allows topology_init() to be removed.

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.

This is a subtle change. Originally:
- on boot, topology_init() would have marked present CPUs that io_master() is true for as hotplug-incapable.
- if a CPU is hotplugged that is an io_master(), it can later be hot-unplugged.

The new behaviour is that any CPU that io_master() is true for will now always be marked as hotplug-incapable, thus even if it was hotplugged, it can no longer be hot-unplugged.

This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC v2:
* Explain the change in behaviour in the patch description (highlighted by Jonathan Cameron - thanks.) Add note about initialisation order change.

---
  arch/loongarch/Kconfig           |  1 +
  arch/loongarch/kernel/topology.c | 29 ++---------------------------
  2 files changed, 3 insertions(+), 27 deletions(-)

From: James Morse <[email protected]>

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.

This allows topology_init() to be removed.

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.

This is a subtle change. Originally:
- on boot, topology_init() would have marked present CPUs that io_master() is true for as hotplug-incapable.
- if a CPU is hotplugged that is an io_master(), it can later be hot-unplugged.

The new behaviour is that any CPU that io_master() is true for will now always be marked as hotplug-incapable, thus even if it was hotplugged, it can no longer be hot-unplugged.

This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.

Signed-off-by James Morse <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

Convert loongarch to use the arch_cpu_is_hotpluggable() helper rather than arch_register_cpu(). Also remove the export as nothing should be using arch_register_cpu() outside of the core kernel/acpi code.

Signed-off-by Russell King (Oracle) <[email protected]>

---
  arch/loongarch/kernel/topology.c | 7 ++-----
  1 file changed, 2 insertions(+), 5 deletions(-)

Convert loongarch to use the arch_cpu_is_hotpluggable() helper rather than arch_register_cpu(). Also remove the export as nothing should be using arch_register_cpu() outside of the core kernel/acpi code.

Signed-off-by Russell King (Oracle) <[email protected]>

From: James Morse <[email protected]>

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.

This allows topology_init() to be removed.

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.

This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.

Signed-off-by James Morse <[email protected]>

Reviewed-by Jonathan Cameron <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC v2:
* Add note about initialisation order change.

---
 arch/riscv/Kconfig        |  1 +
 arch/riscv/kernel/setup.c | 19 ++++---------------
 2 files changed, 5 insertions(+), 15 deletions(-)

arch/riscv/Kconfig +1 -0

index eaa15a20e6ae..264aa466aec3 100644
@@ -69,6 +69,7 @@ config RISCV
 	select GENERIC_ARCH_TOPOLOGY
 	select GENERIC_ATOMIC64 if !64BIT
 	select GENERIC_CLOCKEVENTS_BROADCAST if SMP
+	select GENERIC_CPU_DEVICES
 	select GENERIC_EARLY_IOREMAP
 	select GENERIC_ENTRY
 	select GENERIC_GETTIMEOFDAY if HAVE_GENERIC_VDSO

arch/riscv/kernel/setup.c +2 -11

index 0624f44d43ec..f8875ae1b0aa 100644
@@ -50,7 +50,6 @@ atomic_t hart_lottery __section(".sdata")
 #endif
 ;
 unsigned long boot_cpu_hartid;
-static DEFINE_PER_CPU(struct cpu, cpu_devices);
 
 /*
  * Place kernel memory regions on the resource tree so that

@@ -295,23 +294,13 @@ void __init setup_arch(char **cmdline_p)
 	riscv_set_dma_cache_alignment();
 }
 
-static int __init topology_init(void)
+int arch_register_cpu(int cpu)
 {
-	int i, ret;
+	struct cpu *c = &per_cpu(cpu_devices, cpu);
 
-	for_each_possible_cpu(i) {
-		struct cpu *cpu = &per_cpu(cpu_devices, i);
-
-		cpu->hotpluggable = cpu_has_hotplug(i);
-		ret = register_cpu(cpu, i);
-		if (unlikely(ret))
-			pr_warn("Warning: %s: register_cpu %d failed (%d)\n",
-			       __func__, i, ret);

From: James Morse <[email protected]>

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.

This allows topology_init() to be removed.

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.

This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.

Signed-off-by James Morse <[email protected]>

Reviewed-by Jonathan Cameron <[email protected]>

Signed-off-by Russell King (Oracle) <[email protected]>

---
Changes since RFC v2:
* Add note about initialisation order change.

---
  arch/riscv/Kconfig        |  1 +
  arch/riscv/kernel/setup.c | 19 ++++---------------
  2 files changed, 5 insertions(+), 15 deletions(-)

Convert riscv to use the arch_cpu_is_hotpluggable() helper rather than arch_register_cpu().

Signed-off-by Russell King (Oracle) <[email protected]>

---
 arch/riscv/kernel/setup.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

arch/riscv/kernel/setup.c +0 -1

index f8875ae1b0aa..168f0db63d53 100644
@@ -294,12 +294,9 @@ void __init setup_arch(char **cmdline_p)
 	riscv_set_dma_cache_alignment();
 }
 
-int arch_register_cpu(int cpu)

Convert riscv to use the arch_cpu_is_hotpluggable() helper rather than arch_register_cpu().

Signed-off-by Russell King (Oracle) <[email protected]>

---
  arch/riscv/kernel/setup.c | 7 ++-----
  1 file changed, 2 insertions(+), 5 deletions(-)

Convert riscv to use the arch_cpu_is_hotpluggable() helper rather than arch_register_cpu().

Signed-off-by Russell King (Oracle) <[email protected]>