* [PATCH v6 0/2] PCI: AtomicOps: Fix pci_enable_atomic_ops_to_root()
@ 2026-03-25 15:16 Gerd Bayer
2026-03-25 15:16 ` [PATCH v6 1/2] PCI: AtomicOps: Do not enable without support in root complex Gerd Bayer
0 siblings, 1 reply; 4+ messages in thread
From: Gerd Bayer @ 2026-03-25 15:16 UTC (permalink / raw)
To: Bjorn Helgaas, Jay Cornwall, Felix Kuehling,
Christian Borntraeger, Niklas Schnelle
Cc: Gerald Schaefer, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Sven Schnelle, Leon Romanovsky, 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. One particularly rare setup is a
RCiEP that might act as a AtomicOps Requestor - but was subject to a
regression in v4 as reported by Sashiko.
v5 of this series tries to make no functional changes for that class of
devices either.
Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
Changes in v6:
- Incorporate Ilpo's editorial comments.
- Correct logic in pci_is_atomicops_capable_rp() (annotated by Sashiko)
- Link to v5: https://lore.kernel.org/r/20260323-fix_pciatops-v5-0-fada7233aea8@linux.ibm.com
Changes in v5:
- Introduce new pcibios_connects_to_atomicops_capable_rc() so arch's can
declare AtomicOps support outside of PCIe config space. Defaults to
"true" - except s390.
- rebase to 7.0-rc5
- Link to v4: https://lore.kernel.org/r/20260313-fix_pciatops-v4-0-93bc70a63935@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 without support in root complex
PCI: AtomicOps: Update references to PCIe spec
arch/s390/pci/pci.c | 5 +++++
drivers/pci/pci.c | 54 +++++++++++++++++++++++++++++++++--------------------
include/linux/pci.h | 1 +
3 files changed, 40 insertions(+), 20 deletions(-)
---
base-commit: c369299895a591d96745d6492d4888259b004a9e
change-id: 20251106-fix_pciatops-7e8608eccb03
Best regards,
--
Gerd Bayer <gbayer@linux.ibm.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v6 1/2] PCI: AtomicOps: Do not enable without support in root complex
2026-03-25 15:16 [PATCH v6 0/2] PCI: AtomicOps: Fix pci_enable_atomic_ops_to_root() Gerd Bayer
@ 2026-03-25 15:16 ` Gerd Bayer
2026-03-25 20:08 ` Bjorn Helgaas
0 siblings, 1 reply; 4+ messages in thread
From: Gerd Bayer @ 2026-03-25 15:16 UTC (permalink / raw)
To: Bjorn Helgaas, Jay Cornwall, Felix Kuehling,
Christian Borntraeger, Niklas Schnelle
Cc: Gerald Schaefer, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Sven Schnelle, Leon Romanovsky, 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. Similarly,
AtomicOps requests are enabled for root complex integrated endpoints
(RCiEPs) unconditionally.
Change the 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 - or that the support for
completing AtomicOps at the root complex is announced through some other
arch specific way.
Introduce a new pcibios_connects_to_atomicops_capable_rc() function to
implement the check - and default to always "true". This leaves the
semantics for today's RCiEPs intact. Pass in the device in question and
the requested capabilities for future expansions.
For s390, override pcibios_connects_to_atomicops_capable_rc() to
always return "false".
Do not change the enablement of AtomicOps requests if there is no
positive confirmation that the root complex can complete PCIe AtomicOps.
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>
---
arch/s390/pci/pci.c | 5 +++++
drivers/pci/pci.c | 48 +++++++++++++++++++++++++++++++-----------------
include/linux/pci.h | 1 +
3 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 2a430722cbe415dd56c92fed2e513e524f46481a..a0bef77082a153a258fbe4abb1070b22e020888e 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -265,6 +265,11 @@ static int zpci_cfg_store(struct zpci_dev *zdev, int offset, u32 val, u8 len)
return rc;
}
+bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
+{
+ return false;
+}
+
resource_size_t pcibios_align_resource(void *data, const struct resource *res,
resource_size_t size,
resource_size_t align)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8479c2e1f74f1044416281aba11bf071ea89488a..006aa589926cb290de43f152100ddaf9961407d1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3660,6 +3660,19 @@ void pci_acs_init(struct pci_dev *dev)
pci_disable_broken_acs_cap(dev);
}
+static bool pci_is_atomicops_capable_rp(struct pci_dev *dev, u32 cap, u32 cap_mask)
+{
+ if (!dev || !(pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT))
+ return false;
+
+ return (cap & cap_mask) == cap_mask;
+}
+
+bool __weak pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
+{
+ return true;
+}
+
/**
* pci_enable_atomic_ops_to_root - enable AtomicOp requests to root port
* @dev: the PCI device
@@ -3676,8 +3689,9 @@ 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;
- u32 cap, ctl2;
+ struct pci_dev *bridge = NULL;
+ u32 cap = 0;
+ u32 ctl2;
/*
* Per PCIe r5.0, sec 9.3.5.10, the AtomicOp Requester Enable bit
@@ -3714,29 +3728,29 @@ 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
+ * or firmware asserts support
+ */
+ if (!(pci_is_atomicops_capable_rp(bridge, cap, cap_mask) ||
+ pcibios_connects_to_atomicops_capable_rc(dev, cap_mask)))
+ return -EINVAL;
+
pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
PCI_EXP_DEVCTL2_ATOMIC_REQ);
return 0;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1c270f1d512301de4d462fe7e5097c32af5c6f8d..ef90604c39859ea8e61e5392d0bdaa1b0e43874b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -692,6 +692,7 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
void *release_data);
int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
+bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask);
#define PCI_REGION_FLAG_MASK 0x0fU /* These bits of resource flags tell us the PCI region flags */
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v6 1/2] PCI: AtomicOps: Do not enable without support in root complex
2026-03-25 15:16 ` [PATCH v6 1/2] PCI: AtomicOps: Do not enable without support in root complex Gerd Bayer
@ 2026-03-25 20:08 ` Bjorn Helgaas
2026-03-26 9:51 ` Gerd Bayer
0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Helgaas @ 2026-03-25 20:08 UTC (permalink / raw)
To: Gerd Bayer
Cc: Bjorn Helgaas, Jay Cornwall, Felix Kuehling,
Christian Borntraeger, Niklas Schnelle, Gerald Schaefer,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
Leon Romanovsky, Alexander Schmidt, linux-s390, linux-pci,
linux-kernel, netdev, linux-rdma, stable
On Wed, Mar 25, 2026 at 04:16:17PM +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. Similarly,
> AtomicOps requests are enabled for root complex integrated endpoints
> (RCiEPs) unconditionally.
>
> Change the 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 - or that the support for
> completing AtomicOps at the root complex is announced through some other
> arch specific way.
>
> Introduce a new pcibios_connects_to_atomicops_capable_rc() function to
> implement the check - and default to always "true". This leaves the
> semantics for today's RCiEPs intact. Pass in the device in question and
> the requested capabilities for future expansions.
> For s390, override pcibios_connects_to_atomicops_capable_rc() to
> always return "false".
>
> Do not change the enablement of AtomicOps requests if there is no
> positive confirmation that the root complex can complete PCIe AtomicOps.
>
> 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>
> ---
> arch/s390/pci/pci.c | 5 +++++
> drivers/pci/pci.c | 48 +++++++++++++++++++++++++++++++-----------------
> include/linux/pci.h | 1 +
> 3 files changed, 37 insertions(+), 17 deletions(-)
>
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 2a430722cbe415dd56c92fed2e513e524f46481a..a0bef77082a153a258fbe4abb1070b22e020888e 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -265,6 +265,11 @@ static int zpci_cfg_store(struct zpci_dev *zdev, int offset, u32 val, u8 len)
> return rc;
> }
>
> +bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
> +{
> + return false;
> +}
> +
> resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> resource_size_t size,
> resource_size_t align)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8479c2e1f74f1044416281aba11bf071ea89488a..006aa589926cb290de43f152100ddaf9961407d1 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3660,6 +3660,19 @@ void pci_acs_init(struct pci_dev *dev)
> pci_disable_broken_acs_cap(dev);
> }
>
> +static bool pci_is_atomicops_capable_rp(struct pci_dev *dev, u32 cap, u32 cap_mask)
> +{
> + if (!dev || !(pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT))
> + return false;
> +
> + return (cap & cap_mask) == cap_mask;
> +}
> +
> +bool __weak pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
> +{
> + return true;
> +}
> +
> /**
> * pci_enable_atomic_ops_to_root - enable AtomicOp requests to root port
> * @dev: the PCI device
> @@ -3676,8 +3689,9 @@ 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;
> - u32 cap, ctl2;
> + struct pci_dev *bridge = NULL;
> + u32 cap = 0;
> + u32 ctl2;
>
> /*
> * Per PCIe r5.0, sec 9.3.5.10, the AtomicOp Requester Enable bit
> @@ -3714,29 +3728,29 @@ 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
> + * or firmware asserts support
> + */
> + if (!(pci_is_atomicops_capable_rp(bridge, cap, cap_mask) ||
> + pcibios_connects_to_atomicops_capable_rc(dev, cap_mask)))
> + return -EINVAL;
Sashiko says:
Since the generic weak implementation of
pcibios_connects_to_atomicops_capable_rc() unconditionally returns
true, the logical OR expression pci_is_atomicops_capable_rp(...) ||
true will always evaluate to true. This makes the entire if
condition evaluate to false.
Because of this, it appears -EINVAL is never returned here, and any
standard endpoint behind a Root Port will successfully be granted
AtomicOps even if the Root Port lacks the capability in its
PCI_EXP_DEVCAP2 register.
> +
> pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
> PCI_EXP_DEVCTL2_ATOMIC_REQ);
> return 0;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 1c270f1d512301de4d462fe7e5097c32af5c6f8d..ef90604c39859ea8e61e5392d0bdaa1b0e43874b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -692,6 +692,7 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
> void *release_data);
>
> int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
> +bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask);
>
> #define PCI_REGION_FLAG_MASK 0x0fU /* These bits of resource flags tell us the PCI region flags */
>
>
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v6 1/2] PCI: AtomicOps: Do not enable without support in root complex
2026-03-25 20:08 ` Bjorn Helgaas
@ 2026-03-26 9:51 ` Gerd Bayer
0 siblings, 0 replies; 4+ messages in thread
From: Gerd Bayer @ 2026-03-26 9:51 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bjorn Helgaas, Jay Cornwall, Felix Kuehling,
Christian Borntraeger, Niklas Schnelle, Gerald Schaefer,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Sven Schnelle,
Leon Romanovsky, Alexander Schmidt, linux-s390, linux-pci,
linux-kernel, netdev, linux-rdma, stable, Gerd Bayer
On Wed, 2026-03-25 at 15:08 -0500, Bjorn Helgaas wrote:
> On Wed, Mar 25, 2026 at 04:16:17PM +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. Similarly,
> > AtomicOps requests are enabled for root complex integrated endpoints
> > (RCiEPs) unconditionally.
> >
> > Change the 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 - or that the support for
> > completing AtomicOps at the root complex is announced through some other
> > arch specific way.
> >
> > Introduce a new pcibios_connects_to_atomicops_capable_rc() function to
> > implement the check - and default to always "true". This leaves the
> > semantics for today's RCiEPs intact. Pass in the device in question and
> > the requested capabilities for future expansions.
> > For s390, override pcibios_connects_to_atomicops_capable_rc() to
> > always return "false".
> >
> > Do not change the enablement of AtomicOps requests if there is no
> > positive confirmation that the root complex can complete PCIe AtomicOps.
> >
> > 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>
> > ---
> > arch/s390/pci/pci.c | 5 +++++
> > drivers/pci/pci.c | 48 +++++++++++++++++++++++++++++++-----------------
> > include/linux/pci.h | 1 +
> > 3 files changed, 37 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> > index 2a430722cbe415dd56c92fed2e513e524f46481a..a0bef77082a153a258fbe4abb1070b22e020888e 100644
> > --- a/arch/s390/pci/pci.c
> > +++ b/arch/s390/pci/pci.c
> > @@ -265,6 +265,11 @@ static int zpci_cfg_store(struct zpci_dev *zdev, int offset, u32 val, u8 len)
> > return rc;
> > }
> >
> > +bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
> > +{
> > + return false;
> > +}
> > +
> > resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> > resource_size_t size,
> > resource_size_t align)
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 8479c2e1f74f1044416281aba11bf071ea89488a..006aa589926cb290de43f152100ddaf9961407d1 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3660,6 +3660,19 @@ void pci_acs_init(struct pci_dev *dev)
> > pci_disable_broken_acs_cap(dev);
> > }
> >
> > +static bool pci_is_atomicops_capable_rp(struct pci_dev *dev, u32 cap, u32 cap_mask)
> > +{
> > + if (!dev || !(pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT))
> > + return false;
> > +
> > + return (cap & cap_mask) == cap_mask;
> > +}
> > +
> > +bool __weak pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask)
> > +{
> > + return true;
> > +}
> > +
> > /**
> > * pci_enable_atomic_ops_to_root - enable AtomicOp requests to root port
> > * @dev: the PCI device
> > @@ -3676,8 +3689,9 @@ 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;
> > - u32 cap, ctl2;
> > + struct pci_dev *bridge = NULL;
> > + u32 cap = 0;
> > + u32 ctl2;
> >
> > /*
> > * Per PCIe r5.0, sec 9.3.5.10, the AtomicOp Requester Enable bit
> > @@ -3714,29 +3728,29 @@ 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
> > + * or firmware asserts support
> > + */
> > + if (!(pci_is_atomicops_capable_rp(bridge, cap, cap_mask) ||
> > + pcibios_connects_to_atomicops_capable_rc(dev, cap_mask)))
> > + return -EINVAL;
>
> Sashiko says:
>
> Since the generic weak implementation of
> pcibios_connects_to_atomicops_capable_rc() unconditionally returns
> true, the logical OR expression pci_is_atomicops_capable_rp(...) ||
> true will always evaluate to true. This makes the entire if
> condition evaluate to false.
>
> Because of this, it appears -EINVAL is never returned here, and any
> standard endpoint behind a Root Port will successfully be granted
> AtomicOps even if the Root Port lacks the capability in its
> PCI_EXP_DEVCAP2 register.
I've made the generic implementation of
pcibios_connects_to_atomicops_capable_rc() default to return "true" to
preserve the current code's handling of RCiEPs: Since they are not
attached to a root port, their dev->bus->parent is NULL and the entire
while-loop is bypassed - before this patch and after. (Sashiko was
pointing at that being regressed with v4.)
The whole point of pcibios_connects_to_atomicops_capable_rc() is to
allow different architectures to implement a discriminator outside of
PCIe's structure - potentially depending on CPU model or more.
The only point I wonder about: Should
pcibios_connects_to_atomicops_capable_rc() default to return "false"
and deliberately change the behavior for today's RCiEP's (if there are
any...)?
>
> > +
> > pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
> > PCI_EXP_DEVCTL2_ATOMIC_REQ);
> > return 0;
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 1c270f1d512301de4d462fe7e5097c32af5c6f8d..ef90604c39859ea8e61e5392d0bdaa1b0e43874b 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -692,6 +692,7 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
> > void *release_data);
> >
> > int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
> > +bool pcibios_connects_to_atomicops_capable_rc(struct pci_dev *dev, u32 cap_mask);
> >
> > #define PCI_REGION_FLAG_MASK 0x0fU /* These bits of resource flags tell us the PCI region flags */
> >
> >
> > --
> > 2.51.0
> >
Thanks,
Gerd
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-26 9:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-25 15:16 [PATCH v6 0/2] PCI: AtomicOps: Fix pci_enable_atomic_ops_to_root() Gerd Bayer
2026-03-25 15:16 ` [PATCH v6 1/2] PCI: AtomicOps: Do not enable without support in root complex Gerd Bayer
2026-03-25 20:08 ` Bjorn Helgaas
2026-03-26 9:51 ` Gerd Bayer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox