PCI: Fixes and cleanups for native PCIe, SHPC and ACPI hotplug

include/linux/pci_hotplug.h static inline int pci_get_hp_params(struct pci_dev *dev, · line 183

Quoted 7 lines · Mika Westerberg

include/linux/pci_hotplug.h +7 -0

+
+#ifdef CONFIG_HOTPLUG_PCI_PCIE
+#define pciehp_available()	true
+#else
+#define pciehp_available()	false
+#endif
+

More succinctly,

#define pciehp_available() IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE)

(Or use that directly instead of defining an additional macro.)

Currently we request control of native PCIe hotplug unconditionally. That may cause problems because native PCIe hotplug events are handled by pciehp driver and if it is not enabled those events will be lost. Make this bit more robust and request control of native PCIe hotplug only if we are actually prepared to do so (pciehp driver is enabled).

While there rename host->native_hotplug to host->native_pcie_hotplug because we do the same for SHPC hotplug in subsequent patches.

Suggested-by Bjorn Helgaas <[email protected]>

Signed-off-by Mika Westerberg <[email protected]>

---
 drivers/acpi/pci_root.c         | 6 ++++--
 drivers/pci/pcie/portdrv_core.c | 2 +-
 drivers/pci/probe.c             | 2 +-
 include/linux/pci.h             | 2 +-
 include/linux/pci_hotplug.h     | 7 +++++++
 5 files changed, 14 insertions(+), 5 deletions(-)

drivers/acpi/pci_root.c +4 -2

index 0da18bde6a16..97590dff6bd8 100644
@@ -29,6 +29,7 @@
 #include <linux/pci.h>
 #include <linux/pci-acpi.h>
 #include <linux/pci-aspm.h>
+#include <linux/pci_hotplug.h>
 #include <linux/dmar.h>
 #include <linux/acpi.h>
 #include <linux/slab.h>

@@ -472,9 +473,10 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
 	}
 
 	control = OSC_PCI_EXPRESS_CAPABILITY_CONTROL
-		| OSC_PCI_EXPRESS_NATIVE_HP_CONTROL
 		| OSC_PCI_EXPRESS_PME_CONTROL;
 
+	if (pciehp_available())
+		control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
 	if (pci_aer_available()) {
 		if (aer_acpi_firmware_first())
 			dev_info(&device->dev,

@@ -900,7 +902,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 
 	host_bridge = to_pci_host_bridge(bus->bridge);
 	if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL))
-		host_bridge->native_hotplug = 0;
+		host_bridge->native_pcie_hotplug = 0;
 	if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL))
 		host_bridge->native_aer = 0;
 	if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL))

drivers/pci/pcie/portdrv_core.c +1 -1

index c9c0663db282..6cb30aec2452 100644
@@ -199,7 +199,7 @@ static int get_port_device_capability(struct pci_dev *dev)
 	int services = 0;
 
 	if (dev->is_hotplug_bridge &&
-	    (pcie_ports_native || host->native_hotplug)) {
+	    (pcie_ports_native || host->native_pcie_hotplug)) {
 		services |= PCIE_PORT_SERVICE_HP;
 
 		/*

drivers/pci/probe.c +1 -1

index 7c34a2ccb514..a6c3b8d30f8f 100644
@@ -552,7 +552,7 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
 	 * OS from interfering.
 	 */
 	bridge->native_aer = 1;
-	bridge->native_hotplug = 1;
+	bridge->native_pcie_hotplug = 1;
 	bridge->native_pme = 1;
 
 	return bridge;

include/linux/pci.h +1 -1

index 73178a2fcee0..359a197d0310 100644
@@ -471,7 +471,7 @@ struct pci_host_bridge {
 	unsigned int	ignore_reset_delay:1;	/* For entire hierarchy */
 	unsigned int	no_ext_tags:1;		/* No Extended Tags */
 	unsigned int	native_aer:1;		/* OS may use PCIe AER */
-	unsigned int	native_hotplug:1;	/* OS may use PCIe hotplug */
+	unsigned int	native_pcie_hotplug:1;	/* OS may use PCIe hotplug */
 	unsigned int	native_pme:1;		/* OS may use PCIe PME */
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,

include/linux/pci_hotplug.h +0 -0

index 26213024e81b..46fb90b5164b 100644
@@ -174,4 +174,11 @@ static inline int pci_get_hp_params(struct pci_dev *dev,
 }

drivers/pci/pci-acpi.c EXPORT_SYMBOL_GPL(pci_get_hp_params); · line 390

Quoted 3 lines · Mika Westerberg

drivers/pci/pci-acpi.c +2 -0

+	pcie_capability_read_dword(bridge, PCI_EXP_SLTCAP, &slot_cap);
+	if (!(slot_cap & PCI_EXP_SLTCAP_HPC))
 		return false;

Hm, any reason not to use "if (!bridge->is_hotplug_bridge)" ?

Currently pciehp_is_native() returns true for any PCI device in a hierarchy where _OSC says we can use pciehp. This is not correct because there may be bridges without PCI_EXP_SLTCAP_HPC capability and those should be managed by acpiphp instead.

Improve pciehp_is_native() to return true only if the given bridge is actually expected to be handled by pciehp. In any other case return false instead to let acpiphp handle those.

Suggested-by Bjorn Helgaas <[email protected]>

Signed-off-by Mika Westerberg <[email protected]>

---
 drivers/pci/pci-acpi.c      | 28 ++++++++++++++++------------
 drivers/pci/pcie/portdrv.h  |  2 --
 include/linux/pci.h         |  2 ++
 include/linux/pci_hotplug.h |  4 ++--
 4 files changed, 20 insertions(+), 16 deletions(-)

drivers/pci/pci-acpi.c +4 -6

index 1abdbf267c19..d3114f3a7ab8 100644
@@ -370,26 +370,30 @@ EXPORT_SYMBOL_GPL(pci_get_hp_params);
 
 /**
  * pciehp_is_native - Check whether a hotplug port is handled by the OS
- * @pdev: Hotplug port to check
+ * @bridge: Hotplug port to check
  *
- * Walk up from @pdev to the host bridge, obtain its cached _OSC Control Field
- * and return the value of the "PCI Express Native Hot Plug control" bit.
- * On failure to obtain the _OSC Control Field return %false.
+ * Returns true if the given @bridge is handled by the native PCIe hotplug
+ * driver.
  */
-bool pciehp_is_native(struct pci_dev *pdev)
+bool pciehp_is_native(struct pci_dev *bridge)
 {
-	struct acpi_pci_root *root;

Quoted 39 lines · CEST Mika Westerberg

Currently pciehp_is_native() returns true for any PCI device in a hierarchy where _OSC says we can use pciehp. This is not correct because there may be bridges without PCI_EXP_SLTCAP_HPC capability and those should be managed by acpiphp instead.

Improve pciehp_is_native() to return true only if the given bridge is actually expected to be handled by pciehp. In any other case return false instead to let acpiphp handle those.

Suggested-by Bjorn Helgaas <[email protected]>

Signed-off-by Mika Westerberg <[email protected]>

---
 drivers/pci/pci-acpi.c      | 28 ++++++++++++++++------------
 drivers/pci/pcie/portdrv.h  |  2 --
 include/linux/pci.h         |  2 ++
 include/linux/pci_hotplug.h |  4 ++--
 4 files changed, 20 insertions(+), 16 deletions(-)

drivers/pci/pci-acpi.c +4 -6

index 1abdbf267c19..d3114f3a7ab8 100644
@@ -370,26 +370,30 @@ EXPORT_SYMBOL_GPL(pci_get_hp_params);
 
 /**
  * pciehp_is_native - Check whether a hotplug port is handled by the OS
- * @pdev: Hotplug port to check
+ * @bridge: Hotplug port to check
  *
- * Walk up from @pdev to the host bridge, obtain its cached _OSC Control Field
- * and return the value of the "PCI Express Native Hot Plug control" bit.
- * On failure to obtain the _OSC Control Field return %false.
+ * Returns true if the given @bridge is handled by the native PCIe hotplug
+ * driver.
  */
-bool pciehp_is_native(struct pci_dev *pdev)
+bool pciehp_is_native(struct pci_dev *bridge)
 {
-	struct acpi_pci_root *root;

drivers/pci/pci-acpi.c EXPORT_SYMBOL_GPL(pci_get_hp_params); · line 386

Quoted 10 lines

drivers/pci/pci-acpi.c +5 -3

-	acpi_handle handle;
+	const struct pci_host_bridge *host;
+	u32 slot_cap;
 
-	handle = acpi_find_root_bridge_handle(pdev);
-	if (!handle)
+	if (!pciehp_available())
+		return false;
+	if (!bridge)
 		return false;

You could write this as

    if (!pciehp_available() || !bridge)
        return false;

Would be equivalent, but more concise IMO.

Why do we even need to bother checking "bridge" for NULL?

I don't believe in gratuitously checking for NULL because it can hide higher-level bugs, i.e., if this is called with a NULL pointer, that's likely a bug in the caller, and if we silently return "false", we'll never discover the caller's bug.

Quoted 6 lines · Bjorn Helgaas

Why do we even need to bother checking "bridge" for NULL?

I don't believe in gratuitously checking for NULL because it can hide higher-level bugs, i.e., if this is called with a NULL pointer, that's likely a bug in the caller, and if we silently return "false", we'll never discover the caller's bug.

But it also makes caller's life easier because now you don't need to do this:

   if (bus->self && hotplug_is_native(bus->self))

because it is perfectly legal to call that function for host bridge. So you do this instead

   if (hotplug_is_native(bus->self))

and it is not hiding a bug IMHO.

We need to be able coordinate between SHPC and acpiphp in order to determine which driver handles hotplug of a given bridge. Because acpiphp is already bool convert SHPC to be bool as well.

Suggested-by Bjorn Helgaas <[email protected]>

Signed-off-by Mika Westerberg <[email protected]>

---
 drivers/pci/hotplug/Kconfig | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/pci/hotplug/Kconfig +1 -1

index a8f21d051e0c..e9f78eb390d2 100644
@@ -104,14 +104,11 @@ config HOTPLUG_PCI_CPCI_GENERIC
 	  When in doubt, say N.
 
 config HOTPLUG_PCI_SHPC
-	tristate "SHPC PCI Hotplug driver"
+	bool "SHPC PCI Hotplug driver"

Now that the SHPC driver is converted to builtin we can change the way SHPC control is requested to follow more closely what we do for pciehp. We then take advantege of the newly introduced host->native_shpc_hotplug in acpi_get_hp_hw_control_from_firmware() to determine whether OSHP needs to be evaluated.

In addition provide two new functions that can be used to query whether native SHPC driver is expected to handle hotplug of a given bridge or not, following what we do for pciehp.

Suggested-by Bjorn Helgaas <[email protected]>

Signed-off-by Mika Westerberg <[email protected]>

---
 drivers/acpi/pci_root.c           |  4 ++++
 drivers/pci/hotplug/acpi_pcihp.c  | 34 +++++++++++--------------------
 drivers/pci/hotplug/shpchp.h      | 11 ----------
 drivers/pci/hotplug/shpchp_core.c |  2 +-
 drivers/pci/pci-acpi.c            | 23 +++++++++++++++++++++
 drivers/pci/probe.c               |  1 +
 include/linux/pci.h               |  1 +
 include/linux/pci_hotplug.h       | 20 +++++++++++++++++-
 8 files changed, 61 insertions(+), 35 deletions(-)

drivers/acpi/pci_root.c +4 -0

index 97590dff6bd8..42c079f782e3 100644
@@ -477,6 +477,8 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
 
 	if (pciehp_available())
 		control |= OSC_PCI_EXPRESS_NATIVE_HP_CONTROL;
+	if (shpchp_available())
+		control |= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
 	if (pci_aer_available()) {
 		if (aer_acpi_firmware_first())
 			dev_info(&device->dev,

@@ -903,6 +905,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 	host_bridge = to_pci_host_bridge(bus->bridge);
 	if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL))
 		host_bridge->native_pcie_hotplug = 0;
+	if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL))
+		host_bridge->native_shpc_hotplug = 0;
 	if (!(root->osc_control_set & OSC_PCI_EXPRESS_AER_CONTROL))
 		host_bridge->native_aer = 0;
 	if (!(root->osc_control_set & OSC_PCI_EXPRESS_PME_CONTROL))

drivers/pci/hotplug/acpi_pcihp.c +12 -22

index c9816166978e..678c03944bb7 100644
@@ -62,23 +62,18 @@ static acpi_status acpi_run_oshp(acpi_handle handle)
 
 /**
  * acpi_get_hp_hw_control_from_firmware
- * @dev: the pci_dev of the bridge that has a hotplug controller
- * @flags: requested control bits for _OSC
+ * @pdev: the pci_dev of the bridge that has a hotplug controller
  *
  * Attempt to take hotplug control from firmware.
  */
-int acpi_get_hp_hw_control_from_firmware(struct pci_dev *pdev, u32 flags)
+int acpi_get_hp_hw_control_from_firmware(struct pci_dev *pdev)
 {
+	const struct pci_host_bridge *host;
+	const struct acpi_pci_root *root;
 	acpi_status status;
 	acpi_handle chandle, handle;
 	struct acpi_buffer string = { ACPI_ALLOCATE_BUFFER, NULL };
 
-	flags &= OSC_PCI_SHPC_NATIVE_HP_CONTROL;
-	if (!flags) {
-		err("Invalid flags %u specified!\n", flags);
-		return -EINVAL;
-	}
-
 	/*
 	 * Per PCI firmware specification, we should run the ACPI _OSC
 	 * method to get control of hotplug hardware before using it. If

@@ -88,19 +83,14 @@ int acpi_get_hp_hw_control_from_firmware(struct pci_dev *pdev, u32 flags)
 	 * OSHP within the scope of the hotplug controller and its parents,
 	 * up to the host bridge under which this controller exists.
 	 */
-	handle = acpi_find_root_bridge_handle(pdev);
-	if (handle) {
-		acpi_get_name(handle, ACPI_FULL_PATHNAME, &string);
-		dbg("Trying to get hotplug control for %s\n",
-				(char *)string.pointer);
-		status = acpi_pci_osc_control_set(handle, &flags, flags);
-		if (ACPI_SUCCESS(status))
-			goto got_one;
-		if (status == AE_SUPPORT)
-			goto no_control;
-		kfree(string.pointer);
-		string = (struct acpi_buffer){ ACPI_ALLOCATE_BUFFER, NULL };
-	}
+	host = pci_find_host_bridge(pdev->bus);
+	if (host->native_shpc_hotplug)
+		return 0;
+
+	/* If the there is _OSC we should not evaluate OSHP */
+	root = acpi_pci_find_root(ACPI_HANDLE(&host->dev));
+	if (root->osc_support_set)
+		goto no_control;
 
 	handle = ACPI_HANDLE(&pdev->dev);
 	if (!handle) {

drivers/pci/hotplug/shpchp.h +0 -11

index c55730b61c9a..9675ab757323 100644
@@ -173,17 +173,6 @@ static inline const char *slot_name(struct slot *slot)
 	return hotplug_slot_name(slot->hotplug_slot);
 }
 
-#ifdef CONFIG_ACPI
-#include <linux/pci-acpi.h>
-static inline int get_hp_hw_control_from_firmware(struct pci_dev *dev)
-{
-	u32 flags = OSC_PCI_SHPC_NATIVE_HP_CONTROL;
-	return acpi_get_hp_hw_control_from_firmware(dev, flags);
-}
-#else
-#define get_hp_hw_control_from_firmware(dev) (0)
-#endif
-
 struct ctrl_reg {
 	volatile u32 base_offset;
 	volatile u32 slot_avail1;

drivers/pci/hotplug/shpchp_core.c +1 -1

index 1f0f96908b5a..47decc9b3bb3 100644
@@ -277,7 +277,7 @@ static int is_shpc_capable(struct pci_dev *dev)
 		return 1;
 	if (!pci_find_capability(dev, PCI_CAP_ID_SHPC))
 		return 0;
-	if (get_hp_hw_control_from_firmware(dev))
+	if (acpi_get_hp_hw_control_from_firmware(dev))
 		return 0;
 	return 1;
 }

drivers/pci/pci-acpi.c +23 -0

index d3114f3a7ab8..f912916a3a1b 100644
@@ -396,6 +396,29 @@ bool pciehp_is_native(struct pci_dev *bridge)
 	return host->native_pcie_hotplug;
 }
 
+/**
+ * shpchp_is_native - Check whether a hotplug port is handled by the OS
+ * @bridge: Hotplug port to check
+ *
+ * Returns true if the given @bridge is handled by the native SHPC hotplug
+ * driver.
+ */
+bool shpchp_is_native(struct pci_dev *bridge)
+{
+	const struct pci_host_bridge *host;
+
+	if (!shpchp_available())
+		return false;
+	if (!bridge)
+		return false;
+
+	if (!pci_find_capability(bridge, PCI_CAP_ID_SHPC))
+		return false;
+
+	host = pci_find_host_bridge(bridge->bus);
+	return host->native_shpc_hotplug;
+}
+
 /**
  * pci_acpi_wake_bus - Root bus wakeup notification fork function.
  * @context: Device wakeup context.

drivers/pci/probe.c +1 -0

index a6c3b8d30f8f..cd28ab122748 100644
@@ -553,6 +553,7 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
 	 */
 	bridge->native_aer = 1;
 	bridge->native_pcie_hotplug = 1;
+	bridge->native_shpc_hotplug = 1;
 	bridge->native_pme = 1;
 
 	return bridge;

include/linux/pci.h +1 -0

index 2f6689a14e8d..038c6d2d4ff4 100644
@@ -472,6 +472,7 @@ struct pci_host_bridge {
 	unsigned int	no_ext_tags:1;		/* No Extended Tags */
 	unsigned int	native_aer:1;		/* OS may use PCIe AER */
 	unsigned int	native_pcie_hotplug:1;	/* OS may use PCIe hotplug */
+	unsigned int	native_shpc_hotplug:1;	/* OS may use SHPC hotplug */
 	unsigned int	native_pme:1;		/* OS may use PCIe PME */
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,

include/linux/pci_hotplug.h +13 -1

index ee2b1674a601..f7d40d7f8100 100644
@@ -163,7 +163,8 @@ struct hotplug_params {
 #include <linux/acpi.h>
 int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp);
 bool pciehp_is_native(struct pci_dev *bridge);
-int acpi_get_hp_hw_control_from_firmware(struct pci_dev *dev, u32 flags);
+bool shpchp_is_native(struct pci_dev *bridge);
+int acpi_get_hp_hw_control_from_firmware(struct pci_dev *bridge);
 int acpi_pci_check_ejectable(struct pci_bus *pbus, acpi_handle handle);
 int acpi_pci_detect_ejectable(acpi_handle handle);
 #else

@@ -173,12 +174,29 @@ static inline int pci_get_hp_params(struct pci_dev *dev,
 	return -ENODEV;
 }
 static inline bool pciehp_is_native(struct pci_dev *bridge) { return true; }
+static inline bool shpchp_is_native(struct pci_dev *bridge) { return true; }
+
+static inline int acpi_get_hp_hw_control_from_firmware(struct pci_dev *bridge)
+{
+	return 0;
+}
 #endif
 
+static inline bool hotplug_is_native(struct pci_dev *bridge)
+{
+	return pciehp_is_native(bridge) || shpchp_is_native(bridge);
+}
+
 #ifdef CONFIG_HOTPLUG_PCI_PCIE
 #define pciehp_available()	true
 #else

When a system is using native PCIe hotplug for Thunderbolt it will be only present in the system when there is a device connected. This pretty much follows the BIOS assisted hotplug behaviour.

Thunderbolt host router integrated PCIe switch has two additional PCIe downstream bridges that lead to NHI (Thunderbolt host controller) and xHCI (USB 3 host controller) respectively. These downstream bridges are not marked being hotplug capable. Reason for that is to preserve resources. Otherwise the OS would distribute remaining resources between all downstream bridges making these two bridges consume precious resources of the actual hotplug bridges.

Now, because these two bridges are not marked being hotplug capable the OS will not enable hotplug interrupt for them either and will not receive interrupt when devices behind them are hot-added. Solution to this is that the BIOS sends ACPI Notify() to the root port let the OS know it needs to rescan for added and/or removed devices.

Here is how the mechanism is supposed to work when a Thunderbolt endpoint is connected to one of the ports. In case of a standard USB-C device only the xHCI is hot-added otherwise steps are the same.

1. Initially there is only the PCIe root port that is controlled by the pciehp driver

00:1b.0 (Hotplug+) --

2. Then we get native PCIe hotplug interrupt and once it is handled the topology looks as following

00:1b.0 (Hotplug+) -- 01:00.0 --+- 02:00.0 --

                                  +- 02:01.0 (HotPlug+)
                                  \- 02:02.0 --

3. Bridges 02:00.0 and 02:02.0 are not marked as hotplug capable and they don't have anything behind them currently. Bridge 02:01.0 is hotplug capable and used for extending the topology. At this point the required PCIe devices are enabled and ACPI Notify() is sent to the root port. The resulting topology is expected to look like

00:1b.0 (Hotplug+) -- 01:00.0 --+- 02:00.0 -- Thunderbolt host controller

                                  +- 02:01.0 (HotPlug+)
                                  \- 02:02.0 -- xHCI host controller

However, the current ACPI hotplug implementation scans the whole 00:1b.0 hotplug slot and everything behind it regardless whether native PCIe is used or not, and it expects that the BIOS has configured bridge resources upfront. If that's not the case it assigns resources using minimal allocation (everything currently found just barely fit) preventing future extension. In addition to that, if there is another native PCIe hotplug going on we may find the new PCIe switch only partially ready (all links are not fully trained yet) confusing pciehp when it finally starts to enumerate for new devices.

To make this work better with the native PCIe (pciehp) and standard PCI (shpchp) hotplug drivers, we let them handle all slot management and resource allocation for hotplug bridges and restrict ACPI hotplug to non-hotplug bridges.

Signed-off-by Mika Westerberg <[email protected]>

---
 drivers/pci/hotplug/acpiphp_glue.c | 75 +++++++++++++++++++++++-------
 1 file changed, 58 insertions(+), 17 deletions(-)

drivers/pci/hotplug/acpiphp_glue.c +55 -16

index b45b375c0e6c..a8286058f490 100644
@@ -287,11 +287,12 @@ static acpi_status acpiphp_add_context(acpi_handle handle, u32 lvl, void *data,
 	/*
 	 * Expose slots to user space for functions that have _EJ0 or _RMV or
 	 * are located in dock stations.  Do not expose them for devices handled
-	 * by the native PCIe hotplug (PCIeHP), becuase that code is supposed to
-	 * expose slots to user space in those cases.
+	 * by the native PCIe hotplug (PCIeHP) or standard PCI hotplug
+	 * (SHPCHP), because that code is supposed to expose slots to user
+	 * space in those cases.
 	 */
 	if ((acpi_pci_check_ejectable(pbus, handle) || is_dock_device(adev))
-	    && !(pdev && pdev->is_hotplug_bridge && pciehp_is_native(pdev))) {
+	    && !hotplug_is_native(pdev)) {
 		unsigned long long sun;
 		int retval;
 

@@ -430,6 +431,29 @@ static int acpiphp_rescan_slot(struct acpiphp_slot *slot)
 	return pci_scan_slot(slot->bus, PCI_DEVFN(slot->device, 0));
 }
 
+static void acpiphp_native_scan_bridge(struct pci_dev *bridge)
+{
+	struct pci_bus *bus = bridge->subordinate;
+	struct pci_dev *dev;
+	int max;
+
+	if (!bus)
+		return;
+
+	max = bus->busn_res.start;
+	/* Scan already configured non-hotplug bridges */
+	for_each_pci_bridge(dev, bus) {
+		if (!dev->is_hotplug_bridge)
+			max = pci_scan_bridge(bus, dev, max, 0);
+	}
+
+	/* Scan non-hotplug bridges that need to be reconfigured */
+	for_each_pci_bridge(dev, bus) {
+		if (!dev->is_hotplug_bridge)
+			max = pci_scan_bridge(bus, dev, max, 1);
+	}
+}
+
 /**
  * enable_slot - enable, configure a slot
  * @slot: slot to be enabled

@@ -442,25 +466,42 @@ static void enable_slot(struct acpiphp_slot *slot)
 	struct pci_dev *dev;
 	struct pci_bus *bus = slot->bus;
 	struct acpiphp_func *func;
-	int max, pass;
-	LIST_HEAD(add_list);
 
-	acpiphp_rescan_slot(slot);
-	max = acpiphp_max_busnr(bus);
-	for (pass = 0; pass < 2; pass++) {
+	if (hotplug_is_native(bus->self)) {
+		/*
+		 * If native hotplug is used, it will take care of hotplug
+		 * slot management and resource allocation for hotplug
+		 * bridges. However, ACPI hotplug may still be used for
+		 * non-hotplug bridges to bring in additional devices such
+		 * as Thunderbolt host controller.
+		 */
 		for_each_pci_bridge(dev, bus) {
-			if (PCI_SLOT(dev->devfn) != slot->device)
-				continue;
-
-			max = pci_scan_bridge(bus, dev, max, pass);
-			if (pass && dev->subordinate) {
-				check_hotplug_bridge(slot, dev);
-				pcibios_resource_survey_bus(dev->subordinate);
-				__pci_bus_size_bridges(dev->subordinate, &add_list);
+			if (PCI_SLOT(dev->devfn) == slot->device)
+				acpiphp_native_scan_bridge(dev);
+		}
+		pci_assign_unassigned_bridge_resources(bus->self);
+	} else {
+		LIST_HEAD(add_list);
+		int max, pass;
+
+		acpiphp_rescan_slot(slot);
+		max = acpiphp_max_busnr(bus);
+		for (pass = 0; pass < 2; pass++) {
+			for_each_pci_bridge(dev, bus) {
+				if (PCI_SLOT(dev->devfn) != slot->device)
+					continue;
+
+				max = pci_scan_bridge(bus, dev, max, pass);
+				if (pass && dev->subordinate) {
+					check_hotplug_bridge(slot, dev);
+					pcibios_resource_survey_bus(dev->subordinate);
+					__pci_bus_size_bridges(dev->subordinate,

There is a sanity checker in the end of pci_scan_child_bus_extend() that tries to detect badly configured bridges. For example given the below topology:

  +-1b.0-[01-39]----00.0-[02-3a]--+-00.0-[03]----00.0
                                  +-01.0-[04-39]--
                                  \-02.0-[3a]----00.0

The sanity checker notices this and logs following messages:

pci_bus 0000:3a: [bus 3a] partially hidden behind bridge 0000:02 [bus 02-39] pci_bus 0000:3a: [bus 3a] partially hidden behind bridge 0000:01 [bus 01-39]

This is not really helpful to users and the information above is not even correct (0000:02 is a bus not bridge). Make this a bit more understandable by changing the sanity checker to log following message in place of the above two messages:

pci 0000:02:02.0: devices behind bridge are unusable, because [bus 3a] cannot be assigned for them

While there update the comment on top of the sanity checker block to make it clear that it is not only restricted to CardBus.

Suggested-by Bjorn Helgaas <[email protected]>

Signed-off-by Mika Westerberg <[email protected]>

---
 drivers/pci/probe.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

drivers/pci/probe.c +1 -6

index a0c48aa1c42f..10084990fbc3 100644
@@ -1191,20 +1191,16 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
 		(is_cardbus ? "PCI CardBus %04x:%02x" : "PCI Bus %04x:%02x"),
 		pci_domain_nr(bus), child->number);
 
-	/* Has only triggered on CardBus, fixup is in yenta_socket */
+	/* Check that all devices are accessible */
 	while (bus->parent) {
 		if ((child->busn_res.end > bus->busn_res.end) ||
 		    (child->number > bus->busn_res.end) ||
 		    (child->number < bus->number) ||
 		    (child->busn_res.end < bus->number)) {
-			dev_info(&child->dev, "%pR %s hidden behind%s bridge %s %pR\n",
-				&child->busn_res,
-				(bus->number > child->busn_res.end &&
-				 bus->busn_res.end < child->number) ?
-					"wholly" : "partially",

Hi,

When Thunderbolt host router is configured to be in native enumeration mode it is only present in the system if there is something connected to the ports. This pretty much follows how the BIOS assisted mode works.

In native enumeration mode the Thunderbolt host controller (NHI) and USB host controller (xHCI) are not hot-added using native PCIe hotplug but instead they will be hotplugged via BIOS triggered ACPI Notify() to the root port. This is done to preserve resources since NHI and xHCI only need 1 MB of MMIO space and no additional buses. Currently Linux does not support this very well and ends up failing the hotplug in one way or another. More detailed explanation is in changelog of patch [7/12].

This series fixes this issue and in addition includes fixes for few other issues found during testing on a system that has Thunderbolt controller in native PCIe enumeration mode. However, the fixes here are not in any way Thunderbolt specific and should be applicable to other systems as well.

The previous versions of the patch series can be found here:

v5: https://www.spinics.net/lists/linux-pci/msg71339.html
v4: https://www.spinics.net/lists/linux-pci/msg71006.html
v3: https://www.spinics.net/lists/linux-acpi/msg80876.html
v2: https://www.spinics.net/lists/linux-pci/msg69186.html
v1: https://www.spinics.net/lists/linux-acpi/msg80607.html

Changes from v5:

- Dropped patch "PCI: Take bridge window alignment into account when distributing resources"
- Rework pciehp_is_native() to be more stricter
- Make standard PCI hotplug driver (SHPC) builtin
- Rework the way we request OS control of native PCIe hotplug (pciehp) and standard PCI hotplug (SHPC)
- Make acpiphp to avoid bridges where hotplug_is_native() returns true (this is combined from pciehp and SHPC)
- Updated changelog of patch [7/12]
- Use hotplug_bridges + normal_bridges == 1 in patch [9/12] and also remove one indent level from the loop.

Because patch [7/12] looks quite different now I dropped Reviewed-by and stable tag (it now depends on reworked pciehp/SHCP support that most probably is not suitable for stable trees).

I also was not able to test that SHPC really works because I don't have hardware that supports it.

Changes from v4:

- Updated message in patch [8/9] following Rafael's suggestion
- Added Rafael's tag to patches 6, 7, 9.

Changes from v3:

- Added Andy's tag to patches [1-5]
- Improved changelog and subject line of patch [1/9] to match better what it is trying to solve
- Improved comment in patch [1/9]
- Improved changelog of patches [2/9] and [4/9] accordingly to match better why they are needed
- Added cleanup patches [6-9/9]

Changes from v2:

- Added Rafael's tag to patch [1/5]
- Updated changelog of patch [1/5] to include more details about how the problem can be observed from dmesg and lspci output

Changes from v1:

- Drop 'cmax - max ?: 1' and use similar construct than we use in second pass loop in patch [1/5]
- Drop unnecessary parentheses in patch [1/5]
- Added Rafael's tag to patches [2-5/5]

Mika Westerberg (12):
PCI: Take all bridges into account when calculating bus numbers for extension
PCI: pciehp: Clear Presence Detect and Data Link Layer Status Changed on resume
PCI: Request control of native PCIe hotplug only if supported
PCI: Make pciehp_is_native() stricter
PCI: hotplug: Convert SHPC to be builtin only
PCI: Request control of native SHPC hotplug similarly to pciehp ACPI / hotplug / PCI: Do not scan all bridges when native PCIe hotplug is used ACPI / hotplug / PCI: Mark stale PCI devices disconnected
PCI: Move resource distribution for a single bridge outside of the loop
PCI: Document return value of pci_scan_bridge() and pci_scan_bridge_extend()
PCI: Improve "partially hidden behind bridge" log message
ACPI / hotplug / PCI: Drop unnecessary parentheses

I applied the following to pci/hotplug for v4.18:

PCI: pciehp: Clear Presence Detect and Data Link Layer Status Changed on resume
PCI: Request control of native PCIe hotplug only if supported
PCI: Rename host->native_hotplug to host->native_pcie_hotplug
PCI: hotplug: Make pciehp_is_native() stricter
PCI: hotplug: Convert SHPC to be builtin only
PCI: shpchp: Simplify acpi_get_hp_hw_control_from_firmware()
PCI: shpchp: Request control of native SHPC hotplug via _OSC
PCI: Improve "partially hidden behind bridge" log message
ACPI / hotplug / PCI: Drop unnecessary parentheses

I split a couple of these into smaller pieces and removed one NULL pointer check, but otherwise they're straightforward.

These are the simple pieces that probably don't fix anything by themselves. I'm looking at the other parts too and have a couple comments there, but at least we can clear these off the plate.