David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38548 )
Change subject: soc/intel: Add Intel Xeon Scalable Processor support ......................................................................
Patch Set 40:
(7 comments)
https://review.coreboot.org/c/coreboot/+/38548/40/src/soc/intel/xeon_sp/acpi... File src/soc/intel/xeon_sp/acpi.c:
https://review.coreboot.org/c/coreboot/+/38548/40/src/soc/intel/xeon_sp/acpi... PS40, Line 137: size_t hob_size; Iniitialize to zero
https://review.coreboot.org/c/coreboot/+/38548/40/src/soc/intel/xeon_sp/acpi... PS40, Line 146: /* Local APICs */ formatting issue?
https://review.coreboot.org/c/coreboot/+/38548/40/src/soc/intel/xeon_sp/acpi... PS40, Line 150: hob_size This value will only get assigned if the GUID is found, but can be left uninitialized if not. Since we need to assert (the next line) if it doesn't get set, the variable needs to be initialized to 0 to trigger the assertion in the failure path.
Also, you can join this line and the previous one (the statement will become shorter).
https://review.coreboot.org/c/coreboot/+/38548/40/src/soc/intel/xeon_sp/acpi... PS40, Line 160: sizeof(ioapic_ids)/sizeof(int) ARRAY_SIZE(ioapic_ids)
(you'll need include commonlib/helpers.h)
https://review.coreboot.org/c/coreboot/+/38548/40/src/soc/intel/xeon_sp/acpi... PS40, Line 161: sizeof(gsi_bases)/sizeof(int)) ARRAY_SIZE(gsi_bases)
https://review.coreboot.org/c/coreboot/+/38548/40/src/soc/intel/xeon_sp/acpi... PS40, Line 172: sizeof(ioapic_ids)/sizeof(int) ARRAY_SIZE(ioapic_ids)
https://review.coreboot.org/c/coreboot/+/38548/40/src/soc/intel/xeon_sp/acpi... PS40, Line 173: sizeof(gsi_bases)/sizeof(int) ARRAY_SIZE(gsi_bases)