* [PATCH v14 0/4] Simplify PCIe native ownership
@ 2026-09-22 20:45 Kuppuswamy Sathyanarayanan
2026-09-22 20:45 ` [PATCH v14 1/4] PCI: Assume control of portdrv-related features only when portdrv enabled Kuppuswamy Sathyanarayanan
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2026-09-22 20:45 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, linux-acpi, linux-kernel, rafael, lukas, terry.bowman,
kanie, olof, 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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v14 1/4] PCI: Assume control of portdrv-related features only when portdrv enabled 2026-09-22 20:45 [PATCH v14 0/4] Simplify PCIe native ownership Kuppuswamy Sathyanarayanan @ 2026-09-22 20:45 ` Kuppuswamy Sathyanarayanan 2026-09-22 20:45 ` [PATCH v14 2/4] PCI/ACPI: Tidy _OSC control bit checking Kuppuswamy Sathyanarayanan ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2026-09-22 20:45 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-pci, linux-acpi, linux-kernel, rafael, lukas, terry.bowman, kanie, olof, 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v14 2/4] PCI/ACPI: Tidy _OSC control bit checking 2026-09-22 20:45 [PATCH v14 0/4] Simplify PCIe native ownership Kuppuswamy Sathyanarayanan 2026-09-22 20:45 ` [PATCH v14 1/4] PCI: Assume control of portdrv-related features only when portdrv enabled Kuppuswamy Sathyanarayanan @ 2026-09-22 20:45 ` Kuppuswamy Sathyanarayanan 2026-09-22 20:45 ` [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking Kuppuswamy Sathyanarayanan 2026-09-22 20:45 ` [PATCH v14 4/4] PCI: Centralize pci_aer_available() checking Kuppuswamy Sathyanarayanan 3 siblings, 0 replies; 9+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2026-09-22 20:45 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-pci, linux-acpi, linux-kernel, rafael, lukas, terry.bowman, kanie, olof, 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking 2026-09-22 20:45 [PATCH v14 0/4] Simplify PCIe native ownership Kuppuswamy Sathyanarayanan 2026-09-22 20:45 ` [PATCH v14 1/4] PCI: Assume control of portdrv-related features only when portdrv enabled Kuppuswamy Sathyanarayanan 2026-09-22 20:45 ` [PATCH v14 2/4] PCI/ACPI: Tidy _OSC control bit checking Kuppuswamy Sathyanarayanan @ 2026-09-22 20:45 ` Kuppuswamy Sathyanarayanan 2026-09-23 1:44 ` Guixin Liu 2026-10-01 20:54 ` Bjorn Helgaas 2026-09-22 20:45 ` [PATCH v14 4/4] PCI: Centralize pci_aer_available() checking Kuppuswamy Sathyanarayanan 3 siblings, 2 replies; 9+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2026-09-22 20:45 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-pci, linux-acpi, linux-kernel, rafael, lukas, terry.bowman, kanie, olof, 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, ®32); 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking 2026-09-22 20:45 ` [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking Kuppuswamy Sathyanarayanan @ 2026-09-23 1:44 ` Guixin Liu 2026-10-01 20:54 ` Bjorn Helgaas 1 sibling, 0 replies; 9+ messages in thread From: Guixin Liu @ 2026-09-23 1:44 UTC (permalink / raw) To: Kuppuswamy Sathyanarayanan, Bjorn Helgaas Cc: linux-pci, linux-acpi, linux-kernel, rafael, lukas, terry.bowman, olof LGTM. Reviewed-by: Guixin Liu <kanie@linux.alibaba.com> Best Regards, Guixin Liu ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking 2026-09-22 20:45 ` [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking Kuppuswamy Sathyanarayanan 2026-09-23 1:44 ` Guixin Liu @ 2026-10-01 20:54 ` Bjorn Helgaas 2026-10-01 21:48 ` Kuppuswamy Sathyanarayanan 1 sibling, 1 reply; 9+ messages in thread From: Bjorn Helgaas @ 2026-10-01 20:54 UTC (permalink / raw) To: Kuppuswamy Sathyanarayanan Cc: Bjorn Helgaas, linux-pci, linux-acpi, linux-kernel, rafael, lukas, terry.bowman, kanie, olof 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, ®32); > 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking 2026-10-01 20:54 ` Bjorn Helgaas @ 2026-10-01 21:48 ` Kuppuswamy Sathyanarayanan 2026-10-01 22:14 ` Bjorn Helgaas 0 siblings, 1 reply; 9+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2026-10-01 21:48 UTC (permalink / raw) To: Bjorn Helgaas Cc: Bjorn Helgaas, linux-pci, linux-acpi, linux-kernel, rafael, lukas, terry.bowman, kanie, olof 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, ®32); >> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking 2026-10-01 21:48 ` Kuppuswamy Sathyanarayanan @ 2026-10-01 22:14 ` Bjorn Helgaas 0 siblings, 0 replies; 9+ messages in thread From: Bjorn Helgaas @ 2026-10-01 22:14 UTC (permalink / raw) To: Kuppuswamy Sathyanarayanan Cc: Bjorn Helgaas, linux-pci, linux-acpi, linux-kernel, rafael, lukas, terry.bowman, kanie, olof 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v14 4/4] PCI: Centralize pci_aer_available() checking 2026-09-22 20:45 [PATCH v14 0/4] Simplify PCIe native ownership Kuppuswamy Sathyanarayanan ` (2 preceding siblings ...) 2026-09-22 20:45 ` [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking Kuppuswamy Sathyanarayanan @ 2026-09-22 20:45 ` Kuppuswamy Sathyanarayanan 3 siblings, 0 replies; 9+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2026-09-22 20:45 UTC (permalink / raw) To: Bjorn Helgaas Cc: linux-pci, linux-acpi, linux-kernel, rafael, lukas, terry.bowman, kanie, olof, 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-01 22:14 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-22 20:45 [PATCH v14 0/4] Simplify PCIe native ownership Kuppuswamy Sathyanarayanan 2026-09-22 20:45 ` [PATCH v14 1/4] PCI: Assume control of portdrv-related features only when portdrv enabled Kuppuswamy Sathyanarayanan 2026-09-22 20:45 ` [PATCH v14 2/4] PCI/ACPI: Tidy _OSC control bit checking Kuppuswamy Sathyanarayanan 2026-09-22 20:45 ` [PATCH v14 3/4] PCI/ACPI: Centralize pcie_ports_native checking Kuppuswamy Sathyanarayanan 2026-09-23 1:44 ` Guixin Liu 2026-10-01 20:54 ` Bjorn Helgaas 2026-10-01 21:48 ` Kuppuswamy Sathyanarayanan 2026-10-01 22:14 ` Bjorn Helgaas 2026-09-22 20:45 ` [PATCH v14 4/4] PCI: Centralize pci_aer_available() checking Kuppuswamy Sathyanarayanan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®