RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices

Pekovic, Manojlo posted 1 patch 2 weeks, 3 days ago
Failed in applying to current master (apply log)
There is a newer version of this series
hw/vfio/pci.c | 53 ++++++++++++++++++++++++++++++---------------------
1 file changed, 31 insertions(+), 22 deletions(-)
RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices
Posted by Pekovic, Manojlo 2 weeks, 3 days ago
[AMD Official Use Only - AMD Internal Distribution Only]

+ others, on accident


Manojlo Pekovic
Software Development Engineer 2
Cloud Software Team



-----Original Message-----
From: Pekovic, Manojlo
Sent: Tuesday, January 20, 2026 4:59 PM
To: Alex Williamson <alex@shazbot.org>
Subject: RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices

Hi Alex,
Hope you are good. Sorry for such a late response.

Appreciate the comments

I have split up the fix into two patches as you said, and will be sending them into two emails so it's easier for you to check them In this mail, I am sending the helper patch and in the next one the multifunction

Extract the code that reads and converts VFIO atomic capabilities into a separate helper function. This is a preparatory refactor with no functional change, making the code easier to extend for multifunction device support.

Signed-off-by: Manojlo Pekovic <manojlo.pekovic@amd.com>
---
 hw/vfio/pci.c | 53 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 14bcc725c3..6a6c8f1807 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1900,13 +1900,41 @@ static void vfio_add_emulated_long(VFIOPCIDevice *vdev, int pos,
     vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask);  }

-static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
+static uint32_t vfio_get_atomic_cap(VFIOPCIDevice *vdev)
 {
     struct vfio_device_info_cap_pci_atomic_comp *cap;
     g_autofree struct vfio_device_info *info = NULL;
+    struct vfio_info_cap_header *hdr;
+    uint32_t mask = 0;
+
+    info = vfio_get_device_info(vdev->vbasedev.fd);
+    if (!info) {
+        return mask;
+    }
+
+    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
+    if (!hdr) {
+        return mask;
+    }
+
+    cap = (void *)hdr;
+    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
+        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
+    }
+    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
+        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
+    }
+    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
+        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
+    }
+
+    return mask;
+}
+
+static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev) {
     PCIBus *bus = pci_get_bus(&vdev->pdev);
     PCIDevice *parent = bus->parent_dev;
-    struct vfio_info_cap_header *hdr;
     uint32_t mask = 0;
     uint8_t *pos;

@@ -1934,26 +1962,7 @@ static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
         return;
     }

-    info = vfio_get_device_info(vdev->vbasedev.fd);
-    if (!info) {
-        return;
-    }
-
-    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
-    if (!hdr) {
-        return;
-    }
-
-    cap = (void *)hdr;
-    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
-        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
-    }
-    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
-        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
-    }
-    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
-        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
-    }
+    mask = vfio_get_atomic_cap(vdev);

     if (!mask) {
         return;
--
2.43.0



Manojlo Pekovic
Software Development Engineer 2
Cloud Software Team



-----Original Message-----
From: Alex Williamson <alex@shazbot.org>
Sent: Monday, January 12, 2026 5:17 PM
To: Pekovic, Manojlo <Manojlo.Pekovic@amd.com>
Cc: qemu-devel@nongnu.org; alex.williamson@redhat.com; clg@redhat.com; Prica, Nikola <Nikola.Prica@amd.com>; Andjelkovic, Dejan <Dejan.Andjelkovic@amd.com>
Subject: Re: [PATCH] vfio/pci: Enable atomic ops for multifunction devices

On Tue, 9 Dec 2025 14:32:50 +0000
Manojlo Pekovic <manojlo.pekovic@amd.com> wrote:

> 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 <manojlo.pekovic@amd.com>
> ---
>  hw/vfio/pci.c | 104
> +++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 77 insertions(+), 27 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index
> 14bcc725c3..9d1faeabff 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1900,28 +1900,88 @@ static void vfio_add_emulated_long(VFIOPCIDevice *vdev, int pos,
>      vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask);
> }
>
> -static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
> +static uint32_t vfio_get_atomic_cap(VFIOPCIDevice *vdev)
>  {
> -    struct vfio_device_info_cap_pci_atomic_comp *cap;
>      g_autofree struct vfio_device_info *info = NULL;
> +    struct vfio_info_cap_header *hdr;
> +    struct vfio_device_info_cap_pci_atomic_comp *cap;
> +    uint32_t mask = 0;
> +
> +    info = vfio_get_device_info(vdev->vbasedev.fd);
> +    if (!info) {
> +        return mask;
> +    }
> +
> +    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
> +    if (!hdr) {
> +        return mask;
> +    }
> +
> +    cap = (void *)hdr;
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
> +    }
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
> +    }
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
> +    }
> +
> +    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);
> +        uint32_t func_mask = 0;
> +
> +        if (!func_dev) {
> +            continue;
> +        }
> +
> +        func_vdev = (VFIOPCIDevice *)object_dynamic_cast(OBJECT(func_dev),
> +                                                            TYPE_VFIO_PCI);
> +        if (!func_vdev) {
> +            continue;
> +        }

