[PATCH v2 00/13] PCI: Simplify PCIe port driver

[PATCH v2 00/13] PCI: Simplify PCIe port driver

[Bjorn Helgaas]

This is an attempt to move a few things out of the port driver.

I added these new patches since v1:

  Merge pcieport_if.h into portdrv.h
    Merge pcieport_if.h and portdrv.h to reduce clutter

  Remove unnecessary "pcie_ports=auto" parameter
    This is the default setting anyway, so specifying the parameter doesn't
    do anything.

  Encapsulate pcie_ports_auto inside the port driver
    "pcie_ports_auto" was declared in linux/pci.h even though nobody
    outside the port driver used it.

  Rename and reverse sense of pcie_ports_auto
    "pcie_ports_auto" is connected with the "pcie_ports=native" parameter,
    so rename it to match.

Other changes since v1:
  - Rebase onto my pci/portdrv branch.
  - Rename pcie_resume_early() to pcie_pme_root_status_cleanup() as
    suggested by Rafael.
  - Add Rafael's Reviewed-by tags.

v1: https://lkml.kernel.org/r/152040297576.240786.1532465558381209070.stgit@bhelgaas-glaptop.roam.corp.google.com

---

Bjorn Helgaas (13):
      PCI/portdrv: Merge pcieport_if.h into portdrv.h
      PCI/PM: Move pcie_clear_root_pme_status() to core
      PCI/PM: Clear PCIe PME Status bit in core, not PCIe port driver
      PCI/PM: Clear PCIe PME Status bit for Root Complex Event Collectors
      PCI/portdrv: Disable port driver in compat mode
      PCI/portdrv: Remove pcie_port_bus_type link order dependency
      PCI/portdrv: Remove unused PCIE_PORT_SERVICE_VC
      PCI/portdrv: Simplify PCIe feature permission checking
      PCI/portdrv: Remove unnecessary include of <linux/pci-aspm.h>
      PCI/portdrv: Remove "pcie_hp=nomsi" kernel parameter
      PCI/portdrv: Remove unnecessary "pcie_ports=auto" parameter
      PCI/portdrv: Encapsulate pcie_ports_auto inside the port driver
      PCI/portdrv: Rename and reverse sense of pcie_ports_auto


 Documentation/admin-guide/kernel-parameters.txt |   19 ++---
 drivers/acpi/pci_root.c                         |   13 +++
 drivers/pci/hotplug/pciehp.h                    |    2 -
 drivers/pci/pci-driver.c                        |   59 +++++++++++++++
 drivers/pci/pci.c                               |    9 ++
 drivers/pci/pci.h                               |    1 
 drivers/pci/pcie/Makefile                       |    3 -
 drivers/pci/pcie/aer/aerdrv.h                   |    2 -
 drivers/pci/pcie/pcie-dpc.c                     |    2 -
 drivers/pci/pcie/pcieport_if.h                  |   71 -------------------
 drivers/pci/pcie/pme.c                          |    1 
 drivers/pci/pcie/portdrv.h                      |   88 ++++++++++++++++-------
 drivers/pci/pcie/portdrv_acpi.c                 |    3 -
 drivers/pci/pcie/portdrv_bus.c                  |   56 ---------------
 drivers/pci/pcie/portdrv_core.c                 |   73 +++++++------------
 drivers/pci/pcie/portdrv_pci.c                  |   54 ++------------
 drivers/pci/probe.c                             |   10 +++
 include/linux/pci.h                             |    5 +
 18 files changed, 198 insertions(+), 273 deletions(-)
 delete mode 100644 drivers/pci/pcie/pcieport_if.h
 delete mode 100644 drivers/pci/pcie/portdrv_bus.c

[PATCH v2 01/13] PCI/portdrv: Merge pcieport_if.h into portdrv.h

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

pcieport_if.h contained the interfaces to register port service driver,
e.g., pcie_port_service_register().  portdrv.h contained internal data
structures of the port driver.

I don't think it's worth keeping those files separate, since both headers
and their users are all inside the PCI core.

Merge pcieport_if.h directly in drivers/pci/pcie/portdrv.h and update the
users to include that instead.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/hotplug/pciehp.h    |    2 +
 drivers/pci/pcie/aer/aerdrv.h   |    2 +
 drivers/pci/pcie/pcie-dpc.c     |    2 +
 drivers/pci/pcie/pcieport_if.h  |   71 ---------------------------------------
 drivers/pci/pcie/pme.c          |    1 -
 drivers/pci/pcie/portdrv.h      |   61 +++++++++++++++++++++++++++++++++-
 drivers/pci/pcie/portdrv_acpi.c |    1 -
 drivers/pci/pcie/portdrv_bus.c  |    1 -
 drivers/pci/pcie/portdrv_core.c |    1 -
 drivers/pci/pcie/portdrv_pci.c  |    1 -
 10 files changed, 63 insertions(+), 80 deletions(-)
 delete mode 100644 drivers/pci/pcie/pcieport_if.h

diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index 08072bcaa381..88e917c9120f 100644
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -23,7 +23,7 @@
 #include <linux/mutex.h>
 #include <linux/workqueue.h>
 
-#include "../pcie/pcieport_if.h"
+#include "../pcie/portdrv.h"
 
 #define MY_NAME	"pciehp"
 
diff --git a/drivers/pci/pcie/aer/aerdrv.h b/drivers/pci/pcie/aer/aerdrv.h
index 568326f385b7..a884f68bada4 100644
--- a/drivers/pci/pcie/aer/aerdrv.h
+++ b/drivers/pci/pcie/aer/aerdrv.h
@@ -13,7 +13,7 @@
 #include <linux/aer.h>
 #include <linux/interrupt.h>
 
-#include "../pcieport_if.h"
+#include "../portdrv.h"
 
 #define SYSTEM_ERROR_INTR_ON_MESG_MASK	(PCI_EXP_RTCTL_SECEE|	\
 					PCI_EXP_RTCTL_SENFEE|	\
diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c
index bac895de4c72..8c57d607e603 100644
--- a/drivers/pci/pcie/pcie-dpc.c
+++ b/drivers/pci/pcie/pcie-dpc.c
@@ -11,7 +11,7 @@
 #include <linux/init.h>
 #include <linux/pci.h>
 
-#include "pcieport_if.h"
+#include "portdrv.h"
 #include "../pci.h"
 #include "aer/aerdrv.h"
 
diff --git a/drivers/pci/pcie/pcieport_if.h b/drivers/pci/pcie/pcieport_if.h
deleted file mode 100644
index b69769dbf659..000000000000
--- a/drivers/pci/pcie/pcieport_if.h
+++ /dev/null
@@ -1,71 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * File:	pcieport_if.h
- * Purpose:	PCI Express Port Bus Driver's IF Data Structure
- *
- * Copyright (C) 2004 Intel
- * Copyright (C) Tom Long Nguyen (tom.l.nguyen@intel.com)
- */
-
-#ifndef _PCIEPORT_IF_H_
-#define _PCIEPORT_IF_H_
-
-/* Port Type */
-#define PCIE_ANY_PORT			(~0)
-
-/* Service Type */
-#define PCIE_PORT_SERVICE_PME_SHIFT	0	/* Power Management Event */
-#define PCIE_PORT_SERVICE_PME		(1 << PCIE_PORT_SERVICE_PME_SHIFT)
-#define PCIE_PORT_SERVICE_AER_SHIFT	1	/* Advanced Error Reporting */
-#define PCIE_PORT_SERVICE_AER		(1 << PCIE_PORT_SERVICE_AER_SHIFT)
-#define PCIE_PORT_SERVICE_HP_SHIFT	2	/* Native Hotplug */
-#define PCIE_PORT_SERVICE_HP		(1 << PCIE_PORT_SERVICE_HP_SHIFT)
-#define PCIE_PORT_SERVICE_VC_SHIFT	3	/* Virtual Channel */
-#define PCIE_PORT_SERVICE_VC		(1 << PCIE_PORT_SERVICE_VC_SHIFT)
-#define PCIE_PORT_SERVICE_DPC_SHIFT	4	/* Downstream Port Containment */
-#define PCIE_PORT_SERVICE_DPC		(1 << PCIE_PORT_SERVICE_DPC_SHIFT)
-
-struct pcie_device {
-	int		irq;	    /* Service IRQ/MSI/MSI-X Vector */
-	struct pci_dev *port;	    /* Root/Upstream/Downstream Port */
-	u32		service;    /* Port service this device represents */
-	void		*priv_data; /* Service Private Data */
-	struct device	device;     /* Generic Device Interface */
-};
-#define to_pcie_device(d) container_of(d, struct pcie_device, device)
-
-static inline void set_service_data(struct pcie_device *dev, void *data)
-{
-	dev->priv_data = data;
-}
-
-static inline void *get_service_data(struct pcie_device *dev)
-{
-	return dev->priv_data;
-}
-
-struct pcie_port_service_driver {
-	const char *name;
-	int (*probe) (struct pcie_device *dev);
-	void (*remove) (struct pcie_device *dev);
-	int (*suspend) (struct pcie_device *dev);
-	int (*resume) (struct pcie_device *dev);
-
-	/* Device driver may resume normal operations */
-	void (*error_resume)(struct pci_dev *dev);
-
-	/* Link Reset Capability - AER service driver specific */
-	pci_ers_result_t (*reset_link) (struct pci_dev *dev);
-
-	int port_type;  /* Type of the port this driver can handle */
-	u32 service;    /* Port service this device represents */
-
-	struct device_driver driver;
-};
-#define to_service_driver(d) \
-	container_of(d, struct pcie_port_service_driver, driver)
-
-int pcie_port_service_register(struct pcie_port_service_driver *new);
-void pcie_port_service_unregister(struct pcie_port_service_driver *new);
-
-#endif /* _PCIEPORT_IF_H_ */
diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c
index d29678958d92..3ed67676ea2a 100644
--- a/drivers/pci/pcie/pme.c
+++ b/drivers/pci/pcie/pme.c
@@ -16,7 +16,6 @@
 #include <linux/device.h>
 #include <linux/pm_runtime.h>
 
