[PATCH 1/2] lib: sbi: Actually wait for an IPI in sbi_hsm_hart_wait()
Evgenii Shatokhin
e.shatokhin at yadro.com
Sun Feb 19 14:20:30 PST 2023
"wfi" instruction is not guaranteed to wait for an interrupt to be received
by the hart, it is just a hint for the CPU. According to RISC-V Privileged
Architectures spec. v20211203, even an implementation of "wfi" as "nop" is
legal.
As a result, a secondary (non-boot) hart waiting in the while loop in
sbi_hsm_hart_wait() might leave the loop and continue even if it got no
IPI. This could lead to the following race condition when booting Linux,
for example:
Boot hart (#0) Secondary hart (#1)
runs Linux startup code waits in sbi_hsm_hart_wait()
sbi_ecall(SBI_EXT_HSM,
SBI_EXT_HSM_HART_START,
...)
enters sbi_hsm_hart_start()
sets state of hart #1 to START_PENDING
leaves sbi_hsm_hart_wait()
runs to the end of init_warmboot()
returns to scratch->next_addr
(next_addr can be garbage here)
sets next_addr, etc. for hart #1
(no good: hart #1 has already left)
sends IPI to hart #1
(no good either)
If this happens, the secondary hart jumps to a wrong next_addr at the end
of init_warmboot(), which leads to a system hang or crash.
To reproduce the issue more reliably, one could add a delay in
sbi_hsm_hart_start(): after setting the hart's state but before sending
IPI to that hart:
hstate = atomic_cmpxchg(&hdata->state, SBI_HSM_STATE_STOPPED,
SBI_HSM_STATE_START_PENDING);
...
+ sbi_timer_mdelay(10);
init_count = sbi_init_count(hartid);
rscratch->next_arg1 = arg1;
rscratch->next_addr = saddr;
The issue can be reproduced, for example, in a QEMU VM with '-machine virt'
and 2 or more CPUs, with Linux as the guest OS.
Let us actually check in sbi_hsm_hart_wait(), if the waiting hart has
received an IPI.
Signed-off-by: Evgenii Shatokhin <e.shatokhin at yadro.com>
---
lib/sbi/sbi_hsm.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
index 8499bb1..0e5ff3d 100644
--- a/lib/sbi/sbi_hsm.c
+++ b/lib/sbi/sbi_hsm.c
@@ -108,6 +108,8 @@ void sbi_hsm_prepare_next_jump(struct sbi_scratch *scratch, u32 hartid)
static void sbi_hsm_hart_wait(struct sbi_scratch *scratch, u32 hartid)
{
unsigned long saved_mie;
+ unsigned long ipi_received;
+ unsigned int hstate;
struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
hart_data_offset);
/* Save MIE CSR */
@@ -117,9 +119,11 @@ static void sbi_hsm_hart_wait(struct sbi_scratch *scratch, u32 hartid)
csr_set(CSR_MIE, MIP_MSIP | MIP_MEIP);
/* Wait for state transition requested by sbi_hsm_hart_start() */
- while (atomic_read(&hdata->state) != SBI_HSM_STATE_START_PENDING) {
+ do {
wfi();
- };
+ ipi_received = csr_read(CSR_MIP) & (MIP_MSIP | MIP_MEIP);
+ hstate = atomic_read(&hdata->state);
+ } while (hstate != SBI_HSM_STATE_START_PENDING || !ipi_received);
/* Restore MIE CSR */
csr_write(CSR_MIE, saved_mie);
--
2.34.1
More information about the opensbi
mailing list