Re: [PATCH v4 1/2] PCI: AtomicOps: Do not enable if root-port capabilities are unknown
From: Gerd Bayer <gbayer@linux.ibm.com>
Date: 2026-03-20 13:26:31
Also in:
linux-pci, linux-rdma, linux-s390, lkml, stable
On Fri, 2026-03-13 at 17:49 +0100, Gerd Bayer wrote:
quoted hunk ↗ jump to hunk
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. Change to 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. Do not enable AtomicOps requests if nothing can be learned about how the device is attached - e.g. if it is on an "isolated" bus, as in s390. Reported-by: Alexander Schmidt <redacted> Cc: stable@vger.kernel.org Fixes: 430a23689dea ("PCI: Add pci_enable_atomic_ops_to_root()") Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com> --- drivers/pci/pci.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 8479c2e1f74f1044416281aba11bf071ea89488a..94e90988df86b3278b1b6abbc326abf9b4a4a962 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c@@ -3676,7 +3676,7 @@ 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; + struct pci_dev *bridge = NULL; u32 cap, ctl2; /*@@ -3714,29 +3714,27 @@ 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 */ + if ((!bridge) || + (pci_pcie_type(bridge) != PCI_EXP_TYPE_ROOT_PORT) || + ((cap & cap_mask) != cap_mask)) + return -EINVAL; +
Sashiko (the new review tool) annotated that this patch may regress how RCiEP's are treated: https://sashiko.dev/#/patchset/20260313-fix_pciatops-v4-0-93bc70a63935%40linux.ibm.com And it has a point: While AtomicOps Requests were allowed for RCiEP's per default - now they're forbidden per default. Turns out that the situation for RCiEP's on all archs is the same as for "isolated functions" on s390: With pure PCI config ops you cannot tell, whether the root-complex supports them or not. I'm thinking about inverting the logic here - and adding an else clause with an arch-specific hook to serve as discriminator. Thoughts anybody?
pcie_capability_set_word(dev, PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_ATOMIC_REQ); return 0;
Thanks, Gerd