-#include "pcieport_if.h"
 #include "../pci.h"
 #include "portdrv.h"
 
diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index a854bc569117..d4009e35702c 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /*
  * File:	portdrv.h
- * Purpose:	PCI Express Port Bus Driver's Internal Data Structures
+ * Purpose:	PCI Express Port Bus Driver's Data Structures
  *
  * Copyright (C) 2004 Intel
  * Copyright (C) Tom Long Nguyen (tom.l.nguyen@intel.com)
@@ -12,7 +12,66 @@
 
 #include <linux/compiler.h>
 
+/* Service Type */
+#define PCIE_PORT_SERVICE_PME_SHIFT	0	/* Power Management Event */
+#define PCIE_PORT_SERVICE_PME		(1 << PCIE_PORT_SERVICE_PME_SHIFT)
+#define PCIE_PORT_SERVICE_AER_SHIFT	1	/* Advanced Error Reporting */
+#define PCIE_PORT_SERVICE_AER		(1 << PCIE_PORT_SERVICE_AER_SHIFT)
+#define PCIE_PORT_SERVICE_HP_SHIFT	2	/* Native Hotplug */
+#define PCIE_PORT_SERVICE_HP		(1 << PCIE_PORT_SERVICE_HP_SHIFT)
+#define PCIE_PORT_SERVICE_VC_SHIFT	3	/* Virtual Channel */
+#define PCIE_PORT_SERVICE_VC		(1 << PCIE_PORT_SERVICE_VC_SHIFT)
+#define PCIE_PORT_SERVICE_DPC_SHIFT	4	/* Downstream Port Containment */
+#define PCIE_PORT_SERVICE_DPC		(1 << PCIE_PORT_SERVICE_DPC_SHIFT)
+
 #define PCIE_PORT_DEVICE_MAXSERVICES   5
+
+/* Port Type */
+#define PCIE_ANY_PORT			(~0)
+
+struct pcie_device {
+	int		irq;	    /* Service IRQ/MSI/MSI-X Vector */
+	struct pci_dev *port;	    /* Root/Upstream/Downstream Port */
+	u32		service;    /* Port service this device represents */
+	void		*priv_data; /* Service Private Data */
+	struct device	device;     /* Generic Device Interface */
+};
+#define to_pcie_device(d) container_of(d, struct pcie_device, device)
+
+static inline void set_service_data(struct pcie_device *dev, void *data)
+{
+	dev->priv_data = data;
+}
+
+static inline void *get_service_data(struct pcie_device *dev)
+{
+	return dev->priv_data;
+}
+
+struct pcie_port_service_driver {
+	const char *name;
+	int (*probe) (struct pcie_device *dev);
+	void (*remove) (struct pcie_device *dev);
+	int (*suspend) (struct pcie_device *dev);
+	int (*resume) (struct pcie_device *dev);
+
+	/* Device driver may resume normal operations */
+	void (*error_resume)(struct pci_dev *dev);
+
+	/* Link Reset Capability - AER service driver specific */
+	pci_ers_result_t (*reset_link) (struct pci_dev *dev);
+
+	int port_type;  /* Type of the port this driver can handle */
+	u32 service;    /* Port service this device represents */
+
+	struct device_driver driver;
+};
+#define to_service_driver(d) \
+	container_of(d, struct pcie_port_service_driver, driver)
+
+int pcie_port_service_register(struct pcie_port_service_driver *new);
+void pcie_port_service_unregister(struct pcie_port_service_driver *new);
+
 /*
  * The PCIe Capability Interrupt Message Number (PCIe r3.1, sec 7.8.2) must
  * be one of the first 32 MSI-X entries.  Per PCI r3.0, sec 6.8.3.1, MSI
diff --git a/drivers/pci/pcie/portdrv_acpi.c b/drivers/pci/pcie/portdrv_acpi.c
index c7d8debb4a5c..53f60053bd47 100644
--- a/drivers/pci/pcie/portdrv_acpi.c
+++ b/drivers/pci/pcie/portdrv_acpi.c
@@ -11,7 +11,6 @@
 #include <linux/acpi.h>
 #include <linux/pci-acpi.h>
 
-#include "pcieport_if.h"
 #include "aer/aerdrv.h"
 #include "../pci.h"
 #include "portdrv.h"
diff --git a/drivers/pci/pcie/portdrv_bus.c b/drivers/pci/pcie/portdrv_bus.c
index b5c5697cfb30..4969ccf6b214 100644
--- a/drivers/pci/pcie/portdrv_bus.c
+++ b/drivers/pci/pcie/portdrv_bus.c
@@ -13,7 +13,6 @@
 #include <linux/errno.h>
 #include <linux/pm.h>
 
-#include "pcieport_if.h"
 #include "portdrv.h"
 
 static int pcie_port_bus_match(struct device *dev, struct device_driver *drv);
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index bab9cb71130f..4268b2fc2c7a 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -17,7 +17,6 @@
 #include <linux/slab.h>
 #include <linux/aer.h>
 
-#include "pcieport_if.h"
 #include "../pci.h"
 #include "portdrv.h"
 
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 13dbe846a1d1..977bd3cca2e5 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -19,7 +19,6 @@
 #include <linux/dmi.h>
 #include <linux/pci-aspm.h>
 
-#include "pcieport_if.h"
 #include "../pci.h"
 #include "portdrv.h"
 

Re: [PATCH v2 01/13] PCI/portdrv: Merge pcieport_if.h into portdrv.h

[Christoph Hellwig]

On Fri, Mar 09, 2018 at 12:59:54PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> pcieport_if.h contained the interfaces to register port service driver,
> e.g., pcie_port_service_register().  portdrv.h contained internal data
> structures of the port driver.
> 
> I don't think it's worth keeping those files separate, since both headers
> and their users are all inside the PCI core.
> 
> Merge pcieport_if.h directly in drivers/pci/pcie/portdrv.h and update the
> users to include that instead.

Looks good to me.  I always found the old struture rather confusing..

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v2 02/13] PCI/PM: Move pcie_clear_root_pme_status() to core

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

Move pcie_clear_root_pme_status() from the port driver to the PCI core so
it will be available even when the port driver isn't present.  No
functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci.c              |    9 +++++++++
 drivers/pci/pci.h              |    1 +
 drivers/pci/pcie/portdrv.h     |    2 --
 drivers/pci/pcie/portdrv_pci.c |    9 ---------
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index f6a4dd10d9b0..120e3393fc35 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1683,6 +1683,15 @@ int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
 }
 EXPORT_SYMBOL_GPL(pci_set_pcie_reset_state);
 
+/**
+ * pcie_clear_root_pme_status - Clear root port PME interrupt status.
+ * @dev: PCIe root port or event collector.
+ */
+void pcie_clear_root_pme_status(struct pci_dev *dev)
+{
+	pcie_capability_set_dword(dev, PCI_EXP_RTSTA, PCI_EXP_RTSTA_PME);
+}
+
 /**
  * pci_check_pme_status - Check if given device has generated PME.
  * @dev: Device to check.
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index fcd81911b127..813ca2c895d8 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -71,6 +71,7 @@ void pci_update_current_state(struct pci_dev *dev, pci_power_t state);
 void pci_power_up(struct pci_dev *dev);
 void pci_disable_enabled_device(struct pci_dev *dev);
 int pci_finish_runtime_suspend(struct pci_dev *dev);
+void pcie_clear_root_pme_status(struct pci_dev *dev);
 int __pci_pme_wakeup(struct pci_dev *dev, void *ign);
 void pci_pme_restore(struct pci_dev *dev);
 bool pci_dev_keep_suspended(struct pci_dev *dev);
diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index d4009e35702c..7086086e45d0 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -93,8 +93,6 @@ void pcie_port_bus_unregister(void);
 
 struct pci_dev;
 
-void pcie_clear_root_pme_status(struct pci_dev *dev);
-
 #ifdef CONFIG_HOTPLUG_PCI_PCIE
 extern bool pciehp_msi_disabled;
 
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 977bd3cca2e5..d6f10a97d400 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -49,15 +49,6 @@ __setup("pcie_ports=", pcie_port_setup);
 
 /* global data */
 
