[PATCH] riscv: cpu_ops_sbi: retry checking if CPU is stopped

Jimmy Ho posted 1 patch 1 month, 3 weeks ago
arch/riscv/kernel/cpu_ops_sbi.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
[PATCH] riscv: cpu_ops_sbi: retry checking if CPU is stopped
Posted by Jimmy Ho 1 month, 3 weeks ago
Introduce a retry loop with a timeout to wait for the HSM state
of the core being hotplugged down to properly transition to
HSM_STATE_STOPPED.

Suggested-by: Samuel Holland <samuel.holland@sifive.com>
Signed-off-by: Jimmy Ho <jimmy.ho@sifive.com>
---
 arch/riscv/kernel/cpu_ops_sbi.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/cpu_ops_sbi.c b/arch/riscv/kernel/cpu_ops_sbi.c
index ee6e4b5cc39e..41e577400591 100644
--- a/arch/riscv/kernel/cpu_ops_sbi.c
+++ b/arch/riscv/kernel/cpu_ops_sbi.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2020 Western Digital Corporation or its affiliates.
  */
 
+#include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/mm.h>
 #include <linux/sched/task_stack.h>
@@ -87,8 +88,23 @@ static bool sbi_cpu_is_stopped(unsigned int cpuid)
 {
 	int rc;
 	unsigned long hartid = cpuid_to_hartid_map(cpuid);
-
-	rc = sbi_hsm_hart_get_status(hartid);
+	unsigned long start, end;
+
+	/*
+	 * The core that is being hotplugged down might still
+	 * be processing SBI ecall hotplug down.
+	 * So, try again a few times.
+	 */
+
+	start = jiffies;
+	end = start + msecs_to_jiffies(100);
+	do {
+		rc = sbi_hsm_hart_get_status(hartid);
+		if (rc == SBI_HSM_STATE_STOPPED)
+			break;
+
+		usleep_range(100, 1000);
+	} while (time_before(jiffies, end));
 
 	if (rc != SBI_HSM_STATE_STOPPED) {
 		pr_warn("HART%lu isn't stopped; status %d\n", hartid, rc);
-- 
2.43.7
Re: [PATCH] riscv: cpu_ops_sbi: retry checking if CPU is stopped
Posted by Zhan Xusheng 1 month, 3 weeks ago
On Sat,  8 Aug 2026 14:32:41 +0800, Jimmy Ho wrote:
> + start = jiffies;
> + end = start + msecs_to_jiffies(100);
> + do {
> +   rc = sbi_hsm_hart_get_status(hartid);
> +   if (rc == SBI_HSM_STATE_STOPPED)
> +     break;
> +
> +   usleep_range(100, 1000);
> + } while (time_before(jiffies, end));

This is cpu_psci_cpu_kill() from arch/arm64/kernel/psci.c, down to the
locals, both delay values and the closing line of the comment.  Please
say so in the commit message.  Right now the 100 ms reads as a bound
derived from something about HSM, and it is not -- it is the arm64 PSCI
value.  Naming the precedent is a better defence of it than silence.

You also dropped arm64's report of how long the poll took.  Deliberate?
It is jiffy-granular, so in the good case it just prints 0 ms, and the
caller already emits "CPU%u: off" -- but it is also the only way anyone
ever learns whether 100 ms is close to what real firmware needs.  As it
stands @start exists only to compute @end.

Separately, sbi_hsm_hart_get_status() returns a negative errno when the
ecall fails, not an HSM state, so a bad hartid gets polled for the full
100 ms.  Worth breaking out on rc < 0.

Thanks,
Zhan Xusheng