* [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