PCI/PTM: Do not enable PTM automatically for Root and Switch Upstream Ports

Currently we enable PTM automatically for Root and Switch Upstream Ports if
the advertised capabilities support the relevant role. However, there are a
few issues with this. First of all, if there is no Endpoint that actually
needs the PTM functionality, this is just wasting link bandwidth. There are
just a couple of drivers calling pci_ptm_enable() in the tree.

Secondly, we do the enablement in pci_ptm_init() that is called pretty
early for the Switch Upstream Port before Downstream Ports are even
enumerated. Since the Upstream Port configuration affects the whole Switch,
enabling it this early might cause PTM requests to be sent. We actually do
see effects of this:

  pcieport 0000:00:07.1: pciehp: Slot(6-1): Card present
  pcieport 0000:00:07.1: pciehp: Slot(6-1): Link Up
  pci 0000:2c:00.0: [8086:5786] type 01 class 0x060400 PCIe Switch Upstream Port
  ...
  pci 0000:2c:00.0: PTM enabled, 4ns granularity

At this point we have only enumerated the Switch Upstream Port and now
PTM got enabled which immediately triggers a flood of errors:

  pcieport 0000:00:07.1: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
  pcieport 0000:00:07.1: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
  pcieport 0000:00:07.1:   device [8086:d44f] error status/mask=00200000/00000000
  pcieport 0000:00:07.1:    [21] ACSViol                (First)
  pcieport 0000:00:07.1: AER:   TLP Header: 0x34000000 0x00000052 0x00000000 0x00000000
  pcieport 0000:00:07.1: AER: device recovery successful
  pcieport 0000:00:07.1: AER: Uncorrectable (Non-Fatal) error message received from 0000:00:07.1

In the above TLP Header the Requester ID is 0 which causes an error as we
have ACS Source Validation enabled.

Change the PTM enablement to happen at the time pci_enable_ptm() is called.
It will try to enable PTM first for upstream devices before enabling for
the Endpoint itself. For disable path we need to keep count of how many
times PTM has been enabled and disable it only on the last, so change the
dev->ptm_enabled to a counter (and rename it to dev->ptm_enable_cnt
analogous to dev->pci_enable_cnt).

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://patch.msgid.link/20260224111044.3487873-6-mika.westerberg@linux.intel.com

diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
index 2c848ae4f15f2e9322b7506699a1d136b538b297..a41ffd1914de427b0d41fe53fca97e2992f8d2f3 100644 (file)
--- a/drivers/pci/pcie/ptm.c
+++ b/drivers/pci/pcie/ptm.c
@@ -52,6 +52,7 @@ void pci_ptm_init(struct pci_dev *dev)
                return;
 
        dev->ptm_cap = ptm;
+       atomic_set(&dev->ptm_enable_cnt, 0);
        pci_add_ext_cap_save_buffer(dev, PCI_EXT_CAP_ID_PTM, sizeof(u32));
 
        pci_read_config_dword(dev, ptm + PCI_PTM_CAP, &cap);
@@ -85,10 +86,6 @@ void pci_ptm_init(struct pci_dev *dev)
                dev->ptm_responder = 1;
        if (cap & PCI_PTM_CAP_REQ)
                dev->ptm_requester = 1;
-
-       if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
-           pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM)
-               pci_enable_ptm(dev);
 }
 
 void pci_save_ptm_state(struct pci_dev *dev)
@@ -129,26 +126,11 @@ void pci_restore_ptm_state(struct pci_dev *dev)
 static int __pci_enable_ptm(struct pci_dev *dev)
 {
        u16 ptm = dev->ptm_cap;
-       struct pci_dev *ups;
        u32 ctrl;
 
        if (!ptm)
                return -EINVAL;
 
-       /*
-        * A device uses local PTM Messages to request time information
-        * from a PTM Root that's farther upstream.  Every device along the
-        * path must support PTM and have it enabled so it can handle the
-        * messages.  Therefore, if this device is not a PTM Root, the
-        * upstream link partner must have PTM enabled before we can enable
-        * PTM.
-        */
-       if (!dev->ptm_root) {
-               ups = pci_upstream_ptm(dev);
-               if (!ups || !ups->ptm_enabled)
-                       return -EINVAL;
-       }
-
        switch (pci_pcie_type(dev)) {
        case PCI_EXP_TYPE_ROOT_PORT:
                if (!dev->ptm_root)
@@ -193,11 +175,35 @@ int pci_enable_ptm(struct pci_dev *dev)
        int rc;
        char clock_desc[8];
 
+       /*
+        * A device uses local PTM Messages to request time information
+        * from a PTM Root that's farther upstream. Every device along
+        * the path must support PTM and have it enabled so it can
+        * handle the messages. Therefore, if this device is not a PTM
+        * Root, the upstream link partner must have PTM enabled before
+        * we can enable PTM.
+        */
+       if (!dev->ptm_root) {
+               struct pci_dev *parent;
+
+               parent = pci_upstream_ptm(dev);
+               if (!parent)
+                       return -EINVAL;
+               /* Enable PTM for the parent */
+               rc = pci_enable_ptm(parent);
+               if (rc)
+                       return rc;
+       }
+
+       /* Already enabled? */
+       if (atomic_inc_return(&dev->ptm_enable_cnt) > 1)
+               return 0;
+
        rc = __pci_enable_ptm(dev);
-       if (rc)
+       if (rc) {
+               atomic_dec(&dev->ptm_enable_cnt);
                return rc;
-
-       dev->ptm_enabled = 1;
+       }
 
        switch (dev->ptm_granularity) {
        case 0:
@@ -239,27 +245,31 @@ static void __pci_disable_ptm(struct pci_dev *dev)
  */
 void pci_disable_ptm(struct pci_dev *dev)
 {
-       if (dev->ptm_enabled) {
+       struct pci_dev *parent;
+
+       if (atomic_dec_and_test(&dev->ptm_enable_cnt))
                __pci_disable_ptm(dev);
-               dev->ptm_enabled = 0;
-       }
+
+       parent = pci_upstream_ptm(dev);
+       if (parent)
+               pci_disable_ptm(parent);
 }
 EXPORT_SYMBOL(pci_disable_ptm);
 
 /*
- * Disable PTM, but preserve dev->ptm_enabled so we silently re-enable it on
+ * Disable PTM, but preserve dev->ptm_enable_cnt so we silently re-enable it on
  * resume if necessary.
  */
 void pci_suspend_ptm(struct pci_dev *dev)
 {
-       if (dev->ptm_enabled)
+       if (atomic_read(&dev->ptm_enable_cnt))
                __pci_disable_ptm(dev);
 }
 
 /* If PTM was enabled before suspend, re-enable it when resuming */
 void pci_resume_ptm(struct pci_dev *dev)
 {
-       if (dev->ptm_enabled)
+       if (atomic_read(&dev->ptm_enable_cnt))
                __pci_enable_ptm(dev);
 }
 
@@ -268,7 +278,7 @@ bool pcie_ptm_enabled(struct pci_dev *dev)
        if (!dev)
                return false;
 
-       return dev->ptm_enabled;
+       return atomic_read(&dev->ptm_enable_cnt);
 }
 EXPORT_SYMBOL(pcie_ptm_enabled);
 

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8aaa72dcb16460ab50c986d03820783714d3e4b0..c620d4b6c52e04a58a66882b0ffda7cd51210ce6 100644 (file)
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -518,7 +518,7 @@ struct pci_dev {
        unsigned int    ptm_root:1;
        unsigned int    ptm_responder:1;
        unsigned int    ptm_requester:1;
-       unsigned int    ptm_enabled:1;
+       atomic_t        ptm_enable_cnt;
        u8              ptm_granularity;
 #endif
 #ifdef CONFIG_PCI_MSI