Hello Jason Glenesk,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/45050
to review the following change.
Change subject: soc/amd/picasso/acpi: Fix hang caused by IVRS ......................................................................
soc/amd/picasso/acpi: Fix hang caused by IVRS
Previous commit misinterpreted spec as requiring size alignment on all IVHD device entries. The correct requirement specifies only for 4-byte entries. Additionally, the MADT was missing an entry for the second ioapic described in the IVRS.
Remove 8-byte entry alignment and add second ioapic to MADT.
BUG=b:166519072 TEST=Boot fully to morphius board with and without amd_iommu kernel parameter. Dump MADT and IVRS tables. Cross check ioapic entries in MADT against IVRS. Confirm IVRS contains no alignment gaps/corruption.
Change-Id: Iddcff98279be1d910936b13391dd2448a3bb2d74 Signed-off-by: Jason Glenesk jason.glenesk@amd.corp-partner.google.com --- M src/soc/amd/picasso/acpi.c M src/soc/amd/picasso/agesa_acpi.c M src/soc/amd/picasso/include/soc/smu.h 3 files changed, 12 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/45050/1
diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c index 1b9c0ca..1aacf5d 100644 --- a/src/soc/amd/picasso/acpi.c +++ b/src/soc/amd/picasso/acpi.c @@ -26,6 +26,7 @@ #include <soc/gpio.h> #include <version.h> #include "chip.h" +#include <soc/smu.h>
unsigned long acpi_fill_mcfg(unsigned long current) { @@ -45,14 +46,23 @@ unsigned int i; uint8_t irq; uint8_t flags; + uint32_t nb_ioapic_base;
/* create all subtables for processors */ current = acpi_create_madt_lapics(current);
- /* Write Kern IOAPIC, only one */ + /* Write FCH IOAPIC */ current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *)current, CONFIG_MAX_CPUS, IO_APIC_ADDR, 0);
+ /* Read base address assigned to gnb ioapic */ + pci_write_config32(SOC_GNB_DEV, SMU_INDEX_ADDR, REG_NB_IOAPIC_BASE); + nb_ioapic_base = pci_read_config32(SOC_GNB_DEV, SMU_DATA_ADDR); + + /* Write GNB IOAPIC */ + current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *)current, + CONFIG_MAX_CPUS + 1, nb_ioapic_base, 32); + /* 0: mean bus 0--->ISA */ /* 0: PIC 0 */ /* 2: APIC 2 */ diff --git a/src/soc/amd/picasso/agesa_acpi.c b/src/soc/amd/picasso/agesa_acpi.c index a651d6e..c76e943 100644 --- a/src/soc/amd/picasso/agesa_acpi.c +++ b/src/soc/amd/picasso/agesa_acpi.c @@ -47,8 +47,6 @@
unsigned long acpi_fill_ivrs_ioapic(acpi_ivrs_t *ivrs, unsigned long current) { - /* 8-byte IVHD structures must be aligned to the 8-byte boundary. */ - current = ALIGN_UP(current, 8); ivrs_ivhd_special_t *ivhd_ioapic = (ivrs_ivhd_special_t *)current; memset(ivhd_ioapic, 0, sizeof(*ivhd_ioapic));
@@ -75,8 +73,6 @@
static unsigned long ivhd_describe_hpet(unsigned long current) { - /* 8-byte IVHD structures must be aligned to the 8-byte boundary. */ - current = ALIGN_UP(current, 8); ivrs_ivhd_special_t *ivhd_hpet = (ivrs_ivhd_special_t *)current;
ivhd_hpet->type = IVHD_DEV_8_BYTE_EXT_SPECIAL_DEV; @@ -93,8 +89,6 @@ static unsigned long ivhd_describe_f0_device(unsigned long current, uint16_t dev_id, uint8_t datasetting) { - /* 8-byte IVHD structures must be aligned to the 8-byte boundary. */ - current = ALIGN_UP(current, 8); ivrs_ivhd_f0_entry_t *ivhd_f0 = (ivrs_ivhd_f0_entry_t *) current;
ivhd_f0->type = IVHD_DEV_VARIABLE; @@ -154,8 +148,6 @@ ivhd_entry->dte_setting = data; *current += sizeof(ivrs_ivhd_generic_t); } else if (type == IVHD_DEV_8_BYTE_ALIAS_SELECT) { - /* 8-byte IVHD structures must be aligned to the 8-byte boundary. */ - *current = ALIGN_UP(*current, 8); ivrs_ivhd_alias_t *ivhd_entry = (ivrs_ivhd_alias_t *)*current;
ivhd_entry->type = type; diff --git a/src/soc/amd/picasso/include/soc/smu.h b/src/soc/amd/picasso/include/soc/smu.h index 128f4c4..bc9f4f3 100644 --- a/src/soc/amd/picasso/include/soc/smu.h +++ b/src/soc/amd/picasso/include/soc/smu.h @@ -12,6 +12,7 @@ #define REG_ADDR_MESG_ID 0x3b10528 #define REG_ADDR_MESG_RESP 0x3b10564 #define REG_ADDR_MESG_ARGS_BASE 0x0b10998 +#define REG_NB_IOAPIC_BASE 0x13b102f0
/* Argument 0-5 indexed locations are contiguous */ #define SMU_NUM_ARGS 6