Attention is currently required from: Julius Werner, Kyösti Mälkki, Rob Barnes, Karthik Ramasubramanian. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59320 )
Change subject: lib: Add a mutex ......................................................................
Patch Set 8:
(2 comments)
File src/include/mutex.h:
https://review.coreboot.org/c/coreboot/+/59320/comment/6f89e89a_9e00de28 PS6, Line 32: } I'm still a bit confused. If AMD x86 romstage requires locking, then `ENV_STAGE_SUPPORTS_SMP` should be modified to evaluate to true.
Thus I think it makes sense to start by getting rid of the .data requirement of existing <smp/spinlock.h>, use .bss and compiller builtin __atomic_load() etc.
How about we just delete spinlock and force everyone to use mutex? Only reason I kept it was so that I would debug the coop threading scheduler. I can do that by also calling coop_disable() before taking the printk mutex.
I am asking if it is really worth having _mutex_{lock/unlock]_coop() implemented separately. A better optimisation might be to have spinlock implememntations, here _mutex_(un)lock_{coop,smp} always inlined.
I can move it all into the .h if that's the preference.
File src/lib/mutex.c:
https://review.coreboot.org/c/coreboot/+/59320/comment/a029c925_e2e080ae PS8, Line 12: return;
This could go into <smp/spinlock.h> ? […]
They don't belong in spinlock.h since the mutex and spinlock use different algorithms. I can move it into the mutex header.