Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     linux-pci@vger.kernel.org
Cc:     netdev@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
        Rob Herring <robh@kernel.org>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Michael Walle <michael@walle.cc>, linux-kernel@vger.kernel.org
Subject: [PATCH pci] PCI: don't skip probing entire device if first fn OF node has status = "disabled"
Date:   Sun, 21 May 2023 14:51:41 +0300
Message-Id: <20230521115141.2384444-1-vladimir.oltean@nxp.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?g5/Z9US/AnMMlwlmpvY3SXeYUBhFcyoIfH9k51Y7yI9ZvtpfgZlmMFFU83R1?=
 =?us-ascii?Q?qQa9EdgeWMNpiVkoHJj9B1EIKKdBzzC2uMa9N2HZ2rU+PUwi+TK8X+4NJYh2?=
 =?us-ascii?Q?Gt5wKRdg3xw+EkABj1Q+uusFT9x+lqBuI24K4jH/tDAYKYfxuaiR2zWE+y13?=
 =?us-ascii?Q?JXvWCOVGEYOw0gwMBCQWA++eTKay+13dEGasshdB1dJYJmGDKuklfUDycjHW?=
 =?us-ascii?Q?xNNjrRQZgyNU6/vKwghnIn7n2xps4TJcgIZsXfPxkhQhXb1QKN6QvnnvOv0p?=
 =?us-ascii?Q?J9/cxXdOZnIzyQfzSv+U1bTQdMxxGSs9x44hILe1Scmy4FociP486dKr/IaT?=
 =?us-ascii?Q?qO3/9DODfJ7VpwwHsbLbZDPDXleNSwhVbllm6iFaWISO4XN0riwVB9yKdFt4?=
 =?us-ascii?Q?yUHv633CpSQAgMoJ2C29gSD0b26ZWuAgamxHng2yK0fPeCOsI2z46cPqLGFN?=
 =?us-ascii?Q?4nIcdp0s9PvgTu3DSsiOoYftdFmtIux6gCELGZbAo1AEWZ4Fbiz+MzTeOulI?=
 =?us-ascii?Q?fP5RyaeudLfa2KatbwyE5APAjE/ySp98Rkv73ixnHU8CFoEQIdLP5EFlJbYT?=
 =?us-ascii?Q?qSPx4Mf2HFH42ZgHbizGYz5yEJcjyTLKHWvkopyrmnMl+bOnUGiLf/AqO89b?=
 =?us-ascii?Q?yrZdSz+/2+mUxinUL1mhXnOPRseK/2Sozzjp+alY5P1+ykiFfJz7/swSyJiy?=
 =?us-ascii?Q?mKdROugY2v1noG+mtbtEAA9ipReaDJ0e6WjDTlkHJzRaFo7JAbhBH8tGazpB?=
 =?us-ascii?Q?LbyS/DUo4wOxBp1FgEmTDqI9fI9hazS2Bqg2X3PX2qGgROkuEc1RPjXbcIC4?=
 =?us-ascii?Q?emIIDV6LtFGSGSogpoI0rKHaAqg7k6qfA6A2SlVdNe0hTs8qc6uCF4gSsEza?=
 =?us-ascii?Q?qAuIyszCAsGSJWaFrnlb+xXvbALIZFWI6p36SBwYFBd0OtY+I/VmSXy2xENp?=
 =?us-ascii?Q?u0Ho/xZDHSDKA4gnIz55lQe4yh8FcfUhiAAprHYOUcSHoRtsYhhSuPRndSgM?=
 =?us-ascii?Q?I1wgN861H1bj+wGHCFG7EY2egsa1McD3Y5DPoqmkPuJ+S/+VV4y861HvCTQs?=
 =?us-ascii?Q?LBsFDa35R2RvvmxqMSEh7K0c6Epe+IBrGa564DvglbpYdbcVJLblUNl+8lGm?=
 =?us-ascii?Q?LaaFO9a3n7tgoo/+OzqU/V3j2FzXAZu4D/xtMCnL8Wvsqjm+JwkIB7Eg1pd1?=
 =?us-ascii?Q?hiXQYTJG8FMriQjsfnnW73v7Xkjg3rhOGDR3Phb12+byQkgWSUYbjc5YI3hQ?=
 =?us-ascii?Q?08WZDqI/iesZv0/1sTYhkOvHkMN/CCtHItsWYit1JZylnwFf+27RQaM0jnSw?=
 =?us-ascii?Q?jpEyxTzSPrktR+0lLQWnMsy+tTHXvcOs6KaCXtj9Qx9lFgF3ikLNRU/RouCY?=
 =?us-ascii?Q?+URnfylGZC9S6jk0tZFGMTitoNVi1/OKGjmE5Tf29psduo6DN46Qfq6Hu66v?=
 =?us-ascii?Q?VWjJZDrOo5Sh0tFj/NjMgBalfEOwD/tJ6XbjBwJaqAWAxf+8MV4S4cYjiSK2?=
 =?us-ascii?Q?tQksSSWr5iU6xsAs47kyKZlNOt0f2p9d8I8kGT6ElMWPzshAkf2W2VgPOML7?=
 =?us-ascii?Q?XrymJ5iJ4H3O5LMr7QIltURkSN7+Hu5P85NCSScMb3PttkAQQzO6IyYQywxT?=
 =?us-ascii?Q?hg=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3672f4e2-751d-4317-7f58-08db59f1c704
