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 7:
(4 comments)
File src/include/mutex.h:
https://review.coreboot.org/c/coreboot/+/59320/comment/c6459c15_85ee0619 PS6, Line 32: }
I put the following in <rules.h> […]
Right spinlock worked by start with 1 and decrementing. mutex works by starting with 0 and setting to 1 when locked.
I don't quite understand. In romstage `ENV_STAGE_SUPPORTS_SMP == 0`.
File src/lib/mutex.c:
https://review.coreboot.org/c/coreboot/+/59320/comment/5cc4c584_e36003d2 PS5, Line 24: #if ENV_X86
No arch-dependency under lib/ please. […]
I wanted to avoid all the extra noop functions. Done.
https://review.coreboot.org/c/coreboot/+/59320/comment/ea929b82_365ab373 PS5, Line 37: assert(thread_yield() >= 0);
Are the assert() here and below why you did not want to change printk() to mutex? […]
Yeah, otherwise we get into a deadlock. Since printk is the primary debugging mechanism I wanted to keep it functional. I would prefer to keep it functional for threading.
File src/lib/mutex.c:
https://review.coreboot.org/c/coreboot/+/59320/comment/6ee9b37a_9c1432cd PS6, Line 23: if (thread_yield() < 0) {
This path already handles COOP_MULTITHREADING=y, do you really want a separate _mutex_lock_coop() be […]
I wanted to keep the assert in the coop-only case that we are yielding. Otherwise if that fails we will be stuck looping forever. I also wasn't sure how the atomics would work in a CAR environment, so I felt it was safer to not rely on them in the `pre-RAM` stages.
-
Raul Rangel (Code Review)