[PATCH v14 0/4] Simplify PCIe native ownership

[PATCH v14 0/4] Simplify PCIe native ownership

[Kuppuswamy Sathyanarayanan]

This revives Bjorn's v12 from November 2020, rebased onto v7.3-rc3.

  https://lore.kernel.org/all/20201126011816.711106-1-helgaas@kernel.org/

The goal is unchanged.  Decide who owns each PCIe port service in one
place, when we interpret the _OSC results in acpi_pci_root_create(), so
that everywhere else only has to look at host_bridge->native_X.  For AER
specifically, host_bridge->native_aer becomes the single answer to the
question "may Linux touch the AER Capability?".  Today callers each have
to remember to also test pcie_ports_native and pci_aer_available().

I posted v11.  Bjorn took it over at v12, split the _OSC changes in two
and deferred the "pcie_ports=dpc-native" work.  The v12 review comments
were agreed but never respun, and the series stalled.  v13 was that
respin.

  https://lore.kernel.org/r/cover.1603766889.git.sathyanarayanan.kuppuswamy@linux.intel.com [v11]

Bjorn suggested reviving it in response to Guixin Liu's report that
"pcie_ports=native" no longer enables DPC.  The DPC service binds on
host_bridge->native_aer, and that flag did not reflect the command line,
so DPC stayed off when firmware retained AER control.  Patch 3 fixes it
by making the flag reflect it.

  https://lore.kernel.org/linux-pci/20260901064554.2178688-1-kanie@linux.alibaba.com/

Patch 3 has a side effect worth calling out.  drivers/cxl/core/ras.c did
not exist in 2020 and tests host_bridge->native_aer with no
pcie_ports_native fallback, so it has been quietly ignoring
"pcie_ports=native".  Centralizing the check fixes that.

Two things are left for later, to keep this series a cleanup.

 * pci_aer_available() stays in the DPC arm of
   get_port_device_capability(), because "pcie_ports=dpc-native" still
   needs it there.
 * We still gate the DPC service on native_aer and ignore
   OSC_PCI_EXPRESS_DPC_CONTROL, as Bjorn noted in the v12 cover letter.
   Fixing it changes behavior, so it wants its own patch.

Changes since v13:
  * Lukas suggested dropping AER cap check fix (patch 1 of v13). He has
    a series underway to fix it cleanly (removing DPC/AER dependency).
    So dropped the patch as suggested.
    https://lore.kernel.org/r/aq95LGUHL-pnmTlr@wunner.de
  * Addressed use of IS_ENABLED(CONFIG_PCIEPORTBUS) instead of #ifdef
    (Lukas).
  * Added Fixes tag in patch 3 (Guixin Liu).
  * Added Acks from Rafael.

v13 posting
https://lore.kernel.org/r/20260919162655.3499010-1-sathyanarayanan.kuppuswamy@linux.intel.com

Bjorn Helgaas (1):
  PCI: Centralize pci_aer_available() checking

Kuppuswamy Sathyanarayanan (3):
  PCI: Assume control of portdrv-related features only when portdrv
    enabled
  PCI/ACPI: Tidy _OSC control bit checking
  PCI/ACPI: Centralize pcie_ports_native checking

 drivers/acpi/pci_root.c           | 73 ++++++++++++++++++++++++-------
 drivers/pci/hotplug/pciehp_core.c |  2 +-
 drivers/pci/pci-acpi.c            |  3 --
 drivers/pci/pcie/aer.c            |  6 +--
 drivers/pci/pcie/aer_cxl_rch.c    |  2 +-
 drivers/pci/pcie/err.c            |  2 +-
 drivers/pci/pcie/portdrv.c        |  7 ++-
 drivers/pci/probe.c               | 10 +++--
 8 files changed, 73 insertions(+), 32 deletions(-)


base-commit: fd73f4a6659897191fa0d40695fe370925dd3780
-- 
2.43.0

[PATCH v14 1/4] PCI: Assume control of portdrv-related features only when portdrv enabled

[Kuppuswamy Sathyanarayanan]

Native control of PME, AER, DPC, and PCIe hotplug depends on the portdrv,
so default to native handling of them only when CONFIG_PCIEPORTBUS is
enabled.

Native control of LTR and SHPC hotplug does not depend on portdrv, so keep
defaulting those to native regardless.

[bhelgaas: commit log]
Link: https://lore.kernel.org/r/fcbe8a624166a1101a755edfef44a185d32ff493.1603766889.git.sathyanarayanan.kuppuswamy@linux.intel.com
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
Changes since v13

 * Use a local IS_ENABLED(CONFIG_PCIEPORTBUS) variable instead of
   #ifdef (Lukas Wunner)

v13 posting
https://lore.kernel.org/r/20260919162655.3499010-1-sathyanarayanan.kuppuswamy@linux.intel.com

 drivers/pci/probe.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 27008e2ea5af..1e487a213eb0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -658,6 +658,8 @@ static const struct device_type pci_host_bridge_type = {
 
 static void pci_init_host_bridge(struct pci_host_bridge *bridge)
 {
+	bool port_services = IS_ENABLED(CONFIG_PCIEPORTBUS);
+
 	INIT_LIST_HEAD(&bridge->windows);
 	INIT_LIST_HEAD(&bridge->dma_ranges);
 	INIT_LIST_HEAD(&bridge->ports);
@@ -668,12 +670,12 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge)
 	 * may implement its own AER handling and use _OSC to prevent the
 	 * OS from interfering.
 	 */
-	bridge->native_aer = 1;
-	bridge->native_pcie_hotplug = 1;
+	bridge->native_aer = port_services;
+	bridge->native_pcie_hotplug = port_services;
 	bridge->native_shpc_hotplug = 1;
-	bridge->native_pme = 1;
+	bridge->native_pme = port_services;
 	bridge->native_ltr = 1;
-	bridge->native_dpc = 1;
+	bridge->native_dpc = port_services;
 	bridge->domain_nr = PCI_DOMAIN_NR_NOT_SET;
 	bridge->native_cxl_error = 1;
 	bridge->dev.type = &pci_host_bridge_type;
-- 
2.43.0

[PATCH v14 2/4] PCI/ACPI: Tidy _OSC control bit checking

[Kuppuswamy Sathyanarayanan]

Add OSC_OWNER() helper to prettify checking the _OSC control bits to learn
whether the platform has granted us control of PCI features.  No functional
change intended.

[bhelgaas: split to separate patch, commit log]
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
---
Changes since v13

 * No change.  Picked up Rafael's Acked-by.

v13 posting
https://lore.kernel.org/r/20260919162655.3499010-1-sathyanarayanan.kuppuswamy@linux.intel.com

 drivers/acpi/pci_root.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 88c65f34e305..756dc2f055f5 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -993,6 +993,12 @@ static void acpi_pci_root_release_info(struct pci_host_bridge *bridge)
 	__acpi_pci_root_release_info(bridge->release_data);
 }
 
+#define OSC_OWNER(ctrl, bit, flag)		\
+	do {					\
+		if (!((ctrl) & (bit)))		\
+			flag = 0;		\
+	} while (0)
+
 struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 				     struct acpi_pci_root_ops *ops,
 				     struct acpi_pci_root_info *info,
@@ -1003,6 +1009,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 	int node = acpi_get_node(device->handle);
 	struct pci_bus *bus;
 	struct pci_host_bridge *host_bridge;
+	u32 ctrl, ext_ctrl;
 
 	info->root = root;
 	info->bridge = device;
@@ -1028,21 +1035,21 @@ 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))
-		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))
-		host_bridge->native_pme = 0;
-	if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL))
-		host_bridge->native_ltr = 0;
-	if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
-		host_bridge->native_dpc = 0;
-
-	if (!(root->osc_ext_control_set & OSC_CXL_ERROR_REPORTING_CONTROL))
-		host_bridge->native_cxl_error = 0;
+
+	ctrl = root->osc_control_set;
+	ext_ctrl = root->osc_ext_control_set;
+
+	OSC_OWNER(ctrl, OSC_PCI_EXPRESS_NATIVE_HP_CONTROL,
+		  host_bridge->native_pcie_hotplug);
+	OSC_OWNER(ctrl, OSC_PCI_SHPC_NATIVE_HP_CONTROL,
+		  host_bridge->native_shpc_hotplug);
+	OSC_OWNER(ctrl, OSC_PCI_EXPRESS_AER_CONTROL, host_bridge->native_aer);
+	OSC_OWNER(ctrl, OSC_PCI_EXPRESS_PME_CONTROL, host_bridge->native_pme);
+	OSC_OWNER(ctrl, OSC_PCI_EXPRESS_LTR_CONTROL, host_bridge->native_ltr);
+	OSC_OWNER(ctrl, OSC_PCI_EXPRESS_DPC_CONTROL, host_bridge->native_dpc);
+
+	OSC_OWNER(ext_ctrl, OSC_CXL_ERROR_REPORTING_CONTROL,
+		  host_bridge->native_cxl_error);
 
 	acpi_dev_power_up_children_with_adr(device);
 
-- 
2.43.0

[PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking

[Kuppuswamy Sathyanarayanan]

If the user booted with "pcie_ports=native", we take control of the PCIe
port services unconditionally, regardless of what _OSC says.

Centralize the testing of pcie_ports_native in acpi_pci_root_create(),
where we interpret the _OSC results, so other places only have to check
host_bridge->native_X and we don't have to sprinkle tests of
pcie_ports_native everywhere.

Rather than overriding the host_bridge->native_X flags after the fact,
fold "pcie_ports=native" into the _OSC control mask we evaluate, i.e.,
proceed as though the platform had granted control of the port services.
That way there is a single mechanism deciding each native_X flag, and we
can report exactly which features we are overriding _OSC for instead of
just noting that we are overriding something:

  acpi PNP0A08:00: _OSC: OS forcing control ("pcie_ports=native") of [PCIeHotplug PME AER DPC]

Forcing host_bridge->native_aer also restores DPC service binding under
"pcie_ports=native", which regressed in commit 97ca178c899d ("PCI/DPC:
Allow DPC on all Downstream Ports when OS controls AER").  That commit
replaced the AER service test in get_port_device_capability() with
host->native_aer, so DPC stayed off when host_bridge->native_aer was not
set.

This also extends "pcie_ports=native" to host_bridge->native_dpc, which
had no pcie_ports_native fallback before.  That effect is narrow.
native_dpc is only consumed by pci_dpc_recovered(), where with
CONFIG_PCIE_EDR=n it previously gave up immediately and now lets pciehp
wait for DPC recovery before treating a Link Down as a hotplug event.

host_bridge->native_ltr is deliberately not forced.  "pcie_ports="
controls PCIe port services and LTR is not one.  There is no
PCIE_PORT_SERVICE_LTR, and native_ltr is only used by
pci_configure_ltr() to enable ASPM L1.2, so forcing it would be an ASPM
policy decision users did not ask for.

SHPC hotplug is left alone for a simpler reason: SHPC is a conventional
PCI feature rather than a PCIe one, so "pcie_ports=" has no bearing on
it.

Fixes: 97ca178c899d ("PCI/DPC: Allow DPC on all Downstream Ports when OS controls AER")
Closes: https://lore.kernel.org/linux-pci/20260901064554.2178688-1-kanie@linux.alibaba.com/
[bhelgaas: commit log, rework OSC_PCIE_PORT_SERVICE_CONTROLS, logging]
Link: https://lore.kernel.org/r/bc87c9e675118960949043a832bed86bc22becbd.1603766889.git.sathyanarayanan.kuppuswamy@linux.intel.com
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org> # ACPI core
---
Changes since v13

 * Added the Fixes: tag for 97ca178c899d. (Guixin Liu)
 * Reworded the commit log (Sashiko AI review).
 * No code change. Picked up Rafael's Acked-by.

v13 posting
https://lore.kernel.org/r/20260919162655.3499010-1-sathyanarayanan.kuppuswamy@linux.intel.com

 drivers/acpi/pci_root.c           | 36 +++++++++++++++++++++++++++++++
 drivers/pci/hotplug/pciehp_core.c |  2 +-
 drivers/pci/pci-acpi.c            |  3 ---
 drivers/pci/pcie/aer.c            |  6 +++---
 drivers/pci/pcie/aer_cxl_rch.c    |  2 +-
 drivers/pci/pcie/err.c            |  2 +-
 drivers/pci/pcie/portdrv.c        |  6 +++---
 7 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 756dc2f055f5..2494811dd69b 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -999,6 +999,19 @@ static void acpi_pci_root_release_info(struct pci_host_bridge *bridge)
 			flag = 0;		\
 	} while (0)
 
+#define FLAG(x)		((x) ? '+' : '-')
+
+/*
+ * _OSC control bits for the features implemented by the PCIe port driver,
+ * i.e., the ones "pcie_ports=native" applies to.  LTR and SHPC hotplug are
+ * negotiated via _OSC as well, but they are not portdrv services, so
+ * "pcie_ports=" has no bearing on them.
+ */
+#define OSC_PCIE_PORT_SERVICE_CONTROLS	(OSC_PCI_EXPRESS_NATIVE_HP_CONTROL | \
+					 OSC_PCI_EXPRESS_PME_CONTROL | \
+					 OSC_PCI_EXPRESS_AER_CONTROL | \
+					 OSC_PCI_EXPRESS_DPC_CONTROL)
+
 struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 				     struct acpi_pci_root_ops *ops,
 				     struct acpi_pci_root_info *info,
@@ -1039,6 +1052,21 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 	ctrl = root->osc_control_set;
 	ext_ctrl = root->osc_ext_control_set;
 
