Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45115 )
Change subject: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP ......................................................................
Patch Set 2: Code-Review+2
(2 comments)
I think we should also update https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/amd/common/block/... to use the new Kconfig. However, since that is common code, you would probably need a helper(soc_get_fch_ioapic_id) to obtain the FCH IOAPIC ID, with a weak default to use CONFIG_MAX_CPUS. I think that can be addressed in a separate change. Ideally, you can put it before this change so that this CL can provide the stronger implementation of soc_get_fch_ioapic_id().
https://review.coreboot.org/c/coreboot/+/45115/2/src/soc/amd/picasso/fsp_par... File src/soc/amd/picasso/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45115/2/src/soc/amd/picasso/fsp_par... PS2, Line 127: { nit: I think it might be good to have _Static_assert to ensure the IOAPIC IDs are >= MAX_CPUS: ``` » _Static_assert(CONFIG_PICASSO_GNB_IOAPIC_ID >= CONFIG_MAX_CPUS, » » "PICASSO_GNB_IOAPIC_ID should be >= MAX_CPUS!\n"); » _Static_assert(CONFIG_PICASSO_FCH_IOAPIC_ID >= CONFIG_MAX_CPUS, » » "PICASSO_FCH_IOAPIC_ID should be >= MAX_CPUS!\n"); ```
https://review.coreboot.org/c/coreboot/+/45115/2/src/soc/amd/picasso/fsp_par... PS2, Line 141: fsp_assign_ioapics nit: fsp_configure_ioapic_upds?