Re: [PATCH v7 2/3] PCI: AtomicOps: Do not enable without support in root port

[Bjorn Helgaas]

On Mon, Mar 30, 2026 at 03:09:45PM +0200, Gerd Bayer wrote:
> When inspecting the config space of a Connect-X physical function in an
> s390 system after it was initialized by the mlx5_core device driver, we
> found the function to be enabled to request AtomicOps despite the
> system's root-complex lacking support for completing them:
> 
> 1ed0:00:00.1 Ethernet controller: Mellanox Technologies MT2894 Family [ConnectX-6 Lx]
> 	Subsystem: Mellanox Technologies Device 0002
>   [...]
> 	DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-
> 		 AtomicOpsCtl: ReqEn+
> 		 IDOReq- IDOCompl- LTR- EmergencyPowerReductionReq-
> 		 10BitTagReq- OBFF Disabled, EETLPPrefixBlk-
> 
> Turns out the device driver calls pci_enable_atomic_ops_to_root() which
> defaulted to enable AtomicOps requests even if it had no information
> about the root port that the PCIe device is attached to.
> 
> Change the logic of pci_enable_atomic_ops_to_root() to fully traverse the
> PCIe tree upwards, check that the bridge devices support delivering
> AtomicOps transactions, and finally check that there is a root port at
> the end that does support completing AtomicOps.
> 
> Reported-by: Alexander Schmidt <alexs@xxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 430a23689dea ("PCI: Add pci_enable_atomic_ops_to_root()")
> Signed-off-by: Gerd Bayer <gbayer@xxxxxxxxxxxxx>

OK, I think this is set to go.  It sounds like there are no RCiEPs
that we need to worry about.

I think pci_enable_atomic_ops_to_root() will end up more readable if
we check for the Root Port first and explicitly as in the modified
version.  I *think* it's equivalent but can't easily test it.  What do
you think?

commit 2f3f32f2c180 ("PCI: Enable AtomicOps only if Root Port supports them")
Author: Gerd Bayer <gbayer@xxxxxxxxxxxxx>
Date:   Mon Mar 30 15:09:45 2026 +0200

    PCI: Enable AtomicOps only if Root Port supports them
    
    When inspecting the config space of a Connect-X physical function in an
    s390 system after it was initialized by the mlx5_core device driver, we
    found the function to be enabled to request AtomicOps despite the Root Port
    lacking support for completing them:
    
      00:00.1 Ethernet controller: Mellanox Technologies MT2894 Family [ConnectX-6 Lx]
              Subsystem: Mellanox Technologies Device 0002
              DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-
                       AtomicOpsCtl: ReqEn+
    
    On s390 and many virtualized guests, the Endpoint is visible but the Root
    Port is not.  In this case, pci_enable_atomic_ops_to_root() previously
    enabled AtomicOps in the Endpoint even though it couldn't tell whether
    the Root Port supports them as a completer.
    
    Change pci_enable_atomic_ops_to_root() to fail if there's no Root Port or
    the Root Port doesn't support AtomicOps.
    
    Fixes: 430a23689dea ("PCI: Add pci_enable_atomic_ops_to_root()")
    Reported-by: Alexander Schmidt <alexs@xxxxxxxxxxxxx>
    Signed-off-by: Gerd Bayer <gbayer@xxxxxxxxxxxxx>
    Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
    Cc: stable@xxxxxxxxxxxxxxx

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 135e5b591df4..515f565a4a70 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3675,8 +3675,7 @@ void pci_acs_init(struct pci_dev *dev)
  */
 int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 {
-	struct pci_bus *bus = dev->bus;
-	struct pci_dev *bridge;
+	struct pci_dev *root, *bridge;
 	u32 cap, ctl2;
 
 	/*
@@ -3705,35 +3704,35 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 		return -EINVAL;
 	}
 
-	while (bus->parent) {
-		bridge = bus->self;
+	root = pcie_find_root_port(dev);
+	if (!root)
+		return -EINVAL;
 
-		pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
+	pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
+	if ((cap & cap_mask) != cap_mask)
+		return -EINVAL;
 
+	bridge = pci_upstream_bridge(dev);
+	while (bridge != root) {
 		switch (pci_pcie_type(bridge)) {
-		/* Ensure switch ports support AtomicOp routing */
 		case PCI_EXP_TYPE_UPSTREAM:
-		case PCI_EXP_TYPE_DOWNSTREAM:
-			if (!(cap & PCI_EXP_DEVCAP2_ATOMIC_ROUTE))
-				return -EINVAL;
-			break;
-
-		/* Ensure root port supports all the sizes we care about */
-		case PCI_EXP_TYPE_ROOT_PORT:
-			if ((cap & cap_mask) != cap_mask)
-				return -EINVAL;
-			break;
-		}
-
-		/* Ensure upstream ports don't block AtomicOps on egress */
-		if (pci_pcie_type(bridge) == PCI_EXP_TYPE_UPSTREAM) {
+			/* Upstream ports must not block AtomicOps on egress */
 			pcie_capability_read_dword(bridge, PCI_EXP_DEVCTL2,
 						   &ctl2);
 			if (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_EGRESS_BLOCK)
 				return -EINVAL;
+			fallthrough;
+
+		/* All switch ports need to route AtomicOps */
+		case PCI_EXP_TYPE_DOWNSTREAM:
+			pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2,
+						   &cap);
+			if (!(cap & PCI_EXP_DEVCAP2_ATOMIC_ROUTE))
+				return -EINVAL;
+			break;
 		}
 
-		bus = bus->parent;
+		bridge = pci_upstream_bridge(bridge);
 	}
 
 	pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,