From def7aa7094122147aed5d36b8f50c56496ee7ab5 Mon Sep 17 00:00:00 2001
From: Maximilian Brune <maximilian.brune@9elements.com>
Date: Sun, 16 Nov 2025 20:00:43 +0100
Subject: [PATCH] arch/riscv/smp: Fix race condition

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>
---
 src/arch/riscv/smp.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/src/arch/riscv/smp.c b/src/arch/riscv/smp.c
index 67dc13b8fc..ab6e0add3a 100644
--- a/src/arch/riscv/smp.c
+++ b/src/arch/riscv/smp.c
@@ -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");