[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 <[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); + + 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' <[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 + 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 <[email protected]> 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 <[email protected]> --- 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 <[email protected]> Sent: Monday, January 12, 2026 5:17 PM To: Pekovic, Manojlo <[email protected]> Cc: [email protected]; [email protected]; [email protected]; Prica, Nikola <[email protected]>; Andjelkovic, Dejan <[email protected]> Subject: Re: [PATCH] vfio/pci: Enable atomic ops for multifunction devices On Tue, 9 Dec 2025 14:32:50 +0000 Manojlo Pekovic <[email protected]> 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 <[email protected]> > --- > 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) {
