Attention is currently required from: Raul Rangel, Kyösti Mälkki, Rob Barnes, 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 7:
(3 comments)
File src/include/mutex.h:
https://review.coreboot.org/c/coreboot/+/59320/comment/2c10501b_59716225 PS7, Line 22: else if (CONFIG(COOP_MULTITASKING)) This should be
(ENV_RAMSTAGE || ENV_ROMSTAGE) && CONFIG(COOP_MULTITASKING)
Maybe we should create an ENV_COOP_MULTITASKING to encapsulate that?
File src/include/mutex.h:
https://review.coreboot.org/c/coreboot/+/59320/comment/900671c8_2033ed1c PS6, Line 32: }
Right spinlock worked by start with 1 and decrementing. […]
Are you saying we should make ENV_STATE_SUPPORTS_SMP evaluate to 1 in x86 romstage, Kyösti? But why? The x86 SMP start-up code for the non-boot CPUs is only in ramstage, right? There's no point SMP-proofing anything as long as only one CPU is up. It's not really about where you can implement the spinlock, more about where the spinlock is necessary. (In that sense I think it should probably be changed to (ENV_RAMSTAGE || ENV_SMP) for x86, since postcar should also have no SMP yet, right?)
File src/include/mutex.h:
https://review.coreboot.org/c/coreboot/+/59320/comment/6dfe43a4_40c6b3f4 PS2, Line 11: void mutex_unlock(struct mutex *mutex);
Done
Nothing in coreboot uses LTO, it's not an easy thing to support. There have been experimental attempts for a while, the latest I'm aware of is CB:40815. You can try checking with Jacob how far he got and what was missing if you're interested.
-
Julius Werner (Code Review)