PCI/bwctrl: Replace lbms_count with PCI_LINK_LBMS_SEEN flag

PCIe BW controller counted LBMS assertions for the purposes of the Target
Speed quirk (pcie_failed_link_retrain()). It was also a plan to expose the
LBMS count through sysfs to allow better diagnosing link related issues.
Lukas Wunner suggested, however, that adding a trace event would be better
for diagnostics purposes, leaving only pcie_failed_link_retrain() as a user
of the lbms_count.

The logic in pcie_failed_link_retrain() does not require keeping count of
LBMS assertions, so replace lbms_count with a simple flag in pci_dev's
priv_flags.  The reduced complexity allows removing pcie_bwctrl_lbms_rwsem.

Since pcie_failed_link_retrain() runs before bwctrl is probed during boot,
the LBMS in Link Status register still has to be checked by the quirk.

The priv_flags numbering is not continuous because hotplug code added a few
flags to fill numbers 4-5 (hotplug and bwctrl changes are routed through in
different branches).

Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
[bhelgaas: commit log]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
[kwilczynski: squashed a fix to resolve build failures from
https://lore.kernel.org/all/20250508090036.1528-1-ilpo.jarvinen@linux.intel.com]
Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
Link: https://patch.msgid.link/20250422115548.1483-1-ilpo.jarvinen@linux.intel.com

diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
index d603a7aa74838c748f6ac2d22ffb8b8cfe64e469..bcc938d4420f3ddb301c1ec6b0bce0d7f9541658 100644 (file)
--- a/drivers/pci/hotplug/pciehp_ctrl.c
+++ b/drivers/pci/hotplug/pciehp_ctrl.c
@@ -131,7 +131,7 @@ static void remove_board(struct controller *ctrl, bool safe_removal)
                              INDICATOR_NOOP);
 
        /* Don't carry LBMS indications across */
-       pcie_reset_lbms_count(ctrl->pcie->port);
+       pcie_reset_lbms(ctrl->pcie->port);
 }
 
 static int pciehp_enable_slot(struct controller *ctrl);

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 4d7c9f64ea24ec754a135a2585c99489cfa641a9..3d94cf33c1b63f08624a6be59ec5e1392a521a2a 100644 (file)
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4757,7 +4757,7 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
         * to track link speed or width changes made by hardware itself
         * in attempt to correct unreliable link operation.
         */
-       pcie_reset_lbms_count(pdev);
+       pcie_reset_lbms(pdev);
        return rc;
 }
 

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index b81e99cd4b62a3022c8b07a09f212f6888674487..887811fbe722129a48834d6e801c6b6cb8f6c876 100644 (file)
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -557,6 +557,7 @@ static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused)
 #define PCI_DPC_RECOVERED 1
 #define PCI_DPC_RECOVERING 2
 #define PCI_DEV_REMOVED 3
+#define PCI_LINK_LBMS_SEEN     6
 
 static inline void pci_dev_assign_added(struct pci_dev *dev)
 {
@@ -824,14 +825,9 @@ static inline void pcie_ecrc_get_policy(char *str) { }
 #endif
 
 #ifdef CONFIG_PCIEPORTBUS
-void pcie_reset_lbms_count(struct pci_dev *port);
-int pcie_lbms_count(struct pci_dev *port, unsigned long *val);
+void pcie_reset_lbms(struct pci_dev *port);
 #else
-static inline void pcie_reset_lbms_count(struct pci_dev *port) {}
-static inline int pcie_lbms_count(struct pci_dev *port, unsigned long *val)
-{
-       return -EOPNOTSUPP;
-}
+static inline void pcie_reset_lbms(struct pci_dev *port) {}
 #endif
 
 struct pci_dev_reset_methods {

diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c
index d8d2aa85a22928b99c5bba1d2bcc5647c0edeeb6..f31fbbd5149008af776029e73f098719d1ae77c5 100644 (file)
--- a/drivers/pci/pcie/bwctrl.c
+++ b/drivers/pci/pcie/bwctrl.c
@@ -38,24 +38,14 @@
 /**
  * struct pcie_bwctrl_data - PCIe bandwidth controller
  * @set_speed_mutex:   Serializes link speed changes
- * @lbms_count:                Count for LBMS (since last reset)
  * @cdev:              Thermal cooling device associated with the port
  */
 struct pcie_bwctrl_data {
        struct mutex set_speed_mutex;
-       atomic_t lbms_count;
        struct thermal_cooling_device *cdev;
 };
 
-/*
- * Prevent port removal during LBMS count accessors and Link Speed changes.
- *
- * These have to be differentiated because pcie_bwctrl_change_speed() calls
- * pcie_retrain_link() which uses LBMS count reset accessor on success
- * (using just one rwsem triggers "possible recursive locking detected"
- * warning).
- */
-static DECLARE_RWSEM(pcie_bwctrl_lbms_rwsem);
+/* Prevent port removal during Link Speed changes. */
 static DECLARE_RWSEM(pcie_bwctrl_setspeed_rwsem);
 
 static bool pcie_valid_speed(enum pci_bus_speed speed)
@@ -202,15 +192,14 @@ int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req,
 
 static void pcie_bwnotif_enable(struct pcie_device *srv)
 {
-       struct pcie_bwctrl_data *data = srv->port->link_bwctrl;
        struct pci_dev *port = srv->port;
        u16 link_status;
        int ret;
 
-       /* Count LBMS seen so far as one */
+       /* Note if LBMS has been seen so far */
        ret = pcie_capability_read_word(port, PCI_EXP_LNKSTA, &link_status);
        if (ret == PCIBIOS_SUCCESSFUL && link_status & PCI_EXP_LNKSTA_LBMS)
-               atomic_inc(&data->lbms_count);
+               set_bit(PCI_LINK_LBMS_SEEN, &port->priv_flags);
 
        pcie_capability_set_word(port, PCI_EXP_LNKCTL,
                                 PCI_EXP_LNKCTL_LBMIE | PCI_EXP_LNKCTL_LABIE);
@@ -233,7 +222,6 @@ static void pcie_bwnotif_disable(struct pci_dev *port)
 static irqreturn_t pcie_bwnotif_irq(int irq, void *context)
 {
        struct pcie_device *srv = context;
-       struct pcie_bwctrl_data *data = srv->port->link_bwctrl;
        struct pci_dev *port = srv->port;
        u16 link_status, events;
        int ret;
@@ -247,7 +235,7 @@ static irqreturn_t pcie_bwnotif_irq(int irq, void *context)
                return IRQ_NONE;
 
        if (events & PCI_EXP_LNKSTA_LBMS)
-               atomic_inc(&data->lbms_count);
+               set_bit(PCI_LINK_LBMS_SEEN, &port->priv_flags);
 
        pcie_capability_write_word(port, PCI_EXP_LNKSTA, events);
 
@@ -262,31 +250,10 @@ static irqreturn_t pcie_bwnotif_irq(int irq, void *context)
        return IRQ_HANDLED;
 }
 
-void pcie_reset_lbms_count(struct pci_dev *port)
-{
-       struct pcie_bwctrl_data *data;
-
-       guard(rwsem_read)(&pcie_bwctrl_lbms_rwsem);
-       data = port->link_bwctrl;
-       if (data)
-               atomic_set(&data->lbms_count, 0);
-       else
-               pcie_capability_write_word(port, PCI_EXP_LNKSTA,
-                                          PCI_EXP_LNKSTA_LBMS);
-}
-
-int pcie_lbms_count(struct pci_dev *port, unsigned long *val)
+void pcie_reset_lbms(struct pci_dev *port)
 {
-       struct pcie_bwctrl_data *data;
-
-       guard(rwsem_read)(&pcie_bwctrl_lbms_rwsem);
-       data = port->link_bwctrl;
-       if (!data)
-               return -ENOTTY;
-
-       *val = atomic_read(&data->lbms_count);
-
-       return 0;
+       clear_bit(PCI_LINK_LBMS_SEEN, &port->priv_flags);
+       pcie_capability_write_word(port, PCI_EXP_LNKSTA, PCI_EXP_LNKSTA_LBMS);
 }
 
 static int pcie_bwnotif_probe(struct pcie_device *srv)
@@ -308,18 +275,16 @@ static int pcie_bwnotif_probe(struct pcie_device *srv)
                return ret;
 
        scoped_guard(rwsem_write, &pcie_bwctrl_setspeed_rwsem) {
-               scoped_guard(rwsem_write, &pcie_bwctrl_lbms_rwsem) {
-                       port->link_bwctrl = data;
-
-                       ret = request_irq(srv->irq, pcie_bwnotif_irq,
-                                         IRQF_SHARED, "PCIe bwctrl", srv);
-                       if (ret) {
-                               port->link_bwctrl = NULL;
-                               return ret;
-                       }
+               port->link_bwctrl = data;
 
-                       pcie_bwnotif_enable(srv);
+               ret = request_irq(srv->irq, pcie_bwnotif_irq,
+                                 IRQF_SHARED, "PCIe bwctrl", srv);
+               if (ret) {
+                       port->link_bwctrl = NULL;
+                       return ret;
                }
+
+               pcie_bwnotif_enable(srv);
        }
 
        pci_dbg(port, "enabled with IRQ %d\n", srv->irq);
@@ -339,13 +304,11 @@ static void pcie_bwnotif_remove(struct pcie_device *srv)
        pcie_cooling_device_unregister(data->cdev);
 
        scoped_guard(rwsem_write, &pcie_bwctrl_setspeed_rwsem) {
-               scoped_guard(rwsem_write, &pcie_bwctrl_lbms_rwsem) {
-                       pcie_bwnotif_disable(srv->port);
+               pcie_bwnotif_disable(srv->port);
 
-                       free_irq(srv->irq, srv);
+               free_irq(srv->irq, srv);
 
-                       srv->port->link_bwctrl = NULL;
-               }
+               srv->port->link_bwctrl = NULL;
        }
 }
 

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 8d610c17e0f2f0d59f54ee3fc16d808f119bfd27..64ac1ee944d3af5a12ab19c2228b4cd78d98444c 100644 (file)
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -38,14 +38,10 @@
 
 static bool pcie_lbms_seen(struct pci_dev *dev, u16 lnksta)
 {
-       unsigned long count;
-       int ret;
-
-       ret = pcie_lbms_count(dev, &count);
-       if (ret < 0)
-               return lnksta & PCI_EXP_LNKSTA_LBMS;
+       if (test_bit(PCI_LINK_LBMS_SEEN, &dev->priv_flags))
+               return true;
 
-       return count > 0;
+       return lnksta & PCI_EXP_LNKSTA_LBMS;
 }
 
 /*