Re: Red Hat (Fedora) bug report 1467674 concerning your kernel functional performance enhancements causing PCI Express crashes,

[Sinan Kaya <okaya@codeaurora.org>]

[-- Attachment #1: Type: text/plain, Size: 2581 bytes --]

On 7/4/2017 6:25 PM, Sinan Kaya wrote:
> On 7/4/2017 1:59 PM, Wim ten Have wrote:
>> On Tue, 4 Jul 2017 11:57:37 -0400
>> Sinan Kaya <okaya@codeaurora.org> wrote:
>>
>>> Hi,
>>>
>>> On 7/4/2017 11:32 AM, Bjorn Helgaas wrote:
>>>> [+cc linux-pci]
>>>>
>>>> Thanks very much for the detailed problem report, Wim!  I'm taking the
>>>> liberty to forward to the linux-pci list in case others trip over the
>>>> same thing.
>>>>   
>>>
>>> So, the spec is lying :) and reality doesn't match theory.
> 
> The PCI Express bridge you have is a Broadcom HT 2100 bridge which seems to support
> PCI-Express V1.0 and 1.0a compliant only.
> 
> http://www.hard-net.de/info_wissen/chipsatz/broadcom/HT-2100.pdf
> 
> I can also see this in your lspci output. 
> 
> 00:08.0 PCI bridge: Broadcom HT2100 PCI-Express Bridge (rev a2) (prog-if 00 [Normal decode])
> 	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
> 	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> 	Latency: 0, Cache Line Size: 64 bytes
> 	Interrupt: pin A routed to IRQ 19
> 	NUMA node: 0
> 	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
> 	I/O behind bridge: 0000f000-00000fff [empty]
> 	Memory behind bridge: efe00000-efefffff [size=1M]
> 	Prefetchable memory behind bridge: 00000000fff00000-00000000000fffff [empty]
> 	Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ <SERR- <PERR-
> 	BridgeCtl: Parity+ SERR+ NoISA+ VGA- MAbort- >Reset- FastB2B-
> 		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
> 	Capabilities: [a0] HyperTransport: MSI Mapping Enable+ Fixed-
> 		Mapping Address Base: 00000000fee00000
> 	Capabilities: [b0] Express (v1) Root Port (Slot-), MSI 00
> 
> I'll post a patch to apply extended tags to systems with PCI express v2 and higher
> bridges only.
> 

Please give this patch a try. I can make the patch pretty and re-post if it works for you. 

You should be seeing messages like this during boot.

[    3.949621] pci 0003:01:00.0: clearing extended tags capability
[    3.959540] pci 0003:01:00.1: clearing extended tags capability
[    3.969454] pci 0003:01:00.2: clearing extended tags capability
[    3.979373] pci 0003:01:00.3: clearing extended tags capability
[    3.989290] pci 0003:01:00.4: clearing extended tags capability



-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[-- Attachment #2: 0001-pci-do-not-enable-extended-tags-on-pre-dated-v1.x-sy.patch --]
[-- Type: text/plain, Size: 2658 bytes --]

From a50edf37d58993983ec90dc5aab8ca6d2b8ff10b Mon Sep 17 00:00:00 2001
From: Sinan Kaya <okaya@codeaurora.org>
Date: Tue, 4 Jul 2017 20:39:08 -0400
Subject: [PATCH] pci: do not enable extended tags on pre-dated(v1.x) systems

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/pci/probe.c | 52 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index dfc9a27..c67af22 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1663,21 +1663,58 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	 */
 }

-static void pci_configure_extended_tags(struct pci_dev *dev)
+static bool pcie_bus_exttags_supported(struct pci_bus *bus)
+{
+	bool exttags_supported = true;
+	struct pci_dev *bridge;
+	int rc;
+	u16 flags;
+
+	bridge = bus->self;
+	while (bridge) {
+		if (pci_is_pcie(bridge)) {
+			rc = pcie_capability_read_word(bridge, PCI_EXP_FLAGS,
+						       &flags);
+			if (!rc && ((flags & PCI_EXP_FLAGS_VERS) < 2)) {
+				exttags_supported = false;
+				break;
+			}
+		}
+		if (!bridge->bus->parent)
+			break;
+		bridge = bridge->bus->parent->self;
+	}
+
+	return exttags_supported;
+}
+
+static int pcie_bus_configure_exttags(struct pci_dev *dev, void *data)
 {
 	u32 dev_cap;
 	int ret;
+	bool supported;

 	if (!pci_is_pcie(dev))
-		return;
+		return 0;

 	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
 	if (ret)
-		return;
+		return 0;

-	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
-		pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
-					 PCI_EXP_DEVCTL_EXT_TAG);
+	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG) {
+		supported = pcie_bus_exttags_supported(dev->bus);
+
+		if (supported) {
+			dev_info(&dev->dev, "setting extended tags capability\n");
+			pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
+						 PCI_EXP_DEVCTL_EXT_TAG);
+		} else {
+			dev_info(&dev->dev, "clearing extended tags capability\n");
+			pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
+						   PCI_EXP_DEVCTL_EXT_TAG);
+		}
+	}
+	return 0;
 }

 static void pci_configure_device(struct pci_dev *dev)
@@ -1686,7 +1723,6 @@ static void pci_configure_device(struct pci_dev *dev)
 	int ret;

 	pci_configure_mps(dev);
-	pci_configure_extended_tags(dev);

 	memset(&hpp, 0, sizeof(hpp));
 	ret = pci_get_hp_params(dev, &hpp);
@@ -2231,6 +2267,8 @@ void pcie_bus_configure_settings(struct pci_bus *bus)

 	pcie_bus_configure_set(bus->self, &smpss);
 	pci_walk_bus(bus, pcie_bus_configure_set, &smpss);
+
+	pci_walk_bus(bus, pcie_bus_configure_exttags, NULL);
 }
 EXPORT_SYMBOL_GPL(pcie_bus_configure_settings);

--
1.9.1