[PATCH] PCI: Accept AtomicOps already enabled by the hypervisor

[PATCH] PCI: Accept AtomicOps already enabled by the hypervisor

[Nikola Prica]

From: Nikola Prica <nikola.prica@amd.com>

pci_enable_atomic_ops_to_root() currently fails when no Root Port is
visible. That is common in passthrough guests (ESXi, Hyper-V): the
endpoint is assigned to the VM, but the guest topology has no Root
Port above it.

In those setups the hypervisor may already have enabled AtomicOp
Requester Enable on the device. If PCI_EXP_DEVCTL2_ATOMIC_REQ is set,
treat AtomicOps as already enabled and return success instead of
failing the Root Port walk.

Signed-off-by: Nikola Prica <nikola.prica@amd.com>
---
 drivers/pci/pci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2879a6be5f8..7c62c7f31757 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3768,6 +3768,14 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 		return -EINVAL;
 	}
 
+	/*
+	 * Some hypervisors already set AtomicOp Requester Enable.  If it
+	 * is already set, there is nothing more to do.
+	 */
+	pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl2);
+	if (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_REQ)
+		return 0;
+
 	root = pcie_find_root_port(dev);
 	if (!root)
 		return -EINVAL;

base-commit: 89a312991dc6e638a36adc43ccb91dbc25504c04
-- 
2.34.1

Re: [PATCH] PCI: Accept AtomicOps already enabled by the hypervisor

[Christian König]

On 9/3/26 14:12, Nikola Prica wrote:
> From: Nikola Prica <nikola.prica@amd.com>
> 
> pci_enable_atomic_ops_to_root() currently fails when no Root Port is
> visible. That is common in passthrough guests (ESXi, Hyper-V): the
> endpoint is assigned to the VM, but the guest topology has no Root
> Port above it.
> 
> In those setups the hypervisor may already have enabled AtomicOp
> Requester Enable on the device. If PCI_EXP_DEVCTL2_ATOMIC_REQ is set,
> treat AtomicOps as already enabled and return success instead of
> failing the Root Port walk.
> 
> Signed-off-by: Nikola Prica <nikola.prica@amd.com>
> ---
>  drivers/pci/pci.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b2879a6be5f8..7c62c7f31757 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3768,6 +3768,14 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * Some hypervisors already set AtomicOp Requester Enable.  If it
> +	 * is already set, there is nothing more to do.
> +	 */
> +	pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl2);
> +	if (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_REQ)
> +		return 0;
> +

I think for a bit more defensiveness we should only do this when we can't find a root port. E.g. under the if (!root) below.

Apart from that looks totally sane to me.

Regards,
Christian.

>  	root = pcie_find_root_port(dev);
>  	if (!root)
>  		return -EINVAL;
> 
> base-commit: 89a312991dc6e638a36adc43ccb91dbc25504c04

Re: [PATCH] PCI: Accept AtomicOps already enabled by the hypervisor

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditionally returning success when ATOMIC_REQ is already set bypasses root port and bridge capability validation.
--

commit 174dddecfad7749ccba344bb23492b342569e86c
Author: Nikola Prica <nikola.prica@amd.com>

PCI: Accept AtomicOps already enabled by the hypervisor

This commit modifies pci_enable_atomic_ops_to_root() to check if the
PCI_EXP_DEVCTL2_ATOMIC_REQ bit is already set by a hypervisor. If it is, the
function returns success early to avoid failing in passthrough topologies
without a visible Root Port.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b2879a6be5f80..7c62c7f317572 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3768,6 +3768,14 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * Some hypervisors already set AtomicOp Requester Enable.  If it
> +	 * is already set, there is nothing more to do.
> +	 */
> +	pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl2);
> +	if (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_REQ)
> +		return 0;

[Severity: High]
Will this unconditional early return bypass the capability validation for newly
requested atomic sizes?

When drivers or subsystems sequentially check for atomic support, they call
pci_enable_atomic_ops_to_root() for different sizes. For example, in
drivers/vfio/pci/vfio_pci_core.c:vfio_pci_info_atomic_cap() (around line
1059), the code sequentially checks for 32-bit and 64-bit atomic support:

vfio_pci_core.c:vfio_pci_info_atomic_cap() {
    ...
    if ((devcap2 & PCI_EXP_DEVCAP2_ATOMIC_COMP64) &&
        !pci_enable_atomic_ops_to_root(pdev, PCI_EXP_DEVCAP2_ATOMIC_COMP64))
        cap.flags |= VFIO_PCI_ATOMIC_COMP64;
    ...
}

A similar sequential check occurs in
drivers/infiniband/hw/bnxt_re/qplib_res.c:bnxt_qplib_determine_atomics()
(around line 1001).

The first successful call (e.g., for COMP32) validates the topology and sets
the ATOMIC_REQ bit on the endpoint. If a subsequent call is made for a larger
size (like COMP64), this new logic sees the ATOMIC_REQ bit is already set and
instantly returns success.

By returning early, doesn't this bypass the cap_mask checks on the root port
and bridges for the larger atomic size?

Could this cause the kernel to incorrectly advertise 64-bit or 128-bit atomic
support to VMs or drivers when the root port only supports 32-bit? If a driver
or VM attempts to use these unsupported sizes, the root port would treat them
as an Unsupported Request (UR), potentially triggering a fatal AER error and
crashing the system or terminating the VM.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903121218.4955-1-nikprica@amd.com?part=1