[S] Change in coreboot[main]: [WIP] arch/riscv/smp: Fix race condition

[Maximilian Brune (Code Review)]

Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/87299?usp=email )

Change subject: [WIP] arch/riscv/smp: Fix race condition
......................................................................

[WIP] 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()`.

Signed-off-by: Maximilian Brune 
Change-Id: I1e0485831f71dde400d793b9f7bda88ae6519913
---
M src/arch/riscv/smp.c
1 file changed, 25 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/87299/1

diff --git a/src/arch/riscv/smp.c b/src/arch/riscv/smp.c
index 67dc13b..a08e16b 100644
--- a/src/arch/riscv/smp.c
+++ b/src/arch/riscv/smp.c
@@ -7,6 +7,22 @@
 #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 @@
    	} 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);
    }
 }
@@ -55,6 +76,8 @@
    	if (i == working_hartid)
    		continue;

+		atomic_set(&HLS()->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 +106,8 @@
    	// 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(&HLS()->entry.sync_b, HART_AWAKE);
    	count_awake_harts++;
    }
    printk(BIOS_DEBUG, "all harts up and running...\n");