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

[Gerd Bayer <gbayer@linux.ibm.com>]

On Tue, 2026-03-10 at 16:49 -0500, Bjorn Helgaas wrote:
> On Fri, Mar 06, 2026 at 06:13:58PM +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(-)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 8479c2e1f74f1044416281aba11bf071ea89488a..cc8abe6b1d07661488895876dbbcf8aaeadf4a17 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3663,15 +3663,16 @@ void pci_acs_init(struct pci_dev *dev)
> >  /**
> >   * 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 14f634ab9350d5442192162225b5e5202dbe2308..63ac62b882a94c6873a0db433ba808332ddbea04 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -669,6 +669,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)
> 
> I'm sort of ambivalent about this patch, partly because it adds
> these #defines that aren't used anywhere.  Also, the "BASE" and "FULL"
> names don't contain as much information as mentioning COMP32, COMP64,
> and COMP128 does.

Hi Bjorn,

I see your point. This patch is better suited to lead into a separate
small series that continues on to actually propose corrections of
today's (mis-)use of pci_enable_atomic_ops_to_root() in the
corresponding device drivers.

> If we *do* want this, I think these combo definitions are beyond the
> scope of uapi/linux/pci_regs.h, which generally is just
> transliteration of register bits from the spec.  They could possibly
> go in linux/pci.h where pci_enable_atomic_ops_to_root() is declared.

I like this idea, I'll move the "valid combination" defines to
linux/pci.h

> 
> >  #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.51.0
> > 

Thanks, Gerd