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

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

[Gerd Bayer]

Hi Bjorn et al.

On s390, AtomicOp Requests are enabled on a PCI function that supports
them, despite the helper being ignorant about the root port's capability
to supporting their completion.

Patch 1: Fix the logic in pci_enable_atomic_ops_to_root()
Patch 2: Update references to PCIe spec in that function.

I did test that the issue is fixed with these patches. Also, I verified
that on a Mellanox/Nvidia ConnectX-6 adapter plugged straight into the
root port of a x86 system still gets AtomicOp Requests enabled.

Due to lacking the required hardware, I did not test this with any PCIe
switches between root port and endpoint. So test exposure to other
environments is highly appreciated. One particularly rare setup is a
RCiEP that might act as a AtomicOps Requestor - but was subject to a
regression in v4 as reported by Sashiko.

v5 of this series tries to make no functional changes for that class of
devices either.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
Changes in v6:
- Incorporate Ilpo's editorial comments.
- Correct logic in pci_is_atomicops_capable_rp() (annotated by Sashiko)
- Link to v5: https://lore.kernel.org/r/20260323-fix_pciatops-v5-0-fada7233aea8@linux.ibm.com

Changes in v5:
- Introduce new pcibios_connects_to_atomicops_capable_rc() so arch's can
  declare AtomicOps support outside of PCIe config space. Defaults to
  "true" - except s390.
- rebase to 7.0-rc5
- Link to v4: https://lore.kernel.org/r/20260313-fix_pciatops-v4-0-93bc70a63935@linux.ibm.com

Changes in v4:
- drop patch 1 - it will become the base of a new series
- previous patch 2, now 1: reword commit message
- add a new patch to update references to PCI spec within
  pci_enable_atomic_ops_to_root()
- rebase to latest master
- Link to v3: https://lore.kernel.org/r/20260306-fix_pciatops-v3-0-99d12bcafb19@linux.ibm.com

Changes in v3:
- rebase to 7.0-rc2
- gentle ping
- add netdev and rdma lists for awareness
- Link to v2: https://lore.kernel.org/r/20251216-fix_pciatops-v2-0-d013e9b7e2ee@linux.ibm.com

Changes in v2:
- rebase to 6.19-rc1
- otherwise unchanged to v1
- Link to v1: https://lore.kernel.org/r/20251110-fix_pciatops-v1-0-edc58a57b62e@linux.ibm.com

---
Gerd Bayer (2):
      PCI: AtomicOps: Do not enable without support in root complex
      PCI: AtomicOps: Update references to PCIe spec

 arch/s390/pci/pci.c |  5 +++++
 drivers/pci/pci.c   | 54 +++++++++++++++++++++++++++++++++--------------------
 include/linux/pci.h |  1 +
 3 files changed, 40 insertions(+), 20 deletions(-)
---
base-commit: c369299895a591d96745d6492d4888259b004a9e
change-id: 20251106-fix_pciatops-7e8608eccb03

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

[PATCH v6 1/2] PCI: AtomicOps: Do not enable without support in root complex

[Gerd Bayer]

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. Similarly,
AtomicOps requests are enabled for root complex integrated endpoints
(RCiEPs) unconditionally.

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 - or that the support for
completing AtomicOps at the root complex is announced through some other
arch specific way.

Introduce a new pcibios_connects_to_atomicops_capable_rc() function to
implement the check - and default to always "true". This leaves the
semantics for today's RCiEPs intact. Pass in the device in question and
the requested capabilities for future expansions.
For s390, override pcibios_connects_to_atomicops_capable_rc() to
always return "false".

Do not change the enablement of AtomicOps requests if there is no
positive confirmation that the root complex can complete PCIe AtomicOps.

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>
---
 arch/s390/pci/pci.c |  5 +++++
 drivers/pci/pci.c   | 48 +++++++++++++++++++++++++++++++-----------------
 include/linux/pci.h |  1 +
 3 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 2a430722cbe415dd56c92fed2e513e524f46481a..a0bef77082a153a258fbe4abb1070b22e020888e 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -265,6 +265,11 @@ static int zpci_cfg_store(struct zpci_dev *zdev, int offset, u32 val, u8 len)
 	return rc;
 }
 
+bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
+{
+	return false;
+}
+
 resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				       resource_size_t size,
 				       resource_size_t align)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8479c2e1f74f1044416281aba11bf071ea89488a..006aa589926cb290de43f152100ddaf9961407d1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3660,6 +3660,19 @@ void pci_acs_init(struct pci_dev *dev)
 	pci_disable_broken_acs_cap(dev);
 }
 
+static bool pci_is_atomicops_capable_rp(struct pci_dev *dev, u32 cap, u32 cap_mask)
+{
+	if (!dev || !(pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT))
+		return false;
+
+	return (cap & cap_mask) == cap_mask;
+}
+
+bool __weak pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
+{
+	return true;
+}
+
 /**
  * pci_enable_atomic_ops_to_root - enable AtomicOp requests to root port
  * @dev: the PCI device
@@ -3676,8 +3689,9 @@ 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;
-	u32 cap, ctl2;
+	struct pci_dev *bridge = NULL;
+	u32 cap = 0;
+	u32 ctl2;
 
 	/*
 	 * Per PCIe r5.0, sec 9.3.5.10, the AtomicOp Requester Enable bit
@@ -3714,29 +3728,29 @@ 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
+	 * or firmware asserts support
+	 */
+	if (!(pci_is_atomicops_capable_rp(bridge, cap, cap_mask) ||
+	      pcibios_connects_to_atomicops_capable_rc(dev, cap_mask)))
+		return -EINVAL;
+
 	pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
 				 PCI_EXP_DEVCTL2_ATOMIC_REQ);
 	return 0;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1c270f1d512301de4d462fe7e5097c32af5c6f8d..ef90604c39859ea8e61e5392d0bdaa1b0e43874b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -692,6 +692,7 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
 				 void *release_data);
 
 int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
+bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask);
 
 #define PCI_REGION_FLAG_MASK	0x0fU	/* These bits of resource flags tell us the PCI region flags */
 

-- 
2.51.0

[PATCH v6 2/2] PCI: AtomicOps: Update references to PCIe spec

[Gerd Bayer]

Point to the relevant sections in the most recent release 7.0 of the
PCIe spec. Text has mostly just moved around without any semantic
change.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
 drivers/pci/pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 006aa589926cb290de43f152100ddaf9961407d1..fc211af0b6361cd8f5101b681a97bd1ad1304d9d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3694,7 +3694,7 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 	u32 ctl2;
 
 	/*
-	 * Per PCIe r5.0, sec 9.3.5.10, the AtomicOp Requester Enable bit
+	 * Per PCIe r7.0, sec 7.5.3.16, the AtomicOp Requester Enable bit
 	 * in Device Control 2 is reserved in VFs and the PF value applies
 	 * to all associated VFs.
 	 */
@@ -3705,9 +3705,9 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 		return -EINVAL;
 
 	/*
-	 * Per PCIe r4.0, sec 6.15, endpoints and root ports may be
+	 * Per PCIe r7.0, sec 6.15, endpoints and root ports may be
 	 * AtomicOp requesters.  For now, we only support endpoints as
-	 * requesters and root ports as completers.  No endpoints as
+	 * requesters and root ports as completers. No endpoints as
 	 * completers, and no peer-to-peer.
 	 */
 

-- 
2.51.0

Re: [PATCH v6 1/2] PCI: AtomicOps: Do not enable without support in root complex

[Bjorn Helgaas]

On Wed, Mar 25, 2026 at 04:16:17PM +0100, 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. Similarly,
> AtomicOps requests are enabled for root complex integrated endpoints
> (RCiEPs) unconditionally.
> 
> 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 - or that the support for
> completing AtomicOps at the root complex is announced through some other
> arch specific way.
> 
> Introduce a new pcibios_connects_to_atomicops_capable_rc() function to
> implement the check - and default to always "true". This leaves the
> semantics for today's RCiEPs intact. Pass in the device in question and
> the requested capabilities for future expansions.
> For s390, override pcibios_connects_to_atomicops_capable_rc() to
> always return "false".
> 
> Do not change the enablement of AtomicOps requests if there is no
> positive confirmation that the root complex can complete PCIe AtomicOps.
> 
> 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>
> ---
>  arch/s390/pci/pci.c |  5 +++++
>  drivers/pci/pci.c   | 48 +++++++++++++++++++++++++++++++-----------------
>  include/linux/pci.h |  1 +
>  3 files changed, 37 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 2a430722cbe415dd56c92fed2e513e524f46481a..a0bef77082a153a258fbe4abb1070b22e020888e 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -265,6 +265,11 @@ static int zpci_cfg_store(struct zpci_dev *zdev, int offset, u32 val, u8 len)
>  	return rc;
>  }
>  
> +bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
> +{
> +	return false;
> +}
> +
>  resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  				       resource_size_t size,
>  				       resource_size_t align)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8479c2e1f74f1044416281aba11bf071ea89488a..006aa589926cb290de43f152100ddaf9961407d1 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3660,6 +3660,19 @@ void pci_acs_init(struct pci_dev *dev)
>  	pci_disable_broken_acs_cap(dev);
>  }
>  
> +static bool pci_is_atomicops_capable_rp(struct pci_dev *dev, u32 cap, u32 cap_mask)
> +{
> +	if (!dev || !(pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT))
> +		return false;
> +
> +	return (cap & cap_mask) == cap_mask;
> +}
> +
> +bool __weak pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
> +{
> +	return true;
> +}
> +
>  /**
>   * pci_enable_atomic_ops_to_root - enable AtomicOp requests to root port
>   * @dev: the PCI device
> @@ -3676,8 +3689,9 @@ 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;
> -	u32 cap, ctl2;
> +	struct pci_dev *bridge = NULL;
> +	u32 cap = 0;
> +	u32 ctl2;
>  
>  	/*
>  	 * Per PCIe r5.0, sec 9.3.5.10, the AtomicOp Requester Enable bit
> @@ -3714,29 +3728,29 @@ 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
> +	 * or firmware asserts support
> +	 */
> +	if (!(pci_is_atomicops_capable_rp(bridge, cap, cap_mask) ||
> +	      pcibios_connects_to_atomicops_capable_rc(dev, cap_mask)))
> +		return -EINVAL;

Sashiko says:

  Since the generic weak implementation of
  pcibios_connects_to_atomicops_capable_rc() unconditionally returns
  true, the logical OR expression pci_is_atomicops_capable_rp(...) ||
  true will always evaluate to true. This makes the entire if
  condition evaluate to false.

  Because of this, it appears -EINVAL is never returned here, and any
  standard endpoint behind a Root Port will successfully be granted
  AtomicOps even if the Root Port lacks the capability in its
  PCI_EXP_DEVCAP2 register.

> +
>  	pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
>  				 PCI_EXP_DEVCTL2_ATOMIC_REQ);
>  	return 0;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 1c270f1d512301de4d462fe7e5097c32af5c6f8d..ef90604c39859ea8e61e5392d0bdaa1b0e43874b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -692,6 +692,7 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>  				 void *release_data);
>  
>  int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
> +bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask);
>  
>  #define PCI_REGION_FLAG_MASK	0x0fU	/* These bits of resource flags tell us the PCI region flags */
>  
> 
> -- 
> 2.51.0
> 

Re: [PATCH v6 1/2] PCI: AtomicOps: Do not enable without support in root complex

[Gerd Bayer]

