On Mon, 2 Feb 2026 09:11:28 +0000
"Pekovic, Manojlo" <[email protected]> wrote:

> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Alex,
> 
> Is there any new info on this issue?
> 
> Thanks in advance,
> Manojlo Pekovic
> Software Development Engineer 2
> Cloud Software Team
> 
> 
> 
> -----Original Message-----
> From: Pekovic, Manojlo
> Sent: Tuesday, January 20, 2026 5:01 PM
> To: 'Alex Williamson' <[email protected]>; '[email protected]' 
> <[email protected]>
> Cc: Prica, Nikola <[email protected]>; Andjelkovic, Dejan 
> <[email protected]>; 'Cédric Le Goater' <[email protected]>
> Subject: RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices
> 
> Hi Alex,
> 
> Here is the second part of the patch.
> I didn't take into consideration the non-vfio-device in my first patch.
> I fixed it by using standard that I saw in the code for getting vdev in this 
> scenario, hope that it is satisfactory now
> 
> Previously, PCIe Atomic Ops support was only enabled for single function 
> devices due to potential conflicting capabilities between functions.
> This patch enables atomic ops for multifunction devices by finding the common 
> subset of atomic capabilities supported by all functions.
> 
> The implementation checks all functions on the same slot and advertises only 
> the atomic operations supported by all of them, ensuring compatibility across 
> the multifunction device.
> 
> Signed-off-by: Manojlo Pekovic <[email protected]>
> ---
>  hw/vfio/pci.c | 47 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 6a6c8f1807..a6723063ab 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1931,6 +1931,34 @@ static uint32_t vfio_get_atomic_cap(VFIOPCIDevice 
> *vdev)
>      return mask;
>  }
> 
> +/* Returns biggest subset of supported atomic ops of multifunction
> +device */ static uint32_t vfio_get_multifunction_atomic_cap(VFIOPCIDevice 
> *vdev,
> +                                                    PCIBus *bus) {
> +    PCIDevice *func_dev;
> +    VFIOPCIDevice *func_vdev;
> +    int slot = PCI_SLOT(vdev->pdev.devfn);
> +    int target_devfn;
> +    uint32_t common_mask = PCI_EXP_DEVCAP2_ATOMIC_COMP32 |
> +                           PCI_EXP_DEVCAP2_ATOMIC_COMP64 |
> +                           PCI_EXP_DEVCAP2_ATOMIC_COMP128;
> +
> +    for (int fn = 0; fn < PCI_FUNC_MAX; fn++) {
> +        target_devfn = PCI_DEVFN(slot, fn);
> +        func_dev = pci_find_device(bus, pci_bus_num(bus),
> + target_devfn);
> +
> +        if (!func_dev) {
> +            continue;
> +        }
> +
> +        func_vdev = VFIO_PCI(func_dev);

As Cedric notes, best to send a v2 rather than bury new revisions in
replies.  However, I think this was actually better in the first
version.  Previously it was unclear how we could skip taking
non-vfio-pci device atomic ops support into account when creating the
common mask.  This implementation will segfault if it encounters the
condition of a multi-function device composed of vfio-pci and
non-vfio-pci devices.

Also, since this is called from the function zero realize path, can we
actually rely on the other functions being populated yet?  It seems
like this would need to be evaluated in some sort of machine ready
context.  Thanks,

Alex

> +
> +        common_mask &= vfio_get_atomic_cap(func_vdev);;
> +    }
> +
> +    return common_mask;
> +}
> +
>  static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)  {
>      PCIBus *bus = pci_get_bus(&vdev->pdev); @@ -1941,15 +1969,12 @@ static 
> void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
>      /*
>       * PCIe Atomic Ops completer support is only added automatically for 
> single
>       * function devices downstream of a root port supporting DEVCAP2.  
> Support
> -     * is added during realize and, if added, removed during device exit.  
> The
> -     * single function requirement avoids conflicting requirements should a
> -     * slot be composed of multiple devices with differing capabilities.
> +     * is added during realize and, if added, removed during device exit.
>       */
>      if (pci_bus_is_root(bus) || !parent || !parent->exp.exp_cap ||
>          pcie_cap_get_type(parent) != PCI_EXP_TYPE_ROOT_PORT ||
>          pcie_cap_get_version(parent) != PCI_EXP_FLAGS_VER2 ||
> -        vdev->pdev.devfn ||
> -        vdev->pdev.cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> +        vdev->pdev.devfn) {
>          return;
>      }
> 
> @@ -1961,8 +1986,18 @@ static void vfio_pci_enable_rp_atomics(VFIOPCIDevice 
> *vdev)
>                               PCI_EXP_DEVCAP2_ATOMIC_COMP128)) {
>          return;
>      }
> +
> 
> -    mask = vfio_get_atomic_cap(vdev);
> +    if (vdev->pdev.cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> +        /*
> +         * Only process the through the root function
> +         * We get through here only on function 0,
> +         * through check vdev->pdev.devfn
> +         */
> +        mask = vfio_get_multifunction_atomic_cap(vdev, bus);
> +    } else {
> +        mask = vfio_get_atomic_cap(vdev);
> +    }
> 
>      if (!mask) {
>          return;
> --
> 2.43.0

Reply via email to