X-MS-Exchange-CrossTenant-AuthSource: AM0PR04MB6452.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 May 2023 11:51:56.7206
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 5mYM8q80JA2MXgUjV9HliKmYmVR1zGWIEHShCT/qXMuEtC+l2RMLdgRGJLL7QF5p2344rN2lVf7jRL/ItF73xA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS4PR04MB9508
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,SPF_HELO_PASS,
        T_SCC_BODY_TEXT_LINE,T_SPF_PERMERROR autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

pci_scan_child_bus_extend() calls pci_scan_slot() with devfn
(bus:device:function) being a multiple of 8, i.e. for each unique
device.

pci_scan_slot() has logic to say that if the function 0 of a device is
absent, the entire device is absent and we can skip the other functions
entirely. Traditionally, this has meant that pci_bus_read_dev_vendor_id()
returns an error code for that function.

However, since the blamed commit, there is an extra confounding
condition: function 0 of the device exists and has a valid vendor id,
but it is disabled in the device tree. In that case, pci_scan_slot()
would incorrectly skip the entire device instead of just that function.

Such is the case with the NXP LS1028A SoC, which has an ECAM
for embedded Ethernet (see pcie@1f0000000 in
arm64/boot/dts/freescale/fsl-ls1028a.dtsi). Each Ethernet port
represents a function within the ENETC ECAM, with function 0 going
to ENETC Ethernet port 0, connected to SERDES port 0 (SGMII or USXGMII).

When using a SERDES protocol such as 0x9999, all 4 SERDES lanes go to
the Ethernet switch (function 5 on this ECAM) and none go to ENETC
port 0. So, ENETC port 0 needs to have status = "disabled", and embedded
Ethernet takes place just through the other functions (fn 2 is the DSA
master, fn 3 is the MDIO controller, fn 5 is the DSA switch etc).
Contrast this with other SERDES protocols like 0x85bb, where the switch
takes up a single SERDES lane and uses the QSGMII protocol - so ENETC
port 0 also gets access to a SERDES lane.

Therefore, here, function 0 being unused has nothing to do with the
entire PCI device being unused.

Add a "bool present_but_skipped" which is propagated from the caller
of pci_set_of_node() all the way to pci_scan_slot(), so that it can
distinguish an error reading the ECAM from a disabled device in the
device tree.

Fixes: 6fffbc7ae137 ("PCI: Honor firmware's device disabled status")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/pci/pci.h   |  1 +
 drivers/pci/probe.c | 58 +++++++++++++++++++++++++++++++--------------
 2 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 2475098f6518..dc11e0945744 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -240,6 +240,7 @@ bool pci_bus_generic_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
 					int crs_timeout);
 int pci_idt_bus_quirk(struct pci_bus *bus, int devfn, u32 *pl, int crs_timeout);
 
+int __pci_setup_device(struct pci_dev *dev, bool *present_but_skipped);
 int pci_setup_device(struct pci_dev *dev);
 int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 		    struct resource *res, unsigned int reg);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 0b2826c4a832..17a51fa55020 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1811,17 +1811,7 @@ static void early_dump_pci_device(struct pci_dev *pdev)
 		       value, 256, false);
 }
 
