Nico Huber submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Mimoja: Looks good to me, but someone else must approve Maxim Polyakov: Looks good to me, approved
soc/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, define them as macros and use these values directly to
generate the DRHD.

Change-Id: I7eb20182380b953a1842083e7a3c67919d6971b9
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35108
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Mimoja <coreboot@mimoja.de>
Reviewed-by: Maxim Polyakov <max.senia.poliak@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
---
M src/soc/intel/skylake/acpi.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
4 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/src/soc/intel/skylake/acpi.c b/src/soc/intel/skylake/acpi.c
index ccfc7b7..c3757b0 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>
@@ -39,7 +38,6 @@
#include <soc/cpu.h>
#include <soc/iomap.h>
#include <soc/msr.h>
-#include <soc/p2sb.h>
#include <soc/pci_devs.h>
#include <soc/pm.h>
#include <soc/ramstage.h>
@@ -575,30 +573,20 @@
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);
+ current += acpi_create_dmar_ds_ioapic(current, 2, V_P2SB_IBDF_BUS,
+ V_P2SB_IBDF_DEV, V_P2SB_IBDF_FUN);

- 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_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/chip_fsp20.c b/src/soc/intel/skylake/chip_fsp20.c
index e9c37d6..de86936 100644
--- a/src/soc/intel/skylake/chip_fsp20.c
+++ b/src/soc/intel/skylake/chip_fsp20.c
@@ -490,9 +490,9 @@
tconfig->VtdDisable = 0;

params->PchIoApicBdfValid = 1;
- params->PchIoApicBusNumber = 250;
- params->PchIoApicDeviceNumber = 31;
- params->PchIoApicFunctionNumber = 0;
+ params->PchIoApicBusNumber = V_P2SB_IBDF_BUS;
+ params->PchIoApicDeviceNumber = V_P2SB_IBDF_DEV;
+ params->PchIoApicFunctionNumber = V_P2SB_IBDF_FUN;
}

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..565c885 100644
--- a/src/soc/intel/skylake/include/soc/systemagent.h
+++ b/src/soc/intel/skylake/include/soc/systemagent.h
@@ -61,4 +61,14 @@
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_P2SB_HBDF_BUS 250
+#define V_P2SB_HBDF_DEV 15
+#define V_P2SB_HBDF_FUN 0
+
#endif
diff --git a/src/soc/intel/skylake/romstage/romstage_fsp20.c b/src/soc/intel/skylake/romstage/romstage_fsp20.c
index ecd1428..002a284 100644
--- a/src/soc/intel/skylake/romstage/romstage_fsp20.c
+++ b/src/soc/intel/skylake/romstage/romstage_fsp20.c
@@ -30,6 +30,7 @@
#include <soc/pci_devs.h>
#include <soc/pm.h>
#include <soc/romstage.h>
+#include <soc/systemagent.h>
#include <string.h>
#include <security/vboot/vboot_common.h>

@@ -247,9 +248,9 @@

if (!config->ignore_vtd) {
m_cfg->PchHpetBdfValid = 1;
- m_cfg->PchHpetBusNumber = 250;
- m_cfg->PchHpetDeviceNumber = 15;
- m_cfg->PchHpetFunctionNumber = 0;
+ m_cfg->PchHpetBusNumber = V_P2SB_HBDF_BUS;
+ m_cfg->PchHpetDeviceNumber = V_P2SB_HBDF_DEV;
+ m_cfg->PchHpetFunctionNumber = V_P2SB_HBDF_FUN;
}
}


To view, visit change 35108. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7eb20182380b953a1842083e7a3c67919d6971b9
Gerrit-Change-Number: 35108
Gerrit-PatchSet: 10
Gerrit-Owner: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com>
Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak@gmail.com>
Gerrit-Reviewer: Mimoja <coreboot@mimoja.de>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged