erebion/coreboot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

arch/riscv/smp: Fix race condition

Browse source 
If the APs are much faster then the working hart, it is possible that it
will enter HART_SLEEPING state before the working hart checks whether or
not the APs woke up by checking the HART_AWAKE state.

One can reproduce this issue by adding the following print message and
testing it in QEMU. One will notice that it will get stuck.
+ printk(BIOS_SPEW, "waiting for hart %d\n", i);
while (atomic_read(&OTHER_HLS(i)->entry.sync_a) != HART_AWAKE)

Fix it by adding another sync step at the end of `smp_resume()`.

Tested: QEMU RISC-V with -smp 64 parameter

Signed-off-by: Maximilian Brune <maximilian.brune@9elements.com>
Change-Id: I1e0485831f71dde400d793b9f7bda88ae6519913
Reviewed-on: https://review.coreboot.org/c/coreboot/+/87299
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Matt DeVillier <matt.devillier@gmail.com>
This commit is contained in:
Maximilian Brune 2025-11-16 20:00:43 +01:00 • committed by Matt DeVillier
commit def7aa7094
1 changed files with 28 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

28
src/arch/riscv/smp.c
View file

@ -4,9 +4,25 @@
#include <arch/encoding.h>
#include <arch/smp/smp.h>
#include <arch/smp/atomic.h>
#include <assert.h>
#include <console/console.h>
#include <mcall.h>
/*
* BSP
* --------------------------------- <---
* AP | sync_b=HART_SLEEPING | |
* ---> --------------------------------- --------------------------------- |
* | | sync_a=HART_SLEEPING | ----> | Wait for sync_a=HART_SLEEPING | |
* | --------------------------------- --------------------------------- |
* | | Wait for Interrupt | <---- | set_msip() | |
* | --------------------------------- --------------------------------- |
* | | sync_a=HART_AWAKE | ----> | Wait for sync_a=HART_AWAKE | |
* | --------------------------------- --------------------------------- |
* | | Wait for sync_b=HART_AWAKE | <---- | sync_b=HART_AWAKE | |
* ---- --------------------------------- --------------------------------- ----
*/
// made up value to sync hart state
#define HART_SLEEPING 0x1
#define HART_AWAKE 0x2
@ -29,6 +45,11 @@ void smp_pause(int working_hartid)
} while ((read_csr(mip) & MIP_MSIP) == 0);
atomic_set(&HLS()->entry.sync_a, HART_AWAKE); // mark the hart as awake
// wait for working_hart to notice that this hart is awake
while (atomic_read(&HLS()->entry.sync_b) != HART_AWAKE)
;
HLS()->entry.fn(HLS()->entry.arg);
}
}
@ -47,6 +68,8 @@ void smp_resume(void (*fn)(void *), void *arg)
if (CONFIG(RISCV_GET_HART_COUNT_AT_RUNTIME))
hart_count = smp_get_hart_count();
assert(hart_count <= CONFIG_MAX_CPUS);
// check that all harts are present
u32 count_awake_harts = 0;
@ -55,6 +78,8 @@ void smp_resume(void (*fn)(void *), void *arg)
if (i == working_hartid)
continue;
atomic_set(&OTHER_HLS(i)->entry.sync_b, HART_SLEEPING);
if (atomic_read(&OTHER_HLS(i)->entry.sync_a) != HART_SLEEPING) {
/*
* we assmue here that the time between smp_pause and smp_resume
@ -83,6 +108,9 @@ void smp_resume(void (*fn)(void *), void *arg)
// wait for hart to publish its waking state
while (atomic_read(&OTHER_HLS(i)->entry.sync_a) != HART_AWAKE)
;
// signal to hart that we noticed it woke up
atomic_set(&OTHER_HLS(i)->entry.sync_b, HART_AWAKE);
count_awake_harts++;
}
printk(BIOS_DEBUG, "all harts up and running...\n");