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

From: Zhan Xusheng <zhanxusheng1024@gmail.com>
To: Jimmy Ho <jimmy.ho@sifive.com>, Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>
Cc: Zhan Xusheng <zhanxusheng@xiaomi.com>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Hui Wang <hui.wang@canonical.com>,
	Ben Dooks <ben.dooks@codethink.co.uk>,
	Samuel Holland <samuel.holland@sifive.com>,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] riscv: cpu_ops_sbi: retry checking if CPU is stopped
Date: Sat,  8 Aug 2026 19:47:58 +0800	[thread overview]
Message-ID: <20260808114759.1677578-1-zhanxusheng@xiaomi.com> (raw)
In-Reply-To: <20260808063242.20178-1-jimmy.ho@sifive.com>

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv