Re: [PATCH v13 4/5] PCI/ACPI: Centralize pcie_ports_native checking

[Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>]

On 9/19/2026 9:39 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] pci: implicit system-wide DPC binding contradicts commit message
> --
> 
> commit 07669185337d665d35c6cf7148608c57752e5ecb
> Author: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> PCI/ACPI: Centralize pcie_ports_native checking
> 
> This commit centralizes the testing of pcie_ports_native in 
> acpi_pci_root_create() by folding it into the _OSC control mask. It allows 
> other subsystems to simply check the host_bridge->native_X flags instead of 
> using scattered pcie_ports_native tests.
> 
> Link: https://lore.kernel.org/r/bc87c9e675118960949043a832bed86bc22becbd.1603766889.git.sathyanarayanan.kuppuswamy@linux.intel.com
> 
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index 756dc2f055f51..2494811dd69b2 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
> [ ... ]
>> @@ -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(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);
> 
> [Severity: Medium]
> The commit message states that extending "pcie_ports=native" to DPC has a
> "narrow" effect, only impacting pci_dpc_recovered(). However, does this
> unintentionally enable the DPC port driver system-wide?
> 
> Because pcie_ports=native now forces host_bridge->native_aer to true via the
> OSC_PCI_EXPRESS_AER_CONTROL bit override, this impacts the DPC port service
> binding in get_port_device_capability():
> 
> drivers/pci/pcie/portdrv.c: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;
>         ...
> }
> 
> Since host->native_aer evaluates to true when the pcie_ports=native parameter
> is provided, the DPC capability check will now unconditionally pass. Could
> this break the assumption that the effect on DPC was isolated?

You are right, the commit log is a bit misleading here.  The narrow effect
we talked about is the impact of the native_dpc flag change.  Enabling of
the DPC port service is currently controlled by native_aer, and this patch
enables it intentionally when "pcie_ports=native" is used.

I'll clarify it in v14:

  Forcing host_bridge->native_aer also restores DPC service binding under
  "pcie_ports=native", which regressed in 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.

> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer