[PATCH RFC 0/2] mlx5/pci: Fix enablement of PCIe AtomicOp Requests

[PATCH RFC 0/2] mlx5/pci: Fix enablement of PCIe AtomicOp Requests

[Gerd Bayer]

As I promised in
https://lore.kernel.org/linux-pci/9c7c4217171fb56c505dc90b8c73b2ce079207a9.camel@linux.ibm.com/#t
here's an RFC patch proposing to correct the usage of
pci_enable_atomic_ops_to_root() in the Ethernet and InfiniBand drivers
for Mellanox/NVidia devices.

With this fix, AtomicOp Requests would only be enabled if a root-port
correctly announced its capability to support all AtomicOps including
the optional 128bit CAS - as the drivers intend to.

That said, on the only x86 system that I can test this on, it appears
that UEFI is already enabling AtomicOp Requests for the ConnectX-6 Dx
card in that system.

Overall, both the enablement of PCIe AtomicOps by core PCI code, and
their usage by (very few) device drivers, appear to be in need of some
attention?

Thanks,
Gerd

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
Gerd Bayer (2):
      net/mlx5: Request PCIe AtomicOps enabled for all 3 sizes
      ib/mlx5: Request PCIe AtomicOps enabled for all 3 sizes

 drivers/infiniband/hw/mlx5/data_direct.c       | 6 +++---
 drivers/net/ethernet/mellanox/mlx5/core/main.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)
---
base-commit: 6146a0f1dfae5d37442a9ddcba012add260bceb0
change-id: 20251024-mlxatomics-5729e8af60b9

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

[PATCH RFC 1/2] net/mlx5: Request PCIe AtomicOps enabled for all 3 sizes

[Gerd Bayer]

Pass fully populated capability bit-mask requesting support for all 3
sizes of AtomicOps at once when attempting to enable AtomicOps for PCI
function.

When called individually, pci_enable_atomic_ops_to_root() may enable the
device to send requests as soon as one size is supported. According to
PCIe Spec 7.0 Section 6.15.3.1 support of 32-bit and 64-bit AtomicOps
completer capabilities are tied together for root-ports. Only the
128-bit/CAS completer capabilities is an optional feature, but still we
might end up end up enabling AtomicOps despite 128-bit/CAS is not
supported at the root-port.

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

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 70c156591b0ba9c61dd99818043003e50e177590..69152a9dae2be07e023fa42fd9ef010c8b255c4c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -934,9 +934,9 @@ static int mlx5_pci_init(struct mlx5_core_dev *dev, struct pci_dev *pdev,
 		goto err_clr_master;
 	}
 
-	if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32) &&
-	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP64) &&
-	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP128))
+	if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
+						PCI_EXP_DEVCAP2_ATOMIC_COMP64 |
+						PCI_EXP_DEVCAP2_ATOMIC_COMP128))
 		mlx5_core_dbg(dev, "Enabling pci atomics failed\n");
 
 	dev->iseg_base = dev->bar_addr;

-- 
2.48.1

[PATCH RFC 2/2] ib/mlx5: Request PCIe AtomicOps enabled for all 3 sizes

[Gerd Bayer]

Pass fully populated capability bit-mask requesting support for all 3
sizes of AtomicOps at once when attempting to enable AtomicOps for PCI
function.

When called individually, pci_enable_atomic_ops_to_root() may enable the
device to send requests as soon as one size is supported. According to
PCIe Spec 7.0 Section 6.15.3.1 support of 32-bit and 64-bit AtomicOps
completer capabilities are tied together for root-ports. Only the
128-bit/CAS completer capabilities is an optional feature, but still we
might end up end up enabling AtomicOps despite 128-bit/CAS is not
supported at the root-port.

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

diff --git a/drivers/infiniband/hw/mlx5/data_direct.c b/drivers/infiniband/hw/mlx5/data_direct.c
index b81ac5709b56f6ac0d9f60572ce7144258fa2794..112185be53f1ccc6a797e129f24432bdc86008ae 100644
--- a/drivers/infiniband/hw/mlx5/data_direct.c
+++ b/drivers/infiniband/hw/mlx5/data_direct.c
@@ -179,9 +179,9 @@ static int mlx5_data_direct_probe(struct pci_dev *pdev, const struct pci_device_
 	if (err)
 		goto err_disable;
 
-	if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32) &&
-	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP64) &&
-	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP128))
+	if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
+						PCI_EXP_DEVCAP2_ATOMIC_COMP64 |
+						PCI_EXP_DEVCAP2_ATOMIC_COMP128))
 		dev_dbg(dev->device, "Enabling pci atomics failed\n");
 
 	err = mlx5_data_direct_vpd_get_vuid(dev);

