@@ -79,7 +79,7 @@ void pci_configure_aspm_l1ss(struct pci_dev *pdev)
ERR_PTR(rc));
}
-void pci_save_aspm_l1ss_state(struct pci_dev *pdev)
+static void __pci_save_aspm_l1ss_state(struct pci_dev *pdev)
{
struct pci_cap_saved_state *save_state;
u16 l1ss = pdev->l1ss;
@@ -101,6 +101,22 @@ void pci_save_aspm_l1ss_state(struct pci_dev *pdev)
pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, cap++);
}
+void pci_save_aspm_l1ss_state(struct pci_dev *pdev)
+{
+ struct pci_dev *parent = pdev->bus->self;
+
+ __pci_save_aspm_l1ss_state(pdev);
+
+ /*
+ * Save parent's L1 substate configuration, if the parent has not saved
+ * state. It avoids pci_restore_aspm_l1ss_state() restore wrong value to
+ * parent's L1 substate configuration. However, the parent might be
+ * nothing, if pdev is a PCI bridge.
+ */
+ if (parent && !parent->state_saved)
+ __pci_save_aspm_l1ss_state(parent);
+}
+
void pci_restore_aspm_l1ss_state(struct pci_dev *pdev)
{
struct pci_cap_saved_state *pl_save_state, *cl_save_state;
PCI devices' parameters on the VMD bus have been programmed properly originally. But, cleared after pci_reset_bus() and have not been restored correctly. This leads the link's L1.2 between PCIe Root Port and child device gets wrong configs. Here is a failed example on ASUS B1400CEAE with enabled VMD. Both PCIe bridge and NVMe device should have the same LTR1.2_Threshold value. However, they are configured as different values in this case: 10000:e0:06.0 PCI bridge [0604]: Intel Corporation 11th Gen Core Processor PCIe Controller [8086:9a09] (rev 01) (prog-if 00 [Normal decode]) ... Capabilities: [200 v1] L1 PM Substates L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+ PortCommonModeRestoreTime=45us PortTPowerOnTime=50us L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1- T_CommonMode=0us LTR1.2_Threshold=0ns L1SubCtl2: T_PwrOn=0us 10000:e1:00.0 Non-Volatile memory controller [0108]: Sandisk Corp WD Blue SN550 NVMe SSD [15b7:5009] (rev 01) (prog-if 02 [NVM Express]) ... Capabilities: [900 v1] L1 PM Substates L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1- L1_PM_Substates+ PortCommonModeRestoreTime=32us PortTPowerOnTime=10us L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1- T_CommonMode=0us LTR1.2_Threshold=101376ns L1SubCtl2: T_PwrOn=50us Here is VMD mapped PCI device tree: -+-[0000:00]-+-00.0 Intel Corporation Device 9a04 | ... \-[10000:e0]-+-06.0-[e1]----00.0 Sandisk Corp WD Blue SN550 NVMe SSD \-17.0 Intel Corporation Tiger Lake-LP SATA Controller When pci_reset_bus() resets the bus [e1] of the NVMe, it only saves and restores NVMe's state before and after reset. Then, when it restores the NVMe's state, ASPM code restores L1SS for both the parent bridge and the NVMe in pci_restore_aspm_l1ss_state(). The NVMe's L1SS is restored correctly. But, the parent bridge's L1SS is restored with a wrong value 0x0 because the parent bridge's L1SS wasn't saved by pci_save_aspm_l1ss_state() before reset. To avoid pci_restore_aspm_l1ss_state() restore wrong value to the parent's L1SS config like this example, make pci_save_aspm_l1ss_state() save the parent's L1SS config, if the PCI device has a parent. Link: https://lore.kernel.org/linux-pci/CAPpJ_eexU0gCHMbXw_z924WxXw0+B6SdS4eG9oGpEX1wmnMLkQ@mail.gmail.com/ Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218394 Fixes: 17423360a27a ("PCI/ASPM: Save L1 PM Substates Capability for suspend/resume") Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Jian-Hong Pan <jhp@endlessos.org> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Reviewed-by: David E. Box <david.e.box@linux.intel.com> --- v9: - Drop the v8 fix about drivers/pci/pcie/aspm.c. Use this in VMD instead. v10: - Drop the v9 fix about drivers/pci/controller/vmd.c - Fix in PCIe ASPM to make it symmetric between pci_save_aspm_l1ss_state() and pci_restore_aspm_l1ss_state() v11: - Introduce __pci_save_aspm_l1ss_state as a resusable helper function which is same as the original pci_configure_aspm_l1ss - Make pci_save_aspm_l1ss_state invoke __pci_save_aspm_l1ss_state for both child and parent devices - Smooth the commit message v12: - Update the commit message v13: - Tweak the commit message to make it more like a general fix - When pci_alloc_dev() prepares the pci_dev, it sets the pci_dev's bus. So, let pci_save_aspm_l1ss_state() access pdev's bus directly. - Add comment in pci_save_aspm_l1ss_state() to describe why it does not save both the PCIe device and the parent's L1SS config like pci_restore_aspm_l1ss_state() directly. drivers/pci/pcie/aspm.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)