On Wed, 2026-03-25 at 15:08 -0500, Bjorn Helgaas wrote:
> On Wed, Mar 25, 2026 at 04:16:17PM +0100, 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. Similarly,
> > AtomicOps requests are enabled for root complex integrated endpoints
> > (RCiEPs) unconditionally.
> > 
> > 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 - or that the support for
> > completing AtomicOps at the root complex is announced through some other
> > arch specific way.
> > 
> > Introduce a new pcibios_connects_to_atomicops_capable_rc() function to
> > implement the check - and default to always "true". This leaves the
> > semantics for today's RCiEPs intact. Pass in the device in question and
> > the requested capabilities for future expansions.
> > For s390, override pcibios_connects_to_atomicops_capable_rc() to
> > always return "false".
> > 
> > Do not change the enablement of AtomicOps requests if there is no
> > positive confirmation that the root complex can complete PCIe AtomicOps.
> > 
> > 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>
> > ---
> >  arch/s390/pci/pci.c |  5 +++++
> >  drivers/pci/pci.c   | 48 +++++++++++++++++++++++++++++++-----------------
> >  include/linux/pci.h |  1 +
> >  3 files changed, 37 insertions(+), 17 deletions(-)
> > 
> > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> > index 2a430722cbe415dd56c92fed2e513e524f46481a..a0bef77082a153a258fbe4abb1070b22e020888e 100644
> > --- a/arch/s390/pci/pci.c
> > +++ b/arch/s390/pci/pci.c
> > @@ -265,6 +265,11 @@ static int zpci_cfg_store(struct zpci_dev *zdev, int offset, u32 val, u8 len)
> >  	return rc;
> >  }
> >  
> > +bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
> > +{
> > +	return false;
> > +}
> > +
> >  resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> >  				       resource_size_t size,
> >  				       resource_size_t align)
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 8479c2e1f74f1044416281aba11bf071ea89488a..006aa589926cb290de43f152100ddaf9961407d1 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3660,6 +3660,19 @@ void pci_acs_init(struct pci_dev *dev)
> >  	pci_disable_broken_acs_cap(dev);
> >  }
> >  
> > +static bool pci_is_atomicops_capable_rp(struct pci_dev *dev, u32 cap, u32 cap_mask)
> > +{
> > +	if (!dev || !(pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT))
> > +		return false;
> > +
> > +	return (cap & cap_mask) == cap_mask;
> > +}
> > +
> > +bool __weak pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
> > +{
> > +	return true;
> > +}
> > +
> >  /**
> >   * pci_enable_atomic_ops_to_root - enable AtomicOp requests to root port
> >   * @dev: the PCI device
> > @@ -3676,8 +3689,9 @@ 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;
> > -	u32 cap, ctl2;
> > +	struct pci_dev *bridge = NULL;
> > +	u32 cap = 0;
> > +	u32 ctl2;
> >  
> >  	/*
> >  	 * Per PCIe r5.0, sec 9.3.5.10, the AtomicOp Requester Enable bit
> > @@ -3714,29 +3728,29 @@ 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
> > +	 * or firmware asserts support
> > +	 */
> > +	if (!(pci_is_atomicops_capable_rp(bridge, cap, cap_mask) ||
> > +	      pcibios_connects_to_atomicops_capable_rc(dev, cap_mask)))
> > +		return -EINVAL;
> 
> Sashiko says:
> 
>   Since the generic weak implementation of
>   pcibios_connects_to_atomicops_capable_rc() unconditionally returns
>   true, the logical OR expression pci_is_atomicops_capable_rp(...) ||
>   true will always evaluate to true. This makes the entire if
>   condition evaluate to false.
> 
>   Because of this, it appears -EINVAL is never returned here, and any
>   standard endpoint behind a Root Port will successfully be granted
>   AtomicOps even if the Root Port lacks the capability in its
>   PCI_EXP_DEVCAP2 register.

I've made the generic implementation of
pcibios_connects_to_atomicops_capable_rc() default to return "true" to
preserve the current code's handling of RCiEPs: Since they are not
attached to a root port, their dev->bus->parent is NULL and the entire
while-loop is bypassed - before this patch and after. (Sashiko was
pointing at that being regressed with v4.)

