Attention is currently required from: Raul Rangel, Rob Barnes, Kyösti Mälkki, Karthik Ramasubramanian. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59320 )
Change subject: lib: Add a mutex ......................................................................
Patch Set 2:
(2 comments)
File src/include/mutex.h:
PS2: License
File src/lib/mutex.c:
https://review.coreboot.org/c/coreboot/+/59320/comment/1e15547e_837c1e96 PS2, Line 24: #if ENV_X86 I'm honestly not sure if combining this stuff like this is a good idea, because as you can see here you're forced to get into architectural details pretty quickly. On arm64, the equivalent to the PAUSE instruction would be a WFE instruction here and then a SEV instruction in unlock(). So you need to have different architectural hooks in different places for each architecture to the point where in the end it would probably become easier to just implement the whole thing separately for each. I'm also not exactly sure how GCC translates the generic atomics into instructions on Arm, and the load-acquire/store-release rules there can get very tricky and have weird requirements to make them work.
Usually, each architecture already brings its assembly reference code for these kinds of things anyway, and it's easier to just copy that in (and throw in a call to thread_yield() somewhere) than trying to do it all yourself, especially in generic C.
-
Julius Werner (Code Review)