Attention is currently required from: Raul Rangel. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57628 )
Change subject: lib/thread: Switch to using CPU_INFO_V2 ......................................................................
Patch Set 7:
(1 comment)
File src/lib/thread.c:
https://review.coreboot.org/c/coreboot/+/57628/comment/0ecf05b2_1f24a4dc PS7, Line 49: return cpu_info()->thread; (Sorry for being late, patch itself looks fine to me, just have some furthergoing theoretical questions.)
Is there actually any point in having this current thread info stored in the cpu_info()? I thought (and we only do SMP on x86 which I'm not super familiar with, so please correct me if I'm wrong) that coreboot isn't a "full" SMP system where all cores are equal -- instead most work is done on the boot CPU and the other cores only run some special CPU init code and otherwise try to stay out of the way, right? In particular, this whole cooperative multitasking framework is only supposed to run all threads on the boot CPU, and it would be illegal for any of the other CPUs to call thread_run()? (Because all that mutex stuff and everything else isn't actually written to deal with real concurrency between different cores.)
So then I think we can go further and just put current_thread in a simple global. I think before this patch, this code was only dealing with cpu_info because cpu_info was part of the stack, not because it really needs to be involved in the thread framework. The only thing that I think the non-boot CPUs do here is for when udelay() calls thread_yield_microseconds(), they have to fail on the thread_can_yield() (because current_thread() would be NULL). We can have that easier by just making an is_boot_cpu() API or something like that. It would be nice if cpu_info() could just become a completely arch/x86-specific thing and the other architectures can just hardcode something like is_boot_cpu() to 1.