Re: [PATCH V1] PCI/ASPM: Update saved buffers with latest ASPM configuration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/25/2023 6:22 PM, Vidya Sagar wrote:


On 1/25/2023 8:31 PM, Wysocki, Rafael J wrote:
External email: Use caution opening links or attachments


On 1/25/2023 2:38 PM, Vidya Sagar wrote:
Many PCIe device drivers save the configuration state of their respective
devices during probe and restore the same when their 'slot_reset' hook
is called through PCIe Error Recovery System.
If the system has a change in ASPM policy after the driver's probe is
called and before error event occurred, 'slot_reset' hook restores the
PCIe configuration state to what it was at the time of probe but not with
what it was just before the occurrence of the error event.
This effectively leads to a mismatch in the ASPM configuration between
the device and its upstream parent device.
This patch addresses that issue by updating the saved configuration state of the device with the latest info whenever there is a change w.r.t ASPM
policy.

Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>

If it is a bug fix (which I think it is), a Fixes tag should be present
here.

It is kind of a bug fix but I couldn't pin point to any particular commit that would have introduced it.


If the reporter's names are known, Reported-by tags should be present
here too.

I was experimenting with the error handling code and happen to find this.


If anyone except for you has tested this patch, a Tested-by tag should
be present here.

Only I tested this patch for now. It would be great if more verification is done on this patch.

Fair enough.

Please feel free to add

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

to this patch.




---
  drivers/pci/pci.h       |  4 ++++
  drivers/pci/pcie/aspm.c | 40 ++++++++++++++++++++++++++++++++++++++++
  2 files changed, 44 insertions(+)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9ed3b5550043..f4a91d4fe96d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -566,12 +566,16 @@ bool pcie_wait_for_link(struct pci_dev *pdev, bool active);
  void pcie_aspm_init_link_state(struct pci_dev *pdev);
  void pcie_aspm_exit_link_state(struct pci_dev *pdev);
  void pcie_aspm_powersave_config_link(struct pci_dev *pdev);
+void pci_save_aspm_state(struct pci_dev *dev);
+void pci_restore_aspm_state(struct pci_dev *dev);
  void pci_save_aspm_l1ss_state(struct pci_dev *dev);
  void pci_restore_aspm_l1ss_state(struct pci_dev *dev);
  #else
  static inline void pcie_aspm_init_link_state(struct pci_dev *pdev) { }   static inline void pcie_aspm_exit_link_state(struct pci_dev *pdev) { }   static inline void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { }
+static inline void pci_save_aspm_state(struct pci_dev *dev) { }
+static inline void pci_restore_aspm_state(struct pci_dev *dev) { }
  static inline void pci_save_aspm_l1ss_state(struct pci_dev *dev) { }
  static inline void pci_restore_aspm_l1ss_state(struct pci_dev *dev) { }
  #endif
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 53a1fa306e1e..f25e0440d36b 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -151,6 +151,7 @@ static void pcie_set_clkpm_nocheck(struct pcie_link_state *link, int enable)
PCI_EXP_LNKCTL_CLKREQ_EN,
                                                 val);
      link->clkpm_enabled = !!enable;
+     pci_save_aspm_state(child);
  }

  static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
@@ -757,6 +758,39 @@ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
                              PCI_L1SS_CTL1_L1SS_MASK, val);
  }

+void pci_save_aspm_state(struct pci_dev *dev)
+{
+     int i = 0;
+     struct pci_cap_saved_state *save_state;
+     u16 *cap;
+
+     if (!pci_is_pcie(dev))
+             return;
+
+     save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
+     if (!save_state)
+             return;
+
+     cap = (u16 *)&save_state->cap.data[0];
+     i++;
+     pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &cap[i++]);
+}
+
+void pci_restore_aspm_state(struct pci_dev *dev)
+{
+     int i = 0;
+     struct pci_cap_saved_state *save_state;
+     u16 *cap;
+
+     save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
+     if (!save_state)
+             return;
+
+     cap = (u16 *)&save_state->cap.data[0];
+     i++;
+     pcie_capability_write_word(dev, PCI_EXP_LNKCTL, cap[i++]);
+}
+
  void pci_save_aspm_l1ss_state(struct pci_dev *dev)
  {
      struct pci_cap_saved_state *save_state;
@@ -849,6 +883,12 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
              pcie_config_aspm_dev(parent, upstream);

      link->aspm_enabled = state;
+
+     /* Update latest ASPM configuration in saved context */
+     pci_save_aspm_state(link->downstream);
+     pci_save_aspm_l1ss_state(link->downstream);
+     pci_save_aspm_state(parent);
+     pci_save_aspm_l1ss_state(parent);
  }

  static void pcie_config_aspm_path(struct pcie_link_state *link)



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux