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

[Kuppuswamy Sathyanarayanan]

On 11/4/2026 8:35 AM, Yazen Ghannam wrote:
> 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.

Agreed.  I'll shorten it in v3 to something like:

	/*
	 * Firmware that predates the _OSC DPC control bit grants AER but
	 * masks DPC.  Link DPC control to AER control, as PCIe r7.0 sec
	 * 6.2.11 recommends.
	 */

> 
>> +	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!


> Thanks,
> Yazen

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer