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

[sashiko-bot@kernel.org]

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919162655.3499010-1-sathyanarayanan.kuppuswamy@linux.intel.com?part=4