Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Julius Werner, Kyösti Mälkki, Rob Barnes, Karthik Ramasubramanian. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59021 )
Change subject: arch/x86/smp/spinlock: Fix threading when !STAGE_HAS_SPINLOCKS ......................................................................
Patch Set 2:
(1 comment)
File src/arch/x86/include/arch/smp/spinlock.h:
https://review.coreboot.org/c/coreboot/+/59021/comment/89dd8068_dac5b3e7 PS2, Line 50: thread_coop_disable();
Well, if threading is disabled then thread_mutex_lock() is a no-op right now. […]
So I've been playing around with the mutex idea locally. One thing I realized is that regardless of spin_lock or mutex, we still need to disable coop multi-threading in vprintk. printk is the primary debugging mechanism, so if printk is off yielding every character, it makes it very very difficult to debug the threading code. e.g., if the threading code hits an assert and tries to printk during a printk yield, we end up in a deadlock.
I've audited the places where spin_lock is used, and printk is the only one that causes problems because it calls `udelay` in the UART driver.
So here are the two things I'm going to do first: 1) Remove thread_coop_enable/disable from spin_lock/unlock. 2) Add a thread_coop_disable/enable to vprintk
This will solve the immediate problem of thread coop not working in romstage or earlier.
The second thing I can do is implement a multi-cpu mutex and migrate all spin_lock use cases over to mutex. This will give us a single synchronization primitive that works for both coop and multi-cpus. Having this will reduce the chances of things breaking down the road a udelay() gets added somehow into a critical section. I have the WIP code for this in my local work tree already. It's written in C instead of asm, so we won't need arch specific behavior.