This series cleans up some duplicated and dead code around the RISC-V
CPU operations, that was copied from arm64 but is not needed here. The
result is a bit of memory savings and removal of a few SBI calls during
boot, with no functional change.
Samuel Holland (3):
riscv: Deduplicate code in setup_smp()
riscv: Remove unused members from struct cpu_operations
riscv: Use the same CPU operations for all CPUs
arch/riscv/include/asm/cpu_ops.h | 14 ++--------
arch/riscv/kernel/cpu-hotplug.c | 19 +++++---------
arch/riscv/kernel/cpu_ops.c | 14 ++++------
arch/riscv/kernel/cpu_ops_sbi.c | 19 --------------
arch/riscv/kernel/cpu_ops_spinwait.c | 11 --------
arch/riscv/kernel/head.S | 1 -
arch/riscv/kernel/setup.c | 1 -
arch/riscv/kernel/smp.c | 2 +-
arch/riscv/kernel/smpboot.c | 38 ++++++++--------------------
9 files changed, 24 insertions(+), 95 deletions(-)
--
2.42.0
RISC-V provides no binding (ACPI or DT) to describe per-cpu start/stop
operations, so cpu_set_ops() will always detect the same operations for
every CPU. Replace the cpu_ops array with a single pointer to save space
and reduce boot time.
Signed-off-by: Samuel Holland <[email protected]>
---
arch/riscv/include/asm/cpu_ops.h | 4 ++--
arch/riscv/kernel/cpu-hotplug.c | 10 +++++-----
arch/riscv/kernel/cpu_ops.c | 12 +++++-------
arch/riscv/kernel/smp.c | 2 +-
arch/riscv/kernel/smpboot.c | 13 +++++--------
5 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/arch/riscv/include/asm/cpu_ops.h b/arch/riscv/include/asm/cpu_ops.h
index 18af75e6873c..176b570ef982 100644
--- a/arch/riscv/include/asm/cpu_ops.h
+++ b/arch/riscv/include/asm/cpu_ops.h
@@ -29,7 +29,7 @@ struct cpu_operations {
};
extern const struct cpu_operations cpu_ops_spinwait;
-extern const struct cpu_operations *cpu_ops[NR_CPUS];
-void __init cpu_set_ops(int cpu);
+extern const struct cpu_operations *cpu_ops;
+void __init cpu_set_ops(void);
#endif /* ifndef __ASM_CPU_OPS_H */
diff --git a/arch/riscv/kernel/cpu-hotplug.c b/arch/riscv/kernel/cpu-hotplug.c
index 934eb64da0d0..28b58fc5ad19 100644
--- a/arch/riscv/kernel/cpu-hotplug.c
+++ b/arch/riscv/kernel/cpu-hotplug.c
@@ -18,7 +18,7 @@
bool cpu_has_hotplug(unsigned int cpu)
{
- if (cpu_ops[cpu]->cpu_stop)
+ if (cpu_ops->cpu_stop)
return true;
return false;
@@ -31,7 +31,7 @@ int __cpu_disable(void)
{
unsigned int cpu = smp_processor_id();
- if (!cpu_ops[cpu] || !cpu_ops[cpu]->cpu_stop)
+ if (!cpu_ops->cpu_stop)
return -EOPNOTSUPP;
remove_cpu_topology(cpu);
@@ -55,8 +55,8 @@ void arch_cpuhp_cleanup_dead_cpu(unsigned int cpu)
pr_notice("CPU%u: off\n", cpu);
/* Verify from the firmware if the cpu is really stopped*/
- if (cpu_ops[cpu]->cpu_is_stopped)
- ret = cpu_ops[cpu]->cpu_is_stopped(cpu);
+ if (cpu_ops->cpu_is_stopped)
+ ret = cpu_ops->cpu_is_stopped(cpu);
if (ret)
pr_warn("CPU%d may not have stopped: %d\n", cpu, ret);
}
@@ -70,7 +70,7 @@ void __noreturn arch_cpu_idle_dead(void)
cpuhp_ap_report_dead();
- cpu_ops[smp_processor_id()]->cpu_stop();
+ cpu_ops->cpu_stop();
/* It should never reach here */
BUG();
}
diff --git a/arch/riscv/kernel/cpu_ops.c b/arch/riscv/kernel/cpu_ops.c
index 5540e2880abb..6a8bd8f4db07 100644
--- a/arch/riscv/kernel/cpu_ops.c
+++ b/arch/riscv/kernel/cpu_ops.c
@@ -13,7 +13,7 @@
#include <asm/sbi.h>
#include <asm/smp.h>
-const struct cpu_operations *cpu_ops[NR_CPUS] __ro_after_init;
+const struct cpu_operations *cpu_ops __ro_after_init = &cpu_ops_spinwait;
extern const struct cpu_operations cpu_ops_sbi;
#ifndef CONFIG_RISCV_BOOT_SPINWAIT
@@ -22,14 +22,12 @@ const struct cpu_operations cpu_ops_spinwait = {
};
#endif
-void __init cpu_set_ops(int cpuid)
+void __init cpu_set_ops(void)
{
#if IS_ENABLED(CONFIG_RISCV_SBI)
if (sbi_probe_extension(SBI_EXT_HSM)) {
- if (!cpuid)
- pr_info("SBI HSM extension detected\n");
- cpu_ops[cpuid] = &cpu_ops_sbi;
- } else
+ pr_info("SBI HSM extension detected\n");
+ cpu_ops = &cpu_ops_sbi;
+ }
#endif
- cpu_ops[cpuid] = &cpu_ops_spinwait;
}
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 40420afbb1a0..45dd4035416e 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -81,7 +81,7 @@ static inline void ipi_cpu_crash_stop(unsigned int cpu, struct pt_regs *regs)
#ifdef CONFIG_HOTPLUG_CPU
if (cpu_has_hotplug(cpu))
- cpu_ops[cpu]->cpu_stop();
+ cpu_ops->cpu_stop();
#endif
for(;;)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 5551945255cd..519b6bd946e5 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -166,25 +166,22 @@ void __init setup_smp(void)
{
int cpuid;
- cpu_set_ops(0);
+ cpu_set_ops();
if (acpi_disabled)
of_parse_and_init_cpus();
else
acpi_parse_and_init_cpus();
- for (cpuid = 1; cpuid < nr_cpu_ids; cpuid++) {
- if (cpuid_to_hartid_map(cpuid) != INVALID_HARTID) {
- cpu_set_ops(cpuid);
+ for (cpuid = 1; cpuid < nr_cpu_ids; cpuid++)
+ if (cpuid_to_hartid_map(cpuid) != INVALID_HARTID)
set_cpu_possible(cpuid, true);
- }
- }
}
static int start_secondary_cpu(int cpu, struct task_struct *tidle)
{
- if (cpu_ops[cpu]->cpu_start)
- return cpu_ops[cpu]->cpu_start(cpu, tidle);
+ if (cpu_ops->cpu_start)
+ return cpu_ops->cpu_start(cpu, tidle);
return -EOPNOTSUPP;
}
--
2.42.0
Both the ACPI and DT implementations contain some of the same code.
Move it to the calling function so it is not duplicated.
Signed-off-by: Samuel Holland <[email protected]>
---
arch/riscv/kernel/smpboot.c | 31 +++++++++++--------------------
1 file changed, 11 insertions(+), 20 deletions(-)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index d162bf339beb..1c68e61fb852 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -125,18 +125,7 @@ static int __init acpi_parse_rintc(union acpi_subtable_headers *header, const un
static void __init acpi_parse_and_init_cpus(void)
{
- int cpuid;
-
- cpu_set_ops(0);
-
acpi_table_parse_madt(ACPI_MADT_TYPE_RINTC, acpi_parse_rintc, 0);
-
- for (cpuid = 1; cpuid < nr_cpu_ids; cpuid++) {
- if (cpuid_to_hartid_map(cpuid) != INVALID_HARTID) {
- cpu_set_ops(cpuid);
- set_cpu_possible(cpuid, true);
- }
- }
}
#else
#define acpi_parse_and_init_cpus(...) do { } while (0)
@@ -150,8 +139,6 @@ static void __init of_parse_and_init_cpus(void)
int cpuid = 1;
int rc;
- cpu_set_ops(0);
-
for_each_of_cpu_node(dn) {
rc = riscv_early_of_processor_hartid(dn, &hart);
if (rc < 0)
@@ -179,21 +166,25 @@ static void __init of_parse_and_init_cpus(void)
if (cpuid > nr_cpu_ids)
pr_warn("Total number of cpus [%d] is greater than nr_cpus option value [%d]\n",
cpuid, nr_cpu_ids);
-
- for (cpuid = 1; cpuid < nr_cpu_ids; cpuid++) {
- if (cpuid_to_hartid_map(cpuid) != INVALID_HARTID) {
- cpu_set_ops(cpuid);
- set_cpu_possible(cpuid, true);
- }
- }
}
void __init setup_smp(void)
{
+ int cpuid;
+
+ cpu_set_ops(0);
+
if (acpi_disabled)
of_parse_and_init_cpus();
else
acpi_parse_and_init_cpus();
+
+ for (cpuid = 1; cpuid < nr_cpu_ids; cpuid++) {
+ if (cpuid_to_hartid_map(cpuid) != INVALID_HARTID) {
+ cpu_set_ops(cpuid);
+ set_cpu_possible(cpuid, true);
+ }
+ }
}
static int start_secondary_cpu(int cpu, struct task_struct *tidle)
--
2.42.0
name is not used anywhere at all. cpu_prepare and cpu_disable do nothing
and always return 0 if implemented.
Signed-off-by: Samuel Holland <[email protected]>
---
arch/riscv/include/asm/cpu_ops.h | 10 ----------
arch/riscv/kernel/cpu-hotplug.c | 9 +--------
arch/riscv/kernel/cpu_ops.c | 2 --
arch/riscv/kernel/cpu_ops_sbi.c | 19 -------------------
arch/riscv/kernel/cpu_ops_spinwait.c | 11 -----------
arch/riscv/kernel/head.S | 1 -
arch/riscv/kernel/setup.c | 1 -
arch/riscv/kernel/smpboot.c | 6 ------
8 files changed, 1 insertion(+), 58 deletions(-)
diff --git a/arch/riscv/include/asm/cpu_ops.h b/arch/riscv/include/asm/cpu_ops.h
index aa128466c4d4..18af75e6873c 100644
--- a/arch/riscv/include/asm/cpu_ops.h
+++ b/arch/riscv/include/asm/cpu_ops.h
@@ -13,26 +13,16 @@
/**
* struct cpu_operations - Callback operations for hotplugging CPUs.
*
- * @name: Name of the boot protocol.
- * @cpu_prepare: Early one-time preparation step for a cpu. If there
- * is a mechanism for doing so, tests whether it is
- * possible to boot the given HART.
* @cpu_start: Boots a cpu into the kernel.
- * @cpu_disable: Prepares a cpu to die. May fail for some
- * mechanism-specific reason, which will cause the hot
- * unplug to be aborted. Called from the cpu to be killed.
* @cpu_stop: Makes a cpu leave the kernel. Must not fail. Called from
* the cpu being stopped.
* @cpu_is_stopped: Ensures a cpu has left the kernel. Called from another
* cpu.
*/
struct cpu_operations {
- const char *name;
- int (*cpu_prepare)(unsigned int cpu);
int (*cpu_start)(unsigned int cpu,
struct task_struct *tidle);
#ifdef CONFIG_HOTPLUG_CPU
- int (*cpu_disable)(unsigned int cpu);
void (*cpu_stop)(void);
int (*cpu_is_stopped)(unsigned int cpu);
#endif
diff --git a/arch/riscv/kernel/cpu-hotplug.c b/arch/riscv/kernel/cpu-hotplug.c
index 457a18efcb11..934eb64da0d0 100644
--- a/arch/riscv/kernel/cpu-hotplug.c
+++ b/arch/riscv/kernel/cpu-hotplug.c
@@ -29,25 +29,18 @@ bool cpu_has_hotplug(unsigned int cpu)
*/
int __cpu_disable(void)
{
- int ret = 0;
unsigned int cpu = smp_processor_id();
if (!cpu_ops[cpu] || !cpu_ops[cpu]->cpu_stop)
return -EOPNOTSUPP;
- if (cpu_ops[cpu]->cpu_disable)
- ret = cpu_ops[cpu]->cpu_disable(cpu);
-
- if (ret)
- return ret;
-
remove_cpu_topology(cpu);
numa_remove_cpu(cpu);
set_cpu_online(cpu, false);
riscv_ipi_disable();
irq_migrate_all_off_this_cpu();
- return ret;
+ return 0;
}
#ifdef CONFIG_HOTPLUG_CPU
diff --git a/arch/riscv/kernel/cpu_ops.c b/arch/riscv/kernel/cpu_ops.c
index eb479a88a954..5540e2880abb 100644
--- a/arch/riscv/kernel/cpu_ops.c
+++ b/arch/riscv/kernel/cpu_ops.c
@@ -18,8 +18,6 @@ const struct cpu_operations *cpu_ops[NR_CPUS] __ro_after_init;
extern const struct cpu_operations cpu_ops_sbi;
#ifndef CONFIG_RISCV_BOOT_SPINWAIT
const struct cpu_operations cpu_ops_spinwait = {
- .name = "",
- .cpu_prepare = NULL,
.cpu_start = NULL,
};
#endif
diff --git a/arch/riscv/kernel/cpu_ops_sbi.c b/arch/riscv/kernel/cpu_ops_sbi.c
index efa0f0816634..1cc7df740edd 100644
--- a/arch/riscv/kernel/cpu_ops_sbi.c
+++ b/arch/riscv/kernel/cpu_ops_sbi.c
@@ -79,23 +79,7 @@ static int sbi_cpu_start(unsigned int cpuid, struct task_struct *tidle)
return sbi_hsm_hart_start(hartid, boot_addr, hsm_data);
}
-static int sbi_cpu_prepare(unsigned int cpuid)
-{
- if (!cpu_ops_sbi.cpu_start) {
- pr_err("cpu start method not defined for CPU [%d]\n", cpuid);
- return -ENODEV;
- }
- return 0;
-}
-
#ifdef CONFIG_HOTPLUG_CPU
-static int sbi_cpu_disable(unsigned int cpuid)
-{
- if (!cpu_ops_sbi.cpu_stop)
- return -EOPNOTSUPP;
- return 0;
-}
-
static void sbi_cpu_stop(void)
{
int ret;
@@ -118,11 +102,8 @@ static int sbi_cpu_is_stopped(unsigned int cpuid)
#endif
const struct cpu_operations cpu_ops_sbi = {
- .name = "sbi",
- .cpu_prepare = sbi_cpu_prepare,
.cpu_start = sbi_cpu_start,
#ifdef CONFIG_HOTPLUG_CPU
- .cpu_disable = sbi_cpu_disable,
.cpu_stop = sbi_cpu_stop,
.cpu_is_stopped = sbi_cpu_is_stopped,
#endif
diff --git a/arch/riscv/kernel/cpu_ops_spinwait.c b/arch/riscv/kernel/cpu_ops_spinwait.c
index d98d19226b5f..613872b0a21a 100644
--- a/arch/riscv/kernel/cpu_ops_spinwait.c
+++ b/arch/riscv/kernel/cpu_ops_spinwait.c
@@ -39,15 +39,6 @@ static void cpu_update_secondary_bootdata(unsigned int cpuid,
WRITE_ONCE(__cpu_spinwait_task_pointer[hartid], tidle);
}
-static int spinwait_cpu_prepare(unsigned int cpuid)
-{
- if (!cpu_ops_spinwait.cpu_start) {
- pr_err("cpu start method not defined for CPU [%d]\n", cpuid);
- return -ENODEV;
- }
- return 0;
-}
-
static int spinwait_cpu_start(unsigned int cpuid, struct task_struct *tidle)
{
/*
@@ -64,7 +55,5 @@ static int spinwait_cpu_start(unsigned int cpuid, struct task_struct *tidle)
}
const struct cpu_operations cpu_ops_spinwait = {
- .name = "spinwait",
- .cpu_prepare = spinwait_cpu_prepare,
.cpu_start = spinwait_cpu_start,
};
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index b77397432403..0349e5cdfe1d 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -11,7 +11,6 @@
#include <asm/page.h>
#include <asm/pgtable.h>
#include <asm/csr.h>
-#include <asm/cpu_ops_sbi.h>
#include <asm/hwcap.h>
#include <asm/image.h>
#include <asm/scs.h>
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 535a837de55d..2bf882804624 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -26,7 +26,6 @@
#include <asm/alternative.h>
#include <asm/cacheflush.h>
#include <asm/cpufeature.h>
-#include <asm/cpu_ops.h>
#include <asm/early_ioremap.h>
#include <asm/pgtable.h>
#include <asm/setup.h>
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 1c68e61fb852..5551945255cd 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -49,7 +49,6 @@ void __init smp_prepare_boot_cpu(void)
void __init smp_prepare_cpus(unsigned int max_cpus)
{
int cpuid;
- int ret;
unsigned int curr_cpuid;
init_cpu_topology();
@@ -66,11 +65,6 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
for_each_possible_cpu(cpuid) {
if (cpuid == curr_cpuid)
continue;
- if (cpu_ops[cpuid]->cpu_prepare) {
- ret = cpu_ops[cpuid]->cpu_prepare(cpuid);
- if (ret)
- continue;
- }
set_cpu_present(cpuid, true);
numa_store_cpu_info(cpuid);
}
--
2.42.0
On Tue, Nov 21, 2023 at 03:47:24PM -0800, Samuel Holland wrote:
> Both the ACPI and DT implementations contain some of the same code.
> Move it to the calling function so it is not duplicated.
>
> Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
Cheers,
Conor.
On Tue, Nov 21, 2023 at 03:47:25PM -0800, Samuel Holland wrote:
> name is not used anywhere at all. cpu_prepare and cpu_disable do nothing
> and always return 0 if implemented.
>
> Signed-off-by: Samuel Holland <[email protected]>
Part of me says this should be split so that it can be partially
reverted whenever we grow users for these, but I doubt that the
implementer of that will notice nor that anyone will remember you
removed them. Certainly wouldn't object if there was a split though.
I guess in general this was a case of copy-pasting stuff from arm64
that we did not yet have users for.
Reviewed-by: Conor Dooley <[email protected]>
Cheers,
Conor.
On Tue, Nov 21, 2023 at 03:47:26PM -0800, Samuel Holland wrote:
> RISC-V provides no binding (ACPI or DT) to describe per-cpu start/stop
> operations, so cpu_set_ops() will always detect the same operations for
> every CPU. Replace the cpu_ops array with a single pointer to save space
> and reduce boot time.
>
> Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
Cheers,
Conor.
Hello:
This series was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <[email protected]>:
On Tue, 21 Nov 2023 15:47:23 -0800 you wrote:
> This series cleans up some duplicated and dead code around the RISC-V
> CPU operations, that was copied from arm64 but is not needed here. The
> result is a bit of memory savings and removal of a few SBI calls during
> boot, with no functional change.
>
>
> Samuel Holland (3):
> riscv: Deduplicate code in setup_smp()
> riscv: Remove unused members from struct cpu_operations
> riscv: Use the same CPU operations for all CPUs
>
> [...]
Here is the summary with links:
- [1/3] riscv: Deduplicate code in setup_smp()
https://git.kernel.org/riscv/c/a4166aec1130
- [2/3] riscv: Remove unused members from struct cpu_operations
https://git.kernel.org/riscv/c/79093f3ec39c
- [3/3] riscv: Use the same CPU operations for all CPUs
https://git.kernel.org/riscv/c/62ff262227a4
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html