PCI: Do not wait for disconnected devices when resuming
authorIlpo Järvinen <ilpo.jarvinen@linux.intel.com>
Thu, 8 Feb 2024 13:23:21 +0000 (15:23 +0200)
committerBjorn Helgaas <bhelgaas@google.com>
Thu, 16 May 2024 19:35:09 +0000 (14:35 -0500)
On runtime resume, pci_dev_wait() is called:

  pci_pm_runtime_resume()
    pci_pm_bridge_power_up_actions()
      pci_bridge_wait_for_secondary_bus()
        pci_dev_wait()

While a device is runtime suspended along with its PCI hierarchy, the
device could get disconnected. In such case, the link will not come up no
matter how long pci_dev_wait() waits for it.

Besides the above mentioned case, there could be other ways to get the
device disconnected while pci_dev_wait() is waiting for the link to come
up.

Make pci_dev_wait() exit if the device is already disconnected to avoid
unnecessary delay.

The use cases of pci_dev_wait() boil down to two:

  1. Waiting for the device after reset
  2. pci_bridge_wait_for_secondary_bus()

The callers in both cases seem to benefit from propagating the
disconnection as error even if device disconnection would be more
analoguous to the case where there is no device in the first place which
return 0 from pci_dev_wait(). In the case 2, it results in unnecessary
marking of the devices disconnected again but that is just harmless extra
work.

Also make sure compiler does not become too clever with dev->error_state
and use READ_ONCE() to force a fetch for the up-to-date value.

Link: https://lore.kernel.org/r/20240208132322.4811-1-ilpo.jarvinen@linux.intel.com
Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
drivers/pci/pci.c
include/linux/pci.h

index d3fabfca7e6f6e0b880b7aaaab2d71001c88a746..a9848316c623ff513892c8546d2f1667eb981642 100644 (file)
@@ -1277,6 +1277,11 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
        for (;;) {
                u32 id;
 
+               if (pci_dev_is_disconnected(dev)) {
+                       pci_dbg(dev, "disconnected; not waiting\n");
+                       return -ENOTTY;
+               }
+
                pci_read_config_dword(dev, PCI_COMMAND, &id);
                if (!PCI_POSSIBLE_ERROR(id))
                        break;
index 69b10f2fb6067bf4984f704c945e48c88a7c94cc..2151c7f9be864439436d37e0662dfea6b3e077b0 100644 (file)
@@ -2514,7 +2514,12 @@ static inline struct pci_dev *pcie_find_root_port(struct pci_dev *dev)
 
 static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
 {
-       return dev->error_state == pci_channel_io_perm_failure;
+       /*
+        * error_state is set in pci_dev_set_io_state() using xchg/cmpxchg()
+        * and read w/o common lock. READ_ONCE() ensures compiler cannot cache
+        * the value (e.g. inside the loop in pci_dev_wait()).
+        */
+       return READ_ONCE(dev->error_state) == pci_channel_io_perm_failure;
 }
 
 void pci_request_acs(void);