From d9c5b14f1efe82a902a7a4ed543015948364f296 Mon Sep 17 00:00:00 2001 From: MAULIK V VAGHELA Date: Mon, 14 Feb 2022 22:04:03 +0530 Subject: [PATCH] intelblocks/pcie: Correct mapping between LCAP port and coreboot index coreboot uses port index which is 0 based for all PCIe root ports. In case of PCIe remapping logic, coreboot reads LCAP register from PCIe configuration space which contains port number (mostly 1 based). This assumption might not be true for all the ports in coreboot. TBT's LCAP registers are returning port index which are based on 2. coreboot's PCIe remapping logic returns port index based on index 1. This patch adds variable to pcie_rp_config to pass lcap_port_base to the pcie remapping function, so coreboot can map any n-based LCAP encoding to 0-based indexing scheme. This patch updates correct lcap_port_base variable for all PCIe root ports for all SOCs, so that function returns correct 0-based index from LCAP port number. BUG=b:210933428 BRANCH=None TEST=Check if code compiles for all ADL boards Change-Id: I7f9c3c8e753b982e2ede1a41bf87d6355b82da0f Signed-off-by: MAULIK V VAGHELA Reviewed-on: https://review.coreboot.org/c/coreboot/+/61936 Reviewed-by: Subrata Banik Reviewed-by: Tim Wawrzynczak Reviewed-by: EricR Lai Reviewed-by: Nico Huber Tested-by: build bot (Jenkins) --- src/soc/intel/alderlake/pcie_rp.c | 24 ++++++++++++------- src/soc/intel/cannonlake/chip.c | 10 ++++---- .../block/include/intelblocks/pcie_rp.h | 6 +++++ src/soc/intel/common/block/pcie/pcie_rp.c | 17 ++++++++----- src/soc/intel/elkhartlake/chip.c | 2 +- src/soc/intel/jasperlake/chip.c | 2 +- src/soc/intel/skylake/chip.c | 10 ++++---- src/soc/intel/tigerlake/pcie_rp.c | 14 +++++------ 8 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/soc/intel/alderlake/pcie_rp.c b/src/soc/intel/alderlake/pcie_rp.c index f38105af71..1193047777 100644 --- a/src/soc/intel/alderlake/pcie_rp.c +++ b/src/soc/intel/alderlake/pcie_rp.c @@ -9,14 +9,14 @@ #define CPU_CPIE_VW_IDX_BASE 24 static const struct pcie_rp_group pch_lp_rp_groups[] = { - { .slot = PCH_DEV_SLOT_PCIE, .count = 8 }, - { .slot = PCH_DEV_SLOT_PCIE_1, .count = 4 }, + { .slot = PCH_DEV_SLOT_PCIE, .count = 8, .lcap_port_base = 1 }, + { .slot = PCH_DEV_SLOT_PCIE_1, .count = 4, .lcap_port_base = 1 }, { 0 } }; static const struct pcie_rp_group pch_m_rp_groups[] = { - { .slot = PCH_DEV_SLOT_PCIE, .count = 8 }, - { .slot = PCH_DEV_SLOT_PCIE_1, .count = 2 }, + { .slot = PCH_DEV_SLOT_PCIE, .count = 8, .lcap_port_base = 1 }, + { .slot = PCH_DEV_SLOT_PCIE_1, .count = 2, .lcap_port_base = 1 }, { 0 } }; @@ -35,14 +35,14 @@ const struct pcie_rp_group *get_pch_pcie_rp_table(void) * RP3: PEG62 : 0:6:2 : x4 CPU Slot */ static const struct pcie_rp_group cpu_rp_groups[] = { - { .slot = SA_DEV_SLOT_CPU_6, .start = 0, .count = 1 }, - { .slot = SA_DEV_SLOT_CPU_1, .start = 0, .count = 1 }, - { .slot = SA_DEV_SLOT_CPU_6, .start = 2, .count = 1 }, + { .slot = SA_DEV_SLOT_CPU_6, .start = 0, .count = 1, .lcap_port_base = 1 }, + { .slot = SA_DEV_SLOT_CPU_1, .start = 0, .count = 1, .lcap_port_base = 1 }, + { .slot = SA_DEV_SLOT_CPU_6, .start = 2, .count = 1, .lcap_port_base = 1 }, { 0 } }; static const struct pcie_rp_group cpu_m_rp_groups[] = { - { .slot = SA_DEV_SLOT_CPU_6, .start = 0, .count = 1 }, + { .slot = SA_DEV_SLOT_CPU_6, .start = 0, .count = 1, .lcap_port_base = 1 }, { 0 } }; @@ -61,8 +61,14 @@ const struct pcie_rp_group *get_cpu_pcie_rp_table(void) return cpu_rp_groups; } +/* + * TBT's LCAP registers are returning port index which starts from 2 (Usually for other PCIe + * root ports index starts from 1). Thus keeping lcap_port_base 2 for TBT, so that coreboot's + * PCIe remapping logic can return correct index (0-based) + */ + static const struct pcie_rp_group tbt_rp_groups[] = { - { .slot = SA_DEV_SLOT_TBT, .count = CONFIG_MAX_TBT_ROOT_PORTS}, + { .slot = SA_DEV_SLOT_TBT, .count = CONFIG_MAX_TBT_ROOT_PORTS, .lcap_port_base = 2 }, { 0 } }; diff --git a/src/soc/intel/cannonlake/chip.c b/src/soc/intel/cannonlake/chip.c index 27c49b174d..c223211ba4 100644 --- a/src/soc/intel/cannonlake/chip.c +++ b/src/soc/intel/cannonlake/chip.c @@ -18,15 +18,15 @@ #include "chip.h" static const struct pcie_rp_group pch_lp_rp_groups[] = { - { .slot = PCH_DEV_SLOT_PCIE, .count = 8 }, - { .slot = PCH_DEV_SLOT_PCIE_1, .count = 8 }, + { .slot = PCH_DEV_SLOT_PCIE, .count = 8, .lcap_port_base = 1 }, + { .slot = PCH_DEV_SLOT_PCIE_1, .count = 8, .lcap_port_base = 1 }, { 0 } }; static const struct pcie_rp_group pch_h_rp_groups[] = { - { .slot = PCH_DEV_SLOT_PCIE, .count = 8 }, - { .slot = PCH_DEV_SLOT_PCIE_1, .count = 8 }, - { .slot = PCH_DEV_SLOT_PCIE_2, .count = 8 }, + { .slot = PCH_DEV_SLOT_PCIE, .count = 8, .lcap_port_base = 1 }, + { .slot = PCH_DEV_SLOT_PCIE_1, .count = 8, .lcap_port_base = 1 }, + { .slot = PCH_DEV_SLOT_PCIE_2, .count = 8, .lcap_port_base = 1 }, { 0 } }; diff --git a/src/soc/intel/common/block/include/intelblocks/pcie_rp.h b/src/soc/intel/common/block/include/intelblocks/pcie_rp.h index b43987d13c..2f3b83ce4c 100644 --- a/src/soc/intel/common/block/include/intelblocks/pcie_rp.h +++ b/src/soc/intel/common/block/include/intelblocks/pcie_rp.h @@ -62,11 +62,17 @@ struct pcie_rp_config { * in case the root port numbers are not contiguous within the slot. * `count` is the number of functions within the group starting with the `start` * function number. + * `lcap_port_base` is the starting index of physical port as described in LCAP + * register in PCIe config space. coreboot always uses 0 based indexing while + * referring to the PCIe port but LCAP registers uses 1-based indexing in + * most of the cases. Remapping logic needs to correctly map LCAP port number + * (1-based or n-based) to coreboot indexing (0-based). */ struct pcie_rp_group { unsigned int slot; unsigned int start; unsigned int count; + unsigned int lcap_port_base; }; static inline unsigned int rp_start_fn(const struct pcie_rp_group *group) diff --git a/src/soc/intel/common/block/pcie/pcie_rp.c b/src/soc/intel/common/block/pcie/pcie_rp.c index 221ee03a94..145159f6eb 100644 --- a/src/soc/intel/common/block/pcie/pcie_rp.c +++ b/src/soc/intel/common/block/pcie/pcie_rp.c @@ -31,20 +31,25 @@ static int pcie_rp_original_idx( } const uint32_t lcap = pci_s_read_config32(dev, clist + PCI_EXP_LNKCAP); - /* Read 1-based absolute port number. This reflects the numbering - scheme that Intel uses in their documentation and what we use - as index (0-based, though) in our mapping. */ + + /* Read n-based absolute port number from LCAP register. + This reflects the numbering scheme that Intel uses in their + documentation and what we use as index (0-based, though) in + our mapping. */ const unsigned int port_num = (lcap & PCI_EXP_LNKCAP_PORT) >> 24; - /* `port_num` is 1-based, `offset` is 0-based. */ - if (port_num <= offset || port_num > offset + group->count) { + /* Subtract lcap_port_base from port_num to get 0-based index */ + const unsigned int port_idx = port_num - group->lcap_port_base; + + /* Check if port_idx (0-based) is out of bounds */ + if (port_idx < offset || port_idx >= offset + group->count) { printk(BIOS_WARNING, "%s: Unexpected root-port number '%u'" " at PCI: 00:%02x.%x, ignoring.\n", __func__, port_num, group->slot, PCI_FUNC(PCI_DEV2DEVFN(dev))); return -1; } - return port_num - 1; + return port_idx; } /* Scan actual PCI config space to reconstruct current mapping */ diff --git a/src/soc/intel/elkhartlake/chip.c b/src/soc/intel/elkhartlake/chip.c index 3bed78bece..c138a1fe45 100644 --- a/src/soc/intel/elkhartlake/chip.c +++ b/src/soc/intel/elkhartlake/chip.c @@ -17,7 +17,7 @@ #include static const struct pcie_rp_group pch_rp_groups[] = { - { .slot = PCH_DEV_SLOT_PCIE, .count = 7 }, + { .slot = PCH_DEV_SLOT_PCIE, .count = 7, .lcap_port_base = 1 }, { 0 } }; diff --git a/src/soc/intel/jasperlake/chip.c b/src/soc/intel/jasperlake/chip.c index a8e3c88128..7f325c4f59 100644 --- a/src/soc/intel/jasperlake/chip.c +++ b/src/soc/intel/jasperlake/chip.c @@ -17,7 +17,7 @@ #include static const struct pcie_rp_group pch_rp_groups[] = { - { .slot = PCH_DEV_SLOT_PCIE, .count = 8 }, + { .slot = PCH_DEV_SLOT_PCIE, .count = 8, .lcap_port_base = 1 }, { 0 } }; diff --git a/src/soc/intel/skylake/chip.c b/src/soc/intel/skylake/chip.c index 5c1cc6113f..7c77a8966c 100644 --- a/src/soc/intel/skylake/chip.c +++ b/src/soc/intel/skylake/chip.c @@ -34,18 +34,18 @@ #include "chip.h" static const struct pcie_rp_group pch_lp_rp_groups[] = { - { .slot = PCH_DEV_SLOT_PCIE, .count = 8 }, - { .slot = PCH_DEV_SLOT_PCIE_1, .count = 4 }, + { .slot = PCH_DEV_SLOT_PCIE, .count = 8, .lcap_port_base = 1 }, + { .slot = PCH_DEV_SLOT_PCIE_1, .count = 4, .lcap_port_base = 1 }, { 0 } }; static const struct pcie_rp_group pch_h_rp_groups[] = { - { .slot = PCH_DEV_SLOT_PCIE, .count = 8 }, - { .slot = PCH_DEV_SLOT_PCIE_1, .count = 8 }, + { .slot = PCH_DEV_SLOT_PCIE, .count = 8, .lcap_port_base = 1 }, + { .slot = PCH_DEV_SLOT_PCIE_1, .count = 8, .lcap_port_base = 1 }, /* Sunrise Point PCH-H actually only has 4 ports in the third group. But that would require a runtime check and probing 4 non-existent ports shouldn't hurt. */ - { .slot = PCH_DEV_SLOT_PCIE_2, .count = 8 }, + { .slot = PCH_DEV_SLOT_PCIE_2, .count = 8, .lcap_port_base = 1 }, { 0 } }; diff --git a/src/soc/intel/tigerlake/pcie_rp.c b/src/soc/intel/tigerlake/pcie_rp.c index f45a9c4e96..57a5cf29dc 100644 --- a/src/soc/intel/tigerlake/pcie_rp.c +++ b/src/soc/intel/tigerlake/pcie_rp.c @@ -7,21 +7,21 @@ #define CPU_CPIE_VW_IDX_BASE 24 static const struct pcie_rp_group pch_lp_rp_groups[] = { - { .slot = PCH_DEV_SLOT_PCIE, .count = 8 }, - { .slot = PCH_DEV_SLOT_PCIE_1, .count = 4 }, + { .slot = PCH_DEV_SLOT_PCIE, .count = 8, .lcap_port_base = 1 }, + { .slot = PCH_DEV_SLOT_PCIE_1, .count = 4, .lcap_port_base = 1 }, { 0 } }; static const struct pcie_rp_group pch_h_rp_groups[] = { - { .slot = PCH_DEV_SLOT_PCIE, .count = 8 }, - { .slot = PCH_DEV_SLOT_PCIE_1, .count = 8 }, - { .slot = PCH_DEV_SLOT_PCIE_2, .count = 8 }, + { .slot = PCH_DEV_SLOT_PCIE, .count = 8, .lcap_port_base = 1 }, + { .slot = PCH_DEV_SLOT_PCIE_1, .count = 8, .lcap_port_base = 1 }, + { .slot = PCH_DEV_SLOT_PCIE_2, .count = 8, .lcap_port_base = 1 }, { 0 } }; static const struct pcie_rp_group cpu_rp_groups[] = { - { .slot = SA_DEV_SLOT_PEG, .start = 0, .count = 3 }, - { .slot = SA_DEV_SLOT_CPU_PCIE, .start = 0, .count = 1 }, + { .slot = SA_DEV_SLOT_PEG, .start = 0, .count = 3, .lcap_port_base = 1 }, + { .slot = SA_DEV_SLOT_CPU_PCIE, .start = 0, .count = 1, .lcap_port_base = 1 }, { 0 } };