| Message ID | 20240808121708.2523-1-ilpo.jarvinen@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3e40aa29d47e231a54640addf6a09c1f64c5b63f |
| Headers | show |
| Series | [1/1] PCI: Wait for Link before restoring Downstream Buses | expand |
On Thu, Aug 08, 2024 at 03:17:07PM +0300, Ilpo Järvinen wrote: > __pci_reset_bus() calls pci_bridge_secondary_bus_reset() to perform the > reset and also waits for the Secondary Bus to become again accessible. > __pci_reset_bus() then calls pci_bus_restore_locked() that restores the > PCI devices connected to the bus, and if necessary, recursively restores > also the subordinate buses and their devices. > > The logic in pci_bus_restore_locked() does not take into account that > after restoring a device on one level, there might be another Link > Downstream that can only start to come up after restore has been > performed for its Downstream Port device. That is, the Link may > require additional wait until it becomes accessible. > > Similarly, pci_slot_restore_locked() lacks wait. > > Amend pci_bus_restore_locked() and pci_slot_restore_locked() to wait > for the Secondary Bus before recursively performing the restore of that > bus. > > Fixes: 090a3c5322e9 ("PCI: Add pci_reset_slot() and pci_reset_bus()") > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Seems reasonable to me, applied to pci/reset for v6.12, thanks! > --- > > NOTE TO MAINTAINER: I've not seen anything to actually trigger this issue > but only realized this problem exist while looking into the other issues > related to bus reset/restore. The fix regardless seems to make sense so > sending it out. > > drivers/pci/pci.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index e3a49f66982d..98c7b732998a 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -5671,8 +5671,10 @@ static void pci_bus_restore_locked(struct pci_bus *bus) > > list_for_each_entry(dev, &bus->devices, bus_list) { > pci_dev_restore(dev); > - if (dev->subordinate) > + if (dev->subordinate) { > + pci_bridge_wait_for_secondary_bus(dev, "bus reset"); > pci_bus_restore_locked(dev->subordinate); > + } > } > } > > @@ -5706,8 +5708,10 @@ static void pci_slot_restore_locked(struct pci_slot *slot) > if (!dev->slot || dev->slot != slot) > continue; > pci_dev_restore(dev); > - if (dev->subordinate) > + if (dev->subordinate) { > + pci_bridge_wait_for_secondary_bus(dev, "slot reset"); > pci_bus_restore_locked(dev->subordinate); > + } > } > } > > -- > 2.39.2 >
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e3a49f66982d..98c7b732998a 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5671,8 +5671,10 @@ static void pci_bus_restore_locked(struct pci_bus *bus) list_for_each_entry(dev, &bus->devices, bus_list) { pci_dev_restore(dev); - if (dev->subordinate) + if (dev->subordinate) { + pci_bridge_wait_for_secondary_bus(dev, "bus reset"); pci_bus_restore_locked(dev->subordinate); + } } } @@ -5706,8 +5708,10 @@ static void pci_slot_restore_locked(struct pci_slot *slot) if (!dev->slot || dev->slot != slot) continue; pci_dev_restore(dev); - if (dev->subordinate) + if (dev->subordinate) { + pci_bridge_wait_for_secondary_bus(dev, "slot reset"); pci_bus_restore_locked(dev->subordinate); + } } }
__pci_reset_bus() calls pci_bridge_secondary_bus_reset() to perform the reset and also waits for the Secondary Bus to become again accessible. __pci_reset_bus() then calls pci_bus_restore_locked() that restores the PCI devices connected to the bus, and if necessary, recursively restores also the subordinate buses and their devices. The logic in pci_bus_restore_locked() does not take into account that after restoring a device on one level, there might be another Link Downstream that can only start to come up after restore has been performed for its Downstream Port device. That is, the Link may require additional wait until it becomes accessible. Similarly, pci_slot_restore_locked() lacks wait. Amend pci_bus_restore_locked() and pci_slot_restore_locked() to wait for the Secondary Bus before recursively performing the restore of that bus. Fixes: 090a3c5322e9 ("PCI: Add pci_reset_slot() and pci_reset_bus()") Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> --- NOTE TO MAINTAINER: I've not seen anything to actually trigger this issue but only realized this problem exist while looking into the other issues related to bus reset/restore. The fix regardless seems to make sense so sending it out. drivers/pci/pci.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)