rtfb / riscv-hobby-os

Other
27 stars 5 forks source link

Fix kinit spinlock bug and rewrite spinlock in C #47

Closed rtfb closed 2 years ago

rtfb commented 2 years ago

There was a tricky bug that affected the spinlock that's used in kinit() to prevent harts from initializing simultaneously. We used to run the BSS zeroing code unconditionally, which in some cases ended up zeroing out the spinlock variable after hart0 already set it to 1. Now let's run BSS zero loop on one hart only, guarded by a bss_zero_loop_lock.

This commit also rewrites the spinlock in C, just to give the compiler a hint to NOT decide to reorder instructions in some aggressive manner.

Tested by running a modified target:

 .PHONY: run-baremetal
 run-baremetal: $(OUT)/user_sifive_u
-       $(QEMU_LAUNCHER) --binary=$<
+       $(QEMU_LAUNCHER) --timeout=3s --binary=$<

in a loop:

for ((i=1; i<=1000; i++)) do make run-baremetal; done > testlog.txt

and then verifying the log has an expected number of good lines:

$ grep 'kinit: cpu 0000000000000000' testlog.txt | wc -l
1000
$ grep 'cpu parked: 0000000000000001' testlog.txt | wc -l
1000