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

[Bjorn Helgaas <helgaas@kernel.org>]

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.

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.

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