[PATCH 0/2] PCI: AtomicOps: Fix pci_enable_atomic_ops_to_root()

[PATCH 0/2] PCI: AtomicOps: Fix pci_enable_atomic_ops_to_root()

[Gerd Bayer]

Hi Bjorn et al.

this series addresses a few issues that have come up with the helper
function that enables Atomic Op Requests to be initiated by PCI
enpoints:

A. Most in-tree users of this helper use it incorrectly [0].
B. On s390, Atomic Op Requests are enabled, although the helper
   cannot know whether the root port is really supporting them.
C. Loop control in the helper function does not guarantee that a root
   port's capabilities are ever checked against those requested by the
   caller.

Address these issue with the following patches:
Patch 1: Make it harder to mis-use the enablement function,
Patch 2: Addresses issues B. and C.

I did test that issue B is fixed with these patches. Also, I verified
that Atomic Ops enablement on a Mellanox/Nvidia ConnectX-6 adapter
plugged straight into the root port of a x86 system still gets AtomicOp
Requests enabled. However, I did not test this with any PCIe switches
between root port and endpoint.

Ideally, both patches would be incorporated immediately, so we could
start correcting the mis-uses in the device drivers. I don't know of any
complaints when using Atomic Ops on devices where the driver is
mis-using the helper. Patch 2 however, is fixing an obseved issue.

[0]: https://lore.kernel.org/all/fbe34de16f5c0bf25a16f9819a57fdd81e5bb08c.camel@linux.ibm.com/
[1]: https://lore.kernel.org/all/20251105-mlxatomics-v1-0-10c71649e08d@linux.ibm.com/

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
Gerd Bayer (2):
      PCI: AtomicOps: Define valid root port capabilities
      PCI: AtomicOps: Fix logic in enable function

 drivers/pci/pci.c             | 43 +++++++++++++++++++++----------------------
 include/uapi/linux/pci_regs.h |  8 ++++++++
 2 files changed, 29 insertions(+), 22 deletions(-)
---
base-commit: e9a6fb0bcdd7609be6969112f3fbfcce3b1d4a7c
change-id: 20251106-fix_pciatops-7e8608eccb03

Best regards,
-- 
Gerd Bayer <gbayer@linux.ibm.com>

[PATCH 1/2] PCI: AtomicOps: Define valid root port capabilities

[Gerd Bayer]

Provide the two combinations of Atomic Op Completion size attributes
that a root port may support per PCIe Spec 7.0 section 6.15.3.1. -
besides the trivial "No support" - as two new defines.

Change documentation of pci_enable_atomic_ops_to_root() that these are
the only ones that should be used. Also, spell out that all requested
capabilities need to be supported at the root port for enable to
succeed. Also emphasize that on success, this sets AtomicOpsCtl:ReqEn to
1, and leaves it untouched in case of failure.

Suggested-by: Leon Romanovsky <leon@kernel.org>
Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
 drivers/pci/pci.c             | 13 +++++++------
 include/uapi/linux/pci_regs.h |  8 ++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b14dd064006cca80ec5275e45a35d6dc2b4d0bbc..597bf419c3a6867f8df7ebdc14fc8ca47d0958a6 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3809,15 +3809,16 @@ int pci_rebar_set_size(struct pci_dev *pdev, int bar, int size)
 /**
  * pci_enable_atomic_ops_to_root - enable AtomicOp requests to root port
  * @dev: the PCI device
- * @cap_mask: mask of desired AtomicOp sizes, including one or more of:
- *	PCI_EXP_DEVCAP2_ATOMIC_COMP32
- *	PCI_EXP_DEVCAP2_ATOMIC_COMP64
- *	PCI_EXP_DEVCAP2_ATOMIC_COMP128
+ * @cap_mask: root port must support combinations of AtomicOp sizes
+ *	PCI_EXP_ROOT_PORT_ATOMIC_BASE
+ *	PCI_EXP_ROOT_PORT_ATOMIC_FULL
  *
  * Return 0 if all upstream bridges support AtomicOp routing, egress
  * blocking is disabled on all upstream ports, and the root port supports
- * the requested completion capabilities (32-bit, 64-bit and/or 128-bit
- * AtomicOp completion), or negative otherwise.
+ * all the requested completion capabilities (BASE: 32-bit, 64-bit or
+ * FULL: 32/64- and 128-bit AtomicOp completion). In that case enable the
+ * device to send AtomicOp requests. Otherwise, return negative and leave
+ * the enablement in the PCI config space untouched.
  */
 int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 {
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 07e06aafec502af7c12379f7207e2e3321dc2ff1..0435306b4d26dc4caf27ae0391a5e6b930538213 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -663,6 +663,14 @@
 #define  PCI_EXP_DEVCAP2_ATOMIC_COMP32	0x00000080 /* 32b AtomicOp completion */
 #define  PCI_EXP_DEVCAP2_ATOMIC_COMP64	0x00000100 /* 64b AtomicOp completion */
 #define  PCI_EXP_DEVCAP2_ATOMIC_COMP128	0x00000200 /* 128b AtomicOp completion */
+/* PCIe spec 7.0 6.15.3.1: Root ports may support one of 2 sets of Atomic Ops */
+#define  PCI_EXP_ROOT_PORT_ATOMIC_BASE		\
+	(PCI_EXP_DEVCAP2_ATOMIC_COMP32 |	\
+	 PCI_EXP_DEVCAP2_ATOMIC_COMP64)
+#define  PCI_EXP_ROOT_PORT_ATOMIC_FULL		\
+	(PCI_EXP_DEVCAP2_ATOMIC_COMP32 |	\
+	 PCI_EXP_DEVCAP2_ATOMIC_COMP64 |	\
+	 PCI_EXP_DEVCAP2_ATOMIC_COMP128)
 #define  PCI_EXP_DEVCAP2_LTR		0x00000800 /* Latency tolerance reporting */
 #define  PCI_EXP_DEVCAP2_TPH_COMP_MASK	0x00003000 /* TPH completer support */
 #define  PCI_EXP_DEVCAP2_OBFF_MASK	0x000c0000 /* OBFF support mechanism */

-- 
2.48.1

Re: [PATCH 1/2] PCI: AtomicOps: Define valid root port capabilities

[Leon Romanovsky]

On Mon, Nov 10, 2025 at 02:25:05PM +0100, Gerd Bayer wrote:
> Provide the two combinations of Atomic Op Completion size attributes
> that a root port may support per PCIe Spec 7.0 section 6.15.3.1. -
> besides the trivial "No support" - as two new defines.
> 
> Change documentation of pci_enable_atomic_ops_to_root() that these are
> the only ones that should be used. Also, spell out that all requested
> capabilities need to be supported at the root port for enable to
> succeed. Also emphasize that on success, this sets AtomicOpsCtl:ReqEn to
> 1, and leaves it untouched in case of failure.
> 
> Suggested-by: Leon Romanovsky <leon@kernel.org>
> Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> ---
>  drivers/pci/pci.c             | 13 +++++++------
>  include/uapi/linux/pci_regs.h |  8 ++++++++
>  2 files changed, 15 insertions(+), 6 deletions(-)

<...>

> +/* PCIe spec 7.0 6.15.3.1: Root ports may support one of 2 sets of Atomic Ops */
> +#define  PCI_EXP_ROOT_PORT_ATOMIC_BASE		\
> +	(PCI_EXP_DEVCAP2_ATOMIC_COMP32 |	\
> +	 PCI_EXP_DEVCAP2_ATOMIC_COMP64)
> +#define  PCI_EXP_ROOT_PORT_ATOMIC_FULL		\
> +	(PCI_EXP_DEVCAP2_ATOMIC_COMP32 |	\
> +	 PCI_EXP_DEVCAP2_ATOMIC_COMP64 |	\
> +	 PCI_EXP_DEVCAP2_ATOMIC_COMP128)

Thanks for doing this.

[PATCH 2/2] PCI: AtomicOps: Fix logic in enable function

[Gerd Bayer]

Move the check for root port requirements past the loop within
pci_enable_atomic_ops_to_root() that checks on potential switch
(up- and downstream) ports.

Inside the loop traversing the PCI tree upwards, prepend the switch case
to validate the routing capability on any port with a fallthrough-case
that does the additional check for Atomic Ops not being blocked on
upstream ports.

Do not enable Atomic Op Requests if nothing can be learned about how the
device is attached - e.g. if it is on an "isolated" bus, as in s390.

Reported-by: Alexander Schmidt <alexs@linux.ibm.com>
Cc: stable@vger.kernel.org
Fixes: 430a23689dea ("PCI: Add pci_enable_atomic_ops_to_root()")
Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
 drivers/pci/pci.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 597bf419c3a6867f8df7ebdc14fc8ca47d0958a6..9a188fe8639d8a3d05e73e65114ce241d0f88bbf 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3823,7 +3823,7 @@ int pci_rebar_set_size(struct pci_dev *pdev, int bar, int size)
 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 *bridge = NULL;
 	u32 cap, ctl2;
 
 	/*
@@ -3861,29 +3861,27 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 		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:
+			if (!(cap & PCI_EXP_DEVCAP2_ATOMIC_ROUTE))
+				return -EINVAL;
+			break;
 		}
-
 		bus = bus->parent;
 	}
 
+	/* Finally, last bridge must be root port and support requested sizes */
+	if ((!bridge) ||
+	    (pci_pcie_type(bridge) != PCI_EXP_TYPE_ROOT_PORT) ||
+	    ((cap & cap_mask) != cap_mask))
+		return -EINVAL;
+
 	pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
 				 PCI_EXP_DEVCTL2_ATOMIC_REQ);
 	return 0;

-- 
2.48.1