+	/*
+	 * If the user specified "pcie_ports=native", use the PCIe port
+	 * services regardless of what _OSC says, i.e., proceed as though the
+	 * platform had granted us control of them.  This may conflict with
+	 * firmware that expects to own those features.
+	 */
+	if (pcie_ports_native) {
+		u32 override = OSC_PCIE_PORT_SERVICE_CONTROLS & ~ctrl;
+
+		if (override)
+			decode_osc_control(root, "OS forcing control (\"pcie_ports=native\") of",
+					   override);
+		ctrl |= override;
+	}
+
 	OSC_OWNER(ctrl, OSC_PCI_EXPRESS_NATIVE_HP_CONTROL,
 		  host_bridge->native_pcie_hotplug);
 	OSC_OWNER(ctrl, OSC_PCI_SHPC_NATIVE_HP_CONTROL,
@@ -1051,6 +1079,14 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 	OSC_OWNER(ext_ctrl, OSC_CXL_ERROR_REPORTING_CONTROL,
 		  host_bridge->native_cxl_error);
 
+	dev_info(&root->device->dev, "OS native features: SHPCHotplug%c PCIeHotplug%c PME%c AER%c DPC%c LTR%c\n",
+		 FLAG(host_bridge->native_shpc_hotplug),
+		 FLAG(host_bridge->native_pcie_hotplug),
+		 FLAG(host_bridge->native_pme),
+		 FLAG(host_bridge->native_aer),
+		 FLAG(host_bridge->native_dpc),
+		 FLAG(host_bridge->native_ltr));
+
 	acpi_dev_power_up_children_with_adr(device);
 
 	pci_scan_child_bus(bus);
diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index 2cafd3b26f34..b42829cf1377 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -258,7 +258,7 @@ static bool pme_is_native(struct pcie_device *dev)
 	const struct pci_host_bridge *host;
 
 	host = pci_find_host_bridge(dev->port->bus);
-	return pcie_ports_native || host->native_pme;
+	return host->native_pme;
 }
 
 static void pciehp_disable_interrupt(struct pcie_device *dev)
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 42d545edd7fa..1150f2fbabf4 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -812,9 +812,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/aer.c b/drivers/pci/pcie/aer.c
index d8dcd238fda1..e84dd686582a 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -260,7 +260,7 @@ int pcie_aer_is_native(struct pci_dev *dev)
 	if (!dev->aer_cap)
 		return 0;
 
-	return pcie_ports_native || host->native_aer;
+	return host->native_aer;
 }
 EXPORT_SYMBOL_NS_GPL(pcie_aer_is_native, "CXL");
 
@@ -1847,7 +1847,7 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
 	 */
 	aer = root ? root->aer_cap : 0;
 
-	if ((host->native_aer || pcie_ports_native) && aer)
+	if (host->native_aer && aer)
 		aer_disable_irq(root);
 
 	if (type == PCI_EXP_TYPE_RC_EC || type == PCI_EXP_TYPE_RC_END) {
@@ -1862,7 +1862,7 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
 			pci_is_root_bus(dev->bus) ? "Root" : "Downstream", rc);
 	}
 
-	if ((host->native_aer || pcie_ports_native) && aer) {
+	if (host->native_aer && aer) {
 		/* Clear Root Error Status */
 		pci_read_config_dword(root, aer + PCI_ERR_ROOT_STATUS, &reg32);
 		pci_write_config_dword(root, aer + PCI_ERR_ROOT_STATUS, reg32);
diff --git a/drivers/pci/pcie/aer_cxl_rch.c b/drivers/pci/pcie/aer_cxl_rch.c
index e471eefec9c4..b480dad8bbf4 100644
--- a/drivers/pci/pcie/aer_cxl_rch.c
+++ b/drivers/pci/pcie/aer_cxl_rch.c
@@ -31,7 +31,7 @@ static bool cxl_error_is_native(struct pci_dev *dev)
 {
 	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
 
-	return (pcie_ports_native || host->native_aer);
+	return host->native_aer;
 }
 
 static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index d77403d8855b..1a7fc71c79d8 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -273,7 +273,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
 	 * it is responsible for clearing this status.  In that case, the
 	 * signaling device may not even be visible to the OS.
 	 */
-	if (host->native_aer || pcie_ports_native) {
+	if (host->native_aer) {
 		pcie_clear_device_status(dev);
 		pci_aer_clear_nonfatal_status(dev);
 	}
diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index a9cbfc1d2bc7..32fc623dd410 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;
 
 		/*
@@ -240,14 +240,14 @@ static int get_port_device_capability(struct pci_dev *dev)
 	if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
              pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC) &&
 	    dev->aer_cap && pci_aer_available() &&
-	    (pcie_ports_native || host->native_aer))
+	    host->native_aer)
 		services |= PCIE_PORT_SERVICE_AER;
 #endif
 
 	/* Root Ports and Root Complex Event Collectors may generate PMEs */
 	if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
 	     pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC) &&
-	    (pcie_ports_native || host->native_pme)) {
+	    host->native_pme) {
 		services |= PCIE_PORT_SERVICE_PME;
 
 		/*
-- 
2.43.0

[PATCH v14 4/4] PCI: Centralize pci_aer_available() checking

[Kuppuswamy Sathyanarayanan]

From: Bjorn Helgaas <bhelgaas@google.com>

"pci=noaer" tells us not to use AER.  pci_aer_available() reports that,
and it also reports the other cases where the OS cannot use AER at all,
namely CONFIG_PCIEAER=n and MSI being unavailable.

Set host_bridge->native_aer from pci_aer_available() when we initialize
the host bridge, so callers only have to look at native_aer and we do not
have to test pci_aer_available() separately in each of them.

Do this in pci_init_host_bridge() rather than in acpi_pci_root_create()
so it also covers host bridges that are not described by ACPI and never
reach acpi_pci_root_create().

This subsumes the CONFIG_PCIEPORTBUS check for native_aer, since
pci_aer_available() is false when CONFIG_PCIEAER=n and PCIEAER depends on
PCIEPORTBUS.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Co-developed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
Changes since v13

 * No change.

v13 posting
https://lore.kernel.org/r/20260919162655.3499010-1-sathyanarayanan.kuppuswamy@linux.intel.com

 drivers/pci/pcie/portdrv.c | 3 +--
 drivers/pci/probe.c        | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index 32fc623dd410..9f8c6dd434c5 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -239,8 +239,7 @@ static int get_port_device_capability(struct pci_dev *dev)
 #ifdef CONFIG_PCIEAER
 	if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
              pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC) &&
-	    dev->aer_cap && pci_aer_available() &&
-	    host->native_aer)
+	    dev->aer_cap && host->native_aer)
 		services |= PCIE_PORT_SERVICE_AER;
 #endif
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 1e487a213eb0..e1ca8096bcd5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -670,7 +670,7 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge)
 	 * may implement its own AER handling and use _OSC to prevent the
 	 * OS from interfering.
 	 */
-	bridge->native_aer = port_services;
+	bridge->native_aer = pci_aer_available();
 	bridge->native_pcie_hotplug = port_services;
 	bridge->native_shpc_hotplug = 1;
 	bridge->native_pme = port_services;
-- 
2.43.0

Re: [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking

[Guixin Liu]

LGTM.

Reviewed-by: Guixin Liu <kanie@linux.alibaba.com>

Best Regards,
Guixin Liu

Re: [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking

[Bjorn Helgaas]

On Tue, Sep 22, 2026 at 01:45:47PM -0700, Kuppuswamy Sathyanarayanan wrote:
> If the user booted with "pcie_ports=native", we take control of the PCIe
> port services unconditionally, regardless of what _OSC says.
> 
> Centralize the testing of pcie_ports_native in acpi_pci_root_create(),
> where we interpret the _OSC results, so other places only have to check
> host_bridge->native_X and we don't have to sprinkle tests of
> pcie_ports_native everywhere.
> 
> Rather than overriding the host_bridge->native_X flags after the fact,
> fold "pcie_ports=native" into the _OSC control mask we evaluate, i.e.,
> proceed as though the platform had granted control of the port services.
> That way there is a single mechanism deciding each native_X flag, and we
> can report exactly which features we are overriding _OSC for instead of
> just noting that we are overriding something:
> 
>   acpi PNP0A08:00: _OSC: OS forcing control ("pcie_ports=native") of [PCIeHotplug PME AER DPC]
> 
> Forcing host_bridge->native_aer also restores DPC service binding under
> "pcie_ports=native", which regressed in commit 97ca178c899d ("PCI/DPC:
> Allow DPC on all Downstream Ports when OS controls AER").  That commit
> replaced the AER service test in get_port_device_capability() with
> host->native_aer, so DPC stayed off when host_bridge->native_aer was not
> set.
> 
> This also extends "pcie_ports=native" to host_bridge->native_dpc, which
> had no pcie_ports_native fallback before.  That effect is narrow.
> native_dpc is only consumed by pci_dpc_recovered(), where with
> CONFIG_PCIE_EDR=n it previously gave up immediately and now lets pciehp
> wait for DPC recovery before treating a Link Down as a hotplug event.
> 
> host_bridge->native_ltr is deliberately not forced.  "pcie_ports="
> controls PCIe port services and LTR is not one.  There is no
> PCIE_PORT_SERVICE_LTR, and native_ltr is only used by
> pci_configure_ltr() to enable ASPM L1.2, so forcing it would be an ASPM
> policy decision users did not ask for.
> 
> SHPC hotplug is left alone for a simpler reason: SHPC is a conventional
> PCI feature rather than a PCIe one, so "pcie_ports=" has no bearing on
> it.
> 
> Fixes: 97ca178c899d ("PCI/DPC: Allow DPC on all Downstream Ports when OS controls AER")
> Closes: https://lore.kernel.org/linux-pci/20260901064554.2178688-1-kanie@linux.alibaba.com/
> [bhelgaas: commit log, rework OSC_PCIE_PORT_SERVICE_CONTROLS, logging]
> Link: https://lore.kernel.org/r/bc87c9e675118960949043a832bed86bc22becbd.1603766889.git.sathyanarayanan.kuppuswamy@linux.intel.com
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org> # ACPI core
> ---
> Changes since v13
> 
>  * Added the Fixes: tag for 97ca178c899d. (Guixin Liu)
>  * Reworded the commit log (Sashiko AI review).
>  * No code change. Picked up Rafael's Acked-by.
> 
> v13 posting
> https://lore.kernel.org/r/20260919162655.3499010-1-sathyanarayanan.kuppuswamy@linux.intel.com
> 
>  drivers/acpi/pci_root.c           | 36 +++++++++++++++++++++++++++++++
>  drivers/pci/hotplug/pciehp_core.c |  2 +-
>  drivers/pci/pci-acpi.c            |  3 ---
>  drivers/pci/pcie/aer.c            |  6 +++---
>  drivers/pci/pcie/aer_cxl_rch.c    |  2 +-
>  drivers/pci/pcie/err.c            |  2 +-
>  drivers/pci/pcie/portdrv.c        |  6 +++---
>  7 files changed, 45 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 756dc2f055f5..2494811dd69b 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -999,6 +999,19 @@ static void acpi_pci_root_release_info(struct pci_host_bridge *bridge)
>  			flag = 0;		\
>  	} while (0)
>  
> +#define FLAG(x)		((x) ? '+' : '-')
> +
> +/*
> + * _OSC control bits for the features implemented by the PCIe port driver,
> + * i.e., the ones "pcie_ports=native" applies to.  LTR and SHPC hotplug are
> + * negotiated via _OSC as well, but they are not portdrv services, so
> + * "pcie_ports=" has no bearing on them.
> + */
> +#define OSC_PCIE_PORT_SERVICE_CONTROLS	(OSC_PCI_EXPRESS_NATIVE_HP_CONTROL | \
> +					 OSC_PCI_EXPRESS_PME_CONTROL | \
> +					 OSC_PCI_EXPRESS_AER_CONTROL | \
> +					 OSC_PCI_EXPRESS_DPC_CONTROL)
> +
>  struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>  				     struct acpi_pci_root_ops *ops,
>  				     struct acpi_pci_root_info *info,
> @@ -1039,6 +1052,21 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>  	ctrl = root->osc_control_set;
>  	ext_ctrl = root->osc_ext_control_set;
>  
> +	/*
> +	 * If the user specified "pcie_ports=native", use the PCIe port
> +	 * services regardless of what _OSC says, i.e., proceed as though the
> +	 * platform had granted us control of them.  This may conflict with
> +	 * firmware that expects to own those features.
> +	 */
> +	if (pcie_ports_native) {
> +		u32 override = OSC_PCIE_PORT_SERVICE_CONTROLS & ~ctrl;
> +
> +		if (override)
> +			decode_osc_control(root, "OS forcing control (\"pcie_ports=native\") of",
> +					   override);
> +		ctrl |= override;
> +	}
> +
>  	OSC_OWNER(ctrl, OSC_PCI_EXPRESS_NATIVE_HP_CONTROL,
>  		  host_bridge->native_pcie_hotplug);
>  	OSC_OWNER(ctrl, OSC_PCI_SHPC_NATIVE_HP_CONTROL,
> @@ -1051,6 +1079,14 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>  	OSC_OWNER(ext_ctrl, OSC_CXL_ERROR_REPORTING_CONTROL,
>  		  host_bridge->native_cxl_error);
>  
> +	dev_info(&root->device->dev, "OS native features: SHPCHotplug%c PCIeHotplug%c PME%c AER%c DPC%c LTR%c\n",
> +		 FLAG(host_bridge->native_shpc_hotplug),
> +		 FLAG(host_bridge->native_pcie_hotplug),
> +		 FLAG(host_bridge->native_pme),
> +		 FLAG(host_bridge->native_aer),
> +		 FLAG(host_bridge->native_dpc),

I think the native_dpc printed here is inaccurate if booted with
"pci=dpc-native", isn't it?

Can we test pcie_ports_dpc_native above, similar to what we did with
pcie_ports_native?

The DPC probe handling still seems kind of convoluted.  In portdrv.c,
we have this:

  get_port_device_capability()
  {
    if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
	pci_aer_available() &&
	(pcie_ports_dpc_native || host->native_aer))
	    services |= PCIE_PORT_SERVICE_DPC;

And then we have this in dpc.c:

  dpc_probe()
  {
    if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native)
	    return -ENOTSUPP;

The combination of get_port_device_capability() and dpc_probe() makes
my head hurt.

It seems like the DPC clause of get_port_device_capability() should be
parallel to the AER clause, i.e., set PCIE_PORT_SERVICE_DPC if the
device has a DPC Capability and the OS owns DPC (host->native_dpc).
Anything other than that needs explanation.

I'm confused about why host->native_dpc is not involved in the
dpc_probe() path at all.

> +		 FLAG(host_bridge->native_ltr));
> +
>  	acpi_dev_power_up_children_with_adr(device);
>  
>  	pci_scan_child_bus(bus);
> diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
> index 2cafd3b26f34..b42829cf1377 100644
> --- a/drivers/pci/hotplug/pciehp_core.c
> +++ b/drivers/pci/hotplug/pciehp_core.c
> @@ -258,7 +258,7 @@ static bool pme_is_native(struct pcie_device *dev)
>  	const struct pci_host_bridge *host;
>  
>  	host = pci_find_host_bridge(dev->port->bus);
> -	return pcie_ports_native || host->native_pme;
> +	return host->native_pme;
>  }
>  
>  static void pciehp_disable_interrupt(struct pcie_device *dev)
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index 42d545edd7fa..1150f2fbabf4 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -812,9 +812,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/aer.c b/drivers/pci/pcie/aer.c
> index d8dcd238fda1..e84dd686582a 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -260,7 +260,7 @@ int pcie_aer_is_native(struct pci_dev *dev)
>  	if (!dev->aer_cap)
>  		return 0;
>  
> -	return pcie_ports_native || host->native_aer;
> +	return host->native_aer;
>  }
>  EXPORT_SYMBOL_NS_GPL(pcie_aer_is_native, "CXL");
>  
> @@ -1847,7 +1847,7 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
>  	 */
>  	aer = root ? root->aer_cap : 0;
>  
> -	if ((host->native_aer || pcie_ports_native) && aer)
> +	if (host->native_aer && aer)
>  		aer_disable_irq(root);
>  
>  	if (type == PCI_EXP_TYPE_RC_EC || type == PCI_EXP_TYPE_RC_END) {
> @@ -1862,7 +1862,7 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
>  			pci_is_root_bus(dev->bus) ? "Root" : "Downstream", rc);
>  	}
>  
> -	if ((host->native_aer || pcie_ports_native) && aer) {
> +	if (host->native_aer && aer) {
>  		/* Clear Root Error Status */
>  		pci_read_config_dword(root, aer + PCI_ERR_ROOT_STATUS, &reg32);
>  		pci_write_config_dword(root, aer + PCI_ERR_ROOT_STATUS, reg32);
> diff --git a/drivers/pci/pcie/aer_cxl_rch.c b/drivers/pci/pcie/aer_cxl_rch.c
> index e471eefec9c4..b480dad8bbf4 100644
> --- a/drivers/pci/pcie/aer_cxl_rch.c
> +++ b/drivers/pci/pcie/aer_cxl_rch.c
> @@ -31,7 +31,7 @@ static bool cxl_error_is_native(struct pci_dev *dev)
>  {
>  	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
>  
> -	return (pcie_ports_native || host->native_aer);
> +	return host->native_aer;
>  }
>  
>  static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index d77403d8855b..1a7fc71c79d8 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -273,7 +273,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
>  	 * it is responsible for clearing this status.  In that case, the
>  	 * signaling device may not even be visible to the OS.
>  	 */
> -	if (host->native_aer || pcie_ports_native) {
> +	if (host->native_aer) {
>  		pcie_clear_device_status(dev);
>  		pci_aer_clear_nonfatal_status(dev);
>  	}
> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index a9cbfc1d2bc7..32fc623dd410 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;
>  
>  		/*
> @@ -240,14 +240,14 @@ static int get_port_device_capability(struct pci_dev *dev)
>  	if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
>               pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC) &&
>  	    dev->aer_cap && pci_aer_available() &&
> -	    (pcie_ports_native || host->native_aer))
> +	    host->native_aer)
>  		services |= PCIE_PORT_SERVICE_AER;
>  #endif
>  
>  	/* Root Ports and Root Complex Event Collectors may generate PMEs */
>  	if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
>  	     pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC) &&
> -	    (pcie_ports_native || host->native_pme)) {
> +	    host->native_pme) {
>  		services |= PCIE_PORT_SERVICE_PME;
>  
>  		/*
> -- 
> 2.43.0
> 

Re: [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking

[Kuppuswamy Sathyanarayanan]

Hi Bjorn,

On 10/1/2026 1:54 PM, Bjorn Helgaas wrote:
> On Tue, Sep 22, 2026 at 01:45:47PM -0700, Kuppuswamy Sathyanarayanan wrote:
>> If the user booted with "pcie_ports=native", we take control of the PCIe
>> port services unconditionally, regardless of what _OSC says.
>>
>> Centralize the testing of pcie_ports_native in acpi_pci_root_create(),
>> where we interpret the _OSC results, so other places only have to check
>> host_bridge->native_X and we don't have to sprinkle tests of
>> pcie_ports_native everywhere.
>>
>> Rather than overriding the host_bridge->native_X flags after the fact,
>> fold "pcie_ports=native" into the _OSC control mask we evaluate, i.e.,
>> proceed as though the platform had granted control of the port services.
>> That way there is a single mechanism deciding each native_X flag, and we
>> can report exactly which features we are overriding _OSC for instead of
>> just noting that we are overriding something:
>>
>>   acpi PNP0A08:00: _OSC: OS forcing control ("pcie_ports=native") of [PCIeHotplug PME AER DPC]
>>
>> Forcing host_bridge->native_aer also restores DPC service binding under
>> "pcie_ports=native", which regressed in commit 97ca178c899d ("PCI/DPC:
>> Allow DPC on all Downstream Ports when OS controls AER").  That commit
>> replaced the AER service test in get_port_device_capability() with
>> host->native_aer, so DPC stayed off when host_bridge->native_aer was not
>> set.
>>
>> This also extends "pcie_ports=native" to host_bridge->native_dpc, which
>> had no pcie_ports_native fallback before.  That effect is narrow.
>> native_dpc is only consumed by pci_dpc_recovered(), where with
>> CONFIG_PCIE_EDR=n it previously gave up immediately and now lets pciehp
>> wait for DPC recovery before treating a Link Down as a hotplug event.
>>
>> host_bridge->native_ltr is deliberately not forced.  "pcie_ports="
>> controls PCIe port services and LTR is not one.  There is no
>> PCIE_PORT_SERVICE_LTR, and native_ltr is only used by
>> pci_configure_ltr() to enable ASPM L1.2, so forcing it would be an ASPM
>> policy decision users did not ask for.
>>
>> SHPC hotplug is left alone for a simpler reason: SHPC is a conventional
>> PCI feature rather than a PCIe one, so "pcie_ports=" has no bearing on
>> it.
>>
>> Fixes: 97ca178c899d ("PCI/DPC: Allow DPC on all Downstream Ports when OS controls AER")
>> Closes: https://lore.kernel.org/linux-pci/20260901064554.2178688-1-kanie@linux.alibaba.com/
>> [bhelgaas: commit log, rework OSC_PCIE_PORT_SERVICE_CONTROLS, logging]
>> Link: https://lore.kernel.org/r/bc87c9e675118960949043a832bed86bc22becbd.1603766889.git.sathyanarayanan.kuppuswamy@linux.intel.com
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org> # ACPI core
>> ---
>> Changes since v13
>>
>>  * Added the Fixes: tag for 97ca178c899d. (Guixin Liu)
>>  * Reworded the commit log (Sashiko AI review).
>>  * No code change. Picked up Rafael's Acked-by.
>>
>> v13 posting
>> https://lore.kernel.org/r/20260919162655.3499010-1-sathyanarayanan.kuppuswamy@linux.intel.com
>>
>>  drivers/acpi/pci_root.c           | 36 +++++++++++++++++++++++++++++++
>>  drivers/pci/hotplug/pciehp_core.c |  2 +-
>>  drivers/pci/pci-acpi.c            |  3 ---
>>  drivers/pci/pcie/aer.c            |  6 +++---
>>  drivers/pci/pcie/aer_cxl_rch.c    |  2 +-
>>  drivers/pci/pcie/err.c            |  2 +-
>>  drivers/pci/pcie/portdrv.c        |  6 +++---
>>  7 files changed, 45 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index 756dc2f055f5..2494811dd69b 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -999,6 +999,19 @@ static void acpi_pci_root_release_info(struct pci_host_bridge *bridge)
>>  			flag = 0;		\
>>  	} while (0)
>>  
>> +#define FLAG(x)		((x) ? '+' : '-')
>> +
>> +/*
>> + * _OSC control bits for the features implemented by the PCIe port driver,
>> + * i.e., the ones "pcie_ports=native" applies to.  LTR and SHPC hotplug are
>> + * negotiated via _OSC as well, but they are not portdrv services, so
>> + * "pcie_ports=" has no bearing on them.
>> + */
>> +#define OSC_PCIE_PORT_SERVICE_CONTROLS	(OSC_PCI_EXPRESS_NATIVE_HP_CONTROL | \
>> +					 OSC_PCI_EXPRESS_PME_CONTROL | \
>> +					 OSC_PCI_EXPRESS_AER_CONTROL | \
>> +					 OSC_PCI_EXPRESS_DPC_CONTROL)
>> +
>>  struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>>  				     struct acpi_pci_root_ops *ops,
>>  				     struct acpi_pci_root_info *info,
>> @@ -1039,6 +1052,21 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>>  	ctrl = root->osc_control_set;
>>  	ext_ctrl = root->osc_ext_control_set;
>>  
>> +	/*
>> +	 * If the user specified "pcie_ports=native", use the PCIe port
>> +	 * services regardless of what _OSC says, i.e., proceed as though the
>> +	 * platform had granted us control of them.  This may conflict with
>> +	 * firmware that expects to own those features.
>> +	 */
>> +	if (pcie_ports_native) {
>> +		u32 override = OSC_PCIE_PORT_SERVICE_CONTROLS & ~ctrl;
>> +
>> +		if (override)
>> +			decode_osc_control(root, "OS forcing control (\"pcie_ports=native\") of",
>> +					   override);
>> +		ctrl |= override;
>> +	}
>> +
>>  	OSC_OWNER(ctrl, OSC_PCI_EXPRESS_NATIVE_HP_CONTROL,
>>  		  host_bridge->native_pcie_hotplug);
>>  	OSC_OWNER(ctrl, OSC_PCI_SHPC_NATIVE_HP_CONTROL,
>> @@ -1051,6 +1079,14 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>>  	OSC_OWNER(ext_ctrl, OSC_CXL_ERROR_REPORTING_CONTROL,
>>  		  host_bridge->native_cxl_error);
>>  
>> +	dev_info(&root->device->dev, "OS native features: SHPCHotplug%c PCIeHotplug%c PME%c AER%c DPC%c LTR%c\n",
>> +		 FLAG(host_bridge->native_shpc_hotplug),
>> +		 FLAG(host_bridge->native_pcie_hotplug),
>> +		 FLAG(host_bridge->native_pme),
>> +		 FLAG(host_bridge->native_aer),
>> +		 FLAG(host_bridge->native_dpc),
> 
> I think the native_dpc printed here is inaccurate if booted with
> "pci=dpc-native", isn't it?

Yes, it is inaccurate.  "pcie_ports=dpc-native" has never set
native_dpc, but this patch is the first to print it, so I will fix it
here.

I will fold it into the same override block, so both parameters go
through the control mask and get logged the same way, something like:

	u32 override = 0;

	if (pcie_ports_native)
		override = OSC_PCIE_PORT_SERVICE_CONTROLS & ~ctrl;
	else if (pcie_ports_dpc_native)
		override = OSC_PCI_EXPRESS_DPC_CONTROL & ~ctrl;

	if (override) {
		decode_osc_control(root, pcie_ports_native ?
				   "OS forcing control (\"pcie_ports=native\") of" :
				   "OS forcing control (\"pcie_ports=dpc-native\") of",
				   override);
		ctrl |= override;
	}

> 
> Can we test pcie_ports_dpc_native above, similar to what we did with
> pcie_ports_native?
> 
> The DPC probe handling still seems kind of convoluted.  In portdrv.c,
> we have this:
> 
>   get_port_device_capability()
>   {
>     if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> 	pci_aer_available() &&
> 	(pcie_ports_dpc_native || host->native_aer))
> 	    services |= PCIE_PORT_SERVICE_DPC;
> 
> And then we have this in dpc.c:
> 
>   dpc_probe()
>   {
>     if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native)
> 	    return -ENOTSUPP;
> 
> The combination of get_port_device_capability() and dpc_probe() makes
> my head hurt.
> 
> It seems like the DPC clause of get_port_device_capability() should be
> parallel to the AER clause, i.e., set PCIE_PORT_SERVICE_DPC if the
> device has a DPC Capability and the OS owns DPC (host->native_dpc).
> Anything other than that needs explanation.
> 
> I'm confused about why host->native_dpc is not involved in the
> dpc_probe() path at all.

Agreed, DPC should be decided on its own, independent of AER.  Today it
is tied to AER in several places:

1. portdrv binds DPC on host->native_aer, not native_dpc, following
   the PCIe r7.0 sec 6.2.11 implementation note that links DPC control
   to AER control (97ca178c899d).
2. The pci_aer_available() test in the same clause means "pci=noaer"
   also disables DPC.
3. dpc_probe() repeats the ownership check via pcie_aer_is_native(),
   which also tests dev->aer_cap, so DPC never binds on ports without
   an AER Capability.
4. native_dpc is only consumed by pci_dpc_recovered().

With the change above, both "pcie_ports=native" and
"pcie_ports=dpc-native" set native_dpc, so binding on native_dpc would
work for them.  The problem is the default case, with no "pcie_ports="
parameter.  We only request OSC_PCI_EXPRESS_DPC_CONTROL when
CONFIG_PCIE_EDR=y, as the PCI Firmware spec requires, so on an EDR=n
kernel native_dpc is never set on ACPI systems.  Today DPC still binds
there through native_aer, and switching portdrv to test native_dpc
alone would silently turn DPC off on those systems.

I think the simplest fix is to make PCIE_DPC "select PCIE_EDR if ACPI",
so an ACPI kernel with DPC always negotiates DPC control and native_dpc
reflects what firmware actually granted.  Then portdrv can bind DPC on
"DPC Capability && native_dpc", just like the AER clause.

Lukas has posted a series that removes the DPC/AER dependency:

  https://lore.kernel.org/r/cover.1790531238.git.lukas@wunner.de

His 1/7 lets DPC work without an AER Capability, and 2/7 removes the
dpc_probe() check entirely, so portdrv becomes the only place that
decides.  The native_dpc change fits naturally on top of that.

Would you like me to do the native_dpc change in this series, or as a
separate patch on top of Lukas's series?  If we do it as a separate
patch on top of Lukas's work, this series can stay a cleanup (plus the
fix for Guixin's issue).

> 
>> +		 FLAG(host_bridge->native_ltr));
>> +
>>  	acpi_dev_power_up_children_with_adr(device);
>>  
>>  	pci_scan_child_bus(bus);
>> diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
>> index 2cafd3b26f34..b42829cf1377 100644
>> --- a/drivers/pci/hotplug/pciehp_core.c
>> +++ b/drivers/pci/hotplug/pciehp_core.c
>> @@ -258,7 +258,7 @@ static bool pme_is_native(struct pcie_device *dev)
>>  	const struct pci_host_bridge *host;
>>  
>>  	host = pci_find_host_bridge(dev->port->bus);
>> -	return pcie_ports_native || host->native_pme;
>> +	return host->native_pme;
>>  }
>>  
>>  static void pciehp_disable_interrupt(struct pcie_device *dev)
>> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
>> index 42d545edd7fa..1150f2fbabf4 100644
>> --- a/drivers/pci/pci-acpi.c
>> +++ b/drivers/pci/pci-acpi.c
>> @@ -812,9 +812,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/aer.c b/drivers/pci/pcie/aer.c
>> index d8dcd238fda1..e84dd686582a 100644
>> --- a/drivers/pci/pcie/aer.c
>> +++ b/drivers/pci/pcie/aer.c
>> @@ -260,7 +260,7 @@ int pcie_aer_is_native(struct pci_dev *dev)
>>  	if (!dev->aer_cap)
>>  		return 0;
>>  
>> -	return pcie_ports_native || host->native_aer;
>> +	return host->native_aer;
>>  }
>>  EXPORT_SYMBOL_NS_GPL(pcie_aer_is_native, "CXL");
>>  
>> @@ -1847,7 +1847,7 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
>>  	 */
>>  	aer = root ? root->aer_cap : 0;
>>  
>> -	if ((host->native_aer || pcie_ports_native) && aer)
>> +	if (host->native_aer && aer)
>>  		aer_disable_irq(root);
>>  
>>  	if (type == PCI_EXP_TYPE_RC_EC || type == PCI_EXP_TYPE_RC_END) {
>> @@ -1862,7 +1862,7 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
>>  			pci_is_root_bus(dev->bus) ? "Root" : "Downstream", rc);
>>  	}
>>  
>> -	if ((host->native_aer || pcie_ports_native) && aer) {
>> +	if (host->native_aer && aer) {
>>  		/* Clear Root Error Status */
>>  		pci_read_config_dword(root, aer + PCI_ERR_ROOT_STATUS, &reg32);
>>  		pci_write_config_dword(root, aer + PCI_ERR_ROOT_STATUS, reg32);
>> diff --git a/drivers/pci/pcie/aer_cxl_rch.c b/drivers/pci/pcie/aer_cxl_rch.c
>> index e471eefec9c4..b480dad8bbf4 100644
>> --- a/drivers/pci/pcie/aer_cxl_rch.c
>> +++ b/drivers/pci/pcie/aer_cxl_rch.c
>> @@ -31,7 +31,7 @@ static bool cxl_error_is_native(struct pci_dev *dev)
>>  {
>>  	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
>>  
>> -	return (pcie_ports_native || host->native_aer);
>> +	return host->native_aer;
>>  }
>>  
>>  static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
>> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
>> index d77403d8855b..1a7fc71c79d8 100644
>> --- a/drivers/pci/pcie/err.c
>> +++ b/drivers/pci/pcie/err.c
>> @@ -273,7 +273,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
>>  	 * it is responsible for clearing this status.  In that case, the
>>  	 * signaling device may not even be visible to the OS.
>>  	 */
>> -	if (host->native_aer || pcie_ports_native) {
>> +	if (host->native_aer) {
>>  		pcie_clear_device_status(dev);
>>  		pci_aer_clear_nonfatal_status(dev);
>>  	}
>> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
>> index a9cbfc1d2bc7..32fc623dd410 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;
>>  
>>  		/*
>> @@ -240,14 +240,14 @@ static int get_port_device_capability(struct pci_dev *dev)
>>  	if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
>>               pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC) &&
>>  	    dev->aer_cap && pci_aer_available() &&
>> -	    (pcie_ports_native || host->native_aer))
>> +	    host->native_aer)
>>  		services |= PCIE_PORT_SERVICE_AER;
>>  #endif
>>  
>>  	/* Root Ports and Root Complex Event Collectors may generate PMEs */
>>  	if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
>>  	     pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC) &&
>> -	    (pcie_ports_native || host->native_pme)) {
>> +	    host->native_pme) {
>>  		services |= PCIE_PORT_SERVICE_PME;
>>  
>>  		/*
>> -- 
>> 2.43.0
>>

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking

[Bjorn Helgaas]

On Thu, Oct 01, 2026 at 02:48:46PM -0700, Kuppuswamy Sathyanarayanan wrote:
> Hi Bjorn,
> 
> On 10/1/2026 1:54 PM, Bjorn Helgaas wrote:
> > On Tue, Sep 22, 2026 at 01:45:47PM -0700, Kuppuswamy Sathyanarayanan wrote:
> >> If the user booted with "pcie_ports=native", we take control of the PCIe
> >> port services unconditionally, regardless of what _OSC says.
> >>
> >> Centralize the testing of pcie_ports_native in acpi_pci_root_create(),
> >> where we interpret the _OSC results, so other places only have to check
> >> host_bridge->native_X and we don't have to sprinkle tests of
> >> pcie_ports_native everywhere.
> >>
> >> Rather than overriding the host_bridge->native_X flags after the fact,
> >> fold "pcie_ports=native" into the _OSC control mask we evaluate, i.e.,
> >> proceed as though the platform had granted control of the port services.
> >> That way there is a single mechanism deciding each native_X flag, and we
> >> can report exactly which features we are overriding _OSC for instead of
> >> just noting that we are overriding something:
> >>
> >>   acpi PNP0A08:00: _OSC: OS forcing control ("pcie_ports=native") of [PCIeHotplug PME AER DPC]
> >>
> >> Forcing host_bridge->native_aer also restores DPC service binding under
> >> "pcie_ports=native", which regressed in commit 97ca178c899d ("PCI/DPC:
> >> Allow DPC on all Downstream Ports when OS controls AER").  That commit
> >> replaced the AER service test in get_port_device_capability() with
> >> host->native_aer, so DPC stayed off when host_bridge->native_aer was not
> >> set.
> >>
> >> This also extends "pcie_ports=native" to host_bridge->native_dpc, which
> >> had no pcie_ports_native fallback before.  That effect is narrow.
> >> native_dpc is only consumed by pci_dpc_recovered(), where with
> >> CONFIG_PCIE_EDR=n it previously gave up immediately and now lets pciehp
> >> wait for DPC recovery before treating a Link Down as a hotplug event.
> >>
> >> host_bridge->native_ltr is deliberately not forced.  "pcie_ports="
> >> controls PCIe port services and LTR is not one.  There is no
> >> PCIE_PORT_SERVICE_LTR, and native_ltr is only used by
> >> pci_configure_ltr() to enable ASPM L1.2, so forcing it would be an ASPM
> >> policy decision users did not ask for.
> >>
> >> SHPC hotplug is left alone for a simpler reason: SHPC is a conventional
> >> PCI feature rather than a PCIe one, so "pcie_ports=" has no bearing on
> >> it.
> ...

> >> +	/*
> >> +	 * If the user specified "pcie_ports=native", use the PCIe port
> >> +	 * services regardless of what _OSC says, i.e., proceed as though the
> >> +	 * platform had granted us control of them.  This may conflict with
> >> +	 * firmware that expects to own those features.
> >> +	 */
> >> +	if (pcie_ports_native) {
> >> +		u32 override = OSC_PCIE_PORT_SERVICE_CONTROLS & ~ctrl;
> >> +
> >> +		if (override)
> >> +			decode_osc_control(root, "OS forcing control (\"pcie_ports=native\") of",
> >> +					   override);
> >> +		ctrl |= override;
> >> +	}
> >> +
> >>  	OSC_OWNER(ctrl, OSC_PCI_EXPRESS_NATIVE_HP_CONTROL,
> >>  		  host_bridge->native_pcie_hotplug);
> >>  	OSC_OWNER(ctrl, OSC_PCI_SHPC_NATIVE_HP_CONTROL,
> >> @@ -1051,6 +1079,14 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
> >>  	OSC_OWNER(ext_ctrl, OSC_CXL_ERROR_REPORTING_CONTROL,
> >>  		  host_bridge->native_cxl_error);
> >>  
> >> +	dev_info(&root->device->dev, "OS native features: SHPCHotplug%c PCIeHotplug%c PME%c AER%c DPC%c LTR%c\n",
> >> +		 FLAG(host_bridge->native_shpc_hotplug),
> >> +		 FLAG(host_bridge->native_pcie_hotplug),
> >> +		 FLAG(host_bridge->native_pme),
> >> +		 FLAG(host_bridge->native_aer),
> >> +		 FLAG(host_bridge->native_dpc),
> > 
> > I think the native_dpc printed here is inaccurate if booted with
> > "pci=dpc-native", isn't it?
> 
> Yes, it is inaccurate.  "pcie_ports=dpc-native" has never set
> native_dpc, but this patch is the first to print it, so I will fix it
> here.
> 
> I will fold it into the same override block, so both parameters go
> through the control mask and get logged the same way, something like:
> 
> 	u32 override = 0;
> 
> 	if (pcie_ports_native)
> 		override = OSC_PCIE_PORT_SERVICE_CONTROLS & ~ctrl;
> 	else if (pcie_ports_dpc_native)
> 		override = OSC_PCI_EXPRESS_DPC_CONTROL & ~ctrl;
> 
> 	if (override) {
> 		decode_osc_control(root, pcie_ports_native ?
> 				   "OS forcing control (\"pcie_ports=native\") of" :
> 				   "OS forcing control (\"pcie_ports=dpc-native\") of",
> 				   override);
> 		ctrl |= override;
> 	}

Sounds good.

> > The DPC probe handling still seems kind of convoluted.  In portdrv.c,
> > we have this:
> > 
> >   get_port_device_capability()
> >   {
> >     if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> > 	pci_aer_available() &&
> > 	(pcie_ports_dpc_native || host->native_aer))
> > 	    services |= PCIE_PORT_SERVICE_DPC;
> > 
> > And then we have this in dpc.c:
> > 
> >   dpc_probe()
> >   {
> >     if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native)
> > 	    return -ENOTSUPP;
> > 
> > The combination of get_port_device_capability() and dpc_probe() makes
> > my head hurt.
> > 
> > It seems like the DPC clause of get_port_device_capability() should be
> > parallel to the AER clause, i.e., set PCIE_PORT_SERVICE_DPC if the
> > device has a DPC Capability and the OS owns DPC (host->native_dpc).
> > Anything other than that needs explanation.
> > 
> > I'm confused about why host->native_dpc is not involved in the
> > dpc_probe() path at all.
> 
> Agreed, DPC should be decided on its own, independent of AER.  Today it
> is tied to AER in several places:
> 
> 1. portdrv binds DPC on host->native_aer, not native_dpc, following
>    the PCIe r7.0 sec 6.2.11 implementation note that links DPC control
>    to AER control (97ca178c899d).
> 2. The pci_aer_available() test in the same clause means "pci=noaer"
>    also disables DPC.
> 3. dpc_probe() repeats the ownership check via pcie_aer_is_native(),
>    which also tests dev->aer_cap, so DPC never binds on ports without
>    an AER Capability.
> 4. native_dpc is only consumed by pci_dpc_recovered().
> 
> With the change above, both "pcie_ports=native" and
> "pcie_ports=dpc-native" set native_dpc, so binding on native_dpc would
> work for them.  The problem is the default case, with no "pcie_ports="
> parameter.  We only request OSC_PCI_EXPRESS_DPC_CONTROL when
> CONFIG_PCIE_EDR=y, as the PCI Firmware spec requires, so on an EDR=n
> kernel native_dpc is never set on ACPI systems.

Right, I forgot about that.  This deserves a comment in the code
somewhere.

I think we still need to be able to use DPC without EDR on non-ACPI
systems.

> Today DPC still binds there through native_aer, and switching
> portdrv to test native_dpc alone would silently turn DPC off on
> those systems.

It would be nice if we could make native_dpc be the end result of AER,
EDR, and anything else that determines whether the OS can actually use
DPC.

> I think the simplest fix is to make PCIE_DPC "select PCIE_EDR if ACPI",
> so an ACPI kernel with DPC always negotiates DPC control and native_dpc
> reflects what firmware actually granted.  Then portdrv can bind DPC on
> "DPC Capability && native_dpc", just like the AER clause.
> 
> Lukas has posted a series that removes the DPC/AER dependency:
> 
>   https://lore.kernel.org/r/cover.1790531238.git.lukas@wunner.de
> 
> His 1/7 lets DPC work without an AER Capability, and 2/7 removes the
> dpc_probe() check entirely, so portdrv becomes the only place that
> decides.  The native_dpc change fits naturally on top of that.
> 
> Would you like me to do the native_dpc change in this series, or as a
> separate patch on top of Lukas's series?  If we do it as a separate
> patch on top of Lukas's work, this series can stay a cleanup (plus the
> fix for Guixin's issue).

Doing it on top of Lukas's series sounds good to me.