FAILED: patch "[PATCH] PCI: aardvark: Fix reporting CRS value" failed to apply to 5.14-stable tree

The patch below does not apply to the 5.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 43f5c77bcbd27cce70bf33c2b86d6726ce95dd66 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <[email protected]>
Date: Thu, 22 Jul 2021 16:40:41 +0200
Subject: [PATCH] PCI: aardvark: Fix reporting CRS value
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Set CRSVIS flag in emulated root PCI bridge to indicate support for Completion Retry Status.

Add check for CRSSVE flag from root PCI brige when issuing Configuration Read Request via PIO to correctly returns fabricated CRS value as it is required by PCIe spec.

Link https://lore.kernel.org/r/[email protected] open here

Fixes 8a3ebd8de328 ("PCI: aardvark: Implement emulated root PCI bridge config space")

Signed-off-by Pali Rohár <[email protected]>

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc [email protected] # e0d9d30b7354 ("PCI: pci-bridge-emul: Fix big-endian support")

drivers/pci/controller/pci-aardvark.c +63 -4

index 5b9e4e79c3ae..0c32283b3276 100644
@@ -178,6 +178,8 @@
 
 #define MSI_IRQ_NUM			32
 
+#define CFG_RD_CRS_VAL			0xffff0001
+
 struct advk_pcie {
 	struct platform_device *pdev;
 	void __iomem *base;

@@ -473,7 +475,7 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
 	advk_writel(pcie, reg, PCIE_CORE_CMD_STATUS_REG);
 }
 
-static int advk_pcie_check_pio_status(struct advk_pcie *pcie, u32 *val)
+static int advk_pcie_check_pio_status(struct advk_pcie *pcie, bool allow_crs, u32 *val)
 {
 	struct device *dev = &pcie->pdev->dev;
 	u32 reg;

@@ -515,9 +517,30 @@ static int advk_pcie_check_pio_status(struct advk_pcie *pcie, u32 *val)
 		strcomp_status = "UR";
 		break;
 	case PIO_COMPLETION_STATUS_CRS:
+		if (allow_crs && val) {
+			/* PCIe r4.0, sec 2.3.2, says:
+			 * If CRS Software Visibility is enabled:
+			 * For a Configuration Read Request that includes both
+			 * bytes of the Vendor ID field of a device Function's
+			 * Configuration Space Header, the Root Complex must
+			 * complete the Request to the host by returning a
+			 * read-data value of 0001h for the Vendor ID field and
+			 * all '1's for any additional bytes included in the
+			 * request.
+			 *
+			 * So CRS in this case is not an error status.
+			 */
+			*val = CFG_RD_CRS_VAL;
+			strcomp_status = NULL;
+			break;
+		}
 		/* PCIe r4.0, sec 2.3.2, says:
 		 * If CRS Software Visibility is not enabled, the Root Complex
 		 * must re-issue the Configuration Request as a new Request.
+		 * If CRS Software Visibility is enabled: For a Configuration
+		 * Write Request or for any other Configuration Read Request,
+		 * the Root Complex must re-issue the Configuration Request as
+		 * a new Request.
 		 * A Root Complex implementation may choose to limit the number
 		 * of Configuration Request/CRS Completion Status loops before
 		 * determining that something is wrong with the target of the

@@ -586,6 +609,7 @@ advk_pci_bridge_emul_pcie_conf_read(struct pci_bridge_emul *bridge,
 	case PCI_EXP_RTCTL: {
 		u32 val = advk_readl(pcie, PCIE_ISR0_MASK_REG);
 		*value = (val & PCIE_MSG_PM_PME_MASK) ? 0 : PCI_EXP_RTCTL_PMEIE;
+		*value |= PCI_EXP_RTCAP_CRSVIS << 16;
 		return PCI_BRIDGE_EMUL_HANDLED;
 	}
 

@@ -667,6 +691,7 @@ static struct pci_bridge_emul_ops advk_pci_bridge_emul_ops = {
 static int advk_sw_pci_bridge_init(struct advk_pcie *pcie)
 {
 	struct pci_bridge_emul *bridge = &pcie->bridge;
+	int ret;
 
 	bridge->conf.vendor =
 		cpu_to_le16(advk_readl(pcie, PCIE_CORE_DEV_ID_REG) & 0xffff);

@@ -690,7 +715,15 @@ static int advk_sw_pci_bridge_init(struct advk_pcie *pcie)
 	bridge->data = pcie;
 	bridge->ops = &advk_pci_bridge_emul_ops;
 
-	return pci_bridge_emul_init(bridge, 0);
+	/* PCIe config space can be initialized after pci_bridge_emul_init() */
+	ret = pci_bridge_emul_init(bridge, 0);
+	if (ret < 0)
+		return ret;
+
+	/* Indicates supports for Completion Retry Status */
+	bridge->pcie_conf.rootcap = cpu_to_le16(PCI_EXP_RTCAP_CRSVIS);
+
+	return 0;
 }
 
 static bool advk_pcie_valid_device(struct advk_pcie *pcie, struct pci_bus *bus,

@@ -742,6 +775,7 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,
 			     int where, int size, u32 *val)
 {
 	struct advk_pcie *pcie = bus->sysdata;
+	bool allow_crs;
 	u32 reg;
 	int ret;
 

@@ -754,7 +788,24 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,
 		return pci_bridge_emul_conf_read(&pcie->bridge, where,
 						 size, val);
 
+	/*
+	 * Completion Retry Status is possible to return only when reading all
+	 * 4 bytes from PCI_VENDOR_ID and PCI_DEVICE_ID registers at once and
+	 * CRSSVE flag on Root Bridge is enabled.
+	 */
+	allow_crs = (where == PCI_VENDOR_ID) && (size == 4) &&
+		    (le16_to_cpu(pcie->bridge.pcie_conf.rootctl) &
+		     PCI_EXP_RTCTL_CRSSVE);
+
 	if (advk_pcie_pio_is_running(pcie)) {
+		/*
+		 * If it is possible return Completion Retry Status so caller
+		 * tries to issue the request again instead of failing.
+		 */
+		if (allow_crs) {
+			*val = CFG_RD_CRS_VAL;
+			return PCIBIOS_SUCCESSFUL;
+		}
 		*val = 0xffffffff;
 		return PCIBIOS_SET_FAILED;
 	}

@@ -782,12 +833,20 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,
 
 	ret = advk_pcie_wait_pio(pcie);
 	if (ret < 0) {
+		/*
+		 * If it is possible return Completion Retry Status so caller
+		 * tries to issue the request again instead of failing.
+		 */
+		if (allow_crs) {
+			*val = CFG_RD_CRS_VAL;
+			return PCIBIOS_SUCCESSFUL;
+		}
 		*val = 0xffffffff;
 		return PCIBIOS_SET_FAILED;
 	}
 
 	/* Check PIO status and get the read result */
-	ret = advk_pcie_check_pio_status(pcie, val);
+	ret = advk_pcie_check_pio_status(pcie, allow_crs, val);
 	if (ret < 0) {
 		*val = 0xffffffff;
 		return PCIBIOS_SET_FAILED;

@@ -856,7 +915,7 @@ static int advk_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 	if (ret < 0)
 		return PCIBIOS_SET_FAILED;
 
-	ret = advk_pcie_check_pio_status(pcie, NULL);
+	ret = advk_pcie_check_pio_status(pcie, false, NULL);
 	if (ret < 0)
 		return PCIBIOS_SET_FAILED;
 

Quoted 4 lines · On Wednesday 15 September 2021 16:30:03 [email protected] wrote:

The patch below does not apply to the 5.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

Hello that patch depends on commit which fixes name of rootcap member:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e902bb7c24a7099d0eb0eb4cba06f2d91e9299f3

Quoted 6 lines · Pali Rohár

Quoted 4 lines · On Wednesday 15 September 2021 16:30:03 [email protected] wrote:

The patch below does not apply to the 5.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

Quoted 2 lines

Hello that patch depends on commit which fixes name of rootcap member:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e902bb7c24a7099d0eb0eb4cba06f2d91e9299f3

Thanks, that worked for 5.10 and 5.14, but not for 5.4.y. If this needs to go there too, can you send a working set of backported patches?

thanks,

greg k-h

Quoted 7 lines · On Wednesday 22 September 2021 16:41:30 Greg KH wrote:

Quoted 6 lines · Pali Rohár

Quoted 4 lines · On Wednesday 15 September 2021 16:30:03 [email protected] wrote:

The patch below does not apply to the 5.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

Quoted 5 lines

Quoted 2 lines

Hello that patch depends on commit which fixes name of rootcap member:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e902bb7c24a7099d0eb0eb4cba06f2d91e9299f3

Thanks, that worked for 5.10 and 5.14, but not for 5.4.y. If this needs to go there too, can you send a working set of backported patches?

Hello Greg!

Applying patches in this order passes:

git cherry-pick e0d9d30b73548fbfe5c024ed630169bdc9a08aee git cherry-pick b1bd5714472cc72e14409f5659b154c765a76c65 git cherry-pick e902bb7c24a7099d0eb0eb4cba06f2d91e9299f3 git cherry-pick 43f5c77bcbd27cce70bf33c2b86d6726ce95dd66

First patch fixes big endian support in driver code and second patch fixes filling return value of function call on error (return value is passed via pointer argument). I do not know why they do not have appropriate Fixes tag, but for me it looks like that both patches are suitable for stable backporting.

What do you think, are first two patches suitable for backport? Or do you need some other solution?