Attention is currently required from: Keith Hui, Branden Waldner, Angel Pons, Patrick Rudolph. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59693 )
Change subject: nb/intel/i440bx: Use PARALLEL_MP ......................................................................
Patch Set 4:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59693/comment/75683ff2_41c2f57f PS4, Line 10: always
This is not true, see `src/mainboard/asus/p2b/Kconfig`. Although the CPUs only have one core, there are mainboards with two CPU sockets.
It took me some time to understand the code but it looks like previously there were 2 ways of adding more 'cores/APIC' to the devicetree linked list and have the lapic init code do its thing: Have a scan_bus operation for the cpu bus (see northbridge/amd/agesa/fam14/northbridge.c l816) or have it somewhere inside the cpu init code ( cpu/intel/model_f2x_init.c initel_sibling_init()). The i440bx code seems to have neither so I don't think multiple CPUs worked on that platform.
File src/northbridge/intel/i440bx/northbridge.c:
https://review.coreboot.org/c/coreboot/+/59693/comment/b9179b00_cfa3c741 PS4, Line 76: 1
Not true for mainboards with two CPU sockets, see comment in commit message.
I'm not sure if there's a generic procedure to detect which CPU sockets are populated. In any case, returning `CONFIG_MAX_CPUS` would at least be slightly more correct.
I don't really know it either... It looks like multi socket CPU would be a new feature on this board anyway so maybe a TODO comment here fits the job? If things regress it's likely not hard to fix.
-
Arthur Heymans (Code Review)