Hello Arthur Heymans, Michael Niewöhner,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/35985
to review the following change.
Change subject: [WIP] intel/skylake: Implement PCIe RP devicetree update based on DID ......................................................................
[WIP] intel/skylake: Implement PCIe RP devicetree update based on DID
Change-Id: Ib17d2b6fd34608603db3936d638bdf5acb46d717 Signed-off-by: Nico Huber nico.h@gmx.de --- M src/soc/intel/skylake/Makefile.inc M src/soc/intel/skylake/chip_fsp20.c A src/soc/intel/skylake/include/soc/pcie_rp.h A src/soc/intel/skylake/pcie_rp.c 4 files changed, 159 insertions(+), 127 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/35985/1
diff --git a/src/soc/intel/skylake/Makefile.inc b/src/soc/intel/skylake/Makefile.inc index ef741f8..691bdbc 100644 --- a/src/soc/intel/skylake/Makefile.inc +++ b/src/soc/intel/skylake/Makefile.inc @@ -57,6 +57,7 @@ ramstage-y += me.c ramstage-y += memmap.c ramstage-y += p2sb.c +ramstage-y += pcie_rp.c ramstage-y += pmc.c ramstage-y += pmutil.c ramstage-$(CONFIG_PLATFORM_USES_FSP2_0) += reset.c diff --git a/src/soc/intel/skylake/chip_fsp20.c b/src/soc/intel/skylake/chip_fsp20.c index 55fedd3..83e3be5 100644 --- a/src/soc/intel/skylake/chip_fsp20.c +++ b/src/soc/intel/skylake/chip_fsp20.c @@ -15,13 +15,10 @@
#include <bootmode.h> #include <bootstate.h> -#include <device/pci.h> #include <fsp/api.h> #include <arch/acpi.h> -#include <device/pci_ops.h> #include <console/console.h> #include <device/device.h> -#include <device/pci_ids.h> #include <fsp/util.h> #include <intelblocks/cfg.h> #include <intelblocks/itss.h> @@ -37,135 +34,13 @@ #include <soc/irq.h> #include <soc/itss.h> #include <soc/pci_devs.h> +#include <soc/pcie_rp.h> #include <soc/ramstage.h> #include <soc/systemagent.h> #include <string.h>
#include "chip.h"
-struct pcie_entry { - unsigned int devfn; - unsigned int func_count; -}; - -/* - * According to table 2-2 in doc#546717: - * PCI bus[function] ID - * D28:[F0 - F7] 0xA110 - 0xA117 - * D29:[F0 - F7] 0xA118 - 0xA11F - * D27:[F0 - F3] 0xA167 - 0xA16A - */ -static const struct pcie_entry pcie_table_skl_pch_h[] = { - {PCH_DEVFN_PCIE1, 8}, - {PCH_DEVFN_PCIE9, 8}, - {PCH_DEVFN_PCIE17, 4}, -}; - -/* - * According to table 2-2 in doc#564464: - * PCI bus[function] ID - * D28:[F0 - F7] 0xA290 - 0xA297 - * D29:[F0 - F7] 0xA298 - 0xA29F - * D27:[F0 - F7] 0xA2E7 - 0xA2EE - */ -static const struct pcie_entry pcie_table_kbl_pch_h[] = { - {PCH_DEVFN_PCIE1, 8}, - {PCH_DEVFN_PCIE9, 8}, - {PCH_DEVFN_PCIE17, 8}, -}; - -/* - * According to table 2-2 in doc#567995/545659: - * PCI bus[function] ID - * D28:[F0 - F7] 0x9D10 - 0x9D17 - * D29:[F0 - F3] 0x9D18 - 0x9D1B - */ -static const struct pcie_entry pcie_table_skl_pch_lp[] = { - {PCH_DEVFN_PCIE1, 8}, - {PCH_DEVFN_PCIE9, 4}, -}; - -/* - * If the PCIe root port at function 0 is disabled, - * the PCIe root ports might be coalesced after FSP silicon init. - * The below function will swap the devfn of the first enabled device - * in devicetree and function 0 resides a pci device - * so that it won't confuse coreboot. - */ -static void pcie_update_device_tree(const struct pcie_entry *pcie_rp_group, - size_t pci_groups) -{ - struct device *func0; - unsigned int devfn, devfn0; - int i, group; - unsigned int inc = PCI_DEVFN(0, 1); - - for (group = 0; group < pci_groups; group++) { - devfn0 = pcie_rp_group[group].devfn; - func0 = pcidev_path_on_root(devfn0); - if (func0 == NULL) - continue; - - /* No more functions if function 0 is disabled. */ - if (pci_read_config32(func0, PCI_VENDOR_ID) == 0xffffffff) - continue; - - devfn = devfn0 + inc; - - /* - * Increase function by 1. - * Then find first enabled device to replace func0 - * as that port was move to func0. - */ - for (i = 1; i < pcie_rp_group[group].func_count; - i++, devfn += inc) { - struct device *dev = pcidev_path_on_root(devfn); - if (dev == NULL || !dev->enabled) - continue; - - /* - * Found the first enabled device in - * a given dev number. - */ - printk(BIOS_INFO, "PCI func %d was swapped" - " to func 0.\n", i); - func0->path.pci.devfn = dev->path.pci.devfn; - dev->path.pci.devfn = devfn0; - break; - } - } -} - -static void pcie_override_devicetree_after_silicon_init(void) -{ - uint16_t id, id_mask; - - id = pci_read_config16(PCH_DEV_PCIE1, PCI_DEVICE_ID); - /* - * We may read an ID other than func 0 after FSP-S. - * Strip out 4 least significant bits. - */ - id_mask = id & ~0xf; - printk(BIOS_INFO, "Override DT after FSP-S, PCH is "); - if (id_mask == (PCI_DEVICE_ID_INTEL_SPT_LP_PCIE_RP1 & ~0xf)) { - printk(BIOS_INFO, "KBL/SKL PCH-LP SKU\n"); - pcie_update_device_tree(&pcie_table_skl_pch_lp[0], - ARRAY_SIZE(pcie_table_skl_pch_lp)); - } else if (id_mask == (PCI_DEVICE_ID_INTEL_KBP_H_PCIE_RP1 & ~0xf)) { - printk(BIOS_INFO, "KBL PCH-H SKU\n"); - pcie_update_device_tree(&pcie_table_kbl_pch_h[0], - ARRAY_SIZE(pcie_table_kbl_pch_h)); - } else if (id_mask == (PCI_DEVICE_ID_INTEL_SPT_H_PCIE_RP1 & ~0xf)) { - printk(BIOS_INFO, "SKL PCH-H SKU\n"); - pcie_update_device_tree(&pcie_table_skl_pch_h[0], - ARRAY_SIZE(pcie_table_skl_pch_h)); - } else { - printk(BIOS_ERR, "[BUG] PCIE Root Port id 0x%x" - " is not found\n", id); - return; - } -} - void soc_init_pre_device(void *chip_info) { /* Snapshot the current GPIO IRQ polarities. FSP is setting a @@ -179,7 +54,7 @@ itss_restore_irq_polarities(GPIO_IRQ_START, GPIO_IRQ_END);
/* swap enabled PCI ports in device tree if needed */ - pcie_override_devicetree_after_silicon_init(); + pcie_rp_update_devicetree(); }
void soc_fsp_load(void) diff --git a/src/soc/intel/skylake/include/soc/pcie_rp.h b/src/soc/intel/skylake/include/soc/pcie_rp.h new file mode 100644 index 0000000..2bc7095 --- /dev/null +++ b/src/soc/intel/skylake/include/soc/pcie_rp.h @@ -0,0 +1,19 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef SOC_INTEL_SKYLAKE_PCIE_RP_H +#define SOC_INTEL_SKYLAKE_PCIE_RP_H + +void pcie_rp_update_devicetree(void); + +#endif /* SOC_INTEL_SKYLAKE_PCIE_RP_H */ diff --git a/src/soc/intel/skylake/pcie_rp.c b/src/soc/intel/skylake/pcie_rp.c new file mode 100644 index 0000000..5882dea --- /dev/null +++ b/src/soc/intel/skylake/pcie_rp.c @@ -0,0 +1,137 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Nico Huber nico.h@gmx.de + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <stdint.h> + +#include <console/console.h> +#include <device/device.h> +#include <device/pci_def.h> +#include <device/pci_ids.h> +#include <device/pci_ops.h> +#include <device/pci_type.h> +#include <soc/pcie_rp.h> + +/* negative if disabled, new function number otherwise */ +static int pcie_rp_fn_mapping[24]; + +/* The original DEVFN in the `devicetree.cb` is supposed to map directly to RPx. */ +static int pcie_rp_idx_from_devfn(const struct device *const dev) +{ + int rp_idx = PCI_FUNC(dev->path.pci.devfn); + switch (PCI_SLOT(dev->path.pci.devfn)) { + case 0x1c: + break; + case 0x1d: + rp_idx += 8; + break; + case 0x1b: + rp_idx += 16; + break; + default: + printk(BIOS_WARNING, + "Unexpected DEVFN for PCIe Root Port: %s.\n", + dev_path(dev)); + rp_idx = -1; + } + + return rp_idx; +} + +/* Check which RP (identified by its DID) has the given DEVFN assigned now. */ +static int pcie_rp_idx_from_did(const pci_devfn_t dev) +{ + unsigned int rp_idx; + + const uint16_t did = pci_s_read_config16(dev, PCI_DEVICE_ID); + + switch (did & ~0xf) { + case PCI_DEVICE_ID_INTEL_SPT_LP_PCIE_RP1: /* Fall through. */ + case PCI_DEVICE_ID_INTEL_SPT_H_PCIE_RP1: /* Fall through. */ + case PCI_DEVICE_ID_INTEL_KBP_H_PCIE_RP1: + rp_idx = did & 0xf; + break; + /* DIDs in the third groups on PCH-H are not aligned: */ + case PCI_DEVICE_ID_INTEL_SPT_H_PCIE_RP17: + rp_idx = 16 + did - PCI_DEVICE_ID_INTEL_SPT_H_PCIE_RP17; + break; + case PCI_DEVICE_ID_INTEL_KBP_H_PCIE_RP17: + rp_idx = 16 + did - PCI_DEVICE_ID_INTEL_KBP_H_PCIE_RP17; + break; + default: + printk(BIOS_WARNING, + "Unexpected DID for PCIe Root Port %02x.%x: %04x.\n", + PCI_SLOT(PCI_DEV2DEVFN(dev)), PCI_FUNC(PCI_DEV2DEVFN(dev)), did); + /* Fall through. */ + case 0xffff: + rp_idx = -1; + } + + return rp_idx; +} + +static void pcie_rp_register_group(const unsigned int devnum) +{ + unsigned int fn; + + /* Reconstruct current mapping */ + for (fn = 0; fn < 7; ++fn) { + const int rp_idx = pcie_rp_idx_from_did(PCI_DEV(0, devnum, fn)); + if (rp_idx < 0) { + if (fn == 0) + break; + else + continue; + } + pcie_rp_fn_mapping[rp_idx] = fn; + } +} + +static void pcie_rp_update_dev(struct device *const dev) +{ + const int rp_idx = pcie_rp_idx_from_devfn(dev); + if (rp_idx < 0) + return; + + const int fn = pcie_rp_fn_mapping[rp_idx]; + if (fn < 0) + dev->enabled = 0; + else + dev->path.pci.devfn = PCI_DEVFN(PCI_SLOT(dev->path.pci.devfn), fn); +} + +void pcie_rp_update_devicetree(void) +{ + const struct bus *const root = pci_root_bus(); + if (!root) + return; + + pcie_rp_register_group(0x1c); + pcie_rp_register_group(0x1d); + if (CONFIG(SKYLAKE_SOC_PCH_H)) + pcie_rp_register_group(0x1b); + + struct device *dev; + for (dev = root->children; dev; dev = dev->sibling) { + if (dev->path.type != DEVICE_PATH_PCI) + continue; + + if (PCI_SLOT(dev->path.pci.devfn) != 0x1c && + PCI_SLOT(dev->path.pci.devfn) != 0x1d && + (!CONFIG(SKYLAKE_SOC_PCH_H) || PCI_SLOT(dev->path.pci.devfn) != 0x1b)) + continue; + + pcie_rp_update_dev(dev); + } +}