Re: [PATCH v12 8/8] PCI/ACPI: Enable EDR support

[Kuppuswamy Sathyanarayanan]

On 1/17/20 12:41 PM, Bjorn Helgaas wrote:
> On Sun, Jan 12, 2020 at 02:44:02PM -0800, sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx wrote:
> > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> >
> > As per PCI firmware specification r3.2 Downstream Port Containment
> > Related Enhancements ECN, sec 4.5.1, OS must implement following steps
> > to enable/use EDR feature.
> >
> > 1. OS can use bit 7 of _OSC Control Field to negotiate control over
> > Downstream Port Containment (DPC) configuration of PCIe port. After _OSC
> > negotiation, firmware will Set this bit to grant OS control over PCIe
> > DPC configuration and Clear it if this feature was requested and denied,
> > or was not requested.
> >
> > 2. Also, if OS supports EDR, it should expose its support to BIOS by
> > setting bit 7 of _OSC Support Field. And if OS sets bit 7 of _OSC
> > Control Field it must also expose support for EDR by setting bit 7 of
> > _OSC Support Field.
> > --- a/drivers/pci/pcie/portdrv_core.c
> > +++ b/drivers/pci/pcie/portdrv_core.c
> > @@ -253,10 +253,13 @@ static int get_port_device_capability(struct pci_dev *dev)
> >   	/*
> >   	 * With dpc-native, allow Linux to use DPC even if it doesn't have
> >   	 * permission to use AER.
> > +	 * If EDR support is enabled in OS, then even if AER is not handled in
> > +	 * OS, DPC service can be enabled.
> Can you clarify this comment?  It talks about AER, but the code you
> added:
Previously with condition
(pci_aer_available() && (pcie_ports_dpc_native || (services & 
PCIE_PORT_SERVICE_AER)))
we have only checked whether AER is enabled and supported before 
enabling DPC.

But with EDR support, we enumerate DPC driver even if AER is not 
supported or handled
in OS. And the or condition (IS_ENABLED(CONFIG_PCIE_EDR) && 
!host->native_dpc) adds this
support.


>    (IS_ENABLED(CONFIG_PCIE_EDR) && !host->native_dpc)
>
> doesn't have anything to do with AER.
>
> >   	 */
> >   	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> > -	    pci_aer_available() &&
> > -	    (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
> > +	    ((IS_ENABLED(CONFIG_PCIE_EDR) && !host->native_dpc) ||
> > +	    (pci_aer_available() &&
> > +	    (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))))
> >   		services |= PCIE_PORT_SERVICE_DPC;

--
Sathyanarayanan Kuppuswamy
Linux kernel developer