Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs using FSP

[Marshall Dawson (Code Review)]

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 
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 (Code Review)]

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 (Code Review)]

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...

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Marshall Dawson (Code Review)]

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 
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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Marshall Dawson (Code Review)]

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.

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Jason Glenesk (Code Review)]

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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Furquan Shaikh (Code Review)]

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?

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Marshall Dawson (Code Review)]

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 
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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Marshall Dawson (Code Review)]

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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Paul Menzel (Code Review)]

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.

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Marshall Dawson (Code Review)]

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 
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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Marshall Dawson (Code Review)]

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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Jason Glenesk (Code Review)]

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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Furquan Shaikh (Code Review)]

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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Furquan Shaikh (Code Review)]

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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Marshall Dawson (Code Review)]

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 
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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Marshall Dawson (Code Review)]

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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Furquan Shaikh (Code Review)]

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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Raul Rangel (Code Review)]

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

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Marshall Dawson (Code Review)]

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 
Change-Id: I57d3d6b2ebd8b5d511dbcb4324ea065cc3111a2d
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45115
Reviewed-by: Furquan Shaikh 
Reviewed-by: Raul Rangel 
Tested-by: build bot (Jenkins) 
---
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 */

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Furquan Shaikh (Code Review)]

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?

Change in coreboot[master]: soc/amd/picasso: Assign IOAPIC IDs, GNB APIC base with FSP

[Marshall Dawson (Code Review)]

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.