Why is it justified to continue here?  It at least seems worthy of a comment why we can ignore non-vfio-pci devices relative to the advertised atomic op support.

> +
> +        func_mask = vfio_get_atomic_cap(func_vdev);
> +
> +        common_mask &= func_mask;

Factor out func_mask.

> +    }
> +
> +    return common_mask;
> +}
> +
> +static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev) {
>      PCIBus *bus = pci_get_bus(&vdev->pdev);
>      PCIDevice *parent = bus->parent_dev;
> -    struct vfio_info_cap_header *hdr;
>      uint32_t mask = 0;
>      uint8_t *pos;
>
>      /*
>       * 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;
>      }
>
> @@ -1934,25 +1994,15 @@ static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
>          return;
>      }
>
> -    info = vfio_get_device_info(vdev->vbasedev.fd);
> -    if (!info) {
> -        return;
> -    }
> -
> -    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
> -    if (!hdr) {
> -        return;
> -    }
> -
> -    cap = (void *)hdr;
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
> -    }
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
> -    }
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;

Nit, it would have been cleaner to factor out this helper as a precursor to multifunction support.  Thanks,

Alex

> +    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) {
RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices
Posted by Pekovic, Manojlo 2 weeks, 3 days ago
[AMD Official Use Only - AMD Internal Distribution Only]

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 <manojlo.pekovic@amd.com>
---
 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);
+
+        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


Thanks and Best Regards,
Manojlo Pekovic
Software Development Engineer 2
Cloud Software Team



-----Original Message-----
From: Pekovic, Manojlo
Sent: Tuesday, January 20, 2026 5:00 PM
To: 'Alex Williamson' <alex@shazbot.org>; qemu-devel@nongnu.org
Cc: Prica, Nikola <Nikola.Prica@amd.com>; Andjelkovic, Dejan <Dejan.Andjelkovic@amd.com>; Cédric Le Goater <clg@redhat.com>
Subject: RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices

+ others, on accident


Manojlo Pekovic
Software Development Engineer 2
Cloud Software Team



-----Original Message-----
From: Pekovic, Manojlo
Sent: Tuesday, January 20, 2026 4:59 PM
To: Alex Williamson <alex@shazbot.org>
Subject: RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices

Hi Alex,
Hope you are good. Sorry for such a late response.

Appreciate the comments

I have split up the fix into two patches as you said, and will be sending them into two emails so it's easier for you to check them In this mail, I am sending the helper patch and in the next one the multifunction

Extract the code that reads and converts VFIO atomic capabilities into a separate helper function. This is a preparatory refactor with no functional change, making the code easier to extend for multifunction device support.

Signed-off-by: Manojlo Pekovic <manojlo.pekovic@amd.com>
---
 hw/vfio/pci.c | 53 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 14bcc725c3..6a6c8f1807 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1900,13 +1900,41 @@ static void vfio_add_emulated_long(VFIOPCIDevice *vdev, int pos,
     vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask);  }

-static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
+static uint32_t vfio_get_atomic_cap(VFIOPCIDevice *vdev)
 {
     struct vfio_device_info_cap_pci_atomic_comp *cap;
     g_autofree struct vfio_device_info *info = NULL;
+    struct vfio_info_cap_header *hdr;
+    uint32_t mask = 0;
+
+    info = vfio_get_device_info(vdev->vbasedev.fd);
+    if (!info) {
+        return mask;
+    }
+
+    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
+    if (!hdr) {
+        return mask;
+    }
+
+    cap = (void *)hdr;
+    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
+        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
+    }
+    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
+        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
+    }
+    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
+        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
+    }
+
+    return mask;
+}
+
+static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev) {
     PCIBus *bus = pci_get_bus(&vdev->pdev);
     PCIDevice *parent = bus->parent_dev;
-    struct vfio_info_cap_header *hdr;
     uint32_t mask = 0;
     uint8_t *pos;

@@ -1934,26 +1962,7 @@ static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
         return;
     }

-    info = vfio_get_device_info(vdev->vbasedev.fd);
-    if (!info) {
-        return;
-    }
-
-    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
-    if (!hdr) {
-        return;
-    }
-
-    cap = (void *)hdr;
-    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
-        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
-    }
-    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
-        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
-    }
-    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
-        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
-    }
+    mask = vfio_get_atomic_cap(vdev);

     if (!mask) {
         return;
--
2.43.0



Manojlo Pekovic
Software Development Engineer 2
Cloud Software Team



-----Original Message-----
From: Alex Williamson <alex@shazbot.org>
Sent: Monday, January 12, 2026 5:17 PM
To: Pekovic, Manojlo <Manojlo.Pekovic@amd.com>
Cc: qemu-devel@nongnu.org; alex.williamson@redhat.com; clg@redhat.com; Prica, Nikola <Nikola.Prica@amd.com>; Andjelkovic, Dejan <Dejan.Andjelkovic@amd.com>
Subject: Re: [PATCH] vfio/pci: Enable atomic ops for multifunction devices

On Tue, 9 Dec 2025 14:32:50 +0000
Manojlo Pekovic <manojlo.pekovic@amd.com> wrote:

> 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 <manojlo.pekovic@amd.com>
> ---
>  hw/vfio/pci.c | 104
> +++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 77 insertions(+), 27 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index
> 14bcc725c3..9d1faeabff 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1900,28 +1900,88 @@ static void vfio_add_emulated_long(VFIOPCIDevice *vdev, int pos,
>      vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask);
> }
>
> -static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
> +static uint32_t vfio_get_atomic_cap(VFIOPCIDevice *vdev)
>  {
> -    struct vfio_device_info_cap_pci_atomic_comp *cap;
>      g_autofree struct vfio_device_info *info = NULL;
> +    struct vfio_info_cap_header *hdr;
> +    struct vfio_device_info_cap_pci_atomic_comp *cap;
> +    uint32_t mask = 0;
> +
> +    info = vfio_get_device_info(vdev->vbasedev.fd);
> +    if (!info) {
> +        return mask;
> +    }
> +
> +    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
> +    if (!hdr) {
> +        return mask;
> +    }
> +
> +    cap = (void *)hdr;
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
> +    }
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
> +    }
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
> +    }
> +
> +    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);
> +        uint32_t func_mask = 0;
> +
> +        if (!func_dev) {
> +            continue;
> +        }
> +
> +        func_vdev = (VFIOPCIDevice *)object_dynamic_cast(OBJECT(func_dev),
> +                                                            TYPE_VFIO_PCI);
> +        if (!func_vdev) {
> +            continue;
> +        }

Why is it justified to continue here?  It at least seems worthy of a comment why we can ignore non-vfio-pci devices relative to the advertised atomic op support.

> +
> +        func_mask = vfio_get_atomic_cap(func_vdev);
> +
> +        common_mask &= func_mask;

Factor out func_mask.

> +    }
> +
> +    return common_mask;
> +}
> +
> +static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev) {
>      PCIBus *bus = pci_get_bus(&vdev->pdev);
>      PCIDevice *parent = bus->parent_dev;
> -    struct vfio_info_cap_header *hdr;
>      uint32_t mask = 0;
>      uint8_t *pos;
>
>      /*
>       * 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;
>      }
>
> @@ -1934,25 +1994,15 @@ static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
>          return;
>      }
>
> -    info = vfio_get_device_info(vdev->vbasedev.fd);
> -    if (!info) {
> -        return;
> -    }
> -
> -    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
> -    if (!hdr) {
> -        return;
> -    }
> -
> -    cap = (void *)hdr;
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
> -    }
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
> -    }
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;

Nit, it would have been cleaner to factor out this helper as a precursor to multifunction support.  Thanks,

Alex

> +    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) {
RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices
Posted by Pekovic, Manojlo 4 days, 20 hours ago
[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' <alex@shazbot.org>; 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>
Cc: Prica, Nikola <Nikola.Prica@amd.com>; Andjelkovic, Dejan <Dejan.Andjelkovic@amd.com>; 'Cédric Le Goater' <clg@redhat.com>
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 <manojlo.pekovic@amd.com>
---
 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);
+
+        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


Thanks and Best Regards,
Manojlo Pekovic
Software Development Engineer 2
Cloud Software Team



-----Original Message-----
From: Pekovic, Manojlo
Sent: Tuesday, January 20, 2026 5:00 PM
To: 'Alex Williamson' <alex@shazbot.org>; qemu-devel@nongnu.org
Cc: Prica, Nikola <Nikola.Prica@amd.com>; Andjelkovic, Dejan <Dejan.Andjelkovic@amd.com>; Cédric Le Goater <clg@redhat.com>
Subject: RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices

+ others, on accident


Manojlo Pekovic
Software Development Engineer 2
Cloud Software Team



-----Original Message-----
From: Pekovic, Manojlo
Sent: Tuesday, January 20, 2026 4:59 PM
To: Alex Williamson <alex@shazbot.org>
Subject: RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices

Hi Alex,
Hope you are good. Sorry for such a late response.

Appreciate the comments

I have split up the fix into two patches as you said, and will be sending them into two emails so it's easier for you to check them In this mail, I am sending the helper patch and in the next one the multifunction

Extract the code that reads and converts VFIO atomic capabilities into a separate helper function. This is a preparatory refactor with no functional change, making the code easier to extend for multifunction device support.

Signed-off-by: Manojlo Pekovic <manojlo.pekovic@amd.com>
---
 hw/vfio/pci.c | 53 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 14bcc725c3..6a6c8f1807 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1900,13 +1900,41 @@ static void vfio_add_emulated_long(VFIOPCIDevice *vdev, int pos,
     vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask);  }

-static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
+static uint32_t vfio_get_atomic_cap(VFIOPCIDevice *vdev)
 {
     struct vfio_device_info_cap_pci_atomic_comp *cap;
     g_autofree struct vfio_device_info *info = NULL;
+    struct vfio_info_cap_header *hdr;
+    uint32_t mask = 0;
+
+    info = vfio_get_device_info(vdev->vbasedev.fd);
+    if (!info) {
+        return mask;
+    }
+
+    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
+    if (!hdr) {
+        return mask;
+    }
+
+    cap = (void *)hdr;
+    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
+        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
+    }
+    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
+        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
+    }
+    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
+        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
+    }
+
+    return mask;
+}
+
+static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev) {
     PCIBus *bus = pci_get_bus(&vdev->pdev);
     PCIDevice *parent = bus->parent_dev;
-    struct vfio_info_cap_header *hdr;
     uint32_t mask = 0;
     uint8_t *pos;

@@ -1934,26 +1962,7 @@ static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
         return;
     }

-    info = vfio_get_device_info(vdev->vbasedev.fd);
-    if (!info) {
-        return;
-    }
-
-    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
-    if (!hdr) {
-        return;
-    }
-
-    cap = (void *)hdr;
-    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
-        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
-    }
-    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
-        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
-    }
-    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
-        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
-    }
+    mask = vfio_get_atomic_cap(vdev);

     if (!mask) {
         return;
--
2.43.0



Manojlo Pekovic
Software Development Engineer 2
Cloud Software Team



-----Original Message-----
From: Alex Williamson <alex@shazbot.org>
Sent: Monday, January 12, 2026 5:17 PM
To: Pekovic, Manojlo <Manojlo.Pekovic@amd.com>
Cc: qemu-devel@nongnu.org; alex.williamson@redhat.com; clg@redhat.com; Prica, Nikola <Nikola.Prica@amd.com>; Andjelkovic, Dejan <Dejan.Andjelkovic@amd.com>
Subject: Re: [PATCH] vfio/pci: Enable atomic ops for multifunction devices

On Tue, 9 Dec 2025 14:32:50 +0000
Manojlo Pekovic <manojlo.pekovic@amd.com> wrote:

> 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 <manojlo.pekovic@amd.com>
> ---
>  hw/vfio/pci.c | 104
> +++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 77 insertions(+), 27 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index
> 14bcc725c3..9d1faeabff 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1900,28 +1900,88 @@ static void vfio_add_emulated_long(VFIOPCIDevice *vdev, int pos,
>      vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask);
> }
>
> -static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
> +static uint32_t vfio_get_atomic_cap(VFIOPCIDevice *vdev)
>  {
> -    struct vfio_device_info_cap_pci_atomic_comp *cap;
>      g_autofree struct vfio_device_info *info = NULL;
> +    struct vfio_info_cap_header *hdr;
> +    struct vfio_device_info_cap_pci_atomic_comp *cap;
> +    uint32_t mask = 0;
> +
> +    info = vfio_get_device_info(vdev->vbasedev.fd);
> +    if (!info) {
> +        return mask;
> +    }
> +
> +    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
> +    if (!hdr) {
> +        return mask;
> +    }
> +
> +    cap = (void *)hdr;
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
> +    }
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
> +    }
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
> +    }
> +
> +    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);
> +        uint32_t func_mask = 0;
> +
> +        if (!func_dev) {
> +            continue;
> +        }
> +
> +        func_vdev = (VFIOPCIDevice *)object_dynamic_cast(OBJECT(func_dev),
> +                                                            TYPE_VFIO_PCI);
> +        if (!func_vdev) {
> +            continue;
> +        }

Why is it justified to continue here?  It at least seems worthy of a comment why we can ignore non-vfio-pci devices relative to the advertised atomic op support.

> +
> +        func_mask = vfio_get_atomic_cap(func_vdev);
> +
> +        common_mask &= func_mask;

Factor out func_mask.

> +    }
> +
> +    return common_mask;
> +}
> +
> +static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev) {
>      PCIBus *bus = pci_get_bus(&vdev->pdev);
>      PCIDevice *parent = bus->parent_dev;
> -    struct vfio_info_cap_header *hdr;
>      uint32_t mask = 0;
>      uint8_t *pos;
>
>      /*
>       * 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;
>      }
>
> @@ -1934,25 +1994,15 @@ static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
>          return;
>      }
>
> -    info = vfio_get_device_info(vdev->vbasedev.fd);
> -    if (!info) {
> -        return;
> -    }
> -
> -    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
> -    if (!hdr) {
> -        return;
> -    }
> -
> -    cap = (void *)hdr;
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
> -    }
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
> -    }
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;

Nit, it would have been cleaner to factor out this helper as a precursor to multifunction support.  Thanks,

Alex

> +    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) {
Re: [PATCH] vfio/pci: Enable atomic ops for multifunction devices
Posted by Alex Williamson 4 days, 7 hours ago
On Mon, 2 Feb 2026 09:11:28 +0000
"Pekovic, Manojlo" <Manojlo.Pekovic@amd.com> 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' <alex@shazbot.org>; 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>
> Cc: Prica, Nikola <Nikola.Prica@amd.com>; Andjelkovic, Dejan <Dejan.Andjelkovic@amd.com>; 'Cédric Le Goater' <clg@redhat.com>
> 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 <manojlo.pekovic@amd.com>
> ---
>  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
Re: [PATCH] vfio/pci: Enable atomic ops for multifunction devices
Posted by Cédric Le Goater 4 days, 16 hours ago
Hello,

On 2/2/26 10:11, Pekovic, Manojlo wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Alex,
> 
> Is there any new info on this issue?


Please send a v2 of the patch.

Thanks,

C.

> 
> 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' <alex@shazbot.org>; 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>
> Cc: Prica, Nikola <Nikola.Prica@amd.com>; Andjelkovic, Dejan <Dejan.Andjelkovic@amd.com>; 'Cédric Le Goater' <clg@redhat.com>
> 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 <manojlo.pekovic@amd.com>
> ---
>   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);
> +
> +        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
> 
> 
> Thanks and Best Regards,
> Manojlo Pekovic
> Software Development Engineer 2
> Cloud Software Team
> 
> 
> 
> -----Original Message-----
> From: Pekovic, Manojlo
> Sent: Tuesday, January 20, 2026 5:00 PM
> To: 'Alex Williamson' <alex@shazbot.org>; qemu-devel@nongnu.org
> Cc: Prica, Nikola <Nikola.Prica@amd.com>; Andjelkovic, Dejan <Dejan.Andjelkovic@amd.com>; Cédric Le Goater <clg@redhat.com>
> Subject: RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices
> 
> + others, on accident
> 
> 
> Manojlo Pekovic
> Software Development Engineer 2
> Cloud Software Team
> 
> 
> 
> -----Original Message-----
> From: Pekovic, Manojlo
> Sent: Tuesday, January 20, 2026 4:59 PM
> To: Alex Williamson <alex@shazbot.org>
> Subject: RE: [PATCH] vfio/pci: Enable atomic ops for multifunction devices
> 
> Hi Alex,
> Hope you are good. Sorry for such a late response.
> 
> Appreciate the comments
> 
> I have split up the fix into two patches as you said, and will be sending them into two emails so it's easier for you to check them In this mail, I am sending the helper patch and in the next one the multifunction
> 
> Extract the code that reads and converts VFIO atomic capabilities into a separate helper function. This is a preparatory refactor with no functional change, making the code easier to extend for multifunction device support.
> 
> Signed-off-by: Manojlo Pekovic <manojlo.pekovic@amd.com>
> ---
>   hw/vfio/pci.c | 53 ++++++++++++++++++++++++++++++---------------------
>   1 file changed, 31 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 14bcc725c3..6a6c8f1807 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1900,13 +1900,41 @@ static void vfio_add_emulated_long(VFIOPCIDevice *vdev, int pos,
>       vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask);  }
> 
> -static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
> +static uint32_t vfio_get_atomic_cap(VFIOPCIDevice *vdev)
>   {
>       struct vfio_device_info_cap_pci_atomic_comp *cap;
>       g_autofree struct vfio_device_info *info = NULL;
> +    struct vfio_info_cap_header *hdr;
> +    uint32_t mask = 0;
> +
> +    info = vfio_get_device_info(vdev->vbasedev.fd);
> +    if (!info) {
> +        return mask;
> +    }
> +
> +    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
> +    if (!hdr) {
> +        return mask;
> +    }
> +
> +    cap = (void *)hdr;
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
> +    }
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
> +    }
> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
> +    }
> +
> +    return mask;
> +}
> +
> +static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev) {
>       PCIBus *bus = pci_get_bus(&vdev->pdev);
>       PCIDevice *parent = bus->parent_dev;
> -    struct vfio_info_cap_header *hdr;
>       uint32_t mask = 0;
>       uint8_t *pos;
> 
> @@ -1934,26 +1962,7 @@ static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
>           return;
>       }
> 
> -    info = vfio_get_device_info(vdev->vbasedev.fd);
> -    if (!info) {
> -        return;
> -    }
> -
> -    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
> -    if (!hdr) {
> -        return;
> -    }
> -
> -    cap = (void *)hdr;
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
> -    }
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
> -    }
> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
> -    }
> +    mask = vfio_get_atomic_cap(vdev);
> 
>       if (!mask) {
>           return;
> --
> 2.43.0
> 
> 
> 
> Manojlo Pekovic
> Software Development Engineer 2
> Cloud Software Team
> 
> 
> 
> -----Original Message-----
> From: Alex Williamson <alex@shazbot.org>
> Sent: Monday, January 12, 2026 5:17 PM
> To: Pekovic, Manojlo <Manojlo.Pekovic@amd.com>
> Cc: qemu-devel@nongnu.org; alex.williamson@redhat.com; clg@redhat.com; Prica, Nikola <Nikola.Prica@amd.com>; Andjelkovic, Dejan <Dejan.Andjelkovic@amd.com>
> Subject: Re: [PATCH] vfio/pci: Enable atomic ops for multifunction devices
> 
> On Tue, 9 Dec 2025 14:32:50 +0000
> Manojlo Pekovic <manojlo.pekovic@amd.com> wrote:
> 
>> 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 <manojlo.pekovic@amd.com>
>> ---
>>   hw/vfio/pci.c | 104
>> +++++++++++++++++++++++++++++++++++++-------------
>>   1 file changed, 77 insertions(+), 27 deletions(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index
>> 14bcc725c3..9d1faeabff 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -1900,28 +1900,88 @@ static void vfio_add_emulated_long(VFIOPCIDevice *vdev, int pos,
>>       vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask);
>> }
>>
>> -static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
>> +static uint32_t vfio_get_atomic_cap(VFIOPCIDevice *vdev)
>>   {
>> -    struct vfio_device_info_cap_pci_atomic_comp *cap;
>>       g_autofree struct vfio_device_info *info = NULL;
>> +    struct vfio_info_cap_header *hdr;
>> +    struct vfio_device_info_cap_pci_atomic_comp *cap;
>> +    uint32_t mask = 0;
>> +
>> +    info = vfio_get_device_info(vdev->vbasedev.fd);
>> +    if (!info) {
>> +        return mask;
>> +    }
>> +
>> +    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
>> +    if (!hdr) {
>> +        return mask;
>> +    }
>> +
>> +    cap = (void *)hdr;
>> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
>> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
>> +    }
>> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
>> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
>> +    }
>> +    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
>> +        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
>> +    }
>> +
>> +    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);
>> +        uint32_t func_mask = 0;
>> +
>> +        if (!func_dev) {
>> +            continue;
>> +        }
>> +
>> +        func_vdev = (VFIOPCIDevice *)object_dynamic_cast(OBJECT(func_dev),
>> +                                                            TYPE_VFIO_PCI);
>> +        if (!func_vdev) {
>> +            continue;
>> +        }
> 
> Why is it justified to continue here?  It at least seems worthy of a comment why we can ignore non-vfio-pci devices relative to the advertised atomic op support.
> 
>> +
>> +        func_mask = vfio_get_atomic_cap(func_vdev);
>> +
>> +        common_mask &= func_mask;
> 
> Factor out func_mask.
> 
>> +    }
>> +
>> +    return common_mask;
>> +}
>> +
>> +static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev) {
>>       PCIBus *bus = pci_get_bus(&vdev->pdev);
>>       PCIDevice *parent = bus->parent_dev;
>> -    struct vfio_info_cap_header *hdr;
>>       uint32_t mask = 0;
>>       uint8_t *pos;
>>
>>       /*
>>        * 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;
>>       }
>>
>> @@ -1934,25 +1994,15 @@ static void vfio_pci_enable_rp_atomics(VFIOPCIDevice *vdev)
>>           return;
>>       }
>>
>> -    info = vfio_get_device_info(vdev->vbasedev.fd);
>> -    if (!info) {
>> -        return;
>> -    }
>> -
>> -    hdr = vfio_get_device_info_cap(info, VFIO_DEVICE_INFO_CAP_PCI_ATOMIC_COMP);
>> -    if (!hdr) {
>> -        return;
>> -    }
>> -
>> -    cap = (void *)hdr;
>> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP32) {
>> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP32;
>> -    }
>> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP64) {
>> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP64;
>> -    }
>> -    if (cap->flags & VFIO_PCI_ATOMIC_COMP128) {
>> -        mask |= PCI_EXP_DEVCAP2_ATOMIC_COMP128;
> 
> Nit, it would have been cleaner to factor out this helper as a precursor to multifunction support.  Thanks,
> 
> Alex
> 
>> +    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) {
>