Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45115 )
Change subject: soc/amd/picasso: Assign IOAPIC IDs using FSP ......................................................................
soc/amd/picasso: Assign IOAPIC IDs using FSP
Add a Kconfig symbol to use for the FCH and GNB IOAPIC IDs, then pass the info to FSP to keep it in sync. The assignment assumes that the FCH IOAPIC is set to CONFIG_MAX_CPUS and the GNB one is the next higher.
BUG=b:167421913, b:166519072 TEST=Boot Morphius and verify settings BRANCH=Zork
Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Change-Id: I57d3d6b2ebd8b5d511dbcb4324ea065cc3111a2d --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/acpi.c M src/soc/amd/picasso/fsp_params.c 3 files changed, 16 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/45115/1
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index ec5ff76..4a3082b 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -243,6 +243,14 @@ hex default 0xfef00000
+config PICASSO_IOAPIC_IDS + hex + default MAX_CPUS + help + The Picasso APU has two IOAPICs, one in the FCH and one in the + northbridge. Set this value for the intended ID to assign to the + FCH IOAPIC. The northbridge's one will be the next higher ID. + config SERIRQ_CONTINUOUS_MODE bool default n diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c index 1b9c0ca..0f1a4c2 100644 --- a/src/soc/amd/picasso/acpi.c +++ b/src/soc/amd/picasso/acpi.c @@ -51,7 +51,7 @@
/* Write Kern IOAPIC, only one */ current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *)current, - CONFIG_MAX_CPUS, IO_APIC_ADDR, 0); + CONFIG_PICASSO_IOAPIC_IDS, IO_APIC_ADDR, 0);
/* 0: mean bus 0--->ISA */ /* 0: PIC 0 */ diff --git a/src/soc/amd/picasso/fsp_params.c b/src/soc/amd/picasso/fsp_params.c index 1dbb8e5..aeccfca 100644 --- a/src/soc/amd/picasso/fsp_params.c +++ b/src/soc/amd/picasso/fsp_params.c @@ -122,6 +122,12 @@ } }
+static void fsp_assign_ioapic_ids(FSP_S_CONFIG *scfg) +{ + scfg->fch_ioapic_id = CONFIG_PICASSO_IOAPIC_IDS; + scfg->gnb_ioapic_id = CONFIG_PICASSO_IOAPIC_IDS + 1; +} + void platform_fsp_silicon_init_params_cb(FSPS_UPD *supd) { const struct soc_amd_picasso_config *cfg; @@ -130,5 +136,6 @@ cfg = config_of_soc(); fsps_update_emmc_config(scfg, cfg); fsp_fill_pcie_ddi_descriptors(scfg); + fsp_assign_ioapic_ids(scfg); fsp_usb_oem_customization(scfg, cfg); }
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45115 )
Change subject: soc/amd/picasso: Assign IOAPIC IDs using FSP ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45115/1/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/45115/1/src/soc/amd/picasso/Kconfig... PS1, Line 246: PICASSO_IOAPIC_IDS nit: I think it would be better to simply add two Kconfigs one for each IOAPIC_ID. That way we don't have to worry about ensuring that the same math is used anywhere the ID is referenced.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45115 )
Change subject: soc/amd/picasso: Assign IOAPIC IDs using FSP ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45115/1/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/45115/1/src/soc/amd/picasso/Kconfig... PS1, Line 246: PICASSO_IOAPIC_IDS
nit: I think it would be better to simply add two Kconfigs one for each IOAPIC_ID. […]
Also, the new Kconfigs should be used in agesa_acpi.c: https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/amd/picasso/agesa...
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45115
to look at the new patch set (#2).
Change subject: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP ......................................................................
soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP
Add Kconfig symbols for the FCH and GNB IOAPIC IDs, then pass the info to FSP to keep it in sync with coreboot. Do the same for the northbridge's IOAPIC base address.
Use the new values where needed, and reserve the resources consumed by the GNB IOAPIC.
BUG=b:167421913, b:166519072 TEST=Boot Morphius and verify settings BRANCH=Zork
Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Change-Id: I57d3d6b2ebd8b5d511dbcb4324ea065cc3111a2d --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/acpi.c M src/soc/amd/picasso/agesa_acpi.c M src/soc/amd/picasso/fsp_params.c M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/root_complex.c 6 files changed, 40 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/45115/2
Marshall Dawson 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:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45115/1/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/45115/1/src/soc/amd/picasso/Kconfig... PS1, Line 246: PICASSO_IOAPIC_IDS
Also, the new Kconfigs should be used in agesa_acpi.c: https://review.coreboot.org/cgit/coreboot. […]
Done. FWIW I'm not thrilled with assigning unrelated default values to each but AFAIK you can't do Kconfig math with the defaults. Hmm, it could be macro-ized perhaps.
Jason Glenesk 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+1
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?
Hello build bot (Jenkins), Furquan Shaikh, Jason Glenesk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45115
to look at the new patch set (#3).
Change subject: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP ......................................................................
soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP
Add Kconfig symbols for the FCH and GNB IOAPIC IDs, then pass the info to FSP to keep it in sync with coreboot. Do the same for the northbridge's IOAPIC base address.
Use the new values where needed, and reserve the resources consumed by the GNB IOAPIC.
BUG=b:167421913, b:166519072 TEST=Boot Morphius and verify settings BRANCH=Zork
Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Change-Id: I57d3d6b2ebd8b5d511dbcb4324ea065cc3111a2d --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/acpi.c M src/soc/amd/picasso/agesa_acpi.c M src/soc/amd/picasso/fsp_params.c M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/root_complex.c 6 files changed, 47 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/45115/3
Marshall Dawson 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 3:
(2 comments)
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().
Ack
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: […]
Done
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?
Done
Paul Menzel 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 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45115/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45115/3//COMMIT_MSG@14 PS3, Line 14: consumed by the GNB IOAPIC. Why should that be configurable, and can be figured out in code (greater than MAX_CPUS and unique)?
https://review.coreboot.org/c/coreboot/+/45115/3/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/45115/3/src/soc/amd/picasso/Kconfig... PS3, Line 253: the GNB's IOAPIC_ID Please add a dot/period at the end of sentences.
https://review.coreboot.org/c/coreboot/+/45115/3/src/soc/amd/picasso/Kconfig... PS3, Line 262: the FCH's IOAPIC_ID Please add a dot/period at the end of sentences.
Hello build bot (Jenkins), Furquan Shaikh, Jason Glenesk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45115
to look at the new patch set (#4).
Change subject: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP ......................................................................
soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP
Add Kconfig symbols for the FCH and GNB IOAPIC IDs, then pass the info to FSP to keep it in sync with coreboot. Do the same for the northbridge's IOAPIC base address.
Use the new values where needed, and reserve the resources consumed by the GNB IOAPIC.
BUG=b:167421913, b:166519072 TEST=Boot Morphius and verify settings BRANCH=Zork
Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Change-Id: I57d3d6b2ebd8b5d511dbcb4324ea065cc3111a2d --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/acpi.c M src/soc/amd/picasso/agesa_acpi.c M src/soc/amd/picasso/fsp_params.c M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/root_complex.c 6 files changed, 47 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/45115/4
Marshall Dawson 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 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45115/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45115/3//COMMIT_MSG@14 PS3, Line 14: consumed by the GNB IOAPIC.
Why should that be configurable, and can be figured out in code (greater than MAX_CPUS and unique)?
Ack. That's kind of the point I was making in this comment. https://review.coreboot.org/c/coreboot/+/45115/1/src/soc/amd/picasso/Kconfig.... All we want is to ensure consistent values are used everywhere. We have some other improvements in mind, so perhaps that can be a follow-on.
https://review.coreboot.org/c/coreboot/+/45115/3/src/soc/amd/picasso/Kconfig File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/45115/3/src/soc/amd/picasso/Kconfig... PS3, Line 253: the GNB's IOAPIC_ID
Please add a dot/period at the end of sentences.
Done
https://review.coreboot.org/c/coreboot/+/45115/3/src/soc/amd/picasso/Kconfig... PS3, Line 262: the FCH's IOAPIC_ID
Please add a dot/period at the end of sentences.
Done
Jason Glenesk 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 4: Code-Review+1
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 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45115/4/src/soc/amd/picasso/fsp_par... File src/soc/amd/picasso/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45115/4/src/soc/amd/picasso/fsp_par... PS4, Line 132: CONFIG_MAX_CPUS PICASSO_FCH_IOAPIC_ID
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 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45115/4/src/soc/amd/picasso/fsp_par... File src/soc/amd/picasso/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45115/4/src/soc/amd/picasso/fsp_par... PS4, Line 132: CONFIG_MAX_CPUS
PICASSO_FCH_IOAPIC_ID
Actually CONFIG_PICASSO_FCH_IOAPIC_ID
Hello build bot (Jenkins), Furquan Shaikh, Jason Glenesk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45115
to look at the new patch set (#5).
Change subject: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP ......................................................................
soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP
Add Kconfig symbols for the FCH and GNB IOAPIC IDs, then pass the info to FSP to keep it in sync with coreboot. Do the same for the northbridge's IOAPIC base address.
Use the new values where needed, and reserve the resources consumed by the GNB IOAPIC.
BUG=b:167421913, b:166519072 TEST=Boot Morphius and verify settings BRANCH=Zork
Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Change-Id: I57d3d6b2ebd8b5d511dbcb4324ea065cc3111a2d --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/acpi.c M src/soc/amd/picasso/agesa_acpi.c M src/soc/amd/picasso/fsp_params.c M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/root_complex.c 6 files changed, 47 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/45115/5
Marshall Dawson 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 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45115/4/src/soc/amd/picasso/fsp_par... File src/soc/amd/picasso/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/45115/4/src/soc/amd/picasso/fsp_par... PS4, Line 132: CONFIG_MAX_CPUS
Actually CONFIG_PICASSO_FCH_IOAPIC_ID
Done
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 5: Code-Review+2
Raul Rangel 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 5: Code-Review+2
Marshall Dawson has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45115 )
Change subject: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP ......................................................................
soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP
Add Kconfig symbols for the FCH and GNB IOAPIC IDs, then pass the info to FSP to keep it in sync with coreboot. Do the same for the northbridge's IOAPIC base address.
Use the new values where needed, and reserve the resources consumed by the GNB IOAPIC.
BUG=b:167421913, b:166519072 TEST=Boot Morphius and verify settings BRANCH=Zork
Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Change-Id: I57d3d6b2ebd8b5d511dbcb4324ea065cc3111a2d Reviewed-on: https://review.coreboot.org/c/coreboot/+/45115 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Raul Rangel rrangel@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/acpi.c M src/soc/amd/picasso/agesa_acpi.c M src/soc/amd/picasso/fsp_params.c M src/soc/amd/picasso/include/soc/iomap.h M src/soc/amd/picasso/root_complex.c 6 files changed, 47 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Raul Rangel: Looks good to me, approved
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index ec5ff76..29ebc6d 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -243,6 +243,24 @@ hex default 0xfef00000
+config PICASSO_FCH_IOAPIC_ID + hex + default 0x8 + help + The Picasso APU has two IOAPICs, one in the FCH and one in the + northbridge. Set this value for the intended ID to assign to the + FCH IOAPIC. The value should be >= MAX_CPUS and different from + the GNB's IOAPIC_ID. + +config PICASSO_GNB_IOAPIC_ID + hex + default 0x9 + help + The Picasso APU has two IOAPICs, one in the FCH and one in the + northbridge. Set this value for the intended ID to assign to the + GNB IOAPIC. The value should be >= MAX_CPUS and different from + the FCH's IOAPIC_ID. + config SERIRQ_CONTINUOUS_MODE bool default n diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c index 1b9c0ca..84be4bd 100644 --- a/src/soc/amd/picasso/acpi.c +++ b/src/soc/amd/picasso/acpi.c @@ -49,9 +49,9 @@ /* create all subtables for processors */ current = acpi_create_madt_lapics(current);
- /* Write Kern IOAPIC, only one */ + /* Write IOAPIC, only one */ current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *)current, - CONFIG_MAX_CPUS, IO_APIC_ADDR, 0); + CONFIG_PICASSO_FCH_IOAPIC_ID, IO_APIC_ADDR, 0);
/* 0: mean bus 0--->ISA */ /* 0: PIC 0 */ diff --git a/src/soc/amd/picasso/agesa_acpi.c b/src/soc/amd/picasso/agesa_acpi.c index c76e943..c2ff81d 100644 --- a/src/soc/amd/picasso/agesa_acpi.c +++ b/src/soc/amd/picasso/agesa_acpi.c @@ -54,7 +54,7 @@ ivhd_ioapic->dte_setting = IVHD_DTE_LINT_1_PASS | IVHD_DTE_LINT_0_PASS | IVHD_DTE_SYS_MGT_NO_TRANS | IVHD_DTE_NMI_PASS | IVHD_DTE_EXT_INT_PASS | IVHD_DTE_INIT_PASS; - ivhd_ioapic->handle = CONFIG_MAX_CPUS; /* FCH IOAPIC ID */ + ivhd_ioapic->handle = CONFIG_PICASSO_FCH_IOAPIC_ID; ivhd_ioapic->source_dev_id = PCI_DEVFN(SMBUS_DEV, SMBUS_FUNC); ivhd_ioapic->variety = IVHD_SPECIAL_DEV_IOAPIC; current += sizeof(ivrs_ivhd_special_t); @@ -63,7 +63,7 @@ memset(ivhd_ioapic, 0, sizeof(*ivhd_ioapic));
ivhd_ioapic->type = IVHD_DEV_8_BYTE_EXT_SPECIAL_DEV; - ivhd_ioapic->handle = CONFIG_MAX_CPUS + 1; /* GNB IOAPIC ID */ + ivhd_ioapic->handle = CONFIG_PICASSO_GNB_IOAPIC_ID; ivhd_ioapic->source_dev_id = PCI_DEVFN(0, 1); ivhd_ioapic->variety = IVHD_SPECIAL_DEV_IOAPIC; current += sizeof(ivrs_ivhd_special_t); diff --git a/src/soc/amd/picasso/fsp_params.c b/src/soc/amd/picasso/fsp_params.c index 1dbb8e5..f7f23b5 100644 --- a/src/soc/amd/picasso/fsp_params.c +++ b/src/soc/amd/picasso/fsp_params.c @@ -2,6 +2,7 @@
#include <assert.h> #include <device/pci.h> +#include <soc/iomap.h> #include <soc/pci_devs.h> #include <soc/platform_descriptors.h> #include <fsp/api.h> @@ -122,6 +123,20 @@ } }
+static void fsp_assign_ioapic_upds(FSP_S_CONFIG *scfg) +{ + _Static_assert(CONFIG_PICASSO_GNB_IOAPIC_ID >= CONFIG_MAX_CPUS, + "PICASSO_GNB_IOAPIC_ID should be >= CONFIG_MAX_CPUS!\n"); + _Static_assert(CONFIG_PICASSO_FCH_IOAPIC_ID >= CONFIG_MAX_CPUS, + "PICASSO_FCH_IOAPIC_ID should be >= CONFIG_MAX_CPUS!\n"); + _Static_assert(CONFIG_PICASSO_GNB_IOAPIC_ID != CONFIG_PICASSO_FCH_IOAPIC_ID, + "PICASSO_GNB_IOAPIC_ID should be != PICASSO_FCH_IOAPIC_ID!\n"); + + scfg->gnb_ioapic_base = GNB_IO_APIC_ADDR; + scfg->gnb_ioapic_id = CONFIG_PICASSO_GNB_IOAPIC_ID; + scfg->fch_ioapic_id = CONFIG_PICASSO_FCH_IOAPIC_ID; +} + void platform_fsp_silicon_init_params_cb(FSPS_UPD *supd) { const struct soc_amd_picasso_config *cfg; @@ -130,5 +145,6 @@ cfg = config_of_soc(); fsps_update_emmc_config(scfg, cfg); fsp_fill_pcie_ddi_descriptors(scfg); + fsp_assign_ioapic_upds(scfg); fsp_usb_oem_customization(scfg, cfg); } diff --git a/src/soc/amd/picasso/include/soc/iomap.h b/src/soc/amd/picasso/include/soc/iomap.h index 8256836..890b1c3 100644 --- a/src/soc/amd/picasso/include/soc/iomap.h +++ b/src/soc/amd/picasso/include/soc/iomap.h @@ -5,6 +5,7 @@
/* MMIO Ranges */ /* IO_APIC_ADDR defined in arch/x86 0xfec00000 */ +#define GNB_IO_APIC_ADDR 0xfec01000 #define SPI_BASE_ADDRESS 0xfec10000
#if CONFIG(HPET_ADDRESS_OVERRIDE) diff --git a/src/soc/amd/picasso/root_complex.c b/src/soc/amd/picasso/root_complex.c index 1c06928..21af481 100644 --- a/src/soc/amd/picasso/root_complex.c +++ b/src/soc/amd/picasso/root_complex.c @@ -12,6 +12,7 @@ #include <fsp/util.h> #include <stdint.h> #include <soc/memmap.h> +#include <soc/iomap.h>
/* * @@ -72,6 +73,7 @@ unsigned int idx = 0; const struct hob_header *hob = fsp_get_hob_list(); const struct hob_resource *res; + struct resource *gnb_apic;
uintptr_t early_reserved_dram_start, early_reserved_dram_end; const struct memmap_early_dram *e = memmap_get_early_dram_usage(); @@ -129,6 +131,12 @@ printk(BIOS_ERR, "Error: failed to set resources for type %d\n", res->type); } + + /* GNB IOAPIC resource */ + gnb_apic = new_resource(dev, GNB_IO_APIC_ADDR); + gnb_apic->base = GNB_IO_APIC_ADDR; + gnb_apic->size = 0x00001000; + gnb_apic->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED; }
/* Used by _SB.PCI0._CRS */
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 6:
Patch Set 3:
(2 comments)
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().
Ack
Marshall, are you planning to push follow-up change for this?
Marshall Dawson 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 6:
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().
Ack
Marshall, are you planning to push follow-up change for this?
Jason will.
-
Furquan Shaikh (Code Review) -
Jason Glenesk (Code Review)
-
Marshall Dawson (Code Review)
-
Paul Menzel (Code Review)
-
Raul Rangel (Code Review)