From aeae4f3e5c38d47bdaef50446dc0ec857307df68 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Tue, 4 Sep 2018 12:34:18 -0500 Subject: [PATCH 1/4] PCI/ASPM: Fix link_state teardown on device removal Upon removal of the last device on a bus, the link_state of the bridge leading to that bus is sought to be torn down by having pci_stop_dev() call pcie_aspm_exit_link_state(). When ASPM was originally introduced by commit 7d715a6c1ae5 ("PCI: add PCI Express ASPM support"), it determined whether the device being removed is the last one by calling list_empty() on the bridge's subordinate devices list. That didn't work because the device is only removed from the list slightly later in pci_destroy_dev(). Commit 3419c75e15f8 ("PCI: properly clean up ASPM link state on device remove") attempted to fix it by calling list_is_last(), but that's not correct either because it checks whether the device is at the *end* of the list, not whether it's the last one *left* in the list. If the user removes the device which happens to be at the end of the list via sysfs but other devices are preceding the device in the list, the link_state is torn down prematurely. The real fix is to move the invocation of pcie_aspm_exit_link_state() to pci_destroy_dev() and reinstate the call to list_empty(). Remove a duplicate check for dev->bus->self because pcie_aspm_exit_link_state() already contains an identical check. Fixes: 7d715a6c1ae5 ("PCI: add PCI Express ASPM support") Signed-off-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Cc: Shaohua Li Cc: stable@vger.kernel.org # v2.6.26 --- drivers/pci/pcie/aspm.c | 2 +- drivers/pci/remove.c | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 5326916715d20..f78860ce884bc 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -991,7 +991,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev) * All PCIe functions are in one slot, remove one function will remove * the whole slot, so just wait until we are the last function left. */ - if (!list_is_last(&pdev->bus_list, &parent->subordinate->devices)) + if (!list_empty(&parent->subordinate->devices)) goto out; link = parent->link_state; diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c index 461e7fd2756fb..e9c6b120cf451 100644 --- a/drivers/pci/remove.c +++ b/drivers/pci/remove.c @@ -25,9 +25,6 @@ static void pci_stop_dev(struct pci_dev *dev) pci_dev_assign_added(dev, false); } - - if (dev->bus->self) - pcie_aspm_exit_link_state(dev); } static void pci_destroy_dev(struct pci_dev *dev) @@ -41,6 +38,7 @@ static void pci_destroy_dev(struct pci_dev *dev) list_del(&dev->bus_list); up_write(&pci_bus_sem); + pcie_aspm_exit_link_state(dev); pci_bridge_d3_update(dev); pci_free_resources(dev); put_device(&dev->dev); From 1ad61b612b95980a4d970c52022aa01dfc0f6068 Mon Sep 17 00:00:00 2001 From: Sinan Kaya Date: Fri, 10 Aug 2018 04:32:11 +0000 Subject: [PATCH 2/4] PCI/ACPI: Correct error message for ASPM disabling If _OSC execution fails today for platforms without an _OSC entry, code is printing a misleading message saying disabling ASPM as follows: acpi PNP0A03:00: _OSC failed (AE_NOT_FOUND); disabling ASPM We need to ensure that platform supports ASPM to begin with. Reported-by: Michael Kelley Signed-off-by: Sinan Kaya Signed-off-by: Bjorn Helgaas --- drivers/acpi/pci_root.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index 7433035ded955..e465e720eab20 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -455,8 +455,9 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm) decode_osc_support(root, "OS supports", support); status = acpi_pci_osc_support(root, support); if (ACPI_FAILURE(status)) { - dev_info(&device->dev, "_OSC failed (%s); disabling ASPM\n", - acpi_format_exception(status)); + dev_info(&device->dev, "_OSC failed (%s)%s\n", + acpi_format_exception(status), + pcie_aspm_support_enabled() ? "; disabling ASPM" : ""); *no_aspm = 1; return; } From c238252f86c1833fc0cbee3ce3b4c295d3ab24ef Mon Sep 17 00:00:00 2001 From: Sinan Kaya Date: Fri, 10 Aug 2018 04:32:12 +0000 Subject: [PATCH 3/4] PCI/ACPI: Allow _OSC presence to be optional for PCI The PCI Firmware Spec, r3.2, sec 4.5.1, says: For a host bridge device that originates a PCI Express hierarchy, the _OSC interface defined in this section is required. For a host bridge device that originates a PCI/PCI-X bus hierarchy, inclusion of an _OSC object is optional. Allow PCI host bridges to bail out silently if _OSC is not found. Reported-by: Michael Kelley Signed-off-by: Sinan Kaya [bhelgaas: cite PCI Firmware spec, the authoritative source] Signed-off-by: Bjorn Helgaas --- drivers/acpi/pci_root.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index e465e720eab20..707aafc7c2aa9 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -421,7 +421,8 @@ acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 req) } EXPORT_SYMBOL(acpi_pci_osc_control_set); -static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm) +static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm, + bool is_pcie) { u32 support, control, requested; acpi_status status; @@ -455,10 +456,15 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm) decode_osc_support(root, "OS supports", support); status = acpi_pci_osc_support(root, support); if (ACPI_FAILURE(status)) { + *no_aspm = 1; + + /* _OSC is optional for PCI host bridges */ + if ((status == AE_NOT_FOUND) && !is_pcie) + return; + dev_info(&device->dev, "_OSC failed (%s)%s\n", acpi_format_exception(status), pcie_aspm_support_enabled() ? "; disabling ASPM" : ""); - *no_aspm = 1; return; } @@ -534,6 +540,7 @@ static int acpi_pci_root_add(struct acpi_device *device, acpi_handle handle = device->handle; int no_aspm = 0; bool hotadd = system_state == SYSTEM_RUNNING; + bool is_pcie; root = kzalloc(sizeof(struct acpi_pci_root), GFP_KERNEL); if (!root) @@ -591,7 +598,8 @@ static int acpi_pci_root_add(struct acpi_device *device, root->mcfg_addr = acpi_pci_root_get_mcfg_addr(handle); - negotiate_os_control(root, &no_aspm); + is_pcie = strcmp(acpi_device_hid(device), "PNP0A08") == 0; + negotiate_os_control(root, &no_aspm, is_pcie); /* * TBD: Need PCI interface for enumeration/configuration of roots. From 17c91487364fb33797ed84022564ee7544ac4945 Mon Sep 17 00:00:00 2001 From: Patrick Talbert Date: Wed, 5 Sep 2018 09:12:53 +0200 Subject: [PATCH 4/4] PCI/ASPM: Do not initialize link state when aspm_disabled is set Now that ASPM is configured for *all* PCIe devices at boot, a problem is seen with systems that set the FADT NO_ASPM bit. This bit indicates that the OS should not alter the ASPM state, but when pcie_aspm_init_link_state() runs it only checks for !aspm_support_enabled. This misses the ACPI_FADT_NO_ASPM case because that is setting aspm_disabled. The result is systems may hang at boot after 1302fcf; avoidable if they boot with pcie_aspm=off (sets !aspm_support_enabled). Fix this by having aspm_init_link_state() check for either !aspm_support_enabled or acpm_disabled. Link: https://bugzilla.kernel.org/show_bug.cgi?id=201001 Fixes: 1302fcf0d03e ("PCI: Configure *all* devices, not just hot-added ones") Signed-off-by: Patrick Talbert Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aspm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index f78860ce884bc..dcb29cb76dc69 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -895,7 +895,7 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev) struct pcie_link_state *link; int blacklist = !!pcie_aspm_sanity_check(pdev); - if (!aspm_support_enabled) + if (!aspm_support_enabled || aspm_disabled) return; if (pdev->link_state)