In commit f492edb40b54 ("PCI: vmd: Add quirk to configure PCIe ASPM and
LTR") the VMD driver calls pci_enabled_link_state as a callback from
pci_bus_walk. Both will acquire the pci_bus_sem lock leading to a lockdep
warning. Instead of doing the pci_bus_walk, move the fix to quirks.c using
DECLARE_PCI_FIXUP_FINAL.

Fixes: f492edb40b54 ("PCI: vmd: Add quirk to configure PCIe ASPM and LTR")
Suggested-by: Bjorn Helgaas <bhelg...@google.com>
Signed-off-by: David E. Box <david.e....@linux.intel.com>
---

V2 - Instead of adding a lock flag argument to pci_enabled_link_state, move
     the fix to quirks.c

 drivers/pci/controller/vmd.c | 55 +--------------------------
 drivers/pci/quirks.c         | 72 ++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 54 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 990630ec57c6..47fa3e5f2dc5 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -66,22 +66,11 @@ enum vmd_features {
         * interrupt handling.
         */
        VMD_FEAT_CAN_BYPASS_MSI_REMAP           = (1 << 4),
-
-       /*
-        * Enable ASPM on the PCIE root ports and set the default LTR of the
-        * storage devices on platforms where these values are not configured by
-        * BIOS. This is needed for laptops, which require these settings for
-        * proper power management of the SoC.
-        */
-       VMD_FEAT_BIOS_PM_QUIRK          = (1 << 5),
 };
 
-#define VMD_BIOS_PM_QUIRK_LTR  0x1003  /* 3145728 ns */
-
 #define VMD_FEATS_CLIENT       (VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |     \
                                 VMD_FEAT_HAS_BUS_RESTRICTIONS |        \
-                                VMD_FEAT_OFFSET_FIRST_VECTOR |         \
-                                VMD_FEAT_BIOS_PM_QUIRK)
+                                VMD_FEAT_OFFSET_FIRST_VECTOR)
 
 static DEFINE_IDA(vmd_instance_ida);
 
@@ -724,46 +713,6 @@ static void vmd_copy_host_bridge_flags(struct 
pci_host_bridge *root_bridge,
        vmd_bridge->native_dpc = root_bridge->native_dpc;
 }
 
-/*
- * Enable ASPM and LTR settings on devices that aren't configured by BIOS.
- */
-static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
-{
-       unsigned long features = *(unsigned long *)userdata;
-       u16 ltr = VMD_BIOS_PM_QUIRK_LTR;
-       u32 ltr_reg;
-       int pos;
-
-       if (!(features & VMD_FEAT_BIOS_PM_QUIRK))
-               return 0;
-
-       pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL);
-
-       pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
-       if (!pos)
-               return 0;
-
-       /*
-        * Skip if the max snoop LTR is non-zero, indicating BIOS has set it
-        * so the LTR quirk is not needed.
-        */
-       pci_read_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, &ltr_reg);
-       if (!!(ltr_reg & (PCI_LTR_VALUE_MASK | PCI_LTR_SCALE_MASK)))
-               return 0;
-
-       /*
-        * Set the default values to the maximum required by the platform to
-        * allow the deepest power management savings. Write as a DWORD where
-        * the lower word is the max snoop latency and the upper word is the
-        * max non-snoop latency.
-        */
-       ltr_reg = (ltr << 16) | ltr;
-       pci_write_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, ltr_reg);
-       pci_info(pdev, "VMD: Default LTR value set by driver\n");
-
-       return 0;
-}
-
 static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 {
        struct pci_sysdata *sd = &vmd->sysdata;
@@ -936,8 +885,6 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned 
long features)
 
        pci_assign_unassigned_bus_resources(vmd->bus);
 
-       pci_walk_bus(vmd->bus, vmd_pm_enable_quirk, &features);
-
        /*
         * VMD root buses are virtual and don't return true on pci_is_pcie()
         * and will fail pcie_bus_configure_settings() early. It can instead be
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 44cab813bf95..2d86623f96e3 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -6023,3 +6023,75 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x9a2d, 
dpc_log_size);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x9a2f, dpc_log_size);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x9a31, dpc_log_size);
 #endif
+
+#ifdef CONFIG_VMD
+/*
+ * Enable ASPM on the PCIE root ports under VMD and set the default LTR of the
+ * storage devices on platforms where these values are not configured by BIOS.
+ * This is needed for laptops, which require these settings for proper power
+ * management of the SoC.
+ */
+#define VMD_DEVICE_LTR 0x1003  /* 3145728 ns */
+static void quirk_intel_vmd(struct pci_dev *pdev)
+{
+       struct pci_dev *parent;
+       u16 ltr = VMD_DEVICE_LTR;
+       u32 ltr_reg;
+       int pos;
+
+       /* Check in VMD domain */
+       if (pci_domain_nr(pdev->bus) < 0x10000)
+               return;
+
+       /* Get Root Port */
+       parent = pci_upstream_bridge(pdev);
+       if (!parent || parent->vendor != PCI_VENDOR_ID_INTEL)
+               return;
+
+       /* Get VMD Host Bridge */
+       parent = to_pci_dev(parent->dev.parent);
+       if (!parent)
+               return;
+
+       /* Get RAID controller */
+       parent = to_pci_dev(parent->dev.parent);
+       if (!parent)
+               return;
+
+       switch (parent->device) {
+       case 0x467f:
+       case 0x4c3d:
+       case 0xa77f:
+       case 0x7d0b:
+       case 0xad0b:
+       case 0x9a0b:
+               break;
+       default:
+               return;
+       }
+
+       pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL);
+
+       pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
+       if (!pos)
+               return;
+
+       /* Skip if the max snoop LTR is non-zero, indicating BIOS has set it */
+       pci_read_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, &ltr_reg);
+       if (!!(ltr_reg & (PCI_LTR_VALUE_MASK | PCI_LTR_SCALE_MASK)))
+               return;
+
+       /*
+        * Set the LTR values to the maximum required by the platform to
+        * allow the deepest power management savings. Write as a DWORD where
+        * the lower word is the max snoop latency and the upper word is the
+        * max non-snoop latency.
+        */
+       ltr_reg = (ltr << 16) | ltr;
+       pci_write_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, ltr_reg);
+       pci_info(pdev, "LTR set by VMD PCI quick\n");
+
+}
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
+                             PCI_CLASS_STORAGE_EXPRESS, 0, quirk_intel_vmd);
+#endif
-- 
2.34.1

Reply via email to