-- 
2.48.1

Re: [PATCH RFC 2/2] ib/mlx5: Request PCIe AtomicOps enabled for all 3 sizes

[Leon Romanovsky]

On Wed, Nov 05, 2025 at 06:55:14PM +0100, Gerd Bayer wrote:
> Pass fully populated capability bit-mask requesting support for all 3
> sizes of AtomicOps at once when attempting to enable AtomicOps for PCI
> function.
> 
> When called individually, pci_enable_atomic_ops_to_root() may enable the
> device to send requests as soon as one size is supported. According to
> PCIe Spec 7.0 Section 6.15.3.1 support of 32-bit and 64-bit AtomicOps
> completer capabilities are tied together for root-ports. Only the
> 128-bit/CAS completer capabilities is an optional feature, but still we
> might end up end up enabling AtomicOps despite 128-bit/CAS is not
> supported at the root-port.
> 
> Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> ---
>  drivers/infiniband/hw/mlx5/data_direct.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx5/data_direct.c b/drivers/infiniband/hw/mlx5/data_direct.c
> index b81ac5709b56f6ac0d9f60572ce7144258fa2794..112185be53f1ccc6a797e129f24432bdc86008ae 100644
> --- a/drivers/infiniband/hw/mlx5/data_direct.c
> +++ b/drivers/infiniband/hw/mlx5/data_direct.c
> @@ -179,9 +179,9 @@ static int mlx5_data_direct_probe(struct pci_dev *pdev, const struct pci_device_
>  	if (err)
>  		goto err_disable;
>  
> -	if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32) &&
> -	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP64) &&
> -	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP128))
> +	if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
> +						PCI_EXP_DEVCAP2_ATOMIC_COMP64 |
> +						PCI_EXP_DEVCAP2_ATOMIC_COMP128))

I would expect some new define which combines all together, with some
comment why it exists:
#define PCI_ATOMIC_COMP_v7  PCI_EXP_DEVCAP2_ATOMIC_COMP32 | PCI_EXP_DEVCAP2_ATOMIC_COMP64 | PCI_EXP_DEVCAP2_ATOMIC_COMP128

Anyway the change looks right to me.

Thanks,
Acked-by: Leon Romanovsky <leon@kernel.org>

Re: [PATCH RFC 2/2] ib/mlx5: Request PCIe AtomicOps enabled for all 3 sizes

[Gerd Bayer]

On Thu, 2025-11-06 at 12:19 +0200, Leon Romanovsky wrote:
> On Wed, Nov 05, 2025 at 06:55:14PM +0100, Gerd Bayer wrote:
> > Pass fully populated capability bit-mask requesting support for all 3
> > sizes of AtomicOps at once when attempting to enable AtomicOps for PCI
> > function.
> > 
> > When called individually, pci_enable_atomic_ops_to_root() may enable the
> > device to send requests as soon as one size is supported. According to
> > PCIe Spec 7.0 Section 6.15.3.1 support of 32-bit and 64-bit AtomicOps
> > completer capabilities are tied together for root-ports. Only the
> > 128-bit/CAS completer capabilities is an optional feature, but still we
> > might end up end up enabling AtomicOps despite 128-bit/CAS is not
> > supported at the root-port.
> > 
> > Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> > ---
> >  drivers/infiniband/hw/mlx5/data_direct.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/infiniband/hw/mlx5/data_direct.c b/drivers/infiniband/hw/mlx5/data_direct.c
> > index b81ac5709b56f6ac0d9f60572ce7144258fa2794..112185be53f1ccc6a797e129f24432bdc86008ae 100644
> > --- a/drivers/infiniband/hw/mlx5/data_direct.c
> > +++ b/drivers/infiniband/hw/mlx5/data_direct.c
> > @@ -179,9 +179,9 @@ static int mlx5_data_direct_probe(struct pci_dev *pdev, const struct pci_device_
> >  	if (err)
> >  		goto err_disable;
> >  
> > -	if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32) &&
> > -	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP64) &&
> > -	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP128))
> > +	if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
> > +						PCI_EXP_DEVCAP2_ATOMIC_COMP64 |
> > +						PCI_EXP_DEVCAP2_ATOMIC_COMP128))
> 
> I would expect some new define which combines all together, with some
> comment why it exists:
> #define PCI_ATOMIC_COMP_v7  PCI_EXP_DEVCAP2_ATOMIC_COMP32 | PCI_EXP_DEVCAP2_ATOMIC_COMP64 | PCI_EXP_DEVCAP2_ATOMIC_COMP128

