mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
To: Guixin Liu <kanie@linux.alibaba.com>,
	Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Rafael J Wysocki <rafael@kernel.org>,
	Olof Johansson <olof@lixom.net>
Subject: Re: [PATCH v13 4/5] PCI/ACPI: Centralize pcie_ports_native checking
Date: Mon, 21 Sep 2026 13:41:39 -0700	[thread overview]
Message-ID: <6fa0674c-eeb2-42b7-bea3-61bee9908fbe@linux.intel.com> (raw)
In-Reply-To: <905ddaea-8ed3-4c8b-ae4e-593a81e59833@linux.alibaba.com>



On 9/20/2026 12:24 AM, Guixin Liu wrote:
> 
> 
> 在 2026/9/20 00:26, 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]
>>
>> This also extends "pcie_ports=native" to host_bridge->native_dpc, which
>> had no pcie_ports_native fallback before.  The effect is narrow.
>> native_dpc is only used by pci_dpc_recovered(), and only when
>> CONFIG_PCIE_EDR=n, where hotplug now waits for firmware-owned DPC
>> recovery.
>>
>> 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.
>>
>> [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>、
> Should add Fixes: 97ca178c899d ("PCI/DPC: Allow DPC on all Downstream Ports when OS controls AER").

Sure. Will add it in v14.

> 
> Best Regards,
> Guixin Liu
>> ---
>> Changes since v12
>>
>>  * Rebased to v7.3-rc3.
>>  * Fold "pcie_ports=native" into the _OSC control mask instead of
>>    overriding the native_X flags afterwards, and report which features we
>>    override, as agreed in the v12 review.
>>  * Do not force native_ltr, which v12 did.  LTR is not a PCIe port
>>    service, so "pcie_ports=" should not affect it.
>>
>> v12 posting
>> https://lore.kernel.org/all/20201126011816.711106-1-helgaas@kernel.org/
>>
>>  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;
>>  
>>  		/*
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


  reply	other threads:[~2026-09-21 20:41 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-19 16:26 [PATCH v13 0/5] Simplify PCIe native ownership Kuppuswamy Sathyanarayanan
2026-09-19 16:26 ` [PATCH v13 1/5] PCI/DPC: Ignore devices with no AER Capability Kuppuswamy Sathyanarayanan
2026-09-20  6:11   ` Lukas Wunner
2026-09-21 16:43     ` Kuppuswamy Sathyanarayanan
2026-09-21 19:01       ` Bjorn Helgaas
2026-09-19 16:26 ` [PATCH v13 2/5] PCI: Assume control of portdrv-related features only when portdrv enabled Kuppuswamy Sathyanarayanan
2026-09-20  6:17   ` Lukas Wunner
2026-09-21 18:59     ` Kuppuswamy Sathyanarayanan
2026-09-21 19:01     ` Bjorn Helgaas
2026-09-19 16:26 ` [PATCH v13 3/5] PCI/ACPI: Tidy _OSC control bit checking Kuppuswamy Sathyanarayanan
2026-09-19 16:26 ` [PATCH v13 4/5] PCI/ACPI: Centralize pcie_ports_native checking Kuppuswamy Sathyanarayanan
2026-09-20  7:24   ` Guixin Liu
2026-09-21 20:41     ` Kuppuswamy Sathyanarayanan [this message]
2026-09-19 16:26 ` [PATCH v13 5/5] PCI: Centralize pci_aer_available() checking Kuppuswamy Sathyanarayanan
  -- strict thread matches above, loose matches on Subject: below --
2021-01-23  1:11 [PATCH v13 0/5] Simplify PCIe native ownership Kuppuswamy Sathyanarayanan
2021-01-23  1:11 ` [PATCH v13 4/5] PCI/ACPI: Centralize pcie_ports_native checking Kuppuswamy Sathyanarayanan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6fa0674c-eeb2-42b7-bea3-61bee9908fbe@linux.intel.com \
    --to=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=bhelgaas@google.com \
    --cc=kanie@linux.alibaba.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=olof@lixom.net \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®