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
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>
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>
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
© 2016 - 2025 Red Hat, Inc.