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

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

[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

[Prica, Nikola]

Ok. That makes sense. I'll address it in v2 and incorporate Christians comment too.

On 9/3/2026 2:12 PM, sashiko-bot@kernel.org wrote:
> [You don't often get email from sashiko-bot@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> 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

Re: [PATCH v2] 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] Bypassing the cap_mask capability check blindly assumes all requested PCIe AtomicOp sizes are supported, risking Unsupported Request (UR) errors.
--

commit 23f7770048aa6bf9d34e0cd8e0687398f5697661
Author: Nikola Prica <nikola.prica@amd.com>

PCI: Accept AtomicOps already enabled by the hypervisor

This patch modifies pci_enable_atomic_ops_to_root() to support passthrough
guests in headless topologies where no root port is visible. If the hypervisor
has already set PCI_EXP_DEVCTL2_ATOMIC_REQ on the device, it treats AtomicOps
as enabled and returns success instead of failing the topology walk.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b2879a6be5f80..62729ade496fc 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3769,8 +3769,18 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
>  	}
>  
>  	root = pcie_find_root_port(dev);
> -	if (!root)
> +	if (!root) {
> +		/*
> +		 * A hypervisor may expose a headless topology with no
> +		 * visible root port.  If it has already set AtomicOp
> +		 * Requester Enable, 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]
Does unconditionally returning success here bypass the capability mask
validation?

Guest drivers (e.g., bnxt_re, amdgpu, or vfio) call
pci_enable_atomic_ops_to_root() passing a specific cap_mask to probe whether
certain atomic sizes (e.g., 64-bit or 128-bit) are supported by the routing
topology. By returning early, we skip the cap_mask validation against the
root port's DEVCAP2 register that typically happens right after this check.

If the physical root port only supports 32-bit atomics but the driver
believes 64-bit sizes are supported due to this return, could this result in
PCIe Unsupported Request (UR) errors and potentially trigger fatal Advanced
Error Reporting (AER) events that crash the device or VM?

> +
>  		return -EINVAL;
> +	}
>  
>  	pcie_capability_read_dword(root, PCI_EXP_DEVCAP2, &cap);
>  	if ((cap & cap_mask) != cap_mask)

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

[PATCH v2] 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>
---
Changes in v2:
- Be more strict and only apply logic in headless device case.

 drivers/pci/pci.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2879a6be5f8..62729ade496f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3769,8 +3769,18 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 	}
 
 	root = pcie_find_root_port(dev);
-	if (!root)
+	if (!root) {
+		/*
+		 * A hypervisor may expose a headless topology with no
+		 * visible root port.  If it has already set AtomicOp
+		 * Requester Enable, there is nothing more to do.
+		 */
+		pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl2);
+		if (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_REQ)
+			return 0;
+
 		return -EINVAL;
+	}
 
 	pcie_capability_read_dword(root, PCI_EXP_DEVCAP2, &cap);
 	if ((cap & cap_mask) != cap_mask)
-- 
2.34.1

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

[Prica, Nikola]

On 9/7/2026 6:15 PM, sashiko-bot@kernel.org wrote:
> [You don't often get email from sashiko-bot@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Bypassing the cap_mask capability check blindly assumes all requested PCIe AtomicOp sizes are supported, risking Unsupported Request (UR) errors.
> --

In a headless topology with no root port, the hypervisor owns the device
topology and is responsible for presenting it to the guest. The capability
mask check is not bypassed blindly — we rely on the hypervisor to have
verified AtomicOp support before enabling the Requester capability. This
is consistent with the existing trust model for passthrough configurations,
where the hypervisor already controls which PCIe capabilities are exposed
to the guest.

If the hypervisor enables the AtomicOp Requester bit without verifying
target support, that is a hypervisor bug, not something the guest driver
can or should guard against.


> commit 23f7770048aa6bf9d34e0cd8e0687398f5697661
> Author: Nikola Prica <nikola.prica@amd.com>
> 
> PCI: Accept AtomicOps already enabled by the hypervisor
> 
> This patch modifies pci_enable_atomic_ops_to_root() to support passthrough
> guests in headless topologies where no root port is visible. If the hypervisor
> has already set PCI_EXP_DEVCTL2_ATOMIC_REQ on the device, it treats AtomicOps
> as enabled and returns success instead of failing the topology walk.
> 
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index b2879a6be5f80..62729ade496fc 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -3769,8 +3769,18 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
>>        }
>>
>>        root = pcie_find_root_port(dev);
>> -     if (!root)
>> +     if (!root) {
>> +             /*
>> +              * A hypervisor may expose a headless topology with no
>> +              * visible root port.  If it has already set AtomicOp
>> +              * Requester Enable, 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]
> Does unconditionally returning success here bypass the capability mask
> validation?
> 
> Guest drivers (e.g., bnxt_re, amdgpu, or vfio) call
> pci_enable_atomic_ops_to_root() passing a specific cap_mask to probe whether
> certain atomic sizes (e.g., 64-bit or 128-bit) are supported by the routing
> topology. By returning early, we skip the cap_mask validation against the
> root port's DEVCAP2 register that typically happens right after this check.
> 
> If the physical root port only supports 32-bit atomics but the driver
> believes 64-bit sizes are supported due to this return, could this result in
> PCIe Unsupported Request (UR) errors and potentially trigger fatal Advanced
> Error Reporting (AER) events that crash the device or VM?
> 
>> +
>>                return -EINVAL;
>> +     }
>>
>>        pcie_capability_read_dword(root, PCI_EXP_DEVCAP2, &cap);
>>        if ((cap & cap_mask) != cap_mask)
> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260907161701.454364-1-nikprica@amd.com?part=1

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

[Christian König]

On 9/7/26 18:17, 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>

Reviewed-by: Christian König <christian.koenig@amd.com>

Bjorn first of all any objections to this? It sounds save to me, only a handful of drivers actually use this function and it generally seems to be the right things to do.

Then if you agree any objections to up-streaming it through AMDs GPU branch? That would make things a bit easier for us.

Thanks,
Christian.

> ---
> Changes in v2:
> - Be more strict and only apply logic in headless device case.
> 
>  drivers/pci/pci.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b2879a6be5f8..62729ade496f 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3769,8 +3769,18 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
>  	}
>  
>  	root = pcie_find_root_port(dev);
> -	if (!root)
> +	if (!root) {
> +		/*
> +		 * A hypervisor may expose a headless topology with no
> +		 * visible root port.  If it has already set AtomicOp
> +		 * Requester Enable, there is nothing more to do.
> +		 */
> +		pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl2);
> +		if (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_REQ)
> +			return 0;
> +
>  		return -EINVAL;
> +	}
>  
>  	pcie_capability_read_dword(root, PCI_EXP_DEVCAP2, &cap);
>  	if ((cap & cap_mask) != cap_mask)

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

[Prica, Nikola]

Hi Bjorn, any thoughs on this? Do you need more information?

Thanks,
Nikola

On 9/8/2026 10:47 AM, Christian König wrote:
> On 9/7/26 18:17, 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>
> 
> Reviewed-by: Christian König <christian.koenig@amd.com>
> 
> Bjorn first of all any objections to this? It sounds save to me, only a handful of drivers actually use this function and it generally seems to be the right things to do.
> 
> Then if you agree any objections to up-streaming it through AMDs GPU branch? That would make things a bit easier for us.
> 
> Thanks,
> Christian.
> 
>> ---
>> Changes in v2:
>> - Be more strict and only apply logic in headless device case.
>>
>>   drivers/pci/pci.c | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index b2879a6be5f8..62729ade496f 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -3769,8 +3769,18 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
>>   	}
>>   
>>   	root = pcie_find_root_port(dev);
>> -	if (!root)
>> +	if (!root) {
>> +		/*
>> +		 * A hypervisor may expose a headless topology with no
>> +		 * visible root port.  If it has already set AtomicOp
>> +		 * Requester Enable, there is nothing more to do.
>> +		 */
>> +		pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl2);
>> +		if (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_REQ)
>> +			return 0;
>> +
>>   		return -EINVAL;
>> +	}
>>   
>>   	pcie_capability_read_dword(root, PCI_EXP_DEVCAP2, &cap);
>>   	if ((cap & cap_mask) != cap_mask)
>