-/**
- * pcie_clear_root_pme_status - Clear root port PME interrupt status.
- * @dev: PCIe root port or event collector.
- */
-void pcie_clear_root_pme_status(struct pci_dev *dev)
-{
-	pcie_capability_set_dword(dev, PCI_EXP_RTSTA, PCI_EXP_RTSTA_PME);
-}
-
 static int pcie_portdrv_restore_config(struct pci_dev *dev)
 {
 	int retval;

Re: [PATCH v2 02/13] PCI/PM: Move pcie_clear_root_pme_status() to core

[Christoph Hellwig]

On Fri, Mar 09, 2018 at 01:00:00PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> Move pcie_clear_root_pme_status() from the port driver to the PCI core so
> it will be available even when the port driver isn't present.  No
> functional change intended.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v2 03/13] PCI/PM: Clear PCIe PME Status bit in core, not PCIe port driver

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

fe31e69740ed ("PCI/PCIe: Clear Root PME Status bits early during system
resume") added a .resume_noirq() callback to the PCIe port driver to clear
the PME Status bit during resume to work around a BIOS issue.

The BIOS evidently enabled PME interrupts for ACPI-based runtime wakeups
but did not clear the PME Status bit during resume, which meant PMEs after
resume did not trigger interrupts because PME Status did not transition
from cleared to set.

The fix was in the PCIe port driver, so it worked when CONFIG_PCIEPORTBUS
was set.  But I think we *always* want the fix because the platform may use
PME interrupts even if Linux is built without the PCIe port driver.

Move the fix from the port driver to the PCI core so we can work around
this "PME doesn't work after waking from a sleep state" issue regardless of
CONFIG_PCIEPORTBUS.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci-driver.c       |   14 ++++++++++++++
 drivers/pci/pcie/portdrv_pci.c |   15 ---------------
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 3bed6beda051..e561fa0f456c 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -525,6 +525,18 @@ static void pci_pm_default_resume_early(struct pci_dev *pci_dev)
 	pci_fixup_device(pci_fixup_resume_early, pci_dev);
 }
 
+static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
+{
+	/*
+	 * Some BIOSes forget to clear Root PME Status bits after system
+	 * wakeup, which breaks ACPI-based runtime wakeup on PCI Express.
+	 * Clear those bits now just in case (shouldn't hurt).
+	 */
+	if (pci_is_pcie(pci_dev) &&
+	    pci_pcie_type(pci_dev) == PCI_EXP_TYPE_ROOT_PORT)
+		pcie_clear_root_pme_status(pci_dev);
+}
+
 /*
  * Default "suspend" method for devices that have no driver provided suspend,
  * or not even a driver at all (second part).
@@ -873,6 +885,8 @@ static int pci_pm_resume_noirq(struct device *dev)
 	if (pci_has_legacy_pm_support(pci_dev))
 		return pci_legacy_resume_early(dev);
 
+	pcie_pme_root_status_cleanup(pci_dev);
+
 	if (drv && drv->pm && drv->pm->resume_noirq)
 		error = drv->pm->resume_noirq(dev);
 
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index d6f10a97d400..ec9e936c2a5b 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -61,20 +61,6 @@ static int pcie_portdrv_restore_config(struct pci_dev *dev)
 }
 
 #ifdef CONFIG_PM
-static int pcie_port_resume_noirq(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-
-	/*
-	 * Some BIOSes forget to clear Root PME Status bits after system wakeup
-	 * which breaks ACPI-based runtime wakeup on PCI Express, so clear those
-	 * bits now just in case (shouldn't hurt).
-	 */
-	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT)
-		pcie_clear_root_pme_status(pdev);
-	return 0;
-}
-
 static int pcie_port_runtime_suspend(struct device *dev)
 {
 	return to_pci_dev(dev)->bridge_d3 ? 0 : -EBUSY;
@@ -102,7 +88,6 @@ static const struct dev_pm_ops pcie_portdrv_pm_ops = {
 	.thaw		= pcie_port_device_resume,
 	.poweroff	= pcie_port_device_suspend,
 	.restore	= pcie_port_device_resume,
-	.resume_noirq	= pcie_port_resume_noirq,
 	.runtime_suspend = pcie_port_runtime_suspend,
 	.runtime_resume	= pcie_port_runtime_resume,
 	.runtime_idle	= pcie_port_runtime_idle,

[PATCH v2 04/13] PCI/PM: Clear PCIe PME Status bit for Root Complex Event Collectors

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

Per PCIe r4.0, sec 6.1.6, Root Complex Event Collectors can generate PME
interrupts on behalf of Root Complex Integrated Endpoints.

Linux does not currently enable PME interrupts from RC Event Collectors,
but fe31e69740ed ("PCI/PCIe: Clear Root PME Status bits early during system
resume") suggests PME interrupts may be enabled by the platform for ACPI-
based runtime wakeup.

Clear the PCIe PME Status bit for Root Complex Event Collectors during
resume, just like we already do for Root Ports.

If the BIOS enables PME interrupts for an event collector and neglects to
clear the status bit on resume, this change should fix the same bug as
fe31e69740ed (PMEs not working after waking from a sleep state), but for
Root Complex Integrated Endpoints.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci-driver.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e561fa0f456c..204d2b54c2a4 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -533,7 +533,8 @@ static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
 	 * Clear those bits now just in case (shouldn't hurt).
 	 */
 	if (pci_is_pcie(pci_dev) &&
-	    pci_pcie_type(pci_dev) == PCI_EXP_TYPE_ROOT_PORT)
+	    (pci_pcie_type(pci_dev) == PCI_EXP_TYPE_ROOT_PORT ||
+	     pci_pcie_type(pci_dev) == PCI_EXP_TYPE_RC_EC))
 		pcie_clear_root_pme_status(pci_dev);
 }
 

[PATCH v2 05/13] PCI/portdrv: Disable port driver in compat mode

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

The "pcie_ports=compat" kernel parameter sets pcie_ports_disabled, which is
intended to disable the PCIe port driver.  But even when it was disabled,
we registered pcie_portdriver so we could work around a BIOS PME issue (see
fe31e69740ed ("PCI/PCIe: Clear Root PME Status bits early during system
resume")).

Registering the driver meant that the pcie_portdrv_probe() path called
pci_enable_device(), pci_save_state(), pm_runtime_set_autosuspend_delay(),
pm_runtime_use_autosuspend(), etc., even when the driver was disabled.

We've since moved the BIOS PME workaround from the port driver to the core,
so stop registering the PCIe port driver in compat mode.

This means "pcie_ports=compat" will now be basically the same as turning
off CONFIG_PCIEPORTBUS completely.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pcie/portdrv_core.c |    3 ---
 drivers/pci/pcie/portdrv_pci.c  |    2 +-
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 4268b2fc2c7a..9a41751db332 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -211,9 +211,6 @@ static int get_port_device_capability(struct pci_dev *dev)
 	int services = 0;
 	int cap_mask = 0;
 
-	if (pcie_ports_disabled)
-		return 0;
-
 	cap_mask = PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP
 			| PCIE_PORT_SERVICE_VC;
 	if (pci_aer_available())
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index ec9e936c2a5b..5d9d5305ebef 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -261,7 +261,7 @@ static int __init pcie_portdrv_init(void)
 	int retval;
 
 	if (pcie_ports_disabled)
-		return pci_register_driver(&pcie_portdriver);
+		return -EACCES;
 
 	dmi_check_system(pcie_portdrv_dmi_table);
 

[PATCH v2 06/13] PCI/portdrv: Remove pcie_port_bus_type link order dependency

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

The pcie_port_bus_type must be registered before drivers that depend on it
can be registered.  Those drivers include:

  pcied_init()                # PCIe native hotplug driver
  aer_service_init()          # AER driver
  dpc_service_init()          # DPC driver
  pcie_pme_service_init()     # PME driver

Previously we registered pcie_port_bus_type from pcie_portdrv_init(), a
device_initcall.  The callers of pcie_port_service_register() (above) are
also device_initcalls.  This is fragile because the device_initcall
ordering depends on link order, which is not explicit.

Register pcie_port_bus_type from pci_driver_init() along with pci_bus_type.
This removes the link order dependency between portdrv and the pciehp, AER,
DPC, and PCIe PME drivers.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci-driver.c       |   44 +++++++++++++++++++++++++++++++-
 drivers/pci/pcie/Makefile      |    2 +
 drivers/pci/pcie/portdrv_bus.c |   55 ----------------------------------------
 drivers/pci/pcie/portdrv_pci.c |   13 +--------
 4 files changed, 45 insertions(+), 69 deletions(-)
 delete mode 100644 drivers/pci/pcie/portdrv_bus.c

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 204d2b54c2a4..02ecbdafe38b 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -19,6 +19,7 @@
 #include <linux/suspend.h>
 #include <linux/kexec.h>
 #include "pci.h"
+#include "pcie/portdrv.h"
 
 struct pci_dynid {
 	struct list_head node;
@@ -1553,8 +1554,49 @@ struct bus_type pci_bus_type = {
 };
 EXPORT_SYMBOL(pci_bus_type);
 
+#ifdef CONFIG_PCIEPORTBUS
+static int pcie_port_bus_match(struct device *dev, struct device_driver *drv)
+{
+	struct pcie_device *pciedev;
+	struct pcie_port_service_driver *driver;
+
+	if (drv->bus != &pcie_port_bus_type || dev->bus != &pcie_port_bus_type)
+		return 0;
+
+	pciedev = to_pcie_device(dev);
+	driver = to_service_driver(drv);
+
+	if (driver->service != pciedev->service)
+		return 0;
+
+	if ((driver->port_type != PCIE_ANY_PORT) &&
+	    (driver->port_type != pci_pcie_type(pciedev->port)))
+		return 0;
+
+	return 1;
+}
+
+struct bus_type pcie_port_bus_type = {
+	.name		= "pci_express",
+	.match		= pcie_port_bus_match,
+};
+EXPORT_SYMBOL_GPL(pcie_port_bus_type);
+#endif
+
 static int __init pci_driver_init(void)
 {
-	return bus_register(&pci_bus_type);
+	int ret;
+
+	ret = bus_register(&pci_bus_type);
+	if (ret)
+		return ret;
+
+#ifdef CONFIG_PCIEPORTBUS
+	ret = bus_register(&pcie_port_bus_type);
+	if (ret)
+		return ret;
+#endif
+
+	return 0;
 }
 postcore_initcall(pci_driver_init);
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index 223e4c34c29a..e01c10c97b95 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -6,7 +6,7 @@
 # Build PCI Express ASPM if needed
 obj-$(CONFIG_PCIEASPM)		+= aspm.o
 
-pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o
+pcieportdrv-y			:= portdrv_core.o portdrv_pci.o
 pcieportdrv-$(CONFIG_ACPI)	+= portdrv_acpi.o
 
 obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
diff --git a/drivers/pci/pcie/portdrv_bus.c b/drivers/pci/pcie/portdrv_bus.c
deleted file mode 100644
index 4969ccf6b214..000000000000
--- a/drivers/pci/pcie/portdrv_bus.c
+++ /dev/null
@@ -1,55 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * File:	portdrv_bus.c
- * Purpose:	PCI Express Port Bus Driver's Bus Overloading Functions
- *
- * Copyright (C) 2004 Intel
- * Copyright (C) Tom Long Nguyen (tom.l.nguyen@intel.com)
- */
-
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/kernel.h>
-#include <linux/errno.h>
-#include <linux/pm.h>
-
-#include "portdrv.h"
-
-static int pcie_port_bus_match(struct device *dev, struct device_driver *drv);
-
-struct bus_type pcie_port_bus_type = {
-	.name		= "pci_express",
-	.match		= pcie_port_bus_match,
-};
-EXPORT_SYMBOL_GPL(pcie_port_bus_type);
-
-static int pcie_port_bus_match(struct device *dev, struct device_driver *drv)
-{
-	struct pcie_device *pciedev;
-	struct pcie_port_service_driver *driver;
-
-	if (drv->bus != &pcie_port_bus_type || dev->bus != &pcie_port_bus_type)
-		return 0;
-
-	pciedev = to_pcie_device(dev);
-	driver = to_service_driver(drv);
-
-	if (driver->service != pciedev->service)
-		return 0;
-
-	if ((driver->port_type != PCIE_ANY_PORT) &&
-	    (driver->port_type != pci_pcie_type(pciedev->port)))
-		return 0;
-
-	return 1;
-}
-
-int pcie_port_bus_register(void)
-{
-	return bus_register(&pcie_port_bus_type);
-}
-
-void pcie_port_bus_unregister(void)
-{
-	bus_unregister(&pcie_port_bus_type);
-}
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 5d9d5305ebef..127321e17184 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -258,22 +258,11 @@ static const struct dmi_system_id pcie_portdrv_dmi_table[] __initconst = {
 
 static int __init pcie_portdrv_init(void)
 {
-	int retval;
-
 	if (pcie_ports_disabled)
 		return -EACCES;
 
 	dmi_check_system(pcie_portdrv_dmi_table);
 
-	retval = pcie_port_bus_register();
-	if (retval) {
-		printk(KERN_WARNING "PCIE: bus_register error: %d\n", retval);
-		goto out;
-	}
-	retval = pci_register_driver(&pcie_portdriver);
-	if (retval)
-		pcie_port_bus_unregister();
- out:
-	return retval;
+	return pci_register_driver(&pcie_portdriver);
 }
 device_initcall(pcie_portdrv_init);

Re: [PATCH v2 06/13] PCI/portdrv: Remove pcie_port_bus_type link order dependency

[Christoph Hellwig]

> +	if ((driver->port_type != PCIE_ANY_PORT) &&
> +	    (driver->port_type != pci_pcie_type(pciedev->port)))

No need for the inner braces here.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2 06/13] PCI/portdrv: Remove pcie_port_bus_type link order dependency

[Bjorn Helgaas]

On Mon, Mar 12, 2018 at 01:01:47AM -0700, Christoph Hellwig wrote:
> > +	if ((driver->port_type != PCIE_ANY_PORT) &&
> > +	    (driver->port_type != pci_pcie_type(pciedev->port)))
> 
> No need for the inner braces here.

Thanks, I removed them.

[PATCH v2 07/13] PCI/portdrv: Remove unused PCIE_PORT_SERVICE_VC

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

No driver registers for PCIE_PORT_SERVICE_VC, so remove it.

This removes the VC "service" files from /sys/bus/pci_express/devices,
e.g., 0000:07:00.0:pcie108, 0000:08:04.0:pcie208 (all the files that
contained "8" as the last digit of the "pcieXXX" part).  The port driver
created these files for PCIe port devices that have a VC Capability.

Since this reduces PCIE_PORT_DEVICE_MAXSERVICES and moves DPC down into the
spot where VC used to be, the DPC sysfs files will now be named "pcieXX8".
I don't think there's anything useful userspace can do with those files, so
I hope nobody cares about these filenames.

There is no VC driver that calls pcie_port_service_register(), so there
never was a /sys/bus/pci_express/drivers/vc directory.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pcie/portdrv.h      |    6 ++----
 drivers/pci/pcie/portdrv_acpi.c |    2 +-
 drivers/pci/pcie/portdrv_core.c |   14 ++++----------
 3 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index 7086086e45d0..7bfd75f9197b 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -19,12 +19,10 @@
 #define PCIE_PORT_SERVICE_AER		(1 << PCIE_PORT_SERVICE_AER_SHIFT)
 #define PCIE_PORT_SERVICE_HP_SHIFT	2	/* Native Hotplug */
 #define PCIE_PORT_SERVICE_HP		(1 << PCIE_PORT_SERVICE_HP_SHIFT)
-#define PCIE_PORT_SERVICE_VC_SHIFT	3	/* Virtual Channel */
-#define PCIE_PORT_SERVICE_VC		(1 << PCIE_PORT_SERVICE_VC_SHIFT)
-#define PCIE_PORT_SERVICE_DPC_SHIFT	4	/* Downstream Port Containment */
+#define PCIE_PORT_SERVICE_DPC_SHIFT	3	/* Downstream Port Containment */
 #define PCIE_PORT_SERVICE_DPC		(1 << PCIE_PORT_SERVICE_DPC_SHIFT)
 
-#define PCIE_PORT_DEVICE_MAXSERVICES   5
+#define PCIE_PORT_DEVICE_MAXSERVICES   4
 
 /* Port Type */
 #define PCIE_ANY_PORT			(~0)
diff --git a/drivers/pci/pcie/portdrv_acpi.c b/drivers/pci/pcie/portdrv_acpi.c
index 53f60053bd47..9d12650dc2ae 100644
--- a/drivers/pci/pcie/portdrv_acpi.c
+++ b/drivers/pci/pcie/portdrv_acpi.c
@@ -47,7 +47,7 @@ void pcie_port_acpi_setup(struct pci_dev *port, int *srv_mask)
 
 	flags = root->osc_control_set;
 
-	*srv_mask = PCIE_PORT_SERVICE_VC | PCIE_PORT_SERVICE_DPC;
+	*srv_mask = PCIE_PORT_SERVICE_DPC;
 	if (flags & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)
 		*srv_mask |= PCIE_PORT_SERVICE_HP;
 	if (flags & OSC_PCI_EXPRESS_PME_CONTROL)
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 9a41751db332..bf851da97947 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -188,10 +188,8 @@ static int pcie_init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
 	if (ret < 0)
 		return -ENODEV;
 
-	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++) {
-		if (i != PCIE_PORT_SERVICE_VC_SHIFT)
-			irqs[i] = pci_irq_vector(dev, 0);
-	}
+	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
+		irqs[i] = pci_irq_vector(dev, 0);
 
 	return 0;
 }
@@ -211,8 +209,7 @@ static int get_port_device_capability(struct pci_dev *dev)
 	int services = 0;
 	int cap_mask = 0;
 
-	cap_mask = PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP
-			| PCIE_PORT_SERVICE_VC;
+	cap_mask = PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP;
 	if (pci_aer_available())
 		cap_mask |= PCIE_PORT_SERVICE_AER | PCIE_PORT_SERVICE_DPC;
 
@@ -239,9 +236,6 @@ static int get_port_device_capability(struct pci_dev *dev)
 		 */
 		pci_disable_pcie_error_reporting(dev);
 	}
-	/* VC support */
-	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VC))
-		services |= PCIE_PORT_SERVICE_VC;
 	/* Root ports are capable of generating PME too */
 	if ((cap_mask & PCIE_PORT_SERVICE_PME)
 	    && pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
@@ -331,7 +325,7 @@ int pcie_port_device_register(struct pci_dev *dev)
 	 */
 	status = pcie_init_service_irqs(dev, irqs, capabilities);
 	if (status) {
-		capabilities &= PCIE_PORT_SERVICE_VC | PCIE_PORT_SERVICE_HP;
+		capabilities &= PCIE_PORT_SERVICE_HP;
 		if (!capabilities)
 			goto error_disable;
 	}

Re: [PATCH v2 07/13] PCI/portdrv: Remove unused PCIE_PORT_SERVICE_VC

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v2 08/13] PCI/portdrv: Simplify PCIe feature permission checking

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

