Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45056 )
Change subject: soc/amd/picasso: Add MADT entry for GNB IOAPIC ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45056/6/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/c/coreboot/+/45056/6/src/soc/amd/picasso/acpi.c@... PS6, Line 50: FSP populates this generates this HOB if it initializes second IOAPIC
What are the conditions under which the second IOAPIC won't be initialized?
With the current patches in flight, it'll be enabled all the time. However I can't think of any reason to not enable it. I'd be fine with calling it a standard behavior of the FSP. Then we'll treat it more like an error if it's not found.
https://review.coreboot.org/c/coreboot/+/45056/6/src/soc/amd/picasso/acpi.c@... PS6, Line 54:
nit: space not required.
I suspect it was meant to be indented, e.g.
ACPI: * MADT NB_IOAPIC_BASE HOB was not found.
Hmm, well some of our other messages aren't indented. I'd kind of prefer they be somewhat consistent. I'd suggest a follow-on change for formatting, unless there's a new PS for l.50.