public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] PCI: AtomicOps: Fix pci_enable_atomic_ops_to_root()
@ 2026-03-13 16:49 Gerd Bayer
  2026-03-13 16:49 ` [PATCH v4 1/2] PCI: AtomicOps: Do not enable if root-port capabilities are unknown Gerd Bayer
  2026-03-13 16:49 ` [PATCH v4 2/2] PCI: AtomicOps: Update references to PCIe spec Gerd Bayer
  0 siblings, 2 replies; 4+ messages in thread
From: Gerd Bayer @ 2026-03-13 16:49 UTC (permalink / raw)
  To: Bjorn Helgaas, Jay Cornwall, Felix Kuehling
  Cc: Leon Romanovsky, Niklas Schnelle, Alexander Schmidt, linux-s390,
	linux-pci, linux-kernel, netdev, linux-rdma, Gerd Bayer, stable

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.

Signed-off-by: Gerd Bayer <gbayer@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 if root-port capabilities are unknown
      PCI: AtomicOps: Update references to PCIe spec

 drivers/pci/pci.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)
---
base-commit: 0257f64bdac7fdca30fa3cae0df8b9ecbec7733a
change-id: 20251106-fix_pciatops-7e8608eccb03

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v4 1/2] PCI: AtomicOps: Do not enable if root-port capabilities are unknown
  2026-03-13 16:49 [PATCH v4 0/2] PCI: AtomicOps: Fix pci_enable_atomic_ops_to_root() Gerd Bayer
@ 2026-03-13 16:49 ` Gerd Bayer
  2026-03-20 13:26   ` Gerd Bayer
  2026-03-13 16:49 ` [PATCH v4 2/2] PCI: AtomicOps: Update references to PCIe spec Gerd Bayer
  1 sibling, 1 reply; 4+ messages in thread
From: Gerd Bayer @ 2026-03-13 16:49 UTC (permalink / raw)
  To: Bjorn Helgaas, Jay Cornwall, Felix Kuehling
  Cc: Leon Romanovsky, Niklas Schnelle, Alexander Schmidt, linux-s390,
	linux-pci, linux-kernel, netdev, linux-rdma, Gerd Bayer, stable

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 to 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.

Do not enable AtomicOps 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 8479c2e1f74f1044416281aba11bf071ea89488a..94e90988df86b3278b1b6abbc326abf9b4a4a962 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3676,7 +3676,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 *bridge = NULL;
 	u32 cap, ctl2;
 
 	/*
@@ -3714,29 +3714,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.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v4 2/2] PCI: AtomicOps: Update references to PCIe spec
  2026-03-13 16:49 [PATCH v4 0/2] PCI: AtomicOps: Fix pci_enable_atomic_ops_to_root() Gerd Bayer
  2026-03-13 16:49 ` [PATCH v4 1/2] PCI: AtomicOps: Do not enable if root-port capabilities are unknown Gerd Bayer
@ 2026-03-13 16:49 ` Gerd Bayer
  1 sibling, 0 replies; 4+ messages in thread
From: Gerd Bayer @ 2026-03-13 16:49 UTC (permalink / raw)
  To: Bjorn Helgaas, Jay Cornwall, Felix Kuehling
  Cc: Leon Romanovsky, Niklas Schnelle, Alexander Schmidt, linux-s390,
	linux-pci, linux-kernel, netdev, linux-rdma, 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 94e90988df86b3278b1b6abbc326abf9b4a4a962..b1dd977d811253b70c8dd859a1ff05b1970b7661 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3680,7 +3680,7 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 	u32 cap, 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.
 	 */
@@ -3691,9 +3691,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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 1/2] PCI: AtomicOps: Do not enable if root-port capabilities are unknown
  2026-03-13 16:49 ` [PATCH v4 1/2] PCI: AtomicOps: Do not enable if root-port capabilities are unknown Gerd Bayer
@ 2026-03-20 13:26   ` Gerd Bayer
  0 siblings, 0 replies; 4+ messages in thread
From: Gerd Bayer @ 2026-03-20 13:26 UTC (permalink / raw)
  To: Bjorn Helgaas, Jay Cornwall, Felix Kuehling
  Cc: Leon Romanovsky, Niklas Schnelle, Alexander Schmidt, linux-s390,
	linux-pci, linux-kernel, netdev, linux-rdma, stable

On Fri, 2026-03-13 at 17:49 +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.
> 
> Change to 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.
> 
> Do not enable AtomicOps 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 8479c2e1f74f1044416281aba11bf071ea89488a..94e90988df86b3278b1b6abbc326abf9b4a4a962 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3676,7 +3676,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 *bridge = NULL;
>  	u32 cap, ctl2;
>  
>  	/*
> @@ -3714,29 +3714,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;
> +

Sashiko (the new review tool) annotated that this patch may regress how
RCiEP's are treated:
https://sashiko.dev/#/patchset/20260313-fix_pciatops-v4-0-93bc70a63935%40linux.ibm.com

And it has a point: While AtomicOps Requests were allowed for RCiEP's
per default - now they're forbidden per default. Turns out that the
situation for RCiEP's on all archs is the same as for "isolated
functions" on s390: With pure PCI config ops you cannot tell, whether
the root-complex supports them or not.

I'm thinking about inverting the logic here - and adding an else clause
with an arch-specific hook to serve as discriminator.
Thoughts anybody?

>  	pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
>  				 PCI_EXP_DEVCTL2_ATOMIC_REQ);
>  	return 0;

Thanks,
Gerd

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-20 13:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-13 16:49 [PATCH v4 0/2] PCI: AtomicOps: Fix pci_enable_atomic_ops_to_root() Gerd Bayer
2026-03-13 16:49 ` [PATCH v4 1/2] PCI: AtomicOps: Do not enable if root-port capabilities are unknown Gerd Bayer
2026-03-20 13:26   ` Gerd Bayer
2026-03-13 16:49 ` [PATCH v4 2/2] PCI: AtomicOps: Update references to PCIe spec Gerd Bayer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox