Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35108 )
Change subject: [UNTESTED] skylake: do not rely on P2SB data to generate DRHD ......................................................................
[UNTESTED] skylake: do not rely on P2SB data to generate DRHD
The P2SB PCI device can be "hidden", which causes all sorts of nightmares and bugs. Moreover, FSP tends to hide it, so finding a good solution to this problem is impossible with FSP into the mix.
Since the values for IBDF and HBDF were already hardcoded as FSP parameters, write them with coreboot code and use these values directly to generate the DRHD, so as to avoid using the P2SB when it might be already hidden.
Note that the possibility of fireworks happening has not been ruled out.
Change-Id: I7eb20182380b953a1842083e7a3c67919d6971b9 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/skylake/acpi.c M src/soc/intel/skylake/bootblock/pch.c M src/soc/intel/skylake/chip_fsp20.c M src/soc/intel/skylake/include/soc/systemagent.h M src/soc/intel/skylake/romstage/romstage_fsp20.c 5 files changed, 29 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/35108/1
diff --git a/src/soc/intel/skylake/acpi.c b/src/soc/intel/skylake/acpi.c index aa51cbe..9c5b674 100644 --- a/src/soc/intel/skylake/acpi.c +++ b/src/soc/intel/skylake/acpi.c @@ -30,7 +30,6 @@ #include <ec/google/chromeec/ec.h> #include <intelblocks/cpulib.h> #include <intelblocks/lpc_lib.h> -#include <intelblocks/p2sb.h> #include <intelblocks/sgx.h> #include <intelblocks/uart.h> #include <intelblocks/systemagent.h> @@ -575,30 +574,19 @@ acpi_dmar_rmrr_fixup(tmp, current); }
- struct device *const p2sb_dev = pcidev_path_on_root(PCH_DEVFN_P2SB); const u32 vtvc0bar = MCHBAR32(VTVC0BAR) & ~0xfff; const bool vtvc0en = MCHBAR32(VTVC0BAR) & 1;
/* General VTBAR has to be set and in 32-bit space. */ - if (p2sb_dev && vtvc0bar && vtvc0en && !MCHBAR32(VTVC0BAR + 4)) { + if (vtvc0bar && vtvc0en && !MCHBAR32(VTVC0BAR + 4)) { const unsigned long tmp = current;
- /* P2SB may already be hidden. There's no clear rule, when. */ - const u8 p2sb_hidden = - pci_read_config8(p2sb_dev, PCH_P2SB_E0 + 1); - pci_write_config8(p2sb_dev, PCH_P2SB_E0 + 1, 0); + current += acpi_create_dmar_drhd(current, DRHD_INCLUDE_PCI_ALL, 0, vtvc0bar);
- const u16 ibdf = pci_read_config16(p2sb_dev, PCH_P2SB_IBDF); - const u16 hbdf = pci_read_config16(p2sb_dev, PCH_P2SB_HBDF); - - pci_write_config8(p2sb_dev, PCH_P2SB_E0 + 1, p2sb_hidden); - - current += acpi_create_dmar_drhd(current, - DRHD_INCLUDE_PCI_ALL, 0, vtvc0bar); - current += acpi_create_dmar_ds_ioapic(current, - 2, ibdf >> 8, PCI_SLOT(ibdf), PCI_FUNC(ibdf)); - current += acpi_create_dmar_ds_msi_hpet(current, - 0, hbdf >> 8, PCI_SLOT(hbdf), PCI_FUNC(hbdf)); + current += acpi_create_dmar_ds_ioapic(current, 2, V_P2SB_IBDF_BUS, + V_P2SB_IBDF_DEV, V_P2SB_IBDF_FUN); + current += acpi_create_dmar_ds_msi_hpet(current, 0, V_P2SB_HBDF_BUS, + V_P2SB_HBDF_DEV, V_P2SB_HBDF_FUN);
acpi_dmar_drhd_fixup(tmp, current); } diff --git a/src/soc/intel/skylake/bootblock/pch.c b/src/soc/intel/skylake/bootblock/pch.c index c95a8d8..cc43415 100644 --- a/src/soc/intel/skylake/bootblock/pch.c +++ b/src/soc/intel/skylake/bootblock/pch.c @@ -35,6 +35,7 @@ #include <soc/pm.h> #include <soc/pmc.h> #include <soc/smbus.h> +#include <soc/systemagent.h>
#include "../chip.h"
@@ -50,6 +51,12 @@ fast_spi_early_init(SPI_BASE_ADDRESS); p2sb_enable_bar(); p2sb_configure_hpet(); + + // Write the IOAPIC Bus:Dev.Fun value to P2SB IBDF register + pci_write_config16(PCH_DEV_P2SB, PCH_P2SB_IBDF, V_DEFAULT_IBDF); + + // Write the HPET Bus:Dev.Fun value to P2SB HBDF register + pci_write_config16(PCH_DEV_P2SB, PCH_P2SB_HBDF, V_DEFAULT_HBDF); }
static void soc_config_acpibase(void) diff --git a/src/soc/intel/skylake/chip_fsp20.c b/src/soc/intel/skylake/chip_fsp20.c index 064f71e..fa39607 100644 --- a/src/soc/intel/skylake/chip_fsp20.c +++ b/src/soc/intel/skylake/chip_fsp20.c @@ -484,10 +484,8 @@ params->X2ApicOptOut = 0; tconfig->VtdDisable = 0;
- params->PchIoApicBdfValid = 1; - params->PchIoApicBusNumber = 250; - params->PchIoApicDeviceNumber = 31; - params->PchIoApicFunctionNumber = 0; + // We set that in coreboot already + params->PchIoApicBdfValid = 0; }
soc_irq_settings(params); diff --git a/src/soc/intel/skylake/include/soc/systemagent.h b/src/soc/intel/skylake/include/soc/systemagent.h index d7dec65..42b4eec 100644 --- a/src/soc/intel/skylake/include/soc/systemagent.h +++ b/src/soc/intel/skylake/include/soc/systemagent.h @@ -61,4 +61,16 @@ VTVC0_BASE_SIZE, "VTVC0BAR" }; + +// Hardcoded default values for PCI Bus:Dev.Fun for IOAPIC and HPET +#define V_P2SB_IBDF_BUS 250 +#define V_P2SB_IBDF_DEV 31 +#define V_P2SB_IBDF_FUN 0 +#define V_DEFAULT_IBDF ((V_P2SB_IBDF_BUS << 8) | PCI_DEVFN(V_P2SB_IBDF_DEV, V_P2SB_IBDF_FUN)) + +#define V_P2SB_HBDF_BUS 250 +#define V_P2SB_HBDF_DEV 15 +#define V_P2SB_HBDF_FUN 0 +#define V_DEFAULT_HBDF ((V_P2SB_HBDF_BUS << 8) | PCI_DEVFN(V_P2SB_HBDF_DEV, V_P2SB_HBDF_FUN)) + #endif diff --git a/src/soc/intel/skylake/romstage/romstage_fsp20.c b/src/soc/intel/skylake/romstage/romstage_fsp20.c index 8b5cd18..00cb6fe 100644 --- a/src/soc/intel/skylake/romstage/romstage_fsp20.c +++ b/src/soc/intel/skylake/romstage/romstage_fsp20.c @@ -288,10 +288,8 @@ cpu_flex_override(m_cfg);
if (!config->ignore_vtd) { - m_cfg->PchHpetBdfValid = 1; - m_cfg->PchHpetBusNumber = 250; - m_cfg->PchHpetDeviceNumber = 15; - m_cfg->PchHpetFunctionNumber = 0; + // We set that in coreboot already + m_cfg->PchHpetBdfValid = 0; } }