I see your point. I don't understand the _v7, though.
Reading PCI Express spec 7.0 section 6.15.3.1 where for root ports
basically just 3 combinations are specified:

 - No support (all 3 sizes off)
 - Basic support (32-bit and 64-bit supported)
 - Full support (Base + 128-bit CAS supported)

I would propose to add the following combined defines to
include/uapi/linux/pci_regs.h - and then use them here:

#PCI_EXP_RP_ATOMIC_COMP_BASE(_SUPPORT) to be the "or" of _COMP32 and
_COMP64 and 
#PCI_EXP_RP_ATOMIC_COMP_FULL(_SUPPORT) to include also 128-bit

But I guess that becomes a PCI question then. @Bjorn?

> Anyway the change looks right to me.
> 
> Thanks,
> Acked-by: Leon Romanovsky <leon@kernel.org>

Re: [PATCH RFC 2/2] ib/mlx5: Request PCIe AtomicOps enabled for all 3 sizes

[Leon Romanovsky]

On Thu, Nov 06, 2025 at 01:16:18PM +0100, Gerd Bayer wrote:
> On Thu, 2025-11-06 at 12:19 +0200, Leon Romanovsky wrote:
> > On Wed, Nov 05, 2025 at 06:55:14PM +0100, Gerd Bayer wrote:
> > > Pass fully populated capability bit-mask requesting support for all 3
> > > sizes of AtomicOps at once when attempting to enable AtomicOps for PCI
> > > function.
> > > 
> > > When called individually, pci_enable_atomic_ops_to_root() may enable the
> > > device to send requests as soon as one size is supported. According to
> > > PCIe Spec 7.0 Section 6.15.3.1 support of 32-bit and 64-bit AtomicOps
> > > completer capabilities are tied together for root-ports. Only the
> > > 128-bit/CAS completer capabilities is an optional feature, but still we
> > > might end up end up enabling AtomicOps despite 128-bit/CAS is not
> > > supported at the root-port.
> > > 
> > > Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> > > ---
> > >  drivers/infiniband/hw/mlx5/data_direct.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/infiniband/hw/mlx5/data_direct.c b/drivers/infiniband/hw/mlx5/data_direct.c
> > > index b81ac5709b56f6ac0d9f60572ce7144258fa2794..112185be53f1ccc6a797e129f24432bdc86008ae 100644
> > > --- a/drivers/infiniband/hw/mlx5/data_direct.c
> > > +++ b/drivers/infiniband/hw/mlx5/data_direct.c
> > > @@ -179,9 +179,9 @@ static int mlx5_data_direct_probe(struct pci_dev *pdev, const struct pci_device_
> > >  	if (err)
> > >  		goto err_disable;
> > >  
> > > -	if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32) &&
> > > -	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP64) &&
> > > -	    pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP128))
> > > +	if (pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
> > > +						PCI_EXP_DEVCAP2_ATOMIC_COMP64 |
> > > +						PCI_EXP_DEVCAP2_ATOMIC_COMP128))
> > 
> > I would expect some new define which combines all together, with some
> > comment why it exists:
> > #define PCI_ATOMIC_COMP_v7  PCI_EXP_DEVCAP2_ATOMIC_COMP32 | PCI_EXP_DEVCAP2_ATOMIC_COMP64 | PCI_EXP_DEVCAP2_ATOMIC_COMP128
> 
> I see your point. I don't understand the _v7

v7 - > PCI spec *v7.0*

But it was just suggestion.

Thanks