Hi Bjorn et al.
this series addresses a few issues that have come up with the helper function that enables Atomic Op Requests to be initiated by PCI enpoints:
A. Most in-tree users of this helper use it incorrectly [0]. B. On s390, Atomic Op Requests are enabled, although the helper cannot know whether the root port is really supporting them. C. Loop control in the helper function does not guarantee that a root port's capabilities are ever checked against those requested by the caller.
Address these issue with the following patches: Patch 1: Make it harder to mis-use the enablement function, Patch 2: Addresses issues B. and C.
I did test that issue B is fixed with these patches. Also, I verified that Atomic Ops enablement on a Mellanox/Nvidia ConnectX-6 adapter plugged straight into the root port of a x86 system still gets AtomicOp Requests enabled. However, I did not test this with any PCIe switches between root port and endpoint.
Ideally, both patches would be incorporated immediately, so we could start correcting the mis-uses in the device drivers. I don't know of any complaints when using Atomic Ops on devices where the driver is mis-using the helper. Patch 2 however, is fixing an obseved issue.
[0]: https://lore.kernel.org/all/fbe34de16f5c0bf25a16f9819a57fdd81e5bb08c.camel@l... [1]: https://lore.kernel.org/all/20251105-mlxatomics-v1-0-10c71649e08d@linux.ibm....
Signed-off-by: Gerd Bayer gbayer@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....
--- Gerd Bayer (2): PCI: AtomicOps: Define valid root port capabilities PCI: AtomicOps: Fix logic in enable function
drivers/pci/pci.c | 43 +++++++++++++++++++++---------------------- include/uapi/linux/pci_regs.h | 8 ++++++++ 2 files changed, 29 insertions(+), 22 deletions(-) --- base-commit: 40fbbd64bba6c6e7a72885d2f59b6a3be9991eeb change-id: 20251106-fix_pciatops-7e8608eccb03
Best regards,
Move the check for root port requirements past the loop within pci_enable_atomic_ops_to_root() that checks on potential switch (up- and downstream) ports.
Inside the loop traversing the PCI tree upwards, prepend the switch case to validate the routing capability on any port with a fallthrough-case that does the additional check for Atomic Ops not being blocked on upstream ports.
Do not enable Atomic Op 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 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 --- 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 d2261ac964316f3fc3efc4d5b30cf821ac46d75d..5d25d42eece1bbaf16197068d7b6206937e9c3a0 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3675,7 +3675,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;
/* @@ -3713,29 +3713,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; + pcie_capability_set_word(dev, PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_ATOMIC_REQ); return 0;
linux-stable-mirror@lists.linaro.org
-
Gerd Bayer