Lean Sheng Tan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84794?usp=email )
Change subject: soc/intel/xeon_sp: Add SAD PCI driver ......................................................................
soc/intel/xeon_sp: Add SAD PCI driver
Get rid of some helper functions by properly using a pci_driver.
Configure SAD if necessary and lock SAD if necessary in the newly added SAD PCI driver. This allows to drop lock_pam0123(), unlock_pam_regions() and socket0_get_ubox_busno().
- Fixes SAD instance on secondary sockets not decoding the C-F segments as DRAM, which would prevent those sockets to access the ACPI/SMBIOS table anchor - Adds PCI multi segment support (SKX and CPX only, other were working properly already) - Moves locking of PAM0123_CSR and PAM456_CSR from SoC to driver code
Change-Id: I167b6ce48631fe3f97359ee33704f52ca854dbd1 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/84794 Reviewed-by: Shuo Liu shuo.liu@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/xeon_sp/Makefile.mk M src/soc/intel/xeon_sp/chip_gen1.c M src/soc/intel/xeon_sp/cpx/chip.c M src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h M src/soc/intel/xeon_sp/cpx/soc_util.c M src/soc/intel/xeon_sp/finalize.c M src/soc/intel/xeon_sp/gnr/include/soc/pci_devs.h M src/soc/intel/xeon_sp/include/soc/chip_common.h M src/soc/intel/xeon_sp/include/soc/util.h A src/soc/intel/xeon_sp/sad.c M src/soc/intel/xeon_sp/skx/chip.c M src/soc/intel/xeon_sp/skx/include/soc/soc_util.h M src/soc/intel/xeon_sp/skx/soc_util.c M src/soc/intel/xeon_sp/spr/include/soc/pci_devs.h M src/soc/intel/xeon_sp/spr/include/soc/soc_util.h M src/soc/intel/xeon_sp/spr/soc_util.c M src/soc/intel/xeon_sp/util.c 17 files changed, 72 insertions(+), 90 deletions(-)
Approvals: build bot (Jenkins): Verified Shuo Liu: Looks good to me, approved
diff --git a/src/soc/intel/xeon_sp/Makefile.mk b/src/soc/intel/xeon_sp/Makefile.mk index 1f68f27..0fc32d8 100644 --- a/src/soc/intel/xeon_sp/Makefile.mk +++ b/src/soc/intel/xeon_sp/Makefile.mk @@ -21,6 +21,8 @@ ramstage-$(CONFIG_SOC_INTEL_HAS_CXL) += uncore_acpi_cxl.c ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smmrelocate.c ramstage-$(CONFIG_XEON_SP_HAVE_IIO_IOAPIC) += iio_ioapic.c +ramstage-y += sad.c + smm-y += smihandler.c pmutil.c postcar-y += spi.c
diff --git a/src/soc/intel/xeon_sp/chip_gen1.c b/src/soc/intel/xeon_sp/chip_gen1.c index ee167e2..9737d11 100644 --- a/src/soc/intel/xeon_sp/chip_gen1.c +++ b/src/soc/intel/xeon_sp/chip_gen1.c @@ -181,28 +181,3 @@ else if (CONFIG(HAVE_IOAT_DOMAINS) && is_ioat_iio_stack_res(sr)) create_ioat_domains(dp, bus, sr, pci_segment_group); } - -/* - * Route PAM segment access to DRAM - * Only call this code from socket0! - */ -void unlock_pam_regions(void) -{ - uint32_t pam0123_unlock_dram = 0x33333330; - uint32_t pam456_unlock_dram = 0x00333333; - /* Get UBOX(1) for socket0 */ - uint32_t bus1 = socket0_get_ubox_busno(PCU_IIO_STACK); - - /* Assume socket0 owns PCI segment 0 */ - pci_io_write_config32(PCI_DEV(bus1, SAD_ALL_DEV, SAD_ALL_FUNC), - SAD_ALL_PAM0123_CSR, pam0123_unlock_dram); - pci_io_write_config32(PCI_DEV(bus1, SAD_ALL_DEV, SAD_ALL_FUNC), - SAD_ALL_PAM456_CSR, pam456_unlock_dram); - - uint32_t reg1 = pci_io_read_config32(PCI_DEV(bus1, SAD_ALL_DEV, - SAD_ALL_FUNC), SAD_ALL_PAM0123_CSR); - uint32_t reg2 = pci_io_read_config32(PCI_DEV(bus1, SAD_ALL_DEV, - SAD_ALL_FUNC), SAD_ALL_PAM456_CSR); - printk(BIOS_DEBUG, "%s:%s pam0123_csr: 0x%x, pam456_csr: 0x%x\n", - __FILE__, __func__, reg1, reg2); -} diff --git a/src/soc/intel/xeon_sp/cpx/chip.c b/src/soc/intel/xeon_sp/cpx/chip.c index e6b2bdb..afa5357 100644 --- a/src/soc/intel/xeon_sp/cpx/chip.c +++ b/src/soc/intel/xeon_sp/cpx/chip.c @@ -163,8 +163,6 @@
static void chip_init(void *data) { - unlock_pam_regions(); - printk(BIOS_DEBUG, "coreboot: calling fsp_silicon_init\n"); fsp_silicon_init();
diff --git a/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h b/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h index 60b2a82..a92d9d0 100644 --- a/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h +++ b/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h @@ -34,7 +34,6 @@
const struct SystemMemoryMapHob *get_system_memory_map(void);
-uint8_t socket0_get_ubox_busno(const uint8_t stack); uint8_t get_cxl_node_count(void);
int soc_get_stack_for_port(int port); diff --git a/src/soc/intel/xeon_sp/cpx/soc_util.c b/src/soc/intel/xeon_sp/cpx/soc_util.c index aac5589..cc872c0 100644 --- a/src/soc/intel/xeon_sp/cpx/soc_util.c +++ b/src/soc/intel/xeon_sp/cpx/soc_util.c @@ -35,18 +35,6 @@ return res->Personality == TYPE_UBOX; }
-/* Returns the UBOX(stack) bus number when called from socket0 */ -uint8_t socket0_get_ubox_busno(const uint8_t stack) -{ - if (stack >= MAX_IIO_STACK) { - printk(BIOS_ERR, "%s: Stack %u does not exist!\n", __func__, stack); - return 0; - } - const pci_devfn_t dev = PCI_DEV(UBOX_DECS_BUS, UBOX_DECS_DEV, UBOX_DECS_FUNC); - const uint16_t offset = stack / 4 ? UBOX_DECS_CPUBUSNO1_CSR : UBOX_DECS_CPUBUSNO_CSR; - return pci_io_read_config32(dev, offset) >> (8 * (stack % 4)) & 0xff; -} - /* * EX: CPX-SP * Ports Stack Stack(HOB) IioConfigIou diff --git a/src/soc/intel/xeon_sp/finalize.c b/src/soc/intel/xeon_sp/finalize.c index fa39eb6..a7b36027 100644 --- a/src/soc/intel/xeon_sp/finalize.c +++ b/src/soc/intel/xeon_sp/finalize.c @@ -60,7 +60,6 @@ setbits8(pmc_mmio_regs() + PCH_PWRM_ACPI_TMR_CTL, ACPI_TIM_DIS);
apm_control(APM_CNT_FINALIZE); - lock_pam0123();
if (CONFIG_MAX_SOCKET > 1) { /* This MSR is package scope but run for all cpus for code simplicity */ diff --git a/src/soc/intel/xeon_sp/gnr/include/soc/pci_devs.h b/src/soc/intel/xeon_sp/gnr/include/soc/pci_devs.h index e2c63db..fab090a 100644 --- a/src/soc/intel/xeon_sp/gnr/include/soc/pci_devs.h +++ b/src/soc/intel/xeon_sp/gnr/include/soc/pci_devs.h @@ -37,6 +37,7 @@ #define SAD_ALL_DEV CHA_DEV #define SAD_ALL_FUNC 0 #define SAD_ALL_PAM0123_CSR 0x80 +#define PAM_LOCK BIT(0) #define SAD_ALL_PAM456_CSR 0x84 #define SAD_ALL_DEVID 0x344f
diff --git a/src/soc/intel/xeon_sp/include/soc/chip_common.h b/src/soc/intel/xeon_sp/include/soc/chip_common.h index ed278a3..73eeea5 100644 --- a/src/soc/intel/xeon_sp/include/soc/chip_common.h +++ b/src/soc/intel/xeon_sp/include/soc/chip_common.h @@ -85,8 +85,6 @@
#define is_stack0(socket, stack) (socket == 0 && stack == IioStack0)
-void unlock_pam_regions(void); - size_t vtd_probe_bar_size(struct device *dev);
void soc_pci_domain_fill_ssdt(const struct device *domain); diff --git a/src/soc/intel/xeon_sp/include/soc/util.h b/src/soc/intel/xeon_sp/include/soc/util.h index a7b98f3..4c7c75f 100644 --- a/src/soc/intel/xeon_sp/include/soc/util.h +++ b/src/soc/intel/xeon_sp/include/soc/util.h @@ -9,8 +9,6 @@
#define MEM_ADDR_64MB_SHIFT_BITS 26
-void lock_pam0123(void); - msr_t read_msr_ppin(void); int get_platform_thread_count(void); const IIO_UDS *get_iio_uds(void); diff --git a/src/soc/intel/xeon_sp/sad.c b/src/soc/intel/xeon_sp/sad.c new file mode 100644 index 0000000..78545f5 --- /dev/null +++ b/src/soc/intel/xeon_sp/sad.c @@ -0,0 +1,68 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <device/pci.h> +#include <device/pci_ids.h> +#include <intelblocks/cfg.h> +#include <intelpch/lockdown.h> +#include <soc/pci_devs.h> + +/* + * Driver for SAD device (SKX and CPX only), now called CHA dev. + * SKX/CPX [B(31), D:29, F:0/F:1] + * SPR/GNR [B(31), D:29, F:0/F:1] + */ + +static void sad_enable_resources(struct device *dev) +{ + pci_dev_enable_resources(dev); + + if (!CONFIG(SOC_INTEL_SKYLAKE_SP) && !CONFIG(SOC_INTEL_COOPERLAKE_SP)) + return; + + /* + * Enables the C-F segment as DRAM. Only necessary on SKX and CPX, that do + * not unconditionally enable those segments in FSP. + * The assumption here is that FSP-S does not need to access the C-F segment, + * and nothing in coreboot should access those segments before writing tables. + */ + + uint32_t pam0123_unlock_dram = 0x33333330; + uint32_t pam456_unlock_dram = 0x00333333; + + pci_write_config32(dev, SAD_ALL_PAM0123_CSR, pam0123_unlock_dram); + pci_write_config32(dev, SAD_ALL_PAM456_CSR, pam456_unlock_dram); + + uint32_t reg1 = pci_read_config32(dev, SAD_ALL_PAM0123_CSR); + uint32_t reg2 = pci_read_config32(dev, SAD_ALL_PAM456_CSR); + printk(BIOS_DEBUG, "%s:%s pam0123_csr: 0x%x, pam456_csr: 0x%x\n", + __FILE__, __func__, reg1, reg2); +} + +static void sad_final(struct device *dev) +{ + if (!CONFIG(SOC_INTEL_COMMON_PCH_LOCKDOWN)) + return; + + if (get_lockdown_config() != CHIPSET_LOCKDOWN_COREBOOT) + return; + + pci_or_config32(dev, SAD_ALL_PAM0123_CSR, PAM_LOCK); +} + +static const unsigned short sad_ids[] = { + SAD_ALL_DEVID, + 0 +}; + +static struct device_operations sad_ops = { + .read_resources = pci_dev_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = sad_enable_resources, + .final = sad_final, +}; + +static const struct pci_driver sad_driver __pci_driver = { + .ops = &sad_ops, + .vendor = PCI_VID_INTEL, + .devices = sad_ids, +}; diff --git a/src/soc/intel/xeon_sp/skx/chip.c b/src/soc/intel/xeon_sp/skx/chip.c index 37535a3..653f2b1 100644 --- a/src/soc/intel/xeon_sp/skx/chip.c +++ b/src/soc/intel/xeon_sp/skx/chip.c @@ -38,8 +38,6 @@
static void soc_init(void *data) { - unlock_pam_regions(); - printk(BIOS_DEBUG, "coreboot: calling fsp_silicon_init\n"); fsp_silicon_init();
diff --git a/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h b/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h index b8c000e..4345e46 100644 --- a/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h +++ b/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h @@ -13,8 +13,6 @@
const struct SystemMemoryMapHob *get_system_memory_map(void);
-uint8_t socket0_get_ubox_busno(const uint8_t stack); - int soc_get_stack_for_port(int port); uint8_t get_cxl_node_count(void);
diff --git a/src/soc/intel/xeon_sp/skx/soc_util.c b/src/soc/intel/xeon_sp/skx/soc_util.c index 0938260..9d68d44 100644 --- a/src/soc/intel/xeon_sp/skx/soc_util.c +++ b/src/soc/intel/xeon_sp/skx/soc_util.c @@ -83,18 +83,6 @@ return false; }
-/* Returns the UBOX(stack) bus number when called from socket0 */ -uint8_t socket0_get_ubox_busno(const uint8_t stack) -{ - if (stack >= MAX_IIO_STACK) { - printk(BIOS_ERR, "%s: Stack %u does not exist!\n", __func__, stack); - return 0; - } - const pci_devfn_t dev = PCI_DEV(UBOX_DECS_BUS, UBOX_DECS_DEV, UBOX_DECS_FUNC); - const uint16_t offset = stack / 4 ? UBOX_DECS_CPUBUSNO1_CSR : UBOX_DECS_CPUBUSNO_CSR; - return pci_io_read_config32(dev, offset) >> (8 * (stack % 4)) & 0xff; -} - #if ENV_RAMSTAGE void config_reset_cpl3_csrs(void) { diff --git a/src/soc/intel/xeon_sp/spr/include/soc/pci_devs.h b/src/soc/intel/xeon_sp/spr/include/soc/pci_devs.h index 510a67f..ae1a020 100644 --- a/src/soc/intel/xeon_sp/spr/include/soc/pci_devs.h +++ b/src/soc/intel/xeon_sp/spr/include/soc/pci_devs.h @@ -22,6 +22,7 @@ #define SAD_ALL_DEV CHA_DEV #define SAD_ALL_FUNC 0 #define SAD_ALL_PAM0123_CSR 0x80 +#define PAM_LOCK BIT(0) #define SAD_ALL_PAM456_CSR 0x84 #define SAD_ALL_DEVID 0x344f
diff --git a/src/soc/intel/xeon_sp/spr/include/soc/soc_util.h b/src/soc/intel/xeon_sp/spr/include/soc/soc_util.h index 463e619..df498e9 100644 --- a/src/soc/intel/xeon_sp/spr/include/soc/soc_util.h +++ b/src/soc/intel/xeon_sp/spr/include/soc/soc_util.h @@ -44,7 +44,6 @@
const EWL_PRIVATE_DATA *get_ewl_hob(void);
-uint8_t socket0_get_ubox_busno(uint8_t offset); void soc_set_mrc_cold_boot_flag(bool cold_boot_required); void soc_config_iio(FSPM_UPD *mupd, const UPD_IIO_PCIE_PORT_CONFIG_ENTRY mb_iio_table[CONFIG_MAX_SOCKET][IIO_PORT_SETTINGS], const UINT8 mb_iio_bifur[CONFIG_MAX_SOCKET][5]); diff --git a/src/soc/intel/xeon_sp/spr/soc_util.c b/src/soc/intel/xeon_sp/spr/soc_util.c index 6dd06ac..a6f00c0 100644 --- a/src/soc/intel/xeon_sp/spr/soc_util.c +++ b/src/soc/intel/xeon_sp/spr/soc_util.c @@ -136,20 +136,6 @@ return count; }
-/* Returns the UBOX(offset) bus number for socket0 */ -uint8_t socket0_get_ubox_busno(uint8_t offset) -{ - const IIO_UDS *hob = get_iio_uds(); - - for (int stack = 0; stack < MAX_LOGIC_IIO_STACK; ++stack) { - if (hob->PlatformData.IIO_resource[0].StackRes[stack].Personality - == TYPE_UBOX) - return (hob->PlatformData.IIO_resource[0].StackRes[stack].BusBase - + offset); - } - die("Unable to locate UBOX BUS NO"); -} - void bios_done_msr(void *unused) { msr_t msr = rdmsr(MSR_BIOS_DONE); diff --git a/src/soc/intel/xeon_sp/util.c b/src/soc/intel/xeon_sp/util.c index bf07ee7..69f3c15 100644 --- a/src/soc/intel/xeon_sp/util.c +++ b/src/soc/intel/xeon_sp/util.c @@ -123,20 +123,6 @@
#if ENV_RAMSTAGE /* Setting devtree variables is only allowed in ramstage. */
-void lock_pam0123(void) -{ - const uint32_t pam0123_lock = 0x33333331; - struct device *dev; - - if (get_lockdown_config() != CHIPSET_LOCKDOWN_COREBOOT) - return; - - dev = NULL; - /* Look for SAD_ALL devices on all sockets */ - while ((dev = dev_find_device(PCI_VID_INTEL, SAD_ALL_DEVID, dev))) - pci_write_config32(dev, SAD_ALL_PAM0123_CSR, pam0123_lock); -} - /* return true if command timed out else false */ static bool wait_for_bios_cmd_cpl(struct device *pcu1, uint32_t reg, uint32_t mask, uint32_t target)
-
Lean Sheng Tan (Code Review)