Some PCIe features (AER, DPC, hotplug, PME) can be managed by either the
platform firmware or the OS, so the host bridge driver may have to request
permission from the platform before using them.  On ACPI systems, this is
done by negotiate_os_control() in acpi_pci_root_add().

The PCIe port driver later uses pcie_port_platform_notify() and
pcie_port_acpi_setup() to figure out whether it can use these features.
But all we need is a single bit for each service, so these interfaces are
needlessly complicated.

Simplify this by adding bits in the struct pci_host_bridge to show when the
OS has permission to use each feature:

  + unsigned int use_aer:1;       /* OS may use PCIe AER */
  + unsigned int use_hotplug:1;	  /* OS may use PCIe hotplug */
  + unsigned int use_pme:1;       /* OS may use PCIe PME */

These are set when we create a host bridge, and the host bridge driver can
clear the bits corresponding to any feature the platform doesn't want us to
use.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/pci_root.c         |   13 ++++++++++--
 drivers/pci/pcie/Makefile       |    1 -
 drivers/pci/pcie/portdrv.h      |   11 ----------
 drivers/pci/pcie/portdrv_core.c |   42 ++++++++++++++++++++++++---------------
 drivers/pci/probe.c             |   10 +++++++++
 include/linux/pci.h             |    3 +++
 6 files changed, 50 insertions(+), 30 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 6fc204a52493..65ebefb99815 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -871,6 +871,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 	struct acpi_device *device = root->device;
 	int node = acpi_get_node(device->handle);
 	struct pci_bus *bus;
+	struct pci_host_bridge *host_bridge;
 
 	info->root = root;
 	info->bridge = device;
@@ -895,9 +896,17 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 	if (!bus)
 		goto out_release_info;
 
+	host_bridge = to_pci_host_bridge(bus->bridge);
+	if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL))
+		host_bridge->use_hotplug = 0;
+	if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL))
+		host_bridge->use_aer = 0;
+	if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL))
+		host_bridge->use_pme = 0;
+
 	pci_scan_child_bus(bus);
-	pci_set_host_bridge_release(to_pci_host_bridge(bus->bridge),
-				    acpi_pci_root_release_info, info);
+	pci_set_host_bridge_release(host_bridge, acpi_pci_root_release_info,
+				    info);
 	if (node != NUMA_NO_NODE)
 		dev_printk(KERN_DEBUG, &bus->dev, "on NUMA node %d\n", node);
 	return bus;
diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index e01c10c97b95..11fb633b866c 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -7,7 +7,6 @@
 obj-$(CONFIG_PCIEASPM)		+= aspm.o
 
 pcieportdrv-y			:= portdrv_core.o portdrv_pci.o
-pcieportdrv-$(CONFIG_ACPI)	+= portdrv_acpi.o
 
 obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o
 
diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index 7bfd75f9197b..ed84e767085f 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -123,15 +123,4 @@ static inline bool pcie_pme_no_msi(void) { return false; }
 static inline void pcie_pme_interrupt_enable(struct pci_dev *dev, bool en) {}
 #endif /* !CONFIG_PCIE_PME */
 
-#ifdef CONFIG_ACPI
-void pcie_port_acpi_setup(struct pci_dev *port, int *mask);
-
-static inline void pcie_port_platform_notify(struct pci_dev *port, int *mask)
-{
-	pcie_port_acpi_setup(port, mask);
-}
-#else /* !CONFIG_ACPI */
-static inline void pcie_port_platform_notify(struct pci_dev *port, int *mask){}
-#endif /* !CONFIG_ACPI */
-
 #endif /* _PORTDRV_H_ */
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index bf851da97947..589960fdd8a8 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -206,19 +206,20 @@ static int pcie_init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
  */
 static int get_port_device_capability(struct pci_dev *dev)
 {
+	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
+	bool native;
 	int services = 0;
-	int cap_mask = 0;
 
-	cap_mask = PCIE_PORT_SERVICE_PME | PCIE_PORT_SERVICE_HP;
-	if (pci_aer_available())
-		cap_mask |= PCIE_PORT_SERVICE_AER | PCIE_PORT_SERVICE_DPC;
-
-	if (pcie_ports_auto)
-		pcie_port_platform_notify(dev, &cap_mask);
+	/*
+	 * If the user specified "pcie_ports=native", use the PCIe services
+	 * regardless of whether the platform has given us permission.  On
+	 * ACPI systems, this means we ignore _OSC.
+	 */
+	native = !pcie_ports_auto;
 
-	/* Hot-Plug Capable */
-	if ((cap_mask & PCIE_PORT_SERVICE_HP) && dev->is_hotplug_bridge) {
+	if (dev->is_hotplug_bridge && (native || host->use_hotplug)) {
 		services |= PCIE_PORT_SERVICE_HP;
+
 		/*
 		 * Disable hot-plug interrupts in case they have been enabled
 		 * by the BIOS and the hot-plug service driver is not loaded.
@@ -226,20 +227,27 @@ static int get_port_device_capability(struct pci_dev *dev)
 		pcie_capability_clear_word(dev, PCI_EXP_SLTCTL,
 			  PCI_EXP_SLTCTL_CCIE | PCI_EXP_SLTCTL_HPIE);
 	}
-	/* AER capable */
-	if ((cap_mask & PCIE_PORT_SERVICE_AER)
-	    && pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR)) {
+
+	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR) &&
+	    pci_aer_available() && (native || host->use_aer)) {
 		services |= PCIE_PORT_SERVICE_AER;
+
 		/*
 		 * Disable AER on this port in case it's been enabled by the
 		 * BIOS (the AER service driver will enable it when necessary).
 		 */
 		pci_disable_pcie_error_reporting(dev);
 	}
-	/* Root ports are capable of generating PME too */
-	if ((cap_mask & PCIE_PORT_SERVICE_PME)
-	    && pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
+
+	/*
+	 * Root ports are capable of generating PME too.  Root Complex
+	 * Event Collectors can also generate PMEs, but we don't handle
+	 * those yet.
+	 */
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT &&
+	    (native || host->use_pme)) {
 		services |= PCIE_PORT_SERVICE_PME;
+
 		/*
 		 * Disable PME interrupt on this port in case it's been enabled
 		 * by the BIOS (the PME service driver will enable it when
@@ -247,7 +255,9 @@ static int get_port_device_capability(struct pci_dev *dev)
 		 */
 		pcie_pme_interrupt_enable(dev, false);
 	}
-	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC))
+
+	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
+	    pci_aer_available())
 		services |= PCIE_PORT_SERVICE_DPC;
 
 	return services;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ef5377438a1e..839fb0059900 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -540,6 +540,16 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
 	INIT_LIST_HEAD(&bridge->windows);
 	bridge->dev.release = pci_release_host_bridge_dev;
 