The whole point of pcibios_connects_to_atomicops_capable_rc() is to
allow different architectures to implement a discriminator outside of
PCIe's structure - potentially depending on CPU model or more.

The only point I wonder about: Should
pcibios_connects_to_atomicops_capable_rc() default to return "false"
and deliberately change the behavior for today's RCiEP's (if there are
any...)?

> 
> > +
> >  	pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
> >  				 PCI_EXP_DEVCTL2_ATOMIC_REQ);
> >  	return 0;
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 1c270f1d512301de4d462fe7e5097c32af5c6f8d..ef90604c39859ea8e61e5392d0bdaa1b0e43874b 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -692,6 +692,7 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
> >  				 void *release_data);
> >  
> >  int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
> > +bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask);
> >  
> >  #define PCI_REGION_FLAG_MASK	0x0fU	/* These bits of resource flags tell us the PCI region flags */
> >  
> > 
> > -- 
> > 2.51.0
> > 

Thanks,
Gerd

Re: [PATCH v6 1/2] PCI: AtomicOps: Do not enable without support in root complex

[Bjorn Helgaas]

On Thu, Mar 26, 2026 at 10:51:19AM +0100, Gerd Bayer wrote:
> On Wed, 2026-03-25 at 15:08 -0500, Bjorn Helgaas wrote:
> > On Wed, Mar 25, 2026 at 04:16:17PM +0100, 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. Similarly,
> > > AtomicOps requests are enabled for root complex integrated endpoints
> > > (RCiEPs) unconditionally.
> > > 
> > > 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 - or that the support for
> > > completing AtomicOps at the root complex is announced through some other
> > > arch specific way.
> > > 
> > > Introduce a new pcibios_connects_to_atomicops_capable_rc() function to
> > > implement the check - and default to always "true". This leaves the
> > > semantics for today's RCiEPs intact. Pass in the device in question and
> > > the requested capabilities for future expansions.
> > > For s390, override pcibios_connects_to_atomicops_capable_rc() to
> > > always return "false".
> > > 
> > > Do not change the enablement of AtomicOps requests if there is no
> > > positive confirmation that the root complex can complete PCIe AtomicOps.
> > > 
> > > 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>
> > > ---
> > >  arch/s390/pci/pci.c |  5 +++++
> > >  drivers/pci/pci.c   | 48 +++++++++++++++++++++++++++++++-----------------
> > >  include/linux/pci.h |  1 +
> > >  3 files changed, 37 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> > > index 2a430722cbe415dd56c92fed2e513e524f46481a..a0bef77082a153a258fbe4abb1070b22e020888e 100644
> > > --- a/arch/s390/pci/pci.c
> > > +++ b/arch/s390/pci/pci.c
> > > @@ -265,6 +265,11 @@ static int zpci_cfg_store(struct zpci_dev *zdev, int offset, u32 val, u8 len)
> > >  	return rc;
> > >  }
> > >  
> > > +bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
> > > +{
> > > +	return false;
> > > +}
> > > +
> > >  resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> > >  				       resource_size_t size,
> > >  				       resource_size_t align)
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index 8479c2e1f74f1044416281aba11bf071ea89488a..006aa589926cb290de43f152100ddaf9961407d1 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -3660,6 +3660,19 @@ void pci_acs_init(struct pci_dev *dev)
> > >  	pci_disable_broken_acs_cap(dev);
> > >  }
> > >  
> > > +static bool pci_is_atomicops_capable_rp(struct pci_dev *dev, u32 cap, u32 cap_mask)
> > > +{
> > > +	if (!dev || !(pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT))
> > > +		return false;
> > > +
> > > +	return (cap & cap_mask) == cap_mask;
> > > +}
> > > +
> > > +bool __weak pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
> > > +{
> > > +	return true;
> > > +}
> > > +
> > >  /**
> > >   * pci_enable_atomic_ops_to_root - enable AtomicOp requests to root port
> > >   * @dev: the PCI device
> > > @@ -3676,8 +3689,9 @@ 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;
> > > -	u32 cap, ctl2;
> > > +	struct pci_dev *bridge = NULL;
> > > +	u32 cap = 0;
> > > +	u32 ctl2;
> > >  
> > >  	/*
> > >  	 * Per PCIe r5.0, sec 9.3.5.10, the AtomicOp Requester Enable bit
> > > @@ -3714,29 +3728,29 @@ 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
> > > +	 * or firmware asserts support
> > > +	 */
> > > +	if (!(pci_is_atomicops_capable_rp(bridge, cap, cap_mask) ||
> > > +	      pcibios_connects_to_atomicops_capable_rc(dev, cap_mask)))
> > > +		return -EINVAL;
> > 
> > Sashiko says:
> > 
> >   Since the generic weak implementation of
> >   pcibios_connects_to_atomicops_capable_rc() unconditionally returns
> >   true, the logical OR expression pci_is_atomicops_capable_rp(...) ||
> >   true will always evaluate to true. This makes the entire if
> >   condition evaluate to false.
> > 
> >   Because of this, it appears -EINVAL is never returned here, and any
> >   standard endpoint behind a Root Port will successfully be granted
> >   AtomicOps even if the Root Port lacks the capability in its
> >   PCI_EXP_DEVCAP2 register.
> 
> I've made the generic implementation of
> pcibios_connects_to_atomicops_capable_rc() default to return "true" to
> preserve the current code's handling of RCiEPs: Since they are not
> attached to a root port, their dev->bus->parent is NULL and the entire
> while-loop is bypassed - before this patch and after. (Sashiko was
> pointing at that being regressed with v4.)

The v4 patch definitely changed the behavior for RCiEPs: the current
v7.0-rc1 code always enables AtomicOps for RCiEPs, and the v4 patch
never enables AtomicOps for RCiEPs.  But I'm not sure this is a
regression.  It definitely *could* break an RCiEP, but AFAIK we have
no information about whether the RC supports AtomicOps, so enabling
them and telling the driver that AtomicOps work might be a lie.

IIUC, the motivation for this series was to avoid enabling AtomicOps
on s390 where there is no visible Root Port, and you have platform
knowledg that whatever is upstream from the endpoint in fact does not
support them.

I think we should avoid enabling AtomicOps unless we know for certain
that the completer (Root Port or RC) supports them.  To me that sounds
like:

  1) Never enable AtomicOps for RCiEPs.

  2) Only enable AtomicOps for endpoints below a Root Port that
  supports AtomicOps.

This could be two separate patches, where the second would fix the
s390 issue reported by Alexander.

If we come across RCiEPs that need AtomicOps and we somehow know that
the RC supports them, we can add a quirk or something to take
advantage of it.

We are still hand-waving about peer-to-peer transactions; we don't
even try to account for that because we don't know what peer might be
the completer.

> The whole point of pcibios_connects_to_atomicops_capable_rc() is to
> allow different architectures to implement a discriminator outside of
> PCIe's structure - potentially depending on CPU model or more.
> 
> The only point I wonder about: Should
> pcibios_connects_to_atomicops_capable_rc() default to return "false"
> and deliberately change the behavior for today's RCiEP's (if there are
> any...)?
> 
> > 
> > > +
> > >  	pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
> > >  				 PCI_EXP_DEVCTL2_ATOMIC_REQ);
> > >  	return 0;
> > > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > > index 1c270f1d512301de4d462fe7e5097c32af5c6f8d..ef90604c39859ea8e61e5392d0bdaa1b0e43874b 100644
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -692,6 +692,7 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
> > >  				 void *release_data);
> > >  
> > >  int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
> > > +bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask);
> > >  
> > >  #define PCI_REGION_FLAG_MASK	0x0fU	/* These bits of resource flags tell us the PCI region flags */
> > >  
> > > 
> > > -- 
> > > 2.51.0
> > > 
> 
> Thanks,
> Gerd