-/**
- * pci_setup_device - Fill in class and map information of a device
- * @dev: the device structure to fill
- *
- * Initialize the device structure with information about the device's
- * vendor,class,memory and IO-space addresses, IRQ lines etc.
- * Called at initialisation of the PCI subsystem and by CardBus services.
- * Returns 0 on success and negative if unknown type of device (not normal,
- * bridge or CardBus).
- */
-int pci_setup_device(struct pci_dev *dev)
+int __pci_setup_device(struct pci_dev *dev, bool *present_but_skipped)
 {
 	u32 class;
 	u16 cmd;
@@ -1841,8 +1831,10 @@ int pci_setup_device(struct pci_dev *dev)
 	set_pcie_port_type(dev);
 
 	err = pci_set_of_node(dev);
-	if (err)
+	if (err) {
+		*present_but_skipped = true;
 		return err;
+	}
 	pci_set_acpi_fwnode(dev);
 
 	pci_dev_assign_slot(dev);
@@ -1995,6 +1987,23 @@ int pci_setup_device(struct pci_dev *dev)
 	return 0;
 }
 
+/**
+ * pci_setup_device - Fill in class and map information of a device
+ * @dev: the device structure to fill
+ *
+ * Initialize the device structure with information about the device's
+ * vendor,class,memory and IO-space addresses, IRQ lines etc.
+ * Called at initialisation of the PCI subsystem and by CardBus services.
+ * Returns 0 on success and negative if unknown type of device (not normal,
+ * bridge or CardBus).
+ */
+int pci_setup_device(struct pci_dev *dev)
+{
+	bool present_but_skipped = false;
+
+	return __pci_setup_device(dev, &present_but_skipped);
+}
+
 static void pci_configure_mps(struct pci_dev *dev)
 {
 	struct pci_dev *bridge = pci_upstream_bridge(dev);
@@ -2414,7 +2423,8 @@ EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);
  * Read the config data for a PCI device, sanity-check it,
  * and fill in the dev structure.
  */
-static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
+static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn,
+				       bool *present_but_skipped)
 {
 	struct pci_dev *dev;
 	u32 l;
@@ -2430,7 +2440,7 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 	dev->vendor = l & 0xffff;
 	dev->device = (l >> 16) & 0xffff;
 
-	if (pci_setup_device(dev)) {
+	if (__pci_setup_device(dev, present_but_skipped)) {
 		pci_bus_put(dev->bus);
 		kfree(dev);
 		return NULL;
@@ -2575,17 +2585,20 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	WARN_ON(ret < 0);
 }
 
-struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn)
+static struct pci_dev *__pci_scan_single_device(struct pci_bus *bus, int devfn,
+						bool *present_but_skipped)
 {
 	struct pci_dev *dev;
 
+	*present_but_skipped = false;
+
 	dev = pci_get_slot(bus, devfn);
 	if (dev) {
 		pci_dev_put(dev);
 		return dev;
 	}
 
-	dev = pci_scan_device(bus, devfn);
+	dev = pci_scan_device(bus, devfn, present_but_skipped);
 	if (!dev)
 		return NULL;
 
@@ -2593,6 +2606,13 @@ struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn)
 
 	return dev;
 }
+
+struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn)
+{
+	bool present_but_skipped;
+
+	return __pci_scan_single_device(bus, devfn, &present_but_skipped);
+}
 EXPORT_SYMBOL(pci_scan_single_device);
 
 static int next_ari_fn(struct pci_bus *bus, struct pci_dev *dev, int fn)
@@ -2665,6 +2685,7 @@ static int only_one_child(struct pci_bus *bus)
  */
 int pci_scan_slot(struct pci_bus *bus, int devfn)
 {
+	bool present_but_skipped;
 	struct pci_dev *dev;
 	int fn = 0, nr = 0;
 
@@ -2672,13 +2693,14 @@ int pci_scan_slot(struct pci_bus *bus, int devfn)
 		return 0; /* Already scanned the entire slot */
 
 	do {
-		dev = pci_scan_single_device(bus, devfn + fn);
+		dev = __pci_scan_single_device(bus, devfn + fn,
+					       &present_but_skipped);
 		if (dev) {
 			if (!pci_dev_is_added(dev))
 				nr++;
 			if (fn > 0)
 				dev->multifunction = 1;
-		} else if (fn == 0) {
+		} else if (fn == 0 && !present_but_skipped) {
 			/*
 			 * Function 0 is required unless we are running on
 			 * a hypervisor that passes through individual PCI
-- 
2.34.1