+	/*
+	 * We assume we can manage these PCIe features.  Some systems may
+	 * reserve these for use by the platform itself, e.g., an ACPI BIOS
+	 * may implement its own AER handling and use _OSC to prevent the
+	 * OS from interfering.
+	 */
+	bridge->use_aer = 1;
+	bridge->use_hotplug = 1;
+	bridge->use_pme = 1;
+
 	return bridge;
 }
 EXPORT_SYMBOL(pci_alloc_host_bridge);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 024a1beda008..40aec7a6fdd9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -469,6 +469,9 @@ struct pci_host_bridge {
 	struct msi_controller *msi;
 	unsigned int	ignore_reset_delay:1;	/* For entire hierarchy */
 	unsigned int	no_ext_tags:1;		/* No Extended Tags */
+	unsigned int	use_aer:1;		/* OS may use PCIe AER */
+	unsigned int	use_hotplug:1;		/* OS may use PCIe hotplug */
+	unsigned int	use_pme:1;		/* OS may use PCIe PME */
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,
 			const struct resource *res,

Re: [PATCH v2 08/13] PCI/portdrv: Simplify PCIe feature permission checking

[Christoph Hellwig]

> +	 * We assume we can manage these PCIe features.  Some systems may
> +	 * reserve these for use by the platform itself, e.g., an ACPI BIOS
> +	 * may implement its own AER handling and use _OSC to prevent the
> +	 * OS from interfering.
> +	 */
> +	bridge->use_aer = 1;
> +	bridge->use_hotplug = 1;
> +	bridge->use_pme = 1;

If we start out with enabled maybe these should be disable_foo flags
instead?

Also please use bool (or a bitfield bool) for true/false values.

Re: [PATCH v2 08/13] PCI/portdrv: Simplify PCIe feature permission checking

[Bjorn Helgaas]

On Mon, Mar 12, 2018 at 01:04:02AM -0700, Christoph Hellwig wrote:
> > +	 * We assume we can manage these PCIe features.  Some systems may
> > +	 * reserve these for use by the platform itself, e.g., an ACPI BIOS
> > +	 * may implement its own AER handling and use _OSC to prevent the
> > +	 * OS from interfering.
> > +	 */
> > +	bridge->use_aer = 1;
> > +	bridge->use_hotplug = 1;
> > +	bridge->use_pme = 1;
> 
> If we start out with enabled maybe these should be disable_foo flags
> instead?

I went back and forth on that.  "disable_foo" is nice because the
default value is correct (zero means enabled).  But then you end up
with things like:

  if (pcie_ports_native || !host->disable_hotplug)

where the "!host->disable_hotplug" is a double negative, and I have a
really hard time reading that.

> Also please use bool (or a bitfield bool) for true/false values.

I'm a little ambivalent about bool in structs because of things like
this:

  https://lkml.kernel.org/r/CA+55aFxnePDimkVKVtv3gNmRGcwc8KQ5mHYvUxY8sAQg6yvVYg@mail.gmail.com

Bjorn

Re: [PATCH v2 08/13] PCI/portdrv: Simplify PCIe feature permission checking

[Lukas Wunner]

On Mon, Mar 12, 2018 at 09:03:16AM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 12, 2018 at 01:04:02AM -0700, Christoph Hellwig wrote:
> > > +	 * We assume we can manage these PCIe features.  Some systems may
> > > +	 * reserve these for use by the platform itself, e.g., an ACPI BIOS
> > > +	 * may implement its own AER handling and use _OSC to prevent the
> > > +	 * OS from interfering.
> > > +	 */
> > > +	bridge->use_aer = 1;
> > > +	bridge->use_hotplug = 1;
> > > +	bridge->use_pme = 1;
> > 
> > If we start out with enabled maybe these should be disable_foo flags
> > instead?
> 
> I went back and forth on that.  "disable_foo" is nice because the
> default value is correct (zero means enabled).  But then you end up
> with things like:
> 
>   if (pcie_ports_native || !host->disable_hotplug)
> 
> where the "!host->disable_hotplug" is a double negative, and I have a
> really hard time reading that.

native_hotplug or, if you want it reversed, platform_hotplug
(or firmware_hotplug?) might improve readability.

Thanks,

Lukas

Re: [PATCH v2 08/13] PCI/portdrv: Simplify PCIe feature permission checking

[Bjorn Helgaas]

On Mon, Mar 12, 2018 at 03:20:59PM +0100, Lukas Wunner wrote:
> On Mon, Mar 12, 2018 at 09:03:16AM -0500, Bjorn Helgaas wrote:
> > On Mon, Mar 12, 2018 at 01:04:02AM -0700, Christoph Hellwig wrote:
> > > > +	 * We assume we can manage these PCIe features.  Some systems may
> > > > +	 * reserve these for use by the platform itself, e.g., an ACPI BIOS
> > > > +	 * may implement its own AER handling and use _OSC to prevent the
> > > > +	 * OS from interfering.
> > > > +	 */
> > > > +	bridge->use_aer = 1;
> > > > +	bridge->use_hotplug = 1;
> > > > +	bridge->use_pme = 1;
> > > 
> > > If we start out with enabled maybe these should be disable_foo flags
> > > instead?
> > 
> > I went back and forth on that.  "disable_foo" is nice because the
> > default value is correct (zero means enabled).  But then you end up
> > with things like:
> > 
> >   if (pcie_ports_native || !host->disable_hotplug)
> > 
> > where the "!host->disable_hotplug" is a double negative, and I have a
> > really hard time reading that.
> 
> native_hotplug or, if you want it reversed, platform_hotplug
> (or firmware_hotplug?) might improve readability.

Thanks, I like that.  I renamed them to "native_aer", etc.

Re: [PATCH v2 08/13] PCI/portdrv: Simplify PCIe feature permission checking

[David Woodhouse]

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

On Fri, 2018-03-09 at 13:00 -0600, Bjorn Helgaas wrote:
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -540,6 +540,16 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
>         INIT_LIST_HEAD(&bridge->windows);
>         bridge->dev.release = pci_release_host_bridge_dev;
>  
> +       /*
> +        * We assume we can manage these PCIe features.  Some systems may
> +        * reserve these for use by the platform itself, e.g., an ACPI BIOS
> +        * may implement its own AER handling and use _OSC to prevent the
> +        * OS from interfering.
> +        */
> +       bridge->use_aer = 1;
> +       bridge->use_hotplug = 1;
> +       bridge->use_pme = 1;
> +
>         return bridge;
>  }
>  EXPORT_SYMBOL(pci_alloc_host_bridge);

Is there a good reason why you've done this only for
pci_alloc_host_bridge() and not also for devm_pci_alloc_host_bridge()? 

In fact, perhaps the devm version should actually call
pci_alloc_host_bridge() and then just override the release method?


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

Re: [PATCH v2 08/13] PCI/portdrv: Simplify PCIe feature permission checking

[Bjorn Helgaas]

On Tue, May 07, 2019 at 01:00:03PM +0100, David Woodhouse wrote:
> On Fri, 2018-03-09 at 13:00 -0600, Bjorn Helgaas wrote:
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -540,6 +540,16 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
> >         INIT_LIST_HEAD(&bridge->windows);
> >         bridge->dev.release = pci_release_host_bridge_dev;
> >  
> > +       /*
> > +        * We assume we can manage these PCIe features.  Some systems may
> > +        * reserve these for use by the platform itself, e.g., an ACPI BIOS
> > +        * may implement its own AER handling and use _OSC to prevent the
> > +        * OS from interfering.
> > +        */
> > +       bridge->use_aer = 1;
> > +       bridge->use_hotplug = 1;
> > +       bridge->use_pme = 1;
> > +
> >         return bridge;
> >  }
> >  EXPORT_SYMBOL(pci_alloc_host_bridge);
> 
> Is there a good reason why you've done this only for
> pci_alloc_host_bridge() and not also for devm_pci_alloc_host_bridge()? 

No good reason; I just screwed up.  Should be fixed in v5.2 (and marked for
stable):

https://lore.kernel.org/linux-pci/20190318160718.10925-1-jean-philippe.brucker@arm.com

Sorry about that.

Bjorn

Re: [PATCH v2 08/13] PCI/portdrv: Simplify PCIe feature permission checking

[David Woodhouse]

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

On Tue, 2019-05-07 at 07:49 -0500, Bjorn Helgaas wrote:
> No good reason; I just screwed up.  Should be fixed in v5.2 (and marked for
> stable):
> 
> https://lore.kernel.org/linux-pci/20190318160718.10925-1-jean-philippe.brucker@arm.com

Aha, thanks. And I see 'dwc: Use devm_pci_alloc_host_bridge()' in there
too, which was the reason we were staring at it in the first place.

That's actually fixing a leak in the error path, not just simplifying
it, and might want tagging for stable too if it's realistic that
devm_of_pci_get_host_bridge_resources() would ever actually fail.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

Re: [PATCH v2 08/13] PCI/portdrv: Simplify PCIe feature permission checking

[Bjorn Helgaas]

On Tue, May 07, 2019 at 02:02:34PM +0100, David Woodhouse wrote:
> On Tue, 2019-05-07 at 07:49 -0500, Bjorn Helgaas wrote:
> > No good reason; I just screwed up.  Should be fixed in v5.2 (and marked for
> > stable):
> > 
> > https://lore.kernel.org/linux-pci/20190318160718.10925-1-jean-philippe.brucker@arm.com
> 
> Aha, thanks. And I see 'dwc: Use devm_pci_alloc_host_bridge()' in there
> too, which was the reason we were staring at it in the first place.
> 
> That's actually fixing a leak in the error path, not just simplifying
> it, and might want tagging for stable too if it's realistic that
> devm_of_pci_get_host_bridge_resources() would ever actually fail.

OK, IIUC, you're proposing this, where I added the stable tag to "dwc:
Use devm_pci_alloc_host_bridge()":

  http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?id=e6fdd3bf5aec

Lorenzo, just FYI, I cherry-picked remotes/lorenzo/pci/dwc to a pci/dwc
branch to add that tag.

Bjorn

Re: [PATCH v2 08/13] PCI/portdrv: Simplify PCIe feature permission checking

[David Woodhouse]

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

On Tue, 2019-05-07 at 09:07 -0500, Bjorn Helgaas wrote:
> On Tue, May 07, 2019 at 02:02:34PM +0100, David Woodhouse wrote:
> > On Tue, 2019-05-07 at 07:49 -0500, Bjorn Helgaas wrote:
> > > No good reason; I just screwed up.  Should be fixed in v5.2 (and marked for
> > > stable):
> > > 
> > > https://lore.kernel.org/linux-pci/20190318160718.10925-1-jean-philippe.brucker@arm.com
> > 
> > Aha, thanks. And I see 'dwc: Use devm_pci_alloc_host_bridge()' in there
> > too, which was the reason we were staring at it in the first place.
> > 
> > That's actually fixing a leak in the error path, not just simplifying
> > it, and might want tagging for stable too if it's realistic that
> > devm_of_pci_get_host_bridge_resources() would ever actually fail.
> 
> OK, IIUC, you're proposing this, where I added the stable tag to "dwc:
> Use devm_pci_alloc_host_bridge()":
> 
>   http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?id=e6fdd3bf5aec

Indeed. Thanks.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

[PATCH v2 09/13] PCI/portdrv: Remove unnecessary include of <linux/pci-aspm.h>

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

portdrv_pci.c doesn't use anything from <linux/pci-aspm.h>.  Remove the
include of it.  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pcie/portdrv_pci.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 127321e17184..1997d9f2743e 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -17,7 +17,6 @@
 #include <linux/init.h>
 #include <linux/aer.h>
 #include <linux/dmi.h>
-#include <linux/pci-aspm.h>
 
 #include "../pci.h"
 #include "portdrv.h"

[PATCH v2 10/13] PCI/portdrv: Remove "pcie_hp=nomsi" kernel parameter

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

7570a333d8b0 ("PCI: Add pcie_hp=nomsi to disable MSI/MSI-X for pciehp
driver") added the "pcie_hp=nomsi" kernel parameter to work around this
error on shutdown:

  irq 16: nobody cared (try booting with the "irqpoll" option)
  Pid: 1081, comm: reboot Not tainted 3.2.0 #1
  ...
  Disabling IRQ #16

This happened on an unspecified system (possibly involving the Integrated
Device Technology, Inc. Device 807f bridge) where "an un-wanted interrupt
is generated when PCI driver switches from MSI/MSI-X to INTx while shutting
down the device."

The implication was that the device was buggy, but it is normal for a
device to use INTx after MSI/MSI-X have been disabled.  The only problem
was that the driver was still attached and it wasn't prepared for INTx
interrupts.  Prarit Bhargava fixed this issue with fda78d7a0ead ("PCI/MSI:
Stop disabling MSI/MSI-X in pci_device_shutdown()").

There is no automated way to set this parameter, so it's not very useful
for distributions or end users.  It's really only useful for debugging, and
we have "pci=nomsi" for that purpose.

Revert 7570a333d8b0 to remove the "pcie_hp=nomsi" parameter.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
CC: MUNEDA Takahiro <muneda.takahiro@jp.fujitsu.com>
CC: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
CC: Prarit Bhargava <prarit@redhat.com>
---
 Documentation/admin-guide/kernel-parameters.txt |    4 ----
 drivers/pci/pcie/portdrv.h                      |   12 ------------
 drivers/pci/pcie/portdrv_core.c                 |   20 +++-----------------
 3 files changed, 3 insertions(+), 33 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1d1d53f85ddd..761749562165 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3130,10 +3130,6 @@
 		force	Enable ASPM even on devices that claim not to support it.
 			WARNING: Forcing ASPM on may cause system lockups.
 
-	pcie_hp=	[PCIE] PCI Express Hotplug driver options:
-		nomsi	Do not use MSI for PCI Express Native Hotplug (this
-			makes all PCIe ports use INTx for hotplug services).
-
 	pcie_ports=	[PCIE] PCIe ports handling:
 		auto	Ask the BIOS whether or not to use native PCIe services
 			associated with PCIe ports (PME, hot-plug, AER).  Use
diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index ed84e767085f..86368f9341d7 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -91,18 +91,6 @@ void pcie_port_bus_unregister(void);
 
 struct pci_dev;
 
-#ifdef CONFIG_HOTPLUG_PCI_PCIE
-extern bool pciehp_msi_disabled;
-
-static inline bool pciehp_no_msi(void)
-{
-	return pciehp_msi_disabled;
-}
-
-#else  /* !CONFIG_HOTPLUG_PCI_PCIE */
-static inline bool pciehp_no_msi(void) { return false; }
-#endif /* !CONFIG_HOTPLUG_PCI_PCIE */
-
 #ifdef CONFIG_PCIE_PME
 extern bool pcie_pme_msi_disabled;
 
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 589960fdd8a8..ed24a407518a 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -20,17 +20,6 @@
 #include "../pci.h"
 #include "portdrv.h"
 
-bool pciehp_msi_disabled;
-
-static int __init pciehp_setup(char *str)
-{
-	if (!strncmp(str, "nomsi", 5))
-		pciehp_msi_disabled = true;
-
-	return 1;
-}
-__setup("pcie_hp=", pciehp_setup);
-
 /**
  * release_pcie_device - free PCI Express port service device structure
  * @dev: Port service device to release
@@ -168,16 +157,13 @@ static int pcie_init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
 		irqs[i] = -1;
 
 	/*
-	 * If we support PME or hotplug, but we can't use MSI/MSI-X for
-	 * them, we have to fall back to INTx or other interrupts, e.g., a
-	 * system shared interrupt.
+	 * If we support PME but can't use MSI/MSI-X for it, we have to
+	 * fall back to INTx or other interrupts, e.g., a system shared
+	 * interrupt.
 	 */
 	if ((mask & PCIE_PORT_SERVICE_PME) && pcie_pme_no_msi())
 		goto legacy_irq;
 
-	if ((mask & PCIE_PORT_SERVICE_HP) && pciehp_no_msi())
-		goto legacy_irq;
-
 	/* Try to use MSI-X or MSI if supported */
 	if (pcie_port_enable_irq_vec(dev, irqs, mask) == 0)
 		return 0;

[PATCH v2 11/13] PCI/portdrv: Remove unnecessary "pcie_ports=auto" parameter

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

The "pcie_ports=auto" parameter set pcie_ports_disabled and pcie_ports_auto
to their compiled-in defaults, so specifying the parameter is the same as
not using it at all.

Remove the "pcie_ports=auto" parameter and update the documentation.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 Documentation/admin-guide/kernel-parameters.txt |   15 +++++++--------
 drivers/pci/pcie/portdrv_pci.c                  |    3 ---
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 761749562165..26565794a573 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3130,14 +3130,13 @@
 		force	Enable ASPM even on devices that claim not to support it.
 			WARNING: Forcing ASPM on may cause system lockups.
 
-	pcie_ports=	[PCIE] PCIe ports handling:
-		auto	Ask the BIOS whether or not to use native PCIe services
-			associated with PCIe ports (PME, hot-plug, AER).  Use
-			them only if that is allowed by the BIOS.
-		native	Use native PCIe services associated with PCIe ports
-			unconditionally.
-		compat	Treat PCIe ports as PCI-to-PCI bridges, disable the PCIe
-			ports driver.
+	pcie_ports=	[PCIE] PCIe port services handling:
+		native	Use native PCIe services (PME, AER, DPC, PCIe hotplug)
+			even if the platform doesn't give the OS permission to
+			use them.  This may cause conflicts if the platform
+			also tries to use these services.
+		compat	Disable native PCIe services (PME, AER, DPC, PCIe
+			hotplug).
 
 	pcie_port_pm=	[PCIE] PCIe port power management handling:
 		off	Disable power management of all PCIe ports
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 1997d9f2743e..8b62192342ac 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -37,9 +37,6 @@ static int __init pcie_port_setup(char *str)
 	} else if (!strncmp(str, "native", 6)) {
 		pcie_ports_disabled = false;
 		pcie_ports_auto = false;
-	} else if (!strncmp(str, "auto", 4)) {
-		pcie_ports_disabled = false;
-		pcie_ports_auto = true;
 	}
 
 	return 1;

[PATCH v2 12/13] PCI/portdrv: Encapsulate pcie_ports_auto inside the port driver

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

"pcie_ports_auto" is only used inside the PCIe port driver itself, so
move it from include/linux/pci.h to portdrv.h so it's not visible to the
whole kernel.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pcie/portdrv.h |    2 ++
 include/linux/pci.h        |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index 86368f9341d7..62e28b5afa51 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -12,6 +12,8 @@
 
 #include <linux/compiler.h>
 
+extern bool pcie_ports_auto;
+
 /* Service Type */
 #define PCIE_PORT_SERVICE_PME_SHIFT	0	/* Power Management Event */
 #define PCIE_PORT_SERVICE_PME		(1 << PCIE_PORT_SERVICE_PME_SHIFT)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 40aec7a6fdd9..2c0e5d929fd2 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1449,10 +1449,8 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d,
 
 #ifdef CONFIG_PCIEPORTBUS
 extern bool pcie_ports_disabled;
-extern bool pcie_ports_auto;
 #else
 #define pcie_ports_disabled	true
-#define pcie_ports_auto		false
 #endif
 
 #ifdef CONFIG_PCIEASPM

[PATCH v2 13/13] PCI/portdrv: Rename and reverse sense of pcie_ports_auto

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

The platform may restrict the OS's use of PCIe services, e.g., via the ACPI
_OSC method.  The user may use "pcie_ports=native" to force the port driver
to use PCIe services even if the platform asked us not to.

The "pcie_ports=native" parameter determines the setting of
pcie_ports_auto.  Rename this to pcie_ports_native and reverse the
sense to simplify the code.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pcie/portdrv.h      |    2 +-
 drivers/pci/pcie/portdrv_core.c |   15 ++++-----------
 drivers/pci/pcie/portdrv_pci.c  |   10 +++++-----
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index 62e28b5afa51..3e0058a5500f 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -12,7 +12,7 @@
 
 #include <linux/compiler.h>
 
-extern bool pcie_ports_auto;
+extern bool pcie_ports_native;
 
 /* Service Type */
 #define PCIE_PORT_SERVICE_PME_SHIFT	0	/* Power Management Event */
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index ed24a407518a..a1f838f2646a 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -193,17 +193,10 @@ static int pcie_init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
 static int get_port_device_capability(struct pci_dev *dev)
 {
 	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
-	bool native;
 	int services = 0;
 
-	/*
-	 * If the user specified "pcie_ports=native", use the PCIe services
-	 * regardless of whether the platform has given us permission.  On
-	 * ACPI systems, this means we ignore _OSC.
-	 */
-	native = !pcie_ports_auto;
-
-	if (dev->is_hotplug_bridge && (native || host->use_hotplug)) {
+	if (dev->is_hotplug_bridge &&
+	    (pcie_ports_native || host->use_hotplug)) {
 		services |= PCIE_PORT_SERVICE_HP;
 
 		/*
@@ -215,7 +208,7 @@ static int get_port_device_capability(struct pci_dev *dev)
 	}
 
 	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR) &&
-	    pci_aer_available() && (native || host->use_aer)) {
+	    pci_aer_available() && (pcie_ports_native || host->use_aer)) {
 		services |= PCIE_PORT_SERVICE_AER;
 
 		/*
@@ -231,7 +224,7 @@ static int get_port_device_capability(struct pci_dev *dev)
 	 * those yet.
 	 */
 	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT &&
-	    (native || host->use_pme)) {
+	    (pcie_ports_native || host->use_pme)) {
 		services |= PCIE_PORT_SERVICE_PME;
 
 		/*
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 8b62192342ac..569fd636b3c4 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -25,18 +25,18 @@
 bool pcie_ports_disabled;
 
 /*
- * If this switch is set, ACPI _OSC will be used to determine whether or not to
- * enable PCIe port native services.
+ * If the user specified "pcie_ports=native", use the PCIe services regardless
+ * of whether the platform has given us permission.  On ACPI systems, this
+ * means we ignore _OSC.
  */
-bool pcie_ports_auto = true;
+bool pcie_ports_native;
 
 static int __init pcie_port_setup(char *str)
 {
 	if (!strncmp(str, "compat", 6)) {
 		pcie_ports_disabled = true;
 	} else if (!strncmp(str, "native", 6)) {
-		pcie_ports_disabled = false;
-		pcie_ports_auto = false;
+		pcie_ports_native = true;
 	}
 
 	return 1;

Re: [PATCH v2 00/13] PCI: Simplify PCIe port driver

[Bjorn Helgaas]

On Fri, Mar 09, 2018 at 12:59:49PM -0600, Bjorn Helgaas wrote:
> This is an attempt to move a few things out of the port driver.
> 
> I added these new patches since v1:
> 
>   Merge pcieport_if.h into portdrv.h
>     Merge pcieport_if.h and portdrv.h to reduce clutter
> 
>   Remove unnecessary "pcie_ports=auto" parameter
>     This is the default setting anyway, so specifying the parameter doesn't
>     do anything.
> 
>   Encapsulate pcie_ports_auto inside the port driver
>     "pcie_ports_auto" was declared in linux/pci.h even though nobody
>     outside the port driver used it.
> 
>   Rename and reverse sense of pcie_ports_auto
>     "pcie_ports_auto" is connected with the "pcie_ports=native" parameter,
>     so rename it to match.
> 
> Other changes since v1:
>   - Rebase onto my pci/portdrv branch.
>   - Rename pcie_resume_early() to pcie_pme_root_status_cleanup() as
>     suggested by Rafael.
>   - Add Rafael's Reviewed-by tags.
> 
> v1: https://lkml.kernel.org/r/152040297576.240786.1532465558381209070.stgit@bhelgaas-glaptop.roam.corp.google.com
> 
> ---
> 
> Bjorn Helgaas (13):
>       PCI/portdrv: Merge pcieport_if.h into portdrv.h
>       PCI/PM: Move pcie_clear_root_pme_status() to core
>       PCI/PM: Clear PCIe PME Status bit in core, not PCIe port driver
>       PCI/PM: Clear PCIe PME Status bit for Root Complex Event Collectors
>       PCI/portdrv: Disable port driver in compat mode
>       PCI/portdrv: Remove pcie_port_bus_type link order dependency
>       PCI/portdrv: Remove unused PCIE_PORT_SERVICE_VC
>       PCI/portdrv: Simplify PCIe feature permission checking
>       PCI/portdrv: Remove unnecessary include of <linux/pci-aspm.h>
>       PCI/portdrv: Remove "pcie_hp=nomsi" kernel parameter
>       PCI/portdrv: Remove unnecessary "pcie_ports=auto" parameter
>       PCI/portdrv: Encapsulate pcie_ports_auto inside the port driver
>       PCI/portdrv: Rename and reverse sense of pcie_ports_auto

I applied these with Christoph's acks and Lukas' renaming suggestion to
pci/portdrv for v4.17.