linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage
@ 2025-07-13 14:31 Lukas Wunner
  2025-07-13 14:31 ` [PATCH v2 1/5] PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports Lukas Wunner
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Lukas Wunner @ 2025-07-13 14:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Laurent Bigonville, Mario Limonciello, Rafael J. Wysocki,
	Mika Westerberg, Alan Borzeszkowski, Gil Fine, Rene Sapiens,
	linux-pci

The original impetus of this series is to fix a runtime PM ref imbalance
on hot-removal of PCIe hotplug ports (patch [1/5]).

That is achieved by adding an is_pciehp flag to struct pci_dev.
The new flag is only set on PCIe Hot-Plug Capable ports, unlike the
existing is_hotplug_bridge flag, which is also set on ACPI slots and
Conventional PCI hotplug bridges (via quirk_hotplug_bridge()).

Patches [2/5] to [4/5] replace is_hotplug_bridge with is_pciehp in a
number of places for clarity and to fix some actual bugs.

Optional patch [5/5] follows a suggestion from Bjorn to set
host->native_pcie_hotplug up front based on pcie_ports_native.
That patch needs an ack from Rafael because it touches ACPI code.
Up to Bjorn whether it is a worthwhile improvement or not.

I'm open to suggestions for a different name than is_pciehp,
e.g. is_pciehp_bridge.

I've reviewed this a couple of times, but would appreciate further
reviewing and testing by others to raise the confidence.  Mika is
out of office until July 28, so I'm cc'ing thunderbolt developers
Alan, Gil and Rene.

I've got an additional patch to replace is_hotplug_bridge with is_pciehp
in quirk_thunderbolt_hotplug_msi() and tb_apple_add_links().  I intend
to submit it to Mika separately if/when this series is accepted.

Link to v1, which consisted only of a (problematic) variant of patch [1/5]:
https://lore.kernel.org/r/86c3bd52bda4552d63ffb48f8a30343167e85271.1750698221.git.lukas@wunner.de/

Lukas Wunner (5):
  PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports
  PCI/portdrv: Use is_pciehp instead of is_hotplug_bridge
  PCI: pciehp: Use is_pciehp instead of is_hotplug_bridge
  PCI: Move is_pciehp check out of pciehp_is_native()
  PCI: Set native_pcie_hotplug up front based on pcie_ports_native

 drivers/acpi/pci_root.c          |  3 ++-
 drivers/pci/hotplug/pciehp_hpc.c |  2 +-
 drivers/pci/pci-acpi.c           | 10 +---------
 drivers/pci/pci.c                | 18 +++++++++++++-----
 drivers/pci/pcie/portdrv.c       |  4 ++--
 drivers/pci/probe.c              |  2 +-
 include/linux/pci.h              |  6 ++++++
 include/linux/pci_hotplug.h      |  3 ++-
 8 files changed, 28 insertions(+), 20 deletions(-)

-- 
2.47.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/5] PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports
  2025-07-13 14:31 [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage Lukas Wunner
@ 2025-07-13 14:31 ` Lukas Wunner
  2025-07-26 20:50   ` Rafael J. Wysocki
  2025-07-13 14:31 ` [PATCH v2 2/5] PCI/portdrv: Use is_pciehp instead of is_hotplug_bridge Lukas Wunner
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Lukas Wunner @ 2025-07-13 14:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Laurent Bigonville, Mario Limonciello, Rafael J. Wysocki,
	Mika Westerberg, Alan Borzeszkowski, Gil Fine, Rene Sapiens,
	linux-pci

pci_bridge_d3_possible() is called from both pcie_portdrv_probe() and
pcie_portdrv_remove() to determine whether runtime power management shall
be enabled (on probe) or disabled (on remove) on a PCIe port.

The underlying assumption is that pci_bridge_d3_possible() always returns
the same value, else a runtime PM reference imbalance would occur.  That
assumption is not given if the PCIe port is inaccessible on remove due to
hot-unplug:  pci_bridge_d3_possible() calls pciehp_is_native(), which
accesses Config Space to determine whether the port is Hot-Plug Capable.
An inaccessible port returns "all ones", which is converted to "all
zeroes" by pcie_capability_read_dword().  Hence the port no longer seems
Hot-Plug Capable on remove even though it was on probe.

The resulting runtime PM ref imbalance causes warning messages such as:

  pcieport 0000:02:04.0: Runtime PM usage count underflow!

Avoid the Config Space access (and thus the runtime PM ref imbalance) by
caching the Hot-Plug Capable bit in struct pci_dev.

The struct already contains an "is_hotplug_bridge" flag, which however is
not only set on Hot-Plug Capable PCIe ports, but also Conventional PCI
Hot-Plug bridges and ACPI slots.  The flag identifies bridges which are
allocated additional MMIO and bus number resources to allow for hierarchy
expansion.

The kernel is somewhat sloppily using "is_hotplug_bridge" in a number of
places to identify Hot-Plug Capable PCIe ports, even though the flag
encompasses other devices.  Subsequent commits replace these occurrences
with the new flag to clearly delineate Hot-Plug Capable PCIe ports from
other kinds of hotplug bridges.

Document the existing "is_hotplug_bridge" and the new "is_pciehp" flag
and document the (non-obvious) requirement that pci_bridge_d3_possible()
always returns the same value across the entire lifetime of a bridge,
including its hot-removal.

Fixes: 5352a44a561d ("PCI: pciehp: Make pciehp_is_native() stricter")
Reported-by: Laurent Bigonville <bigon@bigon.be>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220216
Reported-by: Mario Limonciello <mario.limonciello@amd.com>
Closes: https://lore.kernel.org/r/20250609020223.269407-3-superm1@kernel.org/
Link: https://lore.kernel.org/all/20250620025535.3425049-3-superm1@kernel.org/T/#u
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org # v4.18+
---
 drivers/pci/pci-acpi.c | 4 +---
 drivers/pci/pci.c      | 6 +++++-
 drivers/pci/probe.c    | 2 +-
 include/linux/pci.h    | 6 ++++++
 4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index b78e0e417324..efe478e5073e 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -816,13 +816,11 @@ int pci_acpi_program_hp_params(struct pci_dev *dev)
 bool pciehp_is_native(struct pci_dev *bridge)
 {
 	const struct pci_host_bridge *host;
-	u32 slot_cap;
 
 	if (!IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
 		return false;
 
-	pcie_capability_read_dword(bridge, PCI_EXP_SLTCAP, &slot_cap);
-	if (!(slot_cap & PCI_EXP_SLTCAP_HPC))
+	if (!bridge->is_pciehp)
 		return false;
 
 	if (pcie_ports_native)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e9448d55113b..23d8fe98ddf9 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3030,8 +3030,12 @@ static const struct dmi_system_id bridge_d3_blacklist[] = {
  * pci_bridge_d3_possible - Is it possible to put the bridge into D3
  * @bridge: Bridge to check
  *
- * This function checks if it is possible to move the bridge to D3.
  * Currently we only allow D3 for some PCIe ports and for Thunderbolt.
+ *
+ * Return: Whether it is possible to move the bridge to D3.
+ *
+ * The return value is guaranteed to be constant across the entire lifetime
+ * of the bridge, including its hot-removal.
  */
 bool pci_bridge_d3_possible(struct pci_dev *bridge)
 {
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 4b8693ec9e4c..cf50be63bf5f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1678,7 +1678,7 @@ void set_pcie_hotplug_bridge(struct pci_dev *pdev)
 
 	pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &reg32);
 	if (reg32 & PCI_EXP_SLTCAP_HPC)
-		pdev->is_hotplug_bridge = 1;
+		pdev->is_hotplug_bridge = pdev->is_pciehp = 1;
 }
 
 static void set_pcie_thunderbolt(struct pci_dev *dev)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 05e68f35f392..d56d0dd80afb 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -328,6 +328,11 @@ struct rcec_ea;
  *			determined (e.g., for Root Complex Integrated
  *			Endpoints without the relevant Capability
  *			Registers).
+ * @is_hotplug_bridge:	Hotplug bridge of any kind (e.g. PCIe Hot-Plug Capable,
+ *			Conventional PCI Hot-Plug, ACPI slot).
+ *			Such bridges are allocated additional MMIO and bus
+ *			number resources to allow for hierarchy expansion.
+ * @is_pciehp:		PCIe Hot-Plug Capable bridge.
  */
 struct pci_dev {
 	struct list_head bus_list;	/* Node in per-bus list */
@@ -451,6 +456,7 @@ struct pci_dev {
 	unsigned int	is_physfn:1;
 	unsigned int	is_virtfn:1;
 	unsigned int	is_hotplug_bridge:1;
+	unsigned int	is_pciehp:1;
 	unsigned int	shpc_managed:1;		/* SHPC owned by shpchp */
 	unsigned int	is_thunderbolt:1;	/* Thunderbolt controller */
 	/*
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/5] PCI/portdrv: Use is_pciehp instead of is_hotplug_bridge
  2025-07-13 14:31 [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage Lukas Wunner
  2025-07-13 14:31 ` [PATCH v2 1/5] PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports Lukas Wunner
@ 2025-07-13 14:31 ` Lukas Wunner
  2025-07-13 14:31 ` [PATCH v2 3/5] PCI: pciehp: " Lukas Wunner
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Lukas Wunner @ 2025-07-13 14:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Laurent Bigonville, Mario Limonciello, Rafael J. Wysocki,
	Mika Westerberg, Alan Borzeszkowski, Gil Fine, Rene Sapiens,
	linux-pci

The PCIe port driver erroneously creates a subdevice for hotplug on ACPI
slots which are handled by the ACPI hotplug driver.

Avoid by checking the is_pciehp flag instead of is_hotplug_bridge when
deciding whether to create a subdevice.  The latter encompasses ACPI slots
whereas the former doesn't.

The superfluous subdevice has no real negative impact, it occupies memory
and interrupt resources but otherwise just sits there waiting for
interrupts from the slot that are never signaled.

Fixes: f8415222837b ("PCI: Use cached copy of PCI_EXP_SLTCAP_HPC bit")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org # v4.7+
---
 drivers/pci/pcie/portdrv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index e8318fd5f6ed..d1b68c18444f 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -220,7 +220,7 @@ static int get_port_device_capability(struct pci_dev *dev)
 	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
 	int services = 0;
 
-	if (dev->is_hotplug_bridge &&
+	if (dev->is_pciehp &&
 	    (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
 	     pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) &&
 	    (pcie_ports_native || host->native_pcie_hotplug)) {
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/5] PCI: pciehp: Use is_pciehp instead of is_hotplug_bridge
  2025-07-13 14:31 [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage Lukas Wunner
  2025-07-13 14:31 ` [PATCH v2 1/5] PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports Lukas Wunner
  2025-07-13 14:31 ` [PATCH v2 2/5] PCI/portdrv: Use is_pciehp instead of is_hotplug_bridge Lukas Wunner
@ 2025-07-13 14:31 ` Lukas Wunner
  2025-07-13 14:31 ` [PATCH v2 4/5] PCI: Move is_pciehp check out of pciehp_is_native() Lukas Wunner
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Lukas Wunner @ 2025-07-13 14:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Laurent Bigonville, Mario Limonciello, Rafael J. Wysocki,
	Mika Westerberg, Alan Borzeszkowski, Gil Fine, Rene Sapiens,
	linux-pci

The PCIe hotplug driver calculates the depth of a nested hotplug port by
looking at the is_hotplug_bridge flag.  The depth is used as lockdep class
to tell hotplug ports apart.

The is_hotplug_bridge flag encompasses ACPI slots handled by the ACPI
hotplug driver, hence the calculated depth may be too high.  Avoid by
checking the is_pciehp flag instead.

This glitch likely has no user-visible impact:  ACPI slots typically only
exist at the Root Port level, not in nested hotplug hierarchies.  Also,
CONFIG_LOCKDEP is usually only used by developers.  So this is just for
the sake of correctness.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/hotplug/pciehp_hpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index ebd342bda235..d783da1dbd24 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -995,7 +995,7 @@ static inline int pcie_hotplug_depth(struct pci_dev *dev)
 
 	while (bus->parent) {
 		bus = bus->parent;
-		if (bus->self && bus->self->is_hotplug_bridge)
+		if (bus->self && bus->self->is_pciehp)
 			depth++;
 	}
 
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/5] PCI: Move is_pciehp check out of pciehp_is_native()
  2025-07-13 14:31 [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage Lukas Wunner
                   ` (2 preceding siblings ...)
  2025-07-13 14:31 ` [PATCH v2 3/5] PCI: pciehp: " Lukas Wunner
@ 2025-07-13 14:31 ` Lukas Wunner
  2025-07-13 14:31 ` [PATCH v2 5/5] PCI: Set native_pcie_hotplug up front based on pcie_ports_native Lukas Wunner
  2025-07-22 22:35 ` [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage Bjorn Helgaas
  5 siblings, 0 replies; 14+ messages in thread
From: Lukas Wunner @ 2025-07-13 14:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Laurent Bigonville, Mario Limonciello, Rafael J. Wysocki,
	Mika Westerberg, Alan Borzeszkowski, Gil Fine, Rene Sapiens,
	linux-pci

pci_bridge_d3_possible() seeks to forbid runtime power management on:

* Non Hot-Plug Capable PCIe ports which are nevertheless ACPI slots
  (recognizable as: bridge->is_hotplug_bridge && !bridge->is_pciehp)

* Hot-Plug Capable PCIe ports for which platform firmware has not granted
  PCIe Native Hot-Plug control to the operating system
  (recognizable as: bridge->is_pciehp && !pciehp_is_native(bridge))

Somewhat confusingly, the check for is_hotplug_bridge is in
pci_bridge_d3_possible(), whereas the one for is_pciehp is in
pciehp_is_native().

For clarity, check is_pciehp directly in pci_bridge_d3_possible()
(and in the other caller of pciehp_is_native(), hotplug_is_native()).

Rephrase the code comment preceding these checks to no longer mention
"System Management Mode", which is an x86 term inappropriate in generic
PCI code.  Likewise no longer mention "Thunderbolt on non-Macs", because
there is nothing Thunderbolt-specific about these checks.  It used to be
the case that non-Macs relied on the platform for Thunderbolt tunnel
management and hotplug, but they've since moved to OS-native tunnel
management (as Macs always have), hence the code comment is no longer
accurate.

There is a subsequent check for is_hotplug_bridge further down in
pci_bridge_d3_possible().  Change the check to is_pciehp because any
ports matching "bridge->is_hotplug_bridge && !bridge->is_pciehp" are
already filtered out at the top of the function.

Do the same for another check in acpi_pci_bridge_d3(), which is called
from pci_bridge_d3_possible() via platform_pci_bridge_d3().

No functional change intended.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/pci-acpi.c      |  5 +----
 drivers/pci/pci.c           | 12 ++++++++----
 include/linux/pci_hotplug.h |  3 ++-
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index efe478e5073e..ed7ed66a595b 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -820,9 +820,6 @@ bool pciehp_is_native(struct pci_dev *bridge)
 	if (!IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
 		return false;
 
-	if (!bridge->is_pciehp)
-		return false;
-
 	if (pcie_ports_native)
 		return true;
 
@@ -1000,7 +997,7 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev)
 	struct acpi_device *adev, *rpadev;
 	const union acpi_object *obj;
 
-	if (acpi_pci_disabled || !dev->is_hotplug_bridge)
+	if (acpi_pci_disabled || !dev->is_pciehp)
 		return false;
 
 	adev = ACPI_COMPANION(&dev->dev);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 23d8fe98ddf9..749994dad9dc 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3050,10 +3050,14 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
 			return false;
 
 		/*
-		 * Hotplug ports handled by firmware in System Management Mode
-		 * may not be put into D3 by the OS (Thunderbolt on non-Macs).
+		 * Hotplug ports handled by platform firmware may not be put
+		 * into D3 by the OS, e.g. ACPI slots ...
 		 */
-		if (bridge->is_hotplug_bridge && !pciehp_is_native(bridge))
+		if (bridge->is_hotplug_bridge && !bridge->is_pciehp)
+			return false;
+
+		/* ... or PCIe hotplug ports not handled natively by the OS. */
+		if (bridge->is_pciehp && !pciehp_is_native(bridge))
 			return false;
 
 		if (pci_bridge_d3_force)
@@ -3072,7 +3076,7 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
 		 * by vendors for runtime D3 at least until 2018 because there
 		 * was no OS support.
 		 */
-		if (bridge->is_hotplug_bridge)
+		if (bridge->is_pciehp)
 			return false;
 
 		if (dmi_check_system(bridge_d3_blacklist))
diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h
index ec77ccf1fc4d..ddf79641917f 100644
--- a/include/linux/pci_hotplug.h
+++ b/include/linux/pci_hotplug.h
@@ -104,6 +104,7 @@ static inline bool shpchp_is_native(struct pci_dev *bridge) { return true; }
 
 static inline bool hotplug_is_native(struct pci_dev *bridge)
 {
-	return pciehp_is_native(bridge) || shpchp_is_native(bridge);
+	return (bridge->is_pciehp && pciehp_is_native(bridge)) ||
+	       shpchp_is_native(bridge);
 }
 #endif
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/5] PCI: Set native_pcie_hotplug up front based on pcie_ports_native
  2025-07-13 14:31 [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage Lukas Wunner
                   ` (3 preceding siblings ...)
  2025-07-13 14:31 ` [PATCH v2 4/5] PCI: Move is_pciehp check out of pciehp_is_native() Lukas Wunner
@ 2025-07-13 14:31 ` Lukas Wunner
  2025-07-14 23:00   ` Sathyanarayanan Kuppuswamy
  2025-07-22 22:35 ` [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage Bjorn Helgaas
  5 siblings, 1 reply; 14+ messages in thread
From: Lukas Wunner @ 2025-07-13 14:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Laurent Bigonville, Mario Limonciello, Rafael J. Wysocki,
	Mika Westerberg, Alan Borzeszkowski, Gil Fine, Rene Sapiens,
	linux-pci

Bjorn suggests to "set host->native_pcie_hotplug up front based on
CONFIG_HOTPLUG_PCI_PCIE and pcie_ports_native, and pciehp_is_native()
would collapse to just an accessor for host->native_pcie_hotplug".

Unfortunately only half of this is possible:

The check for pcie_ports_native can indeed be moved out of
pciehp_is_native() and into acpi_pci_root_create().

The check for CONFIG_HOTPLUG_PCI_PCIE however cannot be eliminated:

get_port_device_capability() needs to know whether platform firmware has
granted PCIe Native Hot-Plug control to the operating system.  If it has
and CONFIG_HOTPLUG_PCI_PCIE is disabled, the function disables hotplug
interrupts in case BIOS left them enabled.

If host->native_pcie_hotplug would be set up front based on
CONFIG_HOTPLUG_PCI_PCIE, it would later on be impossible for
get_port_device_capability() to tell whether it can safely disable hotplug
interrupts:  It wouldn't know whether Native Hot-Plug control was granted.

Link: https://lore.kernel.org/r/20250627025607.GA1650254@bhelgaas/
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/acpi/pci_root.c    | 3 ++-
 drivers/pci/pci-acpi.c     | 3 ---
 drivers/pci/pcie/portdrv.c | 2 +-
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 74ade4160314..f3de0dc9c533 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -1028,7 +1028,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 		goto out_release_info;
 
 	host_bridge = to_pci_host_bridge(bus->bridge);
-	if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL))
+	if (!pcie_ports_native &&
+	    !(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;
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index ed7ed66a595b..b513826ea293 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -820,9 +820,6 @@ bool pciehp_is_native(struct pci_dev *bridge)
 	if (!IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
 		return false;
 
-	if (pcie_ports_native)
-		return true;
-
 	host = pci_find_host_bridge(bridge->bus);
 	return host->native_pcie_hotplug;
 }
diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index d1b68c18444f..fa83ebdcfecb 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -223,7 +223,7 @@ static int get_port_device_capability(struct pci_dev *dev)
 	if (dev->is_pciehp &&
 	    (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
 	     pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) &&
-	    (pcie_ports_native || host->native_pcie_hotplug)) {
+	    host->native_pcie_hotplug) {
 		services |= PCIE_PORT_SERVICE_HP;
 
 		/*
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/5] PCI: Set native_pcie_hotplug up front based on pcie_ports_native
  2025-07-13 14:31 ` [PATCH v2 5/5] PCI: Set native_pcie_hotplug up front based on pcie_ports_native Lukas Wunner
@ 2025-07-14 23:00   ` Sathyanarayanan Kuppuswamy
  2025-07-14 23:50     ` Sathyanarayanan Kuppuswamy
  0 siblings, 1 reply; 14+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2025-07-14 23:00 UTC (permalink / raw)
  To: Lukas Wunner, Bjorn Helgaas
  Cc: Laurent Bigonville, Mario Limonciello, Rafael J. Wysocki,
	Mika Westerberg, Alan Borzeszkowski, Gil Fine, Rene Sapiens,
	linux-pci


On 7/13/25 7:31 AM, Lukas Wunner wrote:
> Bjorn suggests to "set host->native_pcie_hotplug up front based on
> CONFIG_HOTPLUG_PCI_PCIE and pcie_ports_native, and pciehp_is_native()
> would collapse to just an accessor for host->native_pcie_hotplug".
>
> Unfortunately only half of this is possible:
>
> The check for pcie_ports_native can indeed be moved out of
> pciehp_is_native() and into acpi_pci_root_create().
>
> The check for CONFIG_HOTPLUG_PCI_PCIE however cannot be eliminated:
>
> get_port_device_capability() needs to know whether platform firmware has
> granted PCIe Native Hot-Plug control to the operating system.  If it has
> and CONFIG_HOTPLUG_PCI_PCIE is disabled, the function disables hotplug
> interrupts in case BIOS left them enabled.
>
> If host->native_pcie_hotplug would be set up front based on
> CONFIG_HOTPLUG_PCI_PCIE, it would later on be impossible for
> get_port_device_capability() to tell whether it can safely disable hotplug
> interrupts:  It wouldn't know whether Native Hot-Plug control was granted.

Since pcie_ports_native is a PCI driver specific override option, I am not
sure it is worth referring to this option in ACPI driver, just to reduce the
number of pcie_ports_native checks from 2 to 1.

> Link: https://lore.kernel.org/r/20250627025607.GA1650254@bhelgaas/
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>   drivers/acpi/pci_root.c    | 3 ++-
>   drivers/pci/pci-acpi.c     | 3 ---
>   drivers/pci/pcie/portdrv.c | 2 +-
>   3 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 74ade4160314..f3de0dc9c533 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -1028,7 +1028,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>   		goto out_release_info;
>   
>   	host_bridge = to_pci_host_bridge(bus->bridge);
> -	if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL))
> +	if (!pcie_ports_native &&
> +	    !(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;
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index ed7ed66a595b..b513826ea293 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -820,9 +820,6 @@ bool pciehp_is_native(struct pci_dev *bridge)
>   	if (!IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
>   		return false;
>   
> -	if (pcie_ports_native)
> -		return true;
> -
>   	host = pci_find_host_bridge(bridge->bus);
>   	return host->native_pcie_hotplug;
>   }
> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index d1b68c18444f..fa83ebdcfecb 100644
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -223,7 +223,7 @@ static int get_port_device_capability(struct pci_dev *dev)
>   	if (dev->is_pciehp &&
>   	    (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
>   	     pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) &&
> -	    (pcie_ports_native || host->native_pcie_hotplug)) {
> +	    host->native_pcie_hotplug) {
>   		services |= PCIE_PORT_SERVICE_HP;
>   
>   		/*

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/5] PCI: Set native_pcie_hotplug up front based on pcie_ports_native
  2025-07-14 23:00   ` Sathyanarayanan Kuppuswamy
@ 2025-07-14 23:50     ` Sathyanarayanan Kuppuswamy
  2025-07-22 22:33       ` Bjorn Helgaas
  0 siblings, 1 reply; 14+ messages in thread
From: Sathyanarayanan Kuppuswamy @ 2025-07-14 23:50 UTC (permalink / raw)
  To: Lukas Wunner, Bjorn Helgaas
  Cc: Laurent Bigonville, Mario Limonciello, Rafael J. Wysocki,
	Mika Westerberg, Alan Borzeszkowski, Gil Fine, Rene Sapiens,
	linux-pci


On 7/14/25 4:00 PM, Sathyanarayanan Kuppuswamy wrote:
>
> On 7/13/25 7:31 AM, Lukas Wunner wrote:
>> Bjorn suggests to "set host->native_pcie_hotplug up front based on
>> CONFIG_HOTPLUG_PCI_PCIE and pcie_ports_native, and pciehp_is_native()
>> would collapse to just an accessor for host->native_pcie_hotplug".
>>
>> Unfortunately only half of this is possible:
>>
>> The check for pcie_ports_native can indeed be moved out of
>> pciehp_is_native() and into acpi_pci_root_create().
>>
>> The check for CONFIG_HOTPLUG_PCI_PCIE however cannot be eliminated:
>>
>> get_port_device_capability() needs to know whether platform firmware has
>> granted PCIe Native Hot-Plug control to the operating system. If it has
>> and CONFIG_HOTPLUG_PCI_PCIE is disabled, the function disables hotplug
>> interrupts in case BIOS left them enabled.
>>
>> If host->native_pcie_hotplug would be set up front based on
>> CONFIG_HOTPLUG_PCI_PCIE, it would later on be impossible for
>> get_port_device_capability() to tell whether it can safely disable hotplug
>> interrupts:  It wouldn't know whether Native Hot-Plug control was granted.
>
> Since pcie_ports_native is a PCI driver specific override option, I am not
> sure it is worth referring to this option in ACPI driver, just to reduce the
> number of pcie_ports_native checks from 2 to 1.
>

Never mind. It looks like the goal is to simplify the pciehp_is_native() call.
Since you cannot get rid of CONFIG_HOTPLUG_PCI_PCIE check, do you
still want to do this simplification?

>
>> Link: https://lore.kernel.org/r/20250627025607.GA1650254@bhelgaas/
>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
>> Signed-off-by: Lukas Wunner <lukas@wunner.de>
>> ---
>>   drivers/acpi/pci_root.c    | 3 ++-
>>   drivers/pci/pci-acpi.c     | 3 ---
>>   drivers/pci/pcie/portdrv.c | 2 +-
>>   3 files changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index 74ade4160314..f3de0dc9c533 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -1028,7 +1028,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>>           goto out_release_info;
>>         host_bridge = to_pci_host_bridge(bus->bridge);
>> -    if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL))
>> +    if (!pcie_ports_native &&
>> +        !(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;
>> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
>> index ed7ed66a595b..b513826ea293 100644
>> --- a/drivers/pci/pci-acpi.c
>> +++ b/drivers/pci/pci-acpi.c
>> @@ -820,9 +820,6 @@ bool pciehp_is_native(struct pci_dev *bridge)
>>       if (!IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
>>           return false;
>>   -    if (pcie_ports_native)
>> -        return true;
>> -
>>       host = pci_find_host_bridge(bridge->bus);
>>       return host->native_pcie_hotplug;
>>   }
>> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
>> index d1b68c18444f..fa83ebdcfecb 100644
>> --- a/drivers/pci/pcie/portdrv.c
>> +++ b/drivers/pci/pcie/portdrv.c
>> @@ -223,7 +223,7 @@ static int get_port_device_capability(struct pci_dev *dev)
>>       if (dev->is_pciehp &&
>>           (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
>>            pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) &&
>> -        (pcie_ports_native || host->native_pcie_hotplug)) {
>> +        host->native_pcie_hotplug) {
>>           services |= PCIE_PORT_SERVICE_HP;
>>             /*
>
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/5] PCI: Set native_pcie_hotplug up front based on pcie_ports_native
  2025-07-14 23:50     ` Sathyanarayanan Kuppuswamy
@ 2025-07-22 22:33       ` Bjorn Helgaas
  0 siblings, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2025-07-22 22:33 UTC (permalink / raw)
  To: Sathyanarayanan Kuppuswamy
  Cc: Lukas Wunner, Laurent Bigonville, Mario Limonciello,
	Rafael J. Wysocki, Mika Westerberg, Alan Borzeszkowski, Gil Fine,
	Rene Sapiens, linux-pci

On Mon, Jul 14, 2025 at 04:50:45PM -0700, Sathyanarayanan Kuppuswamy wrote:
> 
> On 7/14/25 4:00 PM, Sathyanarayanan Kuppuswamy wrote:
> > 
> > On 7/13/25 7:31 AM, Lukas Wunner wrote:
> > > Bjorn suggests to "set host->native_pcie_hotplug up front based on
> > > CONFIG_HOTPLUG_PCI_PCIE and pcie_ports_native, and pciehp_is_native()
> > > would collapse to just an accessor for host->native_pcie_hotplug".
> > > 
> > > Unfortunately only half of this is possible:
> > > 
> > > The check for pcie_ports_native can indeed be moved out of
> > > pciehp_is_native() and into acpi_pci_root_create().
> > > 
> > > The check for CONFIG_HOTPLUG_PCI_PCIE however cannot be eliminated:
> > > 
> > > get_port_device_capability() needs to know whether platform firmware has
> > > granted PCIe Native Hot-Plug control to the operating system. If it has
> > > and CONFIG_HOTPLUG_PCI_PCIE is disabled, the function disables hotplug
> > > interrupts in case BIOS left them enabled.
> > > 
> > > If host->native_pcie_hotplug would be set up front based on
> > > CONFIG_HOTPLUG_PCI_PCIE, it would later on be impossible for
> > > get_port_device_capability() to tell whether it can safely disable hotplug
> > > interrupts:  It wouldn't know whether Native Hot-Plug control was granted.
> > 
> > Since pcie_ports_native is a PCI driver specific override option, I am not
> > sure it is worth referring to this option in ACPI driver, just to reduce the
> > number of pcie_ports_native checks from 2 to 1.
> 
> Never mind. It looks like the goal is to simplify the
> pciehp_is_native() call.

I would actually like to see more uses of pcie_ports_native removed.
It's basically an override of the OS/platform negotiation for control
of features, so I think it would make sense to test it somewhere in
the negotiate_os_control() rats nest or in acpi_pci_root_create().

> > > Link: https://lore.kernel.org/r/20250627025607.GA1650254@bhelgaas/
> > > Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > > ---
> > >   drivers/acpi/pci_root.c    | 3 ++-
> > >   drivers/pci/pci-acpi.c     | 3 ---
> > >   drivers/pci/pcie/portdrv.c | 2 +-
> > >   3 files changed, 3 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> > > index 74ade4160314..f3de0dc9c533 100644
> > > --- a/drivers/acpi/pci_root.c
> > > +++ b/drivers/acpi/pci_root.c
> > > @@ -1028,7 +1028,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
> > >           goto out_release_info;
> > >         host_bridge = to_pci_host_bridge(bus->bridge);
> > > -    if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL))
> > > +    if (!pcie_ports_native &&
> > > +        !(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;
> > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> > > index ed7ed66a595b..b513826ea293 100644
> > > --- a/drivers/pci/pci-acpi.c
> > > +++ b/drivers/pci/pci-acpi.c
> > > @@ -820,9 +820,6 @@ bool pciehp_is_native(struct pci_dev *bridge)
> > >       if (!IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
> > >           return false;
> > >   -    if (pcie_ports_native)
> > > -        return true;
> > > -
> > >       host = pci_find_host_bridge(bridge->bus);
> > >       return host->native_pcie_hotplug;
> > >   }
> > > diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> > > index d1b68c18444f..fa83ebdcfecb 100644
> > > --- a/drivers/pci/pcie/portdrv.c
> > > +++ b/drivers/pci/pcie/portdrv.c
> > > @@ -223,7 +223,7 @@ static int get_port_device_capability(struct pci_dev *dev)
> > >       if (dev->is_pciehp &&
> > >           (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
> > >            pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) &&
> > > -        (pcie_ports_native || host->native_pcie_hotplug)) {
> > > +        host->native_pcie_hotplug) {
> > >           services |= PCIE_PORT_SERVICE_HP;
> > >             /*
> > 
> -- 
> Sathyanarayanan Kuppuswamy
> Linux Kernel Developer
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage
  2025-07-13 14:31 [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage Lukas Wunner
                   ` (4 preceding siblings ...)
  2025-07-13 14:31 ` [PATCH v2 5/5] PCI: Set native_pcie_hotplug up front based on pcie_ports_native Lukas Wunner
@ 2025-07-22 22:35 ` Bjorn Helgaas
  2025-07-25  8:19   ` Lukas Wunner
  5 siblings, 1 reply; 14+ messages in thread
From: Bjorn Helgaas @ 2025-07-22 22:35 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Laurent Bigonville, Mario Limonciello, Rafael J. Wysocki,
	Mika Westerberg, Alan Borzeszkowski, Gil Fine, Rene Sapiens,
	linux-pci

On Sun, Jul 13, 2025 at 04:31:00PM +0200, Lukas Wunner wrote:
> The original impetus of this series is to fix a runtime PM ref imbalance
> on hot-removal of PCIe hotplug ports (patch [1/5]).
> 
> That is achieved by adding an is_pciehp flag to struct pci_dev.
> The new flag is only set on PCIe Hot-Plug Capable ports, unlike the
> existing is_hotplug_bridge flag, which is also set on ACPI slots and
> Conventional PCI hotplug bridges (via quirk_hotplug_bridge()).
> 
> Patches [2/5] to [4/5] replace is_hotplug_bridge with is_pciehp in a
> number of places for clarity and to fix some actual bugs.
> 
> Optional patch [5/5] follows a suggestion from Bjorn to set
> host->native_pcie_hotplug up front based on pcie_ports_native.
> That patch needs an ack from Rafael because it touches ACPI code.
> Up to Bjorn whether it is a worthwhile improvement or not.
> 
> I'm open to suggestions for a different name than is_pciehp,
> e.g. is_pciehp_bridge.
> 
> I've reviewed this a couple of times, but would appreciate further
> reviewing and testing by others to raise the confidence.  Mika is
> out of office until July 28, so I'm cc'ing thunderbolt developers
> Alan, Gil and Rene.
> 
> I've got an additional patch to replace is_hotplug_bridge with is_pciehp
> in quirk_thunderbolt_hotplug_msi() and tb_apple_add_links().  I intend
> to submit it to Mika separately if/when this series is accepted.
> 
> Link to v1, which consisted only of a (problematic) variant of patch [1/5]:
> https://lore.kernel.org/r/86c3bd52bda4552d63ffb48f8a30343167e85271.1750698221.git.lukas@wunner.de/
> 
> Lukas Wunner (5):
>   PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports
>   PCI/portdrv: Use is_pciehp instead of is_hotplug_bridge
>   PCI: pciehp: Use is_pciehp instead of is_hotplug_bridge
>   PCI: Move is_pciehp check out of pciehp_is_native()
>   PCI: Set native_pcie_hotplug up front based on pcie_ports_native
> 
>  drivers/acpi/pci_root.c          |  3 ++-
>  drivers/pci/hotplug/pciehp_hpc.c |  2 +-
>  drivers/pci/pci-acpi.c           | 10 +---------
>  drivers/pci/pci.c                | 18 +++++++++++++-----
>  drivers/pci/pcie/portdrv.c       |  4 ++--
>  drivers/pci/probe.c              |  2 +-
>  include/linux/pci.h              |  6 ++++++
>  include/linux/pci_hotplug.h      |  3 ++-
>  8 files changed, 28 insertions(+), 20 deletions(-)

Thanks!  I applied these to pci/hotplug, hoping to put them in v6.17.

I moved the previous pci/hotplug branch to pci/hotplug-pnv_php.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage
  2025-07-22 22:35 ` [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage Bjorn Helgaas
@ 2025-07-25  8:19   ` Lukas Wunner
  2025-07-25 19:33     ` Bjorn Helgaas
  0 siblings, 1 reply; 14+ messages in thread
From: Lukas Wunner @ 2025-07-25  8:19 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Laurent Bigonville, Mario Limonciello, Rafael J. Wysocki,
	Mika Westerberg, Alan Borzeszkowski, Gil Fine, Rene Sapiens,
	linux-pci

On Tue, Jul 22, 2025 at 05:35:22PM -0500, Bjorn Helgaas wrote:
> On Sun, Jul 13, 2025 at 04:31:00PM +0200, Lukas Wunner wrote:
> >   PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports
> >   PCI/portdrv: Use is_pciehp instead of is_hotplug_bridge
> >   PCI: pciehp: Use is_pciehp instead of is_hotplug_bridge
> >   PCI: Move is_pciehp check out of pciehp_is_native()
> >   PCI: Set native_pcie_hotplug up front based on pcie_ports_native
> 
> Thanks!  I applied these to pci/hotplug, hoping to put them in v6.17.
> 
> I moved the previous pci/hotplug branch to pci/hotplug-pnv_php.

Just a heads-up in case it's unintentional, pci/next as of 10 hours ago
does not include the following pci.git branches:

- hotplug

- hotplug-pnv_php
  expected to be applied through powerpc tree per:
  https://lore.kernel.org/r/20250723113912.GA2880767@bhelgaas/

- controller/dwc-stm32
  awaiting a build fix per:
  https://lore.kernel.org/r/20250716192418.GA2550861@bhelgaas/

- trace
  deferred to v6.18 per:
  https://lore.kernel.org/r/20250724222759.GA3065374@bhelgaas/

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage
  2025-07-25  8:19   ` Lukas Wunner
@ 2025-07-25 19:33     ` Bjorn Helgaas
  2025-07-26 20:52       ` Rafael J. Wysocki
  0 siblings, 1 reply; 14+ messages in thread
From: Bjorn Helgaas @ 2025-07-25 19:33 UTC (permalink / raw)
  To: Lukas Wunner, Rafael J. Wysocki
  Cc: Laurent Bigonville, Mario Limonciello, Rafael J. Wysocki,
	Mika Westerberg, Alan Borzeszkowski, Gil Fine, Rene Sapiens,
	linux-pci

[-> to: Rafael for possible ack]

On Fri, Jul 25, 2025 at 10:19:02AM +0200, Lukas Wunner wrote:
> On Tue, Jul 22, 2025 at 05:35:22PM -0500, Bjorn Helgaas wrote:
> > On Sun, Jul 13, 2025 at 04:31:00PM +0200, Lukas Wunner wrote:
> > >   PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports
> > >   PCI/portdrv: Use is_pciehp instead of is_hotplug_bridge
> > >   PCI: pciehp: Use is_pciehp instead of is_hotplug_bridge
> > >   PCI: Move is_pciehp check out of pciehp_is_native()
> > >   PCI: Set native_pcie_hotplug up front based on pcie_ports_native
> > 
> > Thanks!  I applied these to pci/hotplug, hoping to put them in v6.17.
> > 
> > I moved the previous pci/hotplug branch to pci/hotplug-pnv_php.
> 
> Just a heads-up in case it's unintentional, pci/next as of 10 hours ago
> does not include the following pci.git branches:
> 
> - hotplug

Thanks for the reminder, I did miss it.  Rafael, would be glad for
your ack on this one if you have time:

  PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports

Re this one:

  PCI: Set native_pcie_hotplug up front based on pcie_ports_native

I do see your concern about leaking pcie_ports_native into ACPI code.
Currently all uses are in drivers/pci/, so it would be nice to move
the declaration to drivers/pci/pci.h.

Here's the call path:

  acpi_pci_root_add
    negotiate_os_control                   # _OSC platform negotiation
    pci_acpi_scan_root
      acpi_pci_root_create
        pci_create_root_bus
          pci_init_host_bridge
            host_bridge->native_aer = 1    # Linux owns by default
        if (!OSC_PCI_EXPRESS_AER_CONTROL)
          host_bridge->native_aer = 0      # override, platform owns
        pci_scan_child_bus
          pci_scan_child_bus_extend

We could add some kind of "bridge->force_linux_ownership" bit in
struct pci_host_bridge (and we should probably mention this in dmesg
and maybe taint the kernel (is that still a thing?) because it's
definitely a potential conflict with platform firmware.

Then acpi_pci_root_create() could ignore the _OSC results if set.

Maybe I'll just defer this patch for now since it's a cleanup that
doesn't fix anything?  We can come back later and see if it makes
sense to extend this idea to bits other than native_pcie_hotplug.

Bjorn

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/5] PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports
  2025-07-13 14:31 ` [PATCH v2 1/5] PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports Lukas Wunner
@ 2025-07-26 20:50   ` Rafael J. Wysocki
  0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2025-07-26 20:50 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Bjorn Helgaas, Laurent Bigonville, Mario Limonciello,
	Rafael J. Wysocki, Mika Westerberg, Alan Borzeszkowski, Gil Fine,
	Rene Sapiens, linux-pci

On Sun, Jul 13, 2025 at 4:47 PM Lukas Wunner <lukas@wunner.de> wrote:
>
> pci_bridge_d3_possible() is called from both pcie_portdrv_probe() and
> pcie_portdrv_remove() to determine whether runtime power management shall
> be enabled (on probe) or disabled (on remove) on a PCIe port.
>
> The underlying assumption is that pci_bridge_d3_possible() always returns
> the same value, else a runtime PM reference imbalance would occur.  That
> assumption is not given if the PCIe port is inaccessible on remove due to
> hot-unplug:  pci_bridge_d3_possible() calls pciehp_is_native(), which
> accesses Config Space to determine whether the port is Hot-Plug Capable.
> An inaccessible port returns "all ones", which is converted to "all
> zeroes" by pcie_capability_read_dword().  Hence the port no longer seems
> Hot-Plug Capable on remove even though it was on probe.
>
> The resulting runtime PM ref imbalance causes warning messages such as:
>
>   pcieport 0000:02:04.0: Runtime PM usage count underflow!
>
> Avoid the Config Space access (and thus the runtime PM ref imbalance) by
> caching the Hot-Plug Capable bit in struct pci_dev.
>
> The struct already contains an "is_hotplug_bridge" flag, which however is
> not only set on Hot-Plug Capable PCIe ports, but also Conventional PCI
> Hot-Plug bridges and ACPI slots.  The flag identifies bridges which are
> allocated additional MMIO and bus number resources to allow for hierarchy
> expansion.
>
> The kernel is somewhat sloppily using "is_hotplug_bridge" in a number of
> places to identify Hot-Plug Capable PCIe ports, even though the flag
> encompasses other devices.  Subsequent commits replace these occurrences
> with the new flag to clearly delineate Hot-Plug Capable PCIe ports from
> other kinds of hotplug bridges.
>
> Document the existing "is_hotplug_bridge" and the new "is_pciehp" flag
> and document the (non-obvious) requirement that pci_bridge_d3_possible()
> always returns the same value across the entire lifetime of a bridge,
> including its hot-removal.
>
> Fixes: 5352a44a561d ("PCI: pciehp: Make pciehp_is_native() stricter")
> Reported-by: Laurent Bigonville <bigon@bigon.be>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220216
> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
> Closes: https://lore.kernel.org/r/20250609020223.269407-3-superm1@kernel.org/
> Link: https://lore.kernel.org/all/20250620025535.3425049-3-superm1@kernel.org/T/#u
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: stable@vger.kernel.org # v4.18+

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

> ---
>  drivers/pci/pci-acpi.c | 4 +---
>  drivers/pci/pci.c      | 6 +++++-
>  drivers/pci/probe.c    | 2 +-
>  include/linux/pci.h    | 6 ++++++
>  4 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index b78e0e417324..efe478e5073e 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -816,13 +816,11 @@ int pci_acpi_program_hp_params(struct pci_dev *dev)
>  bool pciehp_is_native(struct pci_dev *bridge)
>  {
>         const struct pci_host_bridge *host;
> -       u32 slot_cap;
>
>         if (!IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
>                 return false;
>
> -       pcie_capability_read_dword(bridge, PCI_EXP_SLTCAP, &slot_cap);
> -       if (!(slot_cap & PCI_EXP_SLTCAP_HPC))
> +       if (!bridge->is_pciehp)
>                 return false;
>
>         if (pcie_ports_native)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e9448d55113b..23d8fe98ddf9 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3030,8 +3030,12 @@ static const struct dmi_system_id bridge_d3_blacklist[] = {
>   * pci_bridge_d3_possible - Is it possible to put the bridge into D3
>   * @bridge: Bridge to check
>   *
> - * This function checks if it is possible to move the bridge to D3.
>   * Currently we only allow D3 for some PCIe ports and for Thunderbolt.
> + *
> + * Return: Whether it is possible to move the bridge to D3.
> + *
> + * The return value is guaranteed to be constant across the entire lifetime
> + * of the bridge, including its hot-removal.
>   */
>  bool pci_bridge_d3_possible(struct pci_dev *bridge)
>  {
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4b8693ec9e4c..cf50be63bf5f 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1678,7 +1678,7 @@ void set_pcie_hotplug_bridge(struct pci_dev *pdev)
>
>         pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &reg32);
>         if (reg32 & PCI_EXP_SLTCAP_HPC)
> -               pdev->is_hotplug_bridge = 1;
> +               pdev->is_hotplug_bridge = pdev->is_pciehp = 1;
>  }
>
>  static void set_pcie_thunderbolt(struct pci_dev *dev)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 05e68f35f392..d56d0dd80afb 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -328,6 +328,11 @@ struct rcec_ea;
>   *                     determined (e.g., for Root Complex Integrated
>   *                     Endpoints without the relevant Capability
>   *                     Registers).
> + * @is_hotplug_bridge: Hotplug bridge of any kind (e.g. PCIe Hot-Plug Capable,
> + *                     Conventional PCI Hot-Plug, ACPI slot).
> + *                     Such bridges are allocated additional MMIO and bus
> + *                     number resources to allow for hierarchy expansion.
> + * @is_pciehp:         PCIe Hot-Plug Capable bridge.
>   */
>  struct pci_dev {
>         struct list_head bus_list;      /* Node in per-bus list */
> @@ -451,6 +456,7 @@ struct pci_dev {
>         unsigned int    is_physfn:1;
>         unsigned int    is_virtfn:1;
>         unsigned int    is_hotplug_bridge:1;
> +       unsigned int    is_pciehp:1;
>         unsigned int    shpc_managed:1;         /* SHPC owned by shpchp */
>         unsigned int    is_thunderbolt:1;       /* Thunderbolt controller */
>         /*
> --
> 2.47.2
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage
  2025-07-25 19:33     ` Bjorn Helgaas
@ 2025-07-26 20:52       ` Rafael J. Wysocki
  0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2025-07-26 20:52 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lukas Wunner, Rafael J. Wysocki, Laurent Bigonville,
	Mario Limonciello, Mika Westerberg, Alan Borzeszkowski, Gil Fine,
	Rene Sapiens, linux-pci

On Fri, Jul 25, 2025 at 9:33 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> [-> to: Rafael for possible ack]
>
> On Fri, Jul 25, 2025 at 10:19:02AM +0200, Lukas Wunner wrote:
> > On Tue, Jul 22, 2025 at 05:35:22PM -0500, Bjorn Helgaas wrote:
> > > On Sun, Jul 13, 2025 at 04:31:00PM +0200, Lukas Wunner wrote:
> > > >   PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports
> > > >   PCI/portdrv: Use is_pciehp instead of is_hotplug_bridge
> > > >   PCI: pciehp: Use is_pciehp instead of is_hotplug_bridge
> > > >   PCI: Move is_pciehp check out of pciehp_is_native()
> > > >   PCI: Set native_pcie_hotplug up front based on pcie_ports_native
> > >
> > > Thanks!  I applied these to pci/hotplug, hoping to put them in v6.17.
> > >
> > > I moved the previous pci/hotplug branch to pci/hotplug-pnv_php.
> >
> > Just a heads-up in case it's unintentional, pci/next as of 10 hours ago
> > does not include the following pci.git branches:
> >
> > - hotplug
>
> Thanks for the reminder, I did miss it.  Rafael, would be glad for
> your ack on this one if you have time:
>
>   PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports

ACKed, thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-07-26 20:52 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-13 14:31 [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage Lukas Wunner
2025-07-13 14:31 ` [PATCH v2 1/5] PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports Lukas Wunner
2025-07-26 20:50   ` Rafael J. Wysocki
2025-07-13 14:31 ` [PATCH v2 2/5] PCI/portdrv: Use is_pciehp instead of is_hotplug_bridge Lukas Wunner
2025-07-13 14:31 ` [PATCH v2 3/5] PCI: pciehp: " Lukas Wunner
2025-07-13 14:31 ` [PATCH v2 4/5] PCI: Move is_pciehp check out of pciehp_is_native() Lukas Wunner
2025-07-13 14:31 ` [PATCH v2 5/5] PCI: Set native_pcie_hotplug up front based on pcie_ports_native Lukas Wunner
2025-07-14 23:00   ` Sathyanarayanan Kuppuswamy
2025-07-14 23:50     ` Sathyanarayanan Kuppuswamy
2025-07-22 22:33       ` Bjorn Helgaas
2025-07-22 22:35 ` [PATCH v2 0/5] PCI: Clean up and fix is_hotplug_bridge usage Bjorn Helgaas
2025-07-25  8:19   ` Lukas Wunner
2025-07-25 19:33     ` Bjorn Helgaas
2025-07-26 20:52       ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).