Attention is currently required from: Furquan Shaikh, Sridhar Siricilla, Patrick Rudolph. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59357 )
Change subject: cpu/intel: Get cpu index based on LAPIC id ......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59357/comment/23a2d4d1_9f6a884e PS1, Line 9: The patch adds a method to provide cpu index based on LAPCI Id. : The function determines the cpu's LAPIC id and returns the position of the : cpu's LAPIC id among the other cores' LAPIC ids. Explain why you need it here.
It's also not clear to me what 'position' means. Something like counting lapic up to the lapic id of the cpu calling the function is better.
https://review.coreboot.org/c/coreboot/+/59357/comment/cafd4e0d_676a40c5 PS1, Line 13: TEST=Verified on Brya You're adding a function without calling it, so what exactly is verified?
File src/cpu/x86/mp_init.c:
https://review.coreboot.org/c/coreboot/+/59357/comment/fabb37b4_7fe91cd9 PS1, Line 217: cpus_dev[i]->path.apic.apic_id It looks like this is most certainly not valid for i == 0. Probably best that you dev_find_lapic(0) and loop over all siblings using a while loop.
-
Arthur Heymans (Code Review)