Re: [PATCH v2 6/6] PCI/portdrv: Bind DPC service based on host_bridge->native_dpc

[Yazen Ghannam]

On Tue, Oct 20, 2026 at 11:19:23AM -0700, Kuppuswamy Sathyanarayanan wrote:
> get_port_device_capability() binds the DPC service if the port has a DPC
> capability and either "pcie_ports=dpc-native" was given or the OS
> controls AER.  This repeats decisions that are already made elsewhere.
> It also ignores host_bridge->native_dpc, which is meant to say whether
> the OS owns DPC.
> 
> Make host_bridge->native_dpc the single decider:
> 
>   - In acpi_pci_root_create(), assume DPC control whenever _OSC grants
>     AER control.  PCI Firmware r3.3, sec 4.5.2.4, requires platforms to
>     retain AER if they retain DPC.  Put the other way, firmware that
>     grants AER does not keep DPC.  PCIe r7.0 sec 6.2.11 recommends that
>     the OS link control of DPC to control of AER.  Commit 97ca178c899d
>     ("PCI/DPC: Allow DPC on all Downstream Ports when OS controls AER")
>     relied on this link.  Keeping it means DPC still works with firmware
>     that predates the _OSC DPC control bit.  Such firmware grants AER but
>     masks DPC.  The OS cannot tell such firmware from firmware that
>     retains DPC on purpose, so log a message when it assumes DPC control
>     that _OSC did not grant.  Only do this if the OS requested DPC
>     control, i.e., if CONFIG_PCIE_DPC is enabled.
> 
>   - "pcie_ports=native" and "pcie_ports=dpc-native" are already folded
>     into the _OSC control mask, so they are reflected in native_dpc.
> 
> Then bind the DPC service if the port has a DPC capability and
> host_bridge->native_dpc is set.
> 
> The only change in behavior is for firmware that grants DPC control but
> retains AER control.  PCI Firmware r3.3, sec 4.5.2.4, does not forbid
> this.  The OS now uses DPC there because firmware granted it.  Since
> "PCI/DPC: Reinstate support for AER-incapable ports", dpc_probe() no
> longer requires AER control.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> ---
>  drivers/acpi/pci_root.c    | 16 ++++++++++++++++
>  drivers/pci/pcie/portdrv.c |  6 +-----
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 5c674d5b6bb8..b380b6d8874d 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -1056,6 +1056,22 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>  	ctrl = root->osc_control_set;
>  	ext_ctrl = root->osc_ext_control_set;
>  
> +	/*
> +	 * PCI Firmware r3.3, sec 4.5.2.4, requires platforms to retain AER
> +	 * if they retain DPC.  Put the other way, firmware that grants AER
> +	 * does not keep DPC.  PCIe r7.0 sec 6.2.11 recommends that the OS
> +	 * link control of DPC to control of AER.  Firmware that predates
> +	 * the _OSC DPC control bit grants AER without DPC, so assume control
> +	 * of DPC whenever we control AER.
> +	 */

Oh, I see now that this got rewritten.

But it really does repeat the same info from the commit message. So IMO
it can just be removed or summarized further.

> +	if (IS_ENABLED(CONFIG_PCIE_DPC) &&
> +	    (ctrl & OSC_PCI_EXPRESS_AER_CONTROL) &&
> +	    !(ctrl & OSC_PCI_EXPRESS_DPC_CONTROL)) {
> +		decode_osc_control(root, "OS assuming control (AER granted) of",
> +				   OSC_PCI_EXPRESS_DPC_CONTROL);
> +		ctrl |= OSC_PCI_EXPRESS_DPC_CONTROL;
> +	}
> +
>  	/*
>  	 * If the user specified "pcie_ports=native", use the PCIe port
>  	 * services regardless of what _OSC says, i.e., proceed as though the
> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
> index b77680fd3b75..f1beb440a3dc 100644
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -258,12 +258,8 @@ static int get_port_device_capability(struct pci_dev *dev)
>  		pcie_pme_interrupt_enable(dev, false);
>  	}
>  
> -	/*
> -	 * With dpc-native, allow Linux to use DPC even if it doesn't have
> -	 * permission to use AER.
> -	 */
>  	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> -	    (pcie_ports_dpc_native || host->native_aer))
> +	    host->native_dpc)
>  		services |= PCIE_PORT_SERVICE_DPC;
>  
>  	/* Enable bandwidth control if more than one speed is supported. */
> -- 

Besides my comment on the comment, looks good to me.

Reviewed-by: Yazen Ghannam <yazen.ghannam@xxxxxxx>

Thanks,
Yazen