[2/2] PCI/DPC: Do not enable DPC if AER control is not allowed by the BIOS

Message ID 20180314114125.71132-2-mika.westerberg@linux.intel.com
State Not Applicable
Delegated to: Bjorn Helgaas
Headers show
Series
  • [1/2] PCI/DPC: Disable interrupt generation during suspend
Related show

Commit Message

Mika Westerberg March 14, 2018, 11:41 a.m.
Commit eed85ff4c0da ("PCI/DPC: Enable DPC only if AER is available")
made DPC control dependent whether AER is enabled in the OS. However, it
does not take into account situations where BIOS has not given OS
control of AER:

  acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI]
  acpi PNP0A08:00: _OSC: platform does not support [AER]
  acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME PCIeCapability]

I think here it is better not to enable DPC even if the capability is
available because then it would be against what "Determination of DPC
Control" note in PCIe 4.0 sec 6.1.10 recommends.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/pcie/portdrv_acpi.c | 4 ++--
 drivers/pci/pcie/portdrv_core.c | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

Comments

Rafael J. Wysocki March 14, 2018, 11:50 a.m. | #1
On Wed, Mar 14, 2018 at 12:41 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> Commit eed85ff4c0da ("PCI/DPC: Enable DPC only if AER is available")
> made DPC control dependent whether AER is enabled in the OS. However, it
> does not take into account situations where BIOS has not given OS
> control of AER:
>
>   acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI]
>   acpi PNP0A08:00: _OSC: platform does not support [AER]
>   acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME PCIeCapability]
>
> I think here it is better not to enable DPC even if the capability is
> available because then it would be against what "Determination of DPC
> Control" note in PCIe 4.0 sec 6.1.10 recommends.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

This may clash with the recent PCIe ports initialization rework from
Bjorn, please double check.
Mika Westerberg March 14, 2018, 12:07 p.m. | #2
On Wed, Mar 14, 2018 at 12:50:28PM +0100, Rafael J. Wysocki wrote:
> On Wed, Mar 14, 2018 at 12:41 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > Commit eed85ff4c0da ("PCI/DPC: Enable DPC only if AER is available")
> > made DPC control dependent whether AER is enabled in the OS. However, it
> > does not take into account situations where BIOS has not given OS
> > control of AER:
> >
> >   acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI]
> >   acpi PNP0A08:00: _OSC: platform does not support [AER]
> >   acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME PCIeCapability]
> >
> > I think here it is better not to enable DPC even if the capability is
> > available because then it would be against what "Determination of DPC
> > Control" note in PCIe 4.0 sec 6.1.10 recommends.
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> This may clash with the recent PCIe ports initialization rework from
> Bjorn, please double check.

You are right. I did not notice those changes. I'll rebase this on top
of Bjorn's pci/portdrv branch.

Patch

diff --git a/drivers/pci/pcie/portdrv_acpi.c b/drivers/pci/pcie/portdrv_acpi.c
index 319c94976873..504bedb63f38 100644
--- a/drivers/pci/pcie/portdrv_acpi.c
+++ b/drivers/pci/pcie/portdrv_acpi.c
@@ -48,11 +48,11 @@  void pcie_port_acpi_setup(struct pci_dev *port, int *srv_mask)
 
 	flags = root->osc_control_set;
 
-	*srv_mask = PCIE_PORT_SERVICE_VC | PCIE_PORT_SERVICE_DPC;
+	*srv_mask = PCIE_PORT_SERVICE_VC;
 	if (flags & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)
 		*srv_mask |= PCIE_PORT_SERVICE_HP;
 	if (flags & OSC_PCI_EXPRESS_PME_CONTROL)
 		*srv_mask |= PCIE_PORT_SERVICE_PME;
 	if (flags & OSC_PCI_EXPRESS_AER_CONTROL)
-		*srv_mask |= PCIE_PORT_SERVICE_AER;
+		*srv_mask |= PCIE_PORT_SERVICE_AER | PCIE_PORT_SERVICE_DPC;
 }
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index ef3bad4ad010..2619cd928a5a 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -257,7 +257,8 @@  static int get_port_device_capability(struct pci_dev *dev)
 		 */
 		pcie_pme_interrupt_enable(dev, false);
 	}
-	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC))
+	if ((cap_mask & PCIE_PORT_SERVICE_DPC) &&
+	    pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC))
 		services |= PCIE_PORT_SERVICE_DPC;
 
 	return services;