Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48668 )
Change subject: soc/amd/picasso: Generate GNB IO-APIC PCI routing table ......................................................................
soc/amd/picasso: Generate GNB IO-APIC PCI routing table
This adds support for generating a PCI routing table that routes to the GNB IO-APIC. This means we no longer need to route to the FCH IO-APIC for PCI interrupts.
BUG=b:170595019 TEST=Boot ezkinil to OS with `pci=nomsi amd_iommu=off` and verify all peripherals are working
CPU0 CPU1 0: 112 0 IO-APIC 2-edge timer 1: 0 99 IO-APIC 1-edge i8042 4: 0 2523 IO-APIC 4-edge ttyS0 5: 34632 0 IO-APIC 5-fasteoi mmc1 7: 5646 0 IO-APIC 7-fasteoi pinctrl_amd 8: 0 0 IO-APIC 8-edge rtc0 9: 0 33 IO-APIC 9-fasteoi acpi 10: 88258 0 IO-APIC 10-edge AMD0010:00 11: 0 32485 IO-APIC 11-edge AMD0010:01 24: 3301 0 amd_gpio 3 cr50_i2c 25: 0 235214 IO-APIC 28-fasteoi amdgpu 26: 67408 0 IO-APIC 31-fasteoi xhci-hcd:usb1 27: 0 488876 IO-APIC 8-fasteoi mmc0 28: 1265 0 amd_gpio 9 PNP0C50:00 29: 656 0 amd_gpio 12 ELAN9004:00 30: 413 0 amd_gpio 31 chromeos-ec 31: 14153 0 IO-APIC 4-fasteoi ath10k_pci 32: 2 0 sysfstrig0 cros-ec-accel_consumer3 33: 2 0 sysfstrig0 cros-ec-accel_consumer0 34: 6 0 amd_gpio 62 rt5682 35: 0 38937 IO-APIC 29-fasteoi snd_hda_intel:card0, ACP3x_I2S_IRQ
Cq-Depend: chrome-internal:3452710 Signed-off-by: Raul E Rangel rrangel@chromium.org Change-Id: I3211ab351a332fafb7b5f9ef486bb6646d9a214c --- M src/soc/amd/picasso/pcie_gpp.c 1 file changed, 100 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/48668/1
diff --git a/src/soc/amd/picasso/pcie_gpp.c b/src/soc/amd/picasso/pcie_gpp.c index ca368f9..8b6af4a 100644 --- a/src/soc/amd/picasso/pcie_gpp.c +++ b/src/soc/amd/picasso/pcie_gpp.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */
#include <acpi/acpigen.h> +#include <arch/ioapic.h> #include <assert.h> #include <amdblocks/amd_pci_util.h> #include <device/device.h> @@ -139,9 +140,36 @@ }
acpigen_write_method("_PRT", 0); + + /* If (PMOD) */ + acpigen_write_if(); + acpigen_emit_namestring("PMOD"); + + /* Return (Package{...}) */ acpigen_emit_byte(RETURN_OP);
- acpigen_write_package(4); + acpigen_write_package(4); /* Package - APIC Routing */ + for (unsigned int i = 0; i < 4; ++i) { + irq_index = calculate_irq(pci_routing, i); + + acpigen_write_package(4); + acpigen_write_dword(0x0000FFFF); + acpigen_write_byte(i); + acpigen_write_byte(0); /* Source: GSI */ + /* GNB IO-APIC is located after the FCH IO-APIC */ + acpigen_write_dword(IO_APIC_INTERRUPTS + irq_index); + acpigen_pop_len(); + } + acpigen_pop_len(); /* Package - APIC Routing */ + acpigen_pop_len(); /* End If */ + + /* Else */ + acpigen_write_else(); + + /* Return (Package{...}) */ + acpigen_emit_byte(RETURN_OP); + + acpigen_write_package(4); /* Package - PIC Routing */ for (unsigned int i = 0; i < 4; ++i) { irq_index = calculate_irq(pci_routing, i);
@@ -154,7 +182,9 @@ acpigen_write_dword(0); acpigen_pop_len(); } - acpigen_pop_len(); /* Package */ + acpigen_pop_len(); /* Package - PIC Routing */ + + acpigen_pop_len(); /* End Else */
acpigen_pop_len(); /* Method */ } @@ -174,40 +204,80 @@ * * Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table * { - * Return (Package (0x04) + * If (PMOD) * { - * Package (0x04) + * Return (Package (0x04) * { - * 0x0000FFFF, - * 0x00, - * _SB.INTE, - * 0x00000000 - * }, + * Package (0x04) + * { + * 0x0000FFFF, + * 0x00, + * 0x00, + * 0x00000034 + * }, * - * Package (0x04) - * { - * 0x0000FFFF, - * 0x01, - * _SB.INTF, - * 0x00000000 - * }, + * Package (0x04) + * { + * 0x0000FFFF, + * 0x01, + * 0x00, + * 0x00000035 + * }, * - * Package (0x04) - * { - * 0x0000FFFF, - * 0x02, - * _SB.INTG, - * 0x00000000 - * }, + * Package (0x04) + * { + * 0x0000FFFF, + * 0x02, + * 0x00, + * 0x00000036 + * }, * - * Package (0x04) + * Package (0x04) + * { + * 0x0000FFFF, + * 0x03, + * 0x00, + * 0x00000037 + * } + * }) + * } + * Else + * { + * Return (Package (0x04) * { - * 0x0000FFFF, - * 0x03, - * _SB.INTH, - * 0x00000000 - * } - * }) + * Package (0x04) + * { + * 0x0000FFFF, + * 0x00, + * _SB.INTE, + * 0x00000000 + * }, + * + * Package (0x04) + * { + * 0x0000FFFF, + * 0x01, + * _SB.INTF, + * 0x00000000 + * }, + * + * Package (0x04) + * { + * 0x0000FFFF, + * 0x02, + * _SB.INTG, + * 0x00000000 + * }, + * + * Package (0x04) + * { + * 0x0000FFFF, + * 0x03, + * _SB.INTH, + * 0x00000000 + * } + * }) + * } * } * } * }
Nikolai Vyssotski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48668 )
Change subject: soc/amd/picasso: Generate GNB IO-APIC PCI routing table ......................................................................
Patch Set 1: Code-Review+1
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48668 )
Change subject: soc/amd/picasso: Generate GNB IO-APIC PCI routing table ......................................................................
Patch Set 1:
it's probably a good idea to add this patch https://review.coreboot.org/c/coreboot/+/48269 before this one, so that the GNB IOAPIC's id matches what's in the MADT
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48668 )
Change subject: soc/amd/picasso: Generate GNB IO-APIC PCI routing table ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48668/1/src/soc/amd/picasso/pcie_gp... File src/soc/amd/picasso/pcie_gpp.c:
https://review.coreboot.org/c/coreboot/+/48668/1/src/soc/amd/picasso/pcie_gp... PS1, Line 144: PMOD What does PMOD mean? Is it what chooses whether to route using the GNB or the FCH IOAPIC?
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48668 )
Change subject: soc/amd/picasso: Generate GNB IO-APIC PCI routing table ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48668/1/src/soc/amd/picasso/pcie_gp... File src/soc/amd/picasso/pcie_gpp.c:
https://review.coreboot.org/c/coreboot/+/48668/1/src/soc/amd/picasso/pcie_gp... PS1, Line 144: PMOD
What does PMOD mean? Is it what chooses whether to route using the GNB or the FCH IOAPIC?
It's PIC vs APIC
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th...
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48668 )
Change subject: soc/amd/picasso: Generate GNB IO-APIC PCI routing table ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48668/1/src/soc/amd/picasso/pcie_gp... File src/soc/amd/picasso/pcie_gpp.c:
https://review.coreboot.org/c/coreboot/+/48668/1/src/soc/amd/picasso/pcie_gp... PS1, Line 144: PMOD
It's PIC vs APIC […]
Ah, alright. Thanks for the clarification
Attention is currently required from: Raul Rangel. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48668 )
Change subject: soc/amd/picasso: Generate GNB IO-APIC PCI routing table ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Patchset:
PS1: i locally rebased this on top of CB:49223 and CB:48269 and it resulted in mandolin successfully booting to linux and the irq numbers looked right
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48668 )
Change subject: soc/amd/picasso: Generate GNB IO-APIC PCI routing table ......................................................................
soc/amd/picasso: Generate GNB IO-APIC PCI routing table
This adds support for generating a PCI routing table that routes to the GNB IO-APIC. This means we no longer need to route to the FCH IO-APIC for PCI interrupts.
BUG=b:170595019 TEST=Boot ezkinil to OS with `pci=nomsi amd_iommu=off` and verify all peripherals are working
CPU0 CPU1 0: 112 0 IO-APIC 2-edge timer 1: 0 99 IO-APIC 1-edge i8042 4: 0 2523 IO-APIC 4-edge ttyS0 5: 34632 0 IO-APIC 5-fasteoi mmc1 7: 5646 0 IO-APIC 7-fasteoi pinctrl_amd 8: 0 0 IO-APIC 8-edge rtc0 9: 0 33 IO-APIC 9-fasteoi acpi 10: 88258 0 IO-APIC 10-edge AMD0010:00 11: 0 32485 IO-APIC 11-edge AMD0010:01 24: 3301 0 amd_gpio 3 cr50_i2c 25: 0 235214 IO-APIC 28-fasteoi amdgpu 26: 67408 0 IO-APIC 31-fasteoi xhci-hcd:usb1 27: 0 488876 IO-APIC 8-fasteoi mmc0 28: 1265 0 amd_gpio 9 PNP0C50:00 29: 656 0 amd_gpio 12 ELAN9004:00 30: 413 0 amd_gpio 31 chromeos-ec 31: 14153 0 IO-APIC 4-fasteoi ath10k_pci 32: 2 0 sysfstrig0 cros-ec-accel_consumer3 33: 2 0 sysfstrig0 cros-ec-accel_consumer0 34: 6 0 amd_gpio 62 rt5682 35: 0 38937 IO-APIC 29-fasteoi snd_hda_intel:card0, ACP3x_I2S_IRQ
Cq-Depend: chrome-internal:3452710 Signed-off-by: Raul E Rangel rrangel@chromium.org Change-Id: I3211ab351a332fafb7b5f9ef486bb6646d9a214c Reviewed-on: https://review.coreboot.org/c/coreboot/+/48668 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nikolai Vyssotski nikolai.vyssotski@amd.corp-partner.google.com Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/soc/amd/picasso/pcie_gpp.c 1 file changed, 100 insertions(+), 30 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved Nikolai Vyssotski: Looks good to me, but someone else must approve
diff --git a/src/soc/amd/picasso/pcie_gpp.c b/src/soc/amd/picasso/pcie_gpp.c index ca368f9..8b6af4a 100644 --- a/src/soc/amd/picasso/pcie_gpp.c +++ b/src/soc/amd/picasso/pcie_gpp.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */
#include <acpi/acpigen.h> +#include <arch/ioapic.h> #include <assert.h> #include <amdblocks/amd_pci_util.h> #include <device/device.h> @@ -139,9 +140,36 @@ }
acpigen_write_method("_PRT", 0); + + /* If (PMOD) */ + acpigen_write_if(); + acpigen_emit_namestring("PMOD"); + + /* Return (Package{...}) */ acpigen_emit_byte(RETURN_OP);
- acpigen_write_package(4); + acpigen_write_package(4); /* Package - APIC Routing */ + for (unsigned int i = 0; i < 4; ++i) { + irq_index = calculate_irq(pci_routing, i); + + acpigen_write_package(4); + acpigen_write_dword(0x0000FFFF); + acpigen_write_byte(i); + acpigen_write_byte(0); /* Source: GSI */ + /* GNB IO-APIC is located after the FCH IO-APIC */ + acpigen_write_dword(IO_APIC_INTERRUPTS + irq_index); + acpigen_pop_len(); + } + acpigen_pop_len(); /* Package - APIC Routing */ + acpigen_pop_len(); /* End If */ + + /* Else */ + acpigen_write_else(); + + /* Return (Package{...}) */ + acpigen_emit_byte(RETURN_OP); + + acpigen_write_package(4); /* Package - PIC Routing */ for (unsigned int i = 0; i < 4; ++i) { irq_index = calculate_irq(pci_routing, i);
@@ -154,7 +182,9 @@ acpigen_write_dword(0); acpigen_pop_len(); } - acpigen_pop_len(); /* Package */ + acpigen_pop_len(); /* Package - PIC Routing */ + + acpigen_pop_len(); /* End Else */
acpigen_pop_len(); /* Method */ } @@ -174,40 +204,80 @@ * * Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table * { - * Return (Package (0x04) + * If (PMOD) * { - * Package (0x04) + * Return (Package (0x04) * { - * 0x0000FFFF, - * 0x00, - * _SB.INTE, - * 0x00000000 - * }, + * Package (0x04) + * { + * 0x0000FFFF, + * 0x00, + * 0x00, + * 0x00000034 + * }, * - * Package (0x04) - * { - * 0x0000FFFF, - * 0x01, - * _SB.INTF, - * 0x00000000 - * }, + * Package (0x04) + * { + * 0x0000FFFF, + * 0x01, + * 0x00, + * 0x00000035 + * }, * - * Package (0x04) - * { - * 0x0000FFFF, - * 0x02, - * _SB.INTG, - * 0x00000000 - * }, + * Package (0x04) + * { + * 0x0000FFFF, + * 0x02, + * 0x00, + * 0x00000036 + * }, * - * Package (0x04) + * Package (0x04) + * { + * 0x0000FFFF, + * 0x03, + * 0x00, + * 0x00000037 + * } + * }) + * } + * Else + * { + * Return (Package (0x04) * { - * 0x0000FFFF, - * 0x03, - * _SB.INTH, - * 0x00000000 - * } - * }) + * Package (0x04) + * { + * 0x0000FFFF, + * 0x00, + * _SB.INTE, + * 0x00000000 + * }, + * + * Package (0x04) + * { + * 0x0000FFFF, + * 0x01, + * _SB.INTF, + * 0x00000000 + * }, + * + * Package (0x04) + * { + * 0x0000FFFF, + * 0x02, + * _SB.INTG, + * 0x00000000 + * }, + * + * Package (0x04) + * { + * 0x0000FFFF, + * 0x03, + * _SB.INTH, + * 0x00000000 + * } + * }) + * } * } * } * }
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48668 )
Change subject: soc/amd/picasso: Generate GNB IO-APIC PCI routing table ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2: this shouldn't have been submitted before CB:49223; without that patch being in tree, this one will cause a kernel hang which will cause a false positive when bisecting. I've submitted the required patches now, so there's no action needed any more
-
Angel Pons (Code Review) -
Felix Held (Code Review)
-
Nikolai Vyssotski (Code Review)
-
Patrick Georgi (Code Review)
-
Raul Rangel (Code Review)