Attention is currently required from: Raul Rangel, Julius Werner. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56320 )
Change subject: x86/smp/spinlock: Disable thread coop when taking spinlock ......................................................................
Patch Set 1:
(1 comment)
File src/arch/x86/include/arch/smp/spinlock.h:
https://review.coreboot.org/c/coreboot/+/56320/comment/d44da6e7_8d5fb829 PS1, Line 61: lock->coop = thread_coop_enabled();
These spinlocks are primarily for the actual SMP in ramstage, not for the thread mechanism...However, I think(?) the non-boot CPUs actually can't participate in the thread mechanism anyway. I just see thread_init_non_bsp_cpu() set their cpu_info->thread pointer to NULL and I don't think it ever changes from that. So there's no real point in storing coop status for them.
That is correct. The thread mechanism is implemented for the BSP-only. All other non-BSP CPUs have their thread pointer set to NULL and it stays that way forever. The thread mechanism is for co-operative threads on the BSP where the main thread typically has a lot of busy loops and so the tasks can be multiplexed into multiple threads. In fact, the expectation is that the caller ensures the threads do not use any shared resources (which can be the UART console in this case ;)).
Maybe the best option would be to just make thread_cooperate() and thread_prevent_coop() work like a semaphore instead of a boolean value?
That suggestion makes sense to me. It will ensure that the thread does not yield if it hasn't already prevented coop and will be restored back to the same state after the lock is released.