Skip to content

Commit

Permalink
PCI/AER: Block runtime suspend when handling errors
Browse files Browse the repository at this point in the history
[ Upstream commit 002bf2f ]

PM runtime can be done simultaneously with AER error handling.  Avoid that
by using pm_runtime_get_sync() before and pm_runtime_put() after reset in
pcie_do_recovery() for all recovering devices.

pm_runtime_get_sync() will increase dev->power.usage_count counter to
prevent any possible future request to runtime suspend a device.  It will
also resume a device, if it was previously in D3hot state.

I tested with igc device by doing simultaneous aer_inject and rpm
suspend/resume via /sys/bus/pci/devices/PCI_ID/power/control and can
reproduce:

  igc 0000:02:00.0: not ready 65535ms after bus reset; giving up
  pcieport 0000:00:1c.2: AER: Root Port link has been reset (-25)
  pcieport 0000:00:1c.2: AER: subordinate device reset failed
  pcieport 0000:00:1c.2: AER: device recovery failed
  igc 0000:02:00.0: Unable to change power state from D3hot to D0, device inaccessible

The problem disappears when this patch is applied.

Link: https://lore.kernel.org/r/20240212120135.146068-1-stanislaw.gruszka@linux.intel.com
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael@kernel.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
sgruszka authored and gregkh committed Apr 3, 2024
1 parent e88b5ae commit 12a5790
Showing 1 changed file with 20 additions and 0 deletions.
20 changes: 20 additions & 0 deletions drivers/pci/pcie/err.c
Expand Up @@ -13,6 +13,7 @@
#define dev_fmt(fmt) "AER: " fmt

#include <linux/pci.h>
#include <linux/pm_runtime.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/errno.h>
Expand Down Expand Up @@ -85,6 +86,18 @@ static int report_error_detected(struct pci_dev *dev,
return 0;
}

static int pci_pm_runtime_get_sync(struct pci_dev *pdev, void *data)
{
pm_runtime_get_sync(&pdev->dev);
return 0;
}

static int pci_pm_runtime_put(struct pci_dev *pdev, void *data)
{
pm_runtime_put(&pdev->dev);
return 0;
}

static int report_frozen_detected(struct pci_dev *dev, void *data)
{
return report_error_detected(dev, pci_channel_io_frozen, data);
Expand Down Expand Up @@ -207,6 +220,8 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
else
bridge = pci_upstream_bridge(dev);

pci_walk_bridge(bridge, pci_pm_runtime_get_sync, NULL);

pci_dbg(bridge, "broadcast error_detected message\n");
if (state == pci_channel_io_frozen) {
pci_walk_bridge(bridge, report_frozen_detected, &status);
Expand Down Expand Up @@ -251,10 +266,15 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
pcie_clear_device_status(dev);
pci_aer_clear_nonfatal_status(dev);
}

pci_walk_bridge(bridge, pci_pm_runtime_put, NULL);

pci_info(bridge, "device recovery successful\n");
return status;

failed:
pci_walk_bridge(bridge, pci_pm_runtime_put, NULL);

pci_uevent_ers(bridge, PCI_ERS_RESULT_DISCONNECT);

/* TODO: Should kernel panic here? */
Expand Down

0 comments on commit 12a5